Skip to content

Commit eb9b79f

Browse files
anth-volkclaude
andcommitted
Address Stage 1 review findings
Pin CLOUD_RUN_SERVICE explicitly on the production Cloud Run job and add workflow invariants: every service-targeting Cloud Run job must pin a service, and only the production job may promote the production service. The cloud_run_env.sh default silently targeting production was the mechanism behind the original staging-promotes-to-prod bug. Drop the dead try/except around the backend header assignment; get_api_host_backend() is fail-safe by design, so the except could only mask real header-assignment bugs, and the ASGI path never had one. Document the one-time staging service bootstrap (gcloud rejects --no-traffic when creating a new service, and the deploy service account cannot set the public invoker binding) in a Stage 1 runbook. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
1 parent 4a62334 commit eb9b79f

4 files changed

Lines changed: 125 additions & 4 deletions

File tree

.github/workflows/push.yml

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -475,6 +475,8 @@ jobs:
475475
(github.repository == 'PolicyEngine/policyengine-api')
476476
&& (github.event.head_commit.message == 'Update PolicyEngine API')
477477
environment: production
478+
env:
479+
CLOUD_RUN_SERVICE: policyengine-api
478480
permissions:
479481
contents: read
480482
id-token: write
Lines changed: 79 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,79 @@
1+
# PR 4 Stage 1 Staging Cloud Run Service Runbook
2+
3+
Stage 1 of the public host cutover splits staging Cloud Run deploys onto a dedicated
4+
`policyengine-api-staging` service. Previously, both the staging and production CI tracks
5+
targeted the single `policyengine-api` service, and the staging promote step put a
6+
staging-configured revision on 100% of the production service URL during every push. That
7+
is harmless while nothing public routes to the service URL, and unacceptable once a load
8+
balancer fronts it.
9+
10+
## One-Time Bootstrap (completed 2026-07-02)
11+
12+
The CI pipeline cannot create this service itself, for two reasons:
13+
14+
1. `deploy_cloud_run_candidate.sh` always deploys with `--no-traffic` (the PR 3
15+
candidate-then-promote pattern). gcloud rejects `--no-traffic` when creating a new
16+
service, because a new service must route 100% of traffic to its first revision.
17+
2. The GitHub deploy service account holds `roles/run.developer`, which can create and
18+
deploy services but cannot set IAM policy. The `allUsers` -> `roles/run.invoker`
19+
binding that `--allow-unauthenticated` needs on a new service must be created by a
20+
project owner or `roles/run.admin` principal. (The production service's binding was
21+
created manually during PR 3.)
22+
23+
Bootstrap command, run once by a project owner before merging the Stage 1 PR:
24+
25+
```bash
26+
gcloud run deploy policyengine-api-staging \
27+
--project policyengine-api \
28+
--region us-central1 \
29+
--image us-docker.pkg.dev/cloudrun/container/hello \
30+
--allow-unauthenticated \
31+
--service-account policyengine-api-cr-runtime@policyengine-api.iam.gserviceaccount.com
32+
```
33+
34+
The placeholder image is intentional: the first CI staging deploy replaces the revision
35+
entirely, since `deploy_cloud_run_candidate.sh` sets every flag (image, env vars, secrets,
36+
resources, service account, Cloud SQL attachment) explicitly on each deploy.
37+
38+
## Verification
39+
40+
```bash
41+
gcloud run services describe policyengine-api-staging \
42+
--project policyengine-api --region us-central1 \
43+
--format "value(status.url, spec.template.spec.serviceAccountName)"
44+
gcloud run services get-iam-policy policyengine-api-staging \
45+
--project policyengine-api --region us-central1
46+
curl -s -o /dev/null -w '%{http_code}\n' <service URL>
47+
```
48+
49+
Expected: service URL resolves, runtime service account is
50+
`policyengine-api-cr-runtime@policyengine-api.iam.gserviceaccount.com`, IAM policy contains
51+
`allUsers` with `roles/run.invoker`, and the curl returns 200.
52+
53+
## Steady State After Stage 1
54+
55+
- Staging jobs (`deploy-cloud-run-staging`, `promote-cloud-run-staging`) pin
56+
`CLOUD_RUN_SERVICE: policyengine-api-staging`; the production job
57+
(`deploy-cloud-run-candidate`) pins `CLOUD_RUN_SERVICE: policyengine-api`. Unit tests
58+
enforce that every service-targeting Cloud Run job carries an explicit pin.
59+
- The Docker image name is decoupled from the service name (`CLOUD_RUN_IMAGE_NAME`):
60+
production reuses the image built by the staging track, so both tracks push and pull
61+
`policyengine-api:<sha>` regardless of service.
62+
- Every API response carries `X-PolicyEngine-Backend` (`app_engine` or `cloud_run`) for
63+
in-band backend verification during the host cutover traffic ramps.
64+
- Known follow-up (out of Stage 1 scope): the staging service still uses the prod-named
65+
Secret Manager secrets and the production Cloud SQL instance with staging DB user/name
66+
values, as the shared-service track always did. The service split makes a per-service
67+
secret set possible; migrate in a later stage.
68+
69+
## Exit Gates
70+
71+
Evaluated on the first post-merge push cycle (see the Stage 1 section of
72+
`api-v1-pr4-cloud-run-host-cutover-execution-plan.md` in the planning folder):
73+
74+
- `gcloud run services describe policyengine-api --region us-central1` shows only
75+
`stage3-prod-*` tags at 100% at all times, including after the staging promote step.
76+
- `policyengine-api-staging` is healthy and passes the staging integration suite via its
77+
own URL.
78+
- `X-PolicyEngine-Backend` is present and correct on the App Engine production URL, the
79+
Cloud Run staging service URL, and the Cloud Run production service URL.

policyengine_api/migration_logging.py

Lines changed: 1 addition & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -28,10 +28,7 @@ def set_request_migration_context():
2828

2929
@app.after_request
3030
def log_request_migration_context(response):
31-
try:
32-
response.headers[BACKEND_RESPONSE_HEADER] = get_api_host_backend()
33-
except Exception:
34-
pass
31+
response.headers[BACKEND_RESPONSE_HEADER] = get_api_host_backend()
3532
try:
3633
log_migration_request(
3734
request_id=getattr(flask.g, "request_id", None),

tests/unit/test_cloud_run_deploy_scripts.py

Lines changed: 43 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,14 @@
99

1010
REPO = Path(__file__).resolve().parents[2]
1111
PRODUCTION_CLOUD_SQL_INSTANCE = "policyengine-api:us-central1:policyengine-api-data"
12+
PRODUCTION_CLOUD_RUN_SERVICE = "policyengine-api"
1213
STAGING_CLOUD_RUN_SERVICE = "policyengine-api-staging"
14+
CLOUD_RUN_SERVICE_SCRIPTS = (
15+
"scripts/deploy_cloud_run_candidate.sh",
16+
"scripts/promote_cloud_run_tag.sh",
17+
"scripts/get_cloud_run_tag_url.sh",
18+
"scripts/get_cloud_run_service_url.sh",
19+
)
1320
DEDICATED_CLOUD_RUN_RUNTIME_SERVICE_ACCOUNT = (
1421
"policyengine-api-cr-runtime@policyengine-api.iam.gserviceaccount.com"
1522
)
@@ -385,6 +392,10 @@ def test_promote_cloud_run_tag_dry_run_shifts_service_traffic_to_tag():
385392
assert "--to-latest" not in result.stdout
386393

387394

395+
def _workflow_job_names(workflow: str) -> list[str]:
396+
return re.findall(r"^ ([a-zA-Z0-9_-]+):$", workflow, flags=re.MULTILINE)
397+
398+
388399
def test_push_workflow_isolates_staging_cloud_run_service():
389400
workflow = _push_workflow()
390401
staging_deploy = _workflow_job_block(workflow, "deploy-cloud-run-staging")
@@ -395,6 +406,38 @@ def test_push_workflow_isolates_staging_cloud_run_service():
395406
assert staging_service_env in staging_deploy
396407
assert staging_service_env in staging_promote
397408
assert STAGING_CLOUD_RUN_SERVICE not in production_deploy
409+
assert f"CLOUD_RUN_SERVICE: {PRODUCTION_CLOUD_RUN_SERVICE}\n" in production_deploy
410+
411+
412+
def test_every_cloud_run_job_pins_a_service():
413+
workflow = _push_workflow()
414+
415+
for job_name in _workflow_job_names(workflow):
416+
block = _workflow_job_block(workflow, job_name)
417+
if not any(script in block for script in CLOUD_RUN_SERVICE_SCRIPTS):
418+
continue
419+
assert re.search(r"CLOUD_RUN_SERVICE: \S+", block), (
420+
f"Job {job_name} uses a service-targeting Cloud Run script without "
421+
"pinning CLOUD_RUN_SERVICE; the cloud_run_env.sh default silently "
422+
"targets production"
423+
)
424+
425+
426+
def test_only_production_job_promotes_the_production_cloud_run_service():
427+
workflow = _push_workflow()
428+
429+
for job_name in _workflow_job_names(workflow):
430+
block = _workflow_job_block(workflow, job_name)
431+
if "scripts/promote_cloud_run_tag.sh" not in block:
432+
continue
433+
if job_name == "deploy-cloud-run-candidate":
434+
expected = f"CLOUD_RUN_SERVICE: {PRODUCTION_CLOUD_RUN_SERVICE}\n"
435+
else:
436+
expected = f"CLOUD_RUN_SERVICE: {STAGING_CLOUD_RUN_SERVICE}"
437+
assert expected in block, (
438+
f"Job {job_name} promotes Cloud Run traffic without pinning the "
439+
"expected service"
440+
)
398441

399442

400443
def test_build_cloud_run_image_uri_is_independent_of_service_override():

0 commit comments

Comments
 (0)