Persisted config source not loading properties at runtime (#12157)

Co-authored-by: Dominik Guhr <dguhr@redhat.com>

.github/workflows/ci.yml

@@ -489,7 +489,7 @@ jobs:
 
       - name: Run Quarkus Storage Tests
         run: |
-          ./mvnw clean install -nsu -B -f quarkus/tests/pom.xml -Ptest-database -Dtest=PostgreSQLStartDatabaseTest | misc/log/trimmer.sh
+          ./mvnw clean install -nsu -B -f quarkus/tests/pom.xml -Ptest-database -Dtest=PostgreSQLDistTest | misc/log/trimmer.sh
           TEST_RESULT=${PIPESTATUS[0]}
           find . -path '*/target/surefire-reports/*.xml' | zip -q reports-quarkus-tests.zip -@
           exit $TEST_RESULT

quarkus/runtime/src/main/java/org/keycloak/quarkus/runtime/Environment.java

@@ -31,7 +31,6 @@ import java.util.Optional;
 import java.util.function.Function;
 import java.util.stream.Collectors;
 
-import io.quarkus.bootstrap.runner.RunnerClassLoader;
 import io.quarkus.runtime.LaunchMode;
 import io.quarkus.runtime.configuration.ProfileManager;
 import org.apache.commons.lang3.SystemUtils;
@@ -49,11 +48,11 @@ public final class Environment {
     private Environment() {}
 
     public static Boolean isRebuild() {
-        return !isRuntimeMode();
+        return Boolean.getBoolean("quarkus.launch.rebuild");
     }
 
     public static Boolean isRuntimeMode() {
-        return Thread.currentThread().getContextClassLoader() instanceof RunnerClassLoader;
+        return !isRebuild();
     }
 
     public static String getHomeDir() {

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

@@ -68,7 +68,7 @@ public final class PersistedConfigSource extends PropertiesConfigSource {
     }
 
     private static Map<String, String> readProperties() {
-        if (!Environment.isRebuild()) {
+        if (Environment.isRuntimeMode()) {
             InputStream fileStream = loadPersistedConfig();
 
             if (fileStream == null) {

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

@@ -21,7 +21,6 @@ metrics-enabled=false
 %import_export.http-enabled=true
 %import_export.hostname-strict=false
 %import_export.hostname-strict-https=false
-%import_export.cluster=local
 
 #logging defaults
 log-console-output=default

quarkus/tests/integration/pom.xml

@@ -79,6 +79,14 @@
             <artifactId>approvaltests</artifactId>
             <version>${approvaltests.version}</version>
         </dependency>
+        <dependency>
+            <groupId>org.testcontainers</groupId>
+            <artifactId>postgresql</artifactId>
+        </dependency>
+        <dependency>
+            <groupId>org.testcontainers</groupId>
+            <artifactId>mariadb</artifactId>
+        </dependency>
     </dependencies>
 
     <build>

quarkus/tests/integration/src/main/java/org/keycloak/it/junit5/extension/CLITestExtension.java

@@ -54,6 +54,7 @@ public class CLITestExtension extends QuarkusMainTestExtension {
     private static final String KEY_VALUE_SEPARATOR = "[= ]";
     private KeycloakDistribution dist;
     private final Set<String> testSysProps = new HashSet<>();
+    private DatabaseContainer databaseContainer;
 
     @Override
     public void beforeEach(ExtensionContext context) throws Exception {
@@ -81,6 +82,8 @@ public class CLITestExtension extends QuarkusMainTestExtension {
             }
         }
 
+        configureDatabase(context);
+
         if (distConfig != null) {
             onKeepServerAlive(context.getRequiredTestMethod().getAnnotation(KeepServerAlive.class));
 
@@ -96,7 +99,6 @@ public class CLITestExtension extends QuarkusMainTestExtension {
             }
         } else {
             configureProfile(context);
-            configureDatabase(context);
             super.beforeEach(context);
         }
     }
@@ -148,6 +150,10 @@ public class CLITestExtension extends QuarkusMainTestExtension {
         for (String property : testSysProps) {
             System.getProperties().remove(property);
         }
+        if (databaseContainer != null && databaseContainer.isRunning()) {
+            databaseContainer.stop();
+            databaseContainer = null;
+        }
     }
 
     @Override
@@ -237,10 +243,22 @@ public class CLITestExtension extends QuarkusMainTestExtension {
         WithDatabase database = context.getTestClass().orElse(Object.class).getDeclaredAnnotation(WithDatabase.class);
 
         if (database != null) {
-            configureDevServices();
+            if (dist == null) {
-            setProperty("kc.db", database.alias());
+                configureDevServices();
-            // databases like mssql are very strict about password policy
+                setProperty("kc.db", database.alias());
-            setProperty("kc.db-password", "Password1!");
+                setProperty("kc.db-password", DatabaseContainer.DEFAULT_PASSWORD);
+            } else {
+                databaseContainer = new DatabaseContainer(database.alias());
+
+                databaseContainer.start();
+
+                dist.setProperty("db", database.alias());
+                dist.setProperty("db-username", databaseContainer.getUsername());
+                dist.setProperty("db-password", databaseContainer.getPassword());
+                dist.setProperty("db-url", databaseContainer.getJdbcUrl());
+
+                dist.start(List.of("build"));
+            }
         } else {
             // This is for re-creating the H2 database instead of using the default in home
             setProperty("kc.db-url-path", new QuarkusPlatform().getTmpDirectory().getAbsolutePath());

quarkus/tests/integration/src/main/java/org/keycloak/it/junit5/extension/DatabaseContainer.java

@@ -0,0 +1,76 @@
+/*
+ * Copyright 2021 Red Hat, Inc. and/or its affiliates
+ * and other contributors as indicated by the @author tags.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.keycloak.it.junit5.extension;
+
+import java.time.Duration;
+import org.testcontainers.containers.JdbcDatabaseContainer;
+import org.testcontainers.containers.MariaDBContainer;
+import org.testcontainers.containers.PostgreSQLContainer;
+
+public class DatabaseContainer {
+
+    static final String DEFAULT_PASSWORD = "Password1!";
+
+    private final String alias;
+    private JdbcDatabaseContainer container;
+
+    DatabaseContainer(String alias) {
+        this.alias = alias;
+    }
+
+    void start() {
+        container = createContainer()
+                .withDatabaseName("keycloak")
+                .withUsername(getUsername())
+                .withPassword(getPassword());
+
+        container.withStartupTimeout(Duration.ofMinutes(5)).start();
+    }
+
+    boolean isRunning() {
+        return container.isRunning();
+    }
+
+    String getJdbcUrl() {
+        return container.getJdbcUrl();
+    }
+
+    String getUsername() {
+        return "keycloak";
+    }
+
+    String getPassword() {
+        return DEFAULT_PASSWORD;
+    }
+
+    void stop() {
+        container.stop();
+        container = null;
+    }
+
+    private JdbcDatabaseContainer createContainer() {
+        switch (alias) {
+            case "postgres":
+                return new PostgreSQLContainer("postgres:alpine");
+            case "mariadb":
+                return new MariaDBContainer("mariadb:10.5.9");
+            default:
+                throw new RuntimeException("Unsupported database: " + alias);
+        }
+    }
+}

quarkus/tests/integration/src/main/java/org/keycloak/it/junit5/extension/DistributionTest.java

@@ -38,5 +38,10 @@ public @interface DistributionTest {
     }
 
     ReInstall reInstall() default ReInstall.BEFORE_ALL;
+
+    /**
+     * If any build option must be unset after the running the build command.
+     */
+    boolean removeBuildOptionsAfterBuild() default false;
 }
 

quarkus/tests/integration/src/main/java/org/keycloak/it/junit5/extension/DistributionType.java

@@ -40,7 +40,8 @@ public enum DistributionType {
         return new RawKeycloakDistribution(
                 config.debug(),
                 config.keepAlive(),
-                !DistributionTest.ReInstall.NEVER.equals(config.reInstall()));
+                !DistributionTest.ReInstall.NEVER.equals(config.reInstall()),
+                config.removeBuildOptionsAfterBuild());
     }
 
     private final Function<DistributionTest, KeycloakDistribution> factory;

quarkus/tests/integration/src/main/java/org/keycloak/it/junit5/extension/WithDatabase.java

@@ -33,8 +33,6 @@ public @interface WithDatabase {
 
     /**
      * The database name as per database aliases.
-     *
-     * @return the database alias
      */
     String alias();
 }

quarkus/tests/integration/src/main/java/org/keycloak/it/utils/KeycloakDistribution.java

@@ -46,4 +46,8 @@ public interface KeycloakDistribution {
     default void copyOrReplaceFileFromClasspath(String file, Path distDir) {
         throw new RuntimeException("Not implemented");
     }
+
+    default void removeProperty(String db) {
+        throw new RuntimeException("Not implemented");
+    }
 }

quarkus/tests/integration/src/main/java/org/keycloak/it/utils/RawKeycloakDistribution.java

@@ -39,6 +39,7 @@ import java.util.Comparator;
 import java.util.List;
 import java.util.Properties;
 import java.util.concurrent.*;
+import java.util.function.Consumer;
 import java.util.stream.Collectors;
 import java.util.stream.Stream;
 import javax.net.ssl.HostnameVerifier;
@@ -54,6 +55,9 @@ import org.apache.commons.io.FileUtils;
 
 import org.keycloak.common.Version;
 import org.keycloak.quarkus.runtime.Environment;
+import org.keycloak.quarkus.runtime.cli.command.Build;
+import org.keycloak.quarkus.runtime.configuration.mappers.PropertyMapper;
+import org.keycloak.quarkus.runtime.configuration.mappers.PropertyMappers;
 
 import static org.keycloak.quarkus.runtime.Environment.LAUNCH_MODE;
 
@@ -69,13 +73,15 @@ public final class RawKeycloakDistribution implements KeycloakDistribution {
     private int httpPort;
     private boolean debug;
     private boolean reCreate;
+    private boolean removeBuildOptionsAfterBuild;
     private ExecutorService outputExecutor;
     private boolean inited = false;
 
-    public RawKeycloakDistribution(boolean debug, boolean manualStop, boolean reCreate) {
+    public RawKeycloakDistribution(boolean debug, boolean manualStop, boolean reCreate, boolean removeBuildOptionsAfterBuild) {
         this.debug = debug;
         this.manualStop = manualStop;
         this.reCreate = reCreate;
+        this.removeBuildOptionsAfterBuild = removeBuildOptionsAfterBuild;
         this.distPath = prepareDistribution();
     }
 
@@ -98,6 +104,11 @@ public final class RawKeycloakDistribution implements KeycloakDistribution {
             stop();
             throw new RuntimeException("Failed to start the server", cause);
         } finally {
+            if (arguments.contains(Build.NAME) && removeBuildOptionsAfterBuild) {
+                for (PropertyMapper mapper : PropertyMappers.getBuildTimeMappers()) {
+                    removeProperty(mapper.getFrom().substring(3));
+                }
+            }
             if (!manualStop) {
                 stop();
             }
@@ -108,7 +119,6 @@ public final class RawKeycloakDistribution implements KeycloakDistribution {
     public void stop() {
         if (isRunning()) {
             try {
-
                 if (Environment.isWindows()) {
                     // On Windows, we're executing kc.bat in a runtime as "keycloak",
                     // so tha java process is an actual child process
@@ -409,12 +419,27 @@ public final class RawKeycloakDistribution implements KeycloakDistribution {
 
     @Override
     public void setProperty(String key, String value) {
-        setProperty(key, value, distPath.resolve("conf").resolve("keycloak.conf").toFile());
+        updateProperties(properties -> properties.put(key, value), distPath.resolve("conf").resolve("keycloak.conf").toFile());
+    }
+
+    @Override
+    public void removeProperty(String key) {
+        updateProperties(new Consumer<Properties>() {
+            @Override
+            public void accept(Properties properties) {
+                properties.remove(key);
+            }
+        }, distPath.resolve("conf").resolve("keycloak.conf").toFile());
     }
 
     @Override
     public void setQuarkusProperty(String key, String value) {
-        setProperty(key, value, getQuarkusPropertiesFile());
+        updateProperties(new Consumer<Properties>() {
+            @Override
+            public void accept(Properties properties) {
+                properties.put(key, value);
+            }
+        }, getQuarkusPropertiesFile());
     }
 
     @Override
@@ -439,27 +464,27 @@ public final class RawKeycloakDistribution implements KeycloakDistribution {
         }
     }
 
-    private void setProperty(String key, String value, File confFile) {
+    private void updateProperties(Consumer<Properties> propertiesConsumer, File propertiesFile) {
         Properties properties = new Properties();
 
-        if (confFile.exists()) {
+        if (propertiesFile.exists()) {
             try (
-                FileInputStream in = new FileInputStream(confFile);
+                FileInputStream in = new FileInputStream(propertiesFile);
             ) {
 
                 properties.load(in);
             } catch (Exception e) {
-                throw new RuntimeException("Failed to update " + confFile, e);
+                throw new RuntimeException("Failed to update " + propertiesFile, e);
             }
         }
 
         try (
-            FileOutputStream out = new FileOutputStream(confFile)
+            FileOutputStream out = new FileOutputStream(propertiesFile)
         ) {
-            properties.put(key, value);
+            propertiesConsumer.accept(properties);
             properties.store(out, "");
         } catch (Exception e) {
-            throw new RuntimeException("Failed to update " + confFile, e);
+            throw new RuntimeException("Failed to update " + propertiesFile, e);
         }
     }
 

quarkus/tests/integration/src/test/java/org/keycloak/it/storage/database/BasicDatabaseTest.java

@@ -17,23 +17,27 @@
 
 package org.keycloak.it.storage.database;
 
+import org.junit.jupiter.api.MethodOrderer;
+import org.junit.jupiter.api.Order;
 import org.junit.jupiter.api.Test;
+import org.junit.jupiter.api.TestMethodOrder;
 import org.keycloak.it.junit5.extension.CLIResult;
 
 import io.quarkus.test.junit.main.Launch;
 import io.quarkus.test.junit.main.LaunchResult;
 
-public abstract class AbstractStartDabataseTest {
+@TestMethodOrder(MethodOrderer.OrderAnnotation.class)
+public abstract class BasicDatabaseTest {
 
     @Test
-    @Launch({ "start-dev" })
+    @Launch({ "start", "--http-enabled=true", "--hostname-strict=false" })
     void testSuccessful(LaunchResult result) {
         CLIResult cliResult = (CLIResult) result;
-        cliResult.assertStartedDevMode();
+        cliResult.assertStarted();
     }
 
     @Test
-    @Launch({ "start-dev", "--db-username=wrong" })
+    @Launch({ "start", "--http-enabled=true", "--hostname-strict=false", "--db-username=wrong" })
     void testWrongUsername(LaunchResult result) {
         CLIResult cliResult = (CLIResult) result;
         cliResult.assertMessage("ERROR: Failed to obtain JDBC connection");
@@ -43,7 +47,7 @@ public abstract class AbstractStartDabataseTest {
     protected abstract void assertWrongUsername(CLIResult cliResult);
 
     @Test
-    @Launch({ "start-dev", "--db-password=wrong" })
+    @Launch({ "start", "--http-enabled=true", "--hostname-strict=false", "--db-password=wrong" })
     void testWrongPassword(LaunchResult result) {
         CLIResult cliResult = (CLIResult) result;
         cliResult.assertMessage("ERROR: Failed to obtain JDBC connection");
@@ -51,4 +55,23 @@ public abstract class AbstractStartDabataseTest {
     }
 
     protected abstract void assertWrongPassword(CLIResult cliResult);
+
+    @Order(1)
+    @Test
+    @Launch({ "export", "--dir=./target/export"})
+    public void testExportSucceeds(LaunchResult result) {
+        CLIResult cliResult = (CLIResult) result;
+        cliResult.assertMessage("Full model export requested");
+        cliResult.assertMessage("Export finished successfully");
+    }
+
+    @Order(2)
+    @Test
+    @Launch({ "import", "--dir=./target/export" })
+    void testImportSucceeds(LaunchResult result) {
+        CLIResult cliResult = (CLIResult) result;
+        cliResult.assertMessage("target/export");
+        cliResult.assertMessage("Realm 'master' imported");
+        cliResult.assertMessage("Import finished successfully");
+    }
 }

quarkus/tests/integration/src/test/java/org/keycloak/it/storage/database/MSSQLTest.java

@@ -27,7 +27,7 @@ import io.quarkus.test.junit.main.LaunchResult;
 
 @CLITest
 @WithDatabase(alias = "mssql")
-public class MSSQLStartDatabaseTest extends AbstractStartDabataseTest {
+public class MSSQLTest extends BasicDatabaseTest {
 
     /**
      * It should be possible to enable XA but here we reproduce a managed environment where only nonXA transaction is supported

quarkus/tests/integration/src/test/java/org/keycloak/it/storage/database/MariaDBTest.java

@@ -23,7 +23,7 @@ import org.keycloak.it.junit5.extension.WithDatabase;
 
 @CLITest
 @WithDatabase(alias = "mariadb")
-public class MariaDBStartDatabaseTest extends AbstractStartDabataseTest {
+public class MariaDBTest extends BasicDatabaseTest {
 
     @Override
     protected void assertWrongPassword(CLIResult cliResult) {

quarkus/tests/integration/src/test/java/org/keycloak/it/storage/database/MySQLTest.java

@@ -17,17 +17,13 @@
 
 package org.keycloak.it.storage.database;
 
-import org.junit.jupiter.api.Test;
 import org.keycloak.it.junit5.extension.CLIResult;
 import org.keycloak.it.junit5.extension.CLITest;
 import org.keycloak.it.junit5.extension.WithDatabase;
 
-import io.quarkus.test.junit.main.Launch;
-import io.quarkus.test.junit.main.LaunchResult;
-
 @CLITest
 @WithDatabase(alias = "mysql")
-public class MySQLStartDatabaseTest extends AbstractStartDabataseTest {
+public class MySQLTest extends BasicDatabaseTest {
 
     @Override
     protected void assertWrongUsername(CLIResult cliResult) {

quarkus/tests/integration/src/test/java/org/keycloak/it/storage/database/OracleTest.java

@@ -17,17 +17,13 @@
 
 package org.keycloak.it.storage.database;
 
-import org.junit.jupiter.api.Test;
 import org.keycloak.it.junit5.extension.CLIResult;
 import org.keycloak.it.junit5.extension.CLITest;
 import org.keycloak.it.junit5.extension.WithDatabase;
 
-import io.quarkus.test.junit.main.Launch;
-import io.quarkus.test.junit.main.LaunchResult;
-
 @CLITest
 @WithDatabase(alias = "oracle")
-public class OracleStartDatabaseTest extends AbstractStartDabataseTest {
+public class OracleTest extends BasicDatabaseTest {
 
     @Override
     protected void assertWrongUsername(CLIResult cliResult) {

quarkus/tests/integration/src/test/java/org/keycloak/it/storage/database/PostgreSQLTest.java

@@ -17,17 +17,13 @@
 
 package org.keycloak.it.storage.database;
 
-import org.junit.jupiter.api.Test;
 import org.keycloak.it.junit5.extension.CLIResult;
 import org.keycloak.it.junit5.extension.CLITest;
 import org.keycloak.it.junit5.extension.WithDatabase;
 
-import io.quarkus.test.junit.main.Launch;
-import io.quarkus.test.junit.main.LaunchResult;
-
 @CLITest
 @WithDatabase(alias = "postgres")
-public class PostgreSQLStartDatabaseTest extends AbstractStartDabataseTest {
+public class PostgreSQLTest extends BasicDatabaseTest {
 
     @Override
     protected void assertWrongUsername(CLIResult cliResult) {

quarkus/tests/integration/src/test/java/org/keycloak/it/storage/database/dist/MariaDBDistTest.java

@@ -0,0 +1,38 @@
+/*
+ * Copyright 2021 Red Hat, Inc. and/or its affiliates
+ * and other contributors as indicated by the @author tags.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.keycloak.it.storage.database.dist;
+
+import org.keycloak.it.junit5.extension.CLIResult;
+import org.keycloak.it.junit5.extension.CLITest;
+import org.keycloak.it.junit5.extension.WithDatabase;
+import org.keycloak.it.storage.database.BasicDatabaseTest;
+
+@CLITest
+@WithDatabase(alias = "mariadb")
+public class MariaDBDistTest extends BasicDatabaseTest {
+
+    @Override
+    protected void assertWrongPassword(CLIResult cliResult) {
+        cliResult.assertMessage("Access denied for user");
+    }
+
+    @Override
+    protected void assertWrongUsername(CLIResult cliResult) {
+        cliResult.assertMessage("Access denied for user 'wrong'");
+    }
+}

quarkus/tests/integration/src/test/java/org/keycloak/it/storage/database/dist/PostgreSQLDistTest.java

@@ -0,0 +1,42 @@
+/*
+ * Copyright 2021 Red Hat, Inc. and/or its affiliates
+ * and other contributors as indicated by the @author tags.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.keycloak.it.storage.database.dist;
+
+import static org.hamcrest.CoreMatchers.containsString;
+import static org.hamcrest.MatcherAssert.assertThat;
+
+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.WithDatabase;
+import org.keycloak.it.storage.database.PostgreSQLTest;
+
+import io.quarkus.test.junit.main.Launch;
+import io.quarkus.test.junit.main.LaunchResult;
+
+@DistributionTest(removeBuildOptionsAfterBuild = true)
+@WithDatabase(alias = "postgres")
+public class PostgreSQLDistTest extends PostgreSQLTest {
+
+    @Test
+    @Launch("show-config")
+    public void testDbOptionFromPersistedConfigSource(LaunchResult result) {
+        CLIResult cliResult = (CLIResult) result;
+        assertThat(cliResult.getOutput(),containsString("postgres (PersistedConfigSource)"));
+    }
+}