-
Notifications
You must be signed in to change notification settings - Fork 548
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
Add PKI EST configuration support #2246
Add PKI EST configuration support #2246
Conversation
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.
Looking great! Thanks for working on this, had a few comments/suggestions :)
}, | ||
consts.FieldDefaultPathPolicy: { | ||
Type: schema.TypeString, | ||
Computed: true, |
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.
Why is this marked as Computed? If we don't set the value in the config, does Vault return a default?
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.
Sorry, I just realized this is a data source so please disregard. :)
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.
No worries, but that is the reason on FieldAuditFields within the resource Computed is set, we get back a default set of values from Vault if not set.
- Workaround not releasing 1.16.3 for the fix within hashicorp/vault-enterprise#5785 by not setting the cert_role parameter within the cert authenticator definition
d7c9885
to
2dc02f3
Compare
Updated PR to the latest version of main as #2235 was merged, along with disabling a test to workaround an unreleased bug (will be in 1.16.3) in Vault Enterprise https://github.com/hashicorp/vault-enterprise/pull/5785 |
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.
PR looks great! Had some comments/suggestions on a few documentation lines, but nothing blocking. Thanks for working on this!
- Simplify the dependency's on the data resources in the test - Update docs and field descriptions to match Vault API documentation
Description
This PR adds new data source and resource types for the PKI EST configuration API. The PR is built on top of #2235 as EST support requires the new mount fields.
This also requires Vault 1.16.3+ent at a minimum as the following fixes are required for this to work correctly
Sample TF script used for testing (along with the added tests)
Checklist
Output from acceptance testing:
Community Note