acme_certificate: Allow to disable account management for certificate module #40698
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
ansibot
added
affects_2.6
resmo
reviewed
May 25, 2018
| update_contact=modify_account | ||
| ) | ||
| else: | ||
| # This happens iff modify_account is False and the ACME v1 |
There was a problem hiding this comment.
typo, and I see a discrepancy in the sentence: "This happens if modify account is False or the ACME v1 is used
There was a problem hiding this comment.
Do you mean "iff" with typo? It's not a typo ;) (https://en.wikipedia.org/wiki/Iff) I'll change it for clarity.
About the second: not (modify_account or self.version > 1) is not modify_account and self.version == 1, so this should be correct.
There was a problem hiding this comment.
I've changed it to "if" since "if and only if" is too long ;) I've also rebased to get rid of the conflicts (?).
ansibot
added
needs_rebase
felixfontein
force-pushed
the
acme-certificate-allow-not-to-change-account
branch
from
9bfa4b5
to
d9d6ef2
Compare
ansibot
added
community_review
ansibot
added
automerge
|
Also thanks for this one! |
gothicx
pushed a commit
to gothicx/ansible
that referenced
this pull request
Jun 9, 2018
jacum
pushed a commit
to jacum/ansible
that referenced
this pull request
Jun 26, 2018
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
SUMMARY
Follow-up to #37275: adds a feature (disable account management) to
acme_certificatemodule which makes the newacme_accountmore useful (see below for detailed info why we need this feature).ISSUE TYPE
COMPONENT NAME
acme_certificate
ANSIBLE VERSION
ADDITIONAL INFORMATION
Addition of the new
acme_accountmodule in #37275 allows to do more precise account management: deactivating an account, changing an account key, and updating contacts (also to more than one).There are two things where this is really needed:
In case you modify the account key, and then try to create a certificate with the old account key, it can happen (if the CA doesn't prevent this) that the
acme_certificatemodule will create a new account with the old key. This is most certainly not what you want. Thus, if you want to do all account management with the new module, you can use this option to tellacme_certificateto not touch the account.In case you want to have more than one contact address for the account, you can use the
acme_accountmodule to set them. Unfortunately, theacme_certificatemodule doesn't support this, and as soon as you use it to issue a new certificate, it will notice that the account has a different number of contacts than it has itself, and will adjust the account so it only contains the email address contact specified to it (or no contact at all). With this option, you can disable this behavior.The default value for this new option is to continue doing account management; this is both important for backwards compatibility and to not make it too complicated for beginners (which can only use the
acme_certificatemodule, and don't have to bother about theacme_accountmodule).