task: use simple display names for config sources (#28411)

closes: #24192

Signed-off-by: Steve Hawkins <shawkins@redhat.com>
This commit is contained in:
Steven Hawkins 2024-04-09 11:56:41 -04:00 committed by GitHub
parent 6f8ac65d3c
commit 9774deda6c
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
10 changed files with 72 additions and 35 deletions

View file

@ -67,16 +67,11 @@ public final class PropertyMapperParameterConsumer implements CommandLine.IParam
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) {
return !(arg.startsWith(ARG_PREFIX) || arg.startsWith(Picocli.ARG_SHORT_PREFIX));
}
static String getExpectedValuesMessage(Iterable<String> specCandidates, Iterable<String> optionCandidates) {
public static String getExpectedValuesMessage(Iterable<String> specCandidates, Iterable<String> optionCandidates) {
return optionCandidates.iterator().hasNext() ? " Expected values are: " + String.join(", ", specCandidates) : "";
}

View file

@ -35,7 +35,7 @@ import java.util.stream.Collectors;
import java.util.stream.StreamSupport;
import org.keycloak.quarkus.runtime.Environment;
import org.keycloak.quarkus.runtime.configuration.Configuration;
import org.keycloak.quarkus.runtime.configuration.KeycloakConfigSourceProvider;
import org.keycloak.quarkus.runtime.configuration.MicroProfileConfigProvider;
import org.keycloak.quarkus.runtime.configuration.PersistedConfigSource;
import org.keycloak.quarkus.runtime.configuration.mappers.PropertyMapper;
@ -148,7 +148,7 @@ public final class ShowConfig extends AbstractCommand implements Runnable {
value = getRuntimeProperty(property).orElse(value);
}
spec.commandLine().getOut().printf("\t%s = %s (%s)%n", configValue.getName(), formatValue(configValue.getName(), value), configValue.getConfigSourceName());
spec.commandLine().getOut().printf("\t%s = %s (%s)%n", configValue.getName(), formatValue(configValue.getName(), value), KeycloakConfigSourceProvider.getConfigSourceDisplayName(configValue.getConfigSourceName()));
}
private static String groupProperties(String property) {

View file

@ -29,6 +29,8 @@ import io.smallrye.config.EnvConfigSource;
public class KcEnvConfigSource extends EnvConfigSource {
public static final String NAME = "KcEnvVarConfigSource";
public KcEnvConfigSource() {
super(buildProperties(), 500);
}
@ -63,6 +65,6 @@ public class KcEnvConfigSource extends EnvConfigSource {
@Override
public String getName() {
return "KcEnvVarConfigSource";
return NAME;
}
}

View file

@ -18,8 +18,12 @@
package org.keycloak.quarkus.runtime.configuration;
import java.nio.file.Path;
import java.util.ArrayList;
import java.util.Collection;
import java.util.HashMap;
import java.util.List;
import java.util.Map;
import io.quarkus.runtime.configuration.ConfigBuilder;
import io.smallrye.config.SmallRyeConfigBuilder;
@ -30,6 +34,7 @@ import org.keycloak.quarkus.runtime.Environment;
public class KeycloakConfigSourceProvider implements ConfigSourceProvider, ConfigBuilder {
private static final List<ConfigSource> CONFIG_SOURCES = new ArrayList<>();
private static final Map<String, String> CONFIG_SOURCE_DISPLAY_NAMES = new HashMap<>();
// we initialize in a static block to avoid discovering the config sources multiple times when starting the application
static {
@ -43,17 +48,29 @@ public class KeycloakConfigSourceProvider implements ConfigSourceProvider, Confi
System.setProperty("quarkus.profile", profile);
}
CONFIG_SOURCES.add(new ConfigArgsConfigSource());
CONFIG_SOURCES.add(new KcEnvConfigSource());
addConfigSources("CLI", List.of(new ConfigArgsConfigSource()));
CONFIG_SOURCES.addAll(new QuarkusPropertiesConfigSource().getConfigSources(Thread.currentThread().getContextClassLoader()));
addConfigSources("ENV", List.of(new KcEnvConfigSource()));
CONFIG_SOURCES.add(PersistedConfigSource.getInstance());
addConfigSources("quarkus.properties", new QuarkusPropertiesConfigSource().getConfigSources(Thread.currentThread().getContextClassLoader()));
CONFIG_SOURCES.addAll(new KeycloakPropertiesConfigSource.InFileSystem().getConfigSources(Thread.currentThread().getContextClassLoader()));
addConfigSources("Persisted", List.of(PersistedConfigSource.getInstance()));
KeycloakPropertiesConfigSource.InFileSystem inFileSystem = new KeycloakPropertiesConfigSource.InFileSystem();
Path path = inFileSystem.getConfigurationFile();
if (path != null) {
addConfigSources(path.getFileName().toString(), inFileSystem.getConfigSources(Thread.currentThread().getContextClassLoader(), path));
}
// by enabling this config source we are able to rely on the default settings when running tests
CONFIG_SOURCES.addAll(new KeycloakPropertiesConfigSource.InClassPath().getConfigSources(Thread.currentThread().getContextClassLoader()));
addConfigSources("classpath keycloak.conf", new KeycloakPropertiesConfigSource.InClassPath().getConfigSources(Thread.currentThread().getContextClassLoader()));
}
private static void addConfigSources(String displayName, Collection<ConfigSource> configSources) {
for (ConfigSource cs : configSources) {
CONFIG_SOURCES.add(cs);
CONFIG_SOURCE_DISPLAY_NAMES.put(cs.getName(), displayName);
}
}
/**
@ -62,6 +79,7 @@ public class KeycloakConfigSourceProvider implements ConfigSourceProvider, Confi
*/
public static void reload() {
CONFIG_SOURCES.clear();
CONFIG_SOURCE_DISPLAY_NAMES.clear();
initializeSources();
}
@ -77,4 +95,14 @@ public class KeycloakConfigSourceProvider implements ConfigSourceProvider, Confi
public SmallRyeConfigBuilder configBuilder(SmallRyeConfigBuilder builder) {
return builder.withSources(CONFIG_SOURCES);
}
public static String getConfigSourceDisplayName(String configSource) {
if (configSource == null) {
return "Derived";
}
if (configSource.startsWith("KeyStoreConfigSource")) {
return "config-keystore";
}
return CONFIG_SOURCE_DISPLAY_NAMES.getOrDefault(configSource, configSource);
}
}

View file

@ -106,6 +106,10 @@ public class KeycloakPropertiesConfigSource extends AbstractLocationConfigSource
return Collections.emptyList();
}
return getConfigSources(classLoader, configFile);
}
public List<ConfigSource> getConfigSources(final ClassLoader classLoader, Path configFile) {
return loadConfigSources(configFile.toUri().toString(), 450, classLoader);
}
@ -114,7 +118,7 @@ public class KeycloakPropertiesConfigSource extends AbstractLocationConfigSource
return Collections.emptyList();
}
private Path getConfigurationFile() {
public Path getConfigurationFile() {
String filePath = System.getProperty(KEYCLOAK_CONFIG_FILE_PROP);
if (filePath == null)
@ -164,9 +168,9 @@ public class KeycloakPropertiesConfigSource extends AbstractLocationConfigSource
}
/**
* We need a better namespace resolution so that we don't need to add Quarkus extensions manually. Maybe the easiest
* We need a better namespace resolution so that we don't need to add Quarkus extensions manually. Maybe the easiest
* path is to just have the "kc" namespace for Keycloak-specific properties.
*
*
* @param key the key to transform
* @return the same key but prefixed with the namespace
*/

View file

@ -39,6 +39,8 @@ import org.keycloak.config.OptionCategory;
import org.keycloak.quarkus.runtime.cli.PropertyException;
import org.keycloak.quarkus.runtime.cli.PropertyMapperParameterConsumer;
import org.keycloak.quarkus.runtime.configuration.ConfigArgsConfigSource;
import org.keycloak.quarkus.runtime.configuration.KcEnvConfigSource;
import org.keycloak.quarkus.runtime.configuration.KeycloakConfigSourceProvider;
import org.keycloak.quarkus.runtime.Environment;
import org.keycloak.quarkus.runtime.configuration.MicroProfileConfigProvider;
import org.keycloak.utils.StringUtil;
@ -353,11 +355,9 @@ public class PropertyMapper<T> {
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));
throw new PropertyException("Invalid value for multivalued option " + getOptionAndSourceMessage(configValue)
+ ": list value '" + v + "' should not have leading nor trailing whitespace");
}
singleValidator.accept(configValue, v);
}
@ -367,18 +367,28 @@ public class PropertyMapper<T> {
return Optional.ofNullable(configValue.getConfigSourceName()).filter(name -> name.contains(ConfigArgsConfigSource.NAME)).isPresent();
}
public static boolean isEnvOption(ConfigValue configValue) {
return Optional.ofNullable(configValue.getConfigSourceName()).filter(name -> name.contains(KcEnvConfigSource.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(), v,
expectedValues) + getConfigSourceMessage(configValue, cli));
String.format("Invalid value for option %s: %s.%s", getOptionAndSourceMessage(configValue), v,
PropertyMapperParameterConsumer.getExpectedValuesMessage(expectedValues, expectedValues)));
}
}
String getConfigSourceMessage(ConfigValue configValue, boolean cli) {
return cli ? "" : ". From ConfigSource " + configValue.getConfigSourceName();
String getOptionAndSourceMessage(ConfigValue configValue) {
if (isCliOption(configValue)) {
return String.format("'%s'", this.getCliFormat());
}
if (isEnvOption(configValue)) {
return String.format("'%s'", this.getEnvVarFormat());
}
return String.format("'%s' in %s", getFrom(),
KeycloakConfigSourceProvider.getConfigSourceDisplayName(configValue.getConfigSourceName()));
}
}

View file

@ -76,14 +76,14 @@ class BuildCommandDistTest {
@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");
cliResult.assertError("Invalid value for option 'KC_DB': invalid. Expected values are: dev-file, dev-mem, mariadb, mssql, mysql, oracle, postgres");
}
@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");
cliResult.assertError("Invalid value for option 'kc.db' in keycloak.conf: foo. Expected values are: dev-file, dev-mem, mariadb, mssql, mysql, oracle, postgres");
}
@Test

View file

@ -25,8 +25,6 @@ import static org.keycloak.quarkus.runtime.cli.command.Main.CONFIG_FILE_LONG_NAM
import com.fasterxml.jackson.core.JsonProcessingException;
import io.restassured.RestAssured;
import org.junit.jupiter.api.Test;
import org.junit.jupiter.api.condition.EnabledOnOs;
import org.junit.jupiter.api.condition.OS;
import org.keycloak.config.LoggingOptions;
import org.keycloak.it.junit5.extension.CLIResult;
import org.keycloak.it.junit5.extension.DistributionTest;
@ -145,14 +143,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.assertError("Invalid value for option 'kc.log': foo. Expected values are: console, file, syslog, gelf.");
cliResult.assertError("Invalid value for option 'kc.log' in keycloak.conf: foo. Expected values are: console, file, syslog, 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.assertError("Invalid value for option 'kc.log': . Expected values are: console, file, syslog, gelf.");
cliResult.assertError("Invalid value for option 'kc.log' in emptylog.conf: . Expected values are: console, file, syslog, gelf");
}
@Test

View file

@ -114,7 +114,7 @@ public class StartCommandDistTest {
@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, syslog, gelf. From ConfigSource KcEnvVarConfigSource");
cliResult.assertError("Invalid value for option 'KC_LOG': invalid. Expected values are: console, file, syslog, gelf");
}
@Test

View file

@ -37,6 +37,6 @@ public class PostgreSQLDistTest extends PostgreSQLTest {
@Launch("show-config")
public void testDbOptionFromPersistedConfigSource(LaunchResult result) {
CLIResult cliResult = (CLIResult) result;
assertThat(cliResult.getOutput(),containsString("postgres (PersistedConfigSource)"));
assertThat(cliResult.getOutput(),containsString("postgres (Persisted)"));
}
}