-
Notifications
You must be signed in to change notification settings - Fork 781
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
Issue 1588: pki cert renew bug + duration/renew logic fixes #1590
Merged
+90
−39
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
The code handled the case of getting a cert and using the version on the disk as the cache but missed looping over that logic when the one on disk had expired. This adds the looping needed to renew the cert when expired and extends the refetching test to validate this functionality.
swenson
reviewed
Jun 16, 2022
swenson
approved these changes
Jun 16, 2022
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; I verified locally that this resolves the Vault agent render bug.
mkam
approved these changes
Jun 17, 2022
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.
Looks good to me!
The goodFor() method previously was extremely simple and only used 90% of the certificate lifespan. This had several logic holes in it around edge cases where this new version addresses them.
eikenb
force-pushed
the
issue-1588-pki-cert-renew-bug
branch
from
June 17, 2022 21:22
599218a
to
a6d1e80
Compare
swenson
pushed a commit
to hashicorp/vault
that referenced
this pull request
Jun 21, 2022
So that we get the fix in hashicorp/consul-template#1590. I tested manually that this no longer causes `pkiCert` to get into an infinite failure loop when the cert expires.
swenson
added a commit
to hashicorp/vault
that referenced
this pull request
Jun 27, 2022
Update consul-template to latest for pkiCert fix So that we get the fixes in hashicorp/consul-template#1590 and hashicorp/consul-template#1591. I tested manually that this no longer causes `pkiCert` to get into an infinite failure loop when the cert expires, and that the key and CA certificate are also accessible. Co-authored-by: Theron Voran <[email protected]>
swenson
added a commit
to hashicorp/vault
that referenced
this pull request
Jun 27, 2022
cherry-picked c165363 Update consul-template to latest for pkiCert fix So that we get the fixes in hashicorp/consul-template#1590 and hashicorp/consul-template#1591. I tested manually that this no longer causes `pkiCert` to get into an infinite failure loop when the cert expires, and that the key and CA certificate are also accessible. Co-authored-by: Theron Voran <[email protected]>
swenson
added a commit
to hashicorp/vault
that referenced
this pull request
Jun 27, 2022
cherry-picked c165363 Update consul-template to latest for pkiCert fix So that we get the fixes in hashicorp/consul-template#1590 and hashicorp/consul-template#1591. I tested manually that this no longer causes `pkiCert` to get into an infinite failure loop when the cert expires, and that the key and CA certificate are also accessible. Co-authored-by: Theron Voran <[email protected]> Co-authored-by: Christopher Swenson <[email protected]> Co-authored-by: Theron Voran <[email protected]>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
The code handled the case of getting a cert and using the version on the disk as the cache but missed looping over that logic when the one on disk had expired.
This adds the looping needed to renew the cert when expired and extends the refetch test to validate this functionality.
The second commit could be split out as it's not directly fixing the linked bug, but it is all part of this same code and is needed for this feature to work as intended so I'm including it.
Fixes #1588