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

Share containers in DevServices for Keycloak #19631

Merged
merged 1 commit into from
Aug 27, 2021

Conversation

sberyozkin
Copy link
Member

@sberyozkin sberyozkin commented Aug 24, 2021

This PR adds the code (similarly to what is done in KafkaClient) for Keycloak DevServices DevMode containers to be optionally shared.

For now I've decided to avoid a serviceName property since for Keycloak it is a realmName which is a key identifier so I think it is fine for a label value.

CC @sebastienblanc

@stuartwdouglas
Copy link
Member

stuartwdouglas commented Aug 25, 2021

For the record only as I've removed it from the description:
Sergey:

@stuartwdouglas Most of this code is about a label based container discovery (I copied it from KafkaClient) - something Sebastien also asked about (using the same container when multiple Quarkus applications are started).

At the very end of the PR changes you can see the test code - specifically note how the access tokens are acquired. What ahpoenshappens there is that when that test is launched DevServices binds to the existing container started by the test factory. This may be a total nonsense, but I'd like to understand what are the alternatives.

Stuart:

I don't really think we should be doing this. DevServices is supposed to be a replacement for KeycloakTestResourceLifecycleManager. If you are using KeycloakTestResourceLifecycleManager then DevServices should be disabled (in fact we should add a facility to automatically allow this).

At the moment container based discovery is only intended for use in dev mode, it is intended for tests to get their own isolated instance that is configured correctly for the tests.

@sberyozkin sberyozkin changed the title Make Keycloak test factory and DevServices for Keycloak share the containers Share containers in DevServices for Keycloak Aug 25, 2021
@sberyozkin sberyozkin force-pushed the kc_dev_services_label branch from e09e31a to 6bf8ac4 Compare August 25, 2021 16:42
@sberyozkin sberyozkin marked this pull request as ready for review August 25, 2021 16:43
@sberyozkin sberyozkin force-pushed the kc_dev_services_label branch 2 times, most recently from eec9b4b to b58bc4c Compare August 25, 2021 16:53
Copy link
Member

@stuartwdouglas stuartwdouglas left a comment

Choose a reason for hiding this comment

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

I would rather keep it consistent with regard to using serviceName rather than realmName to match all the other services.

@sberyozkin sberyozkin force-pushed the kc_dev_services_label branch from b58bc4c to 57de14b Compare August 26, 2021 17:36
@sberyozkin
Copy link
Member Author

sberyozkin commented Aug 26, 2021

@stuartwdouglas

I would rather keep it consistent with regard to using serviceName rather than realmName to match all the other services.

This is done

@sberyozkin sberyozkin merged commit 3afc800 into quarkusio:main Aug 27, 2021
@quarkus-bot quarkus-bot bot added this to the 2.3 - main milestone Aug 27, 2021
@sberyozkin sberyozkin deleted the kc_dev_services_label branch August 27, 2021 07:26
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.

2 participants