Show warning message when overriding build options during starts (#20585)

Closes #20582

quarkus/deployment/src/main/java/org/keycloak/quarkus/deployment/KeycloakProcessor.java

@@ -66,7 +66,6 @@ import org.keycloak.common.util.StreamUtil;
 import org.keycloak.config.DatabaseOptions;
 import org.keycloak.config.SecurityOptions;
 import org.keycloak.config.StorageOptions;
-import org.keycloak.config.TransactionOptions;
 import org.keycloak.connections.jpa.DefaultJpaConnectionProviderFactory;
 import org.keycloak.connections.jpa.JpaConnectionProvider;
 import org.keycloak.connections.jpa.JpaConnectionSpi;
@@ -146,7 +145,6 @@ import static org.keycloak.quarkus.runtime.KeycloakRecorder.DEFAULT_HEALTH_ENDPO
 import static org.keycloak.quarkus.runtime.KeycloakRecorder.DEFAULT_METRICS_ENDPOINT;
 import static org.keycloak.quarkus.runtime.Providers.getProviderManager;
 import static org.keycloak.quarkus.runtime.configuration.Configuration.getKcConfigValue;
-import static org.keycloak.quarkus.runtime.configuration.Configuration.getOptionalKcBooleanValue;
 import static org.keycloak.quarkus.runtime.configuration.Configuration.getOptionalKcValue;
 import static org.keycloak.quarkus.runtime.configuration.Configuration.getOptionalValue;
 import static org.keycloak.quarkus.runtime.configuration.Configuration.getPropertyNames;
@@ -501,6 +499,11 @@ class KeycloakProcessor {
             properties.put(String.format("kc.provider.file.%s.last-modified", jar.getName()), String.valueOf(jar.lastModified()));
         }
 
+        if (!Environment.isRebuildCheck()) {
+            // not auto-build (e.g.: start without optimized option) but a regular build to create an optimized server image
+            Configuration.markAsOptimized(properties);
+        }
+
         String profile = Environment.getProfile();
 
         if (profile != null) {
@@ -536,6 +539,10 @@ class KeycloakProcessor {
         }
 
         if (value != null && value.getValue() != null) {
+            if (value.getConfigSourceName() == null) {
+                // only persist build options resolved from config sources and not default values
+                return;
+            }
             String rawValue = value.getRawValue();
 
             if (rawValue == null) {

quarkus/runtime/src/main/java/org/keycloak/quarkus/runtime/cli/Picocli.java

@@ -17,6 +17,8 @@
 
 package org.keycloak.quarkus.runtime.cli;
 
+import static java.util.Optional.ofNullable;
+import static java.util.stream.StreamSupport.stream;
 import static org.keycloak.quarkus.runtime.Environment.isRebuildCheck;
 import static org.keycloak.quarkus.runtime.Environment.isRebuilt;
 import static org.keycloak.quarkus.runtime.cli.command.AbstractStartCommand.*;
@@ -27,6 +29,8 @@ import static org.keycloak.quarkus.runtime.configuration.Configuration.OPTION_PA
 import static org.keycloak.quarkus.runtime.configuration.Configuration.getBuildTimeProperty;
 import static org.keycloak.quarkus.runtime.configuration.Configuration.getConfig;
 import static org.keycloak.quarkus.runtime.Environment.isDevMode;
+import static org.keycloak.quarkus.runtime.configuration.Configuration.getCurrentBuiltTimeProperty;
+import static org.keycloak.quarkus.runtime.configuration.Configuration.getRawPersistedProperty;
 import static org.keycloak.quarkus.runtime.configuration.Configuration.getRuntimeProperty;
 import static org.keycloak.quarkus.runtime.configuration.mappers.PropertyMappers.formatValue;
 import static org.keycloak.quarkus.runtime.configuration.mappers.PropertyMappers.isBuildTimeProperty;
@@ -42,6 +46,7 @@ import java.util.HashMap;
 import java.util.Iterator;
 import java.util.List;
 import java.util.Map;
+import java.util.Objects;
 import java.util.Optional;
 import java.util.Set;
 import java.util.function.BiConsumer;
@@ -59,6 +64,7 @@ import org.keycloak.quarkus.runtime.cli.command.Main;
 import org.keycloak.quarkus.runtime.cli.command.ShowConfig;
 import org.keycloak.quarkus.runtime.cli.command.StartDev;
 import org.keycloak.quarkus.runtime.configuration.ConfigArgsConfigSource;
+import org.keycloak.quarkus.runtime.configuration.Configuration;
 import org.keycloak.quarkus.runtime.configuration.PersistedConfigSource;
 import org.keycloak.quarkus.runtime.configuration.QuarkusPropertiesConfigSource;
 import org.keycloak.quarkus.runtime.configuration.mappers.PropertyMappers;
@@ -67,6 +73,7 @@ import org.keycloak.quarkus.runtime.Environment;
 
 import io.smallrye.config.ConfigValue;
 import picocli.CommandLine;
+import picocli.CommandLine.Help.Ansi;
 import picocli.CommandLine.Model.CommandSpec;
 import picocli.CommandLine.Model.OptionSpec;
 import picocli.CommandLine.Model.ArgGroupSpec;
@@ -170,6 +177,7 @@ public final class Picocli {
     private static int runReAugmentation(List<String> cliArgs, CommandLine cmd) {
         if(!isDevMode() && cmd != null) {
             cmd.getOut().println("Changes detected in configuration. Updating the server image.");
+            checkChangesInBuildOptionsDuringAutoBuild();
         }
 
         int exitCode;
@@ -221,7 +229,7 @@ public final class Picocli {
     }
 
     private static boolean hasConfigChanges(CommandLine cmdCommand) {
-        Optional<String> currentProfile = Optional.ofNullable(Environment.getProfile());
+        Optional<String> currentProfile = ofNullable(Environment.getProfile());
         Optional<String> persistedProfile = getBuildTimeProperty("kc.profile");
 
         if (!persistedProfile.orElse("").equals(currentProfile.orElse(""))) {
@@ -543,4 +551,49 @@ public final class Picocli {
 
         return arg.startsWith(ImportRealmMixin.IMPORT_REALM);
     }
+
+    private static void checkChangesInBuildOptionsDuringAutoBuild() {
+        if (Configuration.isOptimized()) {
+            List<PropertyMapper> buildOptions = stream(Configuration.getPropertyNames(true).spliterator(), false)
+                    .sorted()
+                    .map(PropertyMappers::getMapper)
+                    .filter(Objects::nonNull).collect(Collectors.toList());
+
+            if (buildOptions.isEmpty()) {
+                return;
+            }
+
+            StringBuilder options = new StringBuilder();
+
+            for (PropertyMapper mapper : buildOptions) {
+                String newValue = ofNullable(getCurrentBuiltTimeProperty(mapper.getFrom()))
+                        .map(ConfigValue::getValue)
+                        .orElse("<unset>");
+                String currentValue = getRawPersistedProperty(mapper.getFrom()).get();
+
+                if (newValue.equals(currentValue)) {
+                    continue;
+                }
+
+                String name = mapper.getOption().getKey();
+
+                options.append("\n\t- ")
+                    .append(name).append("=").append(currentValue)
+                    .append(" > ")
+                    .append(name).append("=").append(newValue);
+            }
+
+            if (options.length() > 0) {
+                System.out.println(
+                        Ansi.AUTO.string(
+                                new StringBuilder("@|bold,red ")
+                                        .append("The previous optimized build will be overridden with the following build options:")
+                                        .append(options)
+                                        .append("\nTo avoid that, run the 'build' command again and then start the optimized server instance using the '--optimized' flag.")
+                                        .append("|@").toString()
+                        )
+                );
+            }
+        }
+    }
 }

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

@@ -21,6 +21,7 @@ import static org.keycloak.quarkus.runtime.Environment.getProfileOrDefault;
 import static org.keycloak.quarkus.runtime.cli.Picocli.ARG_PREFIX;
 
 import java.util.Optional;
+import java.util.Properties;
 
 import io.smallrye.config.ConfigValue;
 import io.smallrye.config.SmallRyeConfig;
@@ -40,6 +41,7 @@ public final class Configuration {
 
     public static final char OPTION_PART_SEPARATOR_CHAR = '-';
     public static final String OPTION_PART_SEPARATOR = String.valueOf(OPTION_PART_SEPARATOR_CHAR);
+    private static final String KC_OPTIMIZED = NS_KEYCLOAK_PREFIX + "optimized";
 
     private Configuration() {
 
@@ -78,6 +80,14 @@ public final class Configuration {
     }
 
     public static Iterable<String> getPropertyNames() {
+        return getPropertyNames(false);
+    }
+
+    public static Iterable<String> getPropertyNames(boolean onlyPersisted) {
+        if (onlyPersisted) {
+            return PersistedConfigSource.getInstance().getPropertyNames();
+        }
+
         return getConfig().getPropertyNames();
     }
 
@@ -101,10 +111,6 @@ public final class Configuration {
         return getOptionalValue(name).map(Boolean::parseBoolean);
     }
 
-    public static Optional<Boolean> getOptionalKcBooleanValue(String name) {
-        return getOptionalBooleanValue(NS_KEYCLOAK_PREFIX.concat(name));
-    }
-
     public static String getMappedPropertyName(String key) {
         PropertyMapper mapper = PropertyMappers.getMapper(key);
 
@@ -188,4 +194,23 @@ public final class Configuration {
 
         return value;
     }
+
+    public static boolean isOptimized() {
+        return Configuration.getRawPersistedProperty(KC_OPTIMIZED).isPresent();
+    }
+
+    public static void markAsOptimized(Properties properties) {
+        properties.put(Configuration.KC_OPTIMIZED, Boolean.TRUE.toString());
+    }
+
+    public static ConfigValue getCurrentBuiltTimeProperty(String name) {
+        PersistedConfigSource persistedConfigSource = PersistedConfigSource.getInstance();
+
+        try {
+            persistedConfigSource.enable(false);
+            return getConfigValue(name);
+        } finally {
+            persistedConfigSource.enable(true);
+        }
+    }
 }

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

@@ -27,6 +27,8 @@ import java.util.Collections;
 import java.util.HashMap;
 import java.util.Map;
 import java.util.Properties;
+import java.util.Set;
+import java.util.function.Supplier;
 import java.util.zip.ZipEntry;
 import java.util.zip.ZipInputStream;
 
@@ -43,6 +45,14 @@ public final class PersistedConfigSource extends PropertiesConfigSource {
     public static final String PERSISTED_PROPERTIES = "META-INF/keycloak-persisted.properties";
     private static final PersistedConfigSource INSTANCE = new PersistedConfigSource();
 
+    /**
+     * MicroProfile Config does not allow removing a config source when resolving properties. In order to be able
+     * to resolve the current (not the persisted value) value for a property, even if not explicitly set at runtime, we need
+     * to ignore this config source. Otherwise, default values are not resolved at runtime because the property will be
+     * resolved from this config source, if persisted.
+     */
+    private static final ThreadLocal<Boolean> ENABLED = new ThreadLocal<>();
+
     private PersistedConfigSource() {
         super(readProperties(), "", 200);
     }
@@ -58,13 +68,26 @@ public final class PersistedConfigSource extends PropertiesConfigSource {
 
     @Override
     public String getValue(String propertyName) {
-        String value = super.getValue(propertyName);
+        if (isEnabled()) {
+            String value = super.getValue(propertyName);
 
-        if (value != null) {
+            if (value != null) {
-            return value;
+                return value;
+            }
+
+            return super.getValue(propertyName.replace(Configuration.OPTION_PART_SEPARATOR_CHAR, '.'));
         }
 
-        return super.getValue(propertyName.replace(Configuration.OPTION_PART_SEPARATOR_CHAR, '.'));
+        return null;
+    }
+
+    @Override
+    public Set<String> getPropertyNames() {
+        if (isEnabled()) {
+            return super.getPropertyNames();
+        }
+
+        return Set.of();
     }
 
     private static Map<String, String> readProperties() {
@@ -122,4 +145,17 @@ public final class PersistedConfigSource extends PropertiesConfigSource {
 
         return null;
     }
+
+    public void enable(boolean enabled) {
+        if (enabled) {
+            ENABLED.remove();
+        } else {
+            ENABLED.set(enabled);
+        }
+    }
+
+    private boolean isEnabled() {
+        Boolean result = ENABLED.get();
+        return result == null ? true : result;
+    }
 }

quarkus/runtime/src/main/resources/META-INF/keycloak.conf

@@ -1,13 +1,6 @@
 # Default and non-production grade database vendor
 db=dev-file
 
-# Insecure requests are disabled by default
-http-enabled=false
-
-# Metrics and healthcheck are disabled by default
-health-enabled=false
-metrics-enabled=false
-
 # Default, and insecure, and non-production grade configuration for the development profile
 %dev.http-enabled=true
 %dev.hostname-strict=false

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

@@ -35,13 +35,13 @@ import org.keycloak.it.utils.KeycloakDistribution;
 import io.quarkus.test.junit.main.Launch;
 import io.quarkus.test.junit.main.LaunchResult;
 
-@DistributionTest
+@DistributionTest(defaultOptions = {"--http-enabled=true", "--hostname-strict=false"})
 @RawDistOnly(reason = "Containers are immutable")
 @TestMethodOrder(OrderAnnotation.class)
 public class QuarkusPropertiesAutoBuildDistTest {
 
     @Test
-    @Launch({ "start", "--http-enabled=true", "--hostname-strict=false" })
+    @Launch({ "start" })
     @Order(1)
     void reAugOnFirstRun(LaunchResult result) {
         CLIResult cliResult = (CLIResult) result;
@@ -50,7 +50,7 @@ public class QuarkusPropertiesAutoBuildDistTest {
 
     @Test
     @BeforeStartDistribution(QuarkusPropertiesAutoBuildDistTest.UpdateConsoleLogLevelToWarn.class)
-    @Launch({ "start", "--http-enabled=true", "--hostname-strict=false" })
+    @Launch({ "start" })
     @Order(2)
     void testQuarkusRuntimePropDoesNotTriggerReAug(LaunchResult result) {
         CLIResult cliResult = (CLIResult) result;
@@ -60,7 +60,7 @@ public class QuarkusPropertiesAutoBuildDistTest {
 
     @Test
     @BeforeStartDistribution(UpdateConsoleLogLevelToInfo.class)
-    @Launch({ "start", "--http-enabled=true", "--hostname-strict=false" })
+    @Launch({ "start" })
     @Order(3)
     void testNoReAugAfterChangingRuntimeProperty(LaunchResult result) {
         CLIResult cliResult = (CLIResult) result;
@@ -70,7 +70,7 @@ public class QuarkusPropertiesAutoBuildDistTest {
 
     @Test
     @BeforeStartDistribution(AddAdditionalDatasource.class)
-    @Launch({ "start", "--http-enabled=true", "--hostname-strict=false" })
+    @Launch({ "start" })
     @Order(4)
     void testReAugForAdditionalDatasource(LaunchResult result) {
         CLIResult cliResult = (CLIResult) result;
@@ -79,7 +79,7 @@ public class QuarkusPropertiesAutoBuildDistTest {
 
     @Test
     @BeforeStartDistribution(ChangeAdditionalDatasourceUsername.class)
-    @Launch({ "start", "--http-enabled=true", "--hostname-strict=false" })
+    @Launch({ "start" })
     @Order(5)
     void testNoReAugForAdditionalDatasourceRuntimeProperty(LaunchResult result) {
         CLIResult cliResult = (CLIResult) result;
@@ -88,7 +88,7 @@ public class QuarkusPropertiesAutoBuildDistTest {
 
     @Test
     @BeforeStartDistribution(ChangeAdditionalDatasourceDbKind.class)
-    @Launch({ "start", "--http-enabled=true", "--hostname-strict=false" })
+    @Launch({ "start" })
     @Order(6)
     void testNoReAugWhenBuildTimePropertiesAreTheSame(LaunchResult result) {
         CLIResult cliResult = (CLIResult) result;
@@ -97,7 +97,7 @@ public class QuarkusPropertiesAutoBuildDistTest {
 
     @Test
     @BeforeStartDistribution(AddAdditionalDatasource2.class)
-    @Launch({ "start", "--http-enabled=true", "--hostname-strict=false" })
+    @Launch({ "start" })
     @Order(7)
     void testReAugWhenAnotherDatasourceAdded(LaunchResult result) {
         CLIResult cliResult = (CLIResult) result;
@@ -105,14 +105,13 @@ public class QuarkusPropertiesAutoBuildDistTest {
     }
 
     @Test
-    @BeforeStartDistribution(EnableQuarkusMetrics.class)
+    @BeforeStartDistribution(SetDatabaseKind.class)
-    @Launch({ "start", "--http-enabled=true", "--hostname-strict=false" })
+    @Launch({ "start" })
     @Order(8)
     void testWrappedBuildPropertyTriggersBuildButGetsIgnoredWhenSetByQuarkus(LaunchResult result) {
         CLIResult cliResult = (CLIResult) result;
         cliResult.assertBuild();
-        when().get("/metrics").then()
+        cliResult.assertStarted();
-                .statusCode(404);
     }
 
     public static class UpdateConsoleLogLevelToWarn implements Consumer<KeycloakDistribution> {
@@ -163,11 +162,11 @@ public class QuarkusPropertiesAutoBuildDistTest {
         }
     }
 
-    public static class EnableQuarkusMetrics implements Consumer<KeycloakDistribution> {
+    public static class SetDatabaseKind implements Consumer<KeycloakDistribution> {
         @Override
         public void accept(KeycloakDistribution distribution) {
             distribution.setManualStop(true);
-            distribution.setQuarkusProperty("quarkus.micrometer.enabled","true");
+            distribution.setQuarkusProperty("quarkus.datasource.db-kind", "postgres");
         }
     }
 }

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

@@ -102,16 +102,6 @@ public class QuarkusPropertiesDistTest {
         cliResult.assertNoBuild();
     }
 
-    @Test
-    @BeforeStartDistribution(UpdateHibernateMetricsFromQuarkusProps.class)
-    @Launch({ "start", "--http-enabled=true", "--hostname-strict=false" })
-    @Order(7)
-    void testBuildRunTimeMismatchOnQuarkusBuildPropWarning(LaunchResult result) {
-        CLIResult cliResult = (CLIResult) result;
-        cliResult.assertNoBuild();
-        cliResult.assertBuildRuntimeMismatchWarning(QUARKUS_BUILDTIME_HIBERNATE_METRICS_KEY);
-    }
-
     @Test
     @BeforeStartDistribution(UpdateHibernateMetricsFromQuarkusProps.class)
     @Launch({ "build", "--metrics-enabled=true" })

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

@@ -14,18 +14,18 @@ public class ShowConfigCommandDistTest {
     void testShowConfigPicksUpRightConfigDependingOnCurrentMode(KeycloakDistribution distribution) {
         CLIResult initialResult = distribution.run("show-config");
         initialResult.assertMessage("Current Mode: production");
-        initialResult.assertMessage("kc.http-enabled =  false");
+        initialResult.assertMessage("kc.db =  dev-file");
 
         distribution.run("start-dev");
 
         CLIResult devModeResult = distribution.run("show-config");
         devModeResult.assertMessage("Current Mode: development");
-        devModeResult.assertMessage("kc.http-enabled =  true");
+        devModeResult.assertMessage("kc.db =  dev-file");
 
         distribution.run("build");
 
         CLIResult resetResult = distribution.run("show-config");
         resetResult.assertMessage("Current Mode: production");
-        resetResult.assertMessage("kc.http-enabled =  false");
+        resetResult.assertMessage("kc.db =  dev-file");
     }
 }

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

@@ -30,6 +30,7 @@ import org.keycloak.it.junit5.extension.DistributionTest;
 
 import io.quarkus.test.junit.main.Launch;
 import io.quarkus.test.junit.main.LaunchResult;
+import org.keycloak.it.junit5.extension.RawDistOnly;
 import org.keycloak.it.utils.KeycloakDistribution;
 
 @DistributionTest
@@ -99,4 +100,40 @@ public class StartCommandDistTest {
         cliResult.assertError("Unknown option: '--cache'");
     }
 
+    @Test
+    @RawDistOnly(reason = "Containers are immutable")
+    void testWarningWhenOverridingBuildOptionsDuringStart(KeycloakDistribution dist) {
+        CLIResult cliResult = dist.run("build", "--db=postgres", "--cache=local", "--features=preview");
+        cliResult.assertBuild();
+        cliResult = dist.run("start", "--hostname=localhost", "--http-enabled=true");
+        cliResult.assertMessage("The previous optimized build will be overridden with the following build options:");
+        cliResult.assertMessage("- cache=local > cache=ispn"); // back to the default value
+        cliResult.assertMessage("- db=postgres > db=dev-file"); // back to the default value
+        cliResult.assertMessage("- features=preview > features=<unset>"); // no default value, the <unset> is shown
+        cliResult.assertMessage("To avoid that, run the 'build' command again and then start the optimized server instance using the '--optimized' flag.");
+        cliResult.assertStarted();
+        // should not show warning if the re-augmentation did not happen through the build command
+        // an optimized server image should ideally be created by running a build
+        cliResult = dist.run("start", "--db=dev-mem", "--hostname=localhost", "--http-enabled=true");
+        cliResult.assertNoMessage("The previous optimized build will be overridden with the following build options:");
+        cliResult.assertStarted();
+        dist.run("build", "--db=postgres");
+        cliResult = dist.run("start", "--hostname=localhost", "--http-enabled=true");
+        cliResult.assertMessage("- db=postgres > db=dev-file");
+        cliResult.assertNoMessage("- cache=local > cache=ispn");
+        cliResult.assertNoMessage("- features=preview > features=<unset>");
+        cliResult.assertStarted();
+        dist.run("build", "--db=postgres");
+        cliResult = dist.run("start", "--db=dev-mem", "--hostname=localhost", "--http-enabled=true");
+        cliResult.assertMessage("- db=postgres > db=dev-mem"); // option overridden during the start
+        cliResult.assertStarted();
+        dist.run("build", "--db=dev-mem", "--cache=local");
+        cliResult = dist.run("start", "--db=dev-mem", "--hostname=localhost", "--http-enabled=true");
+        cliResult.assertNoMessage("- db=postgres > db=postgres"); // option did not change not need to show
+        cliResult.assertMessage("- cache=local > cache=ispn");
+        cliResult.assertStarted();
+        dist.run("build", "--db=dev-mem", "--cache=local");
+        cliResult = dist.run("start", "--db=dev-mem", "--cache=local", "--hostname=localhost", "--http-enabled=true");
+        cliResult.assertNoMessage("The previous optimized build will be overridden with the following build options:"); // no message, same values provided during auto-build
+    }
 }