dataware/datawareauthic
2
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions

fix: addresses consistency issues with property mapping (#34250)

Browse source 
localized, and standardized, expression expansion
replaced context.proceed for kc and quarkus properties - where possible

closes: #33741

Signed-off-by: Steve Hawkins <shawkins@redhat.com>
This commit is contained in:
Steven Hawkins 2024-11-07 09:03:21 -05:00 • committed by GitHub
parent f7d9ef890f
commit 4e49246286
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
8 changed files with 74 additions and 29 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

4
quarkus/runtime/src/main/java/org/keycloak/quarkus/runtime/configuration/DisabledMappersInterceptor.java
View file

@ -81,6 +81,10 @@ public class DisabledMappersInterceptor implements ConfigSourceInterceptor {
}
public static void runWithDisabled(Runnable execution) {
if (!isEnabled()) {
execution.run();
return;
}
try {
disable();
execution.run();

3
quarkus/runtime/src/main/java/org/keycloak/quarkus/runtime/configuration/PersistedConfigSource.java
View file

@ -160,6 +160,9 @@ public final class PersistedConfigSource extends PropertiesConfigSource {
}
public <T> T runWithDisabled(Supplier<T> execution) {
if (!isEnabled()) {
return execution.get();
}
try {
disable();
return execution.get();

26
quarkus/runtime/src/main/java/org/keycloak/quarkus/runtime/configuration/PropertyMappingInterceptor.java
View file

@ -23,7 +23,6 @@ import io.smallrye.config.ConfigValue;
import io.smallrye.config.Priorities;
import jakarta.annotation.Priority;
import org.apache.commons.collections4.iterators.FilterIterator;
import org.keycloak.common.util.StringPropertyReplacer;
import org.keycloak.quarkus.runtime.Environment;
import org.keycloak.quarkus.runtime.configuration.mappers.PropertyMapper;
import org.keycloak.quarkus.runtime.configuration.mappers.PropertyMappers;
@ -82,29 +81,6 @@ public class PropertyMappingInterceptor implements ConfigSourceInterceptor {
if (Boolean.TRUE.equals(disable.get())) {
return context.proceed(name);
}
ConfigValue value = PropertyMappers.getValue(context, name);
if (value == null || value.getValue() == null) {
return null;
}
if (!value.getValue().contains("${")) {
return value;
}
// Our mappers might have returned a value containing an expression ${...}.
// However, ExpressionConfigSourceInterceptor was already executed before (to expand e.g. env vars in config file).
// Hence, we need to manually resolve these expressions here. Not ideal, but there's no other way (at least I haven't found one).
return value.withValue(
StringPropertyReplacer.replaceProperties(value.getValue(),
property -> {
ConfigValue prop = context.proceed(property);
if (prop == null) {
return null;
}
return prop.getValue();
}));
return PropertyMappers.getValue(context, name);
}
}

8
quarkus/runtime/src/main/java/org/keycloak/quarkus/runtime/configuration/mappers/DatabasePropertyMappers.java
View file

@ -2,13 +2,15 @@ package org.keycloak.quarkus.runtime.configuration.mappers;
import io.quarkus.datasource.common.runtime.DatabaseKind;
import io.smallrye.config.ConfigSourceInterceptorContext;
import io.smallrye.config.ConfigValue;
import org.keycloak.config.DatabaseOptions;
import org.keycloak.config.TransactionOptions;
import org.keycloak.config.database.Database;
import org.keycloak.quarkus.runtime.configuration.Configuration;
import static org.keycloak.quarkus.runtime.configuration.mappers.PropertyMapper.fromOption;
import java.util.Optional;
final class DatabasePropertyMappers {
private DatabasePropertyMappers(){}
@ -84,8 +86,8 @@ final class DatabasePropertyMappers {
}
private static String getXaOrNonXaDriver(String value, ConfigSourceInterceptorContext context) {
ConfigValue xaEnabledConfigValue = context.proceed("kc.transaction-xa-enabled");
boolean isXaEnabled = xaEnabledConfigValue != null && Boolean.parseBoolean(xaEnabledConfigValue.getValue());
Optional<String> xaEnabledConfigValue = Configuration.getOptionalKcValue(TransactionOptions.TRANSACTION_XA_ENABLED);
boolean isXaEnabled = xaEnabledConfigValue.map(Boolean::parseBoolean).orElse(false);
return Database.getDriver(value, isXaEnabled).orElse(null);
}

38
quarkus/runtime/src/main/java/org/keycloak/quarkus/runtime/configuration/mappers/PropertyMapper.java
View file

@ -23,6 +23,7 @@ import static org.keycloak.quarkus.runtime.configuration.Configuration.OPTION_PA
import static org.keycloak.quarkus.runtime.configuration.Configuration.toCliFormat;
import static org.keycloak.quarkus.runtime.configuration.Configuration.toEnvVarFormat;
import java.util.Iterator;
import java.util.List;
import java.util.Objects;
import java.util.Optional;
@ -35,6 +36,8 @@ import java.util.stream.Stream;
import io.smallrye.config.ConfigSourceInterceptorContext;
import io.smallrye.config.ConfigValue;
import io.smallrye.config.ConfigValue.ConfigValueBuilder;
import io.smallrye.config.ExpressionConfigSourceInterceptor;
import io.smallrye.config.Expressions;
import org.keycloak.config.DeprecatedMetadata;
import org.keycloak.config.Option;
@ -247,6 +250,13 @@ public class PropertyMapper<T> {
mapped = true;
}
// defaults and values from transformers may not have been subject to expansion
if ((mapped || configValue.getConfigSourceName() == null) && mappedValue != null && Expressions.isEnabled() && mappedValue.contains("$")) {
mappedValue = new ExpressionConfigSourceInterceptor().getValue(
new ContextWrapper(context, new ConfigValueBuilder().withName(name).withValue(mappedValue).build()),
name).getValue();
}
if (value == null && mappedValue == null) {
return null;
}
@ -267,6 +277,34 @@ public class PropertyMapper<T> {
return configValue.withValue(ofNullable(configValue.getValue()).map(String::trim).orElse(null));
}
private final class ContextWrapper implements ConfigSourceInterceptorContext {
private final ConfigSourceInterceptorContext context;
private final ConfigValue value;
private ContextWrapper(ConfigSourceInterceptorContext context, ConfigValue value) {
this.context = context;
this.value = value;
}
@Override
public ConfigValue restart(String name) {
return context.restart(name);
}
@Override
public ConfigValue proceed(String name) {
if (name.equals(value.getName())) {
return value;
}
return context.proceed(name);
}
@Override
public Iterator<String> iterateNames() {
return context.iterateNames();
}
}
public static class Builder<T> {
private final Option<T> option;

9
quarkus/runtime/src/test/java/org/keycloak/quarkus/runtime/configuration/test/AbstractConfigurationTest.java
View file

@ -27,6 +27,8 @@ import io.smallrye.config.ConfigValue.ConfigValueBuilder;
import org.eclipse.microprofile.config.ConfigProvider;
import org.eclipse.microprofile.config.spi.ConfigProviderResolver;
import org.junit.After;
import org.junit.AfterClass;
import org.junit.BeforeClass;
import org.keycloak.Config;
import org.keycloak.quarkus.runtime.configuration.ConfigArgsConfigSource;
import org.keycloak.quarkus.runtime.configuration.Configuration;
@ -96,6 +98,11 @@ public abstract class AbstractConfigurationTest {
System.clearProperty(key);
}
}
@AfterClass
public static void resetConfigruation() {
ConfigurationTest.createConfig(); // onAfter doesn't actually reset the config
}
@After
public void onAfter() {
@ -124,7 +131,7 @@ public abstract class AbstractConfigurationTest {
return Config.scope(scope);
}
protected SmallRyeConfig createConfig() {
static protected SmallRyeConfig createConfig() {
KeycloakConfigSourceProvider.reload();
// older versions of quarkus implicitly picked up this config, now we
// must set it manually

9
quarkus/runtime/src/test/java/org/keycloak/quarkus/runtime/configuration/test/ConfigurationTest.java
View file

@ -35,6 +35,7 @@ import io.smallrye.config.SmallRyeConfig;
import org.hibernate.dialect.H2Dialect;
import org.hibernate.dialect.PostgreSQLDialect;
import io.smallrye.config.ConfigValue;
import io.smallrye.config.Expressions;
import io.smallrye.config.PropertiesConfigSource;
import io.smallrye.config.SmallRyeConfigBuilder;
import org.h2.Driver;
@ -205,6 +206,14 @@ public class ConfigurationTest extends AbstractConfigurationTest {
assertEquals(MariaDBDialect.class.getName(), config.getConfigValue("kc.db-dialect").getValue());
assertEquals("jdbc:mariadb:aurora://foo/bar?a=1&b=2", config.getConfigValue("quarkus.datasource.jdbc.url").getValue());
}
@Test
public void testExpansionDisabled() {
ConfigArgsConfigSource.setCliArgs("--db=mysql");
SmallRyeConfig config = createConfig();
String value = Expressions.withoutExpansion(() -> config.getConfigValue("quarkus.datasource.jdbc.url").getValue());
assertEquals("jdbc:mysql://${kc.db-url-host:localhost}:${kc.db-url-port:3306}/${kc.db-url-database:keycloak}${kc.db-url-properties:}", value);
}
@Test
public void testDatabaseDefaults() {

6
quarkus/runtime/src/test/java/org/keycloak/quarkus/runtime/configuration/test/IgnoredArtifactsTest.java
View file

@ -18,6 +18,7 @@
package org.keycloak.quarkus.runtime.configuration.test;
import org.hamcrest.CoreMatchers;
import org.junit.BeforeClass;
import org.junit.Test;
import org.keycloak.common.Profile;
import org.keycloak.common.profile.PropertiesProfileConfigResolver;
@ -48,6 +49,11 @@ import static org.keycloak.quarkus.runtime.configuration.MicroProfileConfigProvi
import static org.keycloak.quarkus.runtime.configuration.test.ConfigurationTest.setSystemProperty;
public class IgnoredArtifactsTest {
@BeforeClass
public static void resetConfigruation() {
ConfigurationTest.createConfig(); // make sure we're dealing with a clean config
}
@Test
public void fipsDisabled() {