aws-samples / cdk-keycloak

CDK construct library that allows you to create KeyCloak on AWS in TypeScript or Python
Apache License 2.0
88 stars 30 forks source link

Health check failures w/ KeyCloak v26.0.0 #218

Open alukach opened 1 month ago

alukach commented 1 month ago

When Keycloak v26.0.0 runs, unauthenticated requests to / are responded to with a 302 redirect to the admin interface. Instead, for Keycloak v26.0.0, health checks should be made to server:9000/health (if Server Health Checks are enabled) or server:8080/admin/master/console/. As currently set up, the ECS Fargate services fail the healthcheck requests to / given that the return a 302 rather than 200. This causes the Cloudformation stack to never stabilize and eventually fail.

How to reproduce

  1. Start the server:

    $ docker run \
      -it --rm \
      -p 8080:8080 \
      -e KC_BOOTSTRAP_ADMIN_USERNAME=admin \
      -e KC_BOOTSTRAP_ADMIN_PASSWORD=admin \
      quay.io/keycloak/keycloak:26.0.0 start-dev
  2. Verify response from /:

    $ curl localhost:8080 --verbose

    You will see a 302 Found response:

    *   Trying 127.0.0.1:8080...
    * Connected to localhost (127.0.0.1) port 8080 (#0)
    > GET / HTTP/1.1
    > Host: localhost:8080
    > User-Agent: curl/8.1.2
    > Accept: */*
    > 
    < HTTP/1.1 302 Found
    < Location: http://localhost:8080/admin/
    < Referrer-Policy: no-referrer
    < Strict-Transport-Security: max-age=31536000; includeSubDomains
    < X-Content-Type-Options: nosniff
    < X-XSS-Protection: 1; mode=block
    < content-length: 0
    < 
    * Connection #0 to host localhost left intact

Possible fix

Support customized Health Check

When creating the ContainerService, we don't permit customizing the health check for our target group:

https://github.com/aws-samples/cdk-keycloak/blob/1b33a79ea1e7130b97095633241281af782fb3ed/src/keycloak.ts#L887-L891

I believe that we should allow passing in a health check argument into the KeyCloak construct that would be passed down to the ContainerService construct and used when registering our application target. This would allow users to customize the health check (e.g. its path) as needed by their version of Keycloak or via their configuration.

Additional container port

Additionally, I believe we should also add port 9000 to the container ports to support the standard Server Health Checks:

https://github.com/aws-samples/cdk-keycloak/blob/1b33a79ea1e7130b97095633241281af782fb3ed/src/keycloak.ts#L774-L780

Uncertainties

Perhaps, instead of adding a customized health check, we should instead auto-configure Keycloak v26 deployments to run with the server health check enabled and with the listener targets checking server:9000/health. I admit that I'm not familiar enough with Keycloak to know when this health check endpoint became available and when / started redirecting to /admin.

Adrastopoulos commented 1 day ago

Yeah, I’ve run into this too. The health endpoint is definitely only on port 9000 if you have server health checks enabled—at least for Quarkus Keycloak like v26.0.0. I’m not sure if it’s like that for all versions, but it seems to be standard.

The 302 redirect from / to /admin is an issue for ECS health checks since they expect a 200. Pointing the check to server:9000/health fixed it for me. You just need to make sure port 9000 is exposed as you mentioned.