Move REST Client configuration to use @ConfigMapping

[radcortez]

I've tried to be somehow conservative with the changes (if you can call this PR that :))

The old REST Client configuration performed look-ups to all acceptable names and merged the final configuration. The standard Config system does not support this (and other extensions don't either). It was specifically implemented with the old behaviour, which is also supported in the new implementation via fallbacks, except for a minor breaking change: simple names REST Client names quoted cannot be mixed with unquoted and FQN. This can easily be added if required with another fallback.

• For Pinned thread on Vault fetch secret #41313
• Removes the CDI dependency from the RestClientConfig
• Removes all the custom config populate code and relies only in the standard Config system

[geoand]

This will conflict head-on with #42100 :)

[radcortez]

This will conflict head-on with #42100 :)

Ops... this was because of #41948 (comment). This should also help with the config overhead. The mapping will load and cache the config at startup time.

Sorry for the extra work :(

[gsmet]

Yeah, so, I won't fight this one as already done but let's not start a massive rewrite of extensions to @ConfigMapping because that makes things a moving target for me. Thanks!

[geoand]

Sorry for the extra work :(

No problem. If we can get something better for REST config, I'm all for it because it's a real pain to maintain

[radcortez]

Yeah, so, I won't fight this one as already done but let's not start a massive rewrite of extensions to @ConfigMapping because that makes things a moving target for me. Thanks!

The intent was not to start a massive rewrite of extensions but to fix only the REST Client config since it neither uses the old @ConfigRoot class implementation nor @ConfigMapping, but a custom-based implementation with CDI dependencies.

One annoying issue is this:

• Pinned thread on Vault fetch secret #41313

And I guess #42092, which I overlooked but would also help.

[gsmet]

Yeah, I understand that, I just wanted it to be clear :).

[geoand]

Assuming CI passes, this is great!

[gsmet]

At least some of the failures look related.

[geoand]

The test failure seems related, but it's probably easily fixable

[radcortez]

The problem here is how we handle the test url properties. There is some sanitization code to remove the ending slash and rewrite the configuration as a system property.

Some pieces of the code may see different configuration values depending on when they are executed. In this case, the REST Client configuration was only queried when the REST Client was created (after the test URL was sanitized). Now, it is created at startup (before the test URL was sanitized). For the REST Client execution, the sanitization doesn't matter, but the tracing code asserted the client URL without an ending slash, which was removed by the sanitization.

I've added a separate commit to handle the sanitization at the config level, so all executions should see the same values.

As a separate discussion, we must consider if changing the configuration after startup and rewriting the system properties is the proper behaviour. It is convenient for sure, and we haven't had many issues around it, but I think it is a problem. See also #41957.

[geoand]

As a separate discussion, we must consider if changing the configuration after startup and rewriting the system properties is the proper behaviour.

We are doing this?

[radcortez]

We are doing this?

Yes. Here are a couple of examples:

• quarkus/test-framework/junit5/src/main/java/io/quarkus/test/junit/QuarkusTestExtension.java

  Lines 273 to 274 in eef5138

  	RestorableSystemProperties restorableSystemProperties = RestorableSystemProperties.setProperties(
  	Collections.singletonMap("test.url", TestHTTPResourceManager.getUri(runningQuarkusApplication)));

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

  Lines 1282 to 1295 in 10654d6

  	if (actualPort != options.getPort()) {
  	// Override quarkus.http(s)?.(test-)?port
  	String schema;
  	if (https) {
  	clearHttpsProperty = true;
  	schema = "https";
  	} else {
  	clearHttpProperty = true;
  	actualHttpPort = actualPort;
  	schema = "http";
  	}
  	portSystemProperties = new PortSystemProperties();
  	portSystemProperties.set(schema, actualPort, launchMode);
  	}

I'm unsure if this is only required for testing or if we have cases where we rely on system properties changed in the prod profile. I'll have to analyze it.

[geoand]

Okay, those are the ones I had in mind

[quarkus-bot]

Status for workflow Quarkus CI

This is the status report for running Quarkus CI on commit 044af63.

Failing Jobs

Status	Name	Step	Failures	Logs	Raw logs	Build scan
✔️	JVM Tests - JDK 17	Build		Logs	Raw logs	🔍
✔️	JVM Tests - JDK 21	Build		Logs	Raw logs	🔍
✖	JVM Tests - JDK 17 Windows	Build	Failures	Logs	Raw logs	🔍

Full information is available in the Build summary check run.
You can consult the Develocity build scans.

Failures

⚙️ JVM Tests - JDK 17 Windows #

- Failing: extensions/smallrye-reactive-messaging-amqp/deployment 
! Skipped: integration-tests/reactive-messaging-amqp integration-tests/virtual-threads/amqp-virtual-threads 

📦 extensions/smallrye-reactive-messaging-amqp/deployment

✖ io.quarkus.smallrye.reactivemessaging.amqp.AnonymousAmqpTest.test line 30 - History - More details - Source on GitHub

Details

org.awaitility.core.ConditionTimeoutException: Condition with Lambda expression in io.quarkus.smallrye.reactivemessaging.amqp.AnonymousAmqpTest was not fulfilled within 1 minutes.
	at org.awaitility.core.ConditionAwaiter.await(ConditionAwaiter.java:167)
	at org.awaitility.core.CallableCondition.await(CallableCondition.java:78)
	at org.awaitility.core.CallableCondition.await(CallableCondition.java:26)
	at org.awaitility.core.ConditionFactory.until(ConditionFactory.java:1006)
	at org.awaitility.core.ConditionFactory.until(ConditionFactory.java:975)
	at io.quarkus.smallrye.reactivemessaging.amqp.AnonymousAmqpTest.test(AnonymousAmqpTest.java:30)
	at java.base/java.lang.reflect.Method.invoke(Method.java:569)

✖ io.quarkus.smallrye.reactivemessaging.amqp.SecuredAmqpTest.test line 28 - History - More details - Source on GitHub

Details

org.awaitility.core.ConditionTimeoutException: Condition with Lambda expression in io.quarkus.smallrye.reactivemessaging.amqp.SecuredAmqpTest was not fulfilled within 10 seconds.
	at org.awaitility.core.ConditionAwaiter.await(ConditionAwaiter.java:167)
	at org.awaitility.core.CallableCondition.await(CallableCondition.java:78)
	at org.awaitility.core.CallableCondition.await(CallableCondition.java:26)
	at org.awaitility.core.ConditionFactory.until(ConditionFactory.java:1006)
	at org.awaitility.core.ConditionFactory.until(ConditionFactory.java:975)
	at io.quarkus.smallrye.reactivemessaging.amqp.SecuredAmqpTest.test(SecuredAmqpTest.java:28)
	at java.base/java.lang.reflect.Method.invoke(Method.java:569)

✖ io.quarkus.smallrye.reactivemessaging.amqp.devmode.AmqpDevModeTest.testCodeUpdate line 44 - History - More details - Source on GitHub

Details

org.awaitility.core.ConditionTimeoutException: Condition with Lambda expression in io.quarkus.smallrye.reactivemessaging.amqp.devmode.AmqpDevModeTest was not fulfilled within 1 minutes.
	at org.awaitility.core.ConditionAwaiter.await(ConditionAwaiter.java:167)
	at org.awaitility.core.CallableCondition.await(CallableCondition.java:78)
	at org.awaitility.core.CallableCondition.await(CallableCondition.java:26)
	at org.awaitility.core.ConditionFactory.until(ConditionFactory.java:1006)
	at org.awaitility.core.ConditionFactory.until(ConditionFactory.java:975)
	at io.quarkus.smallrye.reactivemessaging.amqp.devmode.AmqpDevModeTest.testCodeUpdate(AmqpDevModeTest.java:44)
	at java.base/java.lang.reflect.Method.invoke(Method.java:569)

✖ io.quarkus.smallrye.reactivemessaging.amqp.devmode.nohttp.AmqpDevModeNoHttpTest.testConsumerUpdate line 77 - History - More details - Source on GitHub

Details

org.awaitility.core.ConditionTimeoutException: 
Assertion condition defined as a Lambda expression in io.quarkus.smallrye.reactivemessaging.amqp.devmode.nohttp.AmqpDevModeNoHttpTest 
Expecting size of:
  []
to be greater than or equal to 5 but was 0 within 1 minutes.
	at org.awaitility.core.ConditionAwaiter.await(ConditionAwaiter.java:167)
	at org.awaitility.core.AssertionCondition.await(AssertionCondition.java:119)
	at org.awaitility.core.AssertionCondition.await(AssertionCondition.java:31)

✖ io.quarkus.smallrye.reactivemessaging.amqp.devmode.nohttp.AmqpDevModeNoHttpTest.testProducerUpdate line 48 - History - More details - Source on GitHub

Details

org.awaitility.core.ConditionTimeoutException: 
Assertion condition defined as a Lambda expression in io.quarkus.smallrye.reactivemessaging.amqp.devmode.nohttp.AmqpDevModeNoHttpTest 
Expecting size of:
  []
to be greater than or equal to 5 but was 0 within 1 minutes.
	at org.awaitility.core.ConditionAwaiter.await(ConditionAwaiter.java:167)
	at org.awaitility.core.AssertionCondition.await(AssertionCondition.java:119)
	at org.awaitility.core.AssertionCondition.await(AssertionCondition.java:31)

---

Flaky tests - Develocity

⚙️ JVM Tests - JDK 21

📦 integration-tests/opentelemetry

✖ io.quarkus.it.opentelemetry.OpenTelemetryInjectionsTest.testOTelInjections - History

• Condition with Lambda expression in io.quarkus.it.opentelemetry.OpenTelemetryInjectionsTest was not fulfilled within 5 seconds. - org.awaitility.core.ConditionTimeoutException

Details

org.awaitility.core.ConditionTimeoutException: Condition with Lambda expression in io.quarkus.it.opentelemetry.OpenTelemetryInjectionsTest was not fulfilled within 5 seconds.
	at org.awaitility.core.ConditionAwaiter.await(ConditionAwaiter.java:167)
	at org.awaitility.core.CallableCondition.await(CallableCondition.java:78)
	at org.awaitility.core.CallableCondition.await(CallableCondition.java:26)
	at org.awaitility.core.ConditionFactory.until(ConditionFactory.java:1006)
	at org.awaitility.core.ConditionFactory.until(ConditionFactory.java:975)
	at io.quarkus.it.opentelemetry.OpenTelemetryInjectionsTest.reset(OpenTelemetryInjectionsTest.java:26)
	at java.base/java.lang.reflect.Method.invoke(Method.java:580)