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

Resolve TF state for PKI Multi-Issuer workflows #1973

Merged
merged 7 commits into from
Aug 23, 2023
Merged

Conversation

vinay-gopalan
Copy link
Contributor

This PR aims to address/fix the following issues:

  • Some Key Fields (key_name, key_ref) not being written to Vault due to inaccurate conditionals.
  • Nil check on Issuer Read that was causing a panic when reading from a fresh Vault server
  • Root certs were being destroyed and recreated due to the CustomizeDiff function not accounting for multi-issuer support. Previously, we were comparing the serials for all root certs to the default issuer's PEM certificate. The function has now been updated to read the PEM certificate for a specific issuer with an issuer_id and compare with the correct serial
  • Ensures all root cert and intermediate cert resources have unique IDs since there can now be multiple resources for each type.

Fixes #1943 #1944 #1968

Release note for CHANGELOG:

Resolve the TF state for PKI Multi-Issuer workflows.

if err != nil {
return err
var cert *x509.Certificate
isIssuerAPISupported := provider.IsAPISupported(meta, provider.VaultVersion111)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Prior to multi-issuer support (<= Vault 1.10), this customize diff would read the default CA in PEM format and compare to what was in the TF state. When there is only one issuer, this makes sense since it will be the default. However, for multiple issuers, all Root Cert resources that are not the default issuer will be forced to be recreated. This was leading users into a constant create-destroy cycle when multiple root certs were added in a TF file.

This update now correctly read the specific issuer's certificate in PEM format, and compare to what is in the TF state. If there is a change in the certificate, the Root Cert is recreated. Also, if an issuer is deleted outside of TF, this update correctly resolves the state by recreating the resource.

@github-actions github-actions bot added size/XL and removed size/L labels Aug 16, 2023
@vinay-gopalan vinay-gopalan requested a review from a team August 16, 2023 20:49
Copy link
Contributor

@raymonstah raymonstah left a comment

Choose a reason for hiding this comment

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

🎉

util/util.go Outdated Show resolved Hide resolved
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.

key_name parameter ignored in vault_pki_secret_backend_intermediate_cert_request resource creation
4 participants