quarkiverse / quarkus-vault

Quarkus HashiCorp Vault extension
Apache License 2.0
19 stars 24 forks source link

Make transit engine mount path configurable #208

Closed aaronz-vipaso closed 6 months ago

aaronz-vipaso commented 9 months ago

Currently, the mount path of the Transit engine is hard coded to transit. This PR makes it configurable.

aaronz-vipaso commented 9 months ago

@kdubb Hey there! Please take a look and let me know if there's anything else I should add. Thanks a lot!

kdubb commented 9 months ago

@aaronz-vipaso #184 Adds similar functionality to the KV engine. It also now follows the PKI engine structure.

High level...

  1. Add a mount to VaultTransitManager. Because it pulls double duty as a CDI bean and normal Java instance, you'll need two constructors. One with the mount path configurable, and an @Inject constructor that provides the CDI bean with the default mount path.
  2. Define a VaultTransitSecretEngineFactory interface to vend reactive and regular engines with a provided mount.
  3. Add a bean to implement it VaultTransitManagerFactory. This will create regular (non-CDI) instances of VaultTransitManager using the given mount.
kdubb commented 9 months ago

Also, be sure to remove any overloads in the engine interfaces that provide an explicit mount.

aaronz-vipaso commented 9 months ago

@kdubb Thanks for your quick feedback; I have tried to implement it. Can you please recheck it?

vashistha commented 8 months ago

I am also looking for transit engine client with mount path support. Please can it be merged and released sooner?

kdubb commented 8 months ago

@vashistha @aaronz-vipaso Please look at #214 & #215. It does this and a whole lot more.

If you can please build it locally and comment on #214 with ideas and/or #215 with specific suggestions for changes.

aaronz-vipaso commented 8 months ago

@vashistha @aaronz-vipaso Please look at #214 & #215. It does this and a whole lot more.

If you can please build it locally and comment on #214 with ideas and/or #215 with specific suggestions for changes.

@kdubb, how long do you expect it will take until the client rewrite is merged? If it takes longer than one month, would it be reasonable to merge this PR in the meantime?

kdubb commented 8 months ago

@aaronz-vipaso It's currently quite well tested on it's own (not that there aren't issues lurking inside). I'm pondering the idea of just releasing a version with the new client and no changes to the existing code.

That would allow people to use it soon and not effect the current stuff until the new client has been proven a reliable replacement.

kdubb commented 8 months ago

I already have a VaultClientProducer (not checked in) that provides the new client using the current Quarkus configuration. Aside from it being a little confusing (because they are both named VaultClient albeit in different packages) it would allow users to inject the new VaultClient without any different/extra configuration.

kdubb commented 6 months ago

4.0.0-alpha.2 has been released. Injecting the VaultClient (via @Inject VaultClient client) and using client.secrets().transit(mountPath) allows you to supply any mount path.

I'm closing this now that dynamic mounts should use the client directly instead.