-
Notifications
You must be signed in to change notification settings - Fork 42
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
Kv2 support with a specified secret key #54
Conversation
This adds a new optional parameter 'vault_key' which can address a key within a secret in a kv2 secretengine. it does require padding the unused arguments because deferred takes a list of values. but this means that existing use cases will still work while supporting the current Vault kv secretengine |
02dffb4
to
d087e1c
Compare
Also fixes #30 |
@@ -5,6 +5,7 @@ | |||
optional_param 'Optional[String]', :vault_cert_path_segment | |||
optional_param 'String', :vault_cert_role | |||
optional_param 'String', :vault_namespace | |||
optional_param 'String', :vault_key |
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.
DO we need to allow empty strings, or could we also use:
optional_param 'String', :vault_key | |
optional_param 'String[1]', :vault_key |
(which enforces a minimal string length of 1, or no string at all)
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.
I think we should... because of how this ends up being called sometime you need to pass in the empty string.
and yes while it's the final argument in the list it doesn't matter.. but if we ever need another argument it's problems.
A better way to handle this that works well with deferred would be nice but isn't obvious to me.
Deferred doesn't take named arguments (for better or mostly worse) so for example when not using namespaces you need to pass the empty string to access the key parameter
@firstnevyn thanks for the PR! Can you take a look at the existing tests and maybe add one for this feature? We have unit tests in https://github.com/voxpupuli/puppet-vault_lookup/blob/master/spec/functions/lookup_spec.rb and acceptance tests in https://github.com/voxpupuli/puppet-vault_lookup/blob/master/spec/acceptance/lookup_spec.rb |
thanks for the work! |
Pull Request (PR) description
Adds support for Vault KV2 format secrets and allows addressing a key within a vault secret
With 0.4.0 a kv2 returns a complete hash including creation version and other metadata. this fails to be converted to a string causing an agent compile error
This Pull Request (PR) fixes the following issues
Fixes #21
Fixes #25