diff --git a/.github/scripts/cloud_run_env.sh b/.github/scripts/cloud_run_env.sh index 66ca3c5f1..9f8c0282a 100755 --- a/.github/scripts/cloud_run_env.sh +++ b/.github/scripts/cloud_run_env.sh @@ -5,6 +5,9 @@ cloud_run_set_defaults() { CLOUD_RUN_REGION="${CLOUD_RUN_REGION:-us-central1}" CLOUD_RUN_SERVICE="${CLOUD_RUN_SERVICE:-policyengine-api}" CLOUD_RUN_ARTIFACT_REPOSITORY="${CLOUD_RUN_ARTIFACT_REPOSITORY:-policyengine-api}" + # Image name stays fixed across services: the production deploy reuses the + # image built by the staging track, so it must not embed the service name. + CLOUD_RUN_IMAGE_NAME="${CLOUD_RUN_IMAGE_NAME:-policyengine-api}" CLOUD_RUN_RUNTIME_SERVICE_ACCOUNT="${CLOUD_RUN_RUNTIME_SERVICE_ACCOUNT:-policyengine-api-cr-runtime@policyengine-api.iam.gserviceaccount.com}" CLOUD_RUN_CLOUD_SQL_INSTANCE="${CLOUD_RUN_CLOUD_SQL_INSTANCE:-policyengine-api:us-central1:policyengine-api-data}" CLOUD_RUN_CPU="${CLOUD_RUN_CPU:-4}" @@ -22,7 +25,7 @@ cloud_run_set_defaults() { local sha sha="${GITHUB_SHA:-local}" CLOUD_RUN_IMAGE_TAG="${CLOUD_RUN_IMAGE_TAG:-${sha}}" - CLOUD_RUN_IMAGE_URI="${CLOUD_RUN_IMAGE_URI:-${CLOUD_RUN_REGION}-docker.pkg.dev/${CLOUD_RUN_PROJECT}/${CLOUD_RUN_ARTIFACT_REPOSITORY}/${CLOUD_RUN_SERVICE}:${CLOUD_RUN_IMAGE_TAG}}" + CLOUD_RUN_IMAGE_URI="${CLOUD_RUN_IMAGE_URI:-${CLOUD_RUN_REGION}-docker.pkg.dev/${CLOUD_RUN_PROJECT}/${CLOUD_RUN_ARTIFACT_REPOSITORY}/${CLOUD_RUN_IMAGE_NAME}:${CLOUD_RUN_IMAGE_TAG}}" local short_sha short_sha="${sha:0:7}" @@ -32,6 +35,7 @@ cloud_run_set_defaults() { export CLOUD_RUN_REGION export CLOUD_RUN_SERVICE export CLOUD_RUN_ARTIFACT_REPOSITORY + export CLOUD_RUN_IMAGE_NAME export CLOUD_RUN_RUNTIME_SERVICE_ACCOUNT export CLOUD_RUN_CLOUD_SQL_INSTANCE export CLOUD_RUN_CPU diff --git a/.github/scripts/deploy_cloud_run_candidate.sh b/.github/scripts/deploy_cloud_run_candidate.sh index f97dd9a29..c76388f1a 100755 --- a/.github/scripts/deploy_cloud_run_candidate.sh +++ b/.github/scripts/deploy_cloud_run_candidate.sh @@ -47,6 +47,7 @@ cloud_run_run gcloud run deploy "${CLOUD_RUN_SERVICE}" \ --add-cloudsql-instances "${CLOUD_RUN_CLOUD_SQL_INSTANCE}" \ --port "${CLOUD_RUN_PORT}" \ --cpu "${CLOUD_RUN_CPU}" \ + --cpu-boost \ --memory "${CLOUD_RUN_MEMORY}" \ --timeout "${CLOUD_RUN_TIMEOUT}" \ --min-instances "${CLOUD_RUN_MIN_INSTANCES}" \ diff --git a/.github/scripts/health_check.sh b/.github/scripts/health_check.sh index b61e9f470..41411b365 100644 --- a/.github/scripts/health_check.sh +++ b/.github/scripts/health_check.sh @@ -4,11 +4,14 @@ set -euo pipefail health_url="${1:?health check URL is required}" timeout_seconds="${HEALTH_CHECK_TIMEOUT_SECONDS:-900}" -interval_seconds="${HEALTH_CHECK_INTERVAL_SECONDS:-10}" +interval_seconds="${HEALTH_CHECK_INTERVAL_SECONDS:-5}" +curl_max_time_seconds="${HEALTH_CHECK_CURL_MAX_TIME_SECONDS:-15}" deadline=$((SECONDS + timeout_seconds)) while (( SECONDS < deadline )); do - if curl --silent --show-error --fail "${health_url}" >/dev/null; then + if curl --silent --show-error --fail \ + --max-time "${curl_max_time_seconds}" \ + "${health_url}" >/dev/null; then exit 0 fi sleep "${interval_seconds}" diff --git a/.github/workflows/push.yml b/.github/workflows/push.yml index 501fbd26e..0ed07c284 100644 --- a/.github/workflows/push.yml +++ b/.github/workflows/push.yml @@ -211,6 +211,8 @@ jobs: (github.repository == 'PolicyEngine/policyengine-api') && (github.event.head_commit.message == 'Update PolicyEngine API') environment: staging + env: + CLOUD_RUN_SERVICE: policyengine-api-staging permissions: contents: read id-token: write @@ -316,6 +318,8 @@ jobs: (github.repository == 'PolicyEngine/policyengine-api') && (github.event.head_commit.message == 'Update PolicyEngine API') environment: staging + env: + CLOUD_RUN_SERVICE: policyengine-api-staging permissions: contents: read id-token: write @@ -471,6 +475,8 @@ jobs: (github.repository == 'PolicyEngine/policyengine-api') && (github.event.head_commit.message == 'Update PolicyEngine API') environment: production + env: + CLOUD_RUN_SERVICE: policyengine-api permissions: contents: read id-token: write diff --git a/changelog.d/migration-pr4-stage1-staging-isolation.changed.md b/changelog.d/migration-pr4-stage1-staging-isolation.changed.md new file mode 100644 index 000000000..c8d37b662 --- /dev/null +++ b/changelog.d/migration-pr4-stage1-staging-isolation.changed.md @@ -0,0 +1 @@ +Isolated staging Cloud Run deploys onto a dedicated policyengine-api-staging service and tagged every API response with an X-PolicyEngine-Backend header. diff --git a/docs/migration/cloud-run-operations.md b/docs/migration/cloud-run-operations.md new file mode 100644 index 000000000..5472e08c0 --- /dev/null +++ b/docs/migration/cloud-run-operations.md @@ -0,0 +1,98 @@ +# Cloud Run Operations (living reference) + +This is the single living reference for how API v1's Cloud Run deployments work. It is +edited in place whenever behavior changes. Per-stage records (bootstrap commands, +exit-gate evidence, one-off measurements) are append-only documents under `history/` and +are never updated to match later reality. + +Configuration **values** (CPU, memory, scaling, secrets, service names) live in +`.github/scripts/cloud_run_env.sh` and the deploy scripts — they are the source of truth +and are intentionally not duplicated here. This document explains the **semantics**. + +## Topology + +- The public host `api.policyengine.org` is served by **App Engine** (bound via + `gcp/dispatch.yaml`). No public traffic reaches Cloud Run until the load-balancer + stages of the host cutover plan (`api-v1-pr4-cloud-run-host-cutover-execution-plan.md` + in the planning folder). +- Two Cloud Run services in `policyengine-api` / `us-central1`: + - **`policyengine-api`** — the production candidate. Every push to master deploys a + tagged `--no-traffic` revision (`stage3-prod-*`), which is promoted to the service + URL after integration tests. Only CI and its own health checks call this URL today. + - **`policyengine-api-staging`** — the staging track's service (split from the + production service in migration PR 4, Stage 1). Staging jobs pin + `CLOUD_RUN_SERVICE: policyengine-api-staging`; unit tests enforce that every + service-targeting Cloud Run job carries an explicit pin and that no staging job can + touch production traffic. +- The Docker image name is decoupled from the service name (`CLOUD_RUN_IMAGE_NAME`): + production reuses the image built by the staging track, so both tracks push and pull + `policyengine-api:` regardless of service. +- Every API response carries `X-PolicyEngine-Backend` (`app_engine` or `cloud_run`) for + in-band backend verification during the host-cutover traffic ramps. + +## Startup behavior + +The app import is slow (~230s cold: `policyengine_api/country.py` instantiates five +country packages, ~210s of it) and Cloud Run hard-caps total startup-probe time at 240s — +the default TCP probe already sits at that cap, and **no probe configuration can extend +it**. Two mitigations are therefore built in: + +- `gcp/cloud_run/start.sh` runs **gunicorn with `uvicorn.workers.UvicornWorker` and no + `--preload`**: the gunicorn master binds the listen socket in milliseconds (satisfying + the startup probe immediately), and the import happens in the worker after fork. + `--timeout 0` is required — a worker mid-import does not heartbeat, and gunicorn's + default 30s watchdog would kill it. `--keep-alive 5` and `--forwarded-allow-ips '*'` + preserve the previous uvicorn keep-alive and proxy-header behavior. +- Candidate deploys set `--cpu-boost` to shorten the import. + +Consequences to keep in mind: + +- An instance is "started" (port bound) before the app can answer. Requests arriving in + that window queue until the worker is ready or Cloud Run's request timeout fires. This + is acceptable while the services carry no public traffic; the min-instances decision in + the cutover plan's Stage 3 keeps cold imports off the request path before traffic ramps. +- **Cold-start and readiness measurements must be taken from `/readiness-check` turning + healthy**, not from instance start or port bind — the bind time is no longer meaningful. +- Readiness in CI is governed by `.github/scripts/health_check.sh` polling + `/readiness-check` (defaults: 900s budget, 5s interval, 15s per-request `--max-time`; + all env-overridable). + +## IAM and bootstrap constraints + +- The GitHub deploy service account holds `roles/run.developer`: it can deploy to + existing services but cannot create the `allUsers` → `roles/run.invoker` binding a new + service needs, and `deploy_cloud_run_candidate.sh` always deploys `--no-traffic`, which + gcloud rejects on service creation. **New Cloud Run services are therefore bootstrapped + manually by a project owner** (placeholder image + IAM binding + runtime service + account); the first CI deploy replaces the revision entirely. See + `history/pr4-stage1-staging-service-runbook.md` for the pattern. +- Both services currently run as the dedicated runtime service account and share the + prod-named Secret Manager secrets and the production Cloud SQL instance. Known + follow-up: per-service secrets became possible once the services split; migrate in a + later stage. + +## Verification commands + +```bash +# Service state, runtime SA, traffic tags +gcloud run services describe policyengine-api \ + --project policyengine-api --region us-central1 \ + --format 'value(status.url, spec.template.spec.serviceAccountName, status.traffic)' + +# IAM (expect allUsers -> roles/run.invoker) +gcloud run services get-iam-policy policyengine-api-staging \ + --project policyengine-api --region us-central1 + +# Readiness and backend identification +curl -s -o /dev/null -w '%{http_code}\n' "$SERVICE_URL/readiness-check" +curl -sI "$SERVICE_URL/readiness-check" | grep -i x-policyengine-backend +``` + +## History index + +- `history/migration-pr1-baseline-runbook.md` — PR 1 baseline capture +- `history/migration-pr2-fastapi-shell-runbook.md` — PR 2 FastAPI/ASGI shell +- `history/migration-pr3-cloud-run-candidate-runbook.md` — PR 3 candidate/promote + pipeline (documents the pre-Stage-1 single-service behavior) +- `history/pr4-stage1-staging-service-runbook.md` — PR 4 Stage 1 staging service split, + startup de-flake, and exit gates diff --git a/docs/migration-pr1-baseline-runbook.md b/docs/migration/history/migration-pr1-baseline-runbook.md similarity index 91% rename from docs/migration-pr1-baseline-runbook.md rename to docs/migration/history/migration-pr1-baseline-runbook.md index d9255a221..11dfa6674 100644 --- a/docs/migration-pr1-baseline-runbook.md +++ b/docs/migration/history/migration-pr1-baseline-runbook.md @@ -1,5 +1,8 @@ # PR 1 Migration Baseline Runbook +> Historical record — describes the pipeline as it was when this PR landed. +> Current behavior: see [`../cloud-run-operations.md`](../cloud-run-operations.md). + PR 1 does not shift traffic or change API behavior. Before PR 2 starts, capture baseline production or staging metrics so later cutovers can compare against the same surface. diff --git a/docs/migration-pr2-fastapi-shell-runbook.md b/docs/migration/history/migration-pr2-fastapi-shell-runbook.md similarity index 86% rename from docs/migration-pr2-fastapi-shell-runbook.md rename to docs/migration/history/migration-pr2-fastapi-shell-runbook.md index 8a5a8d6ed..18ed2979f 100644 --- a/docs/migration-pr2-fastapi-shell-runbook.md +++ b/docs/migration/history/migration-pr2-fastapi-shell-runbook.md @@ -1,5 +1,8 @@ # PR 2 FastAPI Shell Runbook +> Historical record — describes the pipeline as it was when this PR landed. +> Current behavior: see [`../cloud-run-operations.md`](../cloud-run-operations.md). + PR 2 adds an ASGI FastAPI shell around the existing Flask API. It is a compatibility step only. diff --git a/docs/migration-pr3-cloud-run-candidate-runbook.md b/docs/migration/history/migration-pr3-cloud-run-candidate-runbook.md similarity index 97% rename from docs/migration-pr3-cloud-run-candidate-runbook.md rename to docs/migration/history/migration-pr3-cloud-run-candidate-runbook.md index 9aac27f3c..5bc8f7472 100644 --- a/docs/migration-pr3-cloud-run-candidate-runbook.md +++ b/docs/migration/history/migration-pr3-cloud-run-candidate-runbook.md @@ -1,5 +1,8 @@ # PR 3 Cloud Run Candidate Runbook +> Historical record — describes the pipeline as it was when this PR landed. +> Current behavior: see [`../cloud-run-operations.md`](../cloud-run-operations.md). + PR 3 adds a production-configured Cloud Run candidate for the FastAPI ASGI shell. It makes the Cloud Run service URL live after staged validation, but it does not migrate the public App Engine/custom API URL. diff --git a/docs/migration/history/pr4-stage1-staging-service-runbook.md b/docs/migration/history/pr4-stage1-staging-service-runbook.md new file mode 100644 index 000000000..1bd35e575 --- /dev/null +++ b/docs/migration/history/pr4-stage1-staging-service-runbook.md @@ -0,0 +1,85 @@ +# PR 4 Stage 1 Runbook: Staging Service Split + Startup De-flake + +> Per-stage record. Current behavior: +> see [`../cloud-run-operations.md`](../cloud-run-operations.md). + +Stage 1 of the public host cutover splits staging Cloud Run deploys onto a dedicated +`policyengine-api-staging` service. Previously, both the staging and production CI tracks +targeted the single `policyengine-api` service, and the staging promote step put a +staging-configured revision on 100% of the production service URL during every push. That +is harmless while nothing public routes to the service URL, and unacceptable once a load +balancer fronts it. + +Stage 1 also de-flakes candidate startup (plan items 3–5): `--cpu-boost` on candidate +deploys, a gunicorn entrypoint that binds the port before the multi-minute app import +(Cloud Run hard-caps startup-probe time at 240s, which the import raced and lost on +roughly half of first attempts), and `--max-time`/interval tuning on `health_check.sh`. +Mechanics and rationale live in the operations doc; the evidence (failing runs, revision +logs, the 240s platform cap) is recorded in the Stage 1 section of +`api-v1-pr4-cloud-run-host-cutover-execution-plan.md` in the planning folder. + +## One-Time Bootstrap (completed 2026-07-02) + +The CI pipeline cannot create this service itself, for two reasons: + +1. `deploy_cloud_run_candidate.sh` always deploys with `--no-traffic` (the PR 3 + candidate-then-promote pattern). gcloud rejects `--no-traffic` when creating a new + service, because a new service must route 100% of traffic to its first revision. +2. The GitHub deploy service account holds `roles/run.developer`, which can create and + deploy services but cannot set IAM policy. The `allUsers` -> `roles/run.invoker` + binding that `--allow-unauthenticated` needs on a new service must be created by a + project owner or `roles/run.admin` principal. (The production service's binding was + created manually during PR 3.) + +Bootstrap command, run once by a project owner before merging the Stage 1 PR: + +```bash +gcloud run deploy policyengine-api-staging \ + --project policyengine-api \ + --region us-central1 \ + --image us-docker.pkg.dev/cloudrun/container/hello \ + --allow-unauthenticated \ + --service-account policyengine-api-cr-runtime@policyengine-api.iam.gserviceaccount.com +``` + +The placeholder image is intentional: the first CI staging deploy replaces the revision +entirely, since `deploy_cloud_run_candidate.sh` sets every flag (image, env vars, secrets, +resources, service account, Cloud SQL attachment) explicitly on each deploy. + +## Verification + +```bash +gcloud run services describe policyengine-api-staging \ + --project policyengine-api --region us-central1 \ + --format "value(status.url, spec.template.spec.serviceAccountName)" +gcloud run services get-iam-policy policyengine-api-staging \ + --project policyengine-api --region us-central1 +curl -s -o /dev/null -w '%{http_code}\n' +``` + +Expected: service URL resolves, runtime service account is +`policyengine-api-cr-runtime@policyengine-api.iam.gserviceaccount.com`, IAM policy contains +`allUsers` with `roles/run.invoker`, and the curl returns 200. + +## Steady State After Stage 1 + +Described in [`../cloud-run-operations.md`](../cloud-run-operations.md) (topology, +startup behavior, IAM/secrets follow-ups) — that document, not this record, tracks +current reality. + +## Exit Gates + +Evaluated on the first post-merge push cycle (source of truth: the Stage 1 section of +`api-v1-pr4-cloud-run-host-cutover-execution-plan.md` in the planning folder): + +- `gcloud run services describe policyengine-api --region us-central1` shows only + `stage3-prod-*` tags at 100% at all times, including after the staging promote step. +- `policyengine-api-staging` is healthy and passes the staging integration suite via its + own URL. +- `X-PolicyEngine-Backend` is present and correct on the App Engine production URL, the + Cloud Run staging service URL, and the Cloud Run production service URL. +- New revisions show startup CPU boost in `gcloud run services describe`; every Cloud Run + deploy job (staging + prod candidate) green on first attempt. +- Revision logs show the port bound within ~15s of instance start and `/readiness-check` + healthy within the `health_check.sh` budget; the container-start → ready duration is + recorded as an input to Stage 2's timing qualification. diff --git a/gcp/cloud_run/start.sh b/gcp/cloud_run/start.sh index 35a27a455..31e2b188b 100755 --- a/gcp/cloud_run/start.sh +++ b/gcp/cloud_run/start.sh @@ -10,21 +10,21 @@ REDIS_READY_MAX_ATTEMPTS="${REDIS_READY_MAX_ATTEMPTS:-30}" export CACHE_REDIS_HOST CACHE_REDIS_PORT CACHE_REDIS_DB redis_pid="" -uvicorn_pid="" +server_pid="" shutdown() { trap - INT TERM - if [ -n "$uvicorn_pid" ] && kill -0 "$uvicorn_pid" 2>/dev/null; then - kill "$uvicorn_pid" 2>/dev/null || true + if [ -n "$server_pid" ] && kill -0 "$server_pid" 2>/dev/null; then + kill "$server_pid" 2>/dev/null || true fi if [ -n "$redis_pid" ] && kill -0 "$redis_pid" 2>/dev/null; then kill "$redis_pid" 2>/dev/null || true fi - if [ -n "$uvicorn_pid" ]; then - wait "$uvicorn_pid" 2>/dev/null || true + if [ -n "$server_pid" ]; then + wait "$server_pid" 2>/dev/null || true fi if [ -n "$redis_pid" ]; then @@ -58,23 +58,29 @@ until redis-cli -h "$CACHE_REDIS_HOST" -p "$CACHE_REDIS_PORT" ping >/dev/null 2> sleep 1 done -uvicorn policyengine_api.asgi:app \ - --host 0.0.0.0 \ - --port "$PORT" \ +# gunicorn's master binds the listen socket before forking workers, so the +# Cloud Run TCP startup probe passes immediately instead of racing the +# multi-minute app import (which happens in the worker, post-fork, because +# --preload is NOT set). --timeout 0 is required: a worker mid-import does +# not heartbeat, and the default 30s watchdog would kill it before boot. +gunicorn policyengine_api.asgi:app \ + --worker-class uvicorn.workers.UvicornWorker \ --workers "$WEB_CONCURRENCY" \ - --proxy-headers \ + --bind "0.0.0.0:${PORT}" \ + --timeout 0 \ + --keep-alive 5 \ --forwarded-allow-ips '*' & -uvicorn_pid="$!" +server_pid="$!" set +e -wait -n "$redis_pid" "$uvicorn_pid" +wait -n "$redis_pid" "$server_pid" status="$?" set -e if ! kill -0 "$redis_pid" 2>/dev/null; then echo "Redis exited; stopping Cloud Run container" >&2 -elif ! kill -0 "$uvicorn_pid" 2>/dev/null; then - echo "Uvicorn exited; stopping Cloud Run container" >&2 +elif ! kill -0 "$server_pid" 2>/dev/null; then + echo "API server exited; stopping Cloud Run container" >&2 else echo "A supervised Cloud Run process exited; stopping container" >&2 fi diff --git a/policyengine_api/asgi_factory.py b/policyengine_api/asgi_factory.py index 2456ccccd..e0fd10036 100644 --- a/policyengine_api/asgi_factory.py +++ b/policyengine_api/asgi_factory.py @@ -9,6 +9,10 @@ from a2wsgi import WSGIMiddleware from fastapi import FastAPI, HTTPException from policyengine_api.constants import VERSION +from policyengine_api.migration_flags import ( + BACKEND_RESPONSE_HEADER, + get_api_host_backend, +) from policyengine_api.migration_logging import log_migration_request from pydantic import BaseModel from starlette.middleware.gzip import GZipMiddleware @@ -56,6 +60,8 @@ async def add_cors_for_native_routes(request, call_next): started_at = time.time() request_id = request.headers.get("X-Request-ID") or uuid.uuid4().hex response = await call_next(request) + if BACKEND_RESPONSE_HEADER not in response.headers: + response.headers[BACKEND_RESPONSE_HEADER] = get_api_host_backend() origin = request.headers.get("origin") if origin and "access-control-allow-origin" not in response.headers: response.headers["Access-Control-Allow-Origin"] = origin diff --git a/policyengine_api/migration_flags.py b/policyengine_api/migration_flags.py index 0d659eb36..c270fa0e8 100644 --- a/policyengine_api/migration_flags.py +++ b/policyengine_api/migration_flags.py @@ -30,6 +30,8 @@ DEFAULT_SIM_FRONT_DOOR = "old_gateway_direct" DEFAULT_SIM_COMPUTE_BACKEND = "old_gateway" +BACKEND_RESPONSE_HEADER = "X-PolicyEngine-Backend" + @dataclass(frozen=True) class MigrationContext: @@ -112,6 +114,18 @@ def get_db_read(entity: str) -> str: return _read_choice(env_name, DEFAULT_DB_SOURCE, DB_READ_SOURCES) +def get_api_host_backend() -> str: + """Backend value for response tagging; must never break a response.""" + try: + return _read_choice( + "API_HOST_BACKEND", + DEFAULT_API_HOST_BACKEND, + API_HOST_BACKENDS, + ) + except ValueError: + return DEFAULT_API_HOST_BACKEND + + def get_sim_compute(flow: str) -> str: env_name = f"SIM_COMPUTE_{flow.upper()}" return _read_choice( diff --git a/policyengine_api/migration_logging.py b/policyengine_api/migration_logging.py index 5230502d8..aa3c575aa 100644 --- a/policyengine_api/migration_logging.py +++ b/policyengine_api/migration_logging.py @@ -9,6 +9,8 @@ from policyengine_api.gcp_logging import logger from policyengine_api.migration_flags import ( + BACKEND_RESPONSE_HEADER, + get_api_host_backend, get_migration_log_context, infer_route_group, ) @@ -26,6 +28,7 @@ def set_request_migration_context(): @app.after_request def log_request_migration_context(response): + response.headers[BACKEND_RESPONSE_HEADER] = get_api_host_backend() try: log_migration_request( request_id=getattr(flask.g, "request_id", None), diff --git a/tests/unit/routes/test_migration_context_logging.py b/tests/unit/routes/test_migration_context_logging.py index 29f32a72d..9e3b9939b 100644 --- a/tests/unit/routes/test_migration_context_logging.py +++ b/tests/unit/routes/test_migration_context_logging.py @@ -4,6 +4,7 @@ from flask import Flask, Response from policyengine_api.asgi_factory import create_asgi_app +from policyengine_api.migration_flags import BACKEND_RESPONSE_HEADER from policyengine_api.migration_logging import register_migration_request_logging @@ -108,3 +109,49 @@ def test_asgi_shell_does_not_log_unregistered_flask_fallback_routes(): assert response.status_code == 200 assert response.content == b"fallback" mock_logger.log_struct.assert_not_called() + + +def test_flask_responses_include_backend_header_default(): + with patch("policyengine_api.migration_logging.logger"): + response = _app().test_client().get("/readiness-check") + + assert response.status_code == 200 + assert response.headers[BACKEND_RESPONSE_HEADER] == "app_engine" + + +def test_flask_responses_include_backend_header_for_cloud_run(monkeypatch): + monkeypatch.setenv("API_HOST_BACKEND", "cloud_run") + + with patch("policyengine_api.migration_logging.logger"): + response = _app().test_client().get("/readiness-check") + + assert response.headers[BACKEND_RESPONSE_HEADER] == "cloud_run" + + +def test_backend_header_falls_back_to_default_on_invalid_flag(monkeypatch): + monkeypatch.setenv("API_HOST_BACKEND", "not-a-backend") + + with patch("policyengine_api.migration_logging.logger"): + response = _app().test_client().get("/readiness-check") + + assert response.status_code == 200 + assert response.headers[BACKEND_RESPONSE_HEADER] == "app_engine" + + +def test_fastapi_native_routes_include_backend_header(monkeypatch): + monkeypatch.setenv("API_HOST_BACKEND", "cloud_run") + + with patch("policyengine_api.migration_logging.logger"): + response = TestClient(create_asgi_app(_app())).get("/health") + + assert response.status_code == 200 + assert response.headers[BACKEND_RESPONSE_HEADER] == "cloud_run" + + +def test_asgi_shell_adds_backend_header_when_flask_hook_is_absent(): + response = TestClient(create_asgi_app(_app_without_migration_logging())).get( + "/fallback" + ) + + assert response.status_code == 200 + assert response.headers[BACKEND_RESPONSE_HEADER] == "app_engine" diff --git a/tests/unit/test_cloud_run_deploy_scripts.py b/tests/unit/test_cloud_run_deploy_scripts.py index 73f41f81f..2fe65f318 100644 --- a/tests/unit/test_cloud_run_deploy_scripts.py +++ b/tests/unit/test_cloud_run_deploy_scripts.py @@ -9,6 +9,14 @@ REPO = Path(__file__).resolve().parents[2] PRODUCTION_CLOUD_SQL_INSTANCE = "policyengine-api:us-central1:policyengine-api-data" +PRODUCTION_CLOUD_RUN_SERVICE = "policyengine-api" +STAGING_CLOUD_RUN_SERVICE = "policyengine-api-staging" +CLOUD_RUN_SERVICE_SCRIPTS = ( + "scripts/deploy_cloud_run_candidate.sh", + "scripts/promote_cloud_run_tag.sh", + "scripts/get_cloud_run_tag_url.sh", + "scripts/get_cloud_run_service_url.sh", +) DEDICATED_CLOUD_RUN_RUNTIME_SERVICE_ACCOUNT = ( "policyengine-api-cr-runtime@policyengine-api.iam.gserviceaccount.com" ) @@ -262,20 +270,20 @@ def test_cloud_run_dockerfile_runs_startup_with_bash(): assert 'CMD ["/bin/sh", "/app/start.sh"]' not in dockerfile -def test_cloud_run_startup_supervises_redis_and_uvicorn_children(): +def test_cloud_run_startup_supervises_redis_and_server_children(): start_script = (REPO / "gcp/cloud_run/start.sh").read_text(encoding="utf-8") assert "#!/usr/bin/env bash" in start_script assert 'redis_pid="$!"' in start_script - assert 'uvicorn_pid="$!"' in start_script + assert 'server_pid="$!"' in start_script assert "REDIS_READY_MAX_ATTEMPTS" in start_script assert "Redis exited before becoming ready" in start_script assert "Redis did not become ready" in start_script assert "Redis exited; stopping Cloud Run container" in start_script - assert "Uvicorn exited; stopping Cloud Run container" in start_script - assert 'wait -n "$redis_pid" "$uvicorn_pid"' in start_script + assert "API server exited; stopping Cloud Run container" in start_script + assert 'wait -n "$redis_pid" "$server_pid"' in start_script assert 'kill -0 "$redis_pid"' in start_script - assert 'kill -0 "$uvicorn_pid"' in start_script + assert 'kill -0 "$server_pid"' in start_script assert "trap 'shutdown; exit 143' INT TERM" in start_script assert "pkill" not in start_script assert re.search(r"(?m)^ *wait 2>/dev/null", start_script) is None @@ -384,6 +392,103 @@ def test_promote_cloud_run_tag_dry_run_shifts_service_traffic_to_tag(): assert "--to-latest" not in result.stdout +def _workflow_job_names(workflow: str) -> list[str]: + return re.findall(r"^ ([a-zA-Z0-9_-]+):$", workflow, flags=re.MULTILINE) + + +def test_push_workflow_isolates_staging_cloud_run_service(): + workflow = _push_workflow() + staging_deploy = _workflow_job_block(workflow, "deploy-cloud-run-staging") + staging_promote = _workflow_job_block(workflow, "promote-cloud-run-staging") + production_deploy = _workflow_job_block(workflow, "deploy-cloud-run-candidate") + + staging_service_env = f"CLOUD_RUN_SERVICE: {STAGING_CLOUD_RUN_SERVICE}" + assert staging_service_env in staging_deploy + assert staging_service_env in staging_promote + assert STAGING_CLOUD_RUN_SERVICE not in production_deploy + assert f"CLOUD_RUN_SERVICE: {PRODUCTION_CLOUD_RUN_SERVICE}\n" in production_deploy + + +def test_every_cloud_run_job_pins_a_service(): + workflow = _push_workflow() + + for job_name in _workflow_job_names(workflow): + block = _workflow_job_block(workflow, job_name) + if not any(script in block for script in CLOUD_RUN_SERVICE_SCRIPTS): + continue + assert re.search(r"CLOUD_RUN_SERVICE: \S+", block), ( + f"Job {job_name} uses a service-targeting Cloud Run script without " + "pinning CLOUD_RUN_SERVICE; the cloud_run_env.sh default silently " + "targets production" + ) + + +def test_only_production_job_promotes_the_production_cloud_run_service(): + workflow = _push_workflow() + + for job_name in _workflow_job_names(workflow): + block = _workflow_job_block(workflow, job_name) + if "scripts/promote_cloud_run_tag.sh" not in block: + continue + if job_name == "deploy-cloud-run-candidate": + expected = f"CLOUD_RUN_SERVICE: {PRODUCTION_CLOUD_RUN_SERVICE}\n" + else: + expected = f"CLOUD_RUN_SERVICE: {STAGING_CLOUD_RUN_SERVICE}" + assert expected in block, ( + f"Job {job_name} promotes Cloud Run traffic without pinning the " + "expected service" + ) + + +def test_build_cloud_run_image_uri_is_independent_of_service_override(): + result = _run_script( + ".github/scripts/build_cloud_run_image.sh", + _script_env( + GITHUB_SHA="1234567890abcdef", + GITHUB_RUN_NUMBER="42", + CLOUD_RUN_SERVICE=STAGING_CLOUD_RUN_SERVICE, + ), + ) + + assert result.returncode == 0, result.stderr + assert ( + "us-central1-docker.pkg.dev/policyengine-api/policyengine-api/" + "policyengine-api:1234567890abcdef" + ) in result.stdout + assert f"{STAGING_CLOUD_RUN_SERVICE}:" not in result.stdout + + +def test_deploy_cloud_run_candidate_dry_run_targets_service_override(): + result = _run_script( + ".github/scripts/deploy_cloud_run_candidate.sh", + _script_env( + **_required_runtime_env(), + CLOUD_RUN_IMAGE_URI="us-central1-docker.pkg.dev/project/repo/api:sha", + CLOUD_RUN_TAG="stage3-test", + CLOUD_RUN_SERVICE=STAGING_CLOUD_RUN_SERVICE, + ), + ) + + assert result.returncode == 0, result.stderr + assert f"gcloud run deploy {STAGING_CLOUD_RUN_SERVICE}" in result.stdout + + +def test_promote_cloud_run_tag_dry_run_targets_service_override(): + result = _run_script( + ".github/scripts/promote_cloud_run_tag.sh", + _script_env( + CLOUD_RUN_TAG="stage3-test", + CLOUD_RUN_SERVICE=STAGING_CLOUD_RUN_SERVICE, + ), + ) + + assert result.returncode == 0, result.stderr + assert ( + f"gcloud run services update-traffic {STAGING_CLOUD_RUN_SERVICE}" + in result.stdout + ) + + def test_push_workflow_tests_app_engine_and_cloud_run_staging_tracks(): workflow = _push_workflow() app_engine_tests = _workflow_job_block(workflow, "integration-tests-staging")