Fix processing of env variable references (#20462)

Closes #20032

quarkus/runtime/src/main/java/org/keycloak/quarkus/runtime/configuration/PropertyMappingInterceptor.java

@@ -16,38 +16,53 @@
  */
 package org.keycloak.quarkus.runtime.configuration;
 
-import java.util.HashMap;
-import java.util.Map;
-import java.util.Optional;
-import jakarta.annotation.Priority;
-
 import io.smallrye.config.ConfigSourceInterceptor;
 import io.smallrye.config.ConfigSourceInterceptorContext;
 import io.smallrye.config.ConfigValue;
-import io.smallrye.config.Priorities;
-
+import org.keycloak.common.util.StringPropertyReplacer;
 import org.keycloak.quarkus.runtime.configuration.mappers.PropertyMapper;
 import org.keycloak.quarkus.runtime.configuration.mappers.PropertyMappers;
 
 /**
  * <p>This interceptor is responsible for mapping Keycloak properties to their corresponding properties in Quarkus.
- * 
+ *
  * <p>A single property in Keycloak may span a single or multiple properties on Quarkus and for each property we want to map
  * from Quarkus we should configure a {@link PropertyMapper}.
- * 
+ *
  * <p>The {@link PropertyMapper} can either perform a 1:1 mapping where the value of a property from
  * Keycloak (e.g.: https.port) is mapped to a single properties in Quarkus, or perform a 1:N mapping where the value of a property
  * from Keycloak (e.g.: database) is mapped to multiple properties in Quarkus.
  *
- * <p>This interceptor should execute between the {@link io.smallrye.config.ExpressionConfigSourceInterceptor} and the {@link io.smallrye.config.ProfileConfigSourceInterceptor}
- * so that expressions are properly resolved after executing this interceptor while still able to resolve properties based on the current
- * profile.
+ * <p>This interceptor must execute after the {@link io.smallrye.config.ExpressionConfigSourceInterceptor} so that expressions
+ * are properly resolved before executing this interceptor. Hence, leaving the default priority.
  */
-@Priority(Priorities.LIBRARY + 201)
 public class PropertyMappingInterceptor implements ConfigSourceInterceptor {
 
     @Override
     public ConfigValue getValue(ConfigSourceInterceptorContext context, String name) {
-        return PropertyMappers.getValue(context, name);
+        ConfigValue value = PropertyMappers.getValue(context, name);
+
+        if (value == null || value.getValue() == null) {
+            return null;
+        }
+
+        if (!value.getValue().contains("${")) {
+            return value;
+        }
+
+        // Our mappers might have returned a value containing an expression ${...}.
+        // However, ExpressionConfigSourceInterceptor was already executed before (to expand e.g. env vars in config file).
+        // Hence, we need to manually resolve these expressions here. Not ideal, but there's no other way (at least I haven't found one).
+        return value.withValue(
+                StringPropertyReplacer.replaceProperties(value.getValue(),
+                        property -> {
+                            ConfigValue prop = context.proceed(property);
+
+                            if (prop == null) {
+                                return null;
+                            }
+
+                            return prop.getValue();
+                        }));
     }
 }

quarkus/tests/integration/src/test/java/org/keycloak/it/cli/dist/OptionsDistTest.java

@@ -17,17 +17,22 @@
 
 package org.keycloak.it.cli.dist;
 
-import static org.junit.Assert.assertEquals;
-
-import org.junit.jupiter.api.Assertions;
-import org.junit.jupiter.api.Test;
-import org.keycloak.it.junit5.extension.DistributionTest;
-
 import io.quarkus.test.junit.main.Launch;
 import io.quarkus.test.junit.main.LaunchResult;
+import org.junit.jupiter.api.Assertions;
+import org.junit.jupiter.api.Test;
+import org.keycloak.it.junit5.extension.CLIResult;
+import org.keycloak.it.junit5.extension.DistributionTest;
+import org.keycloak.it.junit5.extension.RawDistOnly;
+import org.keycloak.it.utils.KeycloakDistribution;
+
+import java.nio.file.Paths;
+
+import static org.junit.Assert.assertEquals;
+import static org.keycloak.quarkus.runtime.cli.command.Main.CONFIG_FILE_LONG_NAME;
 
 @DistributionTest
-public class OptionValidationDistTest {
+public class OptionsDistTest {
 
     @Test
     @Launch({"build", "--db=invalid"})
@@ -46,4 +51,12 @@ public class OptionValidationDistTest {
     public void testServerDoesNotStartIfValidationFailDuringReAugStart(LaunchResult result) {
         assertEquals(1, result.getErrorStream().stream().filter(s -> s.contains("Unknown option: '--test'")).count());
     }
+
+    @Test
+    @RawDistOnly(reason = "Raw is enough and we avoid issues with including custom conf file in the container")
+    public void testExpressionsInConfigFile(KeycloakDistribution distribution) {
+        distribution.setEnvVar("MY_LOG_LEVEL", "debug");
+        CLIResult result = distribution.run(CONFIG_FILE_LONG_NAME + "=" + Paths.get("src/test/resources/OptionsDistTest/keycloak.conf").toAbsolutePath().normalize(), "start-dev");
+        result.assertMessage("DEBUG [org.keycloak");
+    }
 }

quarkus/tests/integration/src/test/resources/OptionsDistTest/keycloak.conf

@@ -0,0 +1 @@
+log-level=${MY_LOG_LEVEL:warn}