PKI read legacy CA bundle prior to migration #15120
Merged
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
cipherboy
approved these changes
Apr 22, 2022
| // and reset its compatibility mode. | ||
| b.updatePkiStorageVersion(ctx) | ||
| } | ||
| // FIXME: We need to hook into CRL generation here for issuer/bundle updates. |
There was a problem hiding this comment.
Since we merged the CRL stuff, a call to buildCRLs should suffice here. :-)
There was a problem hiding this comment.
Will defer to another PR, since we don't have a request object here this isn't as trivial as calling buildCRLs.
| if err != nil { | ||
| return err | ||
| return migrationInfo, err |
There was a problem hiding this comment.
If this has been deleted (or never created), does it mean we'll err out here?
There was a problem hiding this comment.
Ah, I think we don't err. Only on other storage issues.
There was a problem hiding this comment.
Correct, but this comment led me to a missing nil check within fetchCAInfo so thanks 👍
- Leverage an existing helper method within the PKI backend tests to setup a PKI backend with storage.
…red on secondaries. - Track the migration state forbidding an issuer/key writing api call if we have not migrated - For operations that just need to read the CA bundle, use the same tracking variable to switch between reading the legacy bundle or use the new key/issuer storage. - Add an invalidation function that will listen for updates to our log path to refresh the state on secondary clusters.
stevendpclark
force-pushed
the
stevendpclark/vault-5748-read-legacy-on-secondary
branch
2 times, most recently
from
771c042
to
19a2981
Compare
cipherboy
reviewed
Apr 25, 2022
cipherboy
approved these changes
Apr 25, 2022
- Some PR feedback and handle a case in which the primary cluster does not have a CA bundle within storage but somehow a secondary does.
stevendpclark
force-pushed
the
stevendpclark/vault-5748-read-legacy-on-secondary
branch
from
19a2981
to
0c53f24
Compare
stevendpclark
deleted the
stevendpclark/vault-5748-read-legacy-on-secondary
branch
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.
When the new Vault version starts up and becomes the leader node on a secondary performance cluster it should be able to still support certificate issue/revoke and list commands even when the storage was not migrated.
Known limitations
The new api on the secondary does not return the proper information in regards to issuers and keys as we don't fallback for those api calls.
Testing performed