fix: provide better error messages for list options (#25918)

closes: #25235

Signed-off-by: Steve Hawkins <shawkins@redhat.com>
This commit is contained in:
Steven Hawkins 2024-01-15 12:33:49 -05:00 committed by GitHub
parent b1e3caebba
commit fe39d1b5eb
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
18 changed files with 184 additions and 214 deletions

View file

@ -19,7 +19,7 @@ public class DatabaseOptions {
.category(OptionCategory.DATABASE)
.description("The database vendor.")
.defaultValue("dev-file")
.expectedValues(Database::getDatabaseAliases)
.expectedValues(Database.getDatabaseAliases())
.buildTime(true)
.build();

View file

@ -10,18 +10,18 @@ import java.util.stream.Collectors;
public class FeatureOptions {
public static final Option<List> FEATURES = new OptionBuilder("features", List.class, Profile.Feature.class)
public static final Option<List<String>> FEATURES = OptionBuilder.listOptionBuilder("features", String.class)
.category(OptionCategory.FEATURE)
.description("Enables a set of one or more features.")
.defaultValue(Optional.empty())
.expectedValues(() -> getFeatureValues(true))
.expectedValues(getFeatureValues(true))
.buildTime(true)
.build();
public static final Option<List> FEATURES_DISABLED = new OptionBuilder("features-disabled", List.class, Profile.Feature.class)
public static final Option<List<String>> FEATURES_DISABLED = OptionBuilder.listOptionBuilder("features-disabled", String.class)
.category(OptionCategory.FEATURE)
.description("Disables a set of one or more features.")
.expectedValues(() -> getFeatureValues(false))
.expectedValues(getFeatureValues(false))
.buildTime(true)
.build();

View file

@ -1,6 +1,9 @@
package org.keycloak.config;
import java.io.File;
import java.util.Arrays;
import java.util.List;
import org.keycloak.common.crypto.FipsMode;
public class HttpOptions {
@ -54,10 +57,10 @@ public class HttpOptions {
.description("The cipher suites to use. If none is given, a reasonable default is selected.")
.build();
public static final Option<String> HTTPS_PROTOCOLS = new OptionBuilder<>("https-protocols", String.class)
public static final Option<List<String>> HTTPS_PROTOCOLS = OptionBuilder.listOptionBuilder("https-protocols", String.class)
.category(OptionCategory.HTTP)
.description("The list of protocols to explicitly enable.")
.defaultValue("TLSv1.3,TLSv1.2")
.defaultValue(Arrays.asList("TLSv1.3,TLSv1.2"))
.build();
public static final Option<File> HTTPS_CERTIFICATE_FILE = new OptionBuilder<>("https-certificate-file", File.class)

View file

@ -30,11 +30,11 @@ public class LoggingOptions {
.toList();
}
public static final Option<List> LOG = new OptionBuilder("log", List.class, Handler.class)
public static final Option<List<Handler>> LOG = OptionBuilder.listOptionBuilder("log", Handler.class)
.category(OptionCategory.LOGGING)
.description("Enable one or more log handlers in a comma-separated list.")
.expectedValues(() -> getAvailableHandlerNames())
.defaultValue(DEFAULT_LOG_HANDLER)
.expectedValues(getAvailableHandlerNames())
.defaultValue(Arrays.asList(DEFAULT_LOG_HANDLER))
.build();
public enum Level {
@ -53,9 +53,9 @@ public class LoggingOptions {
}
}
public static final Option<String> LOG_LEVEL = new OptionBuilder<>("log-level", String.class)
public static final Option<List<String>> LOG_LEVEL = OptionBuilder.listOptionBuilder("log-level", String.class)
.category(OptionCategory.LOGGING)
.defaultValue(DEFAULT_LOG_LEVEL.toString())
.defaultValue(Arrays.asList(DEFAULT_LOG_LEVEL.toString()))
.description("The log level of the root category or a comma-separated list of individual categories and their levels. For the root category, you don't need to specify a category.")
.build();

View file

@ -1,19 +0,0 @@
package org.keycloak.config;
import java.util.List;
import java.util.Optional;
import java.util.function.Supplier;
public class MultiOption<T> extends Option<T> {
private final Class auxiliaryType;
public MultiOption(Class type, Class auxiliaryType, String key, OptionCategory category, boolean hidden, boolean buildTime, String description, Optional defaultValue, Supplier<List<String>> expectedValues, DeprecatedMetadata deprecatedMetadata) {
super(type, key, category, hidden, buildTime, description, defaultValue, expectedValues, deprecatedMetadata);
this.auxiliaryType = auxiliaryType;
}
public Class<?> getAuxiliaryType() {
return auxiliaryType;
}
}

View file

@ -2,7 +2,7 @@ package org.keycloak.config;
import java.util.List;
import java.util.Optional;
import java.util.function.Supplier;
import java.util.stream.Collectors;
public class Option<T> {
@ -13,10 +13,10 @@ public class Option<T> {
private final boolean buildTime;
private final String description;
private final Optional<T> defaultValue;
private final Supplier<List<String>> expectedValues;
private final List<String> expectedValues;
private final DeprecatedMetadata deprecatedMetadata;
public Option(Class<T> type, String key, OptionCategory category, boolean hidden, boolean buildTime, String description, Optional<T> defaultValue, Supplier<List<String>> expectedValues, DeprecatedMetadata deprecatedMetadata) {
public Option(Class<T> type, String key, OptionCategory category, boolean hidden, boolean buildTime, String description, Optional<T> defaultValue, List<String> expectedValues, DeprecatedMetadata deprecatedMetadata) {
this.type = type;
this.key = key;
this.category = category;
@ -53,7 +53,7 @@ public class Option<T> {
}
public List<String> getExpectedValues() {
return expectedValues.get();
return expectedValues;
}
public Optional<DeprecatedMetadata> getDeprecatedMetadata() {
@ -90,4 +90,14 @@ public class Option<T> {
return description;
}
public static String getDefaultValueString(Object value) {
if (value == null) {
return null;
}
if (value instanceof List) {
return ((List<?>) value).stream().map(String::valueOf).collect(Collectors.joining(","));
}
return String.valueOf(value);
}
}

View file

@ -1,56 +1,54 @@
package org.keycloak.config;
import java.util.Collection;
import java.util.List;
import java.util.Map;
import java.util.Optional;
import java.util.Set;
import java.util.function.Supplier;
import java.util.stream.Collectors;
public class OptionBuilder<T> {
private static final Supplier<List<String>> EMPTY_VALUES_SUPPLIER = List::of;
private static final Supplier<List<String>> BOOLEAN_TYPE_VALUES = new Supplier<List<String>>() {
List<String> values = List.of(Boolean.TRUE.toString(), Boolean.FALSE.toString());
@Override
public List<String> get() {
return values;
}
};
private static final List<String> BOOLEAN_TYPE_VALUES = List.of(Boolean.TRUE.toString(), Boolean.FALSE.toString());
private final Class<T> type;
private final Class<T> auxiliaryType;
private final String key;
private OptionCategory category;
private boolean hidden;
private boolean build;
private String description;
private Optional<T> defaultValue;
private Supplier<List<String>> expectedValues;
private List<String> expectedValues = List.of();
private DeprecatedMetadata deprecatedMetadata;
public static <A> OptionBuilder<List<A>> listOptionBuilder(String key, Class<A> type) {
return new OptionBuilder(key, List.class, type);
}
public OptionBuilder(String key, Class<T> type) {
this(key, type, null);
}
public OptionBuilder(String key, Class<T> type, Class<T> auxiliaryType) {
private OptionBuilder(String key, Class<T> type, Class<?> auxiliaryType) {
this.type = type;
this.auxiliaryType = auxiliaryType;
if (type.isArray() || ((Collection.class.isAssignableFrom(type) || Map.class.isAssignableFrom(type)) && type != java.util.List.class)) {
throw new IllegalArgumentException("Non-List multi-valued options are not yet supported");
}
this.key = key;
category = OptionCategory.GENERAL;
hidden = false;
build = false;
description = null;
defaultValue = Boolean.class.equals(type) ? Optional.of((T) Boolean.FALSE) : Optional.empty();
expectedValues = EMPTY_VALUES_SUPPLIER;
if (Boolean.class.equals(type)) {
Class<?> expected = type;
if (auxiliaryType != null) {
expected = auxiliaryType;
}
defaultValue = Boolean.class.equals(expected) ? Optional.of((T) Boolean.FALSE) : Optional.empty();
if (Boolean.class.equals(expected)) {
expectedValues(BOOLEAN_TYPE_VALUES);
}
if (Enum.class.isAssignableFrom(type)) {
expectedValues((Class<? extends Enum>) type);
}
if (auxiliaryType != null && Enum.class.isAssignableFrom(auxiliaryType)) {
expectedValues((Class<? extends Enum>) auxiliaryType);
if (Enum.class.isAssignableFrom(expected)) {
expectedValues((Class<? extends Enum>) expected);
}
}
@ -84,32 +82,18 @@ public class OptionBuilder<T> {
return this;
}
public OptionBuilder<T> expectedValues(Supplier<List<String>> expected) {
public OptionBuilder<T> expectedValues(List<String> expected) {
this.expectedValues = expected;
return this;
}
public OptionBuilder<T> expectedValues(Class<? extends Enum> expected) {
this.expectedValues = new Supplier<>() {
List<String> values = List.of(expected.getEnumConstants()).stream().map(Object::toString).collect(Collectors.toList());
@Override
public List<String> get() {
return values;
}
};
this.expectedValues = List.of(expected.getEnumConstants()).stream().map(Object::toString).collect(Collectors.toList());
return this;
}
public OptionBuilder<T> expectedValues(T ... expected) {
this.expectedValues = new Supplier<>() {
List<String> values = List.of(expected).stream().map(v -> v.toString()).collect(Collectors.toList());
@Override
public List<String> get() {
return values;
}
};
this.expectedValues = List.of(expected).stream().map(v -> v.toString()).collect(Collectors.toList());
return this;
}
@ -135,11 +119,7 @@ public class OptionBuilder<T> {
public Option<T> build() {
if (auxiliaryType != null) {
return new MultiOption<T>(type, auxiliaryType, key, category, hidden, build, description, defaultValue, expectedValues, deprecatedMetadata);
} else {
return new Option<T>(type, key, category, hidden, build, description, defaultValue, expectedValues, deprecatedMetadata);
}
}
}

View file

@ -10,7 +10,7 @@ public class SecurityOptions {
public static final Option<FipsMode> FIPS_MODE = new OptionBuilder<>("fips-mode", FipsMode.class)
.category(OptionCategory.SECURITY)
.expectedValues(SecurityOptions::getFipsModeValues)
.expectedValues(getFipsModeValues())
.buildTime(true)
.description("Sets the FIPS mode. If '" + FipsMode.NON_STRICT + "' is set, FIPS is enabled but on non-approved mode. For full FIPS compliance, set '" + FipsMode.STRICT + "' to run on approved mode. "
+ "This option defaults to '" + FipsMode.DISABLED + "' when '" + Profile.Feature.FIPS.getKey() + "' feature is disabled, which is by default. "

View file

@ -2,9 +2,11 @@ package org.keycloak.config;
import org.keycloak.common.enums.HostnameVerificationPolicy;
import java.util.List;
public class TruststoreOptions {
public static final Option<String> TRUSTSTORE_PATHS = new OptionBuilder<>("truststore-paths", String.class)
public static final Option<List<String>> TRUSTSTORE_PATHS = OptionBuilder.listOptionBuilder("truststore-paths", String.class)
.category(OptionCategory.TRUSTSTORE)
.description("List of pkcs12 (p12 or pfx file extensions), PEM files, or directories containing those files that will be used as a system truststore.")
.build();

View file

@ -56,7 +56,7 @@ import java.util.stream.Collectors;
import org.eclipse.microprofile.config.spi.ConfigSource;
import org.jboss.logging.Logger;
import org.keycloak.config.DeprecatedMetadata;
import org.keycloak.config.MultiOption;
import org.keycloak.config.Option;
import org.keycloak.config.OptionCategory;
import org.keycloak.quarkus.runtime.cli.command.AbstractCommand;
import org.keycloak.quarkus.runtime.cli.command.Build;
@ -399,7 +399,7 @@ public final class Picocli {
if (runtimeValue == null && isNotBlank(persistedValue)) {
PropertyMapper<?> mapper = PropertyMappers.getMapper(propertyName);
if (mapper != null && persistedValue.equals(mapper.getDefaultValue().map(Object::toString).orElse(null))) {
if (mapper != null && persistedValue.equals(Option.getDefaultValueString(mapper.getDefaultValue().orElse(null)))) {
// same as default
continue;
}
@ -608,14 +608,11 @@ public final class Picocli {
.hidden(mapper.isHidden());
if (mapper.getDefaultValue().isPresent()) {
optBuilder.defaultValue(mapper.getDefaultValue().get().toString());
optBuilder.defaultValue(Option.getDefaultValueString(mapper.getDefaultValue().get()));
}
if (mapper.getType() != null) {
optBuilder.type(mapper.getType());
if (mapper.getOption() instanceof MultiOption) {
optBuilder.auxiliaryTypes(((MultiOption<?>) mapper.getOption()).getAuxiliaryType());
}
} else {
optBuilder.type(String.class);
}
@ -639,7 +636,7 @@ public final class Picocli {
}
mapper.getDefaultValue()
.map(d -> d.toString().replaceAll("%", "%%")) // escape formats
.map(d -> Option.getDefaultValueString(d).replaceAll("%", "%%")) // escape formats
.map(d -> " Default: " + d + ".")
.ifPresent(transformedDesc::append);

View file

@ -19,11 +19,8 @@ package org.keycloak.quarkus.runtime.cli;
import static org.keycloak.quarkus.runtime.cli.Picocli.ARG_PREFIX;
import java.util.Collection;
import java.util.Stack;
import org.keycloak.utils.StringUtil;
import picocli.CommandLine;
import picocli.CommandLine.Model.ArgSpec;
import picocli.CommandLine.Model.CommandSpec;
@ -52,21 +49,27 @@ public final class PropertyMapperParameterConsumer implements CommandLine.IParam
CommandLine commandLine = commandSpec.commandLine();
if (args.isEmpty() || !isOptionValue(args.peek())) {
throw new ParameterException(
commandLine, "Missing required value for option '" + name + "' (" + argSpec.paramLabel() + ")." + getExpectedValuesMessage(argSpec.completionCandidates(), option.completionCandidates()));
throw new ParameterException(commandLine,
"Missing required value. " + getExpectedMessage(argSpec, option, name));
}
// consumes the value, actual value validation will be performed later
args.pop();
if (!args.isEmpty() && isOptionValue(args.peek())) {
throw new ParameterException(
commandLine, "Option '" + name + "' expects a single value (" + argSpec.paramLabel() + ")" + getExpectedValuesMessage(argSpec.completionCandidates(), option.completionCandidates()));
throw new ParameterException(commandLine, getExpectedMessage(argSpec, option, name));
}
}
public static String getErrorMessage(String name, String value, Iterable<String> specCandidates, Iterable<String> optionCandidates) {
return "Invalid value for option '" + name + "': " + value + "." + getExpectedValuesMessage(specCandidates, optionCandidates);
private String getExpectedMessage(ArgSpec argSpec, OptionSpec option, String name) {
return String.format("Option '%s' (%s) expects %s.%s", name, argSpec.paramLabel(),
option.typeInfo().isMultiValue() ? "one or more comma separated values without whitespace": "a single value",
getExpectedValuesMessage(argSpec.completionCandidates(), option.completionCandidates()));
}
public static String getErrorMessage(String name, String value, Iterable<String> expected) {
return String.format("Invalid value for option '%s': %s.%s", name, value,
getExpectedValuesMessage(expected, expected));
}
private boolean isOptionValue(String arg) {
@ -77,22 +80,4 @@ public final class PropertyMapperParameterConsumer implements CommandLine.IParam
return optionCandidates.iterator().hasNext() ? " Expected values are: " + String.join(", ", specCandidates) : "";
}
public static boolean isExpectedValue(Collection<String> expectedValues, String value) {
if (expectedValues.isEmpty()) {
// accept any
return true;
}
if (StringUtil.isBlank(value)) {
return false;
}
for (String v : value.split(",")) {
if (!expectedValues.contains(v)) {
return false;
}
}
return true;
}
}

View file

@ -9,7 +9,6 @@ import java.util.Set;
import java.util.regex.Matcher;
import java.util.regex.Pattern;
import java.util.stream.Collectors;
import java.util.stream.Stream;
import static org.keycloak.quarkus.runtime.configuration.mappers.PropertyMapper.fromOption;
@ -24,7 +23,8 @@ public final class FeaturePropertyMappers {
return new PropertyMapper[] {
fromOption(FeatureOptions.FEATURES)
.paramLabel("feature")
.validator((mapper, value) -> validateEnabledFeatures(value.getValue()))
.validator((mapper, value) -> mapper.validateExpectedValues(value,
(c, v) -> validateEnabledFeature(v)))
.build(),
fromOption(FeatureOptions.FEATURES_DISABLED)
.paramLabel("feature")
@ -32,8 +32,7 @@ public final class FeaturePropertyMappers {
};
}
public static void validateEnabledFeatures(String s) {
Stream.of(s.split(",")).forEach(feature -> {
public static void validateEnabledFeature(String feature) {
if (!Profile.getFeatureVersions(feature).isEmpty()) {
return;
}
@ -57,11 +56,10 @@ public final class FeaturePropertyMappers {
feature, FeatureOptions.getFeatureValues(false)));
}
int version = Integer.parseInt(matcher.group(2));
if (featureVersions.stream().noneMatch(f -> f.getVersion() == version)) {
if (!featureVersions.stream().anyMatch(f -> f.getVersion() == version)) {
throw new PropertyException(
String.format("%s has an unrecognized feature version, it should be one of %s", feature,
featureVersions.stream().map(Feature::getVersion).map(String::valueOf).collect(Collectors.toList())));
}
});
}
}

View file

@ -24,7 +24,6 @@ import static org.keycloak.quarkus.runtime.configuration.Configuration.toCliForm
import static org.keycloak.quarkus.runtime.configuration.Configuration.toEnvVarFormat;
import java.util.List;
import java.util.Objects;
import java.util.Optional;
import java.util.function.BiConsumer;
import java.util.function.BiFunction;
@ -32,7 +31,6 @@ import java.util.function.BiFunction;
import io.smallrye.config.ConfigSourceInterceptorContext;
import io.smallrye.config.ConfigValue;
import org.jboss.logging.Logger;
import org.keycloak.config.DeprecatedMetadata;
import org.keycloak.config.Option;
import org.keycloak.config.OptionBuilder;
@ -69,8 +67,6 @@ public class PropertyMapper<T> {
private String cliFormat;
private BiConsumer<PropertyMapper<T>, ConfigValue> validator;
private static final Logger logger = Logger.getLogger(PropertyMapper.class);
PropertyMapper(Option<T> option, String to, BiFunction<Optional<String>, ConfigSourceInterceptorContext, Optional<String>> mapper,
String mapFrom, String paramLabel, boolean mask, BiConsumer<PropertyMapper<T>, ConfigValue> validator) {
this.option = option;
@ -119,14 +115,14 @@ public class PropertyMapper<T> {
PropertyMapper<?> parentMapper = PropertyMappers.getMapper(parentKey);
if (parentMapper != null && parentMapper.getDefaultValue().isPresent()) {
parentValue = ConfigValue.builder().withValue(parentMapper.getDefaultValue().get().toString()).build();
parentValue = ConfigValue.builder().withValue(Option.getDefaultValueString(parentMapper.getDefaultValue().get())).build();
}
}
return transformValue(name, ofNullable(parentValue == null ? null : parentValue.getValue()), context, null);
}
ConfigValue defaultValue = transformValue(name, this.option.getDefaultValue().map(Objects::toString), context, null);
ConfigValue defaultValue = transformValue(name, this.option.getDefaultValue().map(Option::getDefaultValueString), context, null);
if (defaultValue != null) {
return defaultValue;
@ -242,7 +238,7 @@ public class PropertyMapper<T> {
private String mapFrom = null;
private boolean isMasked = false;
private String paramLabel;
private BiConsumer<PropertyMapper<T>, ConfigValue> validator = (mapper, value) -> mapper.validateExpectedValues(value);
private BiConsumer<PropertyMapper<T>, ConfigValue> validator = (mapper, value) -> mapper.validateExpectedValues(value, (c, v) -> mapper.validateSingleValue(c, v));
public Builder(Option<T> option) {
this.option = option;
@ -296,15 +292,39 @@ public class PropertyMapper<T> {
}
}
public void validateExpectedValues(ConfigValue value) {
if (PropertyMapperParameterConsumer.isExpectedValue(getExpectedValues(), value.getValue())) {
return;
public void validateExpectedValues(ConfigValue configValue, BiConsumer<ConfigValue, String> singleValidator) {
String value = configValue.getValue();
boolean multiValued = getOption().getType() == java.util.List.class;
String[] values = multiValued ? value.split(",") : new String[] { value };
for (String v : values) {
boolean cli = isCliOption(configValue);
if (multiValued && !v.trim().equals(v)) {
throw new PropertyException("Invalid value for multivalued option '" + (cli ? this.getCliFormat() : getFrom())
+ "': list value '" + v + "' should not have leading nor trailing whitespace"
+ getConfigSourceMessage(configValue, cli));
}
boolean cli = Optional.ofNullable(value.getConfigSourceName()).filter(name -> name.contains(ConfigArgsConfigSource.NAME)).isPresent();
singleValidator.accept(configValue, v);
}
}
private boolean isCliOption(ConfigValue configValue) {
return Optional.ofNullable(configValue.getConfigSourceName()).filter(name -> name.contains(ConfigArgsConfigSource.NAME)).isPresent();
}
void validateSingleValue(ConfigValue configValue, String v) {
List<String> expectedValues = getExpectedValues();
if (!expectedValues.isEmpty() && !expectedValues.contains(v)) {
boolean cli = isCliOption(configValue);
throw new PropertyException(
PropertyMapperParameterConsumer.getErrorMessage(cli ? this.getCliFormat() : getFrom(),
value.getValue(), getExpectedValues(), getExpectedValues())
+ (cli ? "" : ". From ConfigSource " + value.getConfigSourceName()));
PropertyMapperParameterConsumer.getErrorMessage(cli ? this.getCliFormat() : getFrom(), v,
expectedValues) + getConfigSourceMessage(configValue, cli));
}
}
String getConfigSourceMessage(ConfigValue configValue, boolean cli) {
return cli ? "" : ". From ConfigSource " + configValue.getConfigSourceName();
}
}

View file

@ -28,28 +28,28 @@ public class FeaturePropertyMappersTest {
@Test
public void testInvalidFeatureFormat() {
assertThrows(PropertyException.class, () -> FeaturePropertyMappers.validateEnabledFeatures("invalid:"));
assertThrows(PropertyException.class, () -> FeaturePropertyMappers.validateEnabledFeature("invalid:"));
}
@Test
public void testInvalidFeature() {
assertThrows(PropertyException.class, () -> FeaturePropertyMappers.validateEnabledFeatures("invalid"));
assertThrows(PropertyException.class, () -> FeaturePropertyMappers.validateEnabledFeature("invalid"));
}
@Test
public void testInvalidVersionedFeature() {
assertThrows(PropertyException.class, () -> FeaturePropertyMappers.validateEnabledFeatures("invalid:v1"));
assertThrows(PropertyException.class, () -> FeaturePropertyMappers.validateEnabledFeature("invalid:v1"));
}
@Test
public void testInvalidFeatureVersion() {
assertThrows(PropertyException.class, () -> FeaturePropertyMappers.validateEnabledFeatures(Feature.DOCKER.getUnversionedKey() + ":v0"));
assertThrows(PropertyException.class, () -> FeaturePropertyMappers.validateEnabledFeature(Feature.DOCKER.getUnversionedKey() + ":v0"));
}
@Test
public void testValidFeatures() {
FeaturePropertyMappers.validateEnabledFeatures(
Feature.DOCKER.getUnversionedKey() + "," + "preview" + "," + Feature.ACCOUNT2.getVersionedKey());
FeaturePropertyMappers.validateEnabledFeature("preview");
FeaturePropertyMappers.validateEnabledFeature(Feature.ACCOUNT2.getVersionedKey());
}
}

View file

@ -36,14 +36,28 @@ public class OptionValidationTest {
@Launch({"build", "--db"})
public void failMissingOptionValue(LaunchResult result) {
CLIResult cliResult = (CLIResult) result;
assertThat(cliResult.getErrorOutput(), containsString("Missing required value for option '--db' (vendor). Expected values are: dev-file, dev-mem, mariadb, mssql, mysql, oracle, postgres"));
assertThat(cliResult.getErrorOutput(), containsString("Missing required value. Option '--db' (vendor) expects a single value. Expected values are: dev-file, dev-mem, mariadb, mssql, mysql, oracle, postgres"));
}
@Test
@Launch({"build", "--db", "foo", "bar"})
public void failMultipleOptionValue(LaunchResult result) {
CLIResult cliResult = (CLIResult) result;
assertThat(cliResult.getErrorOutput(), containsString("Option '--db' expects a single value (vendor) Expected values are: dev-file, dev-mem, mariadb, mssql, mysql, oracle, postgres"));
assertThat(cliResult.getErrorOutput(), containsString("Option '--db' (vendor) expects a single value. Expected values are: dev-file, dev-mem, mariadb, mssql, mysql, oracle, postgres"));
}
@Test
@Launch({"build", "--features", "account2", "account3"})
public void failMultipleMultiOptionValue(LaunchResult result) {
CLIResult cliResult = (CLIResult) result;
assertThat(cliResult.getErrorOutput(), containsString("Option '--features' (feature) expects one or more comma separated values without whitespace. Expected values are: "));
}
@Test
@Launch({"build", "--features", "xyz,account2"})
public void failInvalidMultiOptionValue(LaunchResult result) {
CLIResult cliResult = (CLIResult) result;
assertThat(cliResult.getErrorOutput(), containsString("xyz is an unrecognized feature, it should be one of"));
}
@Test

View file

@ -145,7 +145,7 @@ public class LoggingDistTest {
void failUnknownHandlersInConfFile(KeycloakDistribution dist) {
dist.copyOrReplaceFileFromClasspath("/logging/keycloak.conf", Paths.get("conf", "keycloak.conf"));
CLIResult cliResult = dist.run("start-dev");
cliResult.assertError("Invalid value for option 'kc.log': foo,console. Expected values are: console, file, gelf.");
cliResult.assertError("Invalid value for option 'kc.log': foo. Expected values are: console, file, gelf.");
}
@Test
@ -159,7 +159,7 @@ public class LoggingDistTest {
@Launch({ "start-dev","--log=foo,bar" })
void failUnknownHandlersInCliCommand(LaunchResult result) {
CLIResult cliResult = (CLIResult) result;
cliResult.assertError("Invalid value for option '--log': foo,bar");
cliResult.assertError("Invalid value for option '--log': foo");
}
@Test

View file

@ -28,7 +28,6 @@ import java.util.concurrent.TimeoutException;
import org.eclipse.microprofile.config.spi.ConfigProviderResolver;
import org.keycloak.common.Version;
import org.keycloak.common.crypto.FipsMode;
import org.keycloak.config.DatabaseOptions;
import org.keycloak.config.HttpOptions;
import org.keycloak.config.LoggingOptions;
import org.keycloak.config.Option;
@ -117,7 +116,7 @@ public class Keycloak {
addOptionIfNotSet(args, HttpOptions.HTTP_PORT);
addOptionIfNotSet(args, HttpOptions.HTTPS_PORT);
boolean isFipsEnabled = ofNullable(getOptionValue(args, SecurityOptions.FIPS_MODE)).orElse(FipsMode.DISABLED).isFipsEnabled();
boolean isFipsEnabled = ofNullable(getOptionValue(args, SecurityOptions.FIPS_MODE)).map(FipsMode::valueOf).orElse(FipsMode.DISABLED).isFipsEnabled();
if (isFipsEnabled) {
String logLevel = getOptionValue(args, LoggingOptions.LOG_LEVEL);
@ -135,41 +134,22 @@ public class Keycloak {
}
private <T> void addOptionIfNotSet(List<String> args, Option<T> option, T defaultValue) {
T value = getOptionValue(args, option);
String value = getOptionValue(args, option);
if (value == null) {
defaultValue = ofNullable(defaultValue).orElseGet(option.getDefaultValue()::get);
args.add(Configuration.toCliFormat(option.getKey()) + "=" + defaultValue);
args.add(Configuration.toCliFormat(option.getKey()) + "=" + Option.getDefaultValueString(defaultValue));
}
}
private <T> T getOptionValue(List<String> args, Option<T> option) {
private String getOptionValue(List<String> args, Option<?> option) {
for (String arg : args) {
if (arg.contains(option.getKey())) {
if (arg.endsWith(option.getKey())) {
throw new IllegalArgumentException("Option '" + arg + "' value must be set using '=' as a separator");
}
String value = arg.substring(Picocli.ARG_PREFIX.length() + option.getKey().length() + 1);
Class<T> type = option.getType();
if (type.equals(String.class)) {
return (T) value;
}
if (type.isEnum()) {
return (T) Enum.valueOf((Class<Enum>) type, value);
}
if (Integer.class.isAssignableFrom(type)) {
return (T) Integer.valueOf(value);
}
if (Boolean.class.isAssignableFrom(type)) {
return (T) Boolean.valueOf(value);
}
throw new RuntimeException("Unsupported option type '" + type + "'");
return arg.substring(Picocli.ARG_PREFIX.length() + option.getKey().length() + 1);
}
}

View file

@ -22,7 +22,10 @@ import io.quarkus.runtime.configuration.QuarkusConfigFactory;
import io.quarkus.test.junit.QuarkusMainTestExtension;
import io.quarkus.test.junit.main.Launch;
import io.quarkus.test.junit.main.LaunchResult;
import org.junit.jupiter.api.extension.ExtensionContext;
import org.junit.jupiter.api.extension.ExtensionContext.Namespace;
import org.junit.jupiter.api.extension.ExtensionContext.Store;
import org.junit.jupiter.api.extension.ParameterContext;
import org.junit.jupiter.api.extension.ParameterResolutionException;
import org.junit.jupiter.api.extension.ReflectiveInvocationContext;
@ -35,20 +38,20 @@ import org.keycloak.quarkus.runtime.cli.command.StartDev;
import org.keycloak.quarkus.runtime.configuration.KeycloakPropertiesConfigSource;
import org.keycloak.quarkus.runtime.configuration.test.TestConfigArgsConfigSource;
import org.keycloak.quarkus.runtime.integration.QuarkusPlatform;
import org.testcontainers.containers.GenericContainer;
import java.io.IOException;
import java.lang.annotation.Annotation;
import java.lang.reflect.Method;
import java.util.Arrays;
import java.util.Collections;
import java.util.HashSet;
import java.util.HashMap;
import java.util.List;
import java.util.Set;
import java.util.Properties;
import java.util.regex.Pattern;
import java.util.stream.Collectors;
import java.util.stream.Stream;
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;
@ -57,9 +60,9 @@ import static org.keycloak.quarkus.runtime.cli.command.Main.CONFIG_FILE_SHORT_NA
public class CLITestExtension extends QuarkusMainTestExtension {
private static final String SYS_PROPS = "sys-props";
private static final String KEY_VALUE_SEPARATOR = "[= ]";
private KeycloakDistribution dist;
private final Set<String> testSysProps = new HashSet<>();
private DatabaseContainer databaseContainer;
private InfinispanContainer infinispanContainer;
private CLIResult result;
@ -68,6 +71,7 @@ public class CLITestExtension extends QuarkusMainTestExtension {
public void beforeEach(ExtensionContext context) throws Exception {
DistributionTest distConfig = getDistributionConfig(context);
Launch launch = context.getRequiredTestMethod().getAnnotation(Launch.class);
getStore(context).put(SYS_PROPS, new HashMap<>(System.getProperties()));
if (launch != null) {
for (String arg : launch.value()) {
@ -117,6 +121,10 @@ public class CLITestExtension extends QuarkusMainTestExtension {
}
}
private Store getStore(ExtensionContext context) {
return context.getStore(Namespace.create(context.getRequiredTestClass(), context.getRequiredTestMethod()));
}
private static LegacyStore getLegacyStoreConfig(ExtensionContext context) {
return context.getTestClass().get().getDeclaredAnnotation(LegacyStore.class);
}
@ -180,18 +188,15 @@ public class CLITestExtension extends QuarkusMainTestExtension {
}
super.afterEach(context);
reset(distConfig);
reset(distConfig, context);
}
private void reset(DistributionTest distConfig) {
private void reset(DistributionTest distConfig, ExtensionContext context) {
QuarkusConfigFactory.setConfig(null);
//remove the config file property if set, and also the profile, to not have side effects in other tests.
System.getProperties().remove(Environment.PROFILE);
System.getProperties().remove("quarkus.profile");
HashMap props = getStore(context).remove(SYS_PROPS, HashMap.class);
System.getProperties().clear();
System.getProperties().putAll(props);
TestConfigArgsConfigSource.setCliArgs(new String[0]);
for (String property : testSysProps) {
System.getProperties().remove(property);
}
if (databaseContainer != null && databaseContainer.isRunning()) {
databaseContainer.stop();
databaseContainer = null;
@ -367,11 +372,6 @@ public class CLITestExtension extends QuarkusMainTestExtension {
}
}
private void setProperty(String name, String value) {
System.setProperty(name, value);
testSysProps.add(name);
}
private List<String> getCliArgs(ExtensionContext context) {
Launch annotation = context.getRequiredTestMethod().getAnnotation(Launch.class);