Skip to content

Commit

Permalink
Do not record local sources in runtime config defaults
Browse files Browse the repository at this point in the history
  • Loading branch information
radcortez committed Mar 21, 2024
1 parent 7eb8479 commit 592b1b2
Show file tree
Hide file tree
Showing 13 changed files with 110 additions and 66 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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());
}
Expand All @@ -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());
}
Expand All @@ -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());
}
}
Expand All @@ -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());
}
Expand All @@ -664,8 +668,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());
}
}
Expand Down Expand Up @@ -1117,6 +1121,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()) {
Expand All @@ -1131,13 +1170,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) {
Expand All @@ -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));
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,11 +8,7 @@
@IfBuildProfile("foo")
@ApplicationScoped
public class HelloService {

@ConfigProperty(name = "name")
String name;

public String name() {
return name;
return "from foo";
}
}

This file was deleted.

Original file line number Diff line number Diff line change
@@ -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";
}
}

This file was deleted.

Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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
Expand Down Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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"));
}
Expand Down
Original file line number Diff line number Diff line change
@@ -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);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
}

This file was deleted.

Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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{" +
Expand Down

0 comments on commit 592b1b2

Please sign in to comment.