fix: refining breaking behavior (#40697)

closes: #39063

Signed-off-by: Steve Hawkins <shawkins@redhat.com>
This commit is contained in:
Steven Hawkins 2025-06-25 11:28:00 -04:00 committed by GitHub
parent 615a82b6c9
commit a50d15be05
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
5 changed files with 45 additions and 26 deletions

View File

@ -3,21 +3,6 @@
Breaking changes are identified as requiring changes from existing users to their configurations.
In minor or patch releases, we will only do breaking changes to fix important bugs.
=== SPI Options naming
If you were relying upon SPI properties ending in `-enabled`, `-provider`, `-provider-default` being seen as build time options for either triggering an auto-build or producing an error when a build is needed, then this is a breaking change.
For everyone else, this is a notable change, and you should update any used SPI option to the new format eventually.
SPI options ending in `-enabled`, `-provider-default`, or `-provider` were previously treated as buildtime. However, in some instances, this was not correct as a provider could have a configuration property ending in one of those suffixes as well.
To resolve this ambiguity, and any potential ambiguity involving SPI and provider names, a new SPI option format was introduced where the scopes and suffix are separated by `--`(double dash) instead of `-`(dash).
An SPI property ending in `-enabled`, `-provider-default`, or `-provider` will only be treated as buildtime if it is using the new format, e.g. `spi-<spi-name>--<provider-name>--enabled` will be recognized as a buildtime option.
For instance, the correct way to reference your custom email template is: `--spi-email-template--mycustomprovider--enabled` (not `--spi-email-template-mycustomprovider-enabled`).
Options using the legacy format and ending in `-enabled`, `-provider-default`, or `-provider` will instead be treated as runtime and a warning will be emitted.
=== Reading information about temporarily locked users
In previous releases there was an inconsistency in the REST endpoint result of getting a user (`+GET /admin/realms/{realm}/users/{user-id}+`) and searching for a user (`+GET /admin/realms/{realm}/users+`). When BruteForce is enabled and a user was temporarily locked out the former endpoint would return `enabled=false` while the latter would return `enabled=true`. If the user was updated and enabled was false due to temporary lockout then the user would be disabled permanently. Both endpoints now return `enabled=true` when a user is temporarily locked out. To check whether a user is temporarily locked out the BruteForceUserResource endpoint should be utilised (`+GET /admin/realms/{realm}/attack-detection/brute-force/users/{userId}+`).
@ -34,6 +19,18 @@ user profile configuration where too much information was returned in the past.
Notable changes where an internal behavior changed to prevent common misconfigurations, fix bugs or simplify running {project_name}.
=== SPI Options naming
SPI options ending in `-enabled`, `-provider-default`, or `-provider` are treated as buildtime options. However, in some instances, this was not correct as a provider could have a configuration property ending in one of those suffixes as well.
To resolve this ambiguity, and any potential ambiguity involving SPI and provider names, a new SPI option format was introduced where the scopes and suffix are separated by `--`(double dash) instead of `-`(dash).
An SPI property ending in `-enabled`, `-provider-default`, or `-provider` should use the new format or else a warning will be emitted - e.g. `spi-<spi-name>--<provider-name>--enabled` will be recognized as a buildtime option without a warning.
For instance, the correct way to reference your custom email template is: `--spi-email-template--mycustomprovider--enabled` (not `--spi-email-template-mycustomprovider-enabled`).
Options using the legacy format and ending in `-enabled`, `-provider-default`, or `-provider` will sill be treated as a buildtime option, but may not be in future releases.
=== Different credentials of a user need to have different names
When adding an OTP, WebAuthn or any other 2FA credentials, the name the user assigns to this credential needs to be unique for the given user.

View File

@ -466,7 +466,7 @@ public class Picocli {
warn("The following used options or option values are DEPRECATED and will be removed or their behaviour changed in a future release:\n" + String.join("\n", deprecatedInUse) + "\nConsult the Release Notes for details.", getOutWriter());
}
if (!ambiguousSpi.isEmpty()) {
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));
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), getOutWriter());

View File

@ -97,11 +97,13 @@ public final class PropertyMappers {
}
public static boolean isSpiBuildTimeProperty(String name) {
return name.startsWith(KC_SPI_PREFIX) && (name.endsWith("--provider") || name.endsWith("--enabled") || name.endsWith("--provider-default"));
// we can't require the new property formant until we're ok with a breaking change
//return name.startsWith(KC_SPI_PREFIX) && (name.endsWith("--provider") || name.endsWith("--enabled") || name.endsWith("--provider-default"));
return name.startsWith(KC_SPI_PREFIX) && (name.endsWith("-provider") || name.endsWith("-enabled") || name.endsWith("-provider-default"));
}
public static boolean isMaybeSpiBuildTimeProperty(String name) {
return name.startsWith(KC_SPI_PREFIX) && (name.endsWith("-provider") || name.endsWith("-enabled") || name.endsWith("-provider-default")) && !name.contains("--");
return isSpiBuildTimeProperty(name) && !name.contains("--");
}
private static boolean isKeycloakRuntime(String name, PropertyMapper<?> mapper) {

View File

@ -35,6 +35,7 @@ import java.nio.file.Paths;
import java.util.List;
import java.util.Map;
import java.util.Properties;
import java.util.function.Consumer;
import java.util.stream.Stream;
import org.apache.commons.io.FileUtils;
@ -326,16 +327,22 @@ public class PicocliTest extends AbstractConfigurationTest {
/**
* Runs a fake build to setup the state of the persisted build properties
*/
@SuppressWarnings({"unchecked", "rawtypes"})
private NonRunningPicocli build(String... args) {
return build(out -> {
assertFalse(out, out.contains("first-class"));
assertFalse(out, out.toUpperCase().contains("WARN"));
}, args);
}
@SuppressWarnings({"unchecked", "rawtypes"})
private NonRunningPicocli build(Consumer<String> outChecker, String... args) {
if (Stream.of(args).anyMatch("start-dev"::equals)) {
Environment.setRebuildCheck(); // auto-build
}
NonRunningPicocli nonRunningPicocli = pseudoLaunch(args);
assertTrue(nonRunningPicocli.reaug);
assertEquals(nonRunningPicocli.getErrString(), CommandLine.ExitCode.OK, nonRunningPicocli.exitCode);
assertFalse(nonRunningPicocli.getOutString(), nonRunningPicocli.getOutString().contains("first-class"));
assertFalse(nonRunningPicocli.getOutString(), nonRunningPicocli.getOutString().toUpperCase().contains("WARN"));
outChecker.accept(nonRunningPicocli.getOutString());
onAfter();
addPersistedConfigValues((Map)nonRunningPicocli.buildProps);
return nonRunningPicocli;
@ -614,14 +621,14 @@ public class PicocliTest extends AbstractConfigurationTest {
public void testAmbiguousSpiOption() {
NonRunningPicocli nonRunningPicocli = pseudoLaunch("start-dev", "--spi-x-y-enabled=true");
assertEquals(CommandLine.ExitCode.OK, nonRunningPicocli.exitCode);
assertThat(nonRunningPicocli.getOutString(), containsString("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: kc.spi-x-y-enabled"));
assertThat(nonRunningPicocli.getOutString(), containsString("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: kc.spi-x-y-enabled"));
}
@Test
public void testAmbiguousSpiOptionBuild() {
NonRunningPicocli nonRunningPicocli = pseudoLaunch("build", "--db=dev-file", "--spi-x-y-enabled=true");
assertEquals(CommandLine.ExitCode.OK, nonRunningPicocli.exitCode);
assertThat(nonRunningPicocli.getOutString(), containsString("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: kc.spi-x-y-enabled"));
assertThat(nonRunningPicocli.getOutString(), containsString("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: kc.spi-x-y-enabled"));
}
@Test
@ -774,4 +781,17 @@ public class PicocliTest extends AbstractConfigurationTest {
assertEquals(CommandLine.ExitCode.USAGE, nonRunningPicocli.exitCode);
assertThat(nonRunningPicocli.getErrString(), containsString("The following build time options have values that differ from what is persisted - the new values will NOT be used until another build is run: kc.spi-events-listener--provider"));
}
@Test
public void spiAmbiguousSpiAutoBuild() {
putEnvVar("KC_SPI_EVENTS_LISTENER_PROVIDER", "jboss-logging");
NonRunningPicocli nonRunningPicocli = build(out -> assertThat(out, containsString("The following SPI options")), "build", "--db=dev-file");
Environment.setRebuildCheck(); // auto-build
putEnvVar("KC_SPI_EVENTS_LISTENER_PROVIDER", "new-jboss-logging");
nonRunningPicocli = pseudoLaunch("start", "--http-enabled=true", "--hostname-strict=false");
assertEquals(CommandLine.ExitCode.OK, nonRunningPicocli.exitCode);
assertTrue(nonRunningPicocli.reaug);
assertThat(nonRunningPicocli.getOutString(), containsString("The following SPI options"));
}
}

View File

@ -32,9 +32,9 @@ public class PropertyMappersTest {
assertTrue(PropertyMappers.isSpiBuildTimeProperty("kc.spi.foo.bar--provider-default"));
// return false for non-build time properties
assertFalse(PropertyMappers.isSpiBuildTimeProperty("kc.spi.foo.bar-provider"));
assertFalse(PropertyMappers.isSpiBuildTimeProperty("kc.spi.foo.bar-enabled"));
assertFalse(PropertyMappers.isSpiBuildTimeProperty("kc.spi.foo.bar-provider-default"));
//assertFalse(PropertyMappers.isSpiBuildTimeProperty("kc.spi.foo.bar-provider"));
//assertFalse(PropertyMappers.isSpiBuildTimeProperty("kc.spi.foo.bar-enabled"));
//assertFalse(PropertyMappers.isSpiBuildTimeProperty("kc.spi.foo.bar-provider-default"));
assertFalse(PropertyMappers.isSpiBuildTimeProperty("some.other.property"));
assertFalse(PropertyMappers.isSpiBuildTimeProperty("kc.spi.foo.bar"));
}