-
Notifications
You must be signed in to change notification settings - Fork 41.2k
Add OAuth2-Client ConnectionDetails and Keycloak Docker-Compose support #39391
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
Add OAuth2-Client ConnectionDetails and Keycloak Docker-Compose support #39391
Conversation
Any opinions on this @philwebb , @mhalbritter? 😀 |
We're currently preparing the Spring Boot 3.3 release. This PR is planned as an enhancement for 3.4 and we'll look into it in detail when 3.3 is out. |
Thanks for the PR, @PhilKes. Can you please describe how you identified the properties that should be part of The four different labels that are being used to supply some of the values feels like a smell to me as it tells me that these settings (probably) aren't coming from the Docker Compose-managed service but from user configuration. Typically, we'd want the user's standard configuration to be reused (so that their use of Docker Compose matches as closely as possible to running their application "properly") and where test-specific configuration is needed, it can be provided through application properties rather than Docker Compose labels. |
Thanks for your remarks @wilkinsona
All other properties should then only be set via About the labels, the Thank you for your feedback. |
I think the breadth of the connection details is going to be quite hard to get right. I'm not sure that it makes sense to be able to override a registration's provider when using Docker Compose or Testcontainers. If you're using one of the common providers, would we want to encourage overriding it to use a "test" provider? I'm not sure that we would as what you're testing would be quite far removed from what will happen at runtime. I think I'll take this to #36777 as that's probably a better place to figure it out. |
Thanks again for the PR, @PhilKes. Unfortunately, it's become clear from #36777 that I don't think we're ready to start implementing something and it looks like this may not be heading in the right direction. I'm going to close this one so that we can take a step back and figure out exactly what we want to do. |
Closes #36777
This PR includes:
Registration
+Provider
Open Questions:
Since the
Registration
has a lot fields that cant simply be extracted from any Keycloak env. variables, I added Docker service labels to set e.g. thescope
of the client, or the client secret. Is this the best way to handle this?I also thought about not ignoring the
PropertiesOAuth2ClientConnectionDetails
if the Docker-Compose support already created anOAuth2ClientConnectionDetails
Bean, but rather merge them together, so that the registration-id, provider are extracted from the Keycloak env. variables and the other settings (scope
,client-secret
, etc.) could be set in theapplication.properties
. I am not sure which to prefer.When the open questions are answered, I would also add the TestContainer-Support for Keycloak.