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

Add support for CMPv2 configuration #2330

Merged
merged 13 commits into from
Dec 9, 2024
Merged

Add support for CMPv2 configuration #2330

merged 13 commits into from
Dec 9, 2024

Conversation

rculpepper
Copy link
Contributor

Description

Relates OR Closes #0000

Checklist

  • Added CHANGELOG entry (only for user-facing changes)
  • Acceptance tests where run against all supported Vault Versions

Output from acceptance testing:

$ make testacc TESTARGS='-run=TestAccXXX'

...

Community Note

  • Please vote on this pull request by adding a 👍 reaction to the original pull request comment to help the community and maintainers prioritize this request
  • Please do not leave "+1" comments, they generate extra noise for pull request followers and do not help prioritize the request

@vinay-gopalan vinay-gopalan requested review from vinay-gopalan and removed request for vinay-gopalan September 19, 2024 21:37
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.

Looking good, I've added some fixes for the data source documentation. You are missing the documentation around the new resource type which should be within the docs/r/ folder

website/docs/d/pki_secret_backend_config_cmpv2.html.md Outdated Show resolved Hide resolved
website/docs/d/pki_secret_backend_config_cmpv2.html.md Outdated Show resolved Hide resolved
website/docs/d/pki_secret_backend_config_cmpv2.html.md Outdated Show resolved Hide resolved
website/docs/d/pki_secret_backend_config_cmpv2.html.md Outdated Show resolved Hide resolved
Copy link

@sgmiller sgmiller left a comment

Choose a reason for hiding this comment

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

Looks good except for the c/p typos Steve noted.

stevendpclark
stevendpclark previously approved these changes Nov 18, 2024
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.

👍

@rculpepper rculpepper requested a review from a team as a code owner November 18, 2024 18:58
@fairclothjm fairclothjm added this to the 4.6.0 milestone Nov 18, 2024
@rculpepper rculpepper requested a review from a team as a code owner November 25, 2024 19:16
sgmiller
sgmiller previously approved these changes Dec 6, 2024
Copy link

@sgmiller sgmiller left a comment

Choose a reason for hiding this comment

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

Assuming tests pass LGTM

@rculpepper rculpepper merged commit 4b62925 into main Dec 9, 2024
11 checks passed
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.

4 participants