quarkiverse / quarkus-vault

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

Implement a new configurable, standalone client #214

Closed kdubb closed 5 months ago

kdubb commented 8 months ago

Overview

This issue serves to outline the current restrictions that are imposed by the current client and design/discuss solutions that will go into a new client to replace it.

Once a new client supports all the engine apis that the current extension & client supports, the extension <API>Engine beans will be re-implemented using this new client.

One big statement upfront. This is not a criticism of the current client in any fashion. It is the collective work of quite a few talented people and the end result is an impressive body of work. The current client works very well while connecting to a number of dynamic and very under-specified HTTP APIs.

The goal is to move the client forward and to do that. I want to outline the restrictions provided by the current client so we can ensure

Problem Statement

The current client has issues with configurability and lack of engine support. The new client aims to fix those issues and make it easier to implement/update Vault engine APIs.

Configurability

The current client is designed to interact with a single Vault instance and is essentially locked to that instance by the rigid configuration. Additionally most of the engines are (or were) locked to a single mount. Many updates have been made to support "mount" configurability to some engines it is not the best solution and requires a bit complex CDI to make both the statically configured and dynamically configured engines available.

In the end, all this works well for a lot of Quarkus users who are just looking to read configuration from Vault but, considering this is the only regularly updated Vault client for the JVM, this should be solved.

A new client should be dynamically configurable in an easy manner. The Quarkus Vault extension can dynamically configure a client to match the static configuration provided by Quarkus.

Complex flow

The current client suffers from having a single "locked" configuration (including authentication) and still needs to make unauthenticated requests to Vault for purposes of initializing authentication tokens. To do this when making a request the VaultClient and its configuration must be passed down from the top of the call chain.

This complexity, while necessary in its current design, can be hard to understand and therefore is a barrier to extending the client to support new authentication engines.

The new client should aim to solve this by separating the authentication flow from the normal flow and allow easily sending authenticated or unauthenticated requests as needed.

Ease of development

Currently we are handwriting the api in two places, one public api that provides a normalized "nice" view of the data provided by Vault and an the internal api that nearly exactly matches Vault's requests and responses.

This is tedious, error prone and provides a large barrier to adding support for new engine apis; or even just updating the current ones.

Code generation should be used to do most of this work in the new client.

Use outside of Quarkus

Currently there is no way to use the client without Quarkus. It is literally built into the extension and very strongly coupled with it and Quarks.

Any new client should be completely decoupled from Quarkus configuration and the Vertx HTTP/Web client. Allowing it to be used in simpler environment with a "simple" executor (e.g. the JDK HttpClient).

Extension

Vault is an extendable server. It provides a large number of built in plugins/engines but users can write their own and use them in the same Vault instance. Currently it is possible, though very hard, to use the client with a custom or unsupported Vault engine/plugin.

The new client should make it easy to send and process custom requests to the server and, if possible, allow itself to be extended by end users.

Testing

There are a lot of tests verifying the current implementation against a real Vault instance and they provide a lot of test coverage for positive usage. At the same time the current code is not testing a lot of error cases and when using a Vault instance it can be hard to get it to generate those errors.

The new client should natively support mocking and interception to be able to write tests for and support for complex scenarios like authentication failures and retries.

aaronz-vipaso commented 8 months ago

The proposed changes sound reasonable. One point:

I can't entirely agree with the generification of this extension so that it can be used without Qurakus. If that is the goal, it should be a separate open-source project and repository outside the quarkiverse scope. Eventually, this extension could use an external pure JVM library. Also if you keep using the Quarkus ecosystem you could use the Quarkus REST client, which reduces boiler plate code.

kdubb commented 8 months ago

@aaronz-vipaso In case you missed it, an implementation exists in PR #215.

WRT breaking it out. I contemplated this with @vsevel in Zulip and in the end I weighed it out and there's a lot of machinery that is managed by Quarkiverse that would need to be replicated in a separate project. Not to mention this project already has collaborators and community. In the end, I didn't see the advantage.

The implementation in #215 supports both a JDK and Vert.x HTTP client.

Finally, I originally attempted to use the Quarkus REST client but given all the POJOs that needed to be created/generated to communicate with Vault there wasn't any advantage in using it.

aaronz-vipaso commented 8 months ago

Thanks for the explanation. Makes sense 👍

vsevel commented 5 months ago

Fixed with https://github.com/quarkiverse/quarkus-vault/pull/215