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.

Sign up for GitHub

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 health check support for Keycloak container #7122

Merged
merged 9 commits into from
Jan 19, 2025
Merged

Add health check support for Keycloak container #7122

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
b4c775e
Add health check support for Keycloak container
paulomorgado Jan 15, 2025
cd15831
Refactor Keycloak tests for endpoint annotations and manifest
paulomorgado Jan 16, 2025
8f5b2d0
Revert change made by mistake.
paulomorgado Jan 16, 2025
8d68ae4
Update src/Aspire.Hosting.Keycloak/KeycloakResourceBuilderExtensions.cs
paulomorgado Jan 17, 2025
8a9e05b
Update src/Aspire.Hosting.Keycloak/KeycloakResourceBuilderExtensions.cs
paulomorgado Jan 17, 2025
b4f542a
Update src/Aspire.Hosting.Keycloak/KeycloakResourceBuilderExtensions.cs
paulomorgado Jan 17, 2025
c219e7d
Update src/Aspire.Hosting.Keycloak/KeycloakResourceBuilderExtensions.cs
paulomorgado Jan 17, 2025
c874176
Refactor Keycloak resource constants and tests
paulomorgado Jan 17, 2025
086e02e
Merge branch 'main' into Keycloak-HealthChecks
davidfowl Jan 18, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions src/Aspire.Hosting.Keycloak/KeycloakResourceBuilderExtensions.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,10 @@ public static class KeycloakResourceBuilderExtensions
{
private const string AdminEnvVarName = "KEYCLOAK_ADMIN";
private const string AdminPasswordEnvVarName = "KEYCLOAK_ADMIN_PASSWORD";
private const string HealthCheckEnvVarName = "KC_HEALTH_ENABLED";
DamianEdwards marked this conversation as resolved.
Show resolved Hide resolved
private const int DefaultContainerPort = 8080;
private const int HealthCheckContainerPort = 9000;
paulomorgado marked this conversation as resolved.
Show resolved Hide resolved
DamianEdwards marked this conversation as resolved.
Show resolved Hide resolved
private const string HealthEndpointName = "health";
paulomorgado marked this conversation as resolved.
Show resolved Hide resolved
private const string RealmImportDirectory = "/opt/keycloak/data/import";

/// <summary>
Expand Down Expand Up @@ -59,10 +62,13 @@ public static IResourceBuilder<KeycloakResource> AddKeycloak(
.WithImageRegistry(KeycloakContainerImageTags.Registry)
.WithImageTag(KeycloakContainerImageTags.Tag)
.WithHttpEndpoint(port: port, targetPort: DefaultContainerPort)
.WithHttpEndpoint(targetPort: HealthCheckContainerPort, name: HealthEndpointName)
paulomorgado marked this conversation as resolved.
Show resolved Hide resolved
Copy link
Member

Choose a reason for hiding this comment

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

Consider adding the management interface port as an optional parameter too (like port) so it can be easily changed by the caller. If it's set then we'd need to set the KC_HTTP_MANAGEMENT_PORT environment variable too.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Wouldn't that mean that we should also support KC_HTTP_PORT?

But maybe we could add a managementPort parameter with the same semantics of the port parameter, without messing with target ports.

Would it be feasible to determine the target ports based on the existence or not of KC_HTTP_PORT and KC_HTTP_MANAGEMENT_PORT?

Copy link
Member

Choose a reason for hiding this comment

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

But maybe we could add a managementPort parameter with the same semantics of the port parameter, without messing with target ports.

Yeah this is what I think I meant 😄 Just let the user specify a different management port which we'll assign to the proxy port. Target port will stay the same always (unless user manually edits the added the endpoint, which of course they can still do).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Added with tests!

.WithHttpHealthCheck(endpointName: HealthEndpointName, path: "/health/ready")
paulomorgado marked this conversation as resolved.
Show resolved Hide resolved
.WithEnvironment(context =>
{
context.EnvironmentVariables[AdminEnvVarName] = resource.AdminReference;
context.EnvironmentVariables[AdminPasswordEnvVarName] = resource.AdminPasswordParameter;
context.EnvironmentVariables[HealthCheckEnvVarName] = "true";
});

if (builder.ExecutionContext.IsRunMode)
Expand Down
20 changes: 18 additions & 2 deletions tests/Aspire.Hosting.Keycloak.Tests/KeycloakResourceBuilderTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ public void AddKeycloakWithDefaultsAddsAnnotationMetadata()
var containerResource = Assert.Single(appModel.Resources.OfType<KeycloakResource>());
Assert.Equal(resourceName, containerResource.Name);

var endpoint = Assert.Single(containerResource.Annotations.OfType<EndpointAnnotation>());
var endpoint = Assert.Single(containerResource.Annotations.OfType<EndpointAnnotation>().Where(e => e.Name == "http"));
Assert.Equal(8080, endpoint.TargetPort);
Assert.False(endpoint.IsExternal);
Assert.Equal("http", endpoint.Name);
Expand All @@ -35,6 +35,15 @@ public void AddKeycloakWithDefaultsAddsAnnotationMetadata()
Assert.Equal("http", endpoint.Transport);
Assert.Equal("http", endpoint.UriScheme);

var healthEndpoint = Assert.Single(containerResource.Annotations.OfType<EndpointAnnotation>().Where(e => e.Name == "health"));
Assert.Equal(9000, healthEndpoint.TargetPort);
Assert.False(healthEndpoint.IsExternal);
Assert.Equal("health", healthEndpoint.Name);
Assert.Null(healthEndpoint.Port);
Assert.Equal(ProtocolType.Tcp, healthEndpoint.Protocol);
Assert.Equal("http", healthEndpoint.Transport);
Assert.Equal("http", healthEndpoint.UriScheme);

var containerAnnotation = Assert.Single(containerResource.Annotations.OfType<ContainerImageAnnotation>());
Assert.Equal(KeycloakContainerImageTags.Tag, containerAnnotation.Tag);
Assert.Equal(KeycloakContainerImageTags.Image, containerAnnotation.Image);
Expand Down Expand Up @@ -142,14 +151,21 @@ public async Task VerifyManifest()
],
"env": {
"KEYCLOAK_ADMIN": "admin",
"KEYCLOAK_ADMIN_PASSWORD": "{keycloak-password.value}"
"KEYCLOAK_ADMIN_PASSWORD": "{keycloak-password.value}",
"KC_HEALTH_ENABLED": "true"
},
"bindings": {
"http": {
"scheme": "http",
"protocol": "tcp",
"transport": "http",
"targetPort": 8080
},
"health": {
"scheme": "http",
"protocol": "tcp",
"transport": "http",
"targetPort": 9000
}
}
}
Expand Down
Loading