From a4df602f625f6f29621a1f1edc42d388a9723043 Mon Sep 17 00:00:00 2001 From: Steven Hawkins Date: Wed, 14 Jan 2026 10:57:48 -0500 Subject: [PATCH] fix: refining arg handling (#44579) relying on picocli parsing as much as possible closes: #44578 Signed-off-by: Steve Hawkins --- .../keycloak/config/ClassLoaderOptions.java | 12 --- .../keycloak/quarkus/runtime/Environment.java | 3 +- .../quarkus/runtime/KeycloakMain.java | 16 +--- .../keycloak/quarkus/runtime/cli/Picocli.java | 85 +++++++------------ .../cli/command/AbstractAutoBuildCommand.java | 29 +------ .../quarkus/runtime/cli/command/Build.java | 20 ++--- .../configuration/ConfigArgsConfigSource.java | 28 ++---- .../mappers/ClassLoaderPropertyMappers.java | 33 ------- .../mappers/PropertyMappers.java | 2 +- .../quarkus/runtime/cli/PicocliTest.java | 8 +- .../ConfigArgsConfigSourceTest.java | 10 +-- .../cli/dist/QuarkusPropertiesDistTest.java | 10 --- .../cli/dist/ShowConfigCommandDistTest.java | 2 +- .../it/cli/dist/StartAutoBuildDistTest.java | 3 +- .../it/cli/dist/StartCommandDistTest.java | 4 +- .../it/storage/database/OracleTest.java | 15 ---- .../src/main/java/org/keycloak/Keycloak.java | 30 ++++++- .../it/junit5/extension/CLITestExtension.java | 25 +++--- 18 files changed, 94 insertions(+), 241 deletions(-) delete mode 100644 quarkus/config-api/src/main/java/org/keycloak/config/ClassLoaderOptions.java delete mode 100644 quarkus/runtime/src/main/java/org/keycloak/quarkus/runtime/configuration/mappers/ClassLoaderPropertyMappers.java diff --git a/quarkus/config-api/src/main/java/org/keycloak/config/ClassLoaderOptions.java b/quarkus/config-api/src/main/java/org/keycloak/config/ClassLoaderOptions.java deleted file mode 100644 index f1df90c5696..00000000000 --- a/quarkus/config-api/src/main/java/org/keycloak/config/ClassLoaderOptions.java +++ /dev/null @@ -1,12 +0,0 @@ -package org.keycloak.config; - -public class ClassLoaderOptions { - - public static final String QUARKUS_REMOVED_ARTIFACTS_PROPERTY = "quarkus.class-loading.removed-artifacts"; - - public static final Option IGNORE_ARTIFACTS = new OptionBuilder<>("class-loader-ignore-artifacts", String.class) - .category(OptionCategory.GENERAL) - .hidden() - .buildTime(true) - .build(); -} 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 a547ae3ae2d..b70dc72d210 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 @@ -19,6 +19,7 @@ package org.keycloak.quarkus.runtime; import java.io.File; import java.io.FilenameFilter; +import java.nio.file.Files; import java.nio.file.Path; import java.nio.file.Paths; import java.util.Arrays; @@ -119,7 +120,7 @@ public final class Environment { public static Map getProviderFiles() { Path providersPath = Environment.getProvidersPath().orElse(null); - if (providersPath == null) { + if (providersPath == null || !Files.exists(providersPath)) { return Collections.emptyMap(); } diff --git a/quarkus/runtime/src/main/java/org/keycloak/quarkus/runtime/KeycloakMain.java b/quarkus/runtime/src/main/java/org/keycloak/quarkus/runtime/KeycloakMain.java index 20bba07ab15..286a54a11fa 100644 --- a/quarkus/runtime/src/main/java/org/keycloak/quarkus/runtime/KeycloakMain.java +++ b/quarkus/runtime/src/main/java/org/keycloak/quarkus/runtime/KeycloakMain.java @@ -17,7 +17,6 @@ package org.keycloak.quarkus.runtime; -import java.util.ArrayList; import java.util.List; import java.util.concurrent.ForkJoinPool; @@ -27,7 +26,6 @@ import org.keycloak.common.Version; import org.keycloak.infinispan.util.InfinispanUtils; import org.keycloak.quarkus.runtime.cli.ExecutionExceptionHandler; import org.keycloak.quarkus.runtime.cli.Picocli; -import org.keycloak.quarkus.runtime.cli.PropertyException; import org.keycloak.quarkus.runtime.cli.command.AbstractNonServerCommand; import org.keycloak.quarkus.runtime.cli.command.DryRunMixin; import org.keycloak.quarkus.runtime.configuration.PersistedConfigSource; @@ -80,24 +78,12 @@ public class KeycloakMain implements QuarkusApplication { } public static void main(String[] args, Picocli picocli) { - List cliArgs = null; - try { - cliArgs = Picocli.parseArgs(args); - } catch (PropertyException e) { - picocli.usageException(e.getMessage(), e.getCause()); - return; - } + List cliArgs = List.of(args.length == 0 ? new String[] {"-h"} : args); if (DryRunMixin.isDryRunBuild() && (cliArgs.contains(DryRunMixin.DRYRUN_OPTION_LONG) || Boolean.valueOf(System.getenv().get(DryRunMixin.KC_DRY_RUN_ENV)))) { PersistedConfigSource.getInstance().useDryRunProperties(); } - if (cliArgs.isEmpty()) { - cliArgs = new ArrayList<>(cliArgs); - // default to show help message - cliArgs.add("-h"); - } - // parse arguments and execute any of the configured commands picocli.parseAndRun(cliArgs); } diff --git a/quarkus/runtime/src/main/java/org/keycloak/quarkus/runtime/cli/Picocli.java b/quarkus/runtime/src/main/java/org/keycloak/quarkus/runtime/cli/Picocli.java index 9c85aff8380..153e1f9d50c 100644 --- a/quarkus/runtime/src/main/java/org/keycloak/quarkus/runtime/cli/Picocli.java +++ b/quarkus/runtime/src/main/java/org/keycloak/quarkus/runtime/cli/Picocli.java @@ -103,6 +103,7 @@ public class Picocli { private Ansi colorMode = hasColorSupport() ? Ansi.ON : Ansi.OFF; private IncludeOptions options; + private Set duplicatedOptionsNames = new HashSet(); public static boolean hasColorSupport() { return QuarkusConsole.hasColorSupport(); @@ -139,31 +140,37 @@ public class Picocli { } else { currentCommand = null; } + + // any unrecognized args can now be normalized to our property mapper based argument expectations + Map normalizedArgs = new LinkedHashMap(); + List unknown = new ArrayList(); + ConfigArgsConfigSource.parseConfigArgs(unrecognizedArgs, (k, v) -> { + if (normalizedArgs.put(k, v) != null) { + duplicatedOptionsNames.add(k); + } + }, unknown::add); + unrecognizedArgs = null; + + ConfigArgsConfigSource.setCliArgs(normalizedArgs.entrySet().stream().map(e -> e.getKey() + "=" + e.getValue()).toArray(String[]::new)); + initConfig(currentCommand); - if (!unrecognizedArgs.isEmpty() && options.allowUnrecognized) { - // TODO: further refactor this as these args should be the source for ConfigArgsConfigSource - unrecognizedArgs.removeIf(arg -> { - boolean hasArg = false; - if (arg.contains("=")) { - arg = arg.substring(0, arg.indexOf("=")); - hasArg = true; - } - PropertyMapper mapper = PropertyMappers.getMapperByCliKey(arg); - if (mapper != null) { - if (!hasArg) { - addCommandOptions(cl, currentCommand); - throw new MissingParameterException(cl, cl.getCommandSpec().optionsMap().get(arg), null); - } - return true; - } - return false; - }); + // now that the property mappers are properly initalized further refine the args + if (options.allowUnrecognized) { + normalizedArgs.keySet().removeIf(arg -> PropertyMappers.getMapperByCliKey(arg) != null || arg.startsWith(ConfigArgsConfigSource.SPI_OPTION_PREFIX)); } - - if (!unrecognizedArgs.isEmpty()) { + unknown.forEach(arg -> { + if (PropertyMappers.getMapperByCliKey(arg) != null) { + addCommandOptions(cl, currentCommand); + throw new MissingParameterException(cl, cl.getCommandSpec().optionsMap().get(arg), null); + } else if (arg.startsWith(ConfigArgsConfigSource.SPI_OPTION_PREFIX)) { + throw new PropertyException(format("spi argument %s requires a value.", arg)); + } + }); + unknown.addAll(normalizedArgs.keySet()); + if (!unknown.isEmpty()) { addCommandOptions(cl, currentCommand); - throw new KcUnmatchedArgumentException(cl, unrecognizedArgs); + throw new KcUnmatchedArgumentException(cl, unknown); } if (isHelpRequested(result)) { @@ -823,33 +830,6 @@ public class Picocli { getOutWriter().println(message); } - public static List parseArgs(String[] rawArgs) throws PropertyException { - if (rawArgs.length == 0) { - return List.of(); - } - - // makes sure cli args are available to the config source - ConfigArgsConfigSource.setCliArgs(rawArgs); - - // TODO: ignore properties for providers for now, need to fetch them from the providers, otherwise CLI will complain about invalid options - // also ignores system properties as they are set when starting the JVM - // change this once we are able to obtain properties from providers - List args = new ArrayList<>(); - ConfigArgsConfigSource.parseConfigArgs(List.of(rawArgs), (arg, value) -> { - if (!arg.startsWith(ConfigArgsConfigSource.SPI_OPTION_PREFIX) && !arg.startsWith("-D")) { - args.add(arg + "=" + value); - } - }, arg -> { - if (arg.startsWith(ConfigArgsConfigSource.SPI_OPTION_PREFIX)) { - throw new PropertyException(format("spi argument %s requires a value.", arg)); - } - if (!arg.startsWith("-D")) { - args.add(arg); - } - }); - return args; - } - public void checkChangesInBuildOptionsDuringAutoBuild(PrintWriter out) { StringBuilder options = new StringBuilder(); @@ -913,6 +893,7 @@ public class Picocli { } public void build() throws Throwable { + Environment.setRebuild(); QuarkusEntryPoint.main(); } @@ -923,10 +904,8 @@ public class Picocli { this.parsedCommand = Optional.ofNullable(command); options = getIncludeOptions(command); - if (!Environment.isRebuilt() && command instanceof AbstractAutoBuildCommand - && !command.isOptimized()) { - Environment.setRebuildCheck(true); - } + Environment.setRebuildCheck(!Environment.isRebuilt() && command instanceof AbstractAutoBuildCommand + && !command.isOptimized()); String profile = Optional.ofNullable(org.keycloak.common.util.Environment.getProfile()) .or(() -> parsedCommand.map(AbstractCommand::getInitProfile)).orElse(Environment.PROD_PROFILE_VALUE); @@ -939,10 +918,8 @@ public class Picocli { // Show warning about duplicated options in CLI public void warnOnDuplicatedOptionsInCli() { - var duplicatedOptionsNames = ConfigArgsConfigSource.getDuplicatedArgNames(); if (!duplicatedOptionsNames.isEmpty()) { warn("Duplicated options present in CLI: %s".formatted(String.join(", ", duplicatedOptionsNames))); - ConfigArgsConfigSource.clearDuplicatedArgNames(); } } diff --git a/quarkus/runtime/src/main/java/org/keycloak/quarkus/runtime/cli/command/AbstractAutoBuildCommand.java b/quarkus/runtime/src/main/java/org/keycloak/quarkus/runtime/cli/command/AbstractAutoBuildCommand.java index 4456e3c6c29..e9990bae1f7 100644 --- a/quarkus/runtime/src/main/java/org/keycloak/quarkus/runtime/cli/command/AbstractAutoBuildCommand.java +++ b/quarkus/runtime/src/main/java/org/keycloak/quarkus/runtime/cli/command/AbstractAutoBuildCommand.java @@ -17,26 +17,19 @@ package org.keycloak.quarkus.runtime.cli.command; -import java.util.ArrayList; -import java.util.List; import java.util.Map; import java.util.Objects; import java.util.Optional; import org.keycloak.quarkus.runtime.Environment; import org.keycloak.quarkus.runtime.cli.Picocli; -import org.keycloak.quarkus.runtime.configuration.ConfigArgsConfigSource; import org.keycloak.quarkus.runtime.configuration.Configuration; -import org.keycloak.quarkus.runtime.configuration.mappers.PropertyMapper; -import org.keycloak.quarkus.runtime.configuration.mappers.PropertyMappers; import picocli.CommandLine; import static org.keycloak.quarkus.runtime.Environment.isDevMode; import static org.keycloak.quarkus.runtime.Environment.isDevProfile; import static org.keycloak.quarkus.runtime.Environment.isRebuildCheck; -import static org.keycloak.quarkus.runtime.configuration.ConfigArgsConfigSource.parseConfigArgs; -import static org.keycloak.quarkus.runtime.configuration.mappers.PropertyMappers.maskValue; public abstract class AbstractAutoBuildCommand extends AbstractCommand { @@ -94,7 +87,7 @@ public abstract class AbstractAutoBuildCommand extends AbstractCommand { directBuild(); if(!isDevMode()) { - spec.commandLine().getOut().printf("Next time you run the server, just run:%n%n\t%s %s %s%n%n", Environment.getCommand(), String.join(" ", getSanitizedRuntimeCliOptions()), OPTIMIZED_BUILD_OPTION_LONG); + spec.commandLine().getOut().printf("Next time you run the server, just add %s to the command to ensure this build is used.\n", OPTIMIZED_BUILD_OPTION_LONG); } } @@ -106,26 +99,6 @@ public abstract class AbstractAutoBuildCommand extends AbstractCommand { build.runCommand(); } - /** - * checks the raw cli input for possible credentials / properties which should be masked, - * and masks them. - * @return a list of potentially masked properties in CLI format, e.g. `--db-password=*******` - * instead of the actual passwords value. - */ - private static List getSanitizedRuntimeCliOptions() { - List properties = new ArrayList<>(); - - parseConfigArgs(ConfigArgsConfigSource.getAllCliArgs(), (key, value) -> { - PropertyMapper mapper = PropertyMappers.getMapperByCliKey(key); - - if (mapper == null || mapper.isRunTime()) { - properties.add(key + "=" + maskValue(value, mapper)); - } - }, properties::add); - - return properties; - } - @Override protected void runCommand() { doBeforeRun(); 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 bce7d7047bf..ca7e602ed58 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 @@ -17,20 +17,17 @@ package org.keycloak.quarkus.runtime.cli.command; -import java.util.Optional; - import org.keycloak.quarkus.runtime.Environment; import org.keycloak.quarkus.runtime.Messages; import org.keycloak.quarkus.runtime.configuration.Configuration; +import org.keycloak.quarkus.runtime.configuration.IgnoredArtifacts; import org.keycloak.quarkus.runtime.configuration.PersistedConfigSource; import io.quarkus.bootstrap.runner.RunnerClassLoader; import io.quarkus.runtime.LaunchMode; -import io.smallrye.config.ConfigValue; import picocli.CommandLine; import picocli.CommandLine.Command; -import static org.keycloak.config.ClassLoaderOptions.QUARKUS_REMOVED_ARTIFACTS_PROPERTY; import static org.keycloak.config.DatabaseOptions.DB; import static org.keycloak.quarkus.runtime.Environment.getHomePath; import static org.keycloak.quarkus.runtime.Environment.isDevProfile; @@ -59,6 +56,8 @@ public final class Build extends AbstractCommand { public static final String NAME = "build"; + public static final String QUARKUS_REMOVED_ARTIFACTS_PROPERTY = "quarkus.class-loading.removed-artifacts"; + @CommandLine.Mixin HelpAllMixin helpAllMixin; @@ -70,17 +69,15 @@ public final class Build extends AbstractCommand { checkProfileAndDb(); // validate before setting that we're rebuilding so that runtime options are still seen + // the validation and setting the artifacts to remove need to be done without the current persisted properties PersistedConfigSource.getInstance().runWithDisabled(() -> { validateConfig(); + System.setProperty(QUARKUS_REMOVED_ARTIFACTS_PROPERTY, String.join(",", IgnoredArtifacts.getDefaultIgnoredArtifacts())); return null; }); - Environment.setRebuild(); - picocli.println("Updating the configuration and installing your custom providers, if any. Please wait."); try { - configureBuildClassLoader(); - beforeReaugmentationOnWindows(); if (!Boolean.TRUE.equals(dryRunMixin.dryRun)) { picocli.build(); @@ -99,13 +96,6 @@ public final class Build extends AbstractCommand { } } - private static void configureBuildClassLoader() { - // ignored artifacts must be set prior to starting re-augmentation - Optional.ofNullable(Configuration.getNonPersistedConfigValue(QUARKUS_REMOVED_ARTIFACTS_PROPERTY)) - .map(ConfigValue::getValue) - .ifPresent(s -> System.setProperty(QUARKUS_REMOVED_ARTIFACTS_PROPERTY, s)); - } - private void checkProfileAndDb() { if (Environment.isDevProfile()) { String cmd = picocli.getParsedCommand().map(AbstractCommand::getName).orElse(getName()); diff --git a/quarkus/runtime/src/main/java/org/keycloak/quarkus/runtime/configuration/ConfigArgsConfigSource.java b/quarkus/runtime/src/main/java/org/keycloak/quarkus/runtime/configuration/ConfigArgsConfigSource.java index 95a4d28c465..23836b2d3c8 100644 --- a/quarkus/runtime/src/main/java/org/keycloak/quarkus/runtime/configuration/ConfigArgsConfigSource.java +++ b/quarkus/runtime/src/main/java/org/keycloak/quarkus/runtime/configuration/ConfigArgsConfigSource.java @@ -20,7 +20,6 @@ package org.keycloak.quarkus.runtime.configuration; import java.util.ArrayList; import java.util.Collections; import java.util.HashMap; -import java.util.HashSet; import java.util.List; import java.util.Map; import java.util.Set; @@ -31,11 +30,11 @@ import java.util.stream.Collectors; import java.util.stream.Stream; import org.keycloak.quarkus.runtime.cli.command.Main; -import org.keycloak.quarkus.runtime.configuration.mappers.PropertyMapper; import org.keycloak.quarkus.runtime.configuration.mappers.PropertyMappers; import io.smallrye.config.PropertiesConfigSource; +import static org.keycloak.quarkus.runtime.cli.Picocli.ARG_PREFIX; import static org.keycloak.quarkus.runtime.cli.Picocli.ARG_SHORT_PREFIX; /** @@ -53,12 +52,14 @@ public class ConfigArgsConfigSource extends PropertiesConfigSource { private static final String CLI_ARGS = "kc.config.args"; public static final String NAME = "CliConfigSource"; private static final Pattern ARG_KEY_VALUE_SPLIT = Pattern.compile("="); - private static Set duplicatedArgNames; protected ConfigArgsConfigSource() { super(parseArguments(), NAME, 600); } + /** + * Should only be called with sanitized args --property=value + */ public static void setCliArgs(String... args) { System.setProperty(CLI_ARGS, Stream.of(args).map(arg -> arg.replaceAll(",", ",,")).collect(Collectors.joining(", "))); @@ -106,28 +107,14 @@ public class ConfigArgsConfigSource extends PropertiesConfigSource { private static Map parseArguments() { final Map properties = new HashMap<>(); - final Set allPropertiesNames = new HashSet<>(); - duplicatedArgNames = new HashSet<>(); parseConfigArgs(getAllCliArgs(), (key, value) -> { - if (!allPropertiesNames.add(key)) { - duplicatedArgNames.add(key); - } PropertyMappers.getKcKeyFromCliKey(key).ifPresent(s -> properties.put(s, value)); }, ignored -> {}); return properties; } - public static Set getDuplicatedArgNames() { - return Collections.unmodifiableSet(duplicatedArgNames); - } - - public static void clearDuplicatedArgNames() { - // after handling these duplicates, clear the memory - duplicatedArgNames = Collections.emptySet(); - } - public static void parseConfigArgs(List args, BiConsumer valueArgConsumer, Consumer unaryConsumer) { for (int i = 0; i < args.size(); i++) { String arg = args.get(i); @@ -147,12 +134,7 @@ public class ConfigArgsConfigSource extends PropertiesConfigSource { unaryConsumer.accept(arg); continue; } - PropertyMapper mapper = PropertyMappers.getMapperByCliKey(key); - // the weaknesses here: - // - runs before propertymapper sanitizing - // - needs to know all of the short name options that accept a value - // - Even though We've explicitly disabled PosixClusteredShortOptionsAllowed, it may not know all of the picocli parsing rules. - if (mapper != null || SHORT_OPTIONS_ACCEPTING_VALUE.contains(key) || arg.startsWith(SPI_OPTION_PREFIX)) { + if (arg.startsWith(ARG_PREFIX)) { i++; // consume next as a value to the key value = args.get(i); } else { diff --git a/quarkus/runtime/src/main/java/org/keycloak/quarkus/runtime/configuration/mappers/ClassLoaderPropertyMappers.java b/quarkus/runtime/src/main/java/org/keycloak/quarkus/runtime/configuration/mappers/ClassLoaderPropertyMappers.java deleted file mode 100644 index b264146104b..00000000000 --- a/quarkus/runtime/src/main/java/org/keycloak/quarkus/runtime/configuration/mappers/ClassLoaderPropertyMappers.java +++ /dev/null @@ -1,33 +0,0 @@ -package org.keycloak.quarkus.runtime.configuration.mappers; - -import java.util.List; - -import org.keycloak.config.ClassLoaderOptions; -import org.keycloak.quarkus.runtime.Environment; -import org.keycloak.quarkus.runtime.configuration.IgnoredArtifacts; - -import io.smallrye.config.ConfigSourceInterceptorContext; - -import static org.keycloak.config.ClassLoaderOptions.QUARKUS_REMOVED_ARTIFACTS_PROPERTY; -import static org.keycloak.quarkus.runtime.configuration.mappers.PropertyMapper.fromOption; - -final class ClassLoaderPropertyMappers implements PropertyMapperGrouping { - - @Override - public List> getPropertyMappers() { - return List.of( - fromOption(ClassLoaderOptions.IGNORE_ARTIFACTS) - .to(QUARKUS_REMOVED_ARTIFACTS_PROPERTY) - .transformer(ClassLoaderPropertyMappers::resolveIgnoredArtifacts) - .build() - ); - } - - private static String resolveIgnoredArtifacts(String value, ConfigSourceInterceptorContext context) { - if (Environment.isRebuildCheck() || Environment.isRebuild()) { - return String.join(",", IgnoredArtifacts.getDefaultIgnoredArtifacts()); - } - - return value; - } -} 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 42adea621df..e4bf980b48b 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 @@ -48,7 +48,7 @@ public final class PropertyMappers { private final static List GROUPINGS; static { GROUPINGS = List.of(new CachingPropertyMappers(), new DatabasePropertyMappers(), - new ConfigKeystorePropertyMappers(), new EventPropertyMappers(), new ClassLoaderPropertyMappers(), + new ConfigKeystorePropertyMappers(), new EventPropertyMappers(), new ExportPropertyMappers(), new BootstrapAdminPropertyMappers(), new HostnameV2PropertyMappers(), new HttpPropertyMappers(), new HttpAccessLogPropertyMappers(), new HealthPropertyMappers(), new FeaturePropertyMappers(), new ImportPropertyMappers(), new ManagementPropertyMappers(), diff --git a/quarkus/runtime/src/test/java/org/keycloak/quarkus/runtime/cli/PicocliTest.java b/quarkus/runtime/src/test/java/org/keycloak/quarkus/runtime/cli/PicocliTest.java index 23adab4a1c1..c4dd0417429 100644 --- a/quarkus/runtime/src/test/java/org/keycloak/quarkus/runtime/cli/PicocliTest.java +++ b/quarkus/runtime/src/test/java/org/keycloak/quarkus/runtime/cli/PicocliTest.java @@ -1017,11 +1017,6 @@ public class PicocliTest extends AbstractConfigurationTest { assertThat(nonRunningPicocli.getErrString(), containsString("Unknown option: '--non-existing'")); onAfter(); - nonRunningPicocli = pseudoLaunch("start-dev", "-Dsome.property=123", "-Dsome.property=456"); - assertEquals(CommandLine.ExitCode.OK, nonRunningPicocli.exitCode); - assertThat(nonRunningPicocli.getOutString(), containsString("WARNING: Duplicated options present in CLI: -Dsome.property")); - onAfter(); - nonRunningPicocli = pseudoLaunch("start-dev", "something-wrong=asdf", "something-wrong=not-here"); assertEquals(CommandLine.ExitCode.USAGE, nonRunningPicocli.exitCode); assertThat(nonRunningPicocli.getOutString(), not(containsString("WARNING: Duplicated options present in CLI: something-wrong"))); @@ -1723,6 +1718,9 @@ public class PicocliTest extends AbstractConfigurationTest { KeycloakMain.main(new String[] {"tools", "windows-service"}, nonRunningPicocli); assertEquals(CommandLine.ExitCode.USAGE, nonRunningPicocli.exitCode); assertTrue(nonRunningPicocli.getErrString().contains("Missing required subcommand")); + onAfter(); + KeycloakMain.main(new String[] {"tools", "windows-service", "uninstall", "--db=bar"}, nonRunningPicocli); + assertEquals(CommandLine.ExitCode.USAGE, nonRunningPicocli.exitCode); } @Test diff --git a/quarkus/runtime/src/test/java/org/keycloak/quarkus/runtime/configuration/ConfigArgsConfigSourceTest.java b/quarkus/runtime/src/test/java/org/keycloak/quarkus/runtime/configuration/ConfigArgsConfigSourceTest.java index 3d2e0a71c7b..6f2c193297b 100644 --- a/quarkus/runtime/src/test/java/org/keycloak/quarkus/runtime/configuration/ConfigArgsConfigSourceTest.java +++ b/quarkus/runtime/src/test/java/org/keycloak/quarkus/runtime/configuration/ConfigArgsConfigSourceTest.java @@ -30,23 +30,23 @@ public class ConfigArgsConfigSourceTest { @Test public void testParseArgs() { List values = new ArrayList<>(); - ConfigArgsConfigSource.parseConfigArgs(Arrays.asList("--key=value", "-cf", "file", "command", "arg", "--db", "value"), (key, value) -> values.add(key+'='+value), values::add); - assertEquals(Arrays.asList("--key=value", "-cf=file", "command", "arg", "--db=value"), values); + ConfigArgsConfigSource.parseConfigArgs(Arrays.asList("--key=value", "command", "arg", "--db", "value"), (key, value) -> values.add(key+'='+value), values::add); + assertEquals(Arrays.asList("--key=value", "command", "arg", "--db=value"), values); } - + @Test public void testParseArgsWithSpi() { List values = new ArrayList<>(); ConfigArgsConfigSource.parseConfigArgs(Arrays.asList("--spi-some-thing-enabled=value", "--spi-some-thing-else", "other-value"), (key, value) -> values.add(key+'='+value), ignored -> {}); assertEquals(Arrays.asList("--spi-some-thing-enabled=value", "--spi-some-thing-else=other-value"), values); } - + @Test public void testArgEndingInSemiColon() { ConfigArgsConfigSource.setCliArgs("--some=thing;", "--else=value"); assertEquals(Arrays.asList("--some=thing;", "--else=value"), ConfigArgsConfigSource.getAllCliArgs()); } - + @Test public void testArgCommas() { ConfigArgsConfigSource.setCliArgs("--some=,,,", "--else=,"); diff --git a/quarkus/tests/integration/src/test/java/org/keycloak/it/cli/dist/QuarkusPropertiesDistTest.java b/quarkus/tests/integration/src/test/java/org/keycloak/it/cli/dist/QuarkusPropertiesDistTest.java index 7e1d0167bf5..e3c79b2b06a 100644 --- a/quarkus/tests/integration/src/test/java/org/keycloak/it/cli/dist/QuarkusPropertiesDistTest.java +++ b/quarkus/tests/integration/src/test/java/org/keycloak/it/cli/dist/QuarkusPropertiesDistTest.java @@ -196,7 +196,6 @@ public class QuarkusPropertiesDistTest { } @Test - @BeforeStartDistribution(ForceRebuild.class) @DisabledOnOs(value = { OS.WINDOWS }, disabledReason = "Windows uses a different path separator.") @Launch({ "start", "--verbose", "--http-enabled=true", "--hostname-strict=false", "--https-certificate-file=/tmp/kc/bin/../conf/server.crt.pem", @@ -208,7 +207,6 @@ public class QuarkusPropertiesDistTest { } @Test - @BeforeStartDistribution(ForceRebuild.class) @EnabledOnOs(value = { OS.WINDOWS }, disabledReason = "Windows uses a different path separator.") @Launch({ "start", "--http-enabled=true", "--hostname-strict=false", "--https-certificate-file=C:\\tmp\\kc\\bin\\..\\conf/server.crt.pem", @@ -255,12 +253,4 @@ public class QuarkusPropertiesDistTest { } } - public static class ForceRebuild implements Consumer { - - @Override - public void accept(KeycloakDistribution distribution) { - CLIResult buildResult = distribution.run("build"); - buildResult.assertBuild(); - } - } } diff --git a/quarkus/tests/integration/src/test/java/org/keycloak/it/cli/dist/ShowConfigCommandDistTest.java b/quarkus/tests/integration/src/test/java/org/keycloak/it/cli/dist/ShowConfigCommandDistTest.java index d8b7e21121e..c46211a6fc3 100644 --- a/quarkus/tests/integration/src/test/java/org/keycloak/it/cli/dist/ShowConfigCommandDistTest.java +++ b/quarkus/tests/integration/src/test/java/org/keycloak/it/cli/dist/ShowConfigCommandDistTest.java @@ -108,7 +108,7 @@ public class ShowConfigCommandDistTest { distribution.setEnvVar("KC_LOG", "file"); distribution.copyOrReplaceFile(Paths.get("src/test/resources/ShowConfigCommandTest/quarkus.properties"), Path.of("conf", "quarkus.properties")); - result = distribution.run(String.format("%s=%s", CONFIG_FILE_LONG_NAME, Paths.get("src/test/resources/ShowConfigCommandTest/keycloak-keystore.conf").toAbsolutePath().normalize()), ShowConfig.NAME, "all"); + result = distribution.run(String.format("%s=%s", CONFIG_FILE_LONG_NAME, Paths.get("src/test/resources/ShowConfigCommandTest/keycloak-keystore.conf").toAbsolutePath().normalize()), ShowConfig.NAME, "all", "--db=dev-file"); result.assertMessage("(CLI)"); result.assertMessage("(ENV)"); diff --git a/quarkus/tests/integration/src/test/java/org/keycloak/it/cli/dist/StartAutoBuildDistTest.java b/quarkus/tests/integration/src/test/java/org/keycloak/it/cli/dist/StartAutoBuildDistTest.java index ce2c4292513..ed0ab57b397 100644 --- a/quarkus/tests/integration/src/test/java/org/keycloak/it/cli/dist/StartAutoBuildDistTest.java +++ b/quarkus/tests/integration/src/test/java/org/keycloak/it/cli/dist/StartAutoBuildDistTest.java @@ -49,8 +49,7 @@ public class StartAutoBuildDistTest { cliResult.assertMessage("Updating the configuration and installing your custom providers, if any. Please wait."); cliResult.assertMessage("Server configuration updated and persisted. Run the following command to review the configuration:"); cliResult.assertMessage(KeycloakDistribution.SCRIPT_CMD + " show-config"); - cliResult.assertMessage("Next time you run the server, just run:"); - cliResult.assertMessage(KeycloakDistribution.SCRIPT_CMD + " --verbose start --http-enabled=true --hostname-strict=false " + OPTIMIZED_BUILD_OPTION_LONG); + cliResult.assertMessage("Next time you run the server, just add --optimized to the command to ensure this build is used."); cliResult.assertNoMessage("--cache"); assertTrue(cliResult.getErrorOutput().isBlank()); } diff --git a/quarkus/tests/integration/src/test/java/org/keycloak/it/cli/dist/StartCommandDistTest.java b/quarkus/tests/integration/src/test/java/org/keycloak/it/cli/dist/StartCommandDistTest.java index 34433ae5bb2..4175a459a76 100644 --- a/quarkus/tests/integration/src/test/java/org/keycloak/it/cli/dist/StartCommandDistTest.java +++ b/quarkus/tests/integration/src/test/java/org/keycloak/it/cli/dist/StartCommandDistTest.java @@ -31,7 +31,6 @@ import org.junit.jupiter.api.Tag; import org.junit.jupiter.api.Test; import org.junit.jupiter.api.TestMethodOrder; -import static org.keycloak.quarkus.runtime.cli.command.AbstractAutoBuildCommand.OPTIMIZED_BUILD_OPTION_LONG; import static org.keycloak.quarkus.runtime.cli.command.Main.CONFIG_FILE_LONG_NAME; import static org.hamcrest.CoreMatchers.containsString; @@ -165,8 +164,7 @@ public class StartCommandDistTest { cliResult.assertMessage("Updating the configuration and installing your custom providers, if any. Please wait."); cliResult.assertMessage("Server configuration updated and persisted. Run the following command to review the configuration:"); cliResult.assertMessage(KeycloakDistribution.SCRIPT_CMD + " show-config"); - cliResult.assertMessage("Next time you run the server, just run:"); - cliResult.assertMessage(KeycloakDistribution.SCRIPT_CMD + " start --http-enabled=true --hostname-strict=false " + OPTIMIZED_BUILD_OPTION_LONG); + cliResult.assertMessage("Next time you run the server, just add --optimized to the command to ensure this build is used."); assertFalse(cliResult.getOutput().contains("--metrics-enabled")); assertTrue(cliResult.getErrorOutput().isBlank(), cliResult.getErrorOutput()); } diff --git a/quarkus/tests/integration/src/test/java/org/keycloak/it/storage/database/OracleTest.java b/quarkus/tests/integration/src/test/java/org/keycloak/it/storage/database/OracleTest.java index e45dfa2aa07..9c92904f941 100644 --- a/quarkus/tests/integration/src/test/java/org/keycloak/it/storage/database/OracleTest.java +++ b/quarkus/tests/integration/src/test/java/org/keycloak/it/storage/database/OracleTest.java @@ -17,18 +17,12 @@ package org.keycloak.it.storage.database; -import java.util.function.Consumer; - -import org.keycloak.it.junit5.extension.BeforeStartDistribution; 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.utils.KeycloakDistribution; -import org.keycloak.it.utils.RawKeycloakDistribution; @CLITest @WithDatabase(alias = "oracle") -@BeforeStartDistribution(OracleTest.CopyOracleJdbcDriver.class) public class OracleTest extends BasicDatabaseTest { @Override @@ -41,13 +35,4 @@ public class OracleTest extends BasicDatabaseTest { cliResult.assertMessage("ORA-01017: invalid username/password; logon denied"); } - public static class CopyOracleJdbcDriver implements Consumer { - - @Override - public void accept(KeycloakDistribution distribution) { - RawKeycloakDistribution rawDist = distribution.unwrap(RawKeycloakDistribution.class); - rawDist.copyProvider("com.oracle.database.jdbc", "ojdbc17"); - rawDist.copyProvider("com.oracle.database.nls", "orai18n"); - } - } } diff --git a/quarkus/tests/junit5/src/main/java/org/keycloak/Keycloak.java b/quarkus/tests/junit5/src/main/java/org/keycloak/Keycloak.java index 607fe31b0c0..f45c927792b 100644 --- a/quarkus/tests/junit5/src/main/java/org/keycloak/Keycloak.java +++ b/quarkus/tests/junit5/src/main/java/org/keycloak/Keycloak.java @@ -33,7 +33,6 @@ import org.keycloak.config.SecurityOptions; import org.keycloak.platform.Platform; import org.keycloak.quarkus.runtime.Environment; import org.keycloak.quarkus.runtime.cli.Picocli; -import org.keycloak.quarkus.runtime.configuration.ConfigArgsConfigSource; import org.keycloak.quarkus.runtime.configuration.Configuration; import org.keycloak.quarkus.runtime.configuration.IgnoredArtifacts; @@ -61,7 +60,6 @@ public class Keycloak { static { System.setProperty("java.util.logging.manager", "org.jboss.logmanager.LogManager"); - System.setProperty(Environment.KC_CONFIG_BUILT, "true"); System.setProperty("quarkus.http.test-port", "${kc.http-port}"); System.setProperty("quarkus.http.test-ssl-port", "${kc.https-port}"); System.setProperty("java.util.concurrent.ForkJoinPool.common.threadFactory", QuarkusForkJoinWorkerThreadFactory.class.getName()); @@ -204,7 +202,7 @@ public class Keycloak { curated = builder.build().bootstrap(); AugmentAction action = curated.createAugmentor(); Environment.setHomeDir(homeDir); - ConfigArgsConfigSource.setCliArgs(args.toArray(new String[0])); + initSys(args.toArray(String[]::new)); System.setProperty(Environment.KC_TEST_REBUILD, "true"); StartupAction startupAction = action.createInitialRuntimeApplication(); System.getProperties().remove(Environment.KC_TEST_REBUILD); @@ -312,4 +310,30 @@ public class Keycloak { application = null; curated = null; } + + /** + * Uses a dummy {@link Picocli} to process the args and set system + * variables needed to run augmentation + */ + public static void initSys(String... args) { + Picocli picocli = new Picocli() { + + @Override + public void build() throws Throwable { + // do nothing + } + + @Override + public void start() { + throw new AssertionError(); + } + + @Override + public void exit(int exitCode) { + // do nothing + } + }; + picocli.parseAndRun(List.of(args)); + System.setProperty(Environment.KC_CONFIG_BUILT, "true"); + } } diff --git a/quarkus/tests/junit5/src/main/java/org/keycloak/it/junit5/extension/CLITestExtension.java b/quarkus/tests/junit5/src/main/java/org/keycloak/it/junit5/extension/CLITestExtension.java index fe654a76d58..e6dbcd47b93 100644 --- a/quarkus/tests/junit5/src/main/java/org/keycloak/it/junit5/extension/CLITestExtension.java +++ b/quarkus/tests/junit5/src/main/java/org/keycloak/it/junit5/extension/CLITestExtension.java @@ -24,9 +24,9 @@ import java.util.Arrays; import java.util.Collections; import java.util.HashMap; import java.util.List; -import java.util.stream.Collectors; import java.util.stream.Stream; +import org.keycloak.Keycloak; import org.keycloak.it.utils.KeycloakDistribution; import org.keycloak.it.utils.RawDistRootPath; import org.keycloak.it.utils.RawKeycloakDistribution; @@ -34,9 +34,7 @@ import org.keycloak.quarkus.runtime.Environment; import org.keycloak.quarkus.runtime.cli.command.DryRunMixin; import org.keycloak.quarkus.runtime.cli.command.Start; 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.KeycloakPropertiesConfigSource; import org.keycloak.quarkus.runtime.integration.QuarkusPlatform; import io.quarkus.deployment.util.FileUtil; @@ -56,8 +54,6 @@ import static java.lang.System.setProperty; import static org.keycloak.it.junit5.extension.DistributionTest.ReInstall.BEFORE_ALL; import static org.keycloak.it.junit5.extension.DistributionType.RAW; import static org.keycloak.quarkus.runtime.Environment.forceTestLaunchMode; -import static org.keycloak.quarkus.runtime.cli.command.Main.CONFIG_FILE_LONG_NAME; -import static org.keycloak.quarkus.runtime.cli.command.Main.CONFIG_FILE_SHORT_NAME; public class CLITestExtension extends QuarkusMainTestExtension { @@ -74,15 +70,14 @@ public class CLITestExtension extends QuarkusMainTestExtension { getStore(context).put(SYS_PROPS, new HashMap<>(System.getProperties())); if (launch != null && distConfig == null) { - ConfigArgsConfigSource.parseConfigArgs(List.of(launch.value()), (arg, value) -> { - if (arg.equals(CONFIG_FILE_SHORT_NAME) || arg.equals(CONFIG_FILE_LONG_NAME)) { - setProperty(KeycloakPropertiesConfigSource.KEYCLOAK_CONFIG_FILE_PROP, value); - } else if (arg.startsWith("-D")) { - setProperty(arg, value); - } - }, arg -> { + Stream.of(launch.value()).forEach(arg -> { if (arg.startsWith("-D")) { - setProperty(arg, ""); + int index = arg.indexOf("="); + if (index > 0) { + setProperty(arg.substring(2, index), arg.substring(index + 1, arg.length())); + } else { + setProperty(arg.substring(2), ""); + } } }); } @@ -112,10 +107,10 @@ public class CLITestExtension extends QuarkusMainTestExtension { } if (launch != null) { - result = dist.run(Stream.concat(List.of(launch.value()).stream(), List.of(distConfig.defaultOptions()).stream()).collect(Collectors.toList())); + result = dist.run(List.of(launch.value())); } } else { - ConfigArgsConfigSource.setCliArgs(launch == null ? new String[] {} : launch.value()); + Keycloak.initSys(launch == null ? new String[] {} : launch.value()); configureProfile(context); super.beforeEach(context); }