From 7e0cbcafae212758016bfb43454153575f6370f5 Mon Sep 17 00:00:00 2001 From: Steven Hawkins Date: Wed, 29 Nov 2023 13:54:10 -0500 Subject: [PATCH] fix: corrects the db property handling and null mapped values (#25088) closes #25010 Signed-off-by: Steve Hawkins --- .../mappers/DatabasePropertyMappers.java | 6 ++---- .../configuration/mappers/PropertyMapper.java | 15 ++++++++------- .../configuration/test/ConfigurationTest.java | 8 ++++++-- 3 files changed, 16 insertions(+), 13 deletions(-) diff --git a/quarkus/runtime/src/main/java/org/keycloak/quarkus/runtime/configuration/mappers/DatabasePropertyMappers.java b/quarkus/runtime/src/main/java/org/keycloak/quarkus/runtime/configuration/mappers/DatabasePropertyMappers.java index e184aa3364..bb46658033 100644 --- a/quarkus/runtime/src/main/java/org/keycloak/quarkus/runtime/configuration/mappers/DatabasePropertyMappers.java +++ b/quarkus/runtime/src/main/java/org/keycloak/quarkus/runtime/configuration/mappers/DatabasePropertyMappers.java @@ -60,13 +60,11 @@ final class DatabasePropertyMappers { .build(), fromOption(DatabaseOptions.DB_USERNAME) .to("quarkus.datasource.username") - .mapFrom("db") .transformer(DatabasePropertyMappers::resolveUsername) .paramLabel("username") .build(), fromOption(DatabaseOptions.DB_PASSWORD) .to("quarkus.datasource.password") - .mapFrom("db") .transformer(DatabasePropertyMappers::resolvePassword) .paramLabel("password") .isMasked(true) @@ -137,7 +135,7 @@ final class DatabasePropertyMappers { return of("sa"); } - return Database.getDatabaseKind(value.get()).isEmpty() ? value : null; + return value; } private static Optional resolvePassword(Optional value, ConfigSourceInterceptorContext context) { @@ -145,7 +143,7 @@ final class DatabasePropertyMappers { return of("password"); } - return Database.getDatabaseKind(value.get()).isEmpty() ? value : null; + return value; } private static boolean isDevModeDatabase(ConfigSourceInterceptorContext context) { diff --git a/quarkus/runtime/src/main/java/org/keycloak/quarkus/runtime/configuration/mappers/PropertyMapper.java b/quarkus/runtime/src/main/java/org/keycloak/quarkus/runtime/configuration/mappers/PropertyMapper.java index b664b8360e..a1e9982f36 100644 --- a/quarkus/runtime/src/main/java/org/keycloak/quarkus/runtime/configuration/mappers/PropertyMapper.java +++ b/quarkus/runtime/src/main/java/org/keycloak/quarkus/runtime/configuration/mappers/PropertyMapper.java @@ -111,10 +111,10 @@ public class PropertyMapper { } } - return transformValue(name, ofNullable(parentValue == null ? null : parentValue.getValue()), context); + return transformValue(name, ofNullable(parentValue == null ? null : parentValue.getValue()), context, null); } - ConfigValue defaultValue = transformValue(name, this.option.getDefaultValue().map(Objects::toString), context); + ConfigValue defaultValue = transformValue(name, this.option.getDefaultValue().map(Objects::toString), context, null); if (defaultValue != null) { return defaultValue; @@ -124,13 +124,13 @@ public class PropertyMapper { ConfigValue current = context.proceed(name); if (current != null) { - return transformValue(name, ofNullable(current.getValue()), context).withConfigSourceName(current.getConfigSourceName()); + return transformValue(name, ofNullable(current.getValue()), context, current.getConfigSourceName()); } return current; } - ConfigValue transformedValue = transformValue(name, ofNullable(config.getValue()), context).withConfigSourceName(config.getConfigSourceName()); + ConfigValue transformedValue = transformValue(name, ofNullable(config.getValue()), context, config.getConfigSourceName()); // we always fallback to the current value from the property we are mapping if (transformedValue == null) { @@ -190,14 +190,14 @@ public class PropertyMapper { return mask; } - private ConfigValue transformValue(String name, Optional value, ConfigSourceInterceptorContext context) { + private ConfigValue transformValue(String name, Optional value, ConfigSourceInterceptorContext context, String configSourceName) { if (value == null) { return null; } if (mapper == null || (mapFrom == null && name.equals(getFrom()))) { // no mapper set or requesting a property that does not depend on other property, just return the value from the config source - return ConfigValue.builder().withName(name).withValue(value.orElse(null)).build(); + return ConfigValue.builder().withName(name).withValue(value.orElse(null)).withConfigSourceName(configSourceName).build(); } Optional mappedValue = mapper.apply(value, context); @@ -206,7 +206,8 @@ public class PropertyMapper { return null; } - return ConfigValue.builder().withName(name).withValue(mappedValue.get()).withRawValue(value.orElse(null)).build(); + return ConfigValue.builder().withName(name).withValue(mappedValue.get()).withRawValue(value.orElse(null)) + .withConfigSourceName(configSourceName).build(); } private ConfigValue convertValue(ConfigValue configValue) { diff --git a/quarkus/runtime/src/test/java/org/keycloak/quarkus/runtime/configuration/test/ConfigurationTest.java b/quarkus/runtime/src/test/java/org/keycloak/quarkus/runtime/configuration/test/ConfigurationTest.java index a14077df2a..de45b2c9d3 100644 --- a/quarkus/runtime/src/test/java/org/keycloak/quarkus/runtime/configuration/test/ConfigurationTest.java +++ b/quarkus/runtime/src/test/java/org/keycloak/quarkus/runtime/configuration/test/ConfigurationTest.java @@ -18,7 +18,6 @@ package org.keycloak.quarkus.runtime.configuration.test; import static org.junit.Assert.assertEquals; -import static org.junit.Assert.assertNull; import static org.junit.Assert.assertTrue; import static org.keycloak.quarkus.runtime.Environment.isWindows; import static org.keycloak.quarkus.runtime.configuration.ConfigArgsConfigSource.CLI_ARGS; @@ -293,6 +292,8 @@ public class ConfigurationTest { System.setProperty(CLI_ARGS, "--db=dev-mem" + ARG_SEPARATOR + "--db-username=other"); config = createConfig(); assertEquals("sa", config.getConfigValue("quarkus.datasource.username").getValue()); + // should be untransformed + assertEquals("other", config.getConfigValue("kc.db-username").getValue()); System.setProperty(CLI_ARGS, "--db=postgres" + ARG_SEPARATOR + "--db-username=other"); config = createConfig(); @@ -300,17 +301,20 @@ public class ConfigurationTest { System.setProperty(CLI_ARGS, "--db=postgres"); config = createConfig(); + // username should not be set, either as the quarkus or kc property assertEquals(null, config.getConfigValue("quarkus.datasource.username").getValue()); + assertEquals(null, config.getConfigValue("kc.db-username").getValue()); } @Test public void testDatabaseKindProperties() { - System.setProperty(CLI_ARGS, "--db=postgres" + ARG_SEPARATOR + "--db-url=jdbc:postgresql://localhost/keycloak"); + System.setProperty(CLI_ARGS, "--db=postgres" + ARG_SEPARATOR + "--db-url=jdbc:postgresql://localhost/keycloak" + ARG_SEPARATOR + "--db-username=postgres"); SmallRyeConfig config = createConfig(); assertEquals("org.hibernate.dialect.PostgreSQLDialect", config.getConfigValue("kc.db-dialect").getValue()); assertEquals("jdbc:postgresql://localhost/keycloak", config.getConfigValue("quarkus.datasource.jdbc.url").getValue()); assertEquals("postgresql", config.getConfigValue("quarkus.datasource.db-kind").getValue()); + assertEquals("postgres", config.getConfigValue("quarkus.datasource.username").getValue()); } @Test