Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: Adds support for Keycloak 26.1 #1256

Merged
merged 1 commit into from
Jan 28, 2025

Conversation

maximilian-krauss
Copy link
Contributor

What this PR does / why we need it:

Adds support for Keycloak 26.1.

Which issue this PR fixes: fixes #1253

PR Readiness Checklist:

Complete these before marking the PR as ready to review:

  • the CHANGELOG.md release notes have been updated to reflect any significant (and particularly user-facing) changes introduced by this PR

@AssahBismarkabah AssahBismarkabah self-requested a review January 17, 2025 11:03
Copy link
Collaborator

@AssahBismarkabah AssahBismarkabah left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi @maximilian-krauss,

Thank you for the fix. , the issue on the ci seems to not be feasible, as we are also experiencing the same failure for those keycloak versions. I would appreciate it if you could look into the root cause.

For the lint-other-files step, ubuntu-latest jumped to ubuntu-24 which did not support python 3.7, you can pin the version to ubuntu-22.04

@maximilian-krauss
Copy link
Contributor Author

maximilian-krauss commented Jan 17, 2025

Hi @AssahBismarkabah,
That's a pretty odd behavior. Its pretty hard to reproduce the failing behaviour and it also seems to be different between KC26 and KC26.1 👀

@maximilian-krauss maximilian-krauss force-pushed the feat/keycloak-26-1 branch 2 times, most recently from 9cb1421 to 274b35a Compare January 17, 2025 14:22
@AssahBismarkabah
Copy link
Collaborator

@maximilian-krauss I also noticed that too, but the error below occurs locally using kc-26.1 at ImportAuthenticationFlowsIT. However, I couldn't replicate the other failure at ChecksumServiceCasesIT.

java.lang.AssertionError: 
Expected: a collection with size <2>
     but: collection size was <1>

	at org.hamcrest.MatcherAssert.assertThat(MatcherAssert.java:20)
	at org.hamcrest.MatcherAssert.assertThat(MatcherAssert.java:6)
	at de.adorsys.keycloak.config.service.ImportAuthenticationFlowsIT.shouldUpdateMultipleExecutionsWithSameAuthenticator(ImportAuthenticationFlowsIT.java:764)
	at java.base/java.lang.reflect.Method.invoke(Method.java:580)
	at java.base/java.util.ArrayList.forEach(ArrayList.java:1596)
	at java.b
	```

@maximilian-krauss
Copy link
Contributor Author

Yes, I also can reproduce the ImportAuthenticationFlowsIT failure, but only if I run it the whole suite via maven. If I execute them via IntelliJ they work just fine

@antikalk
Copy link
Contributor

Yes, I also can reproduce the ImportAuthenticationFlowsIT failure, but only if I run it the whole suite via maven. If I execute them via IntelliJ they work just fine

On my side it also fails when just running the ImportAuthenticationFlowsIT in Intellij. I had a look at the test code, which confuses me:

List<AuthenticatorConfigRepresentation> authConfig;
authConfig = getAuthenticatorConfig(realm, "id1");
assertThat(authConfig, hasSize(2));

Why should we find two configs for the same alias?

@maximilian-krauss
Copy link
Contributor Author

Hey @antikalk,
I can reproduce it here too now. Maybe the behaviour was wrong in the first place and has been fixed recently 🤔

@bohmber
Copy link
Contributor

bohmber commented Jan 26, 2025

The ImportClientsIT.shouldRemoveAuthzPoliciesForRealmManagement failure is caused by a new client scope.
See
Migrating to 26.1.0->Notable changes->New client scope service_account for client_credentials grant mappers

@bohmber
Copy link
Contributor

bohmber commented Jan 27, 2025

if (VersionUtil.lt(KEYCLOAK_VERSION, "26.1")) {
        assertThat(client.getDefaultClientScopes(), containsInAnyOrder("web-origins", "profile", "roles", "email"));
} else {
        assertThat(client.getDefaultClientScopes(), containsInAnyOrder("web-origins", "profile", "roles", "email", "service_account"));
}

would be enough

@bohmber
Copy link
Contributor

bohmber commented Jan 27, 2025

The ImportAuthenticationFlowsIT failures could caused by:
Check alias is unique for authenticator config when it is created

Then the test is no longer correct

@maximilian-krauss maximilian-krauss force-pushed the feat/keycloak-26-1 branch 3 times, most recently from c877a80 to faf2b32 Compare January 27, 2025 12:26
@maximilian-krauss
Copy link
Contributor Author

Hey @bohmber,

thanks for pointing that out. I updated the tests and they are passing now. Seems reasonable why the behavior changed.

Cheers,
Max

@bohmber
Copy link
Contributor

bohmber commented Jan 28, 2025

@maximilian-krauss can you merge the latest changes from main into your branch?

@maximilian-krauss
Copy link
Contributor Author

Hey @bohmber, done 🙂

@bohmber
Copy link
Contributor

bohmber commented Jan 28, 2025

I tested these code changes with our configuration and keycloak 26.1 without problems.

@AssahBismarkabah can you review the changes, please?

@AssahBismarkabah AssahBismarkabah self-requested a review January 28, 2025 11:19
Copy link
Collaborator

@AssahBismarkabah AssahBismarkabah left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It works fine on my end as well @bohmber, thanks @maximilian-krauss .

@AssahBismarkabah AssahBismarkabah merged commit 0a7798e into adorsys:main Jan 28, 2025
17 checks passed
@antikalk
Copy link
Contributor

@AssahBismarkabah is there an ETA on when a KC 26.1 compatible release will be available? :)

@maximilian-krauss maximilian-krauss deleted the feat/keycloak-26-1 branch January 29, 2025 14:03
@AssahBismarkabah
Copy link
Collaborator

Hi @antikalk, we are currently working on that. The release will be available before the end of today. Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Development

Successfully merging this pull request may close these issues.

Keycloak Config CLI fails with Keycloak 26.1
4 participants