diff --git a/operator/src/test/java/org/keycloak/operator/testsuite/integration/KeycloakDeploymentTest.java b/operator/src/test/java/org/keycloak/operator/testsuite/integration/KeycloakDeploymentTest.java index 0de9251cff8..23f18b7634f 100644 --- a/operator/src/test/java/org/keycloak/operator/testsuite/integration/KeycloakDeploymentTest.java +++ b/operator/src/test/java/org/keycloak/operator/testsuite/integration/KeycloakDeploymentTest.java @@ -561,7 +561,7 @@ public class KeycloakDeploymentTest extends BaseOperatorTest { CRAssert.assertKeycloakStatusCondition(current, KeycloakStatusCondition.READY, false); CRAssert.assertKeycloakStatusCondition(current, KeycloakStatusCondition.HAS_ERRORS, true, null).has(new Condition<>( c -> c.getMessage().contains(String.format("Waiting for %s/%s-0 due to CrashLoopBackOff", k8sclient.getNamespace(), kc.getMetadata().getName())) - && c.getMessage().contains("feature doesn't exist"), "message" + && c.getMessage().contains("The following build time options have values"), "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 4087e243f06..01389ea6c52 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 @@ -19,7 +19,6 @@ package org.keycloak.quarkus.runtime.cli; import static java.lang.String.format; import static org.keycloak.quarkus.runtime.Environment.getProviderFiles; -import static org.keycloak.quarkus.runtime.Environment.isRebuild; import static org.keycloak.quarkus.runtime.Environment.isRebuildCheck; import static org.keycloak.quarkus.runtime.cli.OptionRenderer.decorateDuplicitOptionName; import static org.keycloak.quarkus.runtime.cli.command.AbstractAutoBuildCommand.OPTIMIZED_BUILD_OPTION_LONG; @@ -54,6 +53,7 @@ import org.keycloak.quarkus.runtime.KeycloakMain; import org.keycloak.quarkus.runtime.Messages; import org.keycloak.quarkus.runtime.cli.command.AbstractCommand; import org.keycloak.quarkus.runtime.cli.command.AbstractNonServerCommand; +import org.keycloak.quarkus.runtime.cli.command.HelpAllMixin; import org.keycloak.quarkus.runtime.cli.command.AbstractAutoBuildCommand; import org.keycloak.quarkus.runtime.cli.command.Main; import org.keycloak.quarkus.runtime.configuration.ConfigArgsConfigSource; @@ -167,15 +167,19 @@ public class Picocli { } AbstractCommand currentCommand = null; + CommandLine commandLine = null; if (currentSpec != null) { - CommandLine commandLine = currentSpec.commandLine(); - addCommandOptions(cliArgs, commandLine); + commandLine = currentSpec.commandLine(); if (commandLine != null && commandLine.getCommand() instanceof AbstractCommand ac) { currentCommand = ac; } } + // init the config before adding options to properly sanitize mappers initConfig(cliArgs, currentCommand); + if (commandLine != null) { + addCommandOptions(cliArgs, commandLine); + } }); } @@ -235,34 +239,8 @@ public class Picocli { return; } - final List ignoredBuildTime = new ArrayList<>(); - if (!options.includeBuildTime) { - // check for provider changes, or overrides of existing persisted options - // we have to ignore things like the profile properties because the commands set them at runtime - checkChangesInBuildOptions((key, oldValue, newValue) -> { - if (key.startsWith(KC_PROVIDER_FILE_PREFIX)) { - boolean changed = false; - if (newValue == null || oldValue == null) { - changed = true; - } else if (!warnedTimestampChanged && timestampChanged(oldValue, newValue)) { - if (Configuration.getOptionalBooleanKcValue("run-in-container").orElse(false)) { - warnedTimestampChanged = true; - warn(PROVIDER_TIMESTAMP_WARNING); - } else { - changed = true; - } - } - if (changed) { - throw new PropertyException(PROVIDER_TIMESTAMP_ERROR); - } - } else if (newValue != null && !isIgnoredPersistedOption(key) - && isUserModifiable(Configuration.getConfigValue(key)) - // let quarkus handle this - it's unsupported for direct usage in keycloak - && !key.startsWith(MicroProfileConfigProvider.NS_QUARKUS_PREFIX)) { - ignoredBuildTime.add(key); - } - }); + validateBuildtime(); } final boolean disabledMappersInterceptorEnabled = DisabledMappersInterceptor.isEnabled(); // return to the state before the disable @@ -350,10 +328,7 @@ public class Picocli { if (!missingOption.isEmpty()) { throw new PropertyException("The following options are required: \n%s".formatted(String.join("\n", missingOption))); } - if (!ignoredBuildTime.isEmpty()) { - throw new PropertyException(format("The following build time options have values that differ from what is persisted - the new values will NOT be used until another build is run: %s\n", - String.join(", ", ignoredBuildTime))); - } else if (!ignoredRunTime.isEmpty()) { + if (!ignoredRunTime.isEmpty()) { info(format("The following run time options were found, but will be ignored during build time: %s\n", String.join(", ", ignoredRunTime))); } @@ -378,6 +353,40 @@ public class Picocli { } } + private void validateBuildtime() { + final List ignoredBuildTime = new ArrayList<>(); + // check for provider changes, or overrides of existing persisted options + // we have to ignore things like the profile properties because the commands set them at runtime + checkChangesInBuildOptions((key, oldValue, newValue) -> { + if (key.startsWith(KC_PROVIDER_FILE_PREFIX)) { + boolean changed = false; + if (newValue == null || oldValue == null) { + changed = true; + } else if (!warnedTimestampChanged && timestampChanged(oldValue, newValue)) { + if (Configuration.getOptionalBooleanKcValue("run-in-container").orElse(false)) { + warnedTimestampChanged = true; + warn(PROVIDER_TIMESTAMP_WARNING); + } else { + changed = true; + } + } + if (changed) { + throw new PropertyException(PROVIDER_TIMESTAMP_ERROR); + } + } else if (newValue != null && !isIgnoredPersistedOption(key) + && isUserModifiable(Configuration.getConfigValue(key)) + // let quarkus handle this - it's unsupported for direct usage in keycloak + && !key.startsWith(MicroProfileConfigProvider.NS_QUARKUS_PREFIX)) { + ignoredBuildTime.add(key); + } + }); + + if (!ignoredBuildTime.isEmpty()) { + throw new PropertyException(format("The following build time options have values that differ from what is persisted - the new values will NOT be used until another build is run: %s\n", + String.join(", ", ignoredBuildTime))); + } + } + static boolean timestampChanged(String oldValue, String newValue) { long longNewValue = Long.valueOf(newValue); long longOldValue = Long.valueOf(oldValue); @@ -398,6 +407,10 @@ public class Picocli { final List ignoredRunTime, final Set disabledBuildTime, final Set disabledRunTime, final Set deprecatedInUse, final Set missingOption, final Set> disabledMappers, PropertyMapper mapper, String from) { + if (mapper.isBuildTime() && !options.includeBuildTime) { + return; // no need to validate as we've already checked for changes in the build time state + } + ConfigValue configValue = getUnmappedValue(from); String configValueStr = configValue.getValue(); @@ -407,17 +420,13 @@ public class Picocli { } if (disabledMappers.contains(mapper)) { - if (PropertyMappers.getMapper(from) != null) { - return; // we found enabled mapper with the same name - } - - // only check build-time for a rebuild, we'll check the runtime later - if (configValueStr != null && (!mapper.isRunTime() || !isRebuild())) { - if (PropertyMapper.isCliOption(configValue)) { - throw new KcUnmatchedArgumentException(abstractCommand.getCommandLine().orElseThrow(), List.of(mapper.getCliFormat())); - } else { - handleDisabled(mapper.isRunTime() ? disabledRunTime : disabledBuildTime, mapper); - } + // add an error message if there's a value, no enabled propertymapper, and it's not a cli value + // as some cli options may be directly on the command and not + // backed by a property mapper - if they are disabled that should have already been handled as + // an unrecognized arg + if (configValueStr != null && PropertyMappers.getMapper(from) == null + && !PropertyMapper.isCliOption(configValue)) { + handleDisabled(mapper.isRunTime() ? disabledRunTime : disabledBuildTime, mapper); } return; } @@ -938,7 +947,9 @@ public class Picocli { .orElse(Environment.PROD_PROFILE_VALUE)); Environment.setProfile(profile); - parsedCommand.ifPresent(PropertyMappers::sanitizeDisabledMappers); + if (!cliArgs.contains(HelpAllMixin.HELP_ALL_OPTION)) { + parsedCommand.ifPresent(PropertyMappers::sanitizeDisabledMappers); + } } } 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 cec554a59d8..7bf1319abc3 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 @@ -138,6 +138,7 @@ public class ConfigArgsConfigSource extends PropertiesConfigSource { } 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)) { diff --git a/quarkus/runtime/src/main/java/org/keycloak/quarkus/runtime/configuration/KeycloakConfigSourceProvider.java b/quarkus/runtime/src/main/java/org/keycloak/quarkus/runtime/configuration/KeycloakConfigSourceProvider.java index 869f0fadef7..b9f9b042394 100644 --- a/quarkus/runtime/src/main/java/org/keycloak/quarkus/runtime/configuration/KeycloakConfigSourceProvider.java +++ b/quarkus/runtime/src/main/java/org/keycloak/quarkus/runtime/configuration/KeycloakConfigSourceProvider.java @@ -70,13 +70,11 @@ public class KeycloakConfigSourceProvider implements ConfigSourceProvider, Confi } /** - * Mainly for test purposes as MicroProfile Config does not seem to provide a way to reload configsources when the config - * is released + * For test purposes */ public static void reload() { CONFIG_SOURCES.clear(); CONFIG_SOURCE_DISPLAY_NAMES.clear(); - initializeSources(); } @Override diff --git a/quarkus/runtime/src/main/java/org/keycloak/quarkus/runtime/configuration/mappers/ExportPropertyMappers.java b/quarkus/runtime/src/main/java/org/keycloak/quarkus/runtime/configuration/mappers/ExportPropertyMappers.java index 546fa00603e..bd0e73a0210 100644 --- a/quarkus/runtime/src/main/java/org/keycloak/quarkus/runtime/configuration/mappers/ExportPropertyMappers.java +++ b/quarkus/runtime/src/main/java/org/keycloak/quarkus/runtime/configuration/mappers/ExportPropertyMappers.java @@ -52,10 +52,12 @@ public final class ExportPropertyMappers implements PropertyMapperGrouping { fromOption(ExportOptions.FILE) .to("kc.spi-export--single-file--file") .paramLabel("file") + .isEnabled(c -> c instanceof Export) .build(), fromOption(ExportOptions.DIR) .to("kc.spi-export--dir--dir") .paramLabel("dir") + .isEnabled(c -> c instanceof Export) .build(), fromOption(ExportOptions.REALM) .to("kc.spi-export--single-file--realm-name") @@ -109,7 +111,7 @@ public final class ExportPropertyMappers implements PropertyMapperGrouping { } private static boolean isDirProvider() { - return isProvider(DIR); + return !isSingleFileProvider(); } private static boolean isProvider(String provider) { diff --git a/quarkus/runtime/src/main/java/org/keycloak/quarkus/runtime/configuration/mappers/ImportPropertyMappers.java b/quarkus/runtime/src/main/java/org/keycloak/quarkus/runtime/configuration/mappers/ImportPropertyMappers.java index c4e2a491bb3..72c1eea65cb 100644 --- a/quarkus/runtime/src/main/java/org/keycloak/quarkus/runtime/configuration/mappers/ImportPropertyMappers.java +++ b/quarkus/runtime/src/main/java/org/keycloak/quarkus/runtime/configuration/mappers/ImportPropertyMappers.java @@ -50,10 +50,12 @@ public final class ImportPropertyMappers implements PropertyMapperGrouping { fromOption(ImportOptions.FILE) .to("kc.spi-import--single-file--file") .paramLabel("file") + .isEnabled(c -> c instanceof Import) .build(), fromOption(ImportOptions.DIR) .to("kc.spi-import--dir--dir") .paramLabel("dir") + .isEnabled(c -> c instanceof Import) .build(), fromOption(ImportOptions.OVERRIDE) .to("kc.spi-import--single-file--strategy") @@ -87,7 +89,7 @@ public final class ImportPropertyMappers implements PropertyMapperGrouping { } private static boolean isDirProvider() { - return isProvider(DIR); + return !isSingleFileProvider(); } private static boolean isProvider(String provider) { diff --git a/quarkus/runtime/src/main/java/org/keycloak/quarkus/runtime/configuration/mappers/PropertyMapper.java b/quarkus/runtime/src/main/java/org/keycloak/quarkus/runtime/configuration/mappers/PropertyMapper.java index 364f53836ca..b561aad2bfc 100644 --- a/quarkus/runtime/src/main/java/org/keycloak/quarkus/runtime/configuration/mappers/PropertyMapper.java +++ b/quarkus/runtime/src/main/java/org/keycloak/quarkus/runtime/configuration/mappers/PropertyMapper.java @@ -30,6 +30,7 @@ import java.util.function.BiConsumer; import java.util.function.BiFunction; import java.util.function.BooleanSupplier; import java.util.function.Consumer; +import java.util.function.Function; import java.util.stream.Stream; import org.keycloak.config.DeprecatedMetadata; @@ -37,6 +38,7 @@ import org.keycloak.config.Option; import org.keycloak.config.OptionCategory; import org.keycloak.quarkus.runtime.cli.PropertyException; import org.keycloak.quarkus.runtime.cli.ShortErrorMessageHandler; +import org.keycloak.quarkus.runtime.cli.command.AbstractCommand; import org.keycloak.quarkus.runtime.configuration.ConfigArgsConfigSource; import org.keycloak.quarkus.runtime.configuration.KcEnvConfigSource; import org.keycloak.quarkus.runtime.configuration.KeycloakConfigSourceProvider; @@ -53,7 +55,7 @@ public class PropertyMapper { protected final Option option; private final String to; - private BooleanSupplier enabled; + private Function enabled; private String enabledWhen; private final ValueMapper mapper; private final String mapFrom; @@ -76,7 +78,7 @@ public class PropertyMapper { mapper.requiredWhen, from, namedProperty); } - PropertyMapper(Option option, String to, BooleanSupplier enabled, String enabledWhen, + PropertyMapper(Option option, String to, Function enabled, String enabledWhen, ValueMapper mapper, String mapFrom, ValueMapper parentMapper, String paramLabel, boolean mask, BiConsumer, ConfigValue> validator, String description, BooleanSupplier required, String requiredWhen, String from, String namedProperty) { @@ -159,12 +161,12 @@ public class PropertyMapper { return this.option; } - public void setEnabled(BooleanSupplier enabled) { + public void setEnabled(Function enabled) { this.enabled = enabled; } - public boolean isEnabled() { - return enabled.getAsBoolean(); + public boolean isEnabled(AbstractCommand command) { + return enabled.apply(command); } public Optional getEnabledWhen() { @@ -356,7 +358,7 @@ public class PropertyMapper { private String mapFrom = null; private ValueMapper parentMapper; private boolean isMasked = false; - private BooleanSupplier isEnabled = () -> true; + private Function enabled = ignored -> true; private String enabledWhen = ""; private String paramLabel; private BiConsumer, ConfigValue> validator = (mapper, value) -> mapper.validateValues(value, mapper::validateExpectedValues); @@ -428,13 +430,17 @@ public class PropertyMapper { } public Builder isEnabled(BooleanSupplier isEnabled, String enabledWhen) { - this.isEnabled = isEnabled; + this.enabled = ignored -> isEnabled.getAsBoolean(); this.enabledWhen=enabledWhen; return this; } public Builder isEnabled(BooleanSupplier isEnabled) { - this.isEnabled = isEnabled; + return isEnabled(isEnabled, ""); + } + + public Builder isEnabled(Function enabled) { + this.enabled = enabled; return this; } @@ -528,12 +534,12 @@ public class PropertyMapper { paramLabel = Boolean.TRUE + "|" + Boolean.FALSE; } if (option.getKey().contains(WildcardPropertyMapper.WILDCARD_FROM_START)) { - return new WildcardPropertyMapper<>(option, to, isEnabled, enabledWhen, mapper, mapFrom, parentMapper, paramLabel, isMasked, validator, description, isRequired, requiredWhen, wildcardKeysTransformer, wildcardMapFrom); + return new WildcardPropertyMapper<>(option, to, enabled, enabledWhen, mapper, mapFrom, parentMapper, paramLabel, isMasked, validator, description, isRequired, requiredWhen, wildcardKeysTransformer, wildcardMapFrom); } if (wildcardKeysTransformer != null || wildcardMapFrom != null) { throw new AssertionError("Wildcard operations not expected with non-wildcard mapper"); } - return new PropertyMapper<>(option, to, isEnabled, enabledWhen, mapper, mapFrom, parentMapper, paramLabel, isMasked, validator, description, isRequired, requiredWhen, null, null); + return new PropertyMapper<>(option, to, enabled, enabledWhen, mapper, mapFrom, parentMapper, paramLabel, isMasked, validator, description, isRequired, requiredWhen, null, null); } } 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 227fd5b9054..4643d3f7b51 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 @@ -14,7 +14,6 @@ import org.keycloak.quarkus.runtime.Environment; import org.keycloak.quarkus.runtime.cli.Picocli; import org.keycloak.quarkus.runtime.cli.PropertyException; import org.keycloak.quarkus.runtime.cli.command.AbstractCommand; -import org.keycloak.quarkus.runtime.cli.command.ShowConfig; import org.keycloak.quarkus.runtime.configuration.DisabledMappersInterceptor; import org.keycloak.quarkus.runtime.configuration.MicroProfileConfigProvider; import org.keycloak.quarkus.runtime.configuration.NestedPropertyMappingInterceptor; @@ -24,7 +23,6 @@ import java.util.ArrayList; import java.util.Collection; import java.util.Collections; import java.util.EnumMap; -import java.util.EnumSet; import java.util.HashMap; import java.util.HashSet; import java.util.LinkedHashSet; @@ -152,26 +150,22 @@ public final class PropertyMappers { return value; } - private static PropertyMapper getMapperOrDefault(String property, PropertyMapper defaultMapper, OptionCategory category) { + public static PropertyMapper getMapper(String property, OptionCategory category) { final var mappers = new ArrayList<>(MAPPERS.getOrDefault(property, Collections.emptyList())); if (category != null) { mappers.removeIf(m -> !m.getCategory().equals(category)); } return switch (mappers.size()) { - case 0 -> defaultMapper; + case 0 -> null; case 1 -> mappers.get(0); default -> { - log.debugf("Duplicated mappers for key '%s'. Used the first found.", property); + log.tracef("Duplicated mappers for key '%s'. Used the first found.", property); yield mappers.get(0); } }; } - public static PropertyMapper getMapper(String property, OptionCategory category) { - return getMapperOrDefault(property, null, category); - } - public static PropertyMapper getMapper(String property) { return getMapper(property, null); } @@ -224,12 +218,6 @@ public final class PropertyMappers { return getDisabledMapper(property).isPresent() && getMapper(property) == null; } - private static Set> filterDeniedCategories(List> mappers, AbstractCommand command) { - final var allowedCategories = EnumSet.copyOf(command.getOptionCategories()); - - return mappers.stream().filter(f -> allowedCategories.contains(f.getCategory())).collect(Collectors.toSet()); - } - private static class MappersConfig extends MultivaluedHashMap> { private final Map>> buildTimeMappers = new EnumMap<>(OptionCategory.class); @@ -273,9 +261,12 @@ public final class PropertyMappers { } private void remove(String key, PropertyMapper mapper) { - List> list = get(key); + List> list = super.get(key); if (CollectionUtil.isNotEmpty(list)) { list.remove(mapper); + if (list.isEmpty()) { + super.remove(key); + } } } @@ -320,37 +311,13 @@ public final class PropertyMappers { } } - sanitizeMappers(buildTimeMappers, disabledBuildTimeMappers); - sanitizeMappers(runtimeTimeMappers, disabledRuntimeMappers); + sanitizeMappers(buildTimeMappers, disabledBuildTimeMappers, command); + sanitizeMappers(runtimeTimeMappers, disabledRuntimeMappers, command); - assertDuplicatedMappers(command); - }); - } - - private void assertDuplicatedMappers(AbstractCommand command) { - final var duplicatedMappers = entrySet().stream() - .filter(e -> CollectionUtil.isNotEmpty(e.getValue())) - .filter(e -> e.getValue().size() > 1) - .toList(); - - final var isBuildPhase = isRebuild() || isRebuildCheck() || command.includeBuildTime(); - final var allowedForCommand = ShowConfig.NAME.equals(command.getName()); - - if (!duplicatedMappers.isEmpty()) { - duplicatedMappers.forEach(f -> { - final var filteredMappers = filterDeniedCategories(f.getValue(), command); - - if (filteredMappers.size() > 1) { - final var areBuildTimeMappers = filteredMappers.stream().anyMatch(PropertyMapper::isBuildTime); - - // thrown in runtime, or in build time, when some mapper is marked as buildTime + not allowed to have duplicates for specific command - final var shouldBeThrown = !allowedForCommand && (!isBuildPhase || areBuildTimeMappers); - if (shouldBeThrown) { - throw new PropertyException(String.format("Duplicated mapper for key '%s'.", f.getKey())); - } - } + entrySet().stream().filter(e -> e.getValue().size() > 1).findFirst().ifPresent(e -> { + throw new PropertyException(String.format("Duplicated mapper for key '%s'.", e.getKey())); }); - } + }); } public Map>> getRuntimeMappers() { @@ -370,10 +337,10 @@ public final class PropertyMappers { } private static void sanitizeMappers(Map>> mappers, - Map> disabledMappers) { + Map> disabledMappers, AbstractCommand command) { mappers.forEach((category, propertyMappers) -> propertyMappers.removeIf(pm -> { - final boolean shouldRemove = !pm.isEnabled(); + final boolean shouldRemove = !pm.isEnabled(command); if (shouldRemove) { MAPPERS.removeMapper(pm); handleMapper(pm, disabledMappers::put); diff --git a/quarkus/runtime/src/main/java/org/keycloak/quarkus/runtime/configuration/mappers/WildcardPropertyMapper.java b/quarkus/runtime/src/main/java/org/keycloak/quarkus/runtime/configuration/mappers/WildcardPropertyMapper.java index 6acb62b0fa1..f4c49dbd32e 100644 --- a/quarkus/runtime/src/main/java/org/keycloak/quarkus/runtime/configuration/mappers/WildcardPropertyMapper.java +++ b/quarkus/runtime/src/main/java/org/keycloak/quarkus/runtime/configuration/mappers/WildcardPropertyMapper.java @@ -6,12 +6,14 @@ import java.util.Set; import java.util.function.BiConsumer; import java.util.function.BiFunction; import java.util.function.BooleanSupplier; +import java.util.function.Function; import java.util.regex.Pattern; import java.util.stream.Collectors; import java.util.stream.Stream; import org.keycloak.config.LoggingOptions; import org.keycloak.config.Option; +import org.keycloak.quarkus.runtime.cli.command.AbstractCommand; import io.smallrye.config.ConfigValue; @@ -32,7 +34,7 @@ public class WildcardPropertyMapper extends PropertyMapper { private String toSuffix; private Character replacementChar = null; - public WildcardPropertyMapper(Option option, String to, BooleanSupplier enabled, String enabledWhen, ValueMapper mapper, String mapFrom, ValueMapper parentMapper, + public WildcardPropertyMapper(Option option, String to, Function enabled, String enabledWhen, ValueMapper mapper, String mapFrom, ValueMapper parentMapper, String paramLabel, boolean mask, BiConsumer, ConfigValue> validator, String description, BooleanSupplier required, String requiredWhen, BiFunction, Set> wildcardKeysTransformer, ValueMapper wildcardMapFrom) { super(option, to, enabled, enabledWhen, mapper, mapFrom, parentMapper, paramLabel, mask, validator, description, required, requiredWhen, null, null); 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 f8c97391650..ee62b76f839 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 @@ -483,7 +483,7 @@ public class PicocliTest extends AbstractConfigurationTest { @Test public void wrongLevelForCategory() { - NonRunningPicocli nonRunningPicocli = pseudoLaunch("start-dev", "--log-level-org.keycloak=wrong"); + NonRunningPicocli nonRunningPicocli = pseudoLaunch("start-dev", "--log-level-org.keycloak", "wrong"); assertEquals(CommandLine.ExitCode.USAGE, nonRunningPicocli.exitCode); assertTrue(nonRunningPicocli.getErrString().contains("Invalid log level: wrong. Possible values are: warn, trace, debug, error, fatal, info.")); } @@ -802,7 +802,7 @@ public class PicocliTest extends AbstractConfigurationTest { var logHandlerName = logHandler.toString(); var logHandlerOptionsName = logHandlerOptions.toString(); - NonRunningPicocli nonRunningPicocli = pseudoLaunch("start-dev", "--log=%s".formatted(logHandlerName), "--log-%s-async-queue-length=invalid".formatted(logHandlerOptionsName)); + NonRunningPicocli nonRunningPicocli = pseudoLaunch("start-dev", "--log=%s".formatted(logHandlerOptionsName), "--log-%s-async=true".formatted(logHandlerOptionsName), "--log-%s-async-queue-length=invalid".formatted(logHandlerOptionsName)); assertEquals(CommandLine.ExitCode.USAGE, nonRunningPicocli.exitCode); assertThat(nonRunningPicocli.getErrString(), containsString("Invalid value for option '--log-%s-async-queue-length': 'invalid' is not an int".formatted(logHandlerOptionsName))); @@ -861,6 +861,13 @@ public class PicocliTest extends AbstractConfigurationTest { assertThat(nonRunningPicocli.getErrString(), containsString("Invalid value for option '--log-%s-async-queue-length': 'invalid' is not an int".formatted(handlerName))); } + @Test + public void testImportHelpAllSucceeds() { + NonRunningPicocli nonRunningPicocli = pseudoLaunch("import", "--help-all"); + assertEquals(CommandLine.ExitCode.OK, nonRunningPicocli.exitCode); + assertTrue(nonRunningPicocli.getOutString().contains("--db")); + } + @Test public void testUnaryBooleanFails() { NonRunningPicocli nonRunningPicocli = pseudoLaunch("start-dev", "--health-enabled"); @@ -882,6 +889,20 @@ public class PicocliTest extends AbstractConfigurationTest { assertThat(nonRunningPicocli.getErrString(), containsString("Missing required argument: --file")); } + @Test + public void testNoKcDirWarning() { + putEnvVar("KC_DIR", "dir"); + putEnvVar("KC_LOG_LEVEL", "debug"); + var picocli = build("build", "--db=dev-file"); + assertFalse(picocli.getOutString(), picocli.getOutString().contains("kc.dir")); + } + + @Test + public void testUpdatesFileValidation() { + NonRunningPicocli picocli = pseudoLaunch("update-compatibility","check", "--file=not-found"); + assertTrue(picocli.getErrString().contains("Incorrect argument --file.")); + } + @Test public void errorSpiBuildtimeChanged() { putEnvVar("KC_SPI_EVENTS_LISTENER__PROVIDER", "jboss-logging"); diff --git a/quarkus/tests/integration/src/test/java/org/keycloak/it/cli/dist/ImportAtStartupDistTest.java b/quarkus/tests/integration/src/test/java/org/keycloak/it/cli/dist/ImportAtStartupDistTest.java index 765e7f707ee..dad0aa3e9ae 100644 --- a/quarkus/tests/integration/src/test/java/org/keycloak/it/cli/dist/ImportAtStartupDistTest.java +++ b/quarkus/tests/integration/src/test/java/org/keycloak/it/cli/dist/ImportAtStartupDistTest.java @@ -82,7 +82,7 @@ public class ImportAtStartupDistTest { @BeforeStartDistribution(CreateRealmConfigurationFile.class) void testImportFromFileCreatedByExportAllRealms(KeycloakDistribution dist) throws IOException { dist.run("start-dev", "--import-realm"); - dist.run("--profile=dev", "export", "--file=../data/import/realm.json"); + dist.run("--profile=dev", "export", "--file=../data/import/realm.json", "--verbose"); RawKeycloakDistribution rawDist = dist.unwrap(RawKeycloakDistribution.class); FileUtil.deleteDirectory(rawDist.getDistPath().resolve("data").resolve("h2").toAbsolutePath()); 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 39804db2280..07fdc19a528 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 @@ -106,13 +106,6 @@ public class StartCommandDistTest { cliResult.assertStarted(); } - @DryRun - @Test - @Launch({ "-v", "start", "--db=dev-mem", OPTIMIZED_BUILD_OPTION_LONG}) - void failBuildPropertyNotAvailable(CLIResult cliResult) { - cliResult.assertError("Build time option: '--db' not usable with pre-built image and --optimized"); - } - @DryRun @Test @Launch({ "--profile=dev", "start", "--http-enabled=true", "--hostname-strict=false" }) @@ -160,13 +153,6 @@ public class StartCommandDistTest { assertTrue(cliResult.getErrorOutput().isBlank(), cliResult.getErrorOutput()); } - @DryRun - @Test - @Launch({ "start", "--optimized", "--http-enabled=true", "--hostname-strict=false", "--db=postgres" }) - void testStartUsingOptimizedDoesNotAllowBuildOptions(CLIResult cliResult) { - cliResult.assertError("Build time option: '--db' not usable with pre-built image and --optimized"); - } - @DryRun @Test @Launch({ "start", "--db=dev-file", "--http-enabled=true", "--cache-remote-host=localhost", "--hostname-strict=false", "--cache-remote-tls-enabled=false", "--transaction-xa-enabled=true" })