Add support for Vault

[vsevel]

This pull request provides support for getting credentials out of hashicorp vault directly from quarkus when using the agroal connection pool.

The initial discussion is here:
https://groups.google.com/d/msg/quarkus-dev/Izy6KyqNtrk/xyDSIBFfBQAJ
I believe the main points have been addressed.

I have developed a sample application that demonstrates its use:
https://github.com/vsevel/vaultapp

From an application perspective, it is as easy as:

1. configuring the vault server access:

quarkus.vault.url=https://vault.vault.svc:8200
quarkus.vault.role=myapprole

2. configuring a datasource that uses a password stored in vault:

quarkus.datasource.driver=org.postgresql.Driver
quarkus.datasource.url=jdbc:postgresql://postgres:5432/mydb
quarkus.datasource.max-lifetime=1H
quarkus.datasource.username=myuser
quarkus.datasource.vault.secret-path=foo

3. or configuring a datasource to use dynamically generated credentials (username and password):

quarkus.datasource.driver=org.postgresql.Driver
quarkus.datasource.url=jdbc:postgresql://postgres:5432/mydb
quarkus.datasource.max-lifetime=1H
quarkus.datasource.vault.dbrole=mydbrole

depending on the situation, vault has to be configured differently. Please refer to the vaultapp example application, or the VaultTestExtension that supports IT tests.

This PR honors edge cases such as:

• renewing leases/tokens when the ttl is reached
• recreating leases/tokens when the max-ttl is reached or a revocation has been processed

This level of functionality has been made possible by https://issues.jboss.org/browse/AG-116 that was developed in agroal for that use case, plus the max-lifetime feature that forces a connection to be recycled on a regular basis (and allows to go through vault's renewal logic without implementing some kind of timer mechanism in VaultPassword). To benefit from this feature, I had to expose it through a new property maxLifetime in DataSourceRuntimeConfig.

It means that you can use static passwords in vault and revoke the client token, or dynamic credentials and revoke the lease, and force client applications to go through the secret acquisition logic, without any downtime.

Each datasource will have its own VaultPassword (VaultPassword instances are not shared across multiple datasources, which is not an issue because they are lightweight objects).

We support using secrets on the default datasource, and also named datasources.

2 auth mechanisms are supported:

• kubernetes: see https://www.vaultproject.io/docs/auth/kubernetes.html
• userpass: for testing, or outside k8s. see https://www.vaultproject.io/docs/auth/userpass.html

The kubernetes auth is configured by default, and leverages the standard jwt token located at:
/var/run/secrets/kubernetes.io/serviceaccount/token
Even if k8s is the primary target, it is working perfectly as a standalone container, or even as a good old java program running outside docker.

Other auth mechanisms (see https://www.vaultproject.io/docs/auth/index.html) can be easily added in VaultAuthService.
see VaultAuthService

TLS is supported, and active by default. A tls-skip-verify mode has been added as well. And it is possible also to deactivate tls all together (dev mode).
see VaultHttpClientFactory

If running in k8s we can leverage automatically the cacert bundle located here:
/var/run/secrets/kubernetes.io/serviceaccount/ca.crt

For all properties there are sensible defaults, to ease deployment in k8s in particular.
see VaultServerRuntimeConfig

For static passwords, I support the kv engine version 1 and 2 (with versioning).
see https://www.vaultproject.io/docs/secrets/kv/index.html

There is a very simple cache for fetched secrets, with a cache-period that can be set (default=10mins). The cache gets used in the usual situation where we are within the cache period, but also if we get an http exception or a forbidden exception when we attempt to contact the vault. In that case the application will be allowed to use the last known value.

The http client allows to set the read timeout (default is 500ms) and the connect timeout (default is 5000ms).

Most of the logging is done in debug, and there is a log-confidentiality-level property that can be set to print out confidential informations in dev mode.

I have created integration tests that leverage testcontainers to start and configure a complete vault+postgres system.
see VaultTestExtension and Vault*ITCase classes.

I just saw that the windows build was failing with an error on getting the testcontainers lib to work:

WARN: Failure when attempting to lookup auth config (dockerImageName: quay.io/testcontainers/ryuk:0.2.3, configFile: C:\Users\VssAdministrator\.docker\config.json. Falling back to docker-java default behaviour. Exception message: C:\Users\VssAdministrator\.docker\config.json (The system cannot find the path specified)
[ERROR] Tests run: 1, Failures: 0, Errors: 1, Skipped: 0, Time elapsed: 4.254 s <<< FAILURE! - in io.quarkus.agroal.test.VaultITCase
[ERROR] io.quarkus.agroal.test.VaultITCase  Time elapsed: 4.253 s  <<< ERROR!
com.github.dockerjava.api.exception.DockerClientException: Could not pull image: failed to register layer: re-exec error: exit status 1: output: ProcessBaseLayer \\?\C:\ProgramData\docker\windowsfilter\6aaa4d9cf3d93098b15697d0b0b820056b63c44cf6ae1104f9048086bc6985dd: The system cannot find the path specified.

Not sure about that.

The integration test was by far the most challenging to write, and that is why I spent most of my efforts on it. I will complete the testing side with unit tests very shortly.

I have done quite a bit of manual testing as well using the example application vaultapp.

Future improvements to be discussed:

• use the resteasy client extension to call vault (instead of configuring our own http client)
• make vault an extension of its own, and get agroal to depend on it, which would allow one vault
  instance to be shared inside quarkus, plus would open up some other use cases (eg certificates mgmt) by leveraging the other secret engines.

The only minor glitch I encountered was that for some reasons I could not get the org.testcontainers.postgresql artifact to use version ${test-containers.version}, but instead had to hardcode it in the pom. Something that is probably easy to figure out.

I am hoping you can see the value of this PR, given that:

• vault is a standard in a microservice world
• I have tried to respect quarkus's spirit, specifically on the ease of use side
• plus I have spent quite a bit of hours to provide production quality code ;-)

Please reach out if you think this PR has the potential to make its way into the product.
Regards,

Vincent

Fixes #2764

[starksm64]

The windows build is failing due to docker:

2019-06-19T23:19:46.4739175Z WARN: Failure when attempting to lookup auth config (dockerImageName: quay.io/testcontainers/ryuk:0.2.3, configFile: C:\Users\VssAdministrator\.docker\config.json. Falling back to docker-java default behaviour. Exception message: C:\Users\VssAdministrator\.docker\config.json (The system cannot find the path specified)
2019-06-19T23:19:49.8008474Z [ERROR] Tests run: 1, Failures: 0, Errors: 1, Skipped: 0, Time elapsed: 4.254 s <<< FAILURE! - in io.quarkus.agroal.test.VaultITCase
2019-06-19T23:19:49.8009206Z [ERROR] io.quarkus.agroal.test.VaultITCase  Time elapsed: 4.253 s  <<< ERROR!
2019-06-19T23:19:49.8009784Z com.github.dockerjava.api.exception.DockerClientException: Could not pull image: failed to register layer: re-exec error: exit status 1: output: ProcessBaseLayer \\?\C:\ProgramData\docker\windowsfilter\6aaa4d9cf3d93098b15697d0b0b820056b63c44cf6ae1104f9048086bc6985dd: The system cannot find the path specified.

What is the configuration of the windows machine that runs the ci. Does it have docker installed?

[starksm64]

I do like what you have done, but the general issue I'm seeing is that we have a lot of functionality included here that exists in other extensions such as rest proxies, json binding, and the vault service itself would really have to be a separate extension that provides access to the vault features we would want to support. We'll discuss this and come back with how to proceed.

[vsevel]

I do like what you have done, but the general issue I'm seeing is that we have a lot of functionality included here that exists in other extensions such as rest proxies, json binding, and the vault service itself would really have to be a separate extension that provides access to the vault features we would want to support. We'll discuss this and come back with how to proceed.

I agree with the general issue you are stating. I had pointed this out in the poc back in april and in paragraph "Future improvements to be discussed" above.

The question to me was to either go bigger first, possibly drawing more dependencies on other contributors or teams (like I had on agroal with AG-116) hence delaying the feature, or go smaller, with the perspective of having to do some rework at some point, but bringing value more quickly.

Obviously I chose the later (but preferably I would have liked to go big and fast), because I believe there is already a fair amount of value in bringing vault's support with respect to db connection configuration, as this is by far the primary use case for storing secrets.

The compromise I found was to not expose anything to the user beside the configuration parameters, making all I have done an implementation detail. The idea was that as soon as there would be a vault extension available, we could delegate a fair amount of logic to this new extension, and remove code from the agroal extension. The only visible effect for the user would be then if vault's configuration properties were to be changed.

The discussion is really about time to market vs doing the right thing. If we can do both, I am all for it.
Thanks for your review.

[vsevel]

The windows build is failing due to docker:
...
What is the configuration of the windows machine that runs the ci. Does it have docker installed?

The only difference I see with the existing integration tests that already use testcontainers is that I am using the docker java api, as opposed to relying on the fabric8 docker-maven-plugin, which is useful for me because I have plenty of docker exec to do for vault. Since I saw a reference to testcontainers in the bom, I assumed it was already used in quarkus, but I do not see any other reference in the code. And tests that are running the docker-maven-plugin are all from the integration-tests module, which are disabled (the jpa-X tests). So may be you are right there is no docker??

[emmanuelbernard]

@cescoffier do you know if the Windows CI machine has docker installed?

[vsevel]

@starksm64 I reworked the code base, and came up with a vault extension, and an integration inside agroal.

The extension can be used at 2 levels:

• VaultProxy: simple rest client
• VaultService: handles transparently auth (initial, extension and recreate) and 2 main services: reading static secrets (kv version 1 and 2) and generating dynamic db credentials (initial lease creation, extension and recreate)

As suggested in your review, the VaultProxy is now based on resteasy+jsonb. TLS is handled consistently through a trust store.
To use the VaultProxy, you would simply set the mp-rest url.
And to use the VaultService, you need to provide additional properties (mainly auth related: userpass or k8s).

For instance for k8s (default):

io.quarkus.vault.VaultProxy/mp-rest/url=https://localhost:8200
quarkus.vault.role=myapprole

and for userpass:

io.quarkus.vault.VaultProxy/mp-rest/url=https://localhost:8200
quarkus.vault.auth-type=userpass
quarkus.vault.userpass-username=bob
quarkus.vault.userpass-password=sinclair

So that reading secrets is as easy as:

@Inject
VaultService vaultService;
...
String mypassword = vaultService.readSecret("myapp/mydb").get("mypassword");
// or
VaultDynamicCredentials credentials = vaultService.getDynamicCredentials("mydbrole");
String username = credentials.username;
String password = credentials.password;

I have also developed the agroal/vault integration, which works both with static secrets and dynamic db credentials. Everything is done through configuration:

quarkus.datasource.driver=org.postgresql.Driver
quarkus.datasource.url=jdbc:postgresql://localhost:5432/mydb
...
# for static secrets
quarkus.datasource.vault.secret-path=foo

# for dynamic credentials
quarkus.datasource.vault.dbrole=mydbrole

The agroal integration provides a simple cache that will return the previously fetched secret for some time period, or whenever vault is unavailable.

I upgraded my test application https://github.com/vsevel/vaultapp and got it to deploy and run in k8s.

That's for the good news.

For the bad news, the biggest disappointment came from the fact that there is an indirect dependency between quarkus-smallrye-rest-client-deployment and quarkus-agroal-deployment through quarkus-smallrye-fault-tolerance-deployment --> quarkus-smallrye-context-propagation-deployment --> quarkus-hibernate-orm-panache-deployment --> quarkus-hibernate-orm-deployment
So if vault depends on rest, and agroal depends on vault, we now have a cycle. The funny thing is that I was able to test my agroal code by doing a full build without the dependency and the code, then adding the dependency plus the new code and doing only a rebuild of the agroal module.

If the rest->agroal dependency cannot be amended, one solution would be to rely on an indirection (eg: config property resolver) as discussed in the google groups thread, which would make it easier to integrate in any other consumer as a side benefit.
https://groups.google.com/d/topic/quarkus-dev/Izy6KyqNtrk/discussion

But this is above my knowledge at this point.

I have left the code in agroal commented-out to support discussions if needed.

Beside this showstopper, I have left a few TODO in the code, and I had to disable the vault integration test on windows to make the build pass.

I hope this PR can be of value for discussions around vault's support in quarkus.

[vsevel]

As it turned out, the dependency cycle was only for RestClientFallbackTest.
To have a full build with agroal/vault integration in, I just moved this test into smallrye-opentracing, and removed the dependency quarkus-smallrye-fault-tolerance-deployment with test scope from quarkus-smallrye-rest-client-deployment.

[vsevel]

I completed support for a vault config source.
properties stored in vault can now be injected into quarkus's configuration and made available to the application or quarkus itself (ie: any component that uses MP config).

For instance you can write in vault:
vault kv put secret/vaultappconfig quarkus.datasource.password=mypass

Define the following policy & role in vault:

cat <<EOF | vault policy write mypolicy -
path "secret/vaultappconfig" {
  capabilities = ["read"]
  }
EOF

vault write auth/kubernetes/role/myapprole \
      bound_service_account_names=* \
      bound_service_account_namespaces=vaultapp \
      policies=mypolicy ttl=2h max_ttl=12h

Then all you need in application.properties is:

# vault auth (as usual)
quarkus.vault.url=https://vault.vault.svc:8200
quarkus.vault.role=myapprole

# vault path where are stored properties to be mounted in quarkus
quarkus.vault.secret-config-path=vaultappconfig

# property quarkus.datasource.password=mypass can now be used in datasource:
quarkus.datasource.driver=org.postgresql.Driver
quarkus.datasource.url=jdbc:postgresql://postgres:5432/mydb
quarkus.datasource.username=myuser
# no need to define quarkus.datasource.password, it is coming from vault
...

As a side benefit, url can be expressed naturally as quarkus.vault.url, and the rest client property io.quarkus.vault.VaultProxy/mp-rest/url will be transparently provisionned.

properties can also be used from the app through ConfigProvider.getConfig() or injection (ie. @ConfigProperty), like any other property.

properties coming from vault will be cached in the vault config source for some cache-period duration.

I refactored agroal's code to replace the use of SimplePassword by a new class ConfigPropertyPassword that draws the password value from the configuration.

Another benefit is that for that use case we do not need the dependency of agroal on the vault extension module (since we get the password from the configuration).
I kept the dependency anyway for the other use case: dynamic db credentials (cf VaultDynamicPassword).

The vault config source still honors token extension and renewal (ttl and max-ttl).

Since the config source is initialized before any modules are available (including the rest client), I had to rely on a simple apache httpclient for the few rest calls that are done from the vault config source.

The other features have been preserved (VaultService and VaultProxy rest client).

Beside the usual integration tests, a full example is provided in:
https://github.com/vsevel/vaultapp

[sberyozkin]

@vsevel can you please resolve the agroal extension conflict

[vsevel]

@vsevel can you please resolve the agroal extension conflict

done. thanks for your interest.

[sberyozkin]

@vsevel thanks; I do plan to experiment with your PR and I'm positive it will make it into the master eventually as the vault concept has its niche. Sorry it will take a bit of time (so more conflicts may need to be resolved going forward :-) ) since I've just returned from PTO and have to deal with some other pending issues.
The immediate comment I have: please check if VaultProxy can reuse the io.quarkus.vault.url property as opposed to it requiring the one following an MP RestClient format (if necessary, may be io.quarkus.vault.url can be replaced internally with io.quarkus.vault.VaultProxy/mp-rest/url if needed), otherwise there will be 2 properties for specifying the vault url.
Thanks

[vsevel]

@sberyozkin

please check if VaultProxy can reuse the io.quarkus.vault.url property as opposed to it requiring the one following an MP RestClient format
That is the case

io.quarkus.vault.url can be replaced internally with io.quarkus.vault.VaultProxy/mp-rest/url
It is not being replaced, it is being added. if we see a quarkus.vault.url property, then io.quarkus.vault.VaultProxy/mp-rest/url will be transparently provisionned from quarkus.vault.url.

on a different subject, I saw that the kubernetes client extension was using the okhttp http client library. may be that is a better fit than apache httpclient for the VaultConfigSource. I will experiment with it at some point.

[sberyozkin]

@vsevel I've commented on the source as well, you have a rest client dependency and Mp Rest Client proxy but I'm not seeing it being used, don't understand why the client HTTP invocations are coded directly with Apache Http Client. (and the Kub cllient code should probably have avoided bringing one more rest client library as well)

[vsevel]

@vsevel I've commented on the source as well, you have a rest client dependency and Mp Rest Client proxy but I'm not seeing it being used, don't understand why the client HTTP invocations are coded directly with Apache Http Client. (and the Kub cllient code should probably have avoided bringing one more rest client library as well)

the Mp Rest Client proxy is used in agroal (see VaultDynamicPassword), and client applications that would want to access the vault directly from their business logic.

like I said before, if the vault config source could use the rest client, that would simplify a lot.

to be honest, I was wondering if this PR was too ambitious. and I was thinking that may be it should focus on the config source use case, which is already bringing a lot of value (like the zookeeper config source). and providing a VaultProxy and upper abstraction VaultService could come later.

[sberyozkin]

@vsevel thanks for the clarifications so far.
It does feel like that may be the PR has few extra communication options, right now it has VaultProxy, VaultService, VaultAuthService, VaultDynamicDbSecretService. I think the PR is nearly ready to go but I do feel a single communication option should be available.

After looking more carefully at the code and the docs, I honestly think VaultService is the one that should stay because:

• it handles the login transparently
• it is smarter
• it handles the most typical case (lets users get the secrets)
• the users won't need straight away the operations like lookupLease/etc available on the proxy
• It works for agroal

As such I propose to drop VaultProxy, VaultAuthService, VaultDynamicDbSecretService, and have only VaultService supporting the mainstream operations only. It can communicate with Vault using JAX-RS Client API which is available in the quarkus-rest-client (I can help with the prototype of the JAX-RS client call).

What do you think ?

[vsevel]

@sberyozkin

in reply to your last comment

It does feel like that may be the PR has few extra communication options, right now it has VaultProxy, VaultService, VaultAuthService, VaultDynamicDbSecretService

VaultAuthService, VaultDynamicDbSecretService is internal impl stuff. I renamed those into XXXManager to not get them confused with VaultService, which is public api.

As such I propose to drop VaultProxy

even if we do not make VaultProxy available to the app developer, we still need it as an implementation detail, as VaultService needs it.
VaultProxy is the equivalent of the kubernetes client (should we rename it into VaultClient?). sometimes people will want the magic of VaultService, and sometimes they will want the control provided by VaultProxy.
I do not have any problem in hiding VaultProxy but I suspect that once we put out a vault extension, it will not be long before people start asking about a real vault client.

one way to approach it is to only document VaultService (from an app developer's perspective) and leave the rest as an impl detail (even if most of the classes are public).

I can help with the prototype of the JAX-RS client call

that would be awesome

summary

Here is a summary of things I believe we need to nail down:

• circular dependency vault -> rest -> agroal -> vault: see my comment Add support for Vault #2897 (comment) and how I solved it: Add support for Vault #2897 (comment). Is this appropriate?
• better support on k8s ca.crt? right now it is a pain to integrate with k8s's ca.crt because the rest client does not have a way (I believe) to use pem files directly (it needs a truststore). for instance something like https://github.com/BetterCloud/vault-java-driver/blob/master/src/main/java/com/bettercloud/vault/SslConfig.java
• a few TODOs I left in the code
  • VaultProcessor TODO: do we need to add something for ssl?
  • VaultRecorder TODO: is this the right way?
  • AbstractDataSourceProducer TODO: is this the right way
• split VaultProxy in multiple interfaces (auth, sys, kv, database)?
• and related to your last comment what use cases we retain and/or expose to the user in documentation
  • vault config source
  • vault proxy
  • vault service
    • dynamic credentials (low interest if not integrated in agroal)
    • get secrets
    • get token
  • agroal dynamic db credentials
• better documentation
• more integration testing, specifically around renewals (and time related issues)
• a better way to build a VaultServerRuntimeConfig than what is done in https://github.com/vsevel/quarkus/blob/master/extensions/vault/runtime/src/main/java/io/quarkus/vault/VaultConfigSource.java#L112
• build duration question as docker container (vault+postgres) are started&stopped for 3 tests: AgroalVaultITCase, AgroalVaultKv2ITCase & VaultITCase
• windows machine does not have docker apparently. see Add support for Vault #2897 (comment) and https://github.com/vsevel/quarkus/blob/master/extensions/vault/deployment/src/test/java/io/quarkus/vault/VaultITCase.java#L54

[sberyozkin]

@vsevel thanks...
OK, my immediate interest in reviewing this PR is to figure out how the users and the other extensions can use Vault to do this use case: retrieve the secrets, or the key material in general, such as a JWT set, the encoded pubic key or certificates.

So the 1st issue, before anything else, which I'd like to get resolved is VaultProxy vs VaultService.
As a user (or extension developer), all I need from the Vault from a start is this: get my secret or public key, that is it, I don't want to go into the low level mechanics of how the Vault operates in order to be able to get the secret.

This is why my proposal is to offer a REST client proxy (note does not mean it has to be implemented with MP RESTClient), whatever it is called, say VaultService reflecting that it is used to get the secrets (Proxy is a bit technical) which given the url and either k8s or user pass credentials, will return me a secret without me having to login, get a token, etc, all I need is to ask a service, give me a secret/keys.

VaultService does not need an internal REST proxy because it itself is a rest client which will be injected with the producer. It does not need an Apache HTTP code either, unless we fail to use JAX-RS client API at the config source initialization time. I may've missed few technical issues, sorry if yes.

Next, once the users start having more involved requirements, VaultService will grow. Ideally I'd only few methods related to getting the secrets only, the smart things like leases, etc, etc, this I'd consider doing internally, based on the configuration, for example, the user just keeps calling service.getSecret() and under the hood, it would all what is needed to get a refreshed secret with all the low-level calls as needed. Perhaps, a current VaultProxy version could be reintroduced for the sophisticated users which need an ultimate Vault control. But that can be done at the next stage, for the moment, it should be enough just to do a static secret acquisition.

Lets start from supporting the most main stream use case for the users to appreciate the vault faster, and they will drive it further.

Thoughts ?
Thank you for your patience.

[vsevel]

@sberyozkin hello. no problem in putting the focus on VaultService. But it is not clear to me what you want to do with VaultProxy. drop it? it is not possible as it is being used by VaultService. It is just a rest interface.
May be if you show me or describe exactly what you have in mind this might clear some of the confusion.
Also if you can get the VaultConfigSource to use the MP rest client, that would immediately simplify the impl and ease the discussion.

[sberyozkin]

Hi @vsevel, right, I suggest to drop the proxy class, and MP REST Client won't give the flexibility required to support the under the hood login/token acquisition/etc which are needed to have a higher level secret acquisition completed. So the idea is to have the Apache HTTP Client code replaced with the JAX-RS client API since it is offered by Quarkus natively and move it into VaultServiceImpl.
So one have a code like this, prototyping here:

class VaultServiceImpl implements VaultService {
   WebTarget vault;
   VaultServiceImpl (String serviceUrl) {
        vault = Clientbuilder.newClient().target(serviceUrl);
        login();
  }

 public VaultKVSecrets getKVSecrets(String path) {
     // lets say a Jackson provider will populate VaultKVSecrets from a returned JSON
     return target.path("secrets").request("application/json").get(VaultKVSecrets.class);
 }

 public VaultDBSecrets getDBSecrets(String dbrole) {
     // lets say a Jackson provider will populate VaultDBSecrets from a returned JSON
     return target.path("dbsecrets").request("application/json").get(VaultDbSecrets.class);
 }

 // slowly grow with more methods

 private void login() {
     target.path("login").post(new UserPass or K8SAuth);
 }
}

and this is it, VaultServiceProducer will create it, Agroal will use, the config source will use it, user code will use it. Inside we have the control required to do some smart things the Vault supports to ensure the secrets can be rotated, etc.

Does it make sense ? Thanks

[sberyozkin]

@vsevel thanks; it was a known issue already resolved for 0.24.0.

[sberyozkin]

@vsevel Hi Vincent, I'm a bit concerned about you keeping growing the PR, I mean it is a good thing, but I'm worried it can just delay the merge as the PR is large. We've done a lot of review work with yourself, and you are now going to finalize with @gsmet, so please wait a bit and then start applying small PRs against the master :-) Hope you agree, thanks

[vsevel]

hello @sberyozkin

you are right. as the PR moved from one milestone to another, I thought I had time to implement the approle authentication, which is probably more useful than userpass, if the app+vault is running outside k8s.

and I also refactored the config for authentication and tls (moved properties in dedicated classes, rather than having everything at the root level). I thought this was better to do it now, than doing breaking changes later. plus I was able to drop some properties as well (authentication_type is now inferred, and the ca cert is also defaulted when running in k8s). this should improve the developer experience.
note that doing those changes did not change the overall design and code organization we worked on.
this was the last changes I had in mind. so we are in line.

btw I have the call with guillaume on thursday. so I will know more. I will do some more testing in the meantime.

the last build failed with an error that seemed to affect other builds as well WebsocketITCase>WebsocketTestCase.testSendMessageOnOpen:124 expected: <23.0> but was: <null>. I will rebase tomorrow.

thanks for your benevolent eye

[gsmet]

I added 2 minor additional comments following our discussion this morning. Can you take care of them and then squash everything?

But I have a bigger issue that I didn't think of during our discussions: we don't want to mandate Docker when running the tests. So somehow these tests should be disabled and we should have a -Dtest-vault to enable them if they need Docker to be there. See what we did in the Elasticsearch IT.

I got that because I wasn't able to get the tests to run.

I have:

Caused by: java.lang.IllegalStateException: Can not connect to Ryuk

for one

And:

Caused by: org.testcontainers.containers.ContainerLaunchException: Timed out waiting for container port to open (localhost ports: [8200] should be listening)
	at org.testcontainers.containers.wait.strategy.HostPortWaitStrategy.waitUntilReady(HostPortWaitStrategy.java:47)
	at org.testcontainers.containers.wait.strategy.AbstractWaitStrategy.waitUntilReady(AbstractWaitStrategy.java:35)
	at org.testcontainers.containers.wait.HostPortWaitStrategy.waitUntilReady(HostPortWaitStrategy.java:23)
	at org.testcontainers.containers.wait.strategy.AbstractWaitStrategy.waitUntilReady(AbstractWaitStrategy.java:35)
	at org.testcontainers.containers.GenericContainer.waitUntilContainerStarted(GenericContainer.java:675)
	at org.testcontainers.containers.GenericContainer.tryStart(GenericContainer.java:332)

for the other.

The first one, I have no clue but maybe it's related to the second one?

The second one might be due to the fact that sudo is needed to start a Docker container on my box. But it would be nice to be able to run the tests somehow.

Full test report:

[INFO] Running io.quarkus.vault.VaultAppRoleITCase
[ERROR] Tests run: 1, Failures: 0, Errors: 1, Skipped: 0, Time elapsed: 36.364 s <<< FAILURE! - in io.quarkus.vault.VaultAppRoleITCase
[ERROR] io.quarkus.vault.VaultAppRoleITCase  Time elapsed: 36.361 s  <<< ERROR!
java.lang.RuntimeException: Unable to start Quarkus test resource io.quarkus.vault.test.VaultTestLifecycleManager@39a2bb97
	at io.quarkus.test.common.TestResourceManager.start(TestResourceManager.java:45)
	at io.quarkus.test.QuarkusUnitTest.beforeAll(QuarkusUnitTest.java:205)
	at org.junit.jupiter.engine.descriptor.ClassBasedTestDescriptor.lambda$invokeBeforeAllCallbacks$7(ClassBasedTestDescriptor.java:355)
	at org.junit.platform.engine.support.hierarchical.ThrowableCollector.execute(ThrowableCollector.java:73)
	at org.junit.jupiter.engine.descriptor.ClassBasedTestDescriptor.invokeBeforeAllCallbacks(ClassBasedTestDescriptor.java:355)
	at org.junit.jupiter.engine.descriptor.ClassBasedTestDescriptor.before(ClassBasedTestDescriptor.java:189)
	at org.junit.jupiter.engine.descriptor.ClassBasedTestDescriptor.before(ClassBasedTestDescriptor.java:77)
	at org.junit.platform.engine.support.hierarchical.NodeTestTask.lambda$executeRecursively$5(NodeTestTask.java:132)
	at org.junit.platform.engine.support.hierarchical.ThrowableCollector.execute(ThrowableCollector.java:73)
	at org.junit.platform.engine.support.hierarchical.NodeTestTask.lambda$executeRecursively$7(NodeTestTask.java:125)
	at org.junit.platform.engine.support.hierarchical.Node.around(Node.java:135)
	at org.junit.platform.engine.support.hierarchical.NodeTestTask.lambda$executeRecursively$8(NodeTestTask.java:123)
	at org.junit.platform.engine.support.hierarchical.ThrowableCollector.execute(ThrowableCollector.java:73)
	at org.junit.platform.engine.support.hierarchical.NodeTestTask.executeRecursively(NodeTestTask.java:122)
	at org.junit.platform.engine.support.hierarchical.NodeTestTask.execute(NodeTestTask.java:80)
	at java.util.ArrayList.forEach(ArrayList.java:1257)
	at org.junit.platform.engine.support.hierarchical.SameThreadHierarchicalTestExecutorService.invokeAll(SameThreadHierarchicalTestExecutorService.java:38)
	at org.junit.platform.engine.support.hierarchical.NodeTestTask.lambda$executeRecursively$5(NodeTestTask.java:139)
	at org.junit.platform.engine.support.hierarchical.ThrowableCollector.execute(ThrowableCollector.java:73)
	at org.junit.platform.engine.support.hierarchical.NodeTestTask.lambda$executeRecursively$7(NodeTestTask.java:125)
	at org.junit.platform.engine.support.hierarchical.Node.around(Node.java:135)
	at org.junit.platform.engine.support.hierarchical.NodeTestTask.lambda$executeRecursively$8(NodeTestTask.java:123)
	at org.junit.platform.engine.support.hierarchical.ThrowableCollector.execute(ThrowableCollector.java:73)
	at org.junit.platform.engine.support.hierarchical.NodeTestTask.executeRecursively(NodeTestTask.java:122)
	at org.junit.platform.engine.support.hierarchical.NodeTestTask.execute(NodeTestTask.java:80)
	at org.junit.platform.engine.support.hierarchical.SameThreadHierarchicalTestExecutorService.submit(SameThreadHierarchicalTestExecutorService.java:32)
	at org.junit.platform.engine.support.hierarchical.HierarchicalTestExecutor.execute(HierarchicalTestExecutor.java:57)
	at org.junit.platform.engine.support.hierarchical.HierarchicalTestEngine.execute(HierarchicalTestEngine.java:51)
	at org.junit.platform.launcher.core.DefaultLauncher.execute(DefaultLauncher.java:170)
	at org.junit.platform.launcher.core.DefaultLauncher.execute(DefaultLauncher.java:154)
	at org.junit.platform.launcher.core.DefaultLauncher.execute(DefaultLauncher.java:90)
	at org.apache.maven.surefire.junitplatform.JUnitPlatformProvider.invokeAllTests(JUnitPlatformProvider.java:142)
	at org.apache.maven.surefire.junitplatform.JUnitPlatformProvider.invoke(JUnitPlatformProvider.java:117)
	at org.apache.maven.surefire.booter.ForkedBooter.invokeProviderInSameClassLoader(ForkedBooter.java:383)
	at org.apache.maven.surefire.booter.ForkedBooter.runSuitesInProcess(ForkedBooter.java:344)
	at org.apache.maven.surefire.booter.ForkedBooter.execute(ForkedBooter.java:125)
	at org.apache.maven.surefire.booter.ForkedBooter.main(ForkedBooter.java:417)
Caused by: java.lang.IllegalStateException: Can not connect to Ryuk
	at org.testcontainers.utility.ResourceReaper.start(ResourceReaper.java:149)
	at org.testcontainers.DockerClientFactory.client(DockerClientFactory.java:131)
	at org.testcontainers.containers.GenericContainer.<init>(GenericContainer.java:175)
	at org.testcontainers.containers.JdbcDatabaseContainer.<init>(JdbcDatabaseContainer.java:36)
	at org.testcontainers.containers.PostgreSQLContainer.<init>(PostgreSQLContainer.java:32)
	at org.testcontainers.containers.PostgreSQLContainer.<init>(PostgreSQLContainer.java:28)
	at io.quarkus.vault.test.VaultTestExtension.start(VaultTestExtension.java:120)
	at io.quarkus.vault.test.VaultTestLifecycleManager.start(VaultTestLifecycleManager.java:32)
	at io.quarkus.test.common.TestResourceManager.start(TestResourceManager.java:43)
	... 36 more

[INFO] Running io.quarkus.vault.VaultITCase
[ERROR] Tests run: 1, Failures: 0, Errors: 1, Skipped: 0, Time elapsed: 87.583 s <<< FAILURE! - in io.quarkus.vault.VaultITCase
[ERROR] io.quarkus.vault.VaultITCase  Time elapsed: 87.583 s  <<< ERROR!
java.lang.RuntimeException: Unable to start Quarkus test resource io.quarkus.vault.test.VaultTestLifecycleManager@1dd6d4b7
	at io.quarkus.test.common.TestResourceManager.start(TestResourceManager.java:45)
	at io.quarkus.test.QuarkusUnitTest.beforeAll(QuarkusUnitTest.java:205)
	at org.junit.jupiter.engine.descriptor.ClassBasedTestDescriptor.lambda$invokeBeforeAllCallbacks$7(ClassBasedTestDescriptor.java:355)
	at org.junit.platform.engine.support.hierarchical.ThrowableCollector.execute(ThrowableCollector.java:73)
	at org.junit.jupiter.engine.descriptor.ClassBasedTestDescriptor.invokeBeforeAllCallbacks(ClassBasedTestDescriptor.java:355)
	at org.junit.jupiter.engine.descriptor.ClassBasedTestDescriptor.before(ClassBasedTestDescriptor.java:189)
	at org.junit.jupiter.engine.descriptor.ClassBasedTestDescriptor.before(ClassBasedTestDescriptor.java:77)
	at org.junit.platform.engine.support.hierarchical.NodeTestTask.lambda$executeRecursively$5(NodeTestTask.java:132)
	at org.junit.platform.engine.support.hierarchical.ThrowableCollector.execute(ThrowableCollector.java:73)
	at org.junit.platform.engine.support.hierarchical.NodeTestTask.lambda$executeRecursively$7(NodeTestTask.java:125)
	at org.junit.platform.engine.support.hierarchical.Node.around(Node.java:135)
	at org.junit.platform.engine.support.hierarchical.NodeTestTask.lambda$executeRecursively$8(NodeTestTask.java:123)
	at org.junit.platform.engine.support.hierarchical.ThrowableCollector.execute(ThrowableCollector.java:73)
	at org.junit.platform.engine.support.hierarchical.NodeTestTask.executeRecursively(NodeTestTask.java:122)
	at org.junit.platform.engine.support.hierarchical.NodeTestTask.execute(NodeTestTask.java:80)
	at java.util.ArrayList.forEach(ArrayList.java:1257)
	at org.junit.platform.engine.support.hierarchical.SameThreadHierarchicalTestExecutorService.invokeAll(SameThreadHierarchicalTestExecutorService.java:38)
	at org.junit.platform.engine.support.hierarchical.NodeTestTask.lambda$executeRecursively$5(NodeTestTask.java:139)
	at org.junit.platform.engine.support.hierarchical.ThrowableCollector.execute(ThrowableCollector.java:73)
	at org.junit.platform.engine.support.hierarchical.NodeTestTask.lambda$executeRecursively$7(NodeTestTask.java:125)
	at org.junit.platform.engine.support.hierarchical.Node.around(Node.java:135)
	at org.junit.platform.engine.support.hierarchical.NodeTestTask.lambda$executeRecursively$8(NodeTestTask.java:123)
	at org.junit.platform.engine.support.hierarchical.ThrowableCollector.execute(ThrowableCollector.java:73)
	at org.junit.platform.engine.support.hierarchical.NodeTestTask.executeRecursively(NodeTestTask.java:122)
	at org.junit.platform.engine.support.hierarchical.NodeTestTask.execute(NodeTestTask.java:80)
	at org.junit.platform.engine.support.hierarchical.SameThreadHierarchicalTestExecutorService.submit(SameThreadHierarchicalTestExecutorService.java:32)
	at org.junit.platform.engine.support.hierarchical.HierarchicalTestExecutor.execute(HierarchicalTestExecutor.java:57)
	at org.junit.platform.engine.support.hierarchical.HierarchicalTestEngine.execute(HierarchicalTestEngine.java:51)
	at org.junit.platform.launcher.core.DefaultLauncher.execute(DefaultLauncher.java:170)
	at org.junit.platform.launcher.core.DefaultLauncher.execute(DefaultLauncher.java:154)
	at org.junit.platform.launcher.core.DefaultLauncher.execute(DefaultLauncher.java:90)
	at org.apache.maven.surefire.junitplatform.JUnitPlatformProvider.invokeAllTests(JUnitPlatformProvider.java:142)
	at org.apache.maven.surefire.junitplatform.JUnitPlatformProvider.invoke(JUnitPlatformProvider.java:117)
	at org.apache.maven.surefire.booter.ForkedBooter.invokeProviderInSameClassLoader(ForkedBooter.java:383)
	at org.apache.maven.surefire.booter.ForkedBooter.runSuitesInProcess(ForkedBooter.java:344)
	at org.apache.maven.surefire.booter.ForkedBooter.execute(ForkedBooter.java:125)
	at org.apache.maven.surefire.booter.ForkedBooter.main(ForkedBooter.java:417)
Caused by: org.testcontainers.containers.ContainerLaunchException: Container startup failed
	at org.testcontainers.containers.GenericContainer.doStart(GenericContainer.java:290)
	at org.testcontainers.containers.GenericContainer.start(GenericContainer.java:272)
	at io.quarkus.vault.test.VaultTestExtension.start(VaultTestExtension.java:153)
	at io.quarkus.vault.test.VaultTestLifecycleManager.start(VaultTestLifecycleManager.java:32)
	at io.quarkus.test.common.TestResourceManager.start(TestResourceManager.java:43)
	... 36 more
Caused by: org.rnorth.ducttape.RetryCountExceededException: Retry limit hit with exception
	at org.rnorth.ducttape.unreliables.Unreliables.retryUntilSuccess(Unreliables.java:88)
	at org.testcontainers.containers.GenericContainer.doStart(GenericContainer.java:283)
	... 40 more
Caused by: org.testcontainers.containers.ContainerLaunchException: Could not create/start container
	at org.testcontainers.containers.GenericContainer.tryStart(GenericContainer.java:350)
	at org.testcontainers.containers.GenericContainer.lambda$doStart$0(GenericContainer.java:285)
	at org.rnorth.ducttape.unreliables.Unreliables.retryUntilSuccess(Unreliables.java:81)
	... 41 more
Caused by: org.testcontainers.containers.ContainerLaunchException: Timed out waiting for container port to open (localhost ports: [8200] should be listening)
	at org.testcontainers.containers.wait.strategy.HostPortWaitStrategy.waitUntilReady(HostPortWaitStrategy.java:47)
	at org.testcontainers.containers.wait.strategy.AbstractWaitStrategy.waitUntilReady(AbstractWaitStrategy.java:35)
	at org.testcontainers.containers.wait.HostPortWaitStrategy.waitUntilReady(HostPortWaitStrategy.java:23)
	at org.testcontainers.containers.wait.strategy.AbstractWaitStrategy.waitUntilReady(AbstractWaitStrategy.java:35)
	at org.testcontainers.containers.GenericContainer.waitUntilContainerStarted(GenericContainer.java:675)
	at org.testcontainers.containers.GenericContainer.tryStart(GenericContainer.java:332)
	... 43 more

[sberyozkin]

@vsevel np :-), and now with another benevolent eye from @gsmet it does feel like the Vault moment is close :-)

[vsevel]

hello @gsmet
I did make the different changes, but was not able to get a full CI build so far because of the CI cache issue

I did add the test-vault activation property in vsevel@4afe1fd

Caused by: java.lang.IllegalStateException: Can not connect to Ryuk

ryuk is a resource reaper used by testcontainers to cleanup containers, even when tests fail.

Caused by: org.testcontainers.containers.ContainerLaunchException: Timed out waiting for container port to open (localhost ports: [8200] should be listening)

testcontainers knows vault has started by doing a test on vault's port (8200)

I will track progress on the CI cache issue, and trigger a CI build as soon as possible. once I get there I will do a final squash.

[vsevel]

hello @sberyozkin & @gsmet
I finally got a full build to pass, after finding a workaround for the docs generation CI issue, which has been affecting a lot of builds. I provided some feedback in the zulip thread
beside making corrections based on your feedbacks guillaume, I made some final adjustments plus corrected minor issues based on additional testing.
I updated also my quarkus vault kubernetes tutorial, and checked everything worked as expected.
as far as I am concerned, this should be ready for PR merge.
let me know if there are fine tuning corrections you want me to do.
I will do a final squash on monday.

[lordofthejars]

Oh yes please, hope I can start enjoying this extension ASAP.

[gsmet]

@vsevel @sberyozkin and merged!

Thanks @vsevel for your hard work and your patience. I will add a small follow-up commit to add a new metadata file we added recently.

[sberyozkin]

@vsevel Congratulations !
@gsmet Thanks for helping to tune it :-)

[vsevel]

fantastic!

thanks so much to both of you. you too had a lot of patience, and a pragmatic mindset that made it possible.

it would have been easy for you @gsmet to take any of the current limitations (IT not running on windows because of the lack of docker installed, testcontainers usage not consistent with other ITs, rewritten config object creation in the vault config source, okhttpclient instead of jax-rs client), or pretend you were too busy (and that would have been true) and say that this was an interesting idea, but is not ready for prime time.

and it would have been easy for you @sberyozkin to just let it go after so many back and forth discussions - how many did we have in the end??

your comments and guidance were decisive to transform an interesting poc into a product feature.

obviously quarkus does not stop here for me. I have a few follow-ups lined up:

• quarkus vault guide for kubernetes
• quarkus vault guide for dynamic db secrets
• github issues for current design limitations (whether I do them or not)
• a summary of all of these elements in the google group
• listen for feedback
• the issue you encountered guillaume around testing in fedora

I try to monitor conversations on the google group (but not every day). and I am almost never on zulip. so if anything pops up for me, do not hesitate to reach out. I do not plan to disappear.

looking forward for more work with the great quarkus community.

[machi1990]

@vsevel really good to see this one in. Looking forward to see the vault power. Also congratulations.

[vsevel]

@rsvoboda hello,
when you authenticate with vault (using any of the methods such as kubernetes, approle, userpass, ...) you get a response that looks like:

{
  "request_id": "9e923c23-3a45-ff0f-a3ea-5fffd94f1e2f",
  "lease_id": "",
  "renewable": false,
  "lease_duration": 0,
  "data": null,
  "wrap_info": null,
  "warnings": null,
  "auth": {
    "client_token": "s.aWvAbfqWpoqPpbTebdwDEVuu",
    "accessor": "abbJ8vD3JghuH37Up5d0SLYM",
    "policies": [
      "default",
      "mypolicy"
    ],
    "token_policies": [
      "default",
      "mypolicy"
    ],
    "metadata": {
      "role": "myapprole",
      "service_account_name": "default",
      "service_account_namespace": "vaultapp",
      "service_account_secret_name": "default-token-qj4b5",
      "service_account_uid": "27c26105-92d3-11e9-9202-025000000001"
    },
    "lease_duration": 7200,
    "renewable": true,
    "entity_id": "62f850bb-4835-a9ea-471b-006a92017128",
    "token_type": "service",
    "orphan": true
  }
}

Those objects are mapped to XXXAuth objects, such as VaultKubernetesAuth, VaultAppRoleAuth, VaultUserPassAuth (those are the only supported auth methods in the extension at the moment).

then inside those objects we find several attributes plus a auth attribute, mapped onto the VaultKubernetesAuthAuth object for kubernetes, and so on so forth. In the auth-auth structure there is some metadata, hence the name VaultAppRoleAuthAuthMetadata (I just followed the hierarchy).

these are json dtos internal to the extension. it might have been less confusing if the top level object had been named VaultKubernetesLoginResponse, containing a VaultKubernetesAuth for instance. but again those are technical classes, not public api.

[rsvoboda]

@vsevel thank you for details