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

Use fixed ports in Dev Service for Keycloak on shared network & user request #28458

Conversation

michalvavrik
Copy link
Contributor

@michalvavrik michalvavrik commented Oct 8, 2022

fixes: #27763

When Docker host name isn't localhost/127.0.0.1, it's likely that exposed host port don't equal exposed container port. TestContainers are exposing random host port, fixed port is only exposed from the container's perspective. That's a good thing. There are only 2 situations we actually need fixed ports:

  • user configured fixed port (user knows what he is doing)
  • shared network for docker internal communication

I replaced usage of random fixed port for sharedContainer with useSharedNetwork as fixed port has no relation to sharedContainer at all, I think it was a typo at first place introduced here #21999. Second thing I did is to use auto-detected host name and port for external calls only (test runner -> docker, e.g. when Dev Services for Keycloak are creating realm, users and KeycloakTestClient is used inside tests). For internal communication (within shared network), random fixed port is still used as it's okay.

As for tests, to my knowledge you can only test this when Docker endpoint isn't listening on localhost (e.g. remote Docker daemons). I run current test coverage related to Dev Services for Keycloak with local Docker engine and TestContainers Cloud (to verify host name != localhost scenarios). If you have suggestions on how this can be covered with IT, I'm happy to learn.

cc @geoand @sberyozkin

@quarkus-bot quarkus-bot bot added the area/oidc label Oct 8, 2022
@quarkus-bot
Copy link

quarkus-bot bot commented Oct 9, 2022

Failing Jobs - Building 04582da

Status Name Step Failures Logs Raw logs
✔️ JVM Tests - JDK 11
✔️ JVM Tests - JDK 17
JVM Tests - JDK 17 MacOS M1 Set up runner ⚠️ Check → Logs Raw logs
✔️ JVM Tests - JDK 18

Copy link
Member

@sberyozkin sberyozkin left a comment

Choose a reason for hiding this comment

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

@michalvavrik Nice analysis, LGTM, but lets wait for Georgios to review/approve

@sberyozkin sberyozkin merged commit ffb1c67 into quarkusio:main Oct 10, 2022
@quarkus-bot quarkus-bot bot added this to the 2.14 - main milestone Oct 10, 2022
@michalvavrik michalvavrik deleted the feature/dev-services-for-kc-fix-remote-docker branch October 10, 2022 09:01
@gsmet gsmet changed the title Use fixed ports in Dev Svc for Keycloak on shared network & user request Use fixed ports in Dev Service for Keycloak on shared network & user request Oct 10, 2022
@gsmet gsmet modified the milestones: 2.14 - main, 2.13.2.Final Oct 10, 2022
@wjglerum
Copy link
Contributor

Awesome work @michalvavrik! Thanks for this fix. I just managed to verify this locally and also works for me 👍

@michalvavrik
Copy link
Contributor Author

thank you for confirmation @wjglerum :-)

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.

Incorrect Devservices config generated for Keycloak with Testcontainers Cloud
5 participants