Skip to content

FIX: default enable_ssl to be 1 #985

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Jul 29, 2025
Merged
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions templates/web.ssl.template.yml
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
env:
ENABLE_SSL: 1
Comment on lines +1 to +2
Copy link
Contributor

Choose a reason for hiding this comment

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

I don't quite understand why need to include this ENV flag to enable SSL when the fact that this template is included means SSL should be enabled.

Copy link
Member Author

Choose a reason for hiding this comment

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

I'm attempting to break that assumption a bit and allow it to be configurable at runtime

IE go from: "including this template enables ssl unconditionally"
to: "including this template allows ssl to be configured optionally"

This makes it easier for anyone to build and ship pre-bootstrapped images that can be configured at runtime, with no launcher required.

Copy link
Contributor

Choose a reason for hiding this comment

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

to: "including this template allows ssl to be configured optionally"

But this is only true if the template doesn't set ENABLE_SSL for the user. With this change, it seems like we are trying to support the case where a user includes the SSL templates but wants to disable it by passing ENABLE_SSL to 0 at container run time?

Copy link
Contributor

@tgxworld tgxworld Jul 29, 2025

Choose a reason for hiding this comment

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

This makes it easier for anyone to build and ship pre-bootstrapped images that can be configured at runtime, with no launcher required.

If this is the goal, perhaps we can specify a directory which we will look into for SSL certificates in the install-ssl script? If the certificates are present, do the necessary configuration update to nginx. I kind of feel like a ENABLE_SSL knob is unnecessary when we can already make a decision based on the presence of SSL certificates.

Copy link
Member Author

Choose a reason for hiding this comment

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

This particular change was an attempt at mirroring what I did for letsencrypt (via LETSENCRYPT_ACCOUNT_EMAIL), but detecting the presence of an ssl cert makes sense as a better knob to me, I'm game with working in that direction. 👍

Merging this to resolve the immediate issue in the meantime.

run:
- file:
path: /etc/runit/1.d/install-ssl
Expand Down
Loading