- Notifications
You must be signed in to change notification settings - Fork3.1k
feat: boolean variable ACME_HTTP_CHALLENGE_LOCATION#2468
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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
feat: boolean variable ACME_HTTP_CHALLENGE_LOCATION#2468
Uh oh!
There was an error while loading.Please reload this page.
Conversation
buchdag commentedMay 30, 2024
@pini-gh the variable should be set to The priority for now is to avoid breaking existing setups (even if they insist on using |
Uh oh!
There was an error while loading.Please reload this page.
Uh oh!
There was an error while loading.Please reload this page.
| try_files $uri =404; | ||
| break; | ||
| } | ||
| {{- end }} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
| {{- end }} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
There are broken use cases with both defaults:
true
acme_companion configures an extraneous HTTP Challenge location block which is not valid.
false
Use cases where certificates are not generated with acme_companion loose HTTP Challenge configuration and may fail to renew their certificates.
Why not fixing acme_companion instead?
EDIT: The easiest way would be to keep the default astrue and update acme_companion so it wouldn't generate the HTTP Challenge configuration by default (but for standalone certificates).
EDIT1: moving this comment with was on the wrong thread.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
How about having 3 possible values forACME_HTTP_CHALLENGE?
legacy: previous incomplete behaviortrue: generate ACME HTTP Challenge location blocks whenHTTPS_METHODis in [redirect,noredirect] (current default behavior)false: do not generate ACME HTTP Challenge location blocks
And we set the new default tolegacy.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Thelegacy /true /false solution withlegacy being the default (until we change acme-companion default behaviour, which I plan on doing soon) seems perfect to me, if it's not too much additional work for you.
Why not fixing acme_companion instead?
Because I'm a bit short in time this week 😬 but that's planned, yes.
(by the way regarding acme-companion and it's location tinkering, I tried to automatically skip location configuration when not required a few years ago, I could not come up with anything else than very hacky and fragile solutions that did not even work in the end. Manually enabling or disabling location configuration as you suggest seem to be the only realistic way).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Minor request: could we useACME_HTTP_CHALLENGE_LOCATION instead ofACME_HTTP_CHALLENGE ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Done.
Uh oh!
There was an error while loading.Please reload this page.
d8495d1 tob5b1320CompareValues:* `legacy` (default): generate location blocks for ACME HTP Challenge excepted when `HTTPS_METHOD=noredirect` or there is no certificate for the domain* `true`: generate location blocks for ACME HTP Challenge in all cases* `false`: do not generate location blocks for ACME HTP ChallengeThis feature is currently needed because acme-companion may generatethe HTTP Challenge configuration while it was done already by nginx-proxy(seenginx-proxy#2465#issuecomment-2136361373).Also sometimes a hardcoded ACME challenge location is not wanted becausethe challenge validation is not done with acme-companion / Let's Encrypt,and with a challenge location setup differently.
b5b1320 to854427cCompareMontana commentedMay 30, 2024
Hi@pini-gh, Just an idea here for the location block: Perhaps set So you would have a server{server_name localhost.pwn.college;access_log /var/log/nginx/access.log vhost; http2 on;listen80 default_server;{{if eq(or(index$globals.Env"ACME_ENABLED")"false")"false"}}]location /.well-known/acme-challenge/{auth_basic off;allow all;root /usr/share/nginx/html;try_files$uri =404;break;}{{ end}}listen443ssl default_server;ssl_certificate /etc/nginx/certs/default.crt;ssl_certificate_key /etc/nginx/certs/default.key;if($https){return500;}include /etc/nginx/vhost.d/default;location /{proxy_passhttp://localhost.pwn.college;set$upstream_keepalive false;include /etc/nginx/vhost.d/default_location;}} Setting these environment variables, I'm obviously talking about Docker Compose: environment: -ACME_ENABLED=false# set to true in production This is only an idea, maybe it helps or gets something stirring. Have a wonderful Thursday. |
pini-gh commentedMay 31, 2024
All the uses cases are enabled by the current PR already:
|
Values:* `legacy` (default): generate location blocks for ACME HTP Challenge excepted when `HTTPS_METHOD=noredirect` or there is no certificate for the domain* `true`: generate location blocks for ACME HTP Challenge in all cases* `false`: do not generate location blocks for ACME HTP ChallengeThis feature is currently needed because acme-companion may generatethe HTTP Challenge configuration while it was done already by nginx-proxy(seenginx-proxy#2465#issuecomment-2136361373).Also sometimes a hardcoded ACME challenge location is not wanted becausethe challenge validation is not done with acme-companion / Let's Encrypt,and with a challenge location setup differently.
Enable / disable ACME HTTP Challenge blocks generation by nginx-proxy.
Default: true.
This feature is currently needed because acme-companion may generate the HTTP Challenge configuration while it was done already by nginx-proxy (see#2465#issuecomment-2136361373).
Also sometimes a hardcoded ACME challenge location is not wanted because the challenge validation is not done with acme-companion / Let's Encrypt, and with a challenge location setup differently.