Trap errors related to vault pki list-intermediate issuer reading #19165
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
- Add a little more checks/validation when loading the a PKI issuer - Factor out the issuer loading into a dedicated function - Leverage existing health check code to parse issuer certificates
- Read in our parent issuer once instead of running it for every child we want to compare against - Provides clearer error message that we have failed reading from which path to the end user
kitography
reviewed
Feb 13, 2023
command/pki_reissue_intermediate.go
Outdated
| intermediateMount := sanitizePath(args[2]) | ||
|
|
||
| templateCertificateResp, err := client.Logical().Read(sanitizePath(args[1])) | ||
| issuerBundle, err := readIssuer(client, templateIssuer) |
There was a problem hiding this comment.
could we rename this templateIssuerBundle? I'm worried that this is quite a bit confusing with the parent issuer also being one of the arguments.
kitography
approved these changes
Feb 13, 2023
- Rename a variable for clarity - Use readIssuer in the validation of the parent issuer within pkiIssuer - Add some missing return 1 statements in error handlers that had been missed
cipherboy
approved these changes
Feb 14, 2023
stevendpclark
added a commit
that referenced
this pull request
Feb 14, 2023
…9165) * Rename files to match test suite and existing pattern * Factor out issuer loading into a dedicated function - Add a little more checks/validation when loading the a PKI issuer - Factor out the issuer loading into a dedicated function - Leverage existing health check code to parse issuer certificates * Read parent issuer once instead of reloading it for every child - Read in our parent issuer once instead of running it for every child we want to compare against - Provides clearer error message that we have failed reading from which path to the end user * PR Feedback - Rename a variable for clarity - Use readIssuer in the validation of the parent issuer within pkiIssuer - Add some missing return 1 statements in error handlers that had been missed
stevendpclark
added a commit
that referenced
this pull request
Feb 14, 2023
…9165) (#19177) * Rename files to match test suite and existing pattern * Factor out issuer loading into a dedicated function - Add a little more checks/validation when loading the a PKI issuer - Factor out the issuer loading into a dedicated function - Leverage existing health check code to parse issuer certificates * Read parent issuer once instead of reloading it for every child - Read in our parent issuer once instead of running it for every child we want to compare against - Provides clearer error message that we have failed reading from which path to the end user * PR Feedback - Rename a variable for clarity - Use readIssuer in the validation of the parent issuer within pkiIssuer - Add some missing return 1 statements in error handlers that had been missed Co-authored-by: Steven Clark <steven.clark@hashicorp.com>
stevendpclark
deleted the
stevendpclark/vault-13588-cli-command-input
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.
Description
Discovered during testing if an invalid parent issuer url was provided to the list-intermediate pki sub-command we would get a nil exception.
What was modified
Encountered error being addressed