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

Fix DevServices for Keycloak loading the realm file from the file system #21976

Merged

Conversation

sberyozkin
Copy link
Member

@sberyozkin sberyozkin commented Dec 6, 2021

Fixes #21975

(and also removes an unused shared property in StartResult)

See also quarkusio/quarkus-quickstarts#1001

@quarkus-bot quarkus-bot bot added the area/oidc label Dec 6, 2021
@stuartwdouglas
Copy link
Member

Can you add a test for this?

@sberyozkin
Copy link
Member Author

Hi @stuartwdouglas It is in the linked quickstart PR, I did not want to copy the large realm file here; in oidc_token_propagation which uses Dev Services it is tested that Dev Services does not create a realm. Would that quickstart test do ? I don't mind, I can rework some of the existing tests here around the realm file...

@sberyozkin
Copy link
Member Author

@stuartwdouglas I can update @geoand 's test once #21999 get merged to use a file system path to the realm file, 3 quickstarts currently use the class based resource for testing. Though having only one of the quickstarts checking the file system path should also be OK IMHO, it will fail if the regression is introduced here before the release...

@sberyozkin
Copy link
Member Author

sberyozkin commented Dec 8, 2021

Hi Stuart, @stuartwdouglas It took me awhile to find where I can add the realm file, with a few tries, but finally, integration-tests/oidc-client-reactive-filter has come to the rescue :-). It should do it, thanks

@geoand
Copy link
Contributor

geoand commented Dec 9, 2021

Thanks for this @sberyozkin!

@sberyozkin
Copy link
Member Author

Hi @geoand Can you have a look please since you also worked recently on updating Keycloak Dev Services (and I recall it was a more difficult update than adding a file system bind :-) ) - this PR does that and updates the test

@geoand
Copy link
Contributor

geoand commented Dec 9, 2021

Should this be backported to 2.6 and 2.5?

@sberyozkin
Copy link
Member Author

Thanks for a super fast review @geoand :-)

@sberyozkin
Copy link
Member Author

Yeah, if it can make it to 2.5.x it would be fine, let me add the labels

@sberyozkin sberyozkin merged commit 6008c39 into quarkusio:main Dec 9, 2021
@quarkus-bot quarkus-bot bot added this to the 2.7 - main milestone Dec 9, 2021
@sberyozkin sberyozkin deleted the devservices_keycloak_realm_file_path branch December 9, 2021 10:01
@geoand
Copy link
Contributor

geoand commented Dec 9, 2021

Great, thanks!

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

Successfully merging this pull request may close these issues.

Regression: DevServices for Keycloak fails to load a realm file from the file system
4 participants