From f4d1dd9b7f13adc0af8136a79292ca5039f225cb Mon Sep 17 00:00:00 2001 From: Steven Hawkins Date: Fri, 20 Oct 2023 13:21:03 -0400 Subject: [PATCH] improvement: validates the expected values of non-cli properties (#23797) also adds better messages for unknown options closes #13608 --- .../quarkus/runtime/KeycloakMain.java | 11 ++ .../cli/ExecutionExceptionHandler.java | 5 + .../runtime/cli/NonCliPropertyException.java | 26 ++++ .../keycloak/quarkus/runtime/cli/Picocli.java | 130 +++++++++++++++--- .../cli/PropertyMapperParameterConsumer.java | 23 ++-- .../runtime/cli/ShortErrorMessageHandler.java | 39 +++++- .../runtime/cli/command/AbstractCommand.java | 9 ++ .../cli/command/AbstractStartCommand.java | 1 + .../quarkus/runtime/cli/command/Build.java | 8 ++ .../quarkus/runtime/cli/command/Export.java | 5 + .../quarkus/runtime/cli/command/Import.java | 5 + .../runtime/cli/command/ShowConfig.java | 5 + .../quarkus/runtime/cli/command/Start.java | 6 + .../quarkus/runtime/cli/command/StartDev.java | 5 + .../it/cli/dist/BuildCommandDistTest.java | 21 ++- .../keycloak/it/cli/dist/LoggingDistTest.java | 4 +- .../it/cli/dist/StartCommandDistTest.java | 13 +- .../BuildCommandDistTest/keycloak.conf | 1 + .../it/utils/DockerKeycloakDistribution.java | 13 ++ .../src/main/content/conf/keycloak.conf | 2 +- 20 files changed, 293 insertions(+), 39 deletions(-) create mode 100644 quarkus/runtime/src/main/java/org/keycloak/quarkus/runtime/cli/NonCliPropertyException.java create mode 100644 quarkus/tests/integration/src/test/resources/BuildCommandDistTest/keycloak.conf 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 c256a3f472..2a677f89bb 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 @@ -31,6 +31,7 @@ import java.util.ArrayList; import java.util.List; import jakarta.enterprise.context.ApplicationScoped; +import picocli.CommandLine.ExitCode; import io.quarkus.runtime.ApplicationLifecycleManager; import io.quarkus.runtime.Quarkus; @@ -40,6 +41,7 @@ import org.keycloak.Config; import org.keycloak.models.KeycloakSessionFactory; import org.keycloak.models.utils.KeycloakModelUtils; import org.keycloak.quarkus.runtime.cli.ExecutionExceptionHandler; +import org.keycloak.quarkus.runtime.cli.NonCliPropertyException; import org.keycloak.quarkus.runtime.cli.Picocli; import org.keycloak.common.Version; import org.keycloak.quarkus.runtime.cli.command.Start; @@ -75,6 +77,15 @@ public class KeycloakMain implements QuarkusApplication { if (isDevProfileNotAllowed()) { errorHandler.error(errStream, Messages.devProfileNotAllowedError(Start.NAME), null); + System.exit(ExitCode.USAGE); + return; + } + + try { + Picocli.validateNonCliConfig(cliArgs, new Start(), new PrintWriter(System.out, true)); + } catch (NonCliPropertyException e) { + errorHandler.error(errStream, e.getMessage(), null); + System.exit(ExitCode.USAGE); return; } diff --git a/quarkus/runtime/src/main/java/org/keycloak/quarkus/runtime/cli/ExecutionExceptionHandler.java b/quarkus/runtime/src/main/java/org/keycloak/quarkus/runtime/cli/ExecutionExceptionHandler.java index ad45491df9..e2e83b6fc1 100644 --- a/quarkus/runtime/src/main/java/org/keycloak/quarkus/runtime/cli/ExecutionExceptionHandler.java +++ b/quarkus/runtime/src/main/java/org/keycloak/quarkus/runtime/cli/ExecutionExceptionHandler.java @@ -42,6 +42,11 @@ public final class ExecutionExceptionHandler implements CommandLine.IExecutionEx @Override public int handleExecutionException(Exception cause, CommandLine cmd, ParseResult parseResult) { + if (cause instanceof NonCliPropertyException) { + PrintWriter writer = cmd.getErr(); + writer.println(cmd.getColorScheme().errorText(cause.getMessage())); + return ShortErrorMessageHandler.getInvalidInputExitCode(cause, cmd); + } error(cmd.getErr(), "Failed to run '" + parseResult.subcommands().stream() .map(ParseResult::commandSpec) .map(CommandLine.Model.CommandSpec::name) diff --git a/quarkus/runtime/src/main/java/org/keycloak/quarkus/runtime/cli/NonCliPropertyException.java b/quarkus/runtime/src/main/java/org/keycloak/quarkus/runtime/cli/NonCliPropertyException.java new file mode 100644 index 0000000000..02e3c9604f --- /dev/null +++ b/quarkus/runtime/src/main/java/org/keycloak/quarkus/runtime/cli/NonCliPropertyException.java @@ -0,0 +1,26 @@ +/* + * 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.quarkus.runtime.cli; + +public class NonCliPropertyException extends RuntimeException { + + public NonCliPropertyException(String message) { + super(message); + } + +} 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 6e08b5ed09..eef3e96c3c 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 @@ -69,7 +69,9 @@ import org.keycloak.quarkus.runtime.configuration.mappers.PropertyMapper; import org.keycloak.quarkus.runtime.Environment; import io.smallrye.config.ConfigValue; + import picocli.CommandLine; +import picocli.CommandLine.ParameterException; import picocli.CommandLine.Help.Ansi; import picocli.CommandLine.Model.CommandSpec; import picocli.CommandLine.Model.OptionSpec; @@ -82,19 +84,38 @@ public final class Picocli { public static final String NO_PARAM_LABEL = "none"; private static final String ARG_KEY_VALUE_SEPARATOR = "="; + private static class IncludeOptions { + boolean includeRuntime; + boolean includeBuildTime; + } + private Picocli() { } public static void parseAndRun(List cliArgs) { CommandLine cmd = createCommandLine(cliArgs); + String[] argArray = cliArgs.toArray(new String[0]); if (Environment.isRebuildCheck()) { - int exitCode = runReAugmentationIfNeeded(cliArgs, cmd); + int exitCode = 0; + try { + // process the cli args first to init the config file and perform validation + cmd.parseArgs(argArray); + exitCode = runReAugmentationIfNeeded(cliArgs, cmd); + } catch (ParameterException ex) { + try { + exitCode = cmd.getParameterExceptionHandler().handleParseException(ex, argArray); + } catch (Exception e) { + ExecutionExceptionHandler errorHandler = new ExecutionExceptionHandler(); + errorHandler.error(cmd.getErr(), e.getMessage(), null); + exitCode = ex.getCommandLine().getCommandSpec().exitCodeOnInvalidInput(); + } + } exitOnFailure(exitCode, cmd); return; } - int exitCode = cmd.execute(cliArgs.toArray(new String[0])); + int exitCode = cmd.execute(argArray); exitOnFailure(exitCode, cmd); } @@ -228,6 +249,74 @@ public final class Picocli { return false; } + /** + * validate the expected values of non-cli properties + * + * @param cliArgs + * @param abstractCommand + */ + public static void validateNonCliConfig(List cliArgs, AbstractCommand abstractCommand, PrintWriter out) { + IncludeOptions options = getIncludeOptions(cliArgs, abstractCommand, abstractCommand.getName()); + + if (!options.includeBuildTime && !options.includeRuntime) { + return; + } + + List ignoredBuildTime = new ArrayList<>(); + List ignoredRunTime = new ArrayList<>(); + for (OptionCategory category : abstractCommand.getOptionCategories()) { + List mappers = new ArrayList<>(); + Optional.ofNullable(PropertyMappers.getRuntimeMappers().get(category)).ifPresent(mappers::addAll); + Optional.ofNullable(PropertyMappers.getBuildTimeMappers().get(category)).ifPresent(mappers::addAll); + for (PropertyMapper mapper : mappers) { + // bypass the PropertyMappingInterceptor - the transformations may cause unexpected errors + String value = null; + ConfigSource configSource = null; + for (ConfigSource cs : getConfig().getConfigSources()) { + if (cs.getOrdinal() < 300) { + break; // don't consider anything below standard env properties + } + value = cs.getValue(mapper.getFrom()); + if (value != null) { + configSource = cs; + break; + } + } + + if (value == null) { + continue; + } + + if (mapper.isBuildTime() && !options.includeBuildTime) { + ignoredBuildTime.add(mapper.getFrom()); + continue; + } + if (mapper.isRunTime() && !options.includeRuntime) { + ignoredRunTime.add(mapper.getFrom()); + continue; + } + + if (!PropertyMapperParameterConsumer.isExpectedValue(mapper.getExpectedValues(), value)) { + throw new NonCliPropertyException(PropertyMapperParameterConsumer.getErrorMessage(mapper.getFrom(), + value, mapper.getExpectedValues(), mapper.getExpectedValues()) + ". From ConfigSource " + configSource.getName()); + } + } + } + + if (!ignoredBuildTime.isEmpty()) { + outputIgnoredProperties(ignoredBuildTime, true, out); + } else if (!ignoredRunTime.isEmpty()) { + outputIgnoredProperties(ignoredRunTime, false, out); + } + } + + private static void outputIgnoredProperties(List properties, boolean build, PrintWriter out) { + out.write(String.format("The following %s time non-cli properties were found, but will be ignored during %s time: %s\n", + build ? "build" : "run", build ? "run" : "build", + properties.stream().collect(Collectors.joining(", ")))); + out.flush(); + } + private static boolean hasConfigChanges(CommandLine cmdCommand) { Optional currentProfile = ofNullable(Environment.getProfile()); Optional persistedProfile = getBuildTimeProperty("kc.profile"); @@ -379,26 +468,33 @@ public final class Picocli { return cmd; } + private static IncludeOptions getIncludeOptions(List cliArgs, AbstractCommand abstractCommand, String commandName) { + IncludeOptions result = new IncludeOptions(); + if (abstractCommand == null) { + return result; + } + result.includeRuntime = abstractCommand.includeRuntime(); + result.includeBuildTime = abstractCommand.includeBuildTime(); + + if (!result.includeBuildTime && !result.includeRuntime) { + return result; + } else if (result.includeRuntime && !result.includeBuildTime && !ShowConfig.NAME.equals(commandName)) { + result.includeBuildTime = isRebuilt() || !cliArgs.contains(OPTIMIZED_BUILD_OPTION_LONG); + } else if (result.includeBuildTime && !result.includeRuntime) { + result.includeRuntime = isRebuildCheck(); + } + return result; + } + private static void addCommandOptions(List cliArgs, CommandLine command) { - if (command != null) { - boolean includeBuildTime = false; - boolean includeRuntime = false; + if (command != null && command.getCommand() instanceof AbstractCommand) { + IncludeOptions options = getIncludeOptions(cliArgs, command.getCommand(), command.getCommandName()); - if (command.getCommand() instanceof AbstractCommand) { - AbstractCommand abstractCommand = command.getCommand(); - includeRuntime = abstractCommand.includeRuntime(); - includeBuildTime = abstractCommand.includeBuildTime(); - } - - if (!includeBuildTime && !includeRuntime) { + if (!options.includeBuildTime && !options.includeRuntime) { return; - } else if (includeRuntime && !includeBuildTime && !ShowConfig.NAME.equals(command.getCommandName())) { - includeBuildTime = isRebuilt() || !cliArgs.contains(OPTIMIZED_BUILD_OPTION_LONG); - } else if (includeBuildTime && !includeRuntime) { - includeRuntime = isRebuildCheck(); } - addOptionsToCli(command, includeBuildTime, includeRuntime); + addOptionsToCli(command, options.includeBuildTime, options.includeRuntime); } } diff --git a/quarkus/runtime/src/main/java/org/keycloak/quarkus/runtime/cli/PropertyMapperParameterConsumer.java b/quarkus/runtime/src/main/java/org/keycloak/quarkus/runtime/cli/PropertyMapperParameterConsumer.java index 71a605fc26..3bc23460aa 100644 --- a/quarkus/runtime/src/main/java/org/keycloak/quarkus/runtime/cli/PropertyMapperParameterConsumer.java +++ b/quarkus/runtime/src/main/java/org/keycloak/quarkus/runtime/cli/PropertyMapperParameterConsumer.java @@ -19,7 +19,7 @@ package org.keycloak.quarkus.runtime.cli; import static org.keycloak.quarkus.runtime.cli.Picocli.ARG_PREFIX; -import java.util.List; +import java.util.Collection; import java.util.Stack; import java.util.stream.Collectors; import java.util.stream.StreamSupport; @@ -55,7 +55,7 @@ public final class PropertyMapperParameterConsumer implements CommandLine.IParam if (args.isEmpty() || !isOptionValue(args.peek())) { throw new ParameterException( - commandLine, "Missing required value for option '" + name + "' (" + argSpec.paramLabel() + ")." + getExpectedValuesMessage(argSpec, option)); + commandLine, "Missing required value for option '" + name + "' (" + argSpec.paramLabel() + ")." + getExpectedValuesMessage(argSpec.completionCandidates(), option.completionCandidates())); } // consumes the value @@ -63,28 +63,29 @@ public final class PropertyMapperParameterConsumer implements CommandLine.IParam if (!args.isEmpty() && isOptionValue(args.peek())) { throw new ParameterException( - commandLine, "Option '" + name + "' expects a single value (" + argSpec.paramLabel() + ")" + getExpectedValuesMessage(argSpec, option)); + commandLine, "Option '" + name + "' expects a single value (" + argSpec.paramLabel() + ")" + getExpectedValuesMessage(argSpec.completionCandidates(), option.completionCandidates())); } - if (isExpectedValue(option, value)) { + if (isExpectedValue(StreamSupport.stream(option.completionCandidates().spliterator(), false).collect(Collectors.toList()), value)) { return; } - throw new ParameterException( - commandLine, "Invalid value for option '" + name + "': " + value + "." + getExpectedValuesMessage(argSpec, option)); + throw new ParameterException(commandLine, getErrorMessage(name, value, argSpec.completionCandidates(), option.completionCandidates())); + } + + static String getErrorMessage(String name, String value, Iterable specCandidates, Iterable optionCandidates) { + return "Invalid value for option '" + name + "': " + value + "." + getExpectedValuesMessage(specCandidates, optionCandidates); } private boolean isOptionValue(String arg) { return !(arg.startsWith(ARG_PREFIX) || arg.startsWith(Picocli.ARG_SHORT_PREFIX)); } - private String getExpectedValuesMessage(ArgSpec argSpec, OptionSpec option) { - return option.completionCandidates().iterator().hasNext() ? " Expected values are: " + String.join(", ", argSpec.completionCandidates()) : ""; + static String getExpectedValuesMessage(Iterable specCandidates, Iterable optionCandidates) { + return optionCandidates.iterator().hasNext() ? " Expected values are: " + String.join(", ", specCandidates) : ""; } - private boolean isExpectedValue(OptionSpec option, String value) { - List expectedValues = StreamSupport.stream(option.completionCandidates().spliterator(), false).collect(Collectors.toList()); - + static boolean isExpectedValue(Collection expectedValues, String value) { if (expectedValues.isEmpty()) { // accept any return true; diff --git a/quarkus/runtime/src/main/java/org/keycloak/quarkus/runtime/cli/ShortErrorMessageHandler.java b/quarkus/runtime/src/main/java/org/keycloak/quarkus/runtime/cli/ShortErrorMessageHandler.java index 095ca7cc3a..a6a5c7da82 100644 --- a/quarkus/runtime/src/main/java/org/keycloak/quarkus/runtime/cli/ShortErrorMessageHandler.java +++ b/quarkus/runtime/src/main/java/org/keycloak/quarkus/runtime/cli/ShortErrorMessageHandler.java @@ -1,15 +1,24 @@ package org.keycloak.quarkus.runtime.cli; +import org.keycloak.quarkus.runtime.cli.command.AbstractCommand; +import org.keycloak.quarkus.runtime.cli.command.Start; +import org.keycloak.quarkus.runtime.configuration.mappers.PropertyMapper; +import org.keycloak.quarkus.runtime.configuration.mappers.PropertyMappers; + +import java.io.PrintWriter; +import java.util.stream.Stream; + import picocli.CommandLine; import picocli.CommandLine.IParameterExceptionHandler; +import picocli.CommandLine.Model.CommandSpec; import picocli.CommandLine.ParameterException; import picocli.CommandLine.UnmatchedArgumentException; -import picocli.CommandLine.Model.CommandSpec; -import java.io.PrintWriter; +import static org.keycloak.quarkus.runtime.cli.command.AbstractStartCommand.OPTIMIZED_BUILD_OPTION_LONG; public class ShortErrorMessageHandler implements IParameterExceptionHandler { + @Override public int handleParseException(ParameterException ex, String[] args) { CommandLine cmd = ex.getCommandLine(); PrintWriter writer = cmd.getErr(); @@ -20,12 +29,28 @@ public class ShortErrorMessageHandler implements IParameterExceptionHandler { String[] unmatched = getUnmatchedPartsByOptionSeparator(uae,"="); String original = uae.getUnmatched().get(0); - if (unmatched[0].equals(original)) { unmatched = getUnmatchedPartsByOptionSeparator(uae," "); } + + String cliKey = unmatched[0]; - errorMessage = "Unknown option: '" + unmatched[0] + "'"; + PropertyMapper mapper = PropertyMappers.getMapper(cliKey); + + if (mapper == null || !(cmd.getCommand() instanceof AbstractCommand)) { + errorMessage = "Unknown option: '" + cliKey + "'"; + } else { + AbstractCommand command = cmd.getCommand(); + if (!command.getOptionCategories().contains(mapper.getCategory())) { + errorMessage = "Option: '" + cliKey + "' not valid for command " + cmd.getCommandName(); + } else { + if (Stream.of(args).anyMatch(OPTIMIZED_BUILD_OPTION_LONG::equals) && mapper.isBuildTime() && Start.NAME.equals(cmd.getCommandName())) { + errorMessage = "Build time option: '" + cliKey + "' not usable with pre-built image and --optimized"; + } else { + errorMessage = (mapper.isRunTime()?"Run time":"Build time") + " option: '" + cliKey + "' not usable with " + cmd.getCommandName(); + } + } + } } writer.println(cmd.getColorScheme().errorText(errorMessage)); @@ -34,9 +59,13 @@ public class ShortErrorMessageHandler implements IParameterExceptionHandler { CommandSpec spec = cmd.getCommandSpec(); writer.printf("Try '%s --help' for more information on the available options.%n", spec.qualifiedName()); + return getInvalidInputExitCode(ex, cmd); + } + + static int getInvalidInputExitCode(Exception ex, CommandLine cmd) { return cmd.getExitCodeExceptionMapper() != null ? cmd.getExitCodeExceptionMapper().getExitCode(ex) - : spec.exitCodeOnInvalidInput(); + : cmd.getCommandSpec().exitCodeOnInvalidInput(); } private String[] getUnmatchedPartsByOptionSeparator(UnmatchedArgumentException uae, String separator) { diff --git a/quarkus/runtime/src/main/java/org/keycloak/quarkus/runtime/cli/command/AbstractCommand.java b/quarkus/runtime/src/main/java/org/keycloak/quarkus/runtime/cli/command/AbstractCommand.java index 89624881d3..5c0dc281a7 100644 --- a/quarkus/runtime/src/main/java/org/keycloak/quarkus/runtime/cli/command/AbstractCommand.java +++ b/quarkus/runtime/src/main/java/org/keycloak/quarkus/runtime/cli/command/AbstractCommand.java @@ -20,6 +20,9 @@ package org.keycloak.quarkus.runtime.cli.command; import static org.keycloak.quarkus.runtime.Messages.cliExecutionError; import org.keycloak.config.OptionCategory; +import org.keycloak.quarkus.runtime.cli.Picocli; +import org.keycloak.quarkus.runtime.configuration.ConfigArgsConfigSource; + import picocli.CommandLine; import picocli.CommandLine.Model.CommandSpec; import picocli.CommandLine.Spec; @@ -60,4 +63,10 @@ public abstract class AbstractCommand { public List getOptionCategories() { return Arrays.asList(OptionCategory.values()); } + + protected void validateNonCliConfig() { + Picocli.validateNonCliConfig(ConfigArgsConfigSource.getAllCliArgs(), this, spec.commandLine().getOut()); + } + + public abstract String getName(); } diff --git a/quarkus/runtime/src/main/java/org/keycloak/quarkus/runtime/cli/command/AbstractStartCommand.java b/quarkus/runtime/src/main/java/org/keycloak/quarkus/runtime/cli/command/AbstractStartCommand.java index 727ea7e894..7aeb6d11eb 100644 --- a/quarkus/runtime/src/main/java/org/keycloak/quarkus/runtime/cli/command/AbstractStartCommand.java +++ b/quarkus/runtime/src/main/java/org/keycloak/quarkus/runtime/cli/command/AbstractStartCommand.java @@ -29,6 +29,7 @@ public abstract class AbstractStartCommand extends AbstractCommand implements Ru public void run() { doBeforeRun(); CommandLine cmd = spec.commandLine(); + validateNonCliConfig(); KeycloakMain.start((ExecutionExceptionHandler) cmd.getExecutionExceptionHandler(), cmd.getErr(), cmd.getParseResult().originalArgs().toArray(new String[0])); } 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 741a9c9bfc..d876997c0b 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 @@ -72,6 +72,8 @@ public final class Build extends AbstractCommand implements Runnable { exitWithErrorIfDevProfileIsSetAndNotStartDev(); System.setProperty("quarkus.launch.rebuild", "true"); + validateNonCliConfig(); + println(spec.commandLine(), "Updating the configuration and installing your custom providers, if any. Please wait."); try { @@ -106,6 +108,7 @@ public final class Build extends AbstractCommand implements Runnable { return true; } + @Override public List getOptionCategories() { // all options should work for the build command, otherwise re-augmentation might fail due to unknown options return super.getOptionCategories(); @@ -137,4 +140,9 @@ public final class Build extends AbstractCommand implements Runnable { getHomePath().resolve("quarkus-artifact.properties").toFile().delete(); } } + + @Override + public String getName() { + return NAME; + } } diff --git a/quarkus/runtime/src/main/java/org/keycloak/quarkus/runtime/cli/command/Export.java b/quarkus/runtime/src/main/java/org/keycloak/quarkus/runtime/cli/command/Export.java index cfd051917d..9939da4fda 100644 --- a/quarkus/runtime/src/main/java/org/keycloak/quarkus/runtime/cli/command/Export.java +++ b/quarkus/runtime/src/main/java/org/keycloak/quarkus/runtime/cli/command/Export.java @@ -42,4 +42,9 @@ public final class Export extends AbstractExportImportCommand implements Runnabl optionCategory != OptionCategory.IMPORT).collect(Collectors.toList()); } + @Override + public String getName() { + return NAME; + } + } diff --git a/quarkus/runtime/src/main/java/org/keycloak/quarkus/runtime/cli/command/Import.java b/quarkus/runtime/src/main/java/org/keycloak/quarkus/runtime/cli/command/Import.java index d3a7650a6b..b0348de434 100644 --- a/quarkus/runtime/src/main/java/org/keycloak/quarkus/runtime/cli/command/Import.java +++ b/quarkus/runtime/src/main/java/org/keycloak/quarkus/runtime/cli/command/Import.java @@ -42,4 +42,9 @@ public final class Import extends AbstractExportImportCommand implements Runnabl optionCategory != OptionCategory.EXPORT).collect(Collectors.toList()); } + @Override + public String getName() { + return NAME; + } + } diff --git a/quarkus/runtime/src/main/java/org/keycloak/quarkus/runtime/cli/command/ShowConfig.java b/quarkus/runtime/src/main/java/org/keycloak/quarkus/runtime/cli/command/ShowConfig.java index 99e10bfefa..55352d6464 100644 --- a/quarkus/runtime/src/main/java/org/keycloak/quarkus/runtime/cli/command/ShowConfig.java +++ b/quarkus/runtime/src/main/java/org/keycloak/quarkus/runtime/cli/command/ShowConfig.java @@ -171,4 +171,9 @@ public final class ShowConfig extends AbstractCommand implements Runnable { || property.startsWith("%")) && !ignoredPropertyKeys.contains(property); } + + @Override + public String getName() { + return NAME; + } } diff --git a/quarkus/runtime/src/main/java/org/keycloak/quarkus/runtime/cli/command/Start.java b/quarkus/runtime/src/main/java/org/keycloak/quarkus/runtime/cli/command/Start.java index 27acdc2a93..691249feb3 100644 --- a/quarkus/runtime/src/main/java/org/keycloak/quarkus/runtime/cli/command/Start.java +++ b/quarkus/runtime/src/main/java/org/keycloak/quarkus/runtime/cli/command/Start.java @@ -73,6 +73,7 @@ public final class Start extends AbstractStartCommand implements Runnable { return Environment.isDevProfile(); } + @Override public List getOptionCategories() { return super.getOptionCategories().stream().filter(optionCategory -> optionCategory != OptionCategory.EXPORT && optionCategory != OptionCategory.IMPORT).collect(Collectors.toList()); } @@ -81,4 +82,9 @@ public final class Start extends AbstractStartCommand implements Runnable { public boolean includeRuntime() { return true; } + + @Override + public String getName() { + return NAME; + } } diff --git a/quarkus/runtime/src/main/java/org/keycloak/quarkus/runtime/cli/command/StartDev.java b/quarkus/runtime/src/main/java/org/keycloak/quarkus/runtime/cli/command/StartDev.java index e97e04c78a..c9f42c8dce 100644 --- a/quarkus/runtime/src/main/java/org/keycloak/quarkus/runtime/cli/command/StartDev.java +++ b/quarkus/runtime/src/main/java/org/keycloak/quarkus/runtime/cli/command/StartDev.java @@ -58,4 +58,9 @@ public final class StartDev extends AbstractStartCommand implements Runnable { public boolean includeRuntime() { return true; } + + @Override + public String getName() { + return NAME; + } } 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 2664a1e550..36b5b95b7e 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 @@ -20,6 +20,7 @@ package org.keycloak.it.cli.dist; import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertTrue; import static org.keycloak.quarkus.runtime.cli.command.AbstractStartCommand.OPTIMIZED_BUILD_OPTION_LONG; +import static org.keycloak.quarkus.runtime.cli.command.Main.CONFIG_FILE_LONG_NAME; import org.junit.jupiter.api.Test; import org.keycloak.config.database.Database; @@ -30,8 +31,11 @@ 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.junit5.extension.WithEnvVars; import org.keycloak.it.utils.KeycloakDistribution; +import java.nio.file.Paths; + @DistributionTest class BuildCommandDistTest { @@ -64,7 +68,22 @@ class BuildCommandDistTest { @Launch({ "build", "--db=postgres", "--db-username=myuser", "--db-password=mypassword", "--http-enabled=true" }) void testFailRuntimeOptions(LaunchResult result) { CLIResult cliResult = (CLIResult) result; - cliResult.assertError("Unknown option: '--db-username'"); + cliResult.assertError("Run time option: '--db-username' not usable with build"); + } + + @Test + @WithEnvVars({"KC_DB", "invalid"}) + @Launch({ "build" }) + void testFailInvalidOptionInEnv(LaunchResult result) { + CLIResult cliResult = (CLIResult) result; + cliResult.assertError("Invalid value for option 'kc.db': invalid. Expected values are: dev-file, dev-mem, mariadb, mssql, mysql, oracle, postgres. From ConfigSource KcEnvVarConfigSource"); + } + + @Test + @RawDistOnly(reason = "Raw is enough and we avoid issues with including custom conf file in the container") + public void testFailInvalidOptionInConf(KeycloakDistribution distribution) { + CLIResult cliResult = distribution.run(CONFIG_FILE_LONG_NAME + "=" + Paths.get("src/test/resources/BuildCommandDistTest/keycloak.conf").toAbsolutePath().normalize(), "build"); + cliResult.assertError("Invalid value for option 'kc.db': foo. Expected values are: dev-file, dev-mem, mariadb, mssql, mysql, oracle, postgres. From ConfigSource PropertiesConfigSource[source"); } @Test diff --git a/quarkus/tests/integration/src/test/java/org/keycloak/it/cli/dist/LoggingDistTest.java b/quarkus/tests/integration/src/test/java/org/keycloak/it/cli/dist/LoggingDistTest.java index 4f78cbdb28..c1e11256d9 100644 --- a/quarkus/tests/integration/src/test/java/org/keycloak/it/cli/dist/LoggingDistTest.java +++ b/quarkus/tests/integration/src/test/java/org/keycloak/it/cli/dist/LoggingDistTest.java @@ -144,14 +144,14 @@ public class LoggingDistTest { void failUnknownHandlersInConfFile(KeycloakDistribution dist) { dist.copyOrReplaceFileFromClasspath("/logging/keycloak.conf", Paths.get("conf", "keycloak.conf")); CLIResult cliResult = dist.run("start-dev"); - cliResult.assertMessage("Invalid values in list for key: log Values: foo,console. Possible values are a combination of: console,file,gelf"); + cliResult.assertError("Invalid value for option 'kc.log': foo,console. Expected values are: console, file, gelf."); } @Test void failEmptyLogErrorFromConfFileError(KeycloakDistribution dist) { dist.copyOrReplaceFileFromClasspath("/logging/emptylog.conf", Paths.get("conf", "emptylog.conf")); CLIResult cliResult = dist.run(CONFIG_FILE_LONG_NAME+"=../conf/emptylog.conf", "start-dev"); - cliResult.assertMessage("Value for configuration key 'log' is empty."); + cliResult.assertError("Invalid value for option 'kc.log': . Expected values are: console, file, gelf."); } @Test 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 498b3d8e8f..e264f577f1 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,6 +31,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.junit5.extension.WithEnvVars; import org.keycloak.it.utils.KeycloakDistribution; @DistributionTest @@ -61,7 +62,7 @@ public class StartCommandDistTest { @Launch({ "-v", "start", "--db=dev-mem", OPTIMIZED_BUILD_OPTION_LONG}) void failBuildPropertyNotAvailable(LaunchResult result) { CLIResult cliResult = (CLIResult) result; - cliResult.assertError("Unknown option: '--db'"); + cliResult.assertError("Build time option: '--db' not usable with pre-built image and --optimized"); } @Test @@ -97,7 +98,15 @@ public class StartCommandDistTest { @Launch({ "start", "--optimized", "--http-enabled=true", "--hostname-strict=false", "--cache=local" }) void testStartUsingOptimizedDoesNotAllowBuildOptions(LaunchResult result) { CLIResult cliResult = (CLIResult) result; - cliResult.assertError("Unknown option: '--cache'"); + cliResult.assertError("Build time option: '--cache' not usable with pre-built image and --optimized"); + } + + @Test + @WithEnvVars({"KC_LOG", "invalid"}) + @Launch({ "start", "--optimized" }) + void testStartUsingOptimizedInvalidEnvOption(LaunchResult result) { + CLIResult cliResult = (CLIResult) result; + cliResult.assertError("Invalid value for option 'kc.log': invalid. Expected values are: console, file, gelf. From ConfigSource KcEnvVarConfigSource"); } @Test diff --git a/quarkus/tests/integration/src/test/resources/BuildCommandDistTest/keycloak.conf b/quarkus/tests/integration/src/test/resources/BuildCommandDistTest/keycloak.conf new file mode 100644 index 0000000000..9a6c647a61 --- /dev/null +++ b/quarkus/tests/integration/src/test/resources/BuildCommandDistTest/keycloak.conf @@ -0,0 +1 @@ +db=foo \ No newline at end of file diff --git a/quarkus/tests/junit5/src/main/java/org/keycloak/it/utils/DockerKeycloakDistribution.java b/quarkus/tests/junit5/src/main/java/org/keycloak/it/utils/DockerKeycloakDistribution.java index 9e3e9ca5cb..7f439521b4 100644 --- a/quarkus/tests/junit5/src/main/java/org/keycloak/it/utils/DockerKeycloakDistribution.java +++ b/quarkus/tests/junit5/src/main/java/org/keycloak/it/utils/DockerKeycloakDistribution.java @@ -17,7 +17,9 @@ import org.testcontainers.utility.LazyFuture; import java.io.File; import java.lang.reflect.Field; import java.time.Duration; +import java.util.HashMap; import java.util.List; +import java.util.Map; import java.util.concurrent.Executor; import java.util.concurrent.Executors; @@ -38,12 +40,18 @@ public final class DockerKeycloakDistribution implements KeycloakDistribution { private String containerId = null; private Executor parallelReaperExecutor = Executors.newSingleThreadExecutor(); + private Map envVars = new HashMap<>(); public DockerKeycloakDistribution(boolean debug, boolean manualStop, boolean reCreate) { this.debug = debug; this.manualStop = manualStop; } + @Override + public void setEnvVar(String name, String value) { + this.envVars.put(name, value); + } + private GenericContainer getKeycloakContainer() { File distributionFile = new File("../../dist/" + File.separator + "target" + File.separator + "keycloak-" + Version.VERSION + ".tar.gz"); @@ -70,6 +78,7 @@ public final class DockerKeycloakDistribution implements KeycloakDistribution { } return new GenericContainer(image) + .withEnv(envVars) .withExposedPorts(8080) .withStartupAttempts(1) .withStartupTimeout(Duration.ofSeconds(120)) @@ -102,6 +111,10 @@ public final class DockerKeycloakDistribution implements KeycloakDistribution { cleanupContainer(); keycloakContainer = null; LOGGER.warn("Failed to start Keycloak container", cause); + } finally { + if (!manualStop) { + envVars.clear(); + } } trySetRestAssuredPort(); diff --git a/testsuite/integration-arquillian/servers/auth-server/quarkus/src/main/content/conf/keycloak.conf b/testsuite/integration-arquillian/servers/auth-server/quarkus/src/main/content/conf/keycloak.conf index 596b3efbf6..f9c9ab6a68 100644 --- a/testsuite/integration-arquillian/servers/auth-server/quarkus/src/main/content/conf/keycloak.conf +++ b/testsuite/integration-arquillian/servers/auth-server/quarkus/src/main/content/conf/keycloak.conf @@ -15,7 +15,7 @@ https-key-store-file=${kc.home.dir}/conf/keycloak.jks https-key-store-password=secret https-trust-store-file=${kc.home.dir}/conf/keycloak.truststore https-trust-store-password=secret -https-client-auth=REQUEST +https-client-auth=request # Proxy # Using any proxy setting which evaluates the forward proxy header