Merge pull request quarkusio#39604 from radcortez/local-config-source…

…s-record-defaults

Do not record local sources in runtime config defaults.

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;
@@ -513,10 +515,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
@@ -603,7 +607,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());
                         }
@@ -614,7 +618,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());
                     }
@@ -641,7 +645,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());
                     }
                 }
@@ -653,7 +657,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());
                     }
@@ -665,8 +669,8 @@ ReadResult run() {
                 Set<String> 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());
                     }
                 }
@@ -1118,6 +1122,41 @@ public Set<String> 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<String, String> filterActiveProfileProperties(final Map<String, String> properties) {
             Set<String> propertiesToRemove = new HashSet<>();
             for (String property : properties.keySet()) {
@@ -1132,13 +1171,15 @@ private Map<String, String> filterActiveProfileProperties(final Map<String, Stri
             return properties;
         }
 
-        private Map<String, String> getDefaults(final ConfigPatternMap<Container> patternMap) {
+        private static Map<String, String> getDefaults(final SmallRyeConfig config,
+                final ConfigPatternMap<Container> patternMap) {
             Map<String, String> 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<String, String> defaultValues,
                 final StringBuilder propertyName,
                 final ConfigPatternMap<Container> patternMap) {
@@ -1165,7 +1206,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));
             }

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";
     }
 }

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";
     }
 }

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

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

integration-tests/test-extension/extension/deployment/src/test/java/io/quarkus/extest/RuntimeDefaultsTest.java

@@ -12,16 +12,15 @@
 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;
 
 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,20 +30,25 @@ public class RuntimeDefaultsTest {
     void doNotRecordEnvRuntimeDefaults() {
         Optional<ConfigSource> 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
     void doNotRecordActiveUnprofiledPropertiesDefaults() {
         Optional<ConfigSource> 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"));
     }

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);
+    }
+}

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);
     }
 }

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<String> record();
+    Optional<String> doNotRecord();
 
     /** Record values with named profile **/
     Optional<String> recordProfiled();

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<String, Map<String, String>> mapMap;
 
+    /** Do not record **/
+    @ConfigItem
+    public Optional<String> doNotRecord;
+
     @Override
     public String toString() {
         return "TestRunTimeConfig{" +