-
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
feat(pki): Add resource for cluster configuration #1949
Conversation
@fairclothjm could you please review this PR? Thanks! |
@fairclothjm in reference to the comment: #1947 (comment) This is announced as full feature and is being advocated as a working feature from your services teams. Could you please review the pull request so we can get the feature brought in? It's clearly part of the API: https://developer.hashicorp.com/vault/api-docs/secret/pki#acme-certificate-issuance |
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.
Thanks for the contribution @Viper61! I did an initial pass and it looks good so far. If you are interested in carrying this forward could you please make the requested changes and add a changelog entry? Thanks!
99d35b2
to
25210d7
Compare
Thanks for the review @fairclothjm! |
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.
Great work so far @Viper61! A couple of things in addition to my comments:
- Could you please rebase on main as you have merge conflicts and the CHANGELOG is outdated
- After the requested changes are done, if you are ok with me pushing to your branch I can add some regex parsing for import correctness.
Thanks!
2f0508e
to
5e65211
Compare
New rebase done and I updated the CHANGELOG. No more conflict for now.
Sure, be my guest @fairclothjm . |
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.
LGTM! Thanks @Viper61 !
Community Note
Relates #1947
Release note for CHANGELOG:
Output from acceptance testing: