libre.sh/keycloak-scim
3
0
Fork 0
Code Issues 15 Pull requests Releases Packages Activity Actions

fix: additional consolidation / refinement of argument parsing (#31448)

Browse source 
closes: #26339

Signed-off-by: Steve Hawkins <shawkins@redhat.com>
This commit is contained in:
Steven Hawkins 2024-07-24 04:23:23 -04:00 committed by GitHub
parent 913a2aa071
commit 6378dcbac2
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
4 changed files with 36 additions and 61 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

60
quarkus/runtime/src/main/java/org/keycloak/quarkus/runtime/cli/Picocli.java
View file

@ -66,7 +66,6 @@ import org.keycloak.config.OptionCategory;
import org.keycloak.quarkus.runtime.cli.command.AbstractCommand;
import org.keycloak.quarkus.runtime.cli.command.BootstrapAdmin;
import org.keycloak.quarkus.runtime.cli.command.Build;
import org.keycloak.quarkus.runtime.cli.command.ImportRealmMixin;
import org.keycloak.quarkus.runtime.cli.command.Main;
import org.keycloak.quarkus.runtime.cli.command.ShowConfig;
import org.keycloak.quarkus.runtime.cli.command.Start;
@ -100,7 +99,6 @@ public final class Picocli {
public static final String ARG_PREFIX = "--";
public static final String ARG_SHORT_PREFIX = "-";
public static final String NO_PARAM_LABEL = "none";
private static final String ARG_KEY_VALUE_SEPARATOR = "=";
private static class IncludeOptions {
boolean includeRuntime;
@ -277,11 +275,9 @@ public final class Picocli {
public void accept(String key, String value) {
PropertyMapper<?> mapper = PropertyMappers.getMapper(key);
if (mapper != null && mapper.isBuildTime()) {
return;
if (mapper == null || mapper.isRunTime()) {
properties.add(key + "=" + maskValue(key, value));
}
properties.add(key + "=" + maskValue(key, value));
}
}, arg -> {
properties.add(arg);
@ -301,11 +297,9 @@ public final class Picocli {
parseConfigArgs(cliArgs, (k, v) -> {
PropertyMapper<?> mapper = PropertyMappers.getMapper(k);
if (mapper == null || mapper.isRunTime()) {
return;
if (mapper != null && mapper.isBuildTime()) {
configArgsList.add(k + "=" + v);
}
configArgsList.add(k + "=" + v);
}, ignored -> {});
int exitCode = cmd.execute(configArgsList.toArray(new String[0]));
@ -855,42 +849,26 @@ public final class Picocli {
// makes sure cli args are available to the config source
ConfigArgsConfigSource.setCliArgs(rawArgs);
List<String> args = new ArrayList<>(List.of(rawArgs));
Iterator<String> iterator = args.iterator();
while (iterator.hasNext()) {
String arg = iterator.next();
if (arg.startsWith("--spi") || arg.startsWith("-D")) {
// TODO: ignore properties for providers for now, need to fetch them from the providers, otherwise CLI will complain about invalid options
// also ignores system properties as they are set when starting the JVM
// change this once we are able to obtain properties from providers
iterator.remove();
if (!arg.contains(ARG_KEY_VALUE_SEPARATOR)) {
if (!iterator.hasNext()) {
if (arg.startsWith("--spi")) {
throw new PropertyException(format("spi argument %s requires a value.", arg));
}
return args;
}
String next = iterator.next();
if (!next.startsWith("--")) {
// ignore the value if the arg is using space as separator
iterator.remove();
}
}
// TODO: ignore properties for providers for now, need to fetch them from the providers, otherwise CLI will complain about invalid options
// also ignores system properties as they are set when starting the JVM
// change this once we are able to obtain properties from providers
List<String> args = new ArrayList<>();
ConfigArgsConfigSource.parseConfigArgs(List.of(rawArgs), (arg, value) -> {
if (!arg.startsWith("--spi") && !arg.startsWith("-D")) {
args.add(arg + "=" + value);
}
}
}, arg -> {
if (arg.startsWith("--spi")) {
throw new PropertyException(format("spi argument %s requires a value.", arg));
}
if (!arg.startsWith("-D")) {
args.add(arg);
}
});
return args;
}
private static boolean isRuntimeOption(String arg) {
return arg.startsWith(ImportRealmMixin.IMPORT_REALM);
}
private static void checkChangesInBuildOptionsDuringAutoBuild() {
if (Configuration.isOptimized()) {
List<PropertyMapper<?>> buildOptions = stream(Configuration.getPropertyNames(true).spliterator(), false)

3
quarkus/runtime/src/main/java/org/keycloak/quarkus/runtime/configuration/ConfigArgsConfigSource.java
View file

@ -156,8 +156,7 @@ public class ConfigArgsConfigSource extends PropertiesConfigSource {
// 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)) {
if (mapper != null || SHORT_OPTIONS_ACCEPTING_VALUE.contains(key) || arg.startsWith("--spi")) {
i++; // consume next as a value to the key
value = args.get(i);
} else {

7
quarkus/runtime/src/test/java/org/keycloak/quarkus/runtime/configuration/ConfigArgsConfigSourceTest.java
View file

@ -33,5 +33,12 @@ public class ConfigArgsConfigSourceTest {
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);
}
@Test
public void testParseArgsWithSpi() {
List<String> values = new ArrayList<>();
ConfigArgsConfigSource.parseConfigArgs(Arrays.asList("--spi-some-thing-enabled=value", "--spi-some-thing-else", "other-value"), (key, value) -> values.add(key+'='+value), ignored -> {});
assertEquals(Arrays.asList("--spi-some-thing-enabled=value", "--spi-some-thing-else=other-value"), values);
}
}

27
quarkus/tests/junit5/src/main/java/org/keycloak/it/junit5/extension/CLITestExtension.java
View file

@ -46,7 +46,6 @@ import java.util.Arrays;
import java.util.Collections;
import java.util.HashMap;
import java.util.List;
import java.util.regex.Pattern;
import java.util.stream.Collectors;
import java.util.stream.Stream;
@ -60,7 +59,6 @@ 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 DatabaseContainer databaseContainer;
private InfinispanContainer infinispanContainer;
@ -73,24 +71,17 @@ public class CLITestExtension extends QuarkusMainTestExtension {
getStore(context).put(SYS_PROPS, new HashMap<>(System.getProperties()));
if (launch != null && distConfig == null) {
for (String arg : launch.value()) {
if (arg.contains(CONFIG_FILE_SHORT_NAME) || arg.contains(CONFIG_FILE_LONG_NAME)) {
Pattern kvSeparator = Pattern.compile(KEY_VALUE_SEPARATOR);
String[] cfKeyValue = kvSeparator.split(arg);
setProperty(KeycloakPropertiesConfigSource.KEYCLOAK_CONFIG_FILE_PROP, cfKeyValue[1]);
ConfigArgsConfigSource.parseConfigArgs(List.of(launch.value()), (arg, value) -> {
if (arg.equals(CONFIG_FILE_SHORT_NAME) || arg.equals(CONFIG_FILE_LONG_NAME)) {
setProperty(KeycloakPropertiesConfigSource.KEYCLOAK_CONFIG_FILE_PROP, value);
} else if (arg.startsWith("-D")) {
// allow setting system properties from JVM tests
int keyValueSeparator = arg.indexOf('=');
if (keyValueSeparator == -1) {
continue;
}
String name = arg.substring(2, keyValueSeparator);
String value = arg.substring(keyValueSeparator + 1);
setProperty(name, value);
setProperty(arg, value);
}
}
}, arg -> {
if (arg.startsWith("-D")) {
setProperty(arg, "");
}
});
}
configureDatabase(context);