Enable validations for wildcard options

Signed-off-by: Václav Muzikář <[email protected]>
vmuzikar · Nov 29, 2024 · 580d854 · 580d854
1 parent a66fc3f
commit 580d854
Show file tree

Hide file tree

Showing 6 changed files with 60 additions and 38 deletions.
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
@@ -48,6 +48,7 @@
 import java.util.function.Consumer;
 import java.util.function.Function;
 import java.util.stream.Collectors;
+import java.util.stream.StreamSupport;
 
 import org.keycloak.common.profile.ProfileException;
 import org.keycloak.config.DeprecatedMetadata;
@@ -379,17 +380,33 @@ public void validateConfig(List<String> cliArgs, AbstractCommand abstractCommand
                 Optional.ofNullable(PropertyMappers.getRuntimeMappers().get(category)).ifPresent(mappers::addAll);
                 Optional.ofNullable(PropertyMappers.getBuildTimeMappers().get(category)).ifPresent(mappers::addAll);
                 for (PropertyMapper<?> mapper : mappers) {
-                    ConfigValue configValue = Configuration.getConfigValue(mapper.getFrom());
-                    String configValueStr = configValue.getValue();
+                    ConfigValue firstConfigValue;
+                    Map<String, ConfigValue> configValues;
+
+                    if (mapper.hasWildcard()) {
+                        // filter out null values
+                        // this might happen when we're generating some values in wildcardValuesTransformer,
+                        // but mappers are now disabled so such values will be null
+                        // but that's fine, we don't care about these for validation
+                        configValues = Configuration.getKcConfigValues(mapper).entrySet().stream()
+                                .filter(e -> e.getValue().getValue() != null)
+                                .collect(Collectors.toMap(Map.Entry::getKey, Map.Entry::getValue));
+                        firstConfigValue = !configValues.isEmpty() ? configValues.values().iterator().next() : ConfigValue.builder().build();
+                    } else {
+                        firstConfigValue = Configuration.getConfigValue(mapper.getFrom());
+                        configValues = Map.of(mapper.getFrom(), firstConfigValue);
+                    }
+
+                    String firstConfigValueStr = firstConfigValue.getValue();
 
                     // don't consider missing or anything below standard env properties
-                    if (configValueStr == null) {
+                    if (firstConfigValueStr == null) {
                         if (Environment.isRuntimeMode() && mapper.isEnabled() && mapper.isRequired()) {
                             handleRequired(missingOption, mapper);
                         }
                         continue;
                     }
-                    if (!isUserModifiable(configValue)) {
+                    if (!isUserModifiable(firstConfigValue)) {
                         continue;
                     }
 
@@ -400,7 +417,7 @@ public void validateConfig(List<String> cliArgs, AbstractCommand abstractCommand
 
                         // only check build-time for a rebuild, we'll check the runtime later
                         if (!mapper.isRunTime() || !isRebuild()) {
-                            if (PropertyMapper.isCliOption(configValue)) {
+                            if (PropertyMapper.isCliOption(firstConfigValue)) {
                                 throw new KcUnmatchedArgumentException(abstractCommand.getCommandLine().orElseThrow(), List.of(mapper.getCliFormat()));
                             } else {
                                 handleDisabled(mapper.isRunTime() ? disabledRunTime : disabledBuildTime, mapper);
@@ -411,7 +428,7 @@ public void validateConfig(List<String> cliArgs, AbstractCommand abstractCommand
 
                     if (mapper.isBuildTime() && !options.includeBuildTime) {
                         String currentValue = getRawPersistedProperty(mapper.getFrom()).orElse(null);
-                        if (!configValueStr.equals(currentValue)) {
+                        if (!firstConfigValueStr.equals(currentValue)) {
                             ignoredBuildTime.add(mapper.getFrom());
                             continue;
                         }
@@ -421,10 +438,10 @@ public void validateConfig(List<String> cliArgs, AbstractCommand abstractCommand
                         continue;
                     }
 
-                    mapper.validate(configValue);
+                    configValues.forEach((k, v) -> mapper.validate(v));
 
                     mapper.getDeprecatedMetadata().ifPresent(metadata -> {
-                        handleDeprecated(deprecatedInUse, mapper, configValueStr, metadata);
+                        handleDeprecated(deprecatedInUse, mapper, firstConfigValueStr, metadata);
                     });
                 }
             }

diff --git a/...time/src/main/java/org/keycloak/quarkus/runtime/configuration/ConfigArgsConfigSource.java b/...time/src/main/java/org/keycloak/quarkus/runtime/configuration/ConfigArgsConfigSource.java
@@ -131,11 +131,13 @@ public void accept(String key, String value) {
                 if (mapper != null) {
                     String to = mapper.getTo();
 
+                    String wildcardValue = mapper.getWildcardValue(key).orElse(null);
+
                     if (to != null) {
-                        properties.put(mapper.getTo(key), value);
+                        properties.put(mapper.getTo(wildcardValue), value);
                     }
 
-                    properties.put(mapper.getFrom(key), value);
+                    properties.put(mapper.getFrom(wildcardValue), value);
                 }
             }
         }, ignored -> {});

diff --git a/quarkus/runtime/src/main/java/org/keycloak/quarkus/runtime/configuration/Configuration.java b/quarkus/runtime/src/main/java/org/keycloak/quarkus/runtime/configuration/Configuration.java
@@ -22,6 +22,7 @@
 import java.util.Map;
 import java.util.Optional;
 import java.util.Properties;
+import java.util.stream.Collectors;
 
 import io.smallrye.config.ConfigValue;
 import io.smallrye.config.SmallRyeConfig;
@@ -112,6 +113,25 @@ public static ConfigValue getKcConfigValue(String propertyName) {
         return getConfigValue(NS_KEYCLOAK_PREFIX.concat(propertyName));
     }
 
+    /**
+     * Get all Keycloak multivalued config values for the mapper. A multivalued config option is a config option that
+     * has a wildcard in its name, e.g. log-level-<category>.
+     *
+     * @return a map of config values where the key is the resolved wildcard (e.g. category) and the value is the config value
+     */
+    public static Map<String, ConfigValue> getKcConfigValues(PropertyMapper<?> mapper) {
+        return mapper.getWildcardValues().stream()
+                .collect(Collectors.toMap(v -> v, v -> getConfigValue(mapper.getFrom(v))));
+    }
+
+    public static Map<String, ConfigValue> getKcConfigValues(String propertyName) {
+        PropertyMapper<?> mapper = PropertyMappers.getMapper(propertyName);
+        if (mapper == null) {
+            return null;
+        }
+        return getKcConfigValues(mapper);
+    }
+
     public static Optional<String> getOptionalValue(String name) {
         return getConfig().getOptionalValue(name, String.class);
     }

diff --git a/...s/runtime/src/main/java/org/keycloak/quarkus/runtime/configuration/KcEnvConfigSource.java b/...s/runtime/src/main/java/org/keycloak/quarkus/runtime/configuration/KcEnvConfigSource.java
@@ -56,7 +56,7 @@ private static Map<String, String> buildProperties() {
                         properties.put(to, value);
                     }
 
-                    properties.put(mapper.getFrom(key), value);
+                    properties.put(mapper.getFrom(mapper.getWildcardValue(key).orElse(null)), value);
                 }
                 else {
                     // most probably an SPI but could be also something else

diff --git a/...time/src/main/java/org/keycloak/quarkus/runtime/configuration/mappers/PropertyMapper.java b/...time/src/main/java/org/keycloak/quarkus/runtime/configuration/mappers/PropertyMapper.java
@@ -175,20 +175,9 @@ ConfigValue getConfigValue(String name, ConfigSourceInterceptorContext context)
         return context.proceed(name);
     }
 
-    /**
-     * Get all Keycloak multivalued config values for the mapper. A multivalued config option is a config option that
-     * has a wildcard in its name, e.g. log-level-<category>.
-     *
-     * @return a map of config values where the key is the resolved wildcard (e.g. category) and the value is the config value
-     */
-    public Map<String, ConfigValue> getWildcardConfigValues() {
-        return getWildcardValues().stream()
-                .collect(Collectors.toMap(v -> v, Configuration::getKcConfigValue));
-    }
-
     public Set<String> getWildcardValues() {
         if (!hasWildcard()) {
-            throw new IllegalArgumentException("Option does not have wildcard");
+            return Set.of();
         }
 
         // this is not optimal
@@ -252,13 +241,10 @@ public Class<T> getType() {
         return this.option.getType();
     }
 
-    public String getFrom(String keyWithWildcardValue) {
+    public String getFrom(String wildcardValue) {
         String from = this.option.getKey();
-        if (hasWildcard() && keyWithWildcardValue != null) {
-            Optional<String> wildcardValue = getWildcardValue(keyWithWildcardValue);
-            if (wildcardValue.isPresent()) {
-                from = fromWildcardMatcher.replaceFirst(wildcardValue.get());
-            }
+        if (hasWildcard() && wildcardValue != null) {
+            from = fromWildcardMatcher.replaceFirst(wildcardValue);
         }
         return MicroProfileConfigProvider.NS_KEYCLOAK_PREFIX + from;
     }
@@ -301,13 +287,10 @@ public boolean isRunTime() {
         return !this.option.isBuildTime();
     }
 
-    public String getTo(String keyWithWildcardValue) {
+    public String getTo(String wildcardValue) {
         String to = this.to;
-        if (hasWildcard() && keyWithWildcardValue != null) {
-            Optional<String> wildcardValue = getWildcardValue(keyWithWildcardValue);
-            if (wildcardValue.isPresent()) {
-                to = toWildcardMatcher.replaceFirst(wildcardValue.get());
-            }
+        if (hasWildcard() && wildcardValue != null) {
+            to = toWildcardMatcher.replaceFirst(wildcardValue);
         }
         return to;
     }
@@ -350,7 +333,7 @@ public boolean hasWildcard() {
      */
     public boolean matchesWildcardOptionName(String name) {
         if (!hasWildcard()) {
-            throw new IllegalStateException("Option does not have wildcard");
+            return false;
         }
         return fromWildcardPattern.matcher(name).matches() || envVarNameWildcardPattern.matcher(name).matches()
                 || (toWildcardPattern != null && toWildcardPattern.matcher(name).matches());
@@ -363,7 +346,7 @@ public boolean matchesWildcardOptionName(String name) {
      */
     private Optional<String> getWildcardValue(String option, boolean includeMappedToOptions) {
         if (!hasWildcard()) {
-            throw new IllegalStateException("Option does not have wildcard");
+            return Optional.empty();
         }
 
         Matcher matcher = fromWildcardPattern.matcher(option);

diff --git a/quarkus/tests/integration/src/test/java/org/keycloak/it/cli/dist/LoggingDistTest.java b/quarkus/tests/integration/src/test/java/org/keycloak/it/cli/dist/LoggingDistTest.java
@@ -178,7 +178,7 @@ void wrongLevelForHandlers(CLIResult cliResult) {
     @Launch({"start-dev", "--log-level-org.keycloak=wrong"})
     @DryRun
     void wrongLevelForCategory(CLIResult cliResult) {
-        cliResult.assertError("Invalid value for option '--log-level-org.keycloak': wrong. Expected values are (case insensitive): off, fatal, error, warn, info, debug, trace, all");
+        cliResult.assertError("Invalid log level: wrong. Possible values are: warn, trace, debug, error, fatal, info.");
     }
 
     @Test