Skip to content
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

Merged
merged 5 commits into from
May 22, 2018
Merged

Conversation

ldez
Copy link
Contributor

@ldez ldez commented May 20, 2018

What does this PR do?

Force to use ACME v02 endpoint: rewrite API URL if needed.

Motivation

Have a simple configuration.

Related to #3355

@traefiker traefiker added this to the 1.6 milestone May 20, 2018
@ldez ldez force-pushed the refactor/force-acme-v02 branch from 731c084 to b64c600 Compare May 20, 2018 18:01

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)
Copy link
Contributor

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.

Copy link
Contributor

@nmengin nmengin left a comment

Choose a reason for hiding this comment

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

@ldez

Few suggestions.


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)
Copy link
Contributor

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?

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)
Copy link
Contributor

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?

Copy link
Member

@juliens juliens left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link
Contributor

@nmengin nmengin left a comment

Choose a reason for hiding this comment

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

LGTM 👍

Copy link
Contributor

@dtomcej dtomcej left a comment

Choose a reason for hiding this comment

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

LGTM
:shipit:

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants