fix: generalizing the augmentation check (#47748)

closes: #47659

Signed-off-by: Steve Hawkins <shawkins@redhat.com>
This commit is contained in:
Steven Hawkins 2026-04-29 10:06:53 -04:00 committed by GitHub
parent 6e49ecd3c8
commit bc11757f22
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
7 changed files with 25 additions and 20 deletions

View file

@ -40,7 +40,6 @@ public final class Environment {
public static final String KC_CONFIG_REBUILD_CHECK = "kc.config.rebuild-check";
public static final String KC_CONFIG_BUILT = "kc.config.built";
public static final String KC_TEST_REBUILD = "kc.test.rebuild";
public static final String KC_HOME_DIR = "kc.home.dir";
public static final String PROFILE ="kc.profile";
public static final String ENV_PROFILE ="KC_PROFILE";

View file

@ -73,8 +73,8 @@ public class NestedPropertyMappingInterceptor implements ConfigSourceInterceptor
return Optional.ofNullable(recursions.get()).filter(s -> !s.isEmpty()).map(s -> s.iterator().next());
}
public static ConfigValue getValueFromPropertyMappers(ConfigSourceInterceptorContext context, String name) {
Function<String, ConfigValue> resolver = (n) -> PropertyMappers.getValue(context, n);
public static ConfigValue getValueFromPropertyMappers(ConfigSourceInterceptorContext context, String name, boolean augmenting) {
Function<String, ConfigValue> resolver = (n) -> PropertyMappers.getValue(context, n, augmenting);
return resolve(resolver, resolver, name, true);
}

View file

@ -27,7 +27,6 @@ import java.util.stream.StreamSupport;
import jakarta.annotation.Priority;
import org.keycloak.config.OptionCategory;
import org.keycloak.quarkus.runtime.Environment;
import org.keycloak.quarkus.runtime.configuration.mappers.PropertyMapper;
import org.keycloak.quarkus.runtime.configuration.mappers.PropertyMappers;
import org.keycloak.quarkus.runtime.configuration.mappers.WildcardPropertyMapper;
@ -38,7 +37,6 @@ import io.smallrye.config.ConfigValue;
import io.smallrye.config.Priorities;
import org.apache.commons.collections4.IteratorUtils;
import static org.keycloak.quarkus.runtime.Environment.isRebuild;
import static org.keycloak.quarkus.runtime.configuration.MicroProfileConfigProvider.NS_KEYCLOAK_PREFIX;
/**
@ -64,6 +62,8 @@ public class PropertyMappingInterceptor implements ConfigSourceInterceptor {
private static final ThreadLocal<Boolean> disable = new ThreadLocal<>();
private Boolean augmenting;
public static void disable() {
disable.set(true);
}
@ -93,7 +93,7 @@ public class PropertyMappingInterceptor implements ConfigSourceInterceptor {
final Set<PropertyMapper<?>> mappersWithoutValues = PropertyMappers.getMappers();
boolean filterRuntime = isRebuild() || Boolean.getBoolean(Environment.KC_TEST_REBUILD);
boolean filterRuntime = isAugmenting(context);
// this will return different iterations when our configuration is initialized vs not.
// when the configuration is not initialized, we only need to worry about providing
@ -153,6 +153,14 @@ public class PropertyMappingInterceptor implements ConfigSourceInterceptor {
return IteratorUtils.chainedIterator(baseStream.iterator(), inferredValueStream.iterator());
}
private boolean isAugmenting(ConfigSourceInterceptorContext context) {
if (augmenting == null) {
// see BuildTimeConfigurationReader - if a sys properties are excluded, then we're augmenting
augmenting = context.proceed("file.separator") == null;
}
return augmenting;
}
private void appendWildcardsMappedFrom(ConfigSourceInterceptorContext context, String name,
final PropertyMapper<?> mapper, List<String> names) {
var wildCards = PropertyMappers.getWildcardsMappedFrom(mapper.getOption());
@ -210,6 +218,6 @@ public class PropertyMappingInterceptor implements ConfigSourceInterceptor {
}
// Call through NestedPropertyMappingInterceptor to track what we are currently getting the value for
return NestedPropertyMappingInterceptor.getValueFromPropertyMappers(context, name);
return NestedPropertyMappingInterceptor.getValueFromPropertyMappers(context, name, isAugmenting(context));
}
}

View file

@ -34,7 +34,6 @@ import io.smallrye.config.ConfigValue;
import io.smallrye.config.Expressions;
import org.jboss.logging.Logger;
import static org.keycloak.quarkus.runtime.Environment.isRebuild;
import static org.keycloak.quarkus.runtime.Environment.isRebuildCheck;
import static org.keycloak.quarkus.runtime.configuration.KeycloakConfigSourceProvider.isKeyStoreConfigSource;
@ -72,7 +71,7 @@ public final class PropertyMappers {
GROUPINGS.forEach(g -> MAPPERS.addAll(g.getPropertyMappers()));
}
public static ConfigValue getValue(ConfigSourceInterceptorContext context, String name) {
public static ConfigValue getValue(ConfigSourceInterceptorContext context, String name, boolean augmenting) {
PropertyMapper<?> mapper = getMapper(name);
// During re-aug do not resolve server runtime properties and avoid including in the quarkus default value config source.
@ -81,7 +80,7 @@ public final class PropertyMappers {
// and we need to resolve them. That should be fine as they are generally not considered security sensitive.
// If however expressions are not enabled that means quarkus is specifically looking for runtime defaults, and we should not provide a value
// See https://github.com/quarkusio/quarkus/pull/42157
if ((isRebuild() || Boolean.getBoolean(Environment.KC_TEST_REBUILD)) && isKeycloakRuntime(name, mapper)
if (augmenting && isKeycloakRuntime(name, mapper)
&& (NestedPropertyMappingInterceptor.getResolvingRoot().or(() -> Optional.of(name))
.filter(n -> n.startsWith("quarkus.log.") || n.startsWith("quarkus.console.")).isEmpty()
|| !Expressions.isEnabled())) {

View file

@ -38,6 +38,7 @@ import org.keycloak.quarkus.runtime.vault.FilesPlainTextVaultProviderFactory;
import org.keycloak.spi.infinispan.CacheEmbeddedConfigProviderSpi;
import org.keycloak.spi.infinispan.impl.embedded.DefaultCacheEmbeddedConfigProviderFactory;
import io.quarkus.runtime.configuration.ConfigUtils;
import io.smallrye.config.ConfigValue;
import io.smallrye.config.Expressions;
import io.smallrye.config.PropertiesConfigSource;
@ -103,8 +104,8 @@ public class ConfigurationTest extends AbstractConfigurationTest {
assertTrue(Configuration.getConfig().isPropertyPresent("quarkus.log.category.\"io.k8s\".level"));
putEnvVar("SOME_LOG_LEVEL", "debug");
assertEquals("debug", createConfig().getRawValue("kc.log-level"));
Environment.setRebuild();
assertNull(Expressions.withoutExpansion(() -> Configuration.getConfigValue("kc.log-level")).getValue());
SmallRyeConfig config = ConfigUtils.emptyConfigBuilder().setAddDefaultSources(false).addDiscoveredSources().build();
assertNull(Expressions.withoutExpansion(() -> config.getConfigValue("kc.log-level")).getValue());
}
@Test

View file

@ -92,12 +92,13 @@ class BuildCommandDistTest {
@Test
@RawDistOnly(reason = "Containers are immutable")
void testDoNotRecordRuntimeOptionsDuringBuild(KeycloakDistribution distribution) {
distribution.setProperty("proxy", "edge");
distribution.run("build");
distribution.removeProperty("proxy");
distribution.setProperty("db-url", "invalid");
CLIResult cliResult = distribution.run("build");
cliResult.assertBuild();
distribution.removeProperty("db-url");
CLIResult result = distribution.run("start", "--hostname=mykeycloak", "--cache=local", OPTIMIZED_BUILD_OPTION_LONG);
result.assertError("Key material not provided to setup HTTPS");
CLIResult result = distribution.run("start", "--hostname=mykeycloak", "--cache=local", "--http-enabled=true", OPTIMIZED_BUILD_OPTION_LONG);
result.assertStarted();
}
@Test

View file

@ -211,10 +211,7 @@ public class Keycloak {
if (!initSys(args.toArray(String[]::new))) {
return this;
}
System.setProperty(Environment.KC_TEST_REBUILD, "true");
StartupAction startupAction = action.createInitialRuntimeApplication();
System.getProperties().remove(Environment.KC_TEST_REBUILD);
application = startupAction.runMainClass(args.toArray(new String[0]));
return this;