vault-creds
vault-creds copied to clipboard
Remove assumption that there is always a "Username" and "Password" field
Current behaviour tries to pull the "Username" and "Password" fields out of the data returned from Vault and sticks them in this custom "Credentials" struct. This is unnessecary and breaks usage in cases where the returned secret does not have those fields. (For example with the AWS secrets engine, you get "access_key" and "secret_key").
Hey, thanks this looks pretty useful, we don't currently use the aws vault stuff, we've got https://github.com/uswitch/kiam for that, but I definitely see the value in expanding this to handle more generic secrets. We're gonna look at using this for vault generated certs as well in the future.
Your changes looks good to me, but due to our shocking lack of tests in this repo it's hard to be certain so I think I shall go and write some first.
Haha sounds good! I have very minor experience with Go so would definitely recommend testing out my changes first :D
Agreed- it's nice to offer the more general access to secrets rather than exclusively database credentials.
My only comment/concern is that api.Secret
has a different interface to vault.Credentials
(the struct that username, password were read from). I don't know whether we'd just release with a major version bump and make it very obvious that they're different objects or whether it would be controlled via some kind of flag.
I feel like the former is probably sufficient and better.
But yeah, tests to prove behaviour would be good to add @Joseph-Irving 😄
Hey sorry for taking so long, done some refactoring work to this project to make it a bit more atomic. My main concern with this change is how big a breaking change this is, even with a major version change converting all your configmaps/templates etc from .Username to .username would be a giant pain, it would be a lot nicer if we could instead support both.