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

Create "Switching from kube-lego" user guide #136

Closed
munnerz opened this issue Oct 22, 2017 · 5 comments · Fixed by #264
Closed

Create "Switching from kube-lego" user guide #136

munnerz opened this issue Oct 22, 2017 · 5 comments · Fixed by #264
Labels
kind/documentation Categorizes issue or PR as related to documentation.
Projects

Comments

@munnerz
Copy link
Member

munnerz commented Oct 22, 2017

We should provide guides on how to switch from kube-lego to help users migrate their existing deployments.

I'm not 100% sure if kube-lego stores the ACME account URI anywhere once it has registered an account, and so far I've not been able to find any way to look up an existing registration URI for a private key.

We use this URI in order to check if an account is valid (here: https://github.com/jetstack-experimental/cert-manager/blob/master/pkg/issuer/acme/setup.go#L58).

@cpu do you have any advice how I can obtain a registration URI for a private key that is already registered with an ACME server?

/kind documentation

@munnerz munnerz added this to Backlog in v1.0.0 Oct 22, 2017
@jetstack-bot jetstack-bot added the kind/documentation Categorizes issue or PR as related to documentation. label Oct 23, 2017
@cpu
Copy link
Contributor

cpu commented Oct 24, 2017

@cpu do you have any advice how I can obtain a registration URI for a private key that is already registered with an ACME server?

@munnerz With the current Boulder implementation you'll want to follow the advice of draft-06 Section 7.3 and POST a new-reg request with the public key of the private key in question. If an account already exists there will be a conflict response with a Location header that contains the existing account URI.

It's a little bit awkward because the only way to check if an account exists is to try and create it, so if it doesn't exist, it will after you check!

Draft-07 cleans this up (See Section 7.3.1) by adding a only-return-existing parameter that can be used to prevent account creation if the key isn't already in use. We don't implement that presently in Boulder - it will be part of our "ACME v2" effort.

Hope that helps!

@munnerz
Copy link
Member Author

munnerz commented Oct 24, 2017

Ah excellent - this should mean we can offer a seamless transition to cert-manager, so long as the user has retained their private key 😄

It also means that we can accept arbitrary existing private keys/registrations from users if they want to start using cert-manager after they initially don't. The issue of "check or create" isn't such a problem for us right now.

Thanks once again for the advice!

@cpu
Copy link
Contributor

cpu commented Oct 24, 2017

Happy to help!

@munnerz
Copy link
Member Author

munnerz commented Oct 27, 2017

@cpu I've been looking into implementing this in cert-manager, and have found a discrepancy between the spec listed on the draft, and with how the LE staging server (and I expect the prod one, although not checked) is behaving.

Specifically:

 If the server already has an account registered with the provided
   account key, then it MUST return a response with a 200 (OK) status
   code and provide the URI of that account in the Location header
   field.  This allows a client that has an account key but not the
   corresponding account URI to recover the account URI.

On calls to Register, I'm actually seeing a 409 error returned (that is, a 409 HTTP status code), with the following content:

{
  "type": "urn:acme:error:malformed",
  "detail": "Registration key is already in use",
  "status": 409
}

This seems odd? Is that an old draft of the specification? If so, the change in behaviour could definitely break some clients. That said, I've never known it to not return a 409 error.

Worth noting that the Location header is set on the response to a registration request, but the status code is incorrect and the actual account resource is not included in the body of the response (although the spec doesn't specify that this should be the case, so maybe that's expected).

@cpu
Copy link
Contributor

cpu commented Oct 27, 2017

@munnerz Apologies, I should have also linked to the Boulder divergences doc for this:

Boulder uses an HTTP status code 409 (Conflict) response for an already existing registration instead of 200 (OK). Boulder returns the URI of the already existing registration in a Location header field instead of a Content-Location header field.

You can assume that won't change for the V1 API.

the actual account resource is not included in the body of the response (although the spec doesn't specify that this should be the case, so maybe that's expected).

I believe that's expected 👍

jetstack-ci-bot added a commit that referenced this issue Jan 18, 2018
Automatic merge from submit-queue. If you want to cherry-pick this change to another branch, please follow the instructions <a href="https://github.com/kubernetes/community/blob/master/contributors/devel/cherry-picks.md">here</a>.

Expand deployment docs, including docs on ingress-shim

**What this PR does / why we need it**:

Improves our documentation to further explain how Helm deployment works (including RBAC and extraArgs).

It also adds a doc on ingress-shim - what it does, how it works, how to configure it and how to use it.

**Which issue this PR fixes** *(optional, in `fixes #<issue number>(, fixes #<issue_number>, ...)` format, will close that issue when PR gets merged)*: fixes #

Fixes #256, fixes #257, fixes #136 

**Release note**:
```release-note
Improve deployment documentation
```

/cc @ahmetb
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/documentation Categorizes issue or PR as related to documentation.
Projects
No open projects
v1.0.0
Backlog
Development

Successfully merging a pull request may close this issue.

3 participants