From 592b1b2add19cd5880dc51286e3ea570bd184f05 Mon Sep 17 00:00:00 2001 From: Roberto Cortez Date: Mon, 18 Mar 2024 14:37:16 +0000 Subject: [PATCH] Do not record local sources in runtime config defaults --- .../BuildTimeConfigurationReader.java | 67 +++++++++++++++---- .../src/main/java/org/acme/HelloService.java | 6 +- .../src/main/resources/application.properties | 2 - .../src/main/java/org/acme/HelloService.java | 7 +- .../src/main/resources/application.properties | 2 - .../src/main/resources/application.properties | 4 +- .../io/quarkus/extest/ConfiguredBeanTest.java | 19 ++++-- .../quarkus/extest/RuntimeDefaultsTest.java | 18 +++-- .../config/DoNotRecordEnvConfigSource.java | 19 ++++++ .../config/EnvBuildTimeConfigSource.java | 8 ++- ...rdQuarkusSystemPropertiesConfigSource.java | 18 ----- .../runtime/config/TestMappingRunTime.java | 2 +- .../runtime/config/TestRunTimeConfig.java | 4 ++ 13 files changed, 110 insertions(+), 66 deletions(-) delete mode 100644 integration-tests/maven/src/test/resources-filtered/projects/build-mode-quarkus-profile-override/src/main/resources/application.properties delete mode 100644 integration-tests/maven/src/test/resources-filtered/projects/build-mode-quarkus-profile-property/src/main/resources/application.properties create mode 100644 integration-tests/test-extension/extension/runtime/src/main/java/io/quarkus/extest/runtime/config/DoNotRecordEnvConfigSource.java delete mode 100644 integration-tests/test-extension/extension/runtime/src/main/java/io/quarkus/extest/runtime/config/RecordQuarkusSystemPropertiesConfigSource.java diff --git a/core/deployment/src/main/java/io/quarkus/deployment/configuration/BuildTimeConfigurationReader.java b/core/deployment/src/main/java/io/quarkus/deployment/configuration/BuildTimeConfigurationReader.java index af4e8fc63fb8d..e8a7a0a1f4b3c 100644 --- a/core/deployment/src/main/java/io/quarkus/deployment/configuration/BuildTimeConfigurationReader.java +++ b/core/deployment/src/main/java/io/quarkus/deployment/configuration/BuildTimeConfigurationReader.java @@ -9,6 +9,8 @@ import static io.quarkus.runtime.configuration.PropertiesUtil.filterPropertiesInRoots; import static io.smallrye.config.ConfigMappings.ConfigClassWithPrefix.configClassWithPrefix; import static io.smallrye.config.Expressions.withoutExpansion; +import static io.smallrye.config.PropertiesConfigSourceProvider.classPathSources; +import static io.smallrye.config.SmallRyeConfigBuilder.META_INF_MICROPROFILE_CONFIG_PROPERTIES; import static java.util.stream.Collectors.toSet; import java.io.IOException; @@ -512,10 +514,12 @@ ReadResult run() { nameBuilder.setLength(0); } + SmallRyeConfig runtimeConfig = getConfigForRuntimeRecording(); + // Register defaults for Roots - allBuildTimeValues.putAll(getDefaults(buildTimePatternMap)); - buildTimeRunTimeValues.putAll(getDefaults(buildTimeRunTimePatternMap)); - runTimeDefaultValues.putAll(getDefaults(runTimePatternMap)); + allBuildTimeValues.putAll(getDefaults(config, buildTimePatternMap)); + buildTimeRunTimeValues.putAll(getDefaults(config, buildTimeRunTimePatternMap)); + runTimeDefaultValues.putAll(getDefaults(runtimeConfig, runTimePatternMap)); // Register defaults for Mappings // Runtime defaults are added in ConfigGenerationBuildStep.generateBuilders to include user mappings @@ -602,7 +606,7 @@ ReadResult run() { knownProperty = knownProperty || matched != null; if (matched != null) { // it's a run-time default (record for later) - ConfigValue configValue = withoutExpansion(() -> config.getConfigValue(propertyName)); + ConfigValue configValue = withoutExpansion(() -> runtimeConfig.getConfigValue(propertyName)); if (configValue.getValue() != null) { runTimeValues.put(configValue.getNameProfiled(), configValue.getValue()); } @@ -613,7 +617,7 @@ ReadResult run() { } } else { // it's not managed by us; record it - ConfigValue configValue = withoutExpansion(() -> config.getConfigValue(propertyName)); + ConfigValue configValue = withoutExpansion(() -> runtimeConfig.getConfigValue(propertyName)); if (configValue.getValue() != null) { runTimeValues.put(configValue.getNameProfiled(), configValue.getValue()); } @@ -640,7 +644,7 @@ ReadResult run() { for (String property : mappedProperties) { unknownBuildProperties.remove(property); ConfigValue value = config.getConfigValue(property); - if (value != null && value.getRawValue() != null) { + if (value.getRawValue() != null) { allBuildTimeValues.put(value.getNameProfiled(), value.getRawValue()); } } @@ -652,7 +656,7 @@ ReadResult run() { for (String property : mappedProperties) { unknownBuildProperties.remove(property); ConfigValue value = config.getConfigValue(property); - if (value != null && value.getRawValue() != null) { + if (value.getRawValue() != null) { allBuildTimeValues.put(value.getNameProfiled(), value.getRawValue()); buildTimeRunTimeValues.put(value.getNameProfiled(), value.getRawValue()); } @@ -664,8 +668,8 @@ ReadResult run() { Set mappedProperties = ConfigMappings.mappedProperties(mapping, allProperties); for (String property : mappedProperties) { unknownBuildProperties.remove(property); - ConfigValue value = config.getConfigValue(property); - if (value != null && value.getRawValue() != null) { + ConfigValue value = runtimeConfig.getConfigValue(property); + if (value.getRawValue() != null) { runTimeValues.put(value.getNameProfiled(), value.getRawValue()); } } @@ -1117,6 +1121,41 @@ public Set getPropertyNames() { return properties; } + /** + * Use this Config instance to record the runtime default values. We cannot use the main Config + * instance because it may record values coming from local development sources (Environment Variables, + * System Properties, etc.) in at build time. Local config source values may be completely different between the + * build environment and the runtime environment, so it doesn't make sense to record these. + * + * @return a new {@link SmallRyeConfig} instance without the local sources, including SysPropConfigSource, + * EnvConfigSource, .env, and Build system sources. + */ + private SmallRyeConfig getConfigForRuntimeRecording() { + SmallRyeConfigBuilder builder = ConfigUtils.emptyConfigBuilder(); + builder.getSources().clear(); + builder.getSourceProviders().clear(); + builder.setAddDefaultSources(false) + // Customizers may duplicate sources, but not much we can do about it, we need to run them + .addDiscoveredCustomizers() + // Read microprofile-config.properties, because we disabled the default sources + .withSources(classPathSources(META_INF_MICROPROFILE_CONFIG_PROPERTIES, classLoader)); + + // TODO - Should we reset quarkus.config.location to not record from these sources? + for (ConfigSource configSource : config.getConfigSources()) { + if (configSource instanceof SysPropConfigSource) { + continue; + } + if (configSource instanceof EnvConfigSource) { + continue; + } + if ("PropertiesConfigSource[source=Build system]".equals(configSource.getName())) { + continue; + } + builder.withSources(configSource); + } + return builder.build(); + } + private Map filterActiveProfileProperties(final Map properties) { Set propertiesToRemove = new HashSet<>(); for (String property : properties.keySet()) { @@ -1131,13 +1170,15 @@ private Map filterActiveProfileProperties(final Map getDefaults(final ConfigPatternMap patternMap) { + private static Map getDefaults(final SmallRyeConfig config, + final ConfigPatternMap patternMap) { Map defaultValues = new TreeMap<>(); - getDefaults(defaultValues, new StringBuilder(), patternMap); + getDefaults(config, defaultValues, new StringBuilder(), patternMap); return defaultValues; } - private void getDefaults( + private static void getDefaults( + final SmallRyeConfig config, final Map defaultValues, final StringBuilder propertyName, final ConfigPatternMap patternMap) { @@ -1164,7 +1205,7 @@ private void getDefaults( } for (String childName : patternMap.childNames()) { - getDefaults(defaultValues, + getDefaults(config, defaultValues, new StringBuilder(propertyName).append(childName.equals(ConfigPatternMap.WILD_CARD) ? "*" : childName), patternMap.getChild(childName)); } diff --git a/integration-tests/maven/src/test/resources-filtered/projects/build-mode-quarkus-profile-override/src/main/java/org/acme/HelloService.java b/integration-tests/maven/src/test/resources-filtered/projects/build-mode-quarkus-profile-override/src/main/java/org/acme/HelloService.java index a794439b3e88f..93e8ad3335af9 100644 --- a/integration-tests/maven/src/test/resources-filtered/projects/build-mode-quarkus-profile-override/src/main/java/org/acme/HelloService.java +++ b/integration-tests/maven/src/test/resources-filtered/projects/build-mode-quarkus-profile-override/src/main/java/org/acme/HelloService.java @@ -8,11 +8,7 @@ @IfBuildProfile("foo") @ApplicationScoped public class HelloService { - - @ConfigProperty(name = "name") - String name; - public String name() { - return name; + return "from foo"; } } diff --git a/integration-tests/maven/src/test/resources-filtered/projects/build-mode-quarkus-profile-override/src/main/resources/application.properties b/integration-tests/maven/src/test/resources-filtered/projects/build-mode-quarkus-profile-override/src/main/resources/application.properties deleted file mode 100644 index 7757ef9abfce0..0000000000000 --- a/integration-tests/maven/src/test/resources-filtered/projects/build-mode-quarkus-profile-override/src/main/resources/application.properties +++ /dev/null @@ -1,2 +0,0 @@ -name=from default -%foo.name=from foo diff --git a/integration-tests/maven/src/test/resources-filtered/projects/build-mode-quarkus-profile-property/src/main/java/org/acme/HelloService.java b/integration-tests/maven/src/test/resources-filtered/projects/build-mode-quarkus-profile-property/src/main/java/org/acme/HelloService.java index a794439b3e88f..3822b8fa8395f 100644 --- a/integration-tests/maven/src/test/resources-filtered/projects/build-mode-quarkus-profile-property/src/main/java/org/acme/HelloService.java +++ b/integration-tests/maven/src/test/resources-filtered/projects/build-mode-quarkus-profile-property/src/main/java/org/acme/HelloService.java @@ -1,18 +1,13 @@ package org.acme; import io.quarkus.arc.profile.IfBuildProfile; -import org.eclipse.microprofile.config.inject.ConfigProperty; import jakarta.enterprise.context.ApplicationScoped; @IfBuildProfile("foo") @ApplicationScoped public class HelloService { - - @ConfigProperty(name = "name") - String name; - public String name() { - return name; + return "from foo"; } } diff --git a/integration-tests/maven/src/test/resources-filtered/projects/build-mode-quarkus-profile-property/src/main/resources/application.properties b/integration-tests/maven/src/test/resources-filtered/projects/build-mode-quarkus-profile-property/src/main/resources/application.properties deleted file mode 100644 index 7757ef9abfce0..0000000000000 --- a/integration-tests/maven/src/test/resources-filtered/projects/build-mode-quarkus-profile-property/src/main/resources/application.properties +++ /dev/null @@ -1,2 +0,0 @@ -name=from default -%foo.name=from foo diff --git a/integration-tests/test-extension/extension/deployment/src/main/resources/application.properties b/integration-tests/test-extension/extension/deployment/src/main/resources/application.properties index 3ed057571a910..a87bc45f3f982 100644 --- a/integration-tests/test-extension/extension/deployment/src/main/resources/application.properties +++ b/integration-tests/test-extension/extension/deployment/src/main/resources/application.properties @@ -152,8 +152,8 @@ quarkus.arc.unremovable-types=foo # The YAML source may add an indexed property (depending on how the YAML is laid out). This is not supported by @ConfigRoot quarkus.arc.unremovable-types[0]=foo -### Do not record env values in build time -bt.ok.to.record=properties +### recording +bt.ok.to.record=from-app %test.bt.profile.record=properties ### mappings diff --git a/integration-tests/test-extension/extension/deployment/src/test/java/io/quarkus/extest/ConfiguredBeanTest.java b/integration-tests/test-extension/extension/deployment/src/test/java/io/quarkus/extest/ConfiguredBeanTest.java index 4c6ef753d8df1..31783d3c647cc 100644 --- a/integration-tests/test-extension/extension/deployment/src/test/java/io/quarkus/extest/ConfiguredBeanTest.java +++ b/integration-tests/test-extension/extension/deployment/src/test/java/io/quarkus/extest/ConfiguredBeanTest.java @@ -29,6 +29,7 @@ import org.junit.jupiter.api.extension.RegisterExtension; import io.quarkus.extest.runtime.config.AnotherPrefixConfig; +import io.quarkus.extest.runtime.config.DoNotRecordEnvConfigSource; import io.quarkus.extest.runtime.config.MyEnum; import io.quarkus.extest.runtime.config.NestedConfig; import io.quarkus.extest.runtime.config.ObjectOfValue; @@ -52,10 +53,9 @@ public class ConfiguredBeanTest { static final QuarkusUnitTest TEST = new QuarkusUnitTest() .withApplicationRoot((jar) -> jar .addClasses(ConfiguredBean.class) - // Don't change this to types, because of classloader class cast exception. - .addAsServiceProvider("org.eclipse.microprofile.config.spi.ConfigSource", - "io.quarkus.extest.runtime.config.OverrideBuildTimeConfigSource\n" + - "io.quarkus.extest.runtime.config.RecordQuarkusSystemPropertiesConfigSource") + .addAsServiceProvider(ConfigSource.class, + OverrideBuildTimeConfigSource.class, + DoNotRecordEnvConfigSource.class) .addAsResource("application.properties")); @Inject @@ -364,11 +364,16 @@ public void testProfileDefaultValuesSource() { assertEquals("5678", defaultValues.getValue("%dev.my.prop")); assertEquals("1234", defaultValues.getValue("%test.my.prop")); assertEquals("1234", config.getValue("my.prop", String.class)); + + // runtime properties coming from env must not be recorded assertNull(defaultValues.getValue("should.not.be.recorded")); assertNull(defaultValues.getValue("SHOULD_NOT_BE_RECORDED")); - assertEquals("value", defaultValues.getValue("quarkus.mapping.rt.record")); - assertEquals("prod", defaultValues.getValue("%prod.quarkus.mapping.rt.record")); - assertEquals("dev", defaultValues.getValue("%dev.quarkus.mapping.rt.record")); + assertNull(defaultValues.getValue("quarkus.rt.do-not-record")); + assertEquals("value", config.getRawValue("quarkus.rt.do-not-record")); + assertNull(defaultValues.getValue("quarkus.mapping.rt.do-not-record")); + assertNull(defaultValues.getValue("%prod.quarkus.mapping.rt.do-not-record")); + assertNull(defaultValues.getValue("%dev.quarkus.mapping.rt.do-not-record")); + assertEquals("value", config.getRawValue("quarkus.mapping.rt.do-not-record")); } @Test diff --git a/integration-tests/test-extension/extension/deployment/src/test/java/io/quarkus/extest/RuntimeDefaultsTest.java b/integration-tests/test-extension/extension/deployment/src/test/java/io/quarkus/extest/RuntimeDefaultsTest.java index 1e10a13c097df..f949938609e37 100644 --- a/integration-tests/test-extension/extension/deployment/src/test/java/io/quarkus/extest/RuntimeDefaultsTest.java +++ b/integration-tests/test-extension/extension/deployment/src/test/java/io/quarkus/extest/RuntimeDefaultsTest.java @@ -12,6 +12,7 @@ import org.junit.jupiter.api.Test; import org.junit.jupiter.api.extension.RegisterExtension; +import io.quarkus.extest.runtime.config.EnvBuildTimeConfigSource; import io.quarkus.test.QuarkusUnitTest; import io.smallrye.config.SmallRyeConfig; @@ -19,9 +20,7 @@ public class RuntimeDefaultsTest { @RegisterExtension static final QuarkusUnitTest TEST = new QuarkusUnitTest() .withApplicationRoot((jar) -> jar - // Don't change this to types, because of classloader class cast exception. - .addAsServiceProvider("org.eclipse.microprofile.config.spi.ConfigSource", - "io.quarkus.extest.runtime.config.EnvBuildTimeConfigSource") + .addAsServiceProvider(ConfigSource.class, EnvBuildTimeConfigSource.class) .addAsResource("application.properties")); @Inject @@ -31,13 +30,17 @@ public class RuntimeDefaultsTest { void doNotRecordEnvRuntimeDefaults() { Optional defaultValues = config.getConfigSource("DefaultValuesConfigSource"); assertTrue(defaultValues.isPresent()); - // It's ok to record env properties for a Quarkus root - assertEquals("changed", defaultValues.get().getValue("quarkus.rt.rt-string-opt")); - // It's ok to record env properties for a property available in another source - assertEquals("env-source", defaultValues.get().getValue("bt.ok.to.record")); + // Do not record Env values for runtime + assertNull(defaultValues.get().getValue("quarkus.mapping.rt.do-not-record")); + assertEquals("value", config.getRawValue("quarkus.mapping.rt.do-not-record")); + // Property available in both Env and application.properties, ok to record application.properties value + assertEquals("from-app", defaultValues.get().getValue("bt.ok.to.record")); + // You still get the value from Env + assertEquals("from-env", config.getRawValue("bt.ok.to.record")); // Do not record any of the other properties assertNull(defaultValues.get().getValue(("do.not.record"))); assertNull(defaultValues.get().getValue(("DO_NOT_RECORD"))); + assertEquals("value", config.getRawValue("do.not.record")); } @Test @@ -45,6 +48,7 @@ void doNotRecordActiveUnprofiledPropertiesDefaults() { Optional defaultValues = config.getConfigSource("DefaultValuesConfigSource"); assertTrue(defaultValues.isPresent()); assertEquals("properties", config.getRawValue("bt.profile.record")); + // Property needs to be recorded as is, including the profile name assertEquals("properties", defaultValues.get().getValue("%test.bt.profile.record")); assertNull(defaultValues.get().getValue("bt.profile.record")); } diff --git a/integration-tests/test-extension/extension/runtime/src/main/java/io/quarkus/extest/runtime/config/DoNotRecordEnvConfigSource.java b/integration-tests/test-extension/extension/runtime/src/main/java/io/quarkus/extest/runtime/config/DoNotRecordEnvConfigSource.java new file mode 100644 index 0000000000000..bae3698633fd2 --- /dev/null +++ b/integration-tests/test-extension/extension/runtime/src/main/java/io/quarkus/extest/runtime/config/DoNotRecordEnvConfigSource.java @@ -0,0 +1,19 @@ +package io.quarkus.extest.runtime.config; + +import java.util.Map; + +import io.quarkus.runtime.annotations.StaticInitSafe; +import io.smallrye.config.EnvConfigSource; + +@StaticInitSafe +public class DoNotRecordEnvConfigSource extends EnvConfigSource { + public DoNotRecordEnvConfigSource() { + super(Map.of( + "SHOULD_NOT_BE_RECORDED", "value", + "should.not.be.recorded", "value", + "quarkus.rt.do-not-record", "value", + "quarkus.mapping.rt.do-not-record", "value", + "%dev.quarkus.mapping.rt.do-not-record", "dev", + "_PROD_QUARKUS_MAPPING_RT_DO_NOT_RECORD", "prod"), 300); + } +} diff --git a/integration-tests/test-extension/extension/runtime/src/main/java/io/quarkus/extest/runtime/config/EnvBuildTimeConfigSource.java b/integration-tests/test-extension/extension/runtime/src/main/java/io/quarkus/extest/runtime/config/EnvBuildTimeConfigSource.java index bb98ddfbba959..61a24e8956bcc 100644 --- a/integration-tests/test-extension/extension/runtime/src/main/java/io/quarkus/extest/runtime/config/EnvBuildTimeConfigSource.java +++ b/integration-tests/test-extension/extension/runtime/src/main/java/io/quarkus/extest/runtime/config/EnvBuildTimeConfigSource.java @@ -6,8 +6,10 @@ public class EnvBuildTimeConfigSource extends EnvConfigSource { public EnvBuildTimeConfigSource() { - super(Map.of("QUARKUS_RT_RT_STRING_OPT", "changed", - "BT_OK_TO_RECORD", "env-source", - "DO_NOT_RECORD", "record"), Integer.MAX_VALUE); + super(Map.of( + "QUARKUS_MAPPING_RT_DO_NOT_RECORD", "value", + "BT_OK_TO_RECORD", "from-env", + "BT_DO_NOT_RECORD", "value", + "DO_NOT_RECORD", "value"), Integer.MAX_VALUE); } } diff --git a/integration-tests/test-extension/extension/runtime/src/main/java/io/quarkus/extest/runtime/config/RecordQuarkusSystemPropertiesConfigSource.java b/integration-tests/test-extension/extension/runtime/src/main/java/io/quarkus/extest/runtime/config/RecordQuarkusSystemPropertiesConfigSource.java deleted file mode 100644 index 4463006b77573..0000000000000 --- a/integration-tests/test-extension/extension/runtime/src/main/java/io/quarkus/extest/runtime/config/RecordQuarkusSystemPropertiesConfigSource.java +++ /dev/null @@ -1,18 +0,0 @@ -package io.quarkus.extest.runtime.config; - -import java.util.Map; - -import io.quarkus.runtime.annotations.StaticInitSafe; -import io.smallrye.config.EnvConfigSource; - -@StaticInitSafe -public class RecordQuarkusSystemPropertiesConfigSource extends EnvConfigSource { - public RecordQuarkusSystemPropertiesConfigSource() { - super(Map.of( - "SHOULD_NOT_BE_RECORDED", "value", - "should.not.be.recorded", "value", - "quarkus.mapping.rt.record", "value", - "%dev.quarkus.mapping.rt.record", "dev", - "_PROD_QUARKUS_MAPPING_RT_RECORD", "prod"), 0); - } -} diff --git a/integration-tests/test-extension/extension/runtime/src/main/java/io/quarkus/extest/runtime/config/TestMappingRunTime.java b/integration-tests/test-extension/extension/runtime/src/main/java/io/quarkus/extest/runtime/config/TestMappingRunTime.java index e585f5ea520c6..17930dbc19e4a 100644 --- a/integration-tests/test-extension/extension/runtime/src/main/java/io/quarkus/extest/runtime/config/TestMappingRunTime.java +++ b/integration-tests/test-extension/extension/runtime/src/main/java/io/quarkus/extest/runtime/config/TestMappingRunTime.java @@ -20,7 +20,7 @@ public interface TestMappingRunTime { Group group(); /** Record values from env test **/ - Optional record(); + Optional doNotRecord(); /** Record values with named profile **/ Optional recordProfiled(); diff --git a/integration-tests/test-extension/extension/runtime/src/main/java/io/quarkus/extest/runtime/config/TestRunTimeConfig.java b/integration-tests/test-extension/extension/runtime/src/main/java/io/quarkus/extest/runtime/config/TestRunTimeConfig.java index ab963f4ec3000..254a9c3b6c2ea 100644 --- a/integration-tests/test-extension/extension/runtime/src/main/java/io/quarkus/extest/runtime/config/TestRunTimeConfig.java +++ b/integration-tests/test-extension/extension/runtime/src/main/java/io/quarkus/extest/runtime/config/TestRunTimeConfig.java @@ -109,6 +109,10 @@ public class TestRunTimeConfig { public Map> mapMap; + /** Do not record **/ + @ConfigItem + public Optional doNotRecord; + @Override public String toString() { return "TestRunTimeConfig{" +