fix: adding a mapping directly from an env property to a wildcard (#39602)

* fix: adding a map directly from an env property to a wildcard

closes: #38259

Signed-off-by: Steve Hawkins <shawkins@redhat.com>

* switching unit test logic to not directly manipulate env vars

Signed-off-by: Steve Hawkins <shawkins@redhat.com>

* Apply suggestions from code review

Co-authored-by: Martin Bartoš <mabartos@redhat.com>
Signed-off-by: Steven Hawkins <shawkins@redhat.com>

* modifications based upon review feedback

Signed-off-by: Steve Hawkins <shawkins@redhat.com>

---------

Signed-off-by: Steve Hawkins <shawkins@redhat.com>
Signed-off-by: Steven Hawkins <shawkins@redhat.com>
Co-authored-by: Martin Bartoš <mabartos@redhat.com>
This commit is contained in:
Steven Hawkins 2025-06-13 04:26:18 -04:00 committed by GitHub
parent d86dad4ea9
commit 0e28bd3981
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
17 changed files with 118 additions and 82 deletions

View File

@ -85,7 +85,7 @@ db-url-host=mykeycloakdb
Alternatively, see <@links.server id="all-config"/> for all server options.
=== Formats for environment variables
=== Format for referencing environment variables
You can use placeholders to resolve an environment specific value from environment variables inside the `keycloak.conf` file by using the `${r"++${ENV_VAR}++"}` syntax:
[source]
@ -165,6 +165,18 @@ When using PowerShell and your values contain special characters like commas, us
PowerShell handles quotes differently. It interprets the quoted string before passing it to the `kc.bat` script, removing the outer quote characters. Therefore, an extra layer of quotes is needed to preserve the value structure. Otherwise, the comma would be interpreted as a delimiter. In Windows CMD, you can use double quotes directly.
=== 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.
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`.
You have a couple of options in this case:
- create an entry in your `keycloak.conf` file that references an environment variable of your choosing. e.g. `kc.log-level-package.class_name=${r"${CLASS_NAME_LEVEL}"}`. See more on referencing environment variables in <<Format for referencing environment variables>>.
- or in situations where modifying the `keycloak.conf` may not be easy, you can use a pair of environment variables `KC_UNIQUEIFIER=value` and `KCKEY_UNIQUEIFIER=key`, e.g. `KC_MYKEY=debug` and `KCKEY_MYKEY=log-level-package.class_name`, or `KC_LOG_LEVEL_PACKAGE_CLASS_NAME=debug` and `KCKEY_LOG_LEVEL_PACKAGE_CLASS_NAME=log-level-package.class_name`
== Starting {project_name}
You can start {project_name} in `development mode` or `production mode`. Each mode offers different defaults for the intended environment.

View File

@ -492,6 +492,7 @@ public class KeycloakDeploymentDependentResource extends CRUDKubernetesDependent
private List<EnvVar> getDefaultAndAdditionalEnvVars(Keycloak keycloakCR) {
// default config values
List<ValueOrSecret> serverConfigsList = new ArrayList<>(Constants.DEFAULT_DIST_CONFIG_LIST);
Set<String> defaultKeys = serverConfigsList.stream().map(ValueOrSecret::getName).collect(Collectors.toSet());
// merge with the CR; the values in CR take precedence
if (keycloakCR.getSpec().getAdditionalOptions() != null) {
@ -502,7 +503,7 @@ public class KeycloakDeploymentDependentResource extends CRUDKubernetesDependent
// set env vars
List<EnvVar> envVars = serverConfigsList.stream()
.map(v -> {
.flatMap(v -> {
var envBuilder = new EnvVarBuilder().withName(getKeycloakOptionEnvVarName(v.getName()));
var secret = v.getSecret();
if (secret != null) {
@ -511,7 +512,14 @@ public class KeycloakDeploymentDependentResource extends CRUDKubernetesDependent
} else {
envBuilder.withValue(v.getValue());
}
return envBuilder.build();
EnvVar mainVar = envBuilder.build();
if (!defaultKeys.contains(v.getName())) {
EnvVar keyVar = new EnvVarBuilder()
.withName("KCKEY_" + mainVar.getName().substring(KeycloakDistConfigurator.KC_PREFIX.length()))
.withValue(v.getName()).build();
return Stream.of(mainVar, keyVar);
}
return Stream.of(mainVar);
})
.collect(Collectors.toCollection(ArrayList::new));

View File

@ -57,6 +57,7 @@ import static io.smallrye.config.common.utils.StringUtil.replaceNonAlphanumericB
@ApplicationScoped
public class KeycloakDistConfigurator {
public static final String KC_PREFIX = "KC_";
/**
* Specify first-class citizens fields which should not be added as general server configuration property
*/
@ -204,7 +205,7 @@ public class KeycloakDistConfigurator {
public static String getKeycloakOptionEnvVarName(String kcConfigName) {
// TODO make this use impl from Quarkus dist (Configuration.toEnvVarFormat)
return "KC_" + replaceNonAlphanumericByUnderscores(kcConfigName).toUpperCase();
return KC_PREFIX + replaceNonAlphanumericByUnderscores(kcConfigName).toUpperCase();
}
private <T> OptionMapper<T> optionMapper(Function<Keycloak, T> optionSpec) {
@ -262,7 +263,9 @@ public class KeycloakDistConfigurator {
protected <R extends Collection<?>> OptionMapper<T> mapOptionFromCollection(String optionName, Function<T, R> optionValueSupplier) {
return mapOption(optionName, s -> {
var value = optionValueSupplier.apply(s);
if (value == null) return null;
if (value == null) {
return null;
}
return value.stream().filter(Objects::nonNull).map(String::valueOf).collect(Collectors.joining(","));
});
}

View File

@ -514,6 +514,22 @@ public class PodTemplateTest {
&& envVar.getValue().equals("/something"));
}
@Test
public void testAdditionalOptionEnvKey() {
// Arrange
PodTemplateSpec additionalPodTemplate = null;
// Act
var podTemplate = getDeployment(additionalPodTemplate, null,
s -> s.addToAdditionalOptions(new ValueOrSecret("log-level-org.package.some_class", "debug")))
.getSpec().getTemplate();
// Assert
assertThat(podTemplate.getSpec().getContainers().get(0).getEnv().stream())
.anyMatch(envVar -> envVar.getName().equals("KCKEY_LOG_LEVEL_ORG_PACKAGE_SOME_CLASS")
&& envVar.getValue().equals("log-level-org.package.some_class"));
}
@Test
public void testImageForceOptimized() {
// Arrange

View File

@ -20,7 +20,6 @@ package org.keycloak.quarkus.runtime.configuration;
import static org.keycloak.quarkus.runtime.cli.Picocli.ARG_PREFIX;
import java.util.Map;
import java.util.Objects;
import java.util.Optional;
import java.util.Properties;
@ -29,7 +28,6 @@ import io.smallrye.config.ConfigValue;
import io.smallrye.config.SmallRyeConfig;
import org.keycloak.config.Option;
import org.keycloak.quarkus.runtime.cli.Picocli;
import org.keycloak.utils.StringUtil;
import static org.keycloak.quarkus.runtime.configuration.MicroProfileConfigProvider.NS_KEYCLOAK_PREFIX;

View File

@ -20,49 +20,81 @@ package org.keycloak.quarkus.runtime.configuration;
import static io.smallrye.config.common.utils.StringUtil.replaceNonAlphanumericByUnderscores;
import static org.keycloak.quarkus.runtime.configuration.MicroProfileConfigProvider.NS_KEYCLOAK_PREFIX;
import java.util.Collection;
import java.util.HashMap;
import java.util.List;
import java.util.Map;
import io.smallrye.config.PropertiesConfigSource;
import org.eclipse.microprofile.config.spi.ConfigSource;
import org.keycloak.quarkus.runtime.configuration.mappers.PropertyMapper;
import org.keycloak.quarkus.runtime.configuration.mappers.PropertyMappers;
import org.keycloak.quarkus.runtime.configuration.mappers.WildcardPropertyMapper;
import io.smallrye.config.EnvConfigSource;
import io.smallrye.config.PropertiesConfigSource;
// Not extending EnvConfigSource as it's too smart for our own good. It does unnecessary mapping of provided keys
// leading to e.g. duplicate entries (like kc.db-password and kc.db.password), or incorrectly handling getters due to
// how equals() is implemented. We don't need that here as we do our own mapping.
public class KcEnvConfigSource extends PropertiesConfigSource {
public static final String NAME = "KcEnvVarConfigSource";
public static final String KCKEY_PREFIX = "KCKEY_";
public KcEnvConfigSource() {
super(buildProperties(), NAME, 500);
static final Map<String, String> ENV_OVERRIDE = new HashMap<String, String>();
public KcEnvConfigSource(Map<String, String> env) {
super(buildProperties(env), NAME, 500);
}
private static Map<String, String> buildProperties() {
private static Map<String, String> buildProperties(Map<String, String> env) {
Map<String, String> properties = new HashMap<>();
String kcPrefix = replaceNonAlphanumericByUnderscores(NS_KEYCLOAK_PREFIX.toUpperCase());
for (Map.Entry<String, String> entry : System.getenv().entrySet()) {
for (Map.Entry<String, String> entry : env.entrySet()) {
String key = entry.getKey();
String value = entry.getValue();
String transformedKey = null;
if (key.startsWith(kcPrefix)) {
String transformedKey = NS_KEYCLOAK_PREFIX + key.substring(kcPrefix.length()).toLowerCase().replace("_", "-");
if (!key.startsWith(kcPrefix)) {
continue;
}
String baseKey = key.substring(kcPrefix.length());
String actualKey = env.get(KCKEY_PREFIX + baseKey);
if (actualKey != null) {
// use the explicit mapping
transformedKey = NS_KEYCLOAK_PREFIX + actualKey;
} else {
// determine the mapping by convention / wildcard handling
transformedKey = NS_KEYCLOAK_PREFIX + baseKey.toLowerCase().replace("_", "-");
PropertyMapper<?> mapper = PropertyMappers.getMapper(transformedKey);
if (mapper != null && mapper.hasWildcard()) {
// special case - wildcards don't follow the default conversion rule
transformedKey = ((WildcardPropertyMapper<?>) mapper).getKcKeyForEnvKey(key, transformedKey)
WildcardPropertyMapper<?> wildcardPropertyMapper = (WildcardPropertyMapper<?>) mapper;
transformedKey = wildcardPropertyMapper.getKcKeyForEnvKey(key, transformedKey)
.orElseThrow();
}
properties.put(transformedKey, value);
}
properties.put(transformedKey, value);
}
return properties;
}
public static Collection<ConfigSource> getConfigSources() {
Map<String, String> env = System.getenv();
if (ENV_OVERRIDE.isEmpty()) {
return List.of(new KcEnvConfigSource(env));
}
env = new HashMap<String, String>(env);
env.putAll(ENV_OVERRIDE);
return List.of(new KcEnvConfigSource(env), new EnvConfigSource(ENV_OVERRIDE, EnvConfigSource.ORDINAL + 1));
}
}

View File

@ -49,7 +49,7 @@ public class KeycloakConfigSourceProvider implements ConfigSourceProvider, Confi
addConfigSources("CLI", List.of(new ConfigArgsConfigSource()));
addConfigSources("ENV", List.of(new KcEnvConfigSource()));
addConfigSources("ENV", KcEnvConfigSource.getConfigSources());
addConfigSources("quarkus.properties", new QuarkusPropertiesConfigSource().getConfigSources(Thread.currentThread().getContextClassLoader()));

View File

@ -66,7 +66,7 @@ public class WildcardPropertyMapper<T> extends PropertyMapper<T> {
return toPrefix + wildcardKey + toSuffix;
}
String getFrom(String wildcardKey) {
public String getFrom(String wildcardKey) {
return fromPrefix + wildcardKey;
}

View File

@ -43,8 +43,8 @@ import org.junit.Test;
import org.keycloak.config.LoggingOptions;
import org.keycloak.quarkus.runtime.Environment;
import org.keycloak.quarkus.runtime.KeycloakMain;
import org.keycloak.quarkus.runtime.configuration.AbstractConfigurationTest;
import org.keycloak.quarkus.runtime.configuration.ConfigArgsConfigSource;
import org.keycloak.quarkus.runtime.configuration.test.AbstractConfigurationTest;
import io.smallrye.config.SmallRyeConfig;
import picocli.CommandLine;

View File

@ -15,16 +15,16 @@
* limitations under the License.
*/
package org.keycloak.quarkus.runtime.configuration.test;
package org.keycloak.quarkus.runtime.configuration;
import static org.hamcrest.CoreMatchers.is;
import static org.hamcrest.CoreMatchers.notNullValue;
import static org.hamcrest.CoreMatchers.nullValue;
import static org.hamcrest.MatcherAssert.assertThat;
import static org.junit.Assert.assertNotNull;
import static org.junit.Assert.assertTrue;
import java.lang.reflect.Field;
import java.nio.file.Paths;
import java.util.HashMap;
import java.util.Map;
import java.util.Properties;
import java.util.function.Function;
@ -36,11 +36,6 @@ import org.keycloak.Config;
import org.keycloak.common.Profile;
import org.keycloak.quarkus.runtime.Environment;
import org.keycloak.quarkus.runtime.cli.ExecutionExceptionHandler;
import org.keycloak.quarkus.runtime.configuration.ConfigArgsConfigSource;
import org.keycloak.quarkus.runtime.configuration.Configuration;
import org.keycloak.quarkus.runtime.configuration.KeycloakConfigSourceProvider;
import org.keycloak.quarkus.runtime.configuration.MicroProfileConfigProvider;
import org.keycloak.quarkus.runtime.configuration.PersistedConfigSource;
import org.keycloak.quarkus.runtime.configuration.mappers.PropertyMappers;
import io.smallrye.config.ConfigValue;
@ -50,27 +45,11 @@ import io.smallrye.config.SmallRyeConfig;
public abstract class AbstractConfigurationTest {
private static final Properties SYSTEM_PROPERTIES = (Properties) System.getProperties().clone();
private static final Map<String, String> ENVIRONMENT_VARIABLES = new HashMap<>(System.getenv());
@SuppressWarnings("unchecked")
public static void putEnvVar(String name, String value) {
Map<String, String> env = System.getenv();
Field field = null;
try {
field = env.getClass().getDeclaredField("m");
field.setAccessible(true);
if (value == null) {
((Map<String, String>) field.get(env)).remove(name);
} else {
((Map<String, String>) field.get(env)).put(name, value);
}
} catch (Exception cause) {
throw new RuntimeException("Failed to update environment variables", cause);
} finally {
if (field != null) {
field.setAccessible(false);
}
}
assertNotNull(name);
assertNotNull(value);
KcEnvConfigSource.ENV_OVERRIDE.put(name, value);
}
public static void putEnvVars(Map<String, String> map) {
@ -79,19 +58,7 @@ public abstract class AbstractConfigurationTest {
@SuppressWarnings("unchecked")
public static void removeEnvVar(String name) {
Map<String, String> env = System.getenv();
Field field = null;
try {
field = env.getClass().getDeclaredField("m");
field.setAccessible(true);
((Map<String, String>) field.get(env)).remove(name);
} catch (Exception cause) {
throw new RuntimeException("Failed to update environment variables", cause);
} finally {
if (field != null) {
field.setAccessible(false);
}
}
assertTrue(KcEnvConfigSource.ENV_OVERRIDE.remove(name) != null);
}
public static void setSystemProperty(String key, String value, Runnable runnable) {
@ -109,16 +76,7 @@ public abstract class AbstractConfigurationTest {
System.setProperties((Properties) SYSTEM_PROPERTIES.clone());
Environment.setHomeDir(Paths.get("src/test/resources/"));
for (String name : new HashMap<>(System.getenv()).keySet()) {
if (!ENVIRONMENT_VARIABLES.containsKey(name)) {
removeEnvVar(name);
}
}
ENVIRONMENT_VARIABLES.forEach((key, value) -> {
if (!System.getenv(key).equals(value)) {
putEnvVar(key, value);
}
});
KcEnvConfigSource.ENV_OVERRIDE.clear();
PropertyMappers.reset();
ConfigArgsConfigSource.setCliArgs();
@ -172,7 +130,7 @@ public abstract class AbstractConfigurationTest {
protected void assertExternalConfig(Map<String, String> expectedValues) {
expectedValues.forEach(this::assertExternalConfig);
}
protected void assertConfigNull(String key, boolean isExternal) {
Function<String, ConfigValue> getConfig = isExternal ? Configuration::getConfigValue : Configuration::getKcConfigValue;
assertThat(String.format("We expect that the value is null for key '%s'", key), getConfig.apply(key).getValue(), nullValue());

View File

@ -15,7 +15,7 @@
* limitations under the License.
*/
package org.keycloak.quarkus.runtime.configuration.test;
package org.keycloak.quarkus.runtime.configuration;
import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertNotNull;
@ -159,7 +159,7 @@ public class ConfigurationTest extends AbstractConfigurationTest {
assertEquals("false", config.getConfigValue("MY_EXPRESSION").getValue());
// without the env variable set, the expression should use the missing env variable
putEnvVar("KC_HOSTNAME_STRICT", null);
removeEnvVar("KC_HOSTNAME_STRICT");
ConfigArgsConfigSource.setCliArgs("");
config = createConfig();
// check that we get the mapped default value
@ -590,5 +590,5 @@ public class ConfigurationTest extends AbstractConfigurationTest {
private static Config.Scope cacheEmbeddedConfiguration() {
return initConfig(CacheEmbeddedConfigProviderSpi.SPI_NAME, DefaultCacheEmbeddedConfigProviderFactory.PROVIDER_ID);
}
}
}

View File

@ -1,4 +1,4 @@
package org.keycloak.quarkus.runtime.configuration.test;
package org.keycloak.quarkus.runtime.configuration;
import io.smallrye.config.Expressions;
import io.smallrye.config.SmallRyeConfig;

View File

@ -15,7 +15,7 @@
* limitations under the License.
*/
package org.keycloak.quarkus.runtime.configuration.test;
package org.keycloak.quarkus.runtime.configuration;
import org.junit.Test;
import org.keycloak.common.Profile.Feature;

View File

@ -15,7 +15,7 @@
* limitations under the License.
*/
package org.keycloak.quarkus.runtime.configuration.test;
package org.keycloak.quarkus.runtime.configuration;
import org.junit.Test;
import org.keycloak.common.Profile;

View File

@ -15,7 +15,7 @@
* limitations under the License.
*/
package org.keycloak.quarkus.runtime.configuration.test;
package org.keycloak.quarkus.runtime.configuration;
import static org.hamcrest.MatcherAssert.assertThat;
import static org.junit.Assert.assertEquals;
@ -31,7 +31,6 @@ import java.util.stream.Collectors;
import java.util.stream.StreamSupport;
import org.hamcrest.CoreMatchers;
import org.jboss.logmanager.handlers.AsyncHandler;
import org.junit.Test;
import org.keycloak.config.LoggingOptions;
import org.keycloak.quarkus.runtime.Environment;
@ -307,6 +306,16 @@ public class LoggingConfigurationTest extends AbstractConfigurationTest {
assertTrue(keys.contains("quarkus.log.category.\"reproducer.not_ok\".level"));
}
@Test
public void testLogLevelWithUnderscoreEnv() {
putEnvVar("KC_1", "debug");
putEnvVar("KCKEY_1", "log-level-reproducer.not_ok");
SmallRyeConfig config = createConfig();
assertEquals("DEBUG", config.getConfigValue("quarkus.log.category.\"reproducer.not_ok\".level").getValue());
Set<String> keys = StreamSupport.stream(config.getPropertyNames().spliterator(), false).collect(Collectors.toSet());
assertTrue(keys.contains("quarkus.log.category.\"reproducer.not_ok\".level"));
}
@Test(expected = PropertyException.class)
public void testInvalidLogLevel() {
ConfigArgsConfigSource.setCliArgs("--log-level=reproducer.not^ok:debug");

View File

@ -14,7 +14,7 @@
* See the License for the specific language governing permissions and
* limitations under the License.
*/
package org.keycloak.quarkus.runtime.configuration.test;
package org.keycloak.quarkus.runtime.configuration;
import org.junit.Test;
import org.keycloak.quarkus.runtime.configuration.mappers.ManagementPropertyMappers;

View File

@ -15,7 +15,7 @@
* limitations under the License.
*/
package org.keycloak.quarkus.runtime.configuration.test;
package org.keycloak.quarkus.runtime.configuration;
import io.quarkus.opentelemetry.runtime.config.build.SamplerType;
import org.junit.Test;