mirror of
https://github.com/keycloak/keycloak.git
synced 2026-06-11 10:00:06 -04:00
fix: use to values ahead of keycloak defaults (#46871)
* fix: use `to` values ahead of keycloak defaults closes: #46728 Signed-off-by: Steve Hawkins <shawkins@redhat.com> * Update docs/documentation/upgrading/topics/changes/changes-26_6_0.adoc Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> Signed-off-by: Steven Hawkins <shawkins@redhat.com> * Apply suggestion from @shawkins Signed-off-by: Steven Hawkins <shawkins@redhat.com> --------- Signed-off-by: Steve Hawkins <shawkins@redhat.com> Signed-off-by: Steven Hawkins <shawkins@redhat.com> Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
This commit is contained in:
Steven Hawkins
GitHub
parent
9ccf47bd40
commit
29d00b07f3
9 changed files with 118 additions and 37 deletions
|
|
@ -25,16 +25,6 @@ Configuring + defers validation to the client's standard 'Valid redirect URIs'.
|
|||
|
||||
Administrators must ensure that all configured URIs for the fields listed above use https. Clients attempting to update or register with http in these fields will fail validation when using the 'secure-client-uris' executor.
|
||||
|
||||
=== New CLI option overrides Quarkus transaction timeout property
|
||||
|
||||
A new CLI option `transaction-default-timeout` has been introduced to configure the default transaction timeout in {project_name}.
|
||||
This new option will override any value set in the `quarkus.transaction-manager.default-transaction-timeout` Quarkus property.
|
||||
|
||||
While using Quarkus properties directly is not the supported way to configure {project_name}, some users may have used `quarkus.transaction-manager.default-transaction-timeout` to customize the transaction timeout.
|
||||
If you are using this Quarkus property, the new `transaction-default-timeout` CLI option will take precedence and override your configuration when upgrading to this version.
|
||||
|
||||
To maintain your desired transaction timeout configuration, migrate to using the `transaction-default-timeout` CLI option and remove the `quarkus.transaction-manager.default-transaction-timeout` property from your configuration.
|
||||
|
||||
=== The Identity Provider issuer should be unique for the JWT authorization grant and client assertions
|
||||
|
||||
The JWT authorization grant and the client assertion authentication use the `issuer` claim passed in the assertion to locate the Identity Provider defined in {project_name}, and that provider validates the claims and signature in the assertion. Since this version, the `issuer` configuration option should be unique to only identify one provider. The grant operation or the assertion client authentication fail if more providers are returned for a single issuer. In the same way, the Identity Providers, when configured to allow the JWT authorization grant or client assertions, are validated to have a unique issuer defined in its configuration. Any update or add operation will fail if the issuer collides with another provider.
|
||||
|
|
@ -77,6 +67,27 @@ For more information about brokering, see link:{developerguide_link}#_identity_b
|
|||
Notable changes may include internal behavior changes that prevent common misconfigurations, bugs that are fixed, or changes to simplify running {project_name}.
|
||||
It also lists significant changes to internal APIs.
|
||||
|
||||
=== Usage of second-class options
|
||||
|
||||
In prior releases the introduction of a first-class option with a default would cause the second-class option (typically spi or quarkus properties) to no longer be used at all.
|
||||
|
||||
For example, `spi-truststore--file--hostname-verification-policy` was marked as deprecated after the introduction of `tls-hostname-verifier`. However since
|
||||
`tls-hostname-verifier` included a default, direct usage of `spi-truststore--file--hostname-verification-policy` was no longer considered.
|
||||
|
||||
To ensure that the further introduction of new first-class options does not immediately break existing configurations, the logic for determining the value of a second-class option
|
||||
will now consider the direct usage of the second-class value ahead of a default, or derived default, from the first-class option.
|
||||
|
||||
Please review the server startup to see if there are any warnings about the continued usage of second-class options, and update them accordingly.
|
||||
|
||||
=== New option for transaction timeout
|
||||
|
||||
A new option `transaction-default-timeout` has been introduced to configure the default transaction timeout in {project_name}.
|
||||
|
||||
While using Quarkus properties directly is not the supported way to configure {project_name}, some users may have used `quarkus.transaction-manager.default-transaction-timeout` to customize the transaction timeout.
|
||||
If you are using this Quarkus property, the new `transaction-default-timeout` CLI option when set will take precedence and override your configuration when upgrading to this version.
|
||||
|
||||
To maintain your desired transaction timeout configuration, migrate to using the `transaction-default-timeout` CLI option and remove the `quarkus.transaction-manager.default-transaction-timeout` property from your configuration.
|
||||
|
||||
=== Endpoints are opened while Keycloak is initializing
|
||||
|
||||
By default, {project_name} now opens its HTTP(S) and Management ports while initialization is still in progress.
|
||||
|
|
|
|||
|
|
@ -43,6 +43,9 @@ Based on the priority of application, the value that is used at startup is `cliV
|
|||
|
||||
If `--db-url=cliValue` had not been used, the applied value would be `KC_DB_URL=envVarValue`. If the value were not applied by either the command line or an environment variable, `db-url=confFileValue` would be used. If none of the previous values were applied, the value `kc.db-url=keystoreValue` would be used due to the lowest priority among the available configuration sources.
|
||||
|
||||
{project_name} configuration options often map to lower-level Quarkus or spi options. We refer to these as first-class and second-class configuration respectively.
|
||||
There are startup warnings to indicate if you are using a second-class option. For example if you try to directly use `quarkus.http.port`, you will be warned to use `kc.http-port` instead.
|
||||
|
||||
== Formats for configuration
|
||||
The configuration uses a _unified-per-source_ format, which simplifies translation of a key/value pair from one configuration source to another. Note that these formats apply to spi options as well.
|
||||
|
||||
|
|
@ -144,8 +147,6 @@ You can use only a https://github.com/keycloak/keycloak/blob/main/quarkus/runtim
|
|||
|
||||
You can also store Quarkus properties in a Java KeyStore.
|
||||
|
||||
Note that some Quarkus properties are already mapped in the {project_name} configuration, such as `quarkus.http.port` and similar essential properties. If the property is used by {project_Name}, defining that property key in `quarkus.properties` has no effect. The {project_Name} configuration value takes precedence over the Quarkus property value.
|
||||
|
||||
=== Using special characters in values
|
||||
|
||||
{project_name} depends upon Quarkus and MicroProfile for processing configuration values. Be aware that value expressions are supported. For example, `$\{some_key}` evaluates to the value of `some_key`.
|
||||
|
|
|
|||
|
|
@ -268,14 +268,13 @@ public class Picocli {
|
|||
// first validate the advertised property names
|
||||
// - this allows for efficient resolution of wildcard values and checking spi options
|
||||
Configuration.getPropertyNames().forEach(name -> {
|
||||
if (!name.startsWith(MicroProfileConfigProvider.NS_KEYCLOAK_PREFIX)) {
|
||||
return; // there are canonical mappings to kc. values - no need to consider alternative forms
|
||||
}
|
||||
if (!options.includeRuntime) {
|
||||
checkRuntimeSpiOptions(name, ignoredRunTime);
|
||||
}
|
||||
if (PropertyMappers.isMaybeSpiBuildTimeProperty(name)) {
|
||||
ambiguousSpi.add(name);
|
||||
if (name.startsWith(PropertyMappers.KC_SPI_PREFIX)) {
|
||||
if (!options.includeRuntime) {
|
||||
checkRuntimeSpiOptions(name, ignoredRunTime);
|
||||
}
|
||||
if (PropertyMappers.isMaybeSpiBuildTimeProperty(name)) {
|
||||
ambiguousSpi.add(name);
|
||||
}
|
||||
}
|
||||
PropertyMapper<?> mapper = PropertyMappers.getMapper(name);
|
||||
if (mapper == null) {
|
||||
|
|
@ -288,8 +287,10 @@ public class Picocli {
|
|||
secondClassOptions.put(name, forKey.getFrom());
|
||||
}
|
||||
}
|
||||
if (!mapper.hasWildcard()) {
|
||||
return; // non-wildcard options will be validated in the next pass
|
||||
if (!mapper.hasWildcard() // non-wildcard options will be validated in the next pass
|
||||
// validate only canonical mappings to kc. values - no need to consider alternative forms
|
||||
|| !name.startsWith(MicroProfileConfigProvider.NS_KEYCLOAK_PREFIX)) {
|
||||
return;
|
||||
}
|
||||
validateProperty(abstractCommand, options, ignoredRunTime, disabledBuildTime, disabledRunTime,
|
||||
deprecatedInUse, missingOption, disabledMappers.contains(mapper), forKey, unnecessary);
|
||||
|
|
@ -335,7 +336,11 @@ public class Picocli {
|
|||
warn("The following SPI options are using the legacy format and are not being treated as build time options. Please use the new format with the appropriate -- separators to resolve this ambiguity: " + String.join("\n", ambiguousSpi));
|
||||
}
|
||||
secondClassOptions.forEach((key, firstClass) -> {
|
||||
warn("Please use the first-class option `%s` instead of `%s`".formatted(firstClass, key));
|
||||
if (Configuration.getConfigValue(firstClass).getConfigSourceName() != null) {
|
||||
warn("With the first-class option `%s` set, you should remove the usage of `%s`".formatted(firstClass, key));
|
||||
} else {
|
||||
warn("Please use the first-class option `%s` instead of `%s`".formatted(firstClass, key));
|
||||
}
|
||||
});
|
||||
if (!unnecessary.isEmpty()) {
|
||||
info("The following options were specified, but are typically not relevant for this command: " + String.join("\n", unnecessary));
|
||||
|
|
@ -444,9 +449,6 @@ public class Picocli {
|
|||
}
|
||||
|
||||
private static void checkRuntimeSpiOptions(String key, final List<String> ignoredRunTime) {
|
||||
if (!key.startsWith(PropertyMappers.KC_SPI_PREFIX)) {
|
||||
return;
|
||||
}
|
||||
boolean buildTimeOption = PropertyMappers.isSpiBuildTimeProperty(key);
|
||||
|
||||
if (!buildTimeOption) {
|
||||
|
|
|
|||
|
|
@ -104,7 +104,7 @@ public final class Build extends AbstractCommand {
|
|||
if (Start.NAME.equals(cmd) || Build.NAME.equals(cmd)) {
|
||||
executionError(spec.commandLine(), Messages.devProfileNotAllowedError(cmd));
|
||||
}
|
||||
} else if (Configuration.getConfigValue(DB).getConfigSourceOrdinal() == 0) {
|
||||
} else if (Configuration.isDefault(Configuration.getConfigValue(DB))) {
|
||||
picocli.warn("Usage of the default value for the db option in the production profile is deprecated. Please explicitly set the db instead.");
|
||||
}
|
||||
}
|
||||
|
|
|
|||
|
|
@ -22,6 +22,7 @@ import java.util.Optional;
|
|||
import java.util.Properties;
|
||||
|
||||
import org.keycloak.config.Option;
|
||||
import org.keycloak.quarkus.runtime.configuration.mappers.PropertyMapper;
|
||||
import org.keycloak.utils.StringUtil;
|
||||
|
||||
import io.quarkus.runtime.configuration.ConfigUtils;
|
||||
|
|
@ -50,9 +51,16 @@ public final class Configuration {
|
|||
return getOptionalBooleanValue(NS_KEYCLOAK_PREFIX + option.getKey()).orElse(false);
|
||||
}
|
||||
|
||||
/**
|
||||
* Return true if the value is not derived (meaning that the config source name is set)
|
||||
* and the config source is something that a user can change the values in
|
||||
*/
|
||||
public static boolean isUserModifiable(ConfigValue configValue) {
|
||||
// for now we won't validate these as it's not expected for the user to specify options from a source below our properties
|
||||
return configValue.getConfigSourceName() != null && configValue.getConfigSourceOrdinal() >= KeycloakPropertiesConfigSource.PROPERTIES_FILE_ORDINAL;
|
||||
return configValue.getConfigSourceName() != null && !isDefault(configValue);
|
||||
}
|
||||
|
||||
public static boolean isDefault(ConfigValue configValue) {
|
||||
return configValue.getConfigSourceOrdinal() <= PropertyMapper.DEFAULT_VALUE_ORDINAL;
|
||||
}
|
||||
|
||||
public static boolean isSet(Option<?> option) {
|
||||
|
|
|
|||
|
|
@ -61,6 +61,7 @@ import static org.keycloak.quarkus.runtime.configuration.MicroProfileConfigProvi
|
|||
|
||||
public class PropertyMapper<T> {
|
||||
|
||||
public static final int DEFAULT_VALUE_ORDINAL = 251; // keycloak defaults are logically higher than classpath entries
|
||||
protected final Option<T> option;
|
||||
private final String to;
|
||||
private Function<AbstractCommand, Boolean> enabled;
|
||||
|
|
@ -117,12 +118,18 @@ public class PropertyMapper<T> {
|
|||
* <p>
|
||||
* In preference order we are looking for:
|
||||
* <pre>
|
||||
* [ {@link #from} ] ---> [ {@link #mapFrom} ] ---> [ {@link #getDefaultValue()} ] ---> [ {@link #to} ]
|
||||
* (explicit) (derived) (fallback) (fallback)
|
||||
* [ {@link #from} ] ---> [ {@link #mapFrom} ] ---> [ {@link #getDefaultValue()} ]
|
||||
* (explicit) (derived) (fallback)
|
||||
* </pre>
|
||||
* <p>
|
||||
*
|
||||
* <b>2. Transform found value</b>
|
||||
* <b>2. Use to</b>
|
||||
* <p>
|
||||
* If we are looking for the `to` value, and no value was found from step 1 or it is effectively a default value,
|
||||
* then use the `to` value if it was set by the user.
|
||||
* <p>
|
||||
*
|
||||
* <b>3. Transform found value</b>
|
||||
* <p>
|
||||
* If we found a value for the attribute name, it needs to be transformed via {@link #transformValue} method. How to transform it?
|
||||
* <ul>
|
||||
|
|
@ -130,6 +137,10 @@ public class PropertyMapper<T> {
|
|||
* <li>If the value contains an expression, expand it using SmallRye logic
|
||||
* <li>Finally the returned {@link ConfigValue} is made to match what was requested - with the name, value, rawValue, and ordinal set appropriately.
|
||||
* </ul>
|
||||
*
|
||||
* <b>4. Use to</b>
|
||||
* <p>
|
||||
* If no value is found or mapped, return the `to` value.
|
||||
*/
|
||||
ConfigValue getConfigValue(String name, ConfigSourceInterceptorContext context) {
|
||||
String from = getFrom();
|
||||
|
|
@ -138,6 +149,7 @@ public class PropertyMapper<T> {
|
|||
// we don't want the NestedPropertyMappingInterceptor to restart the chain here, so we force a proceed
|
||||
// this ensures that mapFrom transformers, and regular transformers are applied exclusively - not chained
|
||||
ConfigValue config = convertValue(NestedPropertyMappingInterceptor.proceed(context, from));
|
||||
Optional<ConfigValue> directValue = null;
|
||||
|
||||
boolean parentValue = false;
|
||||
if (mapFrom != null && (config == null || config.getValue() == null)) {
|
||||
|
|
@ -148,12 +160,21 @@ public class PropertyMapper<T> {
|
|||
parentValue = true;
|
||||
}
|
||||
|
||||
// instead of using a default or mapping from a default, check if the value is already set and use that instead
|
||||
// a warning should be emitted about this in picocli
|
||||
if (!name.equals(from) && (config == null || config.getValue() == null || (parentValue && Configuration.isDefault(config)))) {
|
||||
directValue = Optional.ofNullable(context.proceed(name));
|
||||
if (directValue.filter(c -> c.getValue() != null && Configuration.isUserModifiable(c)).isPresent()) {
|
||||
return directValue.get();
|
||||
}
|
||||
}
|
||||
|
||||
if (config != null && config.getValue() != null) {
|
||||
config = transformValue(name, config, context, parentValue);
|
||||
} else {
|
||||
String defaultValue = this.option.getDefaultValue().map(Option::getDefaultValueString).orElse(null);
|
||||
config = transformValue(name, new ConfigValueBuilder().withName(name)
|
||||
.withValue(defaultValue).withRawValue(defaultValue).build(),
|
||||
.withValue(defaultValue).withRawValue(defaultValue).withConfigSourceOrdinal(DEFAULT_VALUE_ORDINAL).build(),
|
||||
context, false);
|
||||
}
|
||||
|
||||
|
|
@ -162,6 +183,9 @@ public class PropertyMapper<T> {
|
|||
}
|
||||
|
||||
// now try any defaults from quarkus
|
||||
if (directValue != null) {
|
||||
return directValue.orElse(null);
|
||||
}
|
||||
return context.proceed(name);
|
||||
}
|
||||
|
||||
|
|
|
|||
|
|
@ -756,7 +756,26 @@ public class PicocliTest extends AbstractConfigurationTest {
|
|||
public void derivedPropertyUsage() {
|
||||
NonRunningPicocli nonRunningPicocli = pseudoLaunch("start-dev", "--hostname=localhost", "--spi-hostname-v2-hostname=second-class");
|
||||
assertEquals(CommandLine.ExitCode.OK, nonRunningPicocli.exitCode);
|
||||
assertThat(nonRunningPicocli.getOutString(), containsString("Please use the first-class option `kc.hostname` instead of `kc.spi-hostname-v2-hostname`"));
|
||||
assertThat(nonRunningPicocli.getOutString(), containsString("With the first-class option `kc.hostname` set, you should remove the usage of `kc.spi-hostname-v2-hostname`"));
|
||||
}
|
||||
|
||||
@Test
|
||||
public void quarkusPropertyInQuarkusPropertiesWarning() {
|
||||
putEnvVar("QUARKUS_HTTP_PORT", "9090");
|
||||
NonRunningPicocli nonRunningPicocli = pseudoLaunch("start-dev");
|
||||
assertEquals(CommandLine.ExitCode.OK, nonRunningPicocli.exitCode);
|
||||
assertThat(nonRunningPicocli.getOutString(),
|
||||
containsString("Please use the first-class option `kc.http-port` instead of `quarkus.http.port`"));
|
||||
}
|
||||
|
||||
@Test
|
||||
public void quarkusPropertyInQuarkusPropertiesWarningWhenKcOptionSet() {
|
||||
putEnvVar("QUARKUS_HTTP_PORT", "9090");
|
||||
// When kc.http-port is explicitly set, should still warn about quarkus.http.port
|
||||
NonRunningPicocli nonRunningPicocli = pseudoLaunch("start-dev", "--http-port=7070");
|
||||
assertEquals(CommandLine.ExitCode.OK, nonRunningPicocli.exitCode);
|
||||
assertThat(nonRunningPicocli.getOutString(),
|
||||
containsString("With the first-class option `kc.http-port` set, you should remove the usage of `quarkus.http.port`"));
|
||||
}
|
||||
|
||||
@Test
|
||||
|
|
|
|||
|
|
@ -713,6 +713,25 @@ public class ConfigurationTest extends AbstractConfigurationTest {
|
|||
assertEquals("200k", config.getConfigValue("quarkus.http.limits.max-header-size").getValue());
|
||||
}
|
||||
|
||||
@Test
|
||||
public void testQuarkusPropertyTakesPrecedenceOverDefault() {
|
||||
putEnvVar("QUARKUS_HTTP_PORT", "9090");
|
||||
ConfigArgsConfigSource.setCliArgs("");
|
||||
SmallRyeConfig config = createConfig();
|
||||
assertEquals("9090", config.getConfigValue("quarkus.http.port").getValue());
|
||||
// the kc.http-port still returns the default since no kc.* option was explicitly set
|
||||
assertEquals("8080", config.getConfigValue("kc.http-port").getValue());
|
||||
}
|
||||
|
||||
@Test
|
||||
public void testKcPropertyTakesPrecedenceOverQuarkusProperty() {
|
||||
// Explicitly setting kc.http-port should override the quarkus.properties value (9090)
|
||||
ConfigArgsConfigSource.setCliArgs("--http-port=7070");
|
||||
SmallRyeConfig config = createConfig();
|
||||
assertEquals("7070", config.getConfigValue("quarkus.http.port").getValue());
|
||||
assertEquals("7070", config.getConfigValue("kc.http-port").getValue());
|
||||
}
|
||||
|
||||
@Test
|
||||
public void testPostgresTLSOptions() {
|
||||
doDatabaseTlsOptionTest("postgres",
|
||||
|
|
|
|||
|
|
@ -2,9 +2,6 @@
|
|||
|
||||
# Logging configuration. INFO is the default level for most of the categories
|
||||
quarkus.log.min-level=TRACE
|
||||
quarkus.log.level = INFO
|
||||
quarkus.log.category."org.jboss.resteasy.resteasy_jaxrs.i18n".level=WARN
|
||||
quarkus.log.category."org.infinispan.transaction.lookup.JBossStandaloneJTAManagerLookup".level=WARN
|
||||
|
||||
# For test nested properties
|
||||
quarkus.datasource.foo = jdbc:h2:file:${kc.home.dir:${kc.db.url.path:~}}/data/keycloakdb
|
||||
|
|
|
|||
Loading…
Reference in a new issue