Register default configuration smallrye configuration files as resources

[zakkak]

Relates to #41995

[radcortez]

Actually, we already generate a class with the configuration from all the resources found in the classpath, so we don't need to include additional resources or even load them.

I've been wanting to disable this for a while, but I have yet to determine the implications in JVM mode, so I guess the right fix for now it is to disable it for native image only.

[zakkak]

Actually, we already generate a class with the configuration from all the resources found in the classpath, so we don't need to include additional resources or even load them.

That's right and we have discussed this in the past.

I've been wanting to disable this for a while, but I have yet to determine the implications in JVM mode, so I guess the right fix for now it is to disable it for native image only.

IIRC last time we discussed about it, it was not clear how to achieve this. Did anything change since then?

[gsmet]

but I have yet to determine the implications in JVM mode

What exactly would be different in JVM mode?

[quarkus-bot]

Status for workflow Quarkus CI

This is the status report for running Quarkus CI on commit 289c24c.

Failing Jobs

Status	Name	Step	Failures	Logs	Raw logs	Build scan
✖	Native Tests - Misc3	Build	Failures	Logs	Raw logs	🔍

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

Failures

⚙️ Native Tests - Misc3 #

- Failing: integration-tests/smallrye-config 

📦 integration-tests/smallrye-config

✖ io.quarkus.it.smallrye.config.SmallRyeConfigIT.mpConfigProperties - History - More details - Source on GitHub

java.lang.AssertionError: 
1 expectation failed.
JSON path value doesn't match.
Expected: 9090
  Actual: 8080

	at java.base/java.lang.reflect.Constructor.newInstanceWithCaller(Constructor.java:500)
	at java.base/java.lang.reflect.Constructor.newInstance(Constructor.java:481)

---

Flaky tests - Develocity

⚙️ JVM Tests - JDK 17

📦 extensions/panache/hibernate-reactive-rest-data-panache/deployment

✖ io.quarkus.hibernate.reactive.rest.data.panache.deployment.repository.PanacheRepositoryResourcePutMethodTest.shouldUpdateComplexObject - History

• 1 expectation failed. JSON path name doesn't match. Expected: is "updated collection" Actual: empty collection - java.lang.AssertionError

java.lang.AssertionError: 
1 expectation failed.
JSON path name doesn't match.
Expected: is "updated collection"
  Actual: empty collection

	at java.base/java.lang.reflect.Constructor.newInstanceWithCaller(Constructor.java:500)
	at java.base/java.lang.reflect.Constructor.newInstance(Constructor.java:481)

---

⚙️ JVM Tests - JDK 21

📦 extensions/micrometer/deployment

✖ io.quarkus.micrometer.deployment.binder.VertxHttpClientMetricsTest.testWebClientMetrics - History

• event executor terminated - java.util.concurrent.RejectedExecutionException

java.util.concurrent.RejectedExecutionException: event executor terminated
	at io.netty.util.concurrent.SingleThreadEventExecutor.reject(SingleThreadEventExecutor.java:934)
	at io.netty.util.concurrent.SingleThreadEventExecutor.offerTask(SingleThreadEventExecutor.java:353)
	at io.netty.util.concurrent.SingleThreadEventExecutor.addTask(SingleThreadEventExecutor.java:346)
	at io.netty.util.concurrent.SingleThreadEventExecutor.execute(SingleThreadEventExecutor.java:836)
	at io.netty.util.concurrent.SingleThreadEventExecutor.execute0(SingleThreadEventExecutor.java:827)
	at io.netty.util.concurrent.SingleThreadEventExecutor.execute(SingleThreadEventExecutor.java:817)
	at io.vertx.core.impl.EventLoopExecutor.execute(EventLoopExecutor.java:35)

[zakkak]

IIRC last time we discussed about it, it was not clear how to achieve this. Did anything change since then?

@radcortez is https://github.com/quarkusio/quarkus/pull/42140/files#diff-bdd91808c3b03f0afb6c4740ab405828a44c92815f28e0fd2271eab84104be87 still the way to go? Should I rebase #42140 and close this PR?

What exactly would be different in JVM mode?

Not sure what Roberto refers to, but the way I see it expectations are different in JVM-mode, i.e. it might feel alien to require a re-deploy to apply changes when changing the smallrye config.

[radcortez]

IIRC last time we discussed about it, it was not clear how to achieve this. Did anything change since then?

@radcortez is https://github.com/quarkusio/quarkus/pull/42140/files#diff-bdd91808c3b03f0afb6c4740ab405828a44c92815f28e0fd2271eab84104be87 still the way to go? Should I rebase #42140 and close this PR?

I believe so, but let's discuss this further because we may want to take a bigger stance and do it for JVM mode too.

What exactly would be different in JVM mode?

Not sure what Roberto refers to, but the way I see it expectations are different in JVM-mode, i.e. it might feel alien to require a re-deploy to apply changes when changing the smallrye config.

There is a case (which has always been there), probably a bit of a corner case, and nobody has reported it so far because it only affects native mode.

When we flatten the resources, we lose the ordinality of each source. We use ordinality to determine the value of each configuration property by querying sources using their ordinal. With the flattening, we query the value and record it, but the resulting source will have one ordinal versus multiple ones coming from all the static resources. At first glance, this doesn't seem to be a big issue because the values are recorded following the ordinal lookup during build time.

The problem is that, during runtime, we may have additional sources not present during build time, with ordinals in between the ones that were recorded, which may mess up the final configuration values. As an example:

#ordinal 250
foo.bar=application.properties

#ordinal 100
foo.bar=microprofile-config.properties
foo.baz=microprofile-config.properties

These will be recorded and not discovered as resources during native images. The recorded source is at Integer.MIN_VALUE.

Now consider that a new runtime source is discovered as ordinal=90 and foo.baz=runtime source, foo.baz should be microprofile-config.properties, but it is runtime source because the recorded source is lower than the original.

Changing the recording source to a different ordinal doesn't really help because we are merging multiple ordinals ranging from 100-250 (and even more, depending on user sources), and during runtime, we can have a source that gets in the middle and mess up correct lookups. Ideally, we would need to preserve each config property order to ensure that we don't have this issue. While not hard to do, we have yet more data to store, so I wanted to benchmark before coming out with a final solution.

Dev mode, is also a concern, but I believe it should work out of the box. Regardless, we need to test it properly.

[zakkak]

Closing in favor of #42140