From 7ef44e5f93a63fba160709059b51ba7c9c1f914f Mon Sep 17 00:00:00 2001 From: Steven Hawkins Date: Tue, 2 Sep 2025 11:02:22 -0400 Subject: [PATCH] fix: improve handling when expressions are disabled (#42189) (#42240) closes: #42158 (cherry picked from commit f52421fe4442269be848696e001a4767c7db837f) Signed-off-by: Steve Hawkins --- .../java/org/keycloak/quarkus/runtime/Environment.java | 4 ++++ .../org/keycloak/quarkus/runtime/cli/command/Build.java | 2 +- .../runtime/configuration/mappers/PropertyMapper.java | 8 ++------ .../runtime/configuration/mappers/PropertyMappers.java | 4 +++- .../quarkus/runtime/configuration/ConfigurationTest.java | 5 ++++- .../configuration/DatasourcesConfigurationTest.java | 2 +- .../org/keycloak/it/cli/dist/BuildCommandDistTest.java | 8 ++++++++ 7 files changed, 23 insertions(+), 10 deletions(-) diff --git a/quarkus/runtime/src/main/java/org/keycloak/quarkus/runtime/Environment.java b/quarkus/runtime/src/main/java/org/keycloak/quarkus/runtime/Environment.java index bc3795d01dd..e6194711d29 100644 --- a/quarkus/runtime/src/main/java/org/keycloak/quarkus/runtime/Environment.java +++ b/quarkus/runtime/src/main/java/org/keycloak/quarkus/runtime/Environment.java @@ -265,4 +265,8 @@ public final class Environment { public static void removeHomeDir() { System.getProperties().remove(KC_HOME_DIR); } + + public static void setRebuild() { + System.setProperty("quarkus.launch.rebuild", "true"); + } } diff --git a/quarkus/runtime/src/main/java/org/keycloak/quarkus/runtime/cli/command/Build.java b/quarkus/runtime/src/main/java/org/keycloak/quarkus/runtime/cli/command/Build.java index a697f1a98c1..e469be9118e 100644 --- a/quarkus/runtime/src/main/java/org/keycloak/quarkus/runtime/cli/command/Build.java +++ b/quarkus/runtime/src/main/java/org/keycloak/quarkus/runtime/cli/command/Build.java @@ -80,7 +80,7 @@ public final class Build extends AbstractCommand implements Runnable { validateConfig(); return null; }); - System.setProperty("quarkus.launch.rebuild", "true"); + Environment.setRebuild(); println(spec.commandLine(), "Updating the configuration and installing your custom providers, if any. Please wait."); diff --git a/quarkus/runtime/src/main/java/org/keycloak/quarkus/runtime/configuration/mappers/PropertyMapper.java b/quarkus/runtime/src/main/java/org/keycloak/quarkus/runtime/configuration/mappers/PropertyMapper.java index 69a83b848c0..942f0ffc869 100644 --- a/quarkus/runtime/src/main/java/org/keycloak/quarkus/runtime/configuration/mappers/PropertyMapper.java +++ b/quarkus/runtime/src/main/java/org/keycloak/quarkus/runtime/configuration/mappers/PropertyMapper.java @@ -35,7 +35,6 @@ import java.util.stream.Stream; import org.keycloak.config.DeprecatedMetadata; import org.keycloak.config.Option; import org.keycloak.config.OptionCategory; -import org.keycloak.quarkus.runtime.Environment; import org.keycloak.quarkus.runtime.cli.PropertyException; import org.keycloak.quarkus.runtime.cli.ShortErrorMessageHandler; import org.keycloak.quarkus.runtime.configuration.ConfigArgsConfigSource; @@ -261,7 +260,8 @@ public class PropertyMapper { boolean mapped = false; // fall back to the transformer when no mapper is explicitly specified in .mapFrom() var theMapper = parentValue && parentMapper != null ? this.parentMapper : this.mapper; - if (theMapper != null && (!name.equals(getFrom()) || parentValue)) { + // since our mapping logic assumes fully resolved values, we cannot reliably map if Expressions are disabled + if (Expressions.isEnabled() && theMapper != null && (!name.equals(getFrom()) || parentValue)) { mappedValue = theMapper.map(getNamedProperty().orElse(null), value, context); mapped = true; } @@ -281,10 +281,6 @@ public class PropertyMapper { return configValue; } - if (!isBuildTime() && Environment.isRebuild()) { - value = null; // prevent quarkus from recording these raw values as runtime defaults - } - // by unsetting the ordinal this will not be seen as directly modified by the user return configValue.from().withName(name).withValue(mappedValue).withRawValue(value).withConfigSourceOrdinal(0).build(); } diff --git a/quarkus/runtime/src/main/java/org/keycloak/quarkus/runtime/configuration/mappers/PropertyMappers.java b/quarkus/runtime/src/main/java/org/keycloak/quarkus/runtime/configuration/mappers/PropertyMappers.java index 3fc5e9214dc..ad18b9f5d74 100644 --- a/quarkus/runtime/src/main/java/org/keycloak/quarkus/runtime/configuration/mappers/PropertyMappers.java +++ b/quarkus/runtime/src/main/java/org/keycloak/quarkus/runtime/configuration/mappers/PropertyMappers.java @@ -2,6 +2,7 @@ package org.keycloak.quarkus.runtime.configuration.mappers; import io.smallrye.config.ConfigSourceInterceptorContext; import io.smallrye.config.ConfigValue; +import io.smallrye.config.Expressions; import jakarta.ws.rs.core.MultivaluedHashMap; import org.jboss.logging.Logger; import org.keycloak.common.util.CollectionUtil; @@ -84,9 +85,10 @@ public final class PropertyMappers { // // The special handling of log properties is because some logging runtime properties are requested during build time // 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() && isKeycloakRuntime(name, mapper) - && !NestedPropertyMappingInterceptor.getResolvingRoot().orElse(name).startsWith("quarkus.log.")) { + && (!NestedPropertyMappingInterceptor.getResolvingRoot().orElse(name).startsWith("quarkus.log.") || !Expressions.isEnabled())) { return ConfigValue.builder().withName(name).build(); } diff --git a/quarkus/runtime/src/test/java/org/keycloak/quarkus/runtime/configuration/ConfigurationTest.java b/quarkus/runtime/src/test/java/org/keycloak/quarkus/runtime/configuration/ConfigurationTest.java index 3714023fe02..9d338d87b1d 100644 --- a/quarkus/runtime/src/test/java/org/keycloak/quarkus/runtime/configuration/ConfigurationTest.java +++ b/quarkus/runtime/src/test/java/org/keycloak/quarkus/runtime/configuration/ConfigurationTest.java @@ -82,8 +82,11 @@ public class ConfigurationTest extends AbstractConfigurationTest { @Test public void testKeycloakConfPlaceholder() { assertEquals("info", createConfig().getRawValue("kc.log-level")); + 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()); } @Test @@ -250,7 +253,7 @@ public class ConfigurationTest extends AbstractConfigurationTest { ConfigArgsConfigSource.setCliArgs("--db=mysql"); SmallRyeConfig config = createConfig(); String value = Expressions.withoutExpansion(() -> config.getConfigValue("quarkus.datasource.jdbc.url").getValue()); - assertEquals("jdbc:mysql://${kc.db-url-host:localhost}:${kc.db-url-port:3306}/${kc.db-url-database:keycloak}${kc.db-url-properties:}", value); + assertEquals("mysql", value); } @Test diff --git a/quarkus/runtime/src/test/java/org/keycloak/quarkus/runtime/configuration/DatasourcesConfigurationTest.java b/quarkus/runtime/src/test/java/org/keycloak/quarkus/runtime/configuration/DatasourcesConfigurationTest.java index b593412a04e..7474cc0b206 100644 --- a/quarkus/runtime/src/test/java/org/keycloak/quarkus/runtime/configuration/DatasourcesConfigurationTest.java +++ b/quarkus/runtime/src/test/java/org/keycloak/quarkus/runtime/configuration/DatasourcesConfigurationTest.java @@ -85,7 +85,7 @@ public class DatasourcesConfigurationTest extends AbstractConfigurationTest { ConfigArgsConfigSource.setCliArgs("--db-kind-store=mysql"); SmallRyeConfig config = createConfig(); String value = Expressions.withoutExpansion(() -> config.getConfigValue("quarkus.datasource.\"store\".jdbc.url").getValue()); - assertEquals("jdbc:mysql://${kc.db-url-host-store:localhost}:${kc.db-url-port-store:3306}/${kc.db-url-database-store:keycloak}${kc.db-url-properties-store:}", value); + assertEquals("mysql", value); assertExternalConfig("quarkus.datasource.\"store\".jdbc.url", "jdbc:mysql://localhost:3306/keycloak"); } diff --git a/quarkus/tests/integration/src/test/java/org/keycloak/it/cli/dist/BuildCommandDistTest.java b/quarkus/tests/integration/src/test/java/org/keycloak/it/cli/dist/BuildCommandDistTest.java index c887de3a70b..0894461ef12 100644 --- a/quarkus/tests/integration/src/test/java/org/keycloak/it/cli/dist/BuildCommandDistTest.java +++ b/quarkus/tests/integration/src/test/java/org/keycloak/it/cli/dist/BuildCommandDistTest.java @@ -102,4 +102,12 @@ class BuildCommandDistTest { cliResult.assertError(String.format("ERROR: Unable to find the JDBC driver (%s). You need to install it.", dbDriver)); cliResult.assertNoBuild(); } + + @Test + @RawDistOnly(reason = "Containers are immutable") + @WithEnvVars({"KC_LOG_LEVEL", "${KEYCLOAK_LOG_LEVEL:INFO},org.keycloak.events:DEBUG"}) + @Launch({"build", "--db=dev-file"}) + void logLevelExpressionWithDefault(CLIResult cliResult) { + cliResult.assertBuild(); + } }