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

CLI: add new -header option to be able to add headers to all cli http request #12508

Merged
merged 1 commit into from Oct 27, 2021

Conversation

ylorenzati
Copy link
Contributor

This PR add a new option to the vault cli: -header with can be specified multiple times. All the parameters specified like that will be added to HTTP headers for all requests.

For example, it will enable us to achieve zero trust with Cloudflare access and still be able to use vault cli

This fix #8754

@hashicorp-cla
Copy link

hashicorp-cla commented Sep 8, 2021

CLA assistant check
All committers have signed the CLA.

@vercel vercel bot temporarily deployed to Preview – vault September 8, 2021 09:26 Inactive
@vercel vercel bot temporarily deployed to Preview – vault-storybook September 8, 2021 09:26 Inactive
@vercel vercel bot temporarily deployed to Preview – vault September 8, 2021 09:33 Inactive
@vercel vercel bot temporarily deployed to Preview – vault-storybook September 8, 2021 09:33 Inactive
@vercel vercel bot temporarily deployed to Preview – vault-storybook September 8, 2021 09:44 Inactive
@vercel vercel bot temporarily deployed to Preview – vault September 8, 2021 09:44 Inactive
@ylorenzati ylorenzati marked this pull request as ready for review September 8, 2021 09:54
@ccapurso ccapurso added enhancement core/cli devex Developer Experience labels Oct 26, 2021
@ccapurso
Copy link
Contributor

Hi @ylorenzati, this is an interesting concept. In addition to your changes, we will want to prevent users from providing headers with the prefix X-Vault- as these are reserved for Vault. The CLI command should fail with an error providing all the offending -header values that contain the reserved prefix. Once this change is made, would you also be able to add testing to this PR? It's prudent to prevent regressions given that this new flag would be accessible to all commands as it exists in BaseCommand.

It also appears that there are conflicts in command/base.go that will need to be resolved prior to merging.

@ccapurso ccapurso self-requested a review October 26, 2021 15:43
@vercel vercel bot temporarily deployed to Preview – vault October 26, 2021 15:54 Inactive
@vercel vercel bot temporarily deployed to Preview – vault-storybook October 26, 2021 15:54 Inactive
@ylorenzati
Copy link
Contributor Author

Hi @ccapurso,

Thank you very much for your time and your feedback. I will implement what you asked as soon as possible (Before the end of the week I hope)

@ccapurso
Copy link
Contributor

Sounds good, take your time :).

@vercel vercel bot temporarily deployed to Preview – vault-storybook October 27, 2021 10:08 Inactive
@vercel vercel bot temporarily deployed to Preview – vault October 27, 2021 10:08 Inactive
@vercel vercel bot temporarily deployed to Preview – vault-storybook October 27, 2021 10:09 Inactive
@vercel vercel bot temporarily deployed to Preview – vault October 27, 2021 10:09 Inactive
@ccapurso
Copy link
Contributor

Thank you for the X-Vault- check and test. Looks good!

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

Successfully merging this pull request may close these issues.

Sending Arbitrary Headers in CLI
3 participants