-
Notifications
You must be signed in to change notification settings - Fork 23.7k
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
Allowing to write intermediate certificate into file. #34328
Conversation
The test
|
@@ -92,6 +92,13 @@ | |||
description: The destination file for the certificate. | |||
required: true | |||
aliases: ['cert'] | |||
chain_dest: |
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.
Any reason you have the primary name as chain_dest
and an alias of chain
, then use the alias in the examples
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 primary name for the certificate filename is dest
, with an alias of cert
. That's why I used chain_dest
as primary and chain
as an alias.
The examples don't use cert
either (but dest
); since there's basically only one example (with three alternatives for the first step), either the primary or the alias should be used in all examples.
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.
Now that there's a second example using the aliases (see #35077), I've added this alias there as well. (I amended it into the existing commit.)
@@ -855,6 +868,7 @@ def main(): | |||
data=dict(required=False, no_log=True, default=None, type='dict'), | |||
fullchain=dict(required=False, default=True, type='bool'), | |||
dest=dict(required=True, aliases=['cert'], type='path'), | |||
chain_dest=dict(required=False, deafult=None, aliases=['chain'], type='path'), |
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.
typo of default
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.
Thanks! Fixed that.
@felixfontein This PR contains |
d459f65
to
cd66266
Compare
ready_for_review |
cd66266
to
1d40443
Compare
064885b
to
dbc6526
Compare
Hmm, the failing test doesn't seem to be related to this PR:
|
dbc6526
to
8fec5c7
Compare
Yep, "random fluke". Fixed by rebase without change... |
looks good to me. |
Thanks! |
* Allowing to write intermediate certificate into file. * Fixing merge errors introduced in 25b1db5 (ansible#34916).
SUMMARY
Allows the
letsencrypt
module to export the intermediate certificate to a file.Fixes #18996 and extends #22074 (which allows to only write one combined file with certificate and intermediate certificate).
ISSUE TYPE
COMPONENT NAME
letsencrypt
ANSIBLE VERSION
ADDITIONAL INFORMATION
The original issue #18996 asked for a way to get hold of the intermediate certificate. Since that one is not only needed to create the full chain (i.e. concatenation of certificate and intermediate, which is sent by the webserver to the client so it can validate the certificate), but also to create the root chain (root and intermediate certificate, which is used by the web server to validate OCSP responses to be stapled), the fix in #22074 is not sufficient (without extra processing). (See comment by @8w in original issue.)
This PR allows to write the intermediate certificate to a separate file, so it can be combined with whatever the caller wants (and not just the certificate).