From 5e5cfff4e2f34b9a46b05085fff557d13ced1b67 Mon Sep 17 00:00:00 2001 From: Pedro Igor Date: Wed, 8 Jun 2022 10:31:07 -0300 Subject: [PATCH] Resolve default values for runtime options when running re-aug Closes #10818 --- .../configuration/mappers/PropertyMapper.java | 10 ++++++ .../it/junit5/extension/CLIResult.java | 2 +- .../it/junit5/extension/CLITestExtension.java | 33 ++++++++++--------- .../it/junit5/extension/RawDistOnly.java | 2 +- .../it/utils/DockerKeycloakDistribution.java | 5 ++- .../it/utils/KeycloakDistribution.java | 8 +++-- .../it/utils/RawKeycloakDistribution.java | 9 +++-- .../it/cli/dist/BuildCommandDistTest.java | 13 ++++++++ 8 files changed, 59 insertions(+), 23 deletions(-) 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 606af17fd7..cfe16de79b 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 @@ -16,6 +16,7 @@ */ package org.keycloak.quarkus.runtime.configuration.mappers; +import static org.keycloak.quarkus.runtime.Environment.isRebuild; import static org.keycloak.quarkus.runtime.configuration.Configuration.OPTION_PART_SEPARATOR; import static org.keycloak.quarkus.runtime.configuration.Configuration.OPTION_PART_SEPARATOR_CHAR; import static org.keycloak.quarkus.runtime.configuration.Configuration.toCliFormat; @@ -109,6 +110,11 @@ public class PropertyMapper { from = name.replace(to.substring(0, to.lastIndexOf('.')), from.substring(0, from.lastIndexOf(OPTION_PART_SEPARATOR_CHAR))); } + if (isRebuild() && isRunTime() && name.startsWith(MicroProfileConfigProvider.NS_KEYCLOAK_PREFIX)) { + // during re-aug do not resolve the server runtime properties and avoid they included by quarkus in the default value config source + return ConfigValue.builder().withName(name).build(); + } + // try to obtain the value for the property we want to map first ConfigValue config = context.proceed(from); @@ -186,6 +192,10 @@ public class PropertyMapper { return this.option.isBuildTime(); } + public boolean isRunTime() { + return !this.option.isBuildTime(); + } + public String getTo() { return to; } diff --git a/quarkus/tests/integration/src/main/java/org/keycloak/it/junit5/extension/CLIResult.java b/quarkus/tests/integration/src/main/java/org/keycloak/it/junit5/extension/CLIResult.java index 5d2f7a228d..3230f95400 100644 --- a/quarkus/tests/integration/src/main/java/org/keycloak/it/junit5/extension/CLIResult.java +++ b/quarkus/tests/integration/src/main/java/org/keycloak/it/junit5/extension/CLIResult.java @@ -35,7 +35,7 @@ import org.keycloak.it.junit5.extension.approvalTests.KcNamerFactory; public interface CLIResult extends LaunchResult { - static Object create(List outputStream, List errStream, int exitCode) { + static CLIResult create(List outputStream, List errStream, int exitCode) { return new CLIResult() { @Override public List getOutputStream() { diff --git a/quarkus/tests/integration/src/main/java/org/keycloak/it/junit5/extension/CLITestExtension.java b/quarkus/tests/integration/src/main/java/org/keycloak/it/junit5/extension/CLITestExtension.java index 6c87f5c936..ee75431a35 100644 --- a/quarkus/tests/integration/src/main/java/org/keycloak/it/junit5/extension/CLITestExtension.java +++ b/quarkus/tests/integration/src/main/java/org/keycloak/it/junit5/extension/CLITestExtension.java @@ -55,6 +55,7 @@ public class CLITestExtension extends QuarkusMainTestExtension { private KeycloakDistribution dist; private final Set testSysProps = new HashSet<>(); private DatabaseContainer databaseContainer; + private CLIResult result; @Override public void beforeEach(ExtensionContext context) throws Exception { @@ -95,7 +96,7 @@ public class CLITestExtension extends QuarkusMainTestExtension { onBeforeStartDistribution(context.getRequiredTestClass().getAnnotation(BeforeStartDistribution.class)); onBeforeStartDistribution(context.getRequiredTestMethod().getAnnotation(BeforeStartDistribution.class)); - dist.start(Arrays.asList(launch.value())); + result = dist.run(Arrays.asList(launch.value())); } } else { configureProfile(context); @@ -154,6 +155,7 @@ public class CLITestExtension extends QuarkusMainTestExtension { databaseContainer.stop(); databaseContainer = null; } + result = null; } @Override @@ -190,23 +192,17 @@ public class CLITestExtension extends QuarkusMainTestExtension { Class type = parameterContext.getParameter().getType(); if (type == LaunchResult.class) { - List outputStream; - List errStream; - int exitCode; - boolean isDistribution = getDistributionConfig(context) != null; if (isDistribution) { - outputStream = dist.getOutputStream(); - errStream = dist.getErrorStream(); - exitCode = dist.getExitCode(); - } else { - LaunchResult result = (LaunchResult) super.resolveParameter(parameterContext, context); - outputStream = result.getOutputStream(); - errStream = result.getErrorStream(); - exitCode = result.exitCode(); + return result; } + LaunchResult result = (LaunchResult) super.resolveParameter(parameterContext, context); + List outputStream = result.getOutputStream(); + List errStream = result.getErrorStream(); + int exitCode = result.exitCode(); + return CLIResult.create(outputStream, errStream, exitCode); } @@ -215,6 +211,13 @@ public class CLITestExtension extends QuarkusMainTestExtension { return getDistPath(); } + if (type.equals(KeycloakDistribution.class)) { + if (dist == null) { + throw new RuntimeException("Only tests annotated with " + DistributionTest.class + " can inject a distribution instance"); + } + return dist; + } + // for now, no support for manual launching using QuarkusMainLauncher throw new RuntimeException("Parameter type [" + type + "] not supported"); } @@ -223,7 +226,7 @@ public class CLITestExtension extends QuarkusMainTestExtension { public boolean supportsParameter(ParameterContext parameterContext, ExtensionContext extensionContext) throws ParameterResolutionException { Class type = parameterContext.getParameter().getType(); - return type == LaunchResult.class || type == RawDistRootPath.class; + return type == LaunchResult.class || type == RawDistRootPath.class || (dist != null && type == KeycloakDistribution.class); } private void configureProfile(ExtensionContext context) { @@ -257,7 +260,7 @@ public class CLITestExtension extends QuarkusMainTestExtension { dist.setProperty("db-password", databaseContainer.getPassword()); dist.setProperty("db-url", databaseContainer.getJdbcUrl()); - dist.start(List.of("build")); + dist.run("build"); } } else { // This is for re-creating the H2 database instead of using the default in home diff --git a/quarkus/tests/integration/src/main/java/org/keycloak/it/junit5/extension/RawDistOnly.java b/quarkus/tests/integration/src/main/java/org/keycloak/it/junit5/extension/RawDistOnly.java index d087fd629c..e8683cab0a 100644 --- a/quarkus/tests/integration/src/main/java/org/keycloak/it/junit5/extension/RawDistOnly.java +++ b/quarkus/tests/integration/src/main/java/org/keycloak/it/junit5/extension/RawDistOnly.java @@ -28,7 +28,7 @@ import org.junit.jupiter.api.condition.EnabledIfSystemProperty; * is only enabled when running tests using the {@link DistributionType#RAW} * or running tests in whitebox mode in the same jvm using {@link CLITest} */ -@Target(ElementType.TYPE) +@Target({ElementType.TYPE, ElementType.METHOD}) @Retention(RetentionPolicy.RUNTIME) @EnabledIfSystemProperty(named = "kc.quarkus.tests.dist", matches = "^$|raw") public @interface RawDistOnly { diff --git a/quarkus/tests/integration/src/main/java/org/keycloak/it/utils/DockerKeycloakDistribution.java b/quarkus/tests/integration/src/main/java/org/keycloak/it/utils/DockerKeycloakDistribution.java index 03927223ec..9b4674459f 100644 --- a/quarkus/tests/integration/src/main/java/org/keycloak/it/utils/DockerKeycloakDistribution.java +++ b/quarkus/tests/integration/src/main/java/org/keycloak/it/utils/DockerKeycloakDistribution.java @@ -2,6 +2,7 @@ package org.keycloak.it.utils; import org.jboss.logging.Logger; import org.keycloak.common.Version; +import org.keycloak.it.junit5.extension.CLIResult; import org.testcontainers.containers.GenericContainer; import org.testcontainers.containers.output.OutputFrame; import org.testcontainers.containers.output.ToStringConsumer; @@ -57,7 +58,7 @@ public final class DockerKeycloakDistribution implements KeycloakDistribution { } @Override - public void start(List arguments) { + public CLIResult run(List arguments) { stop(); try { this.exitCode = -1; @@ -86,6 +87,8 @@ public final class DockerKeycloakDistribution implements KeycloakDistribution { keycloakContainer = null; LOGGER.warn("Failed to start Keycloak container", cause); } + + return CLIResult.create(getOutputStream(), getErrorStream(), getExitCode()); } // After the web server is responding we are still producing some logs that got checked in the tests diff --git a/quarkus/tests/integration/src/main/java/org/keycloak/it/utils/KeycloakDistribution.java b/quarkus/tests/integration/src/main/java/org/keycloak/it/utils/KeycloakDistribution.java index 069202664d..492170aa00 100644 --- a/quarkus/tests/integration/src/main/java/org/keycloak/it/utils/KeycloakDistribution.java +++ b/quarkus/tests/integration/src/main/java/org/keycloak/it/utils/KeycloakDistribution.java @@ -1,5 +1,6 @@ package org.keycloak.it.utils; +import org.keycloak.it.junit5.extension.CLIResult; import org.keycloak.quarkus.runtime.Environment; import java.nio.file.Path; import java.util.List; @@ -9,7 +10,10 @@ public interface KeycloakDistribution { String SCRIPT_CMD = Environment.isWindows() ? "kc.bat" : "kc.sh"; String SCRIPT_CMD_INVOKABLE = Environment.isWindows() ? SCRIPT_CMD : "./"+SCRIPT_CMD; - void start(List arguments); + CLIResult run(List arguments); + default CLIResult run(String... arguments) { + return run(List.of(arguments)); + } void stop(); @@ -47,7 +51,7 @@ public interface KeycloakDistribution { throw new RuntimeException("Not implemented"); } - default void removeProperty(String db) { + default void removeProperty(String name) { throw new RuntimeException("Not implemented"); } } diff --git a/quarkus/tests/integration/src/main/java/org/keycloak/it/utils/RawKeycloakDistribution.java b/quarkus/tests/integration/src/main/java/org/keycloak/it/utils/RawKeycloakDistribution.java index 57445bbbb7..7d3e2cd527 100644 --- a/quarkus/tests/integration/src/main/java/org/keycloak/it/utils/RawKeycloakDistribution.java +++ b/quarkus/tests/integration/src/main/java/org/keycloak/it/utils/RawKeycloakDistribution.java @@ -54,6 +54,7 @@ import io.quarkus.fs.util.ZipUtils; import org.apache.commons.io.FileUtils; import org.keycloak.common.Version; +import org.keycloak.it.junit5.extension.CLIResult; import org.keycloak.quarkus.runtime.Environment; import org.keycloak.quarkus.runtime.cli.command.Build; import org.keycloak.quarkus.runtime.configuration.mappers.PropertyMapper; @@ -86,7 +87,7 @@ public final class RawKeycloakDistribution implements KeycloakDistribution { } @Override - public void start(List arguments) { + public CLIResult run(List arguments) { reset(); if (manualStop && isRunning()) { throw new IllegalStateException("Server already running. You should manually stop the server before starting it again."); @@ -113,6 +114,8 @@ public final class RawKeycloakDistribution implements KeycloakDistribution { stop(); } } + + return CLIResult.create(getOutputStream(), getErrorStream(), getExitCode()); } @Override @@ -423,11 +426,11 @@ public final class RawKeycloakDistribution implements KeycloakDistribution { } @Override - public void removeProperty(String key) { + public void removeProperty(String name) { updateProperties(new Consumer() { @Override public void accept(Properties properties) { - properties.remove(key); + properties.remove(name); } }, distPath.resolve("conf").resolve("keycloak.conf").toFile()); } 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 cb4a9dad58..e625e79610 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 @@ -26,6 +26,8 @@ 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 @@ -62,4 +64,15 @@ class BuildCommandDistTest { CLIResult cliResult = (CLIResult) result; cliResult.assertError("Unknown option: '--db-username'"); } + + @Test + @RawDistOnly(reason = "Containers are immutable") + void testDoNotRecordRuntimeOptionsDuringBuild(KeycloakDistribution distribution) { + distribution.setProperty("proxy", "edge"); + distribution.run("build", "--cache=local"); + distribution.removeProperty("proxy"); + + CLIResult result = distribution.run("start", "--hostname=mykeycloak"); + result.assertMessage("Key material not provided to setup HTTPS"); + } }