Read vault token from ~/.vault-token if none supplied #11529
Conversation
There was a problem hiding this comment.
Hi @eraserhd! Thanks for submitting this.
This seems like a great idea, and a fine implementation. I just left a minor comment inline about consistency with the behavior of Vault itself.
builtin/providers/vault/provider.go
Outdated
| token := d.Get("token").(string) | ||
| if token == "" { | ||
| // Use the vault CLI's token, if present. | ||
| tokenFile := fmt.Sprintf("%s/.vault-token", os.Getenv("HOME")) |
There was a problem hiding this comment.
It looks like vault's own implementation of this uses the go-homedir library, which tries various different means to locate the user's home directory, including some specialized support for Windows systems.
Do you think using that here would make sense, so we can ensure that the behavior will be consistent with Vault itself?
There was a problem hiding this comment.
Haha, I noticed and pushed a patch apparently just as you were commenting. This is done.
|
I think the CI issue is not related. This should be good to go. |
builtin/providers/vault/provider.go
Outdated
| // Use the vault CLI's token, if present. | ||
| homePath, err := homedir.Dir() | ||
| if err != nil { | ||
| return nil, fmt.Errorf("No vault token found: %s", err) |
There was a problem hiding this comment.
Wouldn't this error be called if the homedir was unable to be found? The error text might need to be clarified.
|
I'm going to lock this issue because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active issues. If you have found a problem that seems similar to this, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further. |
ghost
locked and limited conversation to collaborators
This closes #11365 (a feature request).