-
-
Notifications
You must be signed in to change notification settings - Fork 5.2k
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
Force to use ACME v02 endpoint. #3358
Conversation
731c084
to
b64c600
Compare
configuration/configuration.go
Outdated
|
||
if strings.Contains(gc.ACME.CAServer, "v01") { | ||
caServer := strings.Replace(gc.ACME.CAServer, "v01", "v02", 1) | ||
log.Warn("the CA server %q refer to a v01 endpoint of the ACME API, please change to %q.", gc.ACME.CAServer, caServer) |
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.
The log entry doesn't mention that you swapped v01 for v02. it is possible that for some reason, someone may want to use a CA server that contains v01
in the URL. I would test for a more exact match, or just warn in the logs.
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.
Few suggestions.
configuration/configuration.go
Outdated
|
||
if strings.HasPrefix(caServerSrc, "https://acme-v01.api.letsencrypt.org") { | ||
caServer := strings.Replace(caServerSrc, "v01", "v02", 1) | ||
log.Warnf("The CA server %q refer to a v01 endpoint of the ACME API, please change to %q.", caServerSrc, caServer) |
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.
s/refer/refers
No sure users will understand that a modification will be automatically done.
WDYT to use a more explicit message?
configuration/configuration.go
Outdated
if strings.HasPrefix(caServerSrc, "https://acme-v01.api.letsencrypt.org") { | ||
caServer := strings.Replace(caServerSrc, "v01", "v02", 1) | ||
log.Warnf("The CA server %q refer to a v01 endpoint of the ACME API, please change to %q.", caServerSrc, caServer) | ||
log.Warnf("ACME CA server fallback to %q.", caServer) |
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.
Why are you using 2 logs instructrions?
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.
LGTM
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.
LGTM 👍
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.
LGTM
98270c8
to
e629222
Compare
What does this PR do?
Force to use ACME v02 endpoint: rewrite API URL if needed.
Motivation
Have a simple configuration.
Related to #3355