Skip to content

fix(ssl): Add --non-interactive flag to certbot commands#109

Merged
nfebe merged 4 commits intomainfrom
fix/certbot-non-interactive
Mar 27, 2026
Merged

fix(ssl): Add --non-interactive flag to certbot commands#109
nfebe merged 4 commits intomainfrom
fix/certbot-non-interactive

Conversation

@nfebe
Copy link
Copy Markdown
Contributor

@nfebe nfebe commented Mar 27, 2026

Certbot fails with EOF error when run via docker without a TTY because it tries to prompt for Terms of Service confirmation. Adding --non-interactive prevents this and allows the webroot challenge to complete successfully.

Also:

  • Make SSL manager executor injectable for testability
  • Gracefully skip SSL when certbot email is not configured
  • Accept certbot_email in setup wizard settings endpoint
  • Add ACME challenge location to nginx default server

Also resolves: #104, #108

@sourceant
Copy link
Copy Markdown

sourceant bot commented Mar 27, 2026

Code Review Summary

This PR fixes a critical issue where Certbot would hang in non-TTY environments (like Docker) by adding the --non-interactive flag. It also improves the flexibility of the SSL manager and enhances Docker service discovery to support the expose directive.

🚀 Key Improvements

  • Added --non-interactive to all Certbot commands to prevent EOF errors in Docker.
  • Introduced ServiceExecutor interface for SSL manager to allow proper unit testing of shell commands.
  • Implemented support for the expose key in Docker Compose files for automatic port detection.
  • Added email validation for Certbot settings using the net/mail package.

💡 Minor Suggestions

  • Add logging suppression for ACME challenges in Nginx config.
  • Ensure errors from SaveMetadata are propagated during compose updates.

Copy link
Copy Markdown

@sourceant sourceant bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Review complete. See the overview comment for a summary.

nfebe added 3 commits March 27, 2026 14:25
Certbot fails with EOF error when run via docker without a TTY
because it tries to prompt for Terms of Service confirmation.
Adding --non-interactive prevents this and allows the webroot
challenge to complete successfully.

Also:
- Make SSL manager executor injectable for testability
- Gracefully skip SSL when certbot email is not configured
- Accept certbot_email in setup wizard settings endpoint
- Add ACME challenge location to nginx default server

Signed-off-by: nfebe <fenn25.fn@gmail.com>
Nextcloud template was missing the required top-level name field
and used direct port mapping instead of the proxy network.
Aligned with other templates: use expose, container_name, and
proxy network for nginx reverse proxying.

Signed-off-by: nfebe <fenn25.fn@gmail.com>
The compose expose field was not parsed, so changes to it never
propagated to service.yml. Added Expose to composeService struct,
use it for container port extraction and primary service detection,
and regenerate service.yml metadata when compose file is updated.

Closes #104

Signed-off-by: nfebe <fenn25.fn@gmail.com>
@nfebe nfebe force-pushed the fix/certbot-non-interactive branch from 7935b8e to 941f1d1 Compare March 27, 2026 13:42
Copy link
Copy Markdown

@sourceant sourceant bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Review complete. See the overview comment for a summary.

Signed-off-by: nfebe <fenn25.fn@gmail.com>
Copy link
Copy Markdown

@sourceant sourceant bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Review complete. No specific code suggestions were generated. See the overview comment for a summary.

@nfebe nfebe merged commit 6166f4e into main Mar 27, 2026
2 checks passed
@nfebe nfebe deleted the fix/certbot-non-interactive branch March 27, 2026 13:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

bug: Updating expose in deployment compose does not update service.yml

1 participant