From 3760e726cd2cee22155eebf3d74c2121c9e9ee99 Mon Sep 17 00:00:00 2001 From: Steven Hawkins Date: Wed, 2 Jul 2025 04:06:29 -0400 Subject: [PATCH] fix: map just logging env wildcards to . (#40834) * fix: map just logging env wildcards to . closes: #40833 Signed-off-by: Steve Hawkins * updates based upon review comments Signed-off-by: Steve Hawkins --------- Signed-off-by: Steve Hawkins --- docs/guides/server/configuration.adoc | 9 ++++++--- .../mappers/WildcardPropertyMapper.java | 13 ++++++++++--- .../configuration/DatasourcesConfigurationTest.java | 13 +++++++------ .../storage/database/dist/DatasourcesDistTest.java | 2 +- 4 files changed, 24 insertions(+), 13 deletions(-) diff --git a/docs/guides/server/configuration.adoc b/docs/guides/server/configuration.adoc index 0fcc85caebc..150d1613433 100644 --- a/docs/guides/server/configuration.adoc +++ b/docs/guides/server/configuration.adoc @@ -167,10 +167,13 @@ PowerShell handles quotes differently. It interprets the quoted string before pa === Formats for environment variable keys with special characters -Some configuration keys, such as those for logging, may contain characters such as `_` or `$` - e.g. `kc.log-level-package.class_name`. Non-alphanumeric characters in your configuration key must be converted to `\_` in the corresponding environment variable key. +Non-alphanumeric characters in your configuration key must be converted to `_` in the corresponding environment variable key. -The automatic handling of the environment variable key may not preserve the intended key. For example `KC_LOG_LEVEL_PACKAGE_CLASS_NAME` will become `kc.log-level-package.class.name` because logging properties default to replacing `_` in the "wildcard" -part of the key with `.` as that is what is most commonly needed in a class / package name. However this does not match the intent of `kc.log-level-package.class_name`. +Environment variables are converted back to normal option keys by lower-casing the name and replacing `\_` with `-`. Logging wildcards are the exception as `_` in the category is replaced with `.` instead. Logging categories are commonly class / package names, which are more likely to use `.` rather than `-`. + +WARNING: Automatic mapping of the environment variable key to option key may not preserve the intended key + +For example `kc.log-level-package.class_name` will become the environment variable key `KC_LOG_LEVEL_PACKAGE_CLASS_NAME`. That will automatically be mapped to `kc.log-level-package.class.name` because `_` in the logging category will be replaced by `.`. Unfortunately this does not match the intent of `kc.log-level-package.class_name`. You have a couple of options in this case: 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 83da1cc8202..885dfd9224a 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 @@ -9,10 +9,9 @@ import java.util.function.BooleanSupplier; import java.util.regex.Pattern; import java.util.stream.Stream; +import org.keycloak.config.LoggingOptions; import org.keycloak.config.Option; -import org.keycloak.quarkus.runtime.configuration.MicroProfileConfigProvider; -import io.smallrye.config.ConfigSourceInterceptorContext; import io.smallrye.config.ConfigValue; import static org.keycloak.quarkus.runtime.configuration.MicroProfileConfigProvider.NS_KEYCLOAK_PREFIX; @@ -30,6 +29,7 @@ public class WildcardPropertyMapper extends PropertyMapper { private final String fromPrefix; private String toPrefix; private String toSuffix; + private Character replacementChar = null; public WildcardPropertyMapper(Option option, String to, BooleanSupplier enabled, String enabledWhen, ValueMapper mapper, String mapFrom, ValueMapper parentMapper, String paramLabel, boolean mask, BiConsumer, ConfigValue> validator, @@ -42,6 +42,10 @@ public class WildcardPropertyMapper extends PropertyMapper { throw new IllegalArgumentException("Invalid wildcard from format. Wildcard must be at the end of the option."); } + if (option == LoggingOptions.LOG_LEVEL_CATEGORY) { + replacementChar = '.'; + } + if (to != null) { if (!to.startsWith(NS_QUARKUS_PREFIX) && !to.startsWith(NS_KEYCLOAK_PREFIX)) { throw new IllegalArgumentException("Wildcards should map to Quarkus or Keycloak options (option '%s' mapped to '%s'). If not, PropertyMappers logic will need adjusted".formatted(option.getKey(), to)); @@ -119,7 +123,10 @@ public class WildcardPropertyMapper extends PropertyMapper { public Optional getKcKeyForEnvKey(String envKey, String transformedKey) { if (transformedKey.startsWith(fromPrefix)) { - return Optional.ofNullable(getFrom(envKey.substring(fromPrefix.length()).toLowerCase().replace("_", "."))); + if (replacementChar != null) { + return Optional.ofNullable(getFrom(envKey.substring(fromPrefix.length()).toLowerCase().replace('_', replacementChar))); + } + return Optional.of(transformedKey); } return Optional.empty(); } diff --git a/quarkus/runtime/src/test/java/org/keycloak/quarkus/runtime/configuration/DatasourcesConfigurationTest.java b/quarkus/runtime/src/test/java/org/keycloak/quarkus/runtime/configuration/DatasourcesConfigurationTest.java index 26a73c6ab53..b289dcf9231 100644 --- a/quarkus/runtime/src/test/java/org/keycloak/quarkus/runtime/configuration/DatasourcesConfigurationTest.java +++ b/quarkus/runtime/src/test/java/org/keycloak/quarkus/runtime/configuration/DatasourcesConfigurationTest.java @@ -8,14 +8,14 @@ import org.hibernate.dialect.MariaDBDialect; import org.hibernate.dialect.PostgreSQLDialect; import org.junit.Test; import org.keycloak.quarkus.runtime.Environment; -import org.keycloak.quarkus.runtime.configuration.ConfigArgsConfigSource; -import org.keycloak.quarkus.runtime.configuration.Configuration; import org.mariadb.jdbc.MariaDbDataSource; import org.postgresql.xa.PGXADataSource; import java.util.Map; +import static org.hamcrest.CoreMatchers.hasItem; import static org.hamcrest.CoreMatchers.is; +import static org.hamcrest.CoreMatchers.not; import static org.hamcrest.MatcherAssert.assertThat; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertTrue; @@ -402,17 +402,18 @@ public class DatasourcesConfigurationTest extends AbstractConfigurationTest { "db-kind-user-store", "postgres", "db-url-full-user-store", "jdbc:postgresql://localhost/KEYCLOAK", "db-username-user-store", "my-username", - "db-kind-my-store", "mariadb", - "db-kind-my.store", "mariadb" + "db-kind-my-store", "mariadb" )); assertExternalConfig(Map.of( "quarkus.datasource.\"user-store\".db-kind", "postgresql", "quarkus.datasource.\"user-store\".jdbc.url", "jdbc:postgresql://localhost/KEYCLOAK", "quarkus.datasource.\"user-store\".username", "my-username", - "quarkus.datasource.\"my-store\".db-kind", "mariadb", - "quarkus.datasource.\"my.store\".db-kind", "mariadb" + "quarkus.datasource.\"my-store\".db-kind", "mariadb" )); + + assertThat(Configuration.getPropertyNames(), hasItem("quarkus.datasource.\"my-store\".db-kind")); + assertThat(Configuration.getPropertyNames(), not(hasItem("quarkus.datasource.\"my.store\".db-kind"))); } @Test diff --git a/quarkus/tests/integration/src/test/java/org/keycloak/it/storage/database/dist/DatasourcesDistTest.java b/quarkus/tests/integration/src/test/java/org/keycloak/it/storage/database/dist/DatasourcesDistTest.java index 1ceea74003b..4bfb3b2d94c 100644 --- a/quarkus/tests/integration/src/test/java/org/keycloak/it/storage/database/dist/DatasourcesDistTest.java +++ b/quarkus/tests/integration/src/test/java/org/keycloak/it/storage/database/dist/DatasourcesDistTest.java @@ -33,7 +33,7 @@ public class DatasourcesDistTest { @WithEnvVars({"KC_DB_KIND_USERS", "postgres", "KC_DB_KIND_MY_AWESOME_CLIENTS", "mariadb"}) @Launch({"build"}) public void specifiedViaEnvVars(CLIResult result) { - result.assertMessage("Multiple datasources are specified: , my.awesome.clients, users"); + result.assertMessage("Multiple datasources are specified: , my-awesome-clients, users"); result.assertBuild(); }