Skip to content

Commit

Permalink
Fix duplicate options in show-config
Browse files Browse the repository at this point in the history
Closes keycloak#32182

Signed-off-by: Václav Muzikář <[email protected]>
  • Loading branch information
vmuzikar committed Aug 16, 2024
1 parent cb418b0 commit 3965e23
Show file tree
Hide file tree
Showing 5 changed files with 79 additions and 85 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -18,34 +18,35 @@
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.HashMap;
import java.util.Map;

import io.smallrye.config.PropertiesConfigSource;
import org.keycloak.quarkus.runtime.configuration.mappers.PropertyMapper;
import org.keycloak.quarkus.runtime.configuration.mappers.PropertyMappers;

import io.smallrye.config.EnvConfigSource;

public class KcEnvConfigSource extends EnvConfigSource {
// 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 KcEnvConfigSource() {
super(buildProperties(), 500);
super(buildProperties(), NAME, 500);
}

private static Map<String, String> buildProperties() {
Map<String, String> properties = new HashMap<>();
String kcPrefix = replaceNonAlphanumericByUnderscores(MicroProfileConfigProvider.NS_KEYCLOAK_PREFIX.toUpperCase());
String kcPrefix = replaceNonAlphanumericByUnderscores(NS_KEYCLOAK_PREFIX.toUpperCase());

for (Map.Entry<String, String> entry : System.getenv().entrySet()) {
String key = entry.getKey();
String value = entry.getValue();

if (key.startsWith(kcPrefix)) {
properties.put(key, value);

PropertyMapper<?> mapper = PropertyMappers.getMapper(key);

if (mapper != null) {
Expand All @@ -57,13 +58,19 @@ private static Map<String, String> buildProperties() {

properties.put(mapper.getFrom(), value);
}
else {
// most probably an SPI but could be also something else
String transformedKey = NS_KEYCLOAK_PREFIX + key.substring(kcPrefix.length()).toLowerCase().replace("_", "-");
properties.put(transformedKey, value);
}
}
}

return properties;
}

@Override
// a workaround for https://github.com/smallrye/smallrye-config/issues/1207
public String getName() {
return NAME;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -183,7 +183,7 @@ public void testPropertyNamesFromConfig() {
System.getProperties().remove("kc.spi-client-registration-openid-connect-static-jwk-url");
putEnvVar("KC_SPI_CLIENT_REGISTRATION_OPENID_CONNECT_STATIC_JWK_URL", "http://c.jwk.url/from-env");
config = initConfig("client-registration", "openid-connect");
assertEquals(1, config.getPropertyNames().size());
assertEquals(2, config.getPropertyNames().size()); // transformed name is coming from KcEnvVarConfigSource, raw env var name is coming from EnvVarConfigSource
assertEquals("http://c.jwk.url/from-env", config.get("static-jwk-url"));
}

Expand Down

This file was deleted.

Original file line number Diff line number Diff line change
@@ -1,10 +1,23 @@
package org.keycloak.it.cli.dist;

import io.quarkus.test.junit.main.Launch;
import io.quarkus.test.junit.main.LaunchResult;
import org.junit.jupiter.api.Assertions;
import org.junit.jupiter.api.Test;
import org.keycloak.it.junit5.extension.CLIResult;
import org.keycloak.it.junit5.extension.DistributionTest;
import org.keycloak.it.junit5.extension.RawDistOnly;
import org.keycloak.it.junit5.extension.WithEnvVars;
import org.keycloak.it.utils.KeycloakDistribution;
import org.keycloak.quarkus.runtime.cli.command.ShowConfig;
import org.keycloak.quarkus.runtime.configuration.mappers.PropertyMappers;

import java.nio.file.Paths;

import static org.hamcrest.MatcherAssert.assertThat;
import static org.hamcrest.Matchers.containsString;
import static org.hamcrest.Matchers.not;
import static org.keycloak.quarkus.runtime.cli.command.Main.CONFIG_FILE_LONG_NAME;

@DistributionTest
public class ShowConfigCommandDistTest {
Expand All @@ -28,4 +41,54 @@ void testShowConfigPicksUpRightConfigDependingOnCurrentMode(KeycloakDistribution
resetResult.assertMessage("Current Mode: production");
resetResult.assertMessage("kc.db = dev-file");
}

@Test
@Launch({ ShowConfig.NAME })
void testShowConfigCommandShowsRuntimeConfig(LaunchResult result) {
Assertions.assertTrue(result.getOutput()
.contains("Current Configuration"));
}

@Test
@Launch({ ShowConfig.NAME, "all" })
void testShowConfigCommandWithAllShowsAllProfiles(LaunchResult result) {
Assertions.assertTrue(result.getOutput()
.contains("Current Configuration"));
Assertions.assertTrue(result.getOutput()
.contains("Quarkus Configuration"));
}

@Test
@RawDistOnly(reason = "Containers are immutable")
void testShowConfigCommandHidesCredentialsInProfiles(KeycloakDistribution distribution) {
CLIResult result = distribution.run(String.format("%s=%s", CONFIG_FILE_LONG_NAME, Paths.get("src/test/resources/ShowConfigCommandTest/keycloak.conf").toAbsolutePath().normalize()), ShowConfig.NAME, "all");
String output = result.getOutput();
Assertions.assertFalse(output.contains("testpw1"));
Assertions.assertFalse(output.contains("testpw2"));
Assertions.assertFalse(output.contains("testpw3"));
Assertions.assertTrue(output.contains("kc.db-password = " + PropertyMappers.VALUE_MASK));
}

@Test
@RawDistOnly(reason = "Containers are immutable")
void testSmallRyeKeyStoreConfigSource(KeycloakDistribution distribution) {
// keystore is shared with QuarkusPropertiesDistTest#testSmallRyeKeyStoreConfigSource
CLIResult result = distribution.run(String.format("%s=%s", CONFIG_FILE_LONG_NAME, Paths.get("src/test/resources/ShowConfigCommandTest/keycloak-keystore.conf").toAbsolutePath().normalize()), ShowConfig.NAME, "all");
String output = result.getOutput();
assertThat(output, containsString("kc.config-keystore-password = " + PropertyMappers.VALUE_MASK));
assertThat(output, containsString("kc.log-level = " + PropertyMappers.VALUE_MASK));

assertThat(output, not(containsString("secret")));
assertThat(output, not(containsString("debug")));
}

@Test
@Launch({ ShowConfig.NAME })
@WithEnvVars({"KC_DB_PASSWORD", "secret-pass"})
void testNoDuplicitEnvVarEntries(LaunchResult result) {
String output = result.getOutput();
assertThat(output, containsString("kc.db-password = " + PropertyMappers.VALUE_MASK));
assertThat(output, not(containsString("kc.db.password")));
assertThat(output, not(containsString("secret-pass")));
}
}
Original file line number Diff line number Diff line change
@@ -1,2 +1,2 @@
config-keystore=src/test/resources/keystore
config-keystore=../../../../src/test/resources/keystore
config-keystore-password=secret

0 comments on commit 3965e23

Please sign in to comment.