Switch Vert.x HTTP to @ConfigMapping

[radcortez]

• Part of Optimize config generation #32209
• Fixes null keys in YAML config break JSON compatibility #34292

[quarkus-bot]

Failing Jobs - Building 3875d03

Status	Name	Step	Failures	Logs	Raw logs
✔️	JVM Tests - JDK 11
✔️	JVM Tests - JDK 17
✖	JVM Tests - JDK 17 Windows	Build	Failures	Logs	Raw logs
✔️	JVM Tests - JDK 20
✖	Native Tests - Misc1	Build	⚠️ Check →	Logs	Raw logs
✖	Native Tests - Security2	Build	Failures	Logs	Raw logs

Full information is available in the Build summary check run.

Failures

⚙️ JVM Tests - JDK 17 Windows #

- Failing: extensions/vertx-http/deployment 
! Skipped: extensions/agroal/deployment extensions/amazon-lambda-http/deployment extensions/amazon-lambda-rest/deployment and 353 more

📦 extensions/vertx-http/deployment

✖ Failed to execute goal org.apache.maven.plugins:maven-surefire-plugin:3.1.2:test (default-test) on project quarkus-vertx-http-deployment: There was a timeout in the fork

---

⚙️ Native Tests - Security2 #

- Failing: integration-tests/oidc 

📦 integration-tests/oidc

✖ io.quarkus.it.keycloak.BearerTokenAuthorizationInGraalITCase.testSecureAccessSuccessWithCors - More details - Source on GitHub

java.lang.AssertionError: 
1 expectation failed.
Expected status code <200> but was <404>.

[gsmet]

@radcortez I'm pretty sure the fact that you fixed https://github.com/quarkusio/quarkus/issues/34292 will require a note in the migration guide?

[radcortez]

Right now both properties quarkus.http.cors and quarkus.http.cors.enabled are valid, but onlyquarkus.http.cors.enabled shows up in the documentation. The old quarkus.http.cors is not deprecated yet, so whoever uses it shouldn't notice any difference.

The plan is to support #33877, so we can deprecate this (and other properties) and provide more direct feedback to the user.

Regardless, I'll add a note.

[mkouba]

@gsmet @radcortez Shouldn't we label this PR as a breaking change? It breaks all Quarkiverse/third-party extensions that consume io.quarkus.vertx.http.runtime.HttpBuildTimeConfig (previously class with fields, now interface with methods).

[gsmet]

Yeah, I noticed this problem in my Quarkus GitHub App extension but with the runtime config.

I never actually considered the config an API (especially given it's in the runtime module) but unfortunately it was the most convenient way to get the values.

I'm not sure if we should ask people to move to the method or if we should advertise a better way of getting this config (maybe by getting the Vert.x instance and get the value from there if possible).

@radcortez @cescoffier thoughts?

[mkouba]

I never actually considered the config an API (especially given it's in the runtime module) but unfortunately it was the most convenient way to get the values.

Well, I'd say that it's the only type-safe way... The truth is that the content of the io.quarkus.vertx.http.runtime package should not be considered part of the public API but since the class was public and there's usually no other "official" way...

I'm not sure if we should ask people to move to the method or if we should advertise a better way of getting this config (maybe by getting the Vert.x instance and get the value from there if possible).

But that would only solve the runtime part, what about the build steps?

[cescoffier]

In my opinion, it’s a breaking change.

Unfortunately, there is no way to get the values from the vertx instance. Keeping a VertxOption instance around sounds a bit overkill (and it’s not a one-to-one mapping).

[radcortez]

Hum... good point.

In reality, because these classes are public, we have no idea if they are being used outside our own internal code. Probably we need to consider that any Config root / mapping classes are public APIs.

[mkouba]

In reality, because these classes are public, we have no idea if they are being used outside our own internal code. Probably we need to consider that any Config root / mapping classes are public APIs.

It's like public build items. You can consume it in your build steps. Moreover, BUILD_AND_RUN_TIME_FIXED configs can be injected in beans, etc.

[radcortez]

It's like public build items. You can consume it in your build steps. Moreover, BUILD_AND_RUN_TIME_FIXED configs can be injected in beans, etc.

Exactly. But we usually always treat this as internal APIs. Build items are bit more obscure for the regular users, so I guess that is not a big deal. I agree that we may see more users relying on the Config objects.

[michalvavrik]

This feels like a stalemate. Quarkus knows whether someone injects config classes. Can't Quarkus generate config classes for few releases only if necessary and log warning that these classes should be migrated to the @ConfigMapping? It should be easy to keep consistency for few releases because you only need to maintain existing properties, new properties can only be added to the @ConfigMapping. Would it be possible for @ConfigMapping and config classes to live together and provide same information?

Is there a Quarkus issue where this migration is tracked, or a plan for the @ConfigMapping migration, please?

I'm mostly interested because QE test framework needs to detect all used build time properties before Quarkus build and I don't want to do it in 2 ways. But after reading #32209 it seems there also are performance objectives.

[radcortez]

This feels like a stalemate. Quarkus knows whether someone injects config classes. Can't Quarkus generate config classes for few releases only if necessary and log warning that these classes should be migrated to the @ConfigMapping?

Eventually, we will have to figure something out here. I didn't want to put a warning message that would reach the user and create unnecessary noise. An option could be to do it at compilation time of Quarkus, or of an Extension, but not a Quarkus App.

It should be easy to keep consistency for few releases because you only need to maintain existing properties, new properties can only be added to the @ConfigMapping. Would it be possible for @ConfigMapping and config classes to live together and provide same information?

Yes they can, but the extension would need to pick which one to use, because it codes directly against the specific type.

Is there a Quarkus issue where this migration is tracked, or a plan for the @ConfigMapping migration, please?

Not at the moment. We talked about this a few times, but we also had a few issues, like this one, that had to be reverted. I believe things should be better now after some improvements we did with performance.

I'm mostly interested because QE test framework needs to detect all used build time properties before Quarkus build and I don't want to do it in 2 ways. But after reading #32209 it seems there also are performance objectives.

What are you doing exactly? Maybe I can help.

[michalvavrik]

What are you doing exactly? Maybe I can help.

We run many tests with different configuration combinations (and test methods) in order to tests many features and scenarios on same application classes and with same dependencies. When they are native tests, we need to know whether we can reuse native executable (which can spare significant test execution time). So far we use hardcoded list, but it is absolutely incomplete and sometimes properties are moved ... I watched this issue as my plan was to build SmallRyeConfig, get all config mappings and check build time / runtime. It can be done for config classes, but don't want to invest time into deprecated solution. I don't want spam this PR, so I can ping you at some other time directly.