Skip to content

Commit 799201f

Browse files
vmuzikarahus1
vmuzikar
authored and
ahus1
committed
Fix duplicate options in show-config
Closes #32182 Signed-off-by: Václav Muzikář <[email protected]>
1 parent 74fec50 commit 799201f

File tree

5 files changed

+79
-85
lines changed

5 files changed

+79
-85
lines changed

‎quarkus/runtime/src/main/java/org/keycloak/quarkus/runtime/configuration/KcEnvConfigSource.java‎

Lines changed: 14 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -18,34 +18,35 @@
1818
package org.keycloak.quarkus.runtime.configuration;
1919

2020
import static io.smallrye.config.common.utils.StringUtil.replaceNonAlphanumericByUnderscores;
21+
import static org.keycloak.quarkus.runtime.configuration.MicroProfileConfigProvider.NS_KEYCLOAK_PREFIX;
2122

2223
import java.util.HashMap;
2324
import java.util.Map;
2425

26+
import io.smallrye.config.PropertiesConfigSource;
2527
import org.keycloak.quarkus.runtime.configuration.mappers.PropertyMapper;
2628
import org.keycloak.quarkus.runtime.configuration.mappers.PropertyMappers;
2729

28-
import io.smallrye.config.EnvConfigSource;
29-
30-
public class KcEnvConfigSource extends EnvConfigSource {
30+
// Not extending EnvConfigSource as it's too smart for our own good. It does unnecessary mapping of provided keys
31+
// leading to e.g. duplicate entries (like kc.db-password and kc.db.password), or incorrectly handling getters due to
32+
// how equals() is implemented. We don't need that here as we do our own mapping.
33+
public class KcEnvConfigSource extends PropertiesConfigSource {
3134

3235
public static final String NAME = "KcEnvVarConfigSource";
3336

3437
public KcEnvConfigSource() {
35-
super(buildProperties(), 500);
38+
super(buildProperties(), NAME, 500);
3639
}
3740

3841
private static Map<String, String> buildProperties() {
3942
Map<String, String> properties = new HashMap<>();
40-
String kcPrefix = replaceNonAlphanumericByUnderscores(MicroProfileConfigProvider.NS_KEYCLOAK_PREFIX.toUpperCase());
43+
String kcPrefix = replaceNonAlphanumericByUnderscores(NS_KEYCLOAK_PREFIX.toUpperCase());
4144

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

4649
if (key.startsWith(kcPrefix)) {
47-
properties.put(key, value);
48-
4950
PropertyMapper<?> mapper = PropertyMappers.getMapper(key);
5051

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

5859
properties.put(mapper.getFrom(), value);
5960
}
61+
else {
62+
// most probably an SPI but could be also something else
63+
String transformedKey = NS_KEYCLOAK_PREFIX + key.substring(kcPrefix.length()).toLowerCase().replace("_", "-");
64+
properties.put(transformedKey, value);
65+
}
6066
}
6167
}
6268

6369
return properties;
6470
}
6571

6672
@Override
73+
// a workaround for https://github.com/smallrye/smallrye-config/issues/1207
6774
public String getName() {
6875
return NAME;
6976
}

‎quarkus/runtime/src/test/java/org/keycloak/quarkus/runtime/configuration/test/ConfigurationTest.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -183,7 +183,7 @@ public void testPropertyNamesFromConfig() {
183183
System.getProperties().remove("kc.spi-client-registration-openid-connect-static-jwk-url");
184184
putEnvVar("KC_SPI_CLIENT_REGISTRATION_OPENID_CONNECT_STATIC_JWK_URL", "http://c.jwk.url/from-env");
185185
config = initConfig("client-registration", "openid-connect");
186-
assertEquals(1, config.getPropertyNames().size());
186+
assertEquals(2, config.getPropertyNames().size()); // transformed name is coming from KcEnvVarConfigSource, raw env var name is coming from EnvVarConfigSource
187187
assertEquals("http://c.jwk.url/from-env", config.get("static-jwk-url"));
188188
}
189189

‎quarkus/tests/integration/src/test/java/org/keycloak/it/cli/ShowConfigCommandTest.java‎

Lines changed: 0 additions & 76 deletions
This file was deleted.

‎quarkus/tests/integration/src/test/java/org/keycloak/it/cli/dist/ShowConfigCommandDistTest.java‎

Lines changed: 63 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,23 @@
11
package org.keycloak.it.cli.dist;
22

3+
import io.quarkus.test.junit.main.Launch;
4+
import io.quarkus.test.junit.main.LaunchResult;
5+
import org.junit.jupiter.api.Assertions;
36
import org.junit.jupiter.api.Test;
47
import org.keycloak.it.junit5.extension.CLIResult;
58
import org.keycloak.it.junit5.extension.DistributionTest;
69
import org.keycloak.it.junit5.extension.RawDistOnly;
10+
import org.keycloak.it.junit5.extension.WithEnvVars;
711
import org.keycloak.it.utils.KeycloakDistribution;
12+
import org.keycloak.quarkus.runtime.cli.command.ShowConfig;
13+
import org.keycloak.quarkus.runtime.configuration.mappers.PropertyMappers;
14+
15+
import java.nio.file.Paths;
16+
17+
import static org.hamcrest.MatcherAssert.assertThat;
18+
import static org.hamcrest.Matchers.containsString;
19+
import static org.hamcrest.Matchers.not;
20+
import static org.keycloak.quarkus.runtime.cli.command.Main.CONFIG_FILE_LONG_NAME;
821

922
@DistributionTest
1023
public class ShowConfigCommandDistTest {
@@ -28,4 +41,54 @@ void testShowConfigPicksUpRightConfigDependingOnCurrentMode(KeycloakDistribution
2841
resetResult.assertMessage("Current Mode: production");
2942
resetResult.assertMessage("kc.db = dev-file");
3043
}
44+
45+
@Test
46+
@Launch({ ShowConfig.NAME })
47+
void testShowConfigCommandShowsRuntimeConfig(LaunchResult result) {
48+
Assertions.assertTrue(result.getOutput()
49+
.contains("Current Configuration"));
50+
}
51+
52+
@Test
53+
@Launch({ ShowConfig.NAME, "all" })
54+
void testShowConfigCommandWithAllShowsAllProfiles(LaunchResult result) {
55+
Assertions.assertTrue(result.getOutput()
56+
.contains("Current Configuration"));
57+
Assertions.assertTrue(result.getOutput()
58+
.contains("Quarkus Configuration"));
59+
}
60+
61+
@Test
62+
@RawDistOnly(reason = "Containers are immutable")
63+
void testShowConfigCommandHidesCredentialsInProfiles(KeycloakDistribution distribution) {
64+
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");
65+
String output = result.getOutput();
66+
Assertions.assertFalse(output.contains("testpw1"));
67+
Assertions.assertFalse(output.contains("testpw2"));
68+
Assertions.assertFalse(output.contains("testpw3"));
69+
Assertions.assertTrue(output.contains("kc.db-password = " + PropertyMappers.VALUE_MASK));
70+
}
71+
72+
@Test
73+
@RawDistOnly(reason = "Containers are immutable")
74+
void testSmallRyeKeyStoreConfigSource(KeycloakDistribution distribution) {
75+
// keystore is shared with QuarkusPropertiesDistTest#testSmallRyeKeyStoreConfigSource
76+
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");
77+
String output = result.getOutput();
78+
assertThat(output, containsString("kc.config-keystore-password = " + PropertyMappers.VALUE_MASK));
79+
assertThat(output, containsString("kc.log-level = " + PropertyMappers.VALUE_MASK));
80+
81+
assertThat(output, not(containsString("secret")));
82+
assertThat(output, not(containsString("debug")));
83+
}
84+
85+
@Test
86+
@Launch({ ShowConfig.NAME })
87+
@WithEnvVars({"KC_DB_PASSWORD", "secret-pass"})
88+
void testNoDuplicitEnvVarEntries(LaunchResult result) {
89+
String output = result.getOutput();
90+
assertThat(output, containsString("kc.db-password = " + PropertyMappers.VALUE_MASK));
91+
assertThat(output, not(containsString("kc.db.password")));
92+
assertThat(output, not(containsString("secret-pass")));
93+
}
3194
}

‎quarkus/tests/integration/src/test/resources/ShowConfigCommandTest/keycloak-keystore.conf‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,2 +1,2 @@
1-
config-keystore=src/test/resources/keystore
1+
config-keystore=../../../../src/test/resources/keystore
22
config-keystore-password=secret

0 commit comments

Comments
 (0)