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

Fix #98: Support Vault token helper. #136

Merged
merged 5 commits into from
Oct 5, 2018

Conversation

jgiles
Copy link
Contributor

@jgiles jgiles commented Jun 4, 2018

Rather than manually pulling a Vault token from ~/.vault-token, use
Vault's DefaultTokenHelper. This will use the token helper configured at
~/.vault (or at VAULT_CONFIG_PATH), falling back to ~/.vault-token.

@jgiles
Copy link
Contributor Author

jgiles commented Jun 4, 2018

@paddycarver this fixes #98.

I've tested it locally, but there wasn't a great spot to work it into the current acceptance tests.

See also hashicorp/vault#4688, but we're having trouble with this currently so I figured I'd go ahead and put the fix in here.

@jgiles
Copy link
Contributor Author

jgiles commented Jun 7, 2018

@paddycarver ping?

@jgiles
Copy link
Contributor Author

jgiles commented Jun 17, 2018

@paddycarver ? @bflad maybe?

@jgiles
Copy link
Contributor Author

jgiles commented Aug 20, 2018

@catsby @paddycarver @bflad @fatmcgav any chance of eyes on this? We're considering running our own fork at this point - token helper support is huge for us.

@jgiles
Copy link
Contributor Author

jgiles commented Sep 18, 2018

@tyrannosaurus-becks if I rebase this, is there some chance it will get a review?

@tyrannosaurus-becks
Copy link
Contributor

@jgiles absolutely! Just working my way through the backlog oldest first. Apologies for the huge delay in getting to this. The provider was in transition from the Terraform team to the Vault team. I should be able to review this this week.

@jgiles
Copy link
Contributor Author

jgiles commented Sep 18, 2018

That's great news @tyrannosaurus-becks ! The Vault team seems like an excellent home for this - we do our Vault configuration almost exclusively through Terraform, and we're excited to help improve the tool.

@tyrannosaurus-becks tyrannosaurus-becks self-assigned this Sep 18, 2018
Copy link
Contributor

@tyrannosaurus-becks tyrannosaurus-becks left a comment

Choose a reason for hiding this comment

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

This looks great! Thank you for writing it.

Would it be possible to add the following?

  • A test verifying that it will still pull a token from ~/.vault-token
  • A test verifying that it will pull a token from a faux (or real) token helper

Those would not only help with this PR, but they would verify that there were no breaking changes going forward as the token helper's underlying code changes.

@jgiles
Copy link
Contributor Author

jgiles commented Sep 23, 2018

Will do, @tyrannosaurus-becks - hopefully in the next couple days.

Rather than manually pulling a Vault token from ~/.vault-token, use
Vault's DefaultTokenHelper. This will use the token helper configured at
~/.vault (or at VAULT_CONFIG_PATH), falling back to ~/.vault-token.
Resolve local modifications.
@jgiles
Copy link
Contributor Author

jgiles commented Sep 30, 2018

@tyrannosaurus-becks I've rebased and added test cases for all the ways the provider can get a Vault token.

Unfortunately, the only way to test the ~/.vault-token case is to actually put a token there. The test restores anything that was there previously, but I've made the test an "acceptance test" because of the way it could interfere with other Vault stuff.

Copy link
Contributor

@tyrannosaurus-becks tyrannosaurus-becks left a comment

Choose a reason for hiding this comment

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

@jgiles this looks fantastic, thank you!

@tyrannosaurus-becks tyrannosaurus-becks merged commit f98f732 into hashicorp:master Oct 5, 2018
@jgiles
Copy link
Contributor Author

jgiles commented Oct 7, 2018

Thanks for reviewing + merging @tyrannosaurus-becks ! Do you know when you might cut the next release?

@jgiles jgiles deleted the token-helper branch October 7, 2018 16:43
dandandy pushed a commit to dandandy/terraform-provider-vault that referenced this pull request Jun 17, 2021
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.

None yet

2 participants