Implement CompatibilityMetadataProvider for Cache CLI args #41163

ryanemerson · 2025-07-15T11:11:54Z

Requires #41027

A limitation with the current implementation is that if a default value is not set for a CLI arg then explicitly setting the value will be classes as "incompatible" even though the semantics are the same. For example, the default cache configuration uses the "jdbc-ping" stack, however --cache-stack jdbc-ping is not set. So if the user executes the update command with --cache-stack jdbc-ping explicitly configured then a RECREATE exit code will be returned by the UpdateCompatibility command.

Given that we state the following in the docs, ^ limitation could be considered acceptable:

Ensure that all configuration options, whether set via environment variables or CLI arguments, are included when running this command.

Closes keycloak#41022 Signed-off-by: Ryan Emerson <remerson@redhat.com>

Signed-off-by: Ryan Emerson <remerson@redhat.com>

Signed-off-by: Alexander Schwartz <aschwart@redhat.com>

Closes keycloak#41138 Signed-off-by: Ryan Emerson <remerson@redhat.com>

keycloak-github-bot · 2025-07-15T12:02:02Z

Unreported flaky test detected

If the flaky tests below are affected by the changes, please review and update the changes accordingly. Otherwise, a maintainer should report the flaky tests prior to merging the PR.

org.keycloak.testsuite.federation.kerberos.KerberosLdapCrossRealmTrustTest#test03SpnegoLoginUsernamePassword

Keycloak CI - Base IT (5)

org.openqa.selenium.TimeoutException: 
java.net.SocketTimeoutException: Read timed out
Build info: version: '4.28.1', revision: '73f5ad48a2'
System info: os.name: 'Linux', os.arch: 'amd64', os.version: '6.11.0-1018-azure', java.version: '21.0.7'
Driver info: driver.version: HtmlUnitDriver
...

Report flaky test

org.keycloak.testsuite.federation.ldap.LDAPReadOnlyTest#testReadOnlyUserGetsPermanentlyLocked

Keycloak CI - Base IT (5)

java.lang.AssertionError
	at org.junit.Assert.fail(Assert.java:87)
	at org.junit.Assert.assertTrue(Assert.java:42)
	at org.junit.Assert.assertFalse(Assert.java:65)
	at org.junit.Assert.assertFalse(Assert.java:75)
...

Report flaky test

keycloak-github-bot

Unreported flaky test detected, please review

pruivo

Overall, it looks good to me. I left some suggestions anticipating some future work around the SPI options.

pruivo · 2025-07-16T10:52:22Z

...finispan/src/main/java/org/keycloak/compatibility/AbstractCompatibilityMetadataProvider.java

+    public AbstractCompatibilityMetadataProvider(String spi, boolean enabled) {
+        this.spi = spi;
+        this.enabled = enabled;
+        this.config = enabled ? Config.scope(spi, "default") : null;


Not all providers have the ID default. I would add the providerId as a parameter to make this class future-proof.

For example, this factory has SPI optoins and the provider ID is not default:

keycloak/model/infinispan/src/main/java/org/keycloak/models/sessions/infinispan/InfinispanUserSessionProviderFactory.java

Line 313 in 9322d71

return InfinispanUtils.EMBEDDED_PROVIDER_ID;

pruivo · 2025-07-16T10:54:17Z

...finispan/src/main/java/org/keycloak/compatibility/AbstractCompatibilityMetadataProvider.java

+
+    @Override
+    public Map<String, String> metadata() {
+        if (!enabled)


I would remove the enabled field and create an abstract method isEnabled(Config.Scope).

The provider factory API has the Config.Scope as a parameter; it can use an SPI option/configuration to decide if it is enabled or not.

keycloak/server-spi-private/src/main/java/org/keycloak/provider/EnvironmentDependentProviderFactory.java

Lines 30 to 36 in 3e0a185

/**

* Check if the provider is supported and should be available based on the provider configuration.

*

* @param config the provider configuration

* @return {@code true} if the provider is supported. Otherwise, {@code false}.

*/

boolean isSupported(Config.Scope config);

pruivo · 2025-07-16T10:55:06Z

...pan/src/main/java/org/keycloak/infinispan/compatibility/CachingEmbeddedMetadataProvider.java

+    @Override
+    public Map<String, String> meta() {
+        return Map.of(
+              "persistence", Boolean.toString(MultiSiteUtils.isPersistentSessionsEnabled()),


It can be removed as the Feature is already tracked.

pruivo · 2025-07-16T10:56:17Z

...ispan/src/main/java/org/keycloak/infinispan/compatibility/CachingRemoteMetadataProvider.java

+
+    @Override
+    public Map<String, String> meta() {
+        return Map.of("persistence", Boolean.toString(MultiSiteUtils.isPersistentSessionsEnabled()));


hmm, I think we can remove it here too.

pruivo · 2025-07-16T10:58:31Z

...finispan/src/main/java/org/keycloak/compatibility/AbstractCompatibilityMetadataProvider.java

+        return spi;
+    }
+
+    protected Map<String, String> meta() {


nitpicking

Suggested change

protected Map<String, String> meta() {

protected Map<String, String> customMetadata() {

pruivo · 2025-07-16T11:02:08Z

...finispan/src/main/java/org/keycloak/compatibility/AbstractCompatibilityMetadataProvider.java

+
+    @Override
+    public String getId() {
+        return spi;


I'm not sure if this should be a combination of SPI and the Provider ID. For example, InfinispanUserSessionProviderFactory and RemoteUserSessionProviderFactory both implement the same SPI.
But let's leave it as is for now.

ryanemerson and others added 5 commits July 9, 2025 12:00

Allow Features to declare that they support Rolling upgrades

baba2b1

Closes keycloak#41022 Signed-off-by: Ryan Emerson <remerson@redhat.com>

Pruivo feedback

9e3332f

Signed-off-by: Ryan Emerson <remerson@redhat.com>

Fix UpdateCommandDistTest

3b99d55

Signed-off-by: Ryan Emerson <remerson@redhat.com>

Review

37ff5cd

Signed-off-by: Alexander Schwartz <aschwart@redhat.com>

Implement CompatibilityMetadataProvider for Cache CLI args

2e4811a

Closes keycloak#41138 Signed-off-by: Ryan Emerson <remerson@redhat.com>

keycloak-github-bot bot added the flaky-test label Jul 15, 2025

keycloak-github-bot bot reviewed Jul 15, 2025

View reviewed changes

pruivo reviewed Jul 16, 2025

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Implement CompatibilityMetadataProvider for Cache CLI args #41163

Implement CompatibilityMetadataProvider for Cache CLI args #41163

ryanemerson commented Jul 15, 2025 •

edited

Loading

Uh oh!

keycloak-github-bot bot commented Jul 15, 2025

Uh oh!

keycloak-github-bot bot left a comment

Uh oh!

pruivo left a comment

Uh oh!

pruivo Jul 16, 2025

Uh oh!

pruivo Jul 16, 2025

Uh oh!

pruivo Jul 16, 2025

Uh oh!

pruivo Jul 16, 2025

Uh oh!

pruivo Jul 16, 2025

Uh oh!

pruivo Jul 16, 2025

Uh oh!

Uh oh!

	/**
	* Check if the provider is supported and should be available based on the provider configuration.
	*
	* @param config the provider configuration
	* @return {@code true} if the provider is supported. Otherwise, {@code false}.
	*/
	boolean isSupported(Config.Scope config);

	protected Map<String, String> meta() {
	protected Map<String, String> customMetadata() {

Implement CompatibilityMetadataProvider for Cache CLI args #41163

Are you sure you want to change the base?

Implement CompatibilityMetadataProvider for Cache CLI args #41163

Conversation

ryanemerson commented Jul 15, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

keycloak-github-bot bot commented Jul 15, 2025

Unreported flaky test detected

org.keycloak.testsuite.federation.kerberos.KerberosLdapCrossRealmTrustTest#test03SpnegoLoginUsernamePassword

org.keycloak.testsuite.federation.ldap.LDAPReadOnlyTest#testReadOnlyUserGetsPermanentlyLocked

Uh oh!

keycloak-github-bot bot left a comment

Choose a reason for hiding this comment

Uh oh!

pruivo left a comment

Choose a reason for hiding this comment

Uh oh!

pruivo Jul 16, 2025

Choose a reason for hiding this comment

Uh oh!

pruivo Jul 16, 2025

Choose a reason for hiding this comment

Uh oh!

pruivo Jul 16, 2025

Choose a reason for hiding this comment

Uh oh!

pruivo Jul 16, 2025

Choose a reason for hiding this comment

Uh oh!

pruivo Jul 16, 2025

Choose a reason for hiding this comment

Uh oh!

pruivo Jul 16, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

ryanemerson commented Jul 15, 2025 •

edited

Loading