-
Notifications
You must be signed in to change notification settings - Fork 48
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
v1 candidate #36
v1 candidate #36
Conversation
caching vault object for entire run and closing sockets when done
adding support for deeply nested object stored in Vault
allow hiera to keep looking if value is not found
This change moves from a one-time backend to a URI based hiera backend wherein the lookup_key function will be called for each configured URI. This change moves the paths settings out of the options hash to the URIs array and uses the mounts options to control what kind of secret backend is being used. It has been tested against kv2, and assumed to work with kv1, structures are in place for other path based backends if needed. This change requires a modified version of the vault ruby gem which correctly calls LIST method endpoints to help reduce the number of calls being made to vault during the lookup process. This change introduces the `environment_delimeter` option which will allow the fields at a secret path to be selected based on the current environment, bare fields will be selected by default. This feature behaves as expected when default field is used. This change will require updating the configuration to use uris instead of putting the paths in the options block.
@traviscosgrave This looks great! The main issue is that I dont really want to merge this until the features are added into the core gem... Let me check on the status of that and see if I can get that merged |
Of course! We're running both forks locally, and I'm not sure my patch to the core gem is the best way to solve the problem of the HTTP client on supporting the Also, this is now up-to-date with our current implementation. I ran into some issues with cached environments and JRubies either garbage collecting or nodes in a weird state (migrating from one cluster to another) hitting the error and the global $vault being blown away. The most recent change removes the forced nil on the $vault client and will recreate one if it get's GC'd. Thanks for following up |
Any news on this? |
I'm still battling the error mentioned above, I hope to have some kind of solution this week and I'll update the PR accordingly. |
This change resolves a race condition, especially with cached environments, where the client connection pool will shutdown during a requst resulting in a catalog compilation failure by using the built in retry functionality of the vault-gem. This change also moves to use the `Vault::logical.list` method over requiring the `LIST` HTTP method as part of PR 201 on the vault gem. The workaround resulted from a bug in the `vault agent` request forwarding whereby query string parameteres failed to be forwarded. Using the latest `vault agent` allows the typical method to be used.
Using the retry feature of the vault client I was able to resolve the race condition whereby a pool is shutting down mid-request especially in cached environments. I've also eliminated the need for the weird support of the LIST http method by instead upgrading the local vault agent to the latest 1.1.3 (in which a bug preventing query string parameters from being proxied was fixed). This is now looking good in our environment. Looking forward to the comments. Asside: I did attempt to use the built in Puppet::HttpPool, but it required some nasty tricks and was about 50% slower on catalogs with many lookups even after forcing it to reuse connections. |
@traviscosgrave You still need hashicorp/vault-ruby#201 before merging this ? |
hashicorp/vault-ruby#201 is only required if you plan to use vault agent before version 1.1.3. It shouldn't be required if you talk directly from your puppet server/agent to vault. (Although that case is currently outside our scope due to a strict vault cipher set that is unsupported by the jruby net:http implementeation). [The current code is what we have have running now.] |
@traviscosgrave Can you resolve the conflict then I can merge? 👍 |
I'll see what I can do, there's a lot that I moved around. |
I've merged #41 in the mean time, which doesnt seem to require changes to the gem, can you test this works for you now? |
I tried #41- and it is broken for v2 stores (and actually doesn't work well for v1 either). I just have two nitpicks:
|
This PR is closer to a request for comment.
I was unable on my host to get the specs running, and as such did not add any (although the approach should allow all of the functionality to exist. Also, these changes require the changes I added to the hiera-vault gem to support real LIST requests hashicorp/vault-ruby#201.
The function has been re-written such that it behaves more like the other standard hiera back ends, running the function once per uri passed in. It takes advantage of the global vault client, but reconfigures it before each lookup to ensure environment caching doesn't cause any problems. Configuration is cached for each path, and the keys found at each path and any values returned are cache to reduce the calls to vault and provide as static a view on the secret set as possible.
There is a construct for adding other backends, and supporting interleaved backends is now possible as the configuration for the mount and the list of paths are handled separately.
A pattern for handling environment specific field values (eg for testing a new version of a secret before rolling it out infra wide) also has been developed with the
environment_delimiter
option.