-
Notifications
You must be signed in to change notification settings - Fork 4.2k
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
Expose secret_id_accessor as WrappedAccessor when wrapping secret-id creation. #12425
Conversation
Hi @melbit-michaelw ! Thanks for submitting this PR! For adding a test for this functionality, I think you are good to skip unit tests, as we don't really have unit tests on wrapping.go. For integration tests, I think adding a new folder for |
Think this comment https://github.com/hashicorp/vault/blob/main/sdk/helper/wrapping/wrapinfo.go#L20-L21 might need to be updated, to reflect the dual nature of WrappedAccessor with this change. |
Thank you very much for the feedback. I've done the minor fixes suggested, and will start looking at the integration testing. |
@pmmukh - Added tests to confirm WrappedAccessor set as expected, and not set when wrapping isn't performed. Let me know if anything else is required. |
hey! Just wanted to drop a note that there's one little point we're hashing out internally on this change, after which I'll give this another pass sometime this week! |
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 for the quick responses on the feedback!
Resolves #12173 for Secret-Ids.
I'm not a Go developer (nor indeed any kind of developer), and was unable to find the right place to add a test for this functionality. Could you please point me in the right direction as to where this should be tested ?