diff --git a/.env b/.env index 0b6b01cf..41d88877 100644 --- a/.env +++ b/.env @@ -68,3 +68,12 @@ APP_KEEP_RESULTS=5 APP_ECONOMICS_URI=https://economics.itkdev.dk APP_ECONOMICS_API_KEY=changeme ###< economics ### + +###> health ### +# Seconds to cache the health check results, so that monitoring polling +# /health/ready cannot amplify into load on the database and the broker. +HEALTH_CACHE_TTL=15 +# Seconds since the last detection result before ingest is reported degraded. +# The harvester currently reports several hundred times an hour. +HEALTH_INGEST_MAX_AGE=1800 +###< health ### diff --git a/CHANGELOG.md b/CHANGELOG.md index 0beda8d0..71f41cfd 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,11 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] +- [#91](https://github.com/itk-dev/devops_itksites/pull/91) Health endpoints + - Add `/health/live`, `/health/ready` and `/health/detail` endpoints + - Add health checks for database, RabbitMQ transport and detection result freshness + - Cache check results in a dedicated `cache.health` pool + - Exclude `^/health` from the firewalls and protect `/health/detail` with `ITKBasicAuth` - [#90](https://github.com/itk-dev/devops_itksites/pull/90) - Fixed user API key migration failing on databases with more than one user - Generated an API key for existing users, as users created since already get diff --git a/README.md b/README.md index 797838ed..64b54161 100644 --- a/README.md +++ b/README.md @@ -63,6 +63,43 @@ Use the API key to make an authenticated request, e.g. curl --header 'accept: application/json' --header 'authorization: Apikey ' https://itksites.local.itkdev.dk/api/sites ``` +## Health checks + +Three endpoints report on the application, in increasing order of detail: + +| Endpoint | Access | Checks | +| --- | --- | --- | +| `/health/live` | Public | Nothing – only that the app responds | +| `/health/ready` | Public | All checks, aggregated status only | +| `/health/detail` | `ITKBasicAuth` in Traefik | Per-check results and timings | + +`/health/ready` answers `200` when everything is well and `503` when it is not. +It deliberately does not say *what* is wrong – point monitoring at this one and +read `/health/detail` when it goes red: + +``` shell +curl --silent https://itksites.local.itkdev.dk/health/detail | jq +``` + +The checks cover the database, the RabbitMQ messenger transport and the +freshness of the most recent detection result. The last one catches an ingest +pipeline that has stopped while the application itself is still serving +requests. + +`HEALTH_INGEST_MAX_AGE` sets how old the most recent detection result may be +before ingest is reported as degraded. + +Results are cached for `HEALTH_CACHE_TTL` seconds so that polling does not turn +into load on the dependencies. The cache is the dedicated, filesystem-backed +`cache.health` pool in `config/packages/cache.yaml` – it has to keep working +while the database and the broker are down, and the adapter can be swapped +there without touching code. + +`^/health` is excluded from the Symfony firewalls: both user providers are +Doctrine entity providers, so an authenticated endpoint would fail to +authenticate during a database outage and answer `500` rather than reporting +that the database is down. + ## Development ```sh diff --git a/config/packages/cache.yaml b/config/packages/cache.yaml index c3eb53dd..c3cab50e 100644 --- a/config/packages/cache.yaml +++ b/config/packages/cache.yaml @@ -15,5 +15,15 @@ framework: #app: cache.adapter.apcu # Namespaced pools use the above "app" backend by default - #pools: - #my.dedicated.cache: null + pools: + # Health check results. + # + # Filesystem-backed on purpose: this pool has to keep working while + # the database and the message broker are down, which is exactly + # when the health endpoints matter. + # + # It is a dedicated pool so the adapter can be swapped without + # touching code: cache.adapter.apcu is faster and shared between + # FPM workers, at the cost of being cleared on every FPM restart. + cache.health: + adapter: cache.adapter.filesystem diff --git a/config/packages/security.yaml b/config/packages/security.yaml index b7b82574..e89023b9 100644 --- a/config/packages/security.yaml +++ b/config/packages/security.yaml @@ -21,6 +21,14 @@ security: dev: pattern: ^/(_(profiler|wdt)|css|images|js)/ security: false + # The health endpoints must answer while the database is down, so they + # cannot go through a firewall: both user providers above are Doctrine + # entity providers and authentication would itself fail. /health/detail + # is protected by the ITKBasicAuth middleware in Traefik instead. + health: + pattern: ^/health + security: false + api: pattern: ^/api custom_authenticators: diff --git a/config/services.yaml b/config/services.yaml index 71d40c95..100e7e4b 100644 --- a/config/services.yaml +++ b/config/services.yaml @@ -27,6 +27,23 @@ services: App\Handler\DetectionResultHandlerInterface: tags: [app.handler.detection_result_handler] + App\Health\HealthCheckInterface: + tags: [app.health.check] + + App\Health\HealthChecker: + arguments: + $checks: !tagged_iterator app.health.check + $cache: '@cache.health' + $cacheTtl: '%env(int:HEALTH_CACHE_TTL)%' + + App\Health\Check\RabbitMqHealthCheck: + arguments: + $transport: '@messenger.transport.async' + + App\Health\Check\IngestFreshnessHealthCheck: + arguments: + $maxAgeSeconds: '%env(int:HEALTH_INGEST_MAX_AGE)%' + App\EventListener\RemovedRelationsListener: tags: - name: 'doctrine.event_listener' diff --git a/docker-compose.server.yml b/docker-compose.server.yml index fffc693b..45f2720c 100644 --- a/docker-compose.server.yml +++ b/docker-compose.server.yml @@ -50,3 +50,8 @@ services: # Cron-metrics protection. - "traefik.http.routers.${COMPOSE_PROJECT_NAME:?}-metrics.rule=Host(`${COMPOSE_SERVER_DOMAIN:?}`) && PathPrefix(`/cron-metrics`) " - "traefik.http.routers.${COMPOSE_PROJECT_NAME:?}-metrics.middlewares=ITKMetricsAuth@file" + # Detailed health check protection. /health/live and /health/ready stay + # public; only /health/detail discloses internals. + - "traefik.http.routers.${COMPOSE_PROJECT_NAME:?}-health.rule=Host(`${COMPOSE_SERVER_DOMAIN:?}`) && PathPrefix(`/health/detail`)" + - "traefik.http.routers.${COMPOSE_PROJECT_NAME:?}-health.entrypoints=websecure" + - "traefik.http.routers.${COMPOSE_PROJECT_NAME:?}-health.middlewares=ITKBasicAuth@file" diff --git a/docker-compose.yml b/docker-compose.yml index 4dff2455..444ce64d 100644 --- a/docker-compose.yml +++ b/docker-compose.yml @@ -75,6 +75,10 @@ services: # Cron-metrics protection. - "traefik.http.routers.${COMPOSE_PROJECT_NAME:?}-metrics.rule=Host(`${COMPOSE_DOMAIN:?}`) && PathPrefix(`/cron-metrics`) " - "traefik.http.routers.${COMPOSE_PROJECT_NAME:?}-metrics.middlewares=ITKMetricsAuth@file" + # Detailed health check protection. /health/live and /health/ready stay + # public; only /health/detail discloses internals. + - "traefik.http.routers.${COMPOSE_PROJECT_NAME:?}-health.rule=Host(`${COMPOSE_DOMAIN:?}`) && PathPrefix(`/health/detail`)" + - "traefik.http.routers.${COMPOSE_PROJECT_NAME:?}-health.middlewares=ITKBasicAuth@file" mail: image: axllent/mailpit diff --git a/src/Controller/HealthController.php b/src/Controller/HealthController.php new file mode 100644 index 00000000..603df20a --- /dev/null +++ b/src/Controller/HealthController.php @@ -0,0 +1,98 @@ +respond(['status' => HealthStatus::Ok->value], true); + } + + #[Route('/health/ready', name: 'app_health_ready', methods: ['GET'])] + public function ready(): JsonResponse + { + $healthy = $this->healthChecker->isHealthy($this->healthChecker->run()); + + return $this->respond( + ['status' => $healthy ? HealthStatus::Ok->value : HealthStatus::Degraded->value], + $healthy + ); + } + + #[Route('/health/detail', name: 'app_health_detail', methods: ['GET'])] + public function detail(): JsonResponse + { + $results = $this->healthChecker->run(); + $healthy = $this->healthChecker->isHealthy($results); + + $checks = []; + foreach ($results as $result) { + $checks[$result->name] = array_filter([ + 'status' => $result->status->value, + 'message' => $result->message, + 'details' => $result->details, + ], static fn (mixed $value): bool => null !== $value && [] !== $value); + } + + return $this->respond([ + 'status' => $healthy ? HealthStatus::Ok->value : HealthStatus::Degraded->value, + 'checks' => $checks, + ], $healthy); + } + + /** + * @param array $payload + */ + private function respond(array $payload, bool $healthy): JsonResponse + { + $response = new JsonResponse( + $payload, + $healthy ? Response::HTTP_OK : Response::HTTP_SERVICE_UNAVAILABLE + ); + + // Health responses are cached inside HealthChecker, never by the client + // or an intermediary. + $response->headers->set('Cache-Control', 'no-store, private'); + + return $response; + } +} diff --git a/src/Health/Check/DatabaseHealthCheck.php b/src/Health/Check/DatabaseHealthCheck.php new file mode 100644 index 00000000..94bb8fbb --- /dev/null +++ b/src/Health/Check/DatabaseHealthCheck.php @@ -0,0 +1,48 @@ +connection->executeQuery('SELECT 1'); + } catch (\Throwable $throwable) { + $this->logger->error('Health check "database" failed: {message}', [ + 'message' => $throwable->getMessage(), + 'exception' => $throwable, + ]); + + // The caller gets no detail; the reason stays in the logs. + return HealthCheckResult::degraded($this->getName(), 'Unable to query the database.'); + } + + return HealthCheckResult::ok($this->getName(), [ + 'response_time_ms' => round((microtime(true) - $start) * 1000, 1), + ]); + } +} diff --git a/src/Health/Check/IngestFreshnessHealthCheck.php b/src/Health/Check/IngestFreshnessHealthCheck.php new file mode 100644 index 00000000..ffe4f1e2 --- /dev/null +++ b/src/Health/Check/IngestFreshnessHealthCheck.php @@ -0,0 +1,78 @@ +repository->findLastContact(); + } catch (\Throwable $throwable) { + $this->logger->error('Health check "ingest_freshness" failed: {message}', [ + 'message' => $throwable->getMessage(), + 'exception' => $throwable, + ]); + + // Almost certainly the database being down, which the database + // check reports separately. + return HealthCheckResult::degraded($this->getName(), 'Unable to query the last detection result.'); + } + + if (!$lastContact instanceof \DateTimeImmutable) { + return HealthCheckResult::degraded( + $this->getName(), + 'No detection results have been received yet.', + ['max_age_seconds' => $this->maxAgeSeconds] + ); + } + + $ageSeconds = time() - $lastContact->getTimestamp(); + $details = [ + 'last_contact' => $lastContact->format(\DATE_ATOM), + 'age_seconds' => $ageSeconds, + 'max_age_seconds' => $this->maxAgeSeconds, + ]; + + if ($ageSeconds > $this->maxAgeSeconds) { + return HealthCheckResult::degraded( + $this->getName(), + \sprintf('No detection result received for %d seconds.', $ageSeconds), + $details + ); + } + + return HealthCheckResult::ok($this->getName(), $details); + } +} diff --git a/src/Health/Check/RabbitMqHealthCheck.php b/src/Health/Check/RabbitMqHealthCheck.php new file mode 100644 index 00000000..77a8ef49 --- /dev/null +++ b/src/Health/Check/RabbitMqHealthCheck.php @@ -0,0 +1,57 @@ +transport instanceof MessageCountAwareInterface) { + return HealthCheckResult::skipped( + $this->getName(), + 'The configured transport does not support message counting.' + ); + } + + try { + $count = $this->transport->getMessageCount(); + } catch (\Throwable $throwable) { + $this->logger->error('Health check "rabbitmq" failed: {message}', [ + 'message' => $throwable->getMessage(), + 'exception' => $throwable, + ]); + + return HealthCheckResult::degraded($this->getName(), 'Unable to reach the message queue.'); + } + + return HealthCheckResult::ok($this->getName(), ['queued_messages' => $count]); + } +} diff --git a/src/Health/HealthCheckInterface.php b/src/Health/HealthCheckInterface.php new file mode 100644 index 00000000..41bc448f --- /dev/null +++ b/src/Health/HealthCheckInterface.php @@ -0,0 +1,24 @@ + $details + */ + private function __construct( + public string $name, + public HealthStatus $status, + public ?string $message = null, + public array $details = [], + ) { + } + + /** + * @param array $details + */ + public static function ok(string $name, array $details = []): self + { + return new self($name, HealthStatus::Ok, null, $details); + } + + /** + * @param array $details + */ + public static function degraded(string $name, string $message, array $details = []): self + { + return new self($name, HealthStatus::Degraded, $message, $details); + } + + public static function skipped(string $name, string $message): self + { + return new self($name, HealthStatus::Skipped, $message); + } + + public function isDegraded(): bool + { + return HealthStatus::Degraded === $this->status; + } +} diff --git a/src/Health/HealthChecker.php b/src/Health/HealthChecker.php new file mode 100644 index 00000000..8f2a7cc3 --- /dev/null +++ b/src/Health/HealthChecker.php @@ -0,0 +1,88 @@ + $checks + */ + public function __construct( + private iterable $checks, + private CacheInterface $cache, + private LoggerInterface $logger, + private int $cacheTtl, + ) { + } + + /** + * @return array + */ + public function run(): array + { + return $this->cache->get(self::CACHE_KEY, function (ItemInterface $item): array { + $item->expiresAfter($this->cacheTtl); + + $results = []; + foreach ($this->checks as $check) { + $results[] = $this->runCheck($check); + } + + return $results; + }); + } + + /** + * @param array $results + */ + public function isHealthy(array $results): bool + { + foreach ($results as $result) { + if ($result->isDegraded()) { + return false; + } + } + + return true; + } + + /** + * Run a check, turning an unexpected failure into a degraded result. + * + * A check that throws must not take down the endpoint that reports on it. + */ + private function runCheck(HealthCheckInterface $check): HealthCheckResult + { + try { + return $check->check(); + } catch (\Throwable $throwable) { + $this->logger->error('Health check "{check}" threw an exception: {message}', [ + 'check' => $check->getName(), + 'message' => $throwable->getMessage(), + 'exception' => $throwable, + ]); + + return HealthCheckResult::degraded($check->getName(), 'The check failed unexpectedly.'); + } + } +} diff --git a/src/Health/HealthStatus.php b/src/Health/HealthStatus.php new file mode 100644 index 00000000..282466b9 --- /dev/null +++ b/src/Health/HealthStatus.php @@ -0,0 +1,20 @@ +createQueryBuilder('d') + ->select('MAX(d.lastContact)') + ->getQuery() + ->getSingleScalarResult(); + + return is_string($lastContact) ? new \DateTimeImmutable($lastContact) : null; + } + /** * Remove detection results base on last contact. * diff --git a/tests/Controller/HealthControllerTest.php b/tests/Controller/HealthControllerTest.php new file mode 100644 index 00000000..b8adf86d --- /dev/null +++ b/tests/Controller/HealthControllerTest.php @@ -0,0 +1,110 @@ +request('GET', '/health/live'); + + $this->assertResponseIsSuccessful(); + $this->assertSame(['status' => 'ok'], $this->decode($client->getResponse())); + } + + /** + * All three endpoints must bypass the firewalls. A redirect here means the + * OIDC entry point has caught them, which is what made a total ingest + * outage look healthy to monitoring. + */ + #[DataProvider('endpointProvider')] + public function testEndpointIsPubliclyReachable(string $path): void + { + $client = static::createClient(); + $client->request('GET', $path); + + $this->assertContains( + $client->getResponse()->getStatusCode(), + [Response::HTTP_OK, Response::HTTP_SERVICE_UNAVAILABLE], + \sprintf('%s should answer 200 or 503, never a redirect to login.', $path) + ); + } + + /** + * @return iterable + */ + public static function endpointProvider(): iterable + { + yield ['/health/live']; + yield ['/health/ready']; + yield ['/health/detail']; + } + + /** + * Readiness is public, so it must disclose the aggregated status and + * nothing else — no check names, no messages, no dependency detail. + */ + public function testReadyDisclosesNothingBeyondStatus(): void + { + $client = static::createClient(); + $client->request('GET', '/health/ready'); + + $payload = $this->decode($client->getResponse()); + + $this->assertSame(['status'], array_keys($payload)); + $this->assertContains($payload['status'], ['ok', 'degraded']); + } + + public function testDetailReportsEveryCheck(): void + { + $client = static::createClient(); + $client->request('GET', '/health/detail'); + + $payload = $this->decode($client->getResponse()); + + $this->assertArrayHasKey('checks', $payload); + $this->assertEqualsCanonicalizing( + ['database', 'rabbitmq', 'ingest_freshness'], + array_keys($payload['checks']) + ); + + foreach ($payload['checks'] as $check) { + $this->assertContains($check['status'], ['ok', 'degraded', 'skipped']); + } + } + + public function testResponsesAreNotCacheableByClients(): void + { + $client = static::createClient(); + $client->request('GET', '/health/ready'); + + $this->assertResponseHeaderSame('Cache-Control', 'no-store, private'); + } + + /** + * @return array + */ + private function decode(Response $response): array + { + $content = $response->getContent(); + $this->assertIsString($content); + + $payload = json_decode($content, true, 512, \JSON_THROW_ON_ERROR); + $this->assertIsArray($payload); + + return $payload; + } +}