Skip to content
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

Fix /intermediate/set-signed, /config/ca import and other minor test scenarios #14999

Closed
wants to merge 4 commits into from

Conversation

cipherboy
Copy link
Contributor

@cipherboy cipherboy commented Apr 11, 2022

Based on top of #14975 and #14960 and #14995; will be rebased after the first merges. Only the top 3 commits are new.


We make three changes:

  • Update the two import paths to reuse the issuer-import logic (/config/ca and /intermediate/set-signed).
  • Fix the error message on missing default issuer.
  • Fix tests related to imports &c to handle the new semantics.

We'll still need to update the root deletion code as well.

stevendpclark and others added 4 commits April 12, 2022 10:19
 - Addresses some test failures due to an incorrect refactoring of a legacy api
   path /sign-verbatim within PKI
The existing bundle import code will satisfy the intermediate import;
use it instead of the old ca_bundle import logic. Additionally, update
/config/ca to use the new import code as well.

While testing, a panic was discovered:

> reflect.Value.SetMapIndex: value of type string is not assignable to type pki.keyId

This was caused by returning a map with type issuerId->keyId; instead
switch to returning string->string maps so the audit log can properly
HMAC them.

Signed-off-by: Alexander Scheel <[email protected]>
When the default issuer and key are missing (and haven't yet been
specified), we should clarify that error message.

Signed-off-by: Alexander Scheel <[email protected]>
This makes two minor changes to the existing test suite:

 1. Importing partial bundles should now succeed, where they'd
    previously error.
 2. fetchCertBySerial no longer handles CA certificates.

Signed-off-by: Alexander Scheel <[email protected]>
@cipherboy cipherboy force-pushed the cipherboy-fix-set-signed branch from 95203ac to f7422e1 Compare April 12, 2022 14:22
@vercel vercel bot temporarily deployed to Preview – vault-storybook April 12, 2022 14:22 Inactive
@vercel vercel bot temporarily deployed to Preview – vault April 12, 2022 14:22 Inactive
@cipherboy cipherboy changed the base branch from pki-pod-rotation to cipherboy-build-cert-path April 12, 2022 14:22
Copy link
Contributor

@stevendpclark stevendpclark left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm!

@cipherboy
Copy link
Contributor Author

Thanks! Manually merging...

@cipherboy cipherboy closed this Apr 12, 2022
@cipherboy cipherboy deleted the cipherboy-fix-set-signed branch May 17, 2022 14:34
@cipherboy
Copy link
Contributor Author

This PR was merged in #15277. See that PR and the relevant docs PR #15238 for more information about this change.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants