Integrate Vertx HTTP with CredentialsProvider #26380

sberyozkin · 2022-06-27T13:22:46Z

This PR makes it possible to retrieve Vertx HTTP SSL keystore, keystore key and truststore passwords from CredentialsProvider.

Using ConfigSource is possible right now, however some users/customers don't like the idea of providing the secrets via ConfigSource, so integrating it with CredentialsProvider provides for a ConfigSource-like solution but via CrdentialsProvider.

CredentialsProvider was originally meant to return a (DB) password only, now also can return a user name, but it returns a Map so returning other secrets which, in case of the keystore, can be considered as admin credentails, is also possible.

For example, after this PR we can advise the users/customers that they can intergrate quarkus-vertx-http with https://github.com/quarkiverse/quarkus-file-vault/.

HashiCorp Vault CredentialsProvider will need to be expanded a bit to be able to return not only the username and password but more custom properties

...nsions/vertx-http/runtime/src/main/java/io/quarkus/vertx/http/runtime/VertxHttpRecorder.java

cescoffier · 2022-06-28T14:27:52Z

...nsions/vertx-http/runtime/src/main/java/io/quarkus/vertx/http/runtime/VertxHttpRecorder.java

+        Map<String, String> credentials = Map.of();
+        if (sslConfig.certificate.credentialsProvider.isPresent()) {
+            String beanName = sslConfig.certificate.credentialsProviderName.orElse(null);
+            CredentialsProvider credentialsProvider = CredentialsProviderFinder.find(beanName);


How does this lookup work? Is it using @Identifier?

It looks for the named beans such as this one, this pattern of checking it is used across various Quarkus extensions. This name does not have to be specified if there is only one bean, that look up code will return the single bean it will find, only needed if several different providers are used.
Once the provider is found, the string (such as custom used in this PR's test) is passed to the found provider to return the credentials, it is not used here, but in Quarkus File Vault it will identify the keystore where to get the secrets from: https://quarkiverse.github.io/quarkiverse-docs/quarkus-file-vault/dev/index.html#_how_to_use_the_extension, for example:

quarkus.datasource.credentials-provider=quarkus.file.vault.provider.db1 quarkus.file.vault.provider.db1.path=dbpasswords.p12 quarkus.file.vault.provider.db1.secret=storepassword quarkus.file.vault.provider.db1.alias=quarkus_test

So here Agroal will pass quarkus.file.vault.provider.db1 to the provider which will figure out that db1 is a keystore tag, and use its db1 configuration to get the data, etc

Yeah... it uses @nAmed. I don't believe we want this. @mkouba WDYT?
My guess is that a proper qualifier would be better.

@cescoffier Sure; @Named would only come into effect if there are more than one CredentialsProvider loaded, with a single one it make no difference. Not sure we can change that though as a few such providers use @Named (the one in quarkus-vault for example)

@cescoffier Sure; @Named would only come into effect if there are more than one CredentialsProvider loaded, with a single one it makes no difference. Not sure we can change that though as a few such providers use @Named (the one in quarkus-vault (the one based aroiund HashiCorp Vault) and also quarkus-file-vault, but there could be custom ones too)

Yeah... it uses @nAmed. I don't believe we want this. @mkouba WDYT? My guess is that a proper qualifier would be better.

+1

I guess it is not something that we can do in this PR, right ?

@cescoffier @mkouba As far as this PR is concerned, I can make a credentials-provider-name property not optional but required if you are concerned about a random provider being picked up, it will ensure that only a provider with a matching Named value will be picked up.

I think the actual CredentialsProvider resolution logic update is out of scope for this PR as many extensions depend on it (making the credentials-provider-name required for all those extensions can be the 1st step forward), changing it willbe a breaking change and will make this PR non-backportable if it will have to be backported.

@cescoffier I've noticed you've approved, thanks, but your concern about @Named is noted for sure. As I said, I can enforce at the vertx-http level that credentials-provider-name is required, however, I'm having a 2nd thought about it now, if CredentialsProvider resolution logic will indeed get changed in the future then having credentials-provider-name explicitly required may make it harder to migrate.
Perhaps a reasonable compromise is to recommend in JavaDocs of this property that it should be used, will do it now

Just pushed a JavaDoc update recommending to use the name property all the time

quarkus-bot · 2022-07-05T18:53:12Z

Failing Jobs - Building `8f2f633`

Status	Name	Step	Failures	Logs	Raw logs
✔️	Gradle Tests - JDK 11
✖	Gradle Tests - JDK 11 Windows	`Build`	Failures	Logs	Raw logs
✔️	JVM Tests - JDK 11
✖	JVM Tests - JDK 11 Windows	`Build`	Failures	Logs	Raw logs
✔️	JVM Tests - JDK 17
✔️	JVM Tests - JDK 18

Full information is available in the Build summary check run.

Failures

⚙️ Gradle Tests - JDK 11 Windows #

- Failing: integration-tests/gradle

📦 integration-tests/gradle

✖ io.quarkus.gradle.devmode.CompositeBuildWithDependenciesDevModeTest.main line 24 - More details - Source on GitHub

org.awaitility.core.ConditionTimeoutException: Condition with lambda expression in io.quarkus.test.devmode.util.DevModeTestUtils that uses java.util.function.Supplier, java.util.function.Supplierjava.util.concurrent.atomic.AtomicReference, java.util.concurrent.atomic.AtomicReferencejava.lang.String, java.lang.Stringboolean was not fulfilled within 1 minutes.
	at org.awaitility.core.ConditionAwaiter.await(ConditionAwaiter.java:167)
	at org.awaitility.core.CallableCondition.await(CallableCondition.java:78)

⚙️ JVM Tests - JDK 11 Windows #

- Failing: extensions/mongodb-client/deployment 
! Skipped: extensions/liquibase-mongodb/deployment extensions/panache/mongodb-panache-common/deployment extensions/panache/mongodb-panache-kotlin/deployment and 8 more

📦 extensions/mongodb-client/deployment

✖ io.quarkus.mongodb.NamedReactiveMongoClientConfigTest. - More details - Source on GitHub

java.lang.RuntimeException: 
java.lang.RuntimeException: io.quarkus.builder.BuildException: Build failure: Build failed due to errors
	[error]: Build step io.quarkus.mongodb.deployment.DevServicesMongoProcessor#startMongo threw an exception: java.lang.RuntimeException: java.lang.IllegalStateException: Previous attempts to find a Docker environment failed. Will not retry. Please see logs and check configuration

sberyozkin · 2022-07-06T09:46:38Z

@cescoffier Let me go ahead with the merge then, we can tune a few things for sure as needed. Please open an issue dedicated to changing CredentialsProvider resolution logic in io.quarkus.credentials.runtime.CredentialsProviderFinder for all to discuss.

gsmet · 2023-01-13T10:35:18Z

Given we already released 2.13, @rsvoboda and I don't think it's a good idea to backport it to 2.7 given it's a new feature.

sberyozkin requested review from stuartwdouglas and cescoffier June 27, 2022 13:22

quarkus-bot bot added the area/vertx label Jun 27, 2022

This comment has been minimized.

Sign in to view

cescoffier requested changes Jun 28, 2022

View reviewed changes

sberyozkin force-pushed the vertx_http_credentials_provider branch from 04c7a4d to 80c837e Compare June 28, 2022 17:17

This comment has been minimized.

Sign in to view

cescoffier approved these changes Jul 5, 2022

View reviewed changes

Integrate Vertx HTTP with CredentialsProvider

8f2f633

sberyozkin force-pushed the vertx_http_credentials_provider branch from 80c837e to 8f2f633 Compare July 5, 2022 14:10

sberyozkin merged commit 20964b6 into quarkusio:main Jul 6, 2022

quarkus-bot bot added the kind/enhancement New feature or request label Jul 6, 2022

quarkus-bot bot added this to the 2.11 - main milestone Jul 6, 2022

sberyozkin deleted the vertx_http_credentials_provider branch July 6, 2022 09:46

sberyozkin added triage/backport-2.10? labels Jul 14, 2022

gsmet removed the triage/backport-2.10? label Jul 18, 2022

gsmet removed the triage/backport-2.7? label Jan 13, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Integrate Vertx HTTP with CredentialsProvider #26380

Integrate Vertx HTTP with CredentialsProvider #26380

sberyozkin commented Jun 27, 2022 •

edited

Loading

This comment has been minimized.

cescoffier Jun 28, 2022

sberyozkin Jun 28, 2022 •

edited

Loading

cescoffier Jul 1, 2022

sberyozkin Jul 1, 2022

sberyozkin Jul 1, 2022 •

edited

Loading

mkouba Jul 4, 2022

sberyozkin Jul 5, 2022

sberyozkin Jul 5, 2022

sberyozkin Jul 5, 2022

sberyozkin Jul 5, 2022

This comment has been minimized.

quarkus-bot bot commented Jul 5, 2022

sberyozkin commented Jul 6, 2022

gsmet commented Jan 13, 2023

Integrate Vertx HTTP with CredentialsProvider #26380

Integrate Vertx HTTP with CredentialsProvider #26380

Conversation

sberyozkin commented Jun 27, 2022 • edited Loading

This comment has been minimized.

Choose a reason for hiding this comment

sberyozkin Jun 28, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sberyozkin Jul 1, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

This comment has been minimized.

quarkus-bot bot commented Jul 5, 2022

Failing Jobs - Building 8f2f633

Failures

⚙️ Gradle Tests - JDK 11 Windows #

📦 integration-tests/gradle

⚙️ JVM Tests - JDK 11 Windows #

📦 extensions/mongodb-client/deployment

sberyozkin commented Jul 6, 2022

gsmet commented Jan 13, 2023

sberyozkin commented Jun 27, 2022 •

edited

Loading

sberyozkin Jun 28, 2022 •

edited

Loading

sberyozkin Jul 1, 2022 •

edited

Loading

Failing Jobs - Building `8f2f633`