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 081920b4b5..f36770aa76 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 @@ -192,7 +192,7 @@ public final class Picocli { private static List getSanitizedRuntimeCliOptions() { List properties = new ArrayList<>(); - parseConfigArgs(new BiConsumer() { + parseConfigArgs(ConfigArgsConfigSource.getAllCliArgs(), new BiConsumer() { @Override public void accept(String key, String value) { PropertyMapper mapper = PropertyMappers.getMapper(key); @@ -203,6 +203,8 @@ public final class Picocli { properties.add(key + "=" + formatValue(key, value)); } + }, arg -> { + properties.add(arg); }); return properties; @@ -225,7 +227,7 @@ public final class Picocli { exitCode = cmd.execute(configArgsList.toArray(new String[0])); if(!isDevMode() && exitCode == cmd.getCommandSpec().exitCodeOnSuccess()) { - cmd.getOut().printf("Next time you run the server, just run:%n%n\t%s %s %s %s%n%n", Environment.getCommand(), commandName, OPTIMIZED_BUILD_OPTION_LONG, String.join(" ", getSanitizedRuntimeCliOptions())); + cmd.getOut().printf("Next time you run the server, just run:%n%n\t%s %s %s%n%n", Environment.getCommand(), String.join(" ", getSanitizedRuntimeCliOptions()), OPTIMIZED_BUILD_OPTION_LONG); } return exitCode; 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 693be04e6f..87042b5f57 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 @@ -17,7 +17,6 @@ package org.keycloak.quarkus.runtime.configuration; -import static java.util.Arrays.asList; import static org.keycloak.quarkus.runtime.cli.Picocli.ARG_SHORT_PREFIX; import static org.keycloak.quarkus.runtime.configuration.Configuration.OPTION_PART_SEPARATOR_CHAR; import static org.keycloak.quarkus.runtime.configuration.MicroProfileConfigProvider.NS_KEYCLOAK_PREFIX; @@ -26,34 +25,37 @@ import java.util.Collections; import java.util.HashMap; import java.util.List; import java.util.Map; +import java.util.Set; import java.util.function.BiConsumer; +import java.util.function.Consumer; import java.util.regex.Pattern; import io.smallrye.config.PropertiesConfigSource; +import org.keycloak.quarkus.runtime.cli.command.Main; import org.keycloak.quarkus.runtime.configuration.mappers.PropertyMapper; import org.keycloak.quarkus.runtime.configuration.mappers.PropertyMappers; -import org.keycloak.utils.StringUtil; /** *

A configuration source for mapping configuration arguments to their corresponding properties so that they can be recognized * when building and running the server. - * + * *

The mapping is based on the system property {@code kc.config.args}, where the value is a comma-separated list of * the arguments passed during build or runtime. E.g: "--http-enabled=true,--http-port=8180,--database-vendor=postgres". - * - *

Each argument is going to be mapped to its corresponding configuration property by prefixing the key with the {@link MicroProfileConfigProvider#NS_KEYCLOAK} namespace. + * + *

Each argument is going to be mapped to its corresponding configuration property by prefixing the key with the {@link MicroProfileConfigProvider#NS_KEYCLOAK} namespace. */ public class ConfigArgsConfigSource extends PropertiesConfigSource { + public static final Set SHORT_OPTIONS_ACCEPTING_VALUE = Set.of(Main.PROFILE_SHORT_NAME, Main.CONFIG_FILE_SHORT_NAME); + public static final String CLI_ARGS = "kc.config.args"; public static final String NAME = "CliConfigSource"; private static final String ARG_SEPARATOR = ";;"; - private static final Pattern ARG_SPLIT = Pattern.compile(";;"); private static final Pattern ARG_KEY_VALUE_SPLIT = Pattern.compile("="); protected ConfigArgsConfigSource() { - super(parseArgument(), NAME, 600); + super(parseArguments(), NAME, 600); } public static void setCliArgs(String[] args) { @@ -98,16 +100,16 @@ public class ConfigArgsConfigSource extends PropertiesConfigSource { return properties.get(propertyName.replace(OPTION_PART_SEPARATOR_CHAR, '.')); } - private static Map parseArgument() { + private static Map parseArguments() { String rawArgs = getRawConfigArgs(); - + if (rawArgs == null || "".equals(rawArgs.trim())) { return Collections.emptyMap(); } Map properties = new HashMap<>(); - parseConfigArgs(new BiConsumer() { + parseConfigArgs(getAllCliArgs(), new BiConsumer() { @Override public void accept(String key, String value) { key = NS_KEYCLOAK_PREFIX + key.substring(2); @@ -126,56 +128,48 @@ public class ConfigArgsConfigSource extends PropertiesConfigSource { properties.put(mapper.getFrom(), value); } } - }); + }, ignored -> {}); return properties; } - public static void parseConfigArgs(BiConsumer cliArgConsumer) { - // init here because the class might be loaded by CL without init - List ignoredArgs = asList("--verbose", "-v", "--help", "-h"); - String rawArgs = getRawConfigArgs(); - - if (StringUtil.isBlank(rawArgs)) { - return; - } - - String[] args = ARG_SPLIT.split(rawArgs); - - for (int i = 0; i < args.length; i++) { - String arg = args[i]; - - if (ignoredArgs.contains(arg)) { - continue; - } + public static void parseConfigArgs(List args, BiConsumer valueArgConsumer, Consumer unaryConsumer) { + for (int i = 0; i < args.size(); i++) { + String arg = args.get(i); if (!arg.startsWith(ARG_SHORT_PREFIX)) { + unaryConsumer.accept(arg); continue; } String[] keyValue = ARG_KEY_VALUE_SPLIT.split(arg, 2); String key = keyValue[0]; - if ("".equals(key.trim())) { - throw new IllegalArgumentException("Invalid argument key"); - } - String value; if (keyValue.length == 1) { - if (args.length <= i + 1) { + if (args.size() <= i + 1) { + unaryConsumer.accept(arg); continue; } - value = args[i + 1]; - } else if (keyValue.length == 2) { + PropertyMapper mapper = PropertyMappers.getMapper(key); + // the weaknesses here: + // - needs to know all of the short name options that accept a value + // - does not know all of the picocli parsing rules. picocli will accept -cffile, and short option grouping - that's not accounted for + // - does not understand spi options, they will be assumed to be unary + if (mapper != null || SHORT_OPTIONS_ACCEPTING_VALUE.contains(key)) { + i++; // consume next as a value to the key + value = args.get(i); + } else { + unaryConsumer.accept(arg); + continue; + } + } else { // the argument has a simple value. Eg.: key=pair value = keyValue[1]; - } else { - // to support cases like --db-url=jdbc:mariadb://localhost/kc?a=1 - value = arg.substring(key.length() + 1); } - cliArgConsumer.accept(key, value); + valueArgConsumer.accept(key, value); } } } diff --git a/quarkus/runtime/src/test/java/org/keycloak/quarkus/runtime/configuration/ConfigArgsConfigSourceTest.java b/quarkus/runtime/src/test/java/org/keycloak/quarkus/runtime/configuration/ConfigArgsConfigSourceTest.java new file mode 100644 index 0000000000..d5bc35b12f --- /dev/null +++ b/quarkus/runtime/src/test/java/org/keycloak/quarkus/runtime/configuration/ConfigArgsConfigSourceTest.java @@ -0,0 +1,37 @@ +/* + * Copyright 2024 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.configuration; + +import org.junit.Test; + +import java.util.ArrayList; +import java.util.Arrays; +import java.util.List; + +import static org.junit.Assert.assertEquals; + +public class ConfigArgsConfigSourceTest { + + @Test + public void testParseArgs() { + List values = new ArrayList<>(); + ConfigArgsConfigSource.parseConfigArgs(Arrays.asList("--key=value", "-cf", "file", "command", "arg", "--db", "value"), (key, value) -> values.add(key+'='+value), values::add); + assertEquals(Arrays.asList("--key=value", "-cf=file", "command", "arg", "--db=value"), values); + } + +} diff --git a/quarkus/tests/integration/src/test/java/org/keycloak/it/cli/dist/StartAutoBuildDistTest.java b/quarkus/tests/integration/src/test/java/org/keycloak/it/cli/dist/StartAutoBuildDistTest.java index 62294df7b9..50a2ceb6c9 100644 --- a/quarkus/tests/integration/src/test/java/org/keycloak/it/cli/dist/StartAutoBuildDistTest.java +++ b/quarkus/tests/integration/src/test/java/org/keycloak/it/cli/dist/StartAutoBuildDistTest.java @@ -38,7 +38,7 @@ import static org.keycloak.quarkus.runtime.cli.command.AbstractStartCommand.OPTI public class StartAutoBuildDistTest { @Test - @Launch({ "start", "--http-enabled=true", "--hostname-strict=false" }) + @Launch({ "--verbose", "start", "--http-enabled=true", "--hostname-strict=false" }) @Order(1) void testStartAutoBuild(LaunchResult result) { CLIResult cliResult = (CLIResult) result; @@ -47,7 +47,7 @@ public class StartAutoBuildDistTest { cliResult.assertMessage("Server configuration updated and persisted. Run the following command to review the configuration:"); cliResult.assertMessage(KeycloakDistribution.SCRIPT_CMD + " show-config"); cliResult.assertMessage("Next time you run the server, just run:"); - cliResult.assertMessage(KeycloakDistribution.SCRIPT_CMD + " start " + OPTIMIZED_BUILD_OPTION_LONG + " --http-enabled=true --hostname-strict=false"); + cliResult.assertMessage(KeycloakDistribution.SCRIPT_CMD + " --verbose start --http-enabled=true --hostname-strict=false " + OPTIMIZED_BUILD_OPTION_LONG); assertFalse(cliResult.getOutput().contains("--cache")); cliResult.assertStarted(); } 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 5a45ab0263..f5ec3a5bc4 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 @@ -96,7 +96,7 @@ public class StartCommandDistTest { cliResult.assertMessage("Server configuration updated and persisted. Run the following command to review the configuration:"); cliResult.assertMessage(KeycloakDistribution.SCRIPT_CMD + " show-config"); cliResult.assertMessage("Next time you run the server, just run:"); - cliResult.assertMessage(KeycloakDistribution.SCRIPT_CMD + " start " + OPTIMIZED_BUILD_OPTION_LONG + " --http-enabled=true --hostname-strict=false"); + cliResult.assertMessage(KeycloakDistribution.SCRIPT_CMD + " start --http-enabled=true --hostname-strict=false " + OPTIMIZED_BUILD_OPTION_LONG); assertFalse(cliResult.getOutput().contains("--metrics-enabled")); cliResult.assertStarted(); }