From abf707f124dff38c77f5c9f82713282a411a9df3 Mon Sep 17 00:00:00 2001 From: Steve Hawkins Date: Tue, 1 Jul 2025 10:00:52 -0400 Subject: [PATCH 1/2] fix: map just logging env wildcards to . closes: #40833 Signed-off-by: Steve Hawkins --- docs/guides/server/configuration.adoc | 9 ++++++--- .../mappers/WildcardPropertyMapper.java | 13 ++++++++++--- .../keycloak/quarkus/runtime/cli/PicocliTest.java | 7 +------ .../configuration/DatasourcesConfigurationTest.java | 13 +++++++------ .../storage/database/dist/DatasourcesDistTest.java | 2 +- 5 files changed, 25 insertions(+), 19 deletions(-) diff --git a/docs/guides/server/configuration.adoc b/docs/guides/server/configuration.adoc index 0fcc85caebc5..16f00adda9d9 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`. +Env 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 period `.` 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 83da1cc8202d..70950ec5726e 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.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 = '-'; 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 WildcardPropertyMapper(Option option, String to, BooleanSupplier enabl 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 boolean matchesWildcardOptionName(String name) { 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/cli/PicocliTest.java b/quarkus/runtime/src/test/java/org/keycloak/quarkus/runtime/cli/PicocliTest.java index f62b1c283731..21122d868b6d 100644 --- a/quarkus/runtime/src/test/java/org/keycloak/quarkus/runtime/cli/PicocliTest.java +++ b/quarkus/runtime/src/test/java/org/keycloak/quarkus/runtime/cli/PicocliTest.java @@ -118,6 +118,7 @@ NonRunningPicocli pseudoLaunch(String... args) { ConfigArgsConfigSource.setCliArgs(args); nonRunningPicocli.config = createConfig(); KeycloakMain.main(args, nonRunningPicocli); + onAfter(); return nonRunningPicocli; } @@ -347,7 +348,6 @@ private NonRunningPicocli build(Consumer outChecker, String... args) { assertTrue(nonRunningPicocli.reaug); assertEquals(nonRunningPicocli.getErrString(), CommandLine.ExitCode.OK, nonRunningPicocli.exitCode); outChecker.accept(nonRunningPicocli.getOutString()); - onAfter(); addPersistedConfigValues((Map)nonRunningPicocli.buildProps); return nonRunningPicocli; } @@ -467,7 +467,6 @@ public void warnDBRequired() { assertEquals(CommandLine.ExitCode.OK, nonRunningPicocli.exitCode); assertThat(nonRunningPicocli.getOutString(), not(containsString("Usage of the default value for the db option"))); - onAfter(); // prod profiles warn about db nonRunningPicocli = pseudoLaunch("build"); @@ -691,17 +690,13 @@ public void logAsyncGlobal() { NonRunningPicocli nonRunningPicocli = pseudoLaunch("start-dev", "--log-async=true", "--log-console-async=false", "--log-console-async-queue-length=222"); assertEquals(CommandLine.ExitCode.USAGE, nonRunningPicocli.exitCode); assertThat(nonRunningPicocli.getErrString(), containsString("Disabled option: '--log-console-async-queue-length'. Available only when Console log handler is activated")); - onAfter(); nonRunningPicocli = pseudoLaunch("start-dev", "--log-async=true", "--log-console-async-queue-length=222"); assertEquals(CommandLine.ExitCode.OK, nonRunningPicocli.exitCode); - onAfter(); - nonRunningPicocli = pseudoLaunch("start-dev", "--log=console,file", "--log-async=true", "--log-console-async=false", "--log-console-async-queue-length=222"); assertEquals(CommandLine.ExitCode.USAGE, nonRunningPicocli.exitCode); assertThat(nonRunningPicocli.getErrString(), containsString("Disabled option: '--log-console-async-queue-length'. Available only when Console log handler is activated")); - onAfter(); nonRunningPicocli = pseudoLaunch("start-dev", "--log=console,file", "--log-async=true", "--log-console-async=false", "--log-file-async-queue-length=222"); assertEquals(CommandLine.ExitCode.OK, nonRunningPicocli.exitCode); 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 26a73c6ab539..b289dcf9231b 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.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 void envVarsHandling() { "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 1ceea74003be..4bfb3b2d94c4 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 void multipleDatasourcesPrint(CLIResult result) { @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(); } From 16dd080d0beed5ded30299c02cf31ceb8dcff82f Mon Sep 17 00:00:00 2001 From: Steve Hawkins Date: Tue, 1 Jul 2025 12:18:26 -0400 Subject: [PATCH 2/2] updates based upon review comments Signed-off-by: Steve Hawkins --- docs/guides/server/configuration.adoc | 4 ++-- .../configuration/mappers/WildcardPropertyMapper.java | 2 +- .../java/org/keycloak/quarkus/runtime/cli/PicocliTest.java | 7 ++++++- 3 files changed, 9 insertions(+), 4 deletions(-) diff --git a/docs/guides/server/configuration.adoc b/docs/guides/server/configuration.adoc index 16f00adda9d9..150d16134336 100644 --- a/docs/guides/server/configuration.adoc +++ b/docs/guides/server/configuration.adoc @@ -169,9 +169,9 @@ PowerShell handles quotes differently. It interprets the quoted string before pa Non-alphanumeric characters in your configuration key must be converted to `_` in the corresponding environment variable key. -Env 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 period `.` instead. Logging categories are commonly class / package names, which are more likely to use `.` rather than `-`. +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 +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`. 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 70950ec5726e..885dfd9224a4 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 @@ -29,7 +29,7 @@ public class WildcardPropertyMapper extends PropertyMapper { private final String fromPrefix; private String toPrefix; private String toSuffix; - private Character replacementChar = '-'; + 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, diff --git a/quarkus/runtime/src/test/java/org/keycloak/quarkus/runtime/cli/PicocliTest.java b/quarkus/runtime/src/test/java/org/keycloak/quarkus/runtime/cli/PicocliTest.java index 21122d868b6d..f62b1c283731 100644 --- a/quarkus/runtime/src/test/java/org/keycloak/quarkus/runtime/cli/PicocliTest.java +++ b/quarkus/runtime/src/test/java/org/keycloak/quarkus/runtime/cli/PicocliTest.java @@ -118,7 +118,6 @@ NonRunningPicocli pseudoLaunch(String... args) { ConfigArgsConfigSource.setCliArgs(args); nonRunningPicocli.config = createConfig(); KeycloakMain.main(args, nonRunningPicocli); - onAfter(); return nonRunningPicocli; } @@ -348,6 +347,7 @@ private NonRunningPicocli build(Consumer outChecker, String... args) { assertTrue(nonRunningPicocli.reaug); assertEquals(nonRunningPicocli.getErrString(), CommandLine.ExitCode.OK, nonRunningPicocli.exitCode); outChecker.accept(nonRunningPicocli.getOutString()); + onAfter(); addPersistedConfigValues((Map)nonRunningPicocli.buildProps); return nonRunningPicocli; } @@ -467,6 +467,7 @@ public void warnDBRequired() { assertEquals(CommandLine.ExitCode.OK, nonRunningPicocli.exitCode); assertThat(nonRunningPicocli.getOutString(), not(containsString("Usage of the default value for the db option"))); + onAfter(); // prod profiles warn about db nonRunningPicocli = pseudoLaunch("build"); @@ -690,13 +691,17 @@ public void logAsyncGlobal() { NonRunningPicocli nonRunningPicocli = pseudoLaunch("start-dev", "--log-async=true", "--log-console-async=false", "--log-console-async-queue-length=222"); assertEquals(CommandLine.ExitCode.USAGE, nonRunningPicocli.exitCode); assertThat(nonRunningPicocli.getErrString(), containsString("Disabled option: '--log-console-async-queue-length'. Available only when Console log handler is activated")); + onAfter(); nonRunningPicocli = pseudoLaunch("start-dev", "--log-async=true", "--log-console-async-queue-length=222"); assertEquals(CommandLine.ExitCode.OK, nonRunningPicocli.exitCode); + onAfter(); + nonRunningPicocli = pseudoLaunch("start-dev", "--log=console,file", "--log-async=true", "--log-console-async=false", "--log-console-async-queue-length=222"); assertEquals(CommandLine.ExitCode.USAGE, nonRunningPicocli.exitCode); assertThat(nonRunningPicocli.getErrString(), containsString("Disabled option: '--log-console-async-queue-length'. Available only when Console log handler is activated")); + onAfter(); nonRunningPicocli = pseudoLaunch("start-dev", "--log=console,file", "--log-async=true", "--log-console-async=false", "--log-file-async-queue-length=222"); assertEquals(CommandLine.ExitCode.OK, nonRunningPicocli.exitCode);