Remote vault patch 1 #1605
Remote vault patch 1 #1605
Conversation
|
|
|
Going to take a look at this but need to revisit my vault knowledge. |
| @@ -40,6 +41,7 @@ Name | Type | Description | |||
| `server` | `string` | The Vault server to connect to. | | yes | |||
| `namespace` | `string` | The Vault namespace to connect to (Vault Enterprise only). | | no | |||
| `path` | `string` | The path to retrieve a secret from. | | yes | |||
| `key` | `string` | The key to retrive a secret from. | | yes | |||
There was a problem hiding this comment.
I feel we need a way to allow for backwards compatibility. Likely we need to have both behaviors. If key is empty use existing behavior.
Nit: Typo in retrieve.
There was a problem hiding this comment.
Hi @mattdurham ,
I get that. So what I can do is wrap it in an if-else.
And I think we have to define the argument key as optional.
Thanks,
Patrick
if-else to allow working with or without `key` argument
|
@mattdurham |
|
|
||
| cfg := fmt.Sprintf(` | ||
| server = "%s" | ||
| path = "" |
There was a problem hiding this comment.
Should this have a path to more accurately test the issue? Should it be secret and key be test?
|
@mattdurham |
|
Looks good to me, @clayton-cornell can you check the doc? Once thats done will merge. |
clayton-cornell
added
the
type/docs
|
Tanks for fixing this fast. Will it be released with a v1.3.2? When is this version due? |
PR Description
Fixed an issue where specifying both path and key in the remote.vault path configuration could result in incorrect URLs. The path and key arguments have been separated to allow for clear and accurate specification of Vault secrets.
Which issue(s) this PR fixes
Fixes #1599
Notes to the Reviewer
Not sure about the config converters part. Please check.
Please also check if the documentation is clear and understandable.
PR Checklist