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: Add Keycloak health port 9000 starting from major version 25 #1213

Merged
merged 10 commits into from
Aug 7, 2024

Conversation

paulomorgado
Copy link
Contributor

What does this PR do?

Starting with tag 25.0 Keycloak uses port 9000 instead of port 8080 for health checks.

Why is it important?

Currently the readiness for image 25.0 and above is not correct and might consider the container ready when it's yet ready.

Related issues

How to test this PR

Use tag 24.0 and tag 25.0 and asset that all correctly wait for the container to be ready.

- Added `KeycloakHealthPort` constant set to 9000 in `KeycloakBuilder` for health check configurations.
- Introduced `DefaultUsername` and `DefaultPassword` constants as "admin" in `KeycloakBuilder` for default access credentials.
- Implemented logic for special handling of Keycloak version 25 and above, including:
  * Parsing the major version from the image tag.
  * Configuring `KeycloakHealthPort` port binding and a wait strategy based on the `/health/ready` endpoint for versions 25+, adapting to new health check mechanisms.
This commit makes significant updates to the KeycloakBuilder and introduces a new test class for Keycloak container health. In the KeycloakBuilder, the Build method has been revised to remove the immediate call to `Validate()` and the direct return of a new `KeycloakContainer`. Instead, it now includes logic to extract the major version number from the `DockerResourceConfiguration.Image.Tag`. If the version is 25 or higher, the builder is modified to add port binding for the Keycloak health port and a wait strategy for the `/health/ready` endpoint. After these potential modifications, `Validate()` is called to ensure the configuration is correct before returning the container.

Additionally, the validation logic that previously checked the Docker image tag's major version has been removed from the direct validation method and incorporated into the Build method. This change streamlines the validation process, focusing it solely on configuration validation.

A new test class, `KeycloakContainerHealthTest`, has been added to verify the health of the Keycloak container. It includes a method `HealthIsReady` that tests whether the container starts successfully and if its health endpoint returns an HTTP OK status. This method dynamically adjusts the `HttpClient.BaseAddress` based on the container's accessible hostname and port, ensuring the test is robust across different environments. The test covers both below and at the threshold versions with inline data for "quay.io/keycloak/keycloak:24.0" and "quay.io/keycloak/keycloak:25.0", ensuring the new health check configuration is properly applied. Containers are disposed of post-test to prevent resource leakage.
Copy link

netlify bot commented Jul 15, 2024

Deploy Preview for testcontainers-dotnet ready!

Name Link
🔨 Latest commit efc84d1
🔍 Latest deploy log https://app.netlify.com/sites/testcontainers-dotnet/deploys/66950d2d8b38de00084f95ab
😎 Deploy Preview https://deploy-preview-1213--testcontainers-dotnet.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

Copy link

netlify bot commented Jul 15, 2024

Deploy Preview for testcontainers-dotnet ready!

Name Link
🔨 Latest commit 1d58852
🔍 Latest deploy log https://app.netlify.com/sites/testcontainers-dotnet/deploys/66b3a8b69fb3550008ee815f
😎 Deploy Preview https://deploy-preview-1213--testcontainers-dotnet.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

@HofmeisterAn
Copy link
Collaborator

For reference, it looks like the changes were introduced in version 25, according to the release notes.

Copy link
Collaborator

@HofmeisterAn HofmeisterAn left a comment

Choose a reason for hiding this comment

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

Sorry it took me so long to review. Thank you for taking the time to contribute, create a PR, and fix the issue. I have a few small change requests. What do you think? Thanks again.

src/Testcontainers.Keycloak/KeycloakBuilder.cs Outdated Show resolved Hide resolved
paulomorgado and others added 7 commits July 31, 2024 19:56
PR suggestion

Co-authored-by: Andre Hofmeister <[email protected]>
Enhanced `KeycloakBuilder` to better handle the `isLatestVersion` flag by including "nightly" tags and refining major version parsing. Updated the wait strategy to use `KeycloakHealthPort` for the latest versions. Renamed `couchbaseBuilder` to `keycloakBuilder` for consistency. Adjusted wait strategy condition to check for any strategies. Added `System.Linq` to global usings in `Usings.cs`.
Enhanced `KeycloakBuilder` to better handle the `isLatestVersion` flag by including "nightly" tags and refining major version parsing. Updated the wait strategy to use `KeycloakHealthPort` for the latest versions. Renamed `couchbaseBuilder` to `keycloakBuilder` for consistency. Adjusted wait strategy condition to check for any strategies. Added `System.Linq` to global usings in `Usings.cs`.
…ners-testcontainers-dotnet into paulomorgado-keycloack-health
@HofmeisterAn HofmeisterAn added the bug Something isn't working label Aug 7, 2024
@HofmeisterAn HofmeisterAn changed the title Add Keycloak health port 9000 starting with tag 25.0 fix: Add Keycloak health port 9000 starting from major version 25 Aug 7, 2024
@HofmeisterAn HofmeisterAn merged commit 1a64762 into testcontainers:develop Aug 7, 2024
11 checks passed
@HofmeisterAn
Copy link
Collaborator

Thanks @paulomorgado 🙏.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Bug]: Keycloak health endpoint is in port 9000 on "quay.io/keycloak/keycloak:25.0.1"
2 participants