amperity / vault-clj

Clojure client for Hashicorp's Vault secret management system.
Other
70 stars 17 forks source link

Support pluggable ExecutorServices for background processes #97

Closed greglook closed 1 year ago

greglook commented 1 year ago

The background lease and token renewal logic runs on a dedicated background "timer" thread, which periodically checks for any necessary maintenance, performs it, and then sleeps for a fixed period. Any callbacks (in the 2.x branch) are invoked directly by the same singular thread. This is a simple approach, but doesn't behave well in the face of blocking - a hung callback could indefinitely prevent the timer thread from performing maintenance.

To address this, and to improve the client's ability to fit into other execution models, the client could opt to use a provided ScheduledExecutorService for the maintenance logic, and optionally another ExecutorService to run the callback actions.

greglook commented 1 year ago

Added 3e8fb11473ff7cc10f979c9ee43a00b78a6f655c for this, but needs a bit of testing.

jasonjckn commented 1 year ago

This might be helpful https://github.com/jarohen/chime which uses ScheduledExecutorService underneath. I've also written a mock 'simulated time' executor for it , sample code @ https://github.com/jarohen/chime/issues/57

jasonjckn commented 1 year ago

I don't know the domain you're working in, but i'm finding a lazy approach a cleaner way to go about it, instead of countless threads + watches to update downstream dependent connections on rotating passwords.

e.g. https://developer.hashicorp.com/vault/docs/secrets/ad#a-note-on-lazy-rotation

passwords are rotated lazily, so why not renew leases lazily, and renew tokens lazily, etc.

greglook commented 1 year ago

Lazy rotation works if your there is some chain of calls where your clients are requesting credentials each time they perform an operation and you have an opportunity to use cached values or reach out for new ones on-demand. The AWS SDK is a good example of this; you can implement an AWSCredentialsProvider that wraps a Vault client to do so. The lazy approach does not work in cases where the code that needs the credentials expects to have them "installed" and uses those statically - one example we have is using a database connection pool, where we need the external rotation to proactively generate new creds and update them in the pool configuration before they expire.

Another issue with lazy rotation is that it can lead to quite significant latency outliers. If your code paths are blocked on issuing new credentials occasionally, that's going to show up in your P99 times. This is even ignoring the potential issue of contention, where many threads in the same process concurrently need to request new credentials - you need a locking mechanism of some kind to make sure that only one request goes through.

If you do want the lazy behavior, that's still supported in vault-clj; you can just omit the :rotate? true parameter. If there's a cached credential lease available, it'll be returned immediately, otherwise the client will go ask for a new one.