Config: detect injected config value mismatch during static init #36281

mkouba · 2023-10-04T08:13:23Z

record the values injected during static intialization phase
if the runtime value differs from the injected value the app startup fails
also introduce ExecutionMode to easily detect the STATIC_INIT bootstrap phase

This is an alternative to #36169.

The error message looks like:

A runtime config property value differs from the value that was injected during the static intialization phase:
 - the runtime value of 'apfelstrudel' is [gizmo] but the value [jandex] was injected into io.quarkus.arc.test.config.staticinit.StaticInitBean#value

NOTE: We completely ignore config mappings as they're not available unless annotated with @StaticInitSafe.

yrodiere

This looks good, thanks!

Do you thing we'd need a configuration option to turn this into a warning, similar to this:

quarkus/core/runtime/src/main/java/io/quarkus/runtime/configuration/ConfigurationRuntimeConfig.java

Lines 10 to 22 in 2478d22

    
               /** 
        
                * What should happen if the application is started with a different build time configuration than it was compiled 
        
                * against. This may be useful to prevent misconfiguration. 
        
                * <p> 
        
                * If this is set to {@code warn} the application will warn at start up. 
        
                * <p> 
        
                * If this is set to {@code fail} the application will fail at start up. 
        
                * <p> 
        
                * Native tests leveraging<code>@io.quarkus.test.junit.TestProfile</code> are always run with 
        
                * {@code quarkus.configuration.build-time-mismatch-at-runtime = fail}. 
        
                */ 
        
               @ConfigItem(defaultValue = "warn") 
        
               public BuildTimeMismatchAtRuntime buildTimeMismatchAtRuntime;

Or is @StaticInitSafe going to be enough?

In any case I would still have the property default to fail, that seems safer.

core/deployment/src/main/java/io/quarkus/deployment/steps/MainClassBuildStep.java

mkouba · 2023-10-04T12:51:01Z

This looks good, thanks!

Do you thing we'd need a configuration option to turn this into a warning, similar to this:

Or is @StaticInitSafe going to be enough?

In any case I would still have the property default to fail, that seems safer.

Hm, I think that @StaticInitSafe is enough.

radcortez · 2023-10-04T15:53:51Z

Hm, I think that @StaticInitSafe is enough.

Agree. The user explicitly tells the mapping with the annotation to be available at STATIC_INIT phase, so we shouldn't invalidate that.

core/runtime/src/main/java/io/quarkus/runtime/ExecutionMode.java

...sions/arc/runtime/src/main/java/io/quarkus/arc/runtime/ConfigStaticInitCheckInterceptor.java

extensions/arc/runtime/src/main/java/io/quarkus/arc/runtime/ConfigStaticInitValues.java

mkouba · 2023-10-05T11:18:51Z

I will send a follow-up PR to update the docs.

mkouba · 2023-10-05T12:58:08Z

The Native Tests - HTTP job now fails with:

Caused by: java.lang.IllegalStateException: A runtime config property value differs from the value that was injected during the static intialization phase:
 - the runtime value of 'test.prop' is [prod] but the value [test] was injected into io.quarkus.it.resteasy.reactive.kotlin.RegisterCustomModuleCustomizer#testProp

@geoand Is it expected that the value differs? If so I will add @StaticInitSafe to the injection point.

- record the values injected during static intialization phase - if the runtime value differs from the injected value the app startup fails - also introduce ExecutionMode to easily detect the STATIC_INIT bootstrap phase

- deprecate ConfigInjectionStaticInitBuildItem

yrodiere · 2023-10-06T08:36:17Z

This looks good, thanks!
Do you thing we'd need a configuration option to turn this into a warning, similar to this:
Or is @StaticInitSafe going to be enough?
In any case I would still have the property default to fail, that seems safer.

Hm, I think that @StaticInitSafe is enough.

Interestingly, I think there are cases where we developers could want to opt out of this check. Most notably, when they don't control the environment their application will run in, like for example the quarkus CLI. See #36291 . It's about build-time configuration, but it's the same idea, really.

I could imagine a developer setting some generic Quarkus configuration property in their environment variables, because they use that value for all the applications they work on while developing. And then they run quarkus ext add, and a failure pops up because that property was set to a different value during the static init of the native build of the quarkus CLI.

So, in the general case this check is great help, but in some edge cases it could be unnecessary and even hurtful. I believe we will have to add a way to opt out on a per-application basis (and not a per-injection-point basis) whether we like it or not. That can certainly be a second step though.

Perhaps another approach would be to allow applications (CLIs in particular) to only allow a specific subset of properties to be set through environment variables, ignoring everything else. That sure seems to make sense for the quarkus CLI.

mkouba · 2023-10-06T08:59:34Z

This looks good, thanks!
Do you thing we'd need a configuration option to turn this into a warning, similar to this:
Or is @StaticInitSafe going to be enough?
In any case I would still have the property default to fail, that seems safer.

Hm, I think that @StaticInitSafe is enough.

Interestingly, I think there are cases where we developers could want to opt out of this check. Most notably, when they don't control the environment their application will run in, like for example the quarkus CLI. See #36291 . It's about build-time configuration, but it's the same idea, really.

This is very confusing ☹️. So you do export QUARKUS_NATIVE_BUILDER_IMAGE_PULL=missing to set the build time property but at the same time when you run a quarkus app built with a different value you get a warning.

The difference is that we would not fail the startup unless the property is used during STATIC_INIT. Which would be IMO an illegal state anyway... and should be probably handled in a special way.

I could imagine a developer setting some generic Quarkus configuration property in their environment variables, because they use that value for all the applications they work on while developing. And then they run quarkus ext add, and a failure pops up because that property was set to a different value during the static init of the native build of the quarkus CLI.

So, in the general case this check is great help, but in some edge cases it could be unnecessary and even hurtful. I believe we will have to add a way to opt out on a per-application basis (and not a per-injection-point basis) whether we like it or not. That can certainly be a second step though.

Certainly, it should be straightforward to add this functionality.

Perhaps another approach would be to allow applications (CLIs in particular) to only allow a specific subset of properties to be set through environment variables, ignoring everything else. That sure seems to make sense for the quarkus CLI.

Yes, CLI is a bit special but still it's confusing...

radcortez · 2023-10-06T09:27:28Z

Yes, CLI is a bit special but still it's confusing...

Probably we need to provide a way to ignore every check related to build time in the CLI, because the user does not control the build in this case and there is nothing he can do about it.

Also, I don't imagine a user trying to set quarkus configuration properties for the CLI (except the ones allowed by the CLI command). So, while it is definitely useful to provide the warnings on a regular application, for the CLI they don't seem to make sense.

quarkus-bot · 2023-10-06T13:10:00Z

Failing Jobs - Building `60c24cb`

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

Full information is available in the Build summary check run.

Failures

⚙️ JVM Tests - JDK 20 #

- Failing: extensions/hibernate-orm/deployment 
! Skipped: extensions/flyway/deployment extensions/hibernate-envers/deployment extensions/hibernate-reactive/deployment and 103 more

📦 extensions/hibernate-orm/deployment

✖ io.quarkus.hibernate.orm.ignore_explicit_for_joined.IgnoreExplicitForJoinedTrueValueWithPersistenceXmlTest. - More details - Source on GitHub

java.lang.RuntimeException: java.lang.RuntimeException: Failed to start quarkus
	at io.quarkus.test.QuarkusUnitTest.beforeAll(QuarkusUnitTest.java:705)
	at java.base/java.util.ArrayList.forEach(ArrayList.java:1511)

mkouba · 2023-10-06T13:25:56Z

The JDK 20 job seems to fail repeatedly with java.lang.OutOfMemoryError: Metaspace:

2023-10-06T10:01:34.6339200Z [INFO] Running io.quarkus.hibernate.orm.ignore_explicit_for_joined.IgnoreExplicitForJoinedTrueValueWithPersistenceXmlTest
2023-10-06T10:01:35.3740938Z 2023-10-06 10:01:35,269 INFO  [io.qua.hib.orm.dep.HibernateOrmProcessor] (build-4) Persistence unit 'templatePU': Enforcing Quarkus defaults for dialect 'org.hibernate.dialect.H2Dialect' by automatically setting 'jakarta.persistence.database-product-version=2.2.220'.
2023-10-06T10:01:35.3743313Z 2023-10-06 10:01:35,270 INFO  [io.qua.hib.orm.dep.HibernateOrmProcessor] (build-4) A legacy persistence.xml file is present in the classpath. This file will be used to configure JPA/Hibernate ORM persistence units, and any configuration of the Hibernate ORM extension will be ignored. To ignore persistence.xml files instead, set the configuration property 'quarkus.hibernate-orm.persistence-xml.ignore' to 'true'.
2023-10-06T10:01:42.2260006Z [ERROR] Tests run: 1, Failures: 0, Errors: 1, Skipped: 0, Time elapsed: 7.514 s <<< FAILURE! -- in io.quarkus.hibernate.orm.ignore_explicit_for_joined.IgnoreExplicitForJoinedTrueValueWithPersistenceXmlTest
2023-10-06T10:01:42.2268817Z [ERROR] io.quarkus.hibernate.orm.ignore_explicit_for_joined.IgnoreExplicitForJoinedTrueValueWithPersistenceXmlTest -- Time elapsed: 7.514 s <<< ERROR!
2023-10-06T10:01:42.2270371Z java.lang.RuntimeException: java.lang.RuntimeException: Failed to start quarkus
2023-10-06T10:01:42.2271236Z 	at io.quarkus.test.QuarkusUnitTest.beforeAll(QuarkusUnitTest.java:705)
2023-10-06T10:01:42.2272202Z 	at java.base/java.util.ArrayList.forEach(ArrayList.java:1511)
2023-10-06T10:01:42.2337696Z Caused by: java.lang.RuntimeException: Failed to start quarkus
2023-10-06T10:01:42.2338206Z 	at io.quarkus.runner.ApplicationImpl.doStart(Unknown Source)
2023-10-06T10:01:42.2338715Z 	at io.quarkus.runtime.Application.start(Application.java:101)
2023-10-06T10:01:42.2339647Z 	at java.base/java.lang.reflect.Method.invoke(Method.java:578)
2023-10-06T10:01:42.2340207Z 	at io.quarkus.runner.bootstrap.StartupActionImpl.run(StartupActionImpl.java:282)
2023-10-06T10:01:42.2340770Z 	at io.quarkus.test.QuarkusUnitTest.beforeAll(QuarkusUnitTest.java:658)
2023-10-06T10:01:42.2341821Z 	... 1 more
2023-10-06T10:01:42.2342219Z Caused by: java.lang.RuntimeException: java.lang.OutOfMemoryError: Metaspace
2023-10-06T10:01:42.2343157Z 	at io.quarkus.hibernate.orm.runtime.JPAConfig.startAll(JPAConfig.java:78)
2023-10-06T10:01:42.2343880Z 	at io.quarkus.hibernate.orm.runtime.HibernateOrmRecorder.startAllPersistenceUnits(HibernateOrmRecorder.java:101)
2023-10-06T10:01:42.2354949Z 	at io.quarkus.deployment.steps.HibernateOrmProcessor$startPersistenceUnits1868654632.deploy_0(Unknown Source)
2023-10-06T10:01:42.2356314Z 	at io.quarkus.deployment.steps.HibernateOrmProcessor$startPersistenceUnits1868654632.deploy(Unknown Source)
2023-10-06T10:01:42.2370422Z 	... 6 more
2023-10-06T10:01:42.2371089Z Caused by: java.lang.OutOfMemoryError: Metaspace
2023-10-06T10:01:42.2371650Z 	at java.base/java.lang.ClassLoader.defineClass1(Native Method)
2023-10-06T10:01:42.2372143Z 	at java.base/java.lang.ClassLoader.defineClass(ClassLoader.java:1013)
2023-10-06T10:01:42.2372749Z 	at io.quarkus.bootstrap.classloading.QuarkusClassLoader.loadClass(QuarkusClassLoader.java:506)
2023-10-06T10:01:42.2373456Z 	at io.quarkus.bootstrap.classloading.QuarkusClassLoader.loadClass(QuarkusClassLoader.java:466)
2023-10-06T10:01:42.2374045Z 	at java.base/java.lang.ClassLoader.defineClass1(Native Method)
2023-10-06T10:01:42.2374499Z 	at java.base/java.lang.ClassLoader.defineClass(ClassLoader.java:1013)
2023-10-06T10:01:42.2375102Z 	at io.quarkus.bootstrap.classloading.QuarkusClassLoader.loadClass(QuarkusClassLoader.java:506)
2023-10-06T10:01:42.2376105Z 	at io.quarkus.bootstrap.classloading.QuarkusClassLoader.loadClass(QuarkusClassLoader.java:466)
2023-10-06T10:01:42.2377344Z 	at org.hibernate.persister.internal.StandardPersisterClassResolver.singleTableEntityPersister(StandardPersisterClassResolver.java:60)
2023-10-06T10:01:42.2378388Z 	at org.hibernate.persister.internal.StandardPersisterClassResolver.getEntityPersisterClass(StandardPersisterClassResolver.java:40)
2023-10-06T10:01:42.2379322Z 	at org.hibernate.persister.internal.PersisterFactoryImpl.createEntityPersister(PersisterFactoryImpl.java:72)
2023-10-06T10:01:42.2380357Z 	at org.hibernate.metamodel.model.domain.internal.MappingMetamodelImpl.processBootEntities(MappingMetamodelImpl.java:247)
2023-10-06T10:01:42.2381294Z 	at org.hibernate.metamodel.model.domain.internal.MappingMetamodelImpl.finishInitialization(MappingMetamodelImpl.java:185)
2023-10-06T10:01:42.2382612Z 	at org.hibernate.internal.SessionFactoryImpl.initializeMappingModel(SessionFactoryImpl.java:321)
2023-10-06T10:01:42.2383653Z 	at org.hibernate.internal.SessionFactoryImpl.<init>(SessionFactoryImpl.java:271)
2023-10-06T10:01:42.2384601Z 	at io.quarkus.hibernate.orm.runtime.boot.FastBootEntityManagerFactoryBuilder.build(FastBootEntityManagerFactoryBuilder.java:84)
2023-10-06T10:01:42.2385660Z 	at io.quarkus.hibernate.orm.runtime.FastBootHibernatePersistenceProvider.createEntityManagerFactory(FastBootHibernatePersistenceProvider.java:74)
2023-10-06T10:01:42.2386625Z 	at jakarta.persistence.Persistence.createEntityManagerFactory(Persistence.java:80)
2023-10-06T10:01:42.2387281Z 	at jakarta.persistence.Persistence.createEntityManagerFactory(Persistence.java:55)
2023-10-06T10:01:42.2388700Z 	at io.quarkus.hibernate.orm.runtime.JPAConfig$LazyPersistenceUnit.get(JPAConfig.java:156)
2023-10-06T10:01:42.2389256Z 	at io.quarkus.hibernate.orm.runtime.JPAConfig$1.run(JPAConfig.java:64)
2023-10-06T10:01:42.2389838Z 	at java.base/java.lang.Thread.runWith(Thread.java:1636)
2023-10-06T10:01:42.2390253Z 	at java.base/java.lang.Thread.run(Thread.java:1623)

@gsmet Does it ring a bell?

yrodiere · 2023-10-06T14:21:48Z

@mkouba It does for me. Probably the QuarkusUnitTest metaspace leaks again. I need to get back to this. Will schedule that first thing on Monday...

mkouba · 2023-10-09T14:56:18Z

@mkouba It does for me. Probably the QuarkusUnitTest metaspace leaks again. I need to get back to this. Will schedule that first thing on Monday...

@yrodiere Did you manage to find something?

Should I merge this PR?

yrodiere · 2023-10-09T15:17:05Z

@yrodiere Did you manage to find something?

Not yet

Should I merge this PR?

Sure, the failure isn't related and only happened in one JDK, so it's very clearly a flake.

yrodiere · 2023-10-09T15:24:15Z

@mkouba do you think we should add something to the migration guide?

mkouba · 2023-10-10T07:22:29Z

@mkouba do you think we should add something to the migration guide?

That's a good idea but I need to send a separate PR to update the docs first ;-).

- follow-up of quarkusio#36281

mkouba · 2023-10-11T09:30:29Z

@mkouba do you think we should add something to the migration guide?

That's a good idea but I need to send a separate PR to update the docs first ;-).

Done. Both ;-)

- resolves quarkusio#37444 - follow-up of quarkusio#36281

- resolves quarkusio#37444 - follow-up of quarkusio#36281 (cherry picked from commit bd98d02)

- follow-up of quarkusio#36281

- resolves quarkusio#37444 - follow-up of quarkusio#36281

quarkus-bot bot added area/arc Issue related to ARC (dependency injection) area/core labels Oct 4, 2023

mkouba requested review from yrodiere and radcortez October 4, 2023 08:13

mkouba force-pushed the static-init-config-validation branch from 5a47e49 to 6e64941 Compare October 4, 2023 08:21

mkouba added the area/config label Oct 4, 2023

quarkus-bot bot added area/resteasy-classic area/undertow labels Oct 4, 2023

mkouba force-pushed the static-init-config-validation branch from 50d26a6 to 87b3506 Compare October 4, 2023 11:49

yrodiere approved these changes Oct 4, 2023

View reviewed changes

core/deployment/src/main/java/io/quarkus/deployment/steps/MainClassBuildStep.java Show resolved Hide resolved

mkouba force-pushed the static-init-config-validation branch from 87b3506 to c02e625 Compare October 4, 2023 13:04

radcortez approved these changes Oct 4, 2023

View reviewed changes

mkouba marked this pull request as ready for review October 5, 2023 05:26

mkouba added the triage/waiting-for-ci Ready to merge when CI successfully finishes label Oct 5, 2023

mkouba mentioned this pull request Oct 5, 2023

Config: injection verification during the native image build #36169

Closed

mkouba force-pushed the static-init-config-validation branch 2 times, most recently from 9b0847f to 71355bb Compare October 5, 2023 09:43

Ladicek reviewed Oct 5, 2023

View reviewed changes

core/runtime/src/main/java/io/quarkus/runtime/ExecutionMode.java Show resolved Hide resolved

Ladicek reviewed Oct 5, 2023

View reviewed changes

...sions/arc/runtime/src/main/java/io/quarkus/arc/runtime/ConfigStaticInitCheckInterceptor.java Show resolved Hide resolved

Ladicek reviewed Oct 5, 2023

View reviewed changes

extensions/arc/runtime/src/main/java/io/quarkus/arc/runtime/ConfigStaticInitValues.java Outdated Show resolved Hide resolved

Ladicek reviewed Oct 5, 2023

View reviewed changes

extensions/arc/runtime/src/main/java/io/quarkus/arc/runtime/ConfigStaticInitValues.java Outdated Show resolved Hide resolved

mkouba force-pushed the static-init-config-validation branch from 71355bb to 0611e6c Compare October 5, 2023 10:20

mkouba added 2 commits October 5, 2023 16:27

Config: detect injected config value mismatch during static init

32047ae

- record the values injected during static intialization phase - if the runtime value differs from the injected value the app startup fails - also introduce ExecutionMode to easily detect the STATIC_INIT bootstrap phase

Config: remove the warning for ConfigInjectionStaticInitBuildItem

60c24cb

- deprecate ConfigInjectionStaticInitBuildItem

mkouba force-pushed the static-init-config-validation branch from 0611e6c to 60c24cb Compare October 5, 2023 14:28

This comment has been minimized.

Sign in to view

mkouba requested a review from Ladicek October 6, 2023 06:41

This comment has been minimized.

Sign in to view

yrodiere merged commit ea956d0 into quarkusio:main Oct 9, 2023
50 of 51 checks passed

quarkus-bot bot added this to the 3.5 - main milestone Oct 9, 2023

quarkus-bot bot removed the triage/waiting-for-ci Ready to merge when CI successfully finishes label Oct 9, 2023

mkouba added a commit to mkouba/quarkus that referenced this pull request Oct 10, 2023

Config: docs for config value mismatch detection

591bd15

- follow-up of quarkusio#36281

mkouba mentioned this pull request Oct 10, 2023

Config: docs for config value mismatch detection #36380

Merged

radcortez mentioned this pull request Dec 1, 2023

Injecting ConfigProperty of type Optional<List<String>> not working in the native mode #37444

Closed

mkouba added a commit to mkouba/quarkus that referenced this pull request Dec 1, 2023

Config: detect injected config value mismatch for missing values

9dd6c84

- resolves quarkusio#37444 - follow-up of quarkusio#36281

mkouba mentioned this pull request Dec 1, 2023

Config: detect injected config value mismatch for missing values #37454

Merged

mkouba added a commit to mkouba/quarkus that referenced this pull request Dec 1, 2023

Config: detect injected config value mismatch for missing values

606fbdc

- resolves quarkusio#37444 - follow-up of quarkusio#36281

mkouba added a commit to mkouba/quarkus that referenced this pull request Dec 4, 2023

Config: detect injected config value mismatch for missing values

bd98d02

- resolves quarkusio#37444 - follow-up of quarkusio#36281

gsmet pushed a commit to gsmet/quarkus that referenced this pull request Dec 4, 2023

Config: detect injected config value mismatch for missing values

58db82b

- resolves quarkusio#37444 - follow-up of quarkusio#36281 (cherry picked from commit bd98d02)

holly-cummins pushed a commit to holly-cummins/quarkus that referenced this pull request Feb 8, 2024

Config: docs for config value mismatch detection

1d96c50

- follow-up of quarkusio#36281

holly-cummins pushed a commit to holly-cummins/quarkus that referenced this pull request Feb 8, 2024

Config: detect injected config value mismatch for missing values

5891be3

- resolves quarkusio#37444 - follow-up of quarkusio#36281

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Config: detect injected config value mismatch during static init #36281

Config: detect injected config value mismatch during static init #36281

mkouba commented Oct 4, 2023 •

edited

Loading

yrodiere left a comment

mkouba commented Oct 4, 2023

radcortez commented Oct 4, 2023

mkouba commented Oct 5, 2023

mkouba commented Oct 5, 2023

This comment has been minimized.

This comment has been minimized.

yrodiere commented Oct 6, 2023 •

edited

Loading

mkouba commented Oct 6, 2023

radcortez commented Oct 6, 2023

quarkus-bot bot commented Oct 6, 2023

mkouba commented Oct 6, 2023

yrodiere commented Oct 6, 2023

mkouba commented Oct 9, 2023

yrodiere commented Oct 9, 2023

yrodiere commented Oct 9, 2023

mkouba commented Oct 10, 2023

mkouba commented Oct 11, 2023

	/**
	* What should happen if the application is started with a different build time configuration than it was compiled
	* against. This may be useful to prevent misconfiguration.
	* <p>
	* If this is set to {@code warn} the application will warn at start up.
	* <p>
	* If this is set to {@code fail} the application will fail at start up.
	* <p>
	* Native tests leveraging<code>@io.quarkus.test.junit.TestProfile</code> are always run with
	* {@code quarkus.configuration.build-time-mismatch-at-runtime = fail}.
	*/
	@ConfigItem(defaultValue = "warn")
	public BuildTimeMismatchAtRuntime buildTimeMismatchAtRuntime;

Config: detect injected config value mismatch during static init #36281

Config: detect injected config value mismatch during static init #36281

Conversation

mkouba commented Oct 4, 2023 • edited Loading

yrodiere left a comment

Choose a reason for hiding this comment

mkouba commented Oct 4, 2023

radcortez commented Oct 4, 2023

mkouba commented Oct 5, 2023

mkouba commented Oct 5, 2023

This comment has been minimized.

This comment has been minimized.

yrodiere commented Oct 6, 2023 • edited Loading

mkouba commented Oct 6, 2023

radcortez commented Oct 6, 2023

quarkus-bot bot commented Oct 6, 2023

Failing Jobs - Building 60c24cb

Failures

⚙️ JVM Tests - JDK 20 #

📦 extensions/hibernate-orm/deployment

mkouba commented Oct 6, 2023

yrodiere commented Oct 6, 2023

mkouba commented Oct 9, 2023

yrodiere commented Oct 9, 2023

yrodiere commented Oct 9, 2023

mkouba commented Oct 10, 2023

mkouba commented Oct 11, 2023

mkouba commented Oct 4, 2023 •

edited

Loading

yrodiere commented Oct 6, 2023 •

edited

Loading

Failing Jobs - Building `60c24cb`