fix: corrects the db property handling and null mapped values (#25088)

closes #25010

Signed-off-by: Steve Hawkins <shawkins@redhat.com>
This commit is contained in:
Steven Hawkins 2023-11-29 13:54:10 -05:00 committed by GitHub
parent d0b86d2f64
commit 7e0cbcafae
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
3 changed files with 16 additions and 13 deletions

View file

@ -60,13 +60,11 @@ final class DatabasePropertyMappers {
.build(), .build(),
fromOption(DatabaseOptions.DB_USERNAME) fromOption(DatabaseOptions.DB_USERNAME)
.to("quarkus.datasource.username") .to("quarkus.datasource.username")
.mapFrom("db")
.transformer(DatabasePropertyMappers::resolveUsername) .transformer(DatabasePropertyMappers::resolveUsername)
.paramLabel("username") .paramLabel("username")
.build(), .build(),
fromOption(DatabaseOptions.DB_PASSWORD) fromOption(DatabaseOptions.DB_PASSWORD)
.to("quarkus.datasource.password") .to("quarkus.datasource.password")
.mapFrom("db")
.transformer(DatabasePropertyMappers::resolvePassword) .transformer(DatabasePropertyMappers::resolvePassword)
.paramLabel("password") .paramLabel("password")
.isMasked(true) .isMasked(true)
@ -137,7 +135,7 @@ final class DatabasePropertyMappers {
return of("sa"); return of("sa");
} }
return Database.getDatabaseKind(value.get()).isEmpty() ? value : null; return value;
} }
private static Optional<String> resolvePassword(Optional<String> value, ConfigSourceInterceptorContext context) { private static Optional<String> resolvePassword(Optional<String> value, ConfigSourceInterceptorContext context) {
@ -145,7 +143,7 @@ final class DatabasePropertyMappers {
return of("password"); return of("password");
} }
return Database.getDatabaseKind(value.get()).isEmpty() ? value : null; return value;
} }
private static boolean isDevModeDatabase(ConfigSourceInterceptorContext context) { private static boolean isDevModeDatabase(ConfigSourceInterceptorContext context) {

View file

@ -111,10 +111,10 @@ public class PropertyMapper<T> {
} }
} }
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) { if (defaultValue != null) {
return defaultValue; return defaultValue;
@ -124,13 +124,13 @@ public class PropertyMapper<T> {
ConfigValue current = context.proceed(name); ConfigValue current = context.proceed(name);
if (current != null) { if (current != null) {
return transformValue(name, ofNullable(current.getValue()), context).withConfigSourceName(current.getConfigSourceName()); return transformValue(name, ofNullable(current.getValue()), context, current.getConfigSourceName());
} }
return current; 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 // we always fallback to the current value from the property we are mapping
if (transformedValue == null) { if (transformedValue == null) {
@ -190,14 +190,14 @@ public class PropertyMapper<T> {
return mask; return mask;
} }
private ConfigValue transformValue(String name, Optional<String> value, ConfigSourceInterceptorContext context) { private ConfigValue transformValue(String name, Optional<String> value, ConfigSourceInterceptorContext context, String configSourceName) {
if (value == null) { if (value == null) {
return null; return null;
} }
if (mapper == null || (mapFrom == null && name.equals(getFrom()))) { 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 // 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<String> mappedValue = mapper.apply(value, context); Optional<String> mappedValue = mapper.apply(value, context);
@ -206,7 +206,8 @@ public class PropertyMapper<T> {
return null; 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) { private ConfigValue convertValue(ConfigValue configValue) {

View file

@ -18,7 +18,6 @@
package org.keycloak.quarkus.runtime.configuration.test; package org.keycloak.quarkus.runtime.configuration.test;
import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertNull;
import static org.junit.Assert.assertTrue; import static org.junit.Assert.assertTrue;
import static org.keycloak.quarkus.runtime.Environment.isWindows; import static org.keycloak.quarkus.runtime.Environment.isWindows;
import static org.keycloak.quarkus.runtime.configuration.ConfigArgsConfigSource.CLI_ARGS; 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"); System.setProperty(CLI_ARGS, "--db=dev-mem" + ARG_SEPARATOR + "--db-username=other");
config = createConfig(); config = createConfig();
assertEquals("sa", config.getConfigValue("quarkus.datasource.username").getValue()); 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"); System.setProperty(CLI_ARGS, "--db=postgres" + ARG_SEPARATOR + "--db-username=other");
config = createConfig(); config = createConfig();
@ -300,17 +301,20 @@ public class ConfigurationTest {
System.setProperty(CLI_ARGS, "--db=postgres"); System.setProperty(CLI_ARGS, "--db=postgres");
config = createConfig(); 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("quarkus.datasource.username").getValue());
assertEquals(null, config.getConfigValue("kc.db-username").getValue());
} }
@Test @Test
public void testDatabaseKindProperties() { 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(); SmallRyeConfig config = createConfig();
assertEquals("org.hibernate.dialect.PostgreSQLDialect", assertEquals("org.hibernate.dialect.PostgreSQLDialect",
config.getConfigValue("kc.db-dialect").getValue()); config.getConfigValue("kc.db-dialect").getValue());
assertEquals("jdbc:postgresql://localhost/keycloak", config.getConfigValue("quarkus.datasource.jdbc.url").getValue()); assertEquals("jdbc:postgresql://localhost/keycloak", config.getConfigValue("quarkus.datasource.jdbc.url").getValue());
assertEquals("postgresql", config.getConfigValue("quarkus.datasource.db-kind").getValue()); assertEquals("postgresql", config.getConfigValue("quarkus.datasource.db-kind").getValue());
assertEquals("postgres", config.getConfigValue("quarkus.datasource.username").getValue());
} }
@Test @Test