-
Notifications
You must be signed in to change notification settings - Fork 4.3k
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
Move default token helper to api #25744
Conversation
CI Results: |
Build Results: |
0035810
to
b429d8a
Compare
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.
This looks good to me! I gave thought to api vs sdk and ultimately I think this makes the most sense: a token is primarily to access Vault's APIs.
Thanks for this -- excited to reduce usage of the Vault dependency.
I wanted to note that there's a related community PR we should close when we merge this: #9786
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.
Thank you both! |
I just want to say thanks. I've been hoping for this for several years, but didn't have the time to dig in myself. I hope that default token helper behavior becomes a more common fixture in the vault libraries for other languages because it's a great way to encapsulate simultaneous login sessions to multiple clusters. |
The Vault Terraform provider currently depends on github.com/hashicorp/vault.
As documented elsewhere, this isn't a supported dependency and puts the provider at risk of being broken by any updates. Most of the code depended on is trivial like constants, or test dependencies like preparing test containers. The hardest dependency to remove is the fact that it depends on the CLI's
DefaultTokenHelper
: https://github.com/hashicorp/terraform-provider-vault/blob/cb919ae6c040cdfc5513b635ec9c4b4762e24ca4/internal/provider/meta.go#L627This PR moves the two related packages (
command/config
andcommand/token
) over to theapi
package so that the provider can continue to share that non-trivial functionality and work towards removing thehashicorp/vault
dependency.For the initial draft I've chosen
api
oversdk
because it seems like a reasonable piece of functionality for any consumer of the API to want to use.Note that I've tried to create a sequence of commits that do one thing each with very limited scope, so it may be easier to review commit-by-commit.