Skip to content

Fix Keycloak urls in example to work with recent versions of Keycloak #250

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

Merged
merged 1 commit into from
Nov 20, 2024

Conversation

mstruk
Copy link
Contributor

@mstruk mstruk commented Nov 20, 2024

The OAuth2 endpoints in Keycloak examples still use the urls as they were by default on legacy versions of Keycloak (e.g. 19.0.3-legacy). In the testsuite we moved some time ago to start using recent versions of Keycloak (e.g. 23.0.x) and we adjusted the OAuth2 endpoint urls there to use the default structure compatible with recent Keycloak releases.

Specifically, where before the url would look like: https://localhost:8443/auth/realms/demo
it now looks like: https://localhost:8443/realms/demo

Signed-off-by: Marko Strukelj <[email protected]>
@mstruk mstruk added this to the 0.16.0 milestone Nov 20, 2024
Copy link
Member

@scholzj scholzj left a comment

Choose a reason for hiding this comment

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

I think this deserves a better description. For example expain why did the URL changed? (new Keycloak version I guess?)

@@ -91,5 +91,5 @@ log4j.logger.state.change.logger=TRACE, stateChangeAppender
log4j.additivity.state.change.logger=false

# Access denials are logged at INFO level, change to DEBUG to also log allowed accesses
log4j.logger.kafka.authorizer.logger=INFO, authorizerAppender
log4j.logger.kafka.authorizer.logger=DEBUG, authorizerAppender
Copy link
Member

Choose a reason for hiding this comment

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

Intentional?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It was intentional but this reminds me now of having once issues with Travis CI failing due to log files becoming to big, and this might cause that issue to appear again.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I guess we can wait and see if CI fails, and if it does revert this change.

@mstruk
Copy link
Contributor Author

mstruk commented Nov 20, 2024

I added a better (hopefully) description.

@mstruk
Copy link
Contributor Author

mstruk commented Nov 20, 2024

Travis is failing for s390x only, and I've seen it fail this way for #249 as well, so I'm pretty sure it's not related to the changes in this PR. And actually, the DEBUG logging adds very minimal extra logs - only some additional info that comes handy when debugging authorization, so I think we can leave it as is.

@mstruk mstruk merged commit a43db9b into strimzi:main Nov 20, 2024
8 of 9 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants