Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Prefixed env vars used during build are included and required during runtime when using ConfigMapping #33213

Closed
rasmushaglund opened this issue May 9, 2023 · 6 comments · Fixed by #33079
Assignees
Labels
area/config kind/bug Something isn't working
Milestone

Comments

@rasmushaglund
Copy link

rasmushaglund commented May 9, 2023

Describe the bug

When using ConfigMapping all environment variables on the build machine starting with the prefix, that is not included in the conf, will be included in the build and required to be present when running the app.

Expected behavior

The build shouldn't include prefixed env vars that are not defined in the ConfigMapping, and you should be able to run the app even though you happen to have an env var on the build machine that looks like it should be mapped.

Actual behavior

Even though PREFIX_NEW is not defined in the ConfigMapping it is required in order to run the app.

Exception in thread "main" java.lang.reflect.InvocationTargetException
	at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
	at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:77)
	at java.base/jdk.internal.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
	at java.base/java.lang.reflect.Method.invoke(Method.java:568)
	at io.quarkus.bootstrap.runner.QuarkusEntryPoint.doRun(QuarkusEntryPoint.java:61)
	at io.quarkus.bootstrap.runner.QuarkusEntryPoint.main(QuarkusEntryPoint.java:32)
Caused by: java.lang.ExceptionInInitializerError
	at io.quarkus.runner.ApplicationImpl.<clinit>(Unknown Source)
	at java.base/jdk.internal.reflect.NativeConstructorAccessorImpl.newInstance0(Native Method)
	at java.base/jdk.internal.reflect.NativeConstructorAccessorImpl.newInstance(NativeConstructorAccessorImpl.java:77)
	at java.base/jdk.internal.reflect.DelegatingConstructorAccessorImpl.newInstance(DelegatingConstructorAccessorImpl.java:45)
	at java.base/java.lang.reflect.Constructor.newInstanceWithCaller(Constructor.java:499)
	at java.base/java.lang.reflect.Constructor.newInstance(Constructor.java:480)
	at io.quarkus.runtime.Quarkus.run(Quarkus.java:70)
	at io.quarkus.runtime.Quarkus.run(Quarkus.java:44)
	at io.quarkus.runtime.Quarkus.run(Quarkus.java:124)
	at io.quarkus.runner.GeneratedMain.main(Unknown Source)
	... 6 more
Caused by: io.smallrye.config.ConfigValidationException: Configuration validation failed:
	prefix.new does not map to any root
	at io.smallrye.config.ConfigMappingProvider.mapConfigurationInternal(ConfigMappingProvider.java:979)
	at io.smallrye.config.ConfigMappingProvider.lambda$mapConfiguration$3(ConfigMappingProvider.java:935)
	at io.smallrye.config.SecretKeys.doUnlocked(SecretKeys.java:28)
	at io.smallrye.config.ConfigMappingProvider.mapConfiguration(ConfigMappingProvider.java:935)
	at io.smallrye.config.ConfigMappings.mapConfiguration(ConfigMappings.java:91)
	at io.smallrye.config.SmallRyeConfigBuilder.build(SmallRyeConfigBuilder.java:624)
	at io.quarkus.runtime.generated.Config.<clinit>(Unknown Source)
	... 16 more

In the ConfigMapping I've only defined the property test. There is no mapping for new.

How to Reproduce?

git clone [email protected]:rasmushaglund/quarkus-env-bug.git
cd quarkus-env-bug
export PREFIX_NEW=hello && ./mvnw clean package && unset PREFIX_NEW && java -jar target/quarkus-app/quarkus-run.jar

When not using the env var it works as expected (make sure to unset all env var starting with PREFIX_ first):

git clone [email protected]:rasmushaglund/quarkus-env-bug.git
cd quarkus-env-bug
./mvnw clean package && java -jar target/quarkus-app/quarkus-run.jar

Output of uname -a or ver

Linux bifrost 5.19.0-38-generic #39-Ubuntu SMP PREEMPT_DYNAMIC Fri Mar 17 17:33:16 UTC 2023 x86_64 x86_64 x86_64 GNU/Linux

Output of java -version

openjdk 17.0.6 2023-01-17 OpenJDK Runtime Environment (build 17.0.6+10-Ubuntu-0ubuntu122.10) OpenJDK 64-Bit Server VM (build 17.0.6+10-Ubuntu-0ubuntu122.10, mixed mode, sharing)

GraalVM version (if different from Java)

No response

Quarkus version or git rev

3.0.2.Final

Build tool (ie. output of mvnw --version or gradlew --version)

Apache Maven 3.8.8 (4c87b05d9aedce574290d1acc98575ed5eb6cd39) Maven home: /home/rasmus/.m2/wrapper/dists/apache-maven-3.8.8-bin/67c30f74/apache-maven-3.8.8 Java version: 17.0.6, vendor: Private Build, runtime: /usr/lib/jvm/java-17-openjdk-amd64 Default locale: en_US, platform encoding: UTF-8 OS name: "linux", version: "5.19.0-38-generic", arch: "amd64", family: "unix"

Additional information

I found out the bug because I have a ConfigMapping with a prefix, let's say, server, and then in the integration tests use the env var SERVER_TOKEN that isn't intended to be included in the mapping. I didn't expect the build process to try to map SERVER_TOKEN to the ConfigMapping server.token and then require server.token to be defined during runtime.

Imagine creating a ConfigMapping with the prefix "http" with properties like "url", "port" but the build server has the env var HTTP_PROXY set. That will build the app with the HTTP_PROXY env var included, and require HTTP_PROXY or http.proxy to be set in order to run the app, which is not intended.

@rasmushaglund rasmushaglund added the kind/bug Something isn't working label May 9, 2023
@geoand
Copy link
Contributor

geoand commented May 9, 2023

cc @radcortez

@mschorsch
Copy link
Contributor

@radcortez we stumbled on this too.

@radcortez
Copy link
Member

I'll have a look.

@radcortez
Copy link
Member

When using ConfigMapping all environment variables on the build machine starting with the prefix, that is not included in the conf, will be included in the build and required to be present when running the app.

Well, this is a combination of how Quarkus works and mappings behaviour. Build configuration, is recorded in the generated binary (either JAR or native), in a low ordinal source to ensure that nothing is missing and Quarkus is able to start. We don't make any distinction if the property comes from a mapping or not. We cannot solely rely on recording mapping property names, because the configuration may not be mapped at all, and may be accessed programmatically by its name.

Mappings have this built-in feature to throw an Exception in the case of a property name is found that is not mapped. Usually from our experience, this is either a bug (the user forgot to map the name), or a typo (the user mistyped the name). If there is a situation where you don't want to map a name, but the name still exists and share the prefix, you can ignore the validation with smallrye.config.mapping.validate-unknown=false.

I don't think there is much else we can do here. Changing or removing the current recording behavior may potentially break other Quarkus applications. Changing the behavior of unmapped properties removes the safety net that users get when dealing with configuration.

@rasmushaglund
Copy link
Author

I understand your point @radcortez, and I think it would be really helpful with some kind of warning when this happens. When I define config mappings (for example in a class annotated with @ConfigMapping) I specify exactly what properties I'd like to map, and would not want other factors to add to that mapping without me being notified about that.

Since there's at least two here who have experienced this problem (I spent a day trying to figure this one out), is there a possibility to add a warning somewhere, or maybe some more context to the error message? For example prefix.new does not map to any root, was the application built with PREFIX_NEW env variable set? or something like that. I'm not sure what can be read from the runtime context.

@radcortez
Copy link
Member

Probably, not with that detail.

Config phases are Quarkus-specific. Mappings are not aware about Quarkus, they are a SmallRye Config feature. They just instantiate the object with the current Config.

We could add the name of the source from where the property was found. Indirectly, the source name could say it is recorded from build time Quarkus. Currently, I think we call it DefaultValuesConfigSource or something.

@radcortez radcortez self-assigned this May 24, 2023
@quarkus-bot quarkus-bot bot added this to the 3.2 - main milestone Jun 13, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/config kind/bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants