canned-prompts/service/tests/test_health.py
tegwick 8a1a2426d5 Service fixes found by the first real deployment
Three defects the test suite could not have caught, because each needed a real
cluster, a mounted secret, or a live PostgreSQL.

env.py read database_url rather than resolved_database_url. `configured` was
true because a file was set, and the field it then read was empty — so Alembic
received an empty URL and the migration could never run in the cluster. The
value is also now escaped for ConfigParser interpolation, since a `%` in a
generated password would otherwise raise at credential rotation, which is the
worst time to find out.

SET ROLE opened an implicit transaction that Alembic then nested inside rather
than owning, so it never committed and leaving the connection block rolled
everything back. Alembic logged "Running upgrade" for every revision against a
database that stayed empty. SET ROLE is session-scoped, so committing
immediately ends the implicit transaction without discarding the role.

A missing optional publish-token file was treated as a hard failure. The
absence is the documented read-only posture — the secret is mounted optional
and deliberately not issued — so treating it as a fault turned an intended
state into a 500 rather than the 503 that explains it. `required` now separates
the two cases: a missing database URL still fails loudly, because there the
silence would hide a real fault.

Migrations also assume the durable owner role rather than creating objects as
the leased migration login, per the rapp-postgres database-owner boundary. The
role name is validated against an identifier pattern because SET ROLE cannot be
parameterised.

Service tests 36 -> 47.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Bjefh8NUiEiahN4JLwoSKM

Assistant: claude-code
Assistant-Model: opus
Assistant-Process: 388925@bnt-lap001
Assistant-Session: 3507023f-e0fd-4a1e-9d90-a0d4217d1502
2026-09-08 08:56:59 +02:00

140 lines
5.4 KiB
Python

"""Health surface.
The point of these tests is the negative cases. An endpoint that returns 200
under every condition tells an orchestrator nothing, and the failure is silent
exactly when it matters.
"""
from __future__ import annotations
from pathlib import Path
import pytest
from alembic import command
from alembic.config import Config
from alembic.script import ScriptDirectory
from fastapi.testclient import TestClient
from canned_prompts_service.api import create_app
from canned_prompts_service.db import check_readiness, make_engine
from canned_prompts_service.settings import Settings
ROOT = Path(__file__).resolve().parents[1]
def expected_head() -> str:
"""Computed, not hardcoded: a new migration must not fail these tests."""
config = Config(str(ROOT / "alembic.ini"))
config.set_main_option("script_location", str(ROOT / "migrations"))
return ScriptDirectory.from_config(config).get_current_head()
def migrated_url(tmp_path: Path) -> str:
url = f"sqlite:///{tmp_path / 'svc.db'}"
config = Config(str(ROOT / "alembic.ini"))
config.set_main_option("script_location", str(ROOT / "migrations"))
config.set_main_option("sqlalchemy.url", url)
command.upgrade(config, "head")
return url
@pytest.fixture()
def ready_client(tmp_path: Path) -> TestClient:
url = migrated_url(tmp_path)
return TestClient(create_app(Settings(database_url=url), make_engine(url)))
def test_healthz_is_up_without_a_database() -> None:
"""Liveness must not depend on the database, or a DB blip restarts pods."""
client = TestClient(create_app(Settings(), None))
assert client.get("/healthz").json() == {"status": "ok"}
def test_readyz_fails_without_a_database() -> None:
client = TestClient(create_app(Settings(), None))
response = client.get("/readyz")
assert response.status_code == 503
assert response.json()["ready"] is False
assert "no database" in response.json()["detail"]
def test_readyz_fails_when_the_database_is_unreachable(tmp_path: Path) -> None:
engine = make_engine("sqlite:////nonexistent/dir/does-not-exist.db")
client = TestClient(create_app(Settings(database_url="x"), engine))
response = client.get("/readyz")
assert response.status_code == 503
assert response.json()["ready"] is False
def test_readyz_fails_when_the_schema_is_not_migrated(tmp_path: Path) -> None:
"""Reachable but unmigrated is not ready — it would 500 on the first query."""
engine = make_engine(f"sqlite:///{tmp_path / 'empty.db'}")
client = TestClient(create_app(Settings(database_url="x"), engine))
response = client.get("/readyz")
assert response.status_code == 503
assert response.json()["detail"] == "schema not migrated"
def test_readyz_reports_the_migration_when_ready(ready_client: TestClient) -> None:
body = ready_client.get("/readyz").json()
assert body["ready"] is True
assert body["migration"] == expected_head()
def test_state_health_matches_the_fleet_shape(ready_client: TestClient) -> None:
body = ready_client.get("/state/health").json()
assert body["status"] == "ok"
assert body["service"] == "canned-prompts"
assert body["db"] == "connected"
assert body["migration"] == expected_head()
def test_state_health_degrades_rather_than_lying() -> None:
client = TestClient(create_app(Settings(), None))
response = client.get("/state/health")
assert response.status_code == 503
assert response.json()["status"] == "degraded"
def test_settings_have_no_database_fallback() -> None:
"""Falling back to a local database when misconfigured hides the mistake."""
assert Settings().database_url == ""
assert Settings().configured is False
def test_database_url_may_come_from_a_file(tmp_path: Path) -> None:
"""A mounted secret should stay a file, not become an env var."""
secret = tmp_path / "url"
secret.write_text("sqlite:///from-file.db\n", encoding="utf-8")
settings = Settings(database_url_file=str(secret))
assert settings.configured is True
assert settings.resolved_database_url == "sqlite:///from-file.db"
def test_file_wins_over_env_when_both_are_set(tmp_path: Path) -> None:
"""A rotated secret must take effect, not be shadowed by a stale env var."""
secret = tmp_path / "url"
secret.write_text("sqlite:///from-file.db", encoding="utf-8")
settings = Settings(database_url="sqlite:///from-env.db", database_url_file=str(secret))
assert settings.resolved_database_url == "sqlite:///from-file.db"
def test_unreadable_secret_file_fails_loudly(tmp_path: Path) -> None:
settings = Settings(database_url_file=str(tmp_path / "missing"))
with pytest.raises(RuntimeError, match="cannot read secret file"):
_ = settings.resolved_database_url
def test_absent_publish_token_file_means_read_only_not_broken(tmp_path: Path) -> None:
"""The token secret is mounted optional and deliberately not issued. Its
absence is the documented read-only posture, not a fault — treating it as
one returned 500 from /packages instead of a 503 explaining why."""
settings = Settings(publish_token_file=str(tmp_path / "absent"))
assert settings.resolved_publish_token == ""
def test_absent_database_file_still_fails_loudly(tmp_path: Path) -> None:
"""The database URL is required; silence there would hide a real fault."""
settings = Settings(database_url_file=str(tmp_path / "absent"))
with pytest.raises(RuntimeError, match="cannot read secret file"):
_ = settings.resolved_database_url