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

VAULT-7698 Fix ignored parameter warnings for endpoint arbitrary data options #16794

Merged
merged 2 commits into from
Aug 23, 2022

Conversation

VioletHynes
Copy link
Contributor

@VioletHynes VioletHynes commented Aug 19, 2022

The changes made to TestHTTP_Wrapping made the test fail before my fix, and this fixes the issue.

The output of an unwrap no longer contains a warning:

vault write -force sys/wrapping/unwrap
Key    Value
---    -----
foo    bar

I could not find any other endpoint that would be appropriate to add TakesArbitraryInput: true to, but it made sense to me to add the new option to the framework instead of simply making an exception for this particular endpoint, which felt messy. It seems likely we'll have need for this boolean in the future.

@VioletHynes VioletHynes marked this pull request as ready for review August 19, 2022 16:02
Copy link
Contributor

@mpalmi mpalmi left a comment

Choose a reason for hiding this comment

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

Changes look good to me, but I'm not sure how the error [warning] came to be. What is it about seal unwrapping that causes non-field data to be populated? Just want to double-check that this behavior is not caused by some other mechanism acting up.

@VioletHynes
Copy link
Contributor Author

Changes look good to me, but I'm not sure how the error [warning] came to be. What is it about seal unwrapping that causes non-field data to be populated? Just want to double-check that this behavior is not caused by some other mechanism acting up.

Wrapping wraps the data presented in the request. It doesn't actually have a Field object, it just literally uses data.Raw to take everything. I think it makes sense in this case - wrapping is a weird endpoint but it makes sense for it to work like this.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants