diff --git a/.github/workflows/cd.yml b/.github/workflows/cd.yml new file mode 100644 index 0000000..2b6ee06 --- /dev/null +++ b/.github/workflows/cd.yml @@ -0,0 +1,171 @@ +name: CD + +# Only after CI has passed on main. `workflow_run` is what makes "green CI" +# a precondition rather than a hope: a push that fails CI never reaches here. +on: + workflow_run: + workflows: [CI] + types: [completed] + branches: [main] + workflow_dispatch: + inputs: + sha: + description: The commit to deploy. Defaults to the head of main. + required: false + +# Federated credentials, so no Azure secret is stored in GitHub (ADR 0023). +permissions: + contents: read + id-token: write + +env: + IMAGE_NAME: policypal-api + +jobs: + build: + # A failed CI run must not deploy. workflow_dispatch has no parent run. + if: >- + github.event_name == 'workflow_dispatch' || + github.event.workflow_run.conclusion == 'success' + runs-on: ubuntu-latest + outputs: + digest: ${{ steps.push.outputs.digest }} + sha: ${{ steps.commit.outputs.sha }} + steps: + # `inputs.sha` is typed by a human and ends up in `ref:` and in an image + # tag, so it is checked to be a full commit id and nothing else. + - id: commit + env: + DISPATCH_SHA: ${{ inputs.sha }} + RUN_SHA: ${{ github.event.workflow_run.head_sha }} + run: | + set -euo pipefail + sha="${DISPATCH_SHA:-${RUN_SHA:-$GITHUB_SHA}}" + if ! printf '%s' "$sha" | grep -Eq '^[0-9a-f]{40}$'; then + echo "not a commit id: refusing to deploy"; exit 1 + fi + echo "sha=$sha" >> "$GITHUB_OUTPUT" + + - uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 + with: + ref: ${{ steps.commit.outputs.sha }} + + - uses: azure/login@a65d910e8af852a8061c627c456678983e180302 # v2.2.0 + with: + client-id: ${{ secrets.AZURE_CLIENT_ID }} + tenant-id: ${{ secrets.AZURE_TENANT_ID }} + subscription-id: ${{ secrets.AZURE_SUBSCRIPTION_ID }} + + - name: Sign in to the registry + env: + ACR_NAME: ${{ vars.ACR_NAME }} + run: az acr login --name "$ACR_NAME" + + - uses: docker/setup-buildx-action@e468171a9de216ec08956ac3ada2f0791b6bd435 # v3.11.1 + + # Tagged with the commit, and deployed by digest: a tag can be moved, + # a digest cannot, so what is approved is exactly what runs. + - id: push + uses: docker/build-push-action@263435318d21b8e681c14492fe198d362a7d2c83 # v6.18.0 + with: + context: . + push: true + tags: ${{ vars.ACR_NAME }}.azurecr.io/policypal-api:${{ steps.commit.outputs.sha }} + cache-from: type=gha + cache-to: type=gha,mode=max + + deploy: + needs: build + runs-on: ubuntu-latest + # Production is a GitHub Environment with a required reviewer: this job + # waits for a human before anything reaches Azure. + environment: + name: production + url: ${{ vars.API_URL }} + steps: + - uses: azure/login@a65d910e8af852a8061c627c456678983e180302 # v2.2.0 + with: + client-id: ${{ secrets.AZURE_CLIENT_ID }} + tenant-id: ${{ secrets.AZURE_TENANT_ID }} + subscription-id: ${{ secrets.AZURE_SUBSCRIPTION_ID }} + + # Migrations run inside Azure, as a Container Apps job on the image being + # deployed, so the database needs no public firewall opening for a + # GitHub runner and the schema is migrated by exactly the code that will + # then serve it. + - name: Migrate the database + env: + RG: ${{ vars.AZURE_RESOURCE_GROUP }} + JOB: ${{ vars.MIGRATION_JOB_NAME }} + IMAGE: ${{ vars.ACR_NAME }}.azurecr.io/policypal-api@${{ needs.build.outputs.digest }} + run: | + set -euo pipefail + az containerapp job update --name "$JOB" --resource-group "$RG" --image "$IMAGE" + execution=$(az containerapp job start --name "$JOB" --resource-group "$RG" \ + --query name -o tsv) + echo "Waiting on $execution" + for _ in $(seq 1 60); do + status=$(az containerapp job execution show --name "$JOB" --resource-group "$RG" \ + --job-execution-name "$execution" --query properties.status -o tsv) + case "$status" in + Succeeded) echo "migrations applied"; exit 0 ;; + Failed|Degraded) echo "migration job $status"; exit 1 ;; + esac + sleep 10 + done + echo "migration job did not finish in 10 minutes"; exit 1 + + - name: Deploy the API + env: + RG: ${{ vars.AZURE_RESOURCE_GROUP }} + APP: ${{ vars.CONTAINERAPP_NAME }} + IMAGE: ${{ vars.ACR_NAME }}.azurecr.io/policypal-api@${{ needs.build.outputs.digest }} + run: | + set -euo pipefail + az containerapp update --name "$APP" --resource-group "$RG" --image "$IMAGE" + az containerapp ingress show --name "$APP" --resource-group "$RG" -o none + + - name: Check it answers + env: + API_URL: ${{ vars.API_URL }} + run: | + set -euo pipefail + for _ in $(seq 1 30); do + code=$(curl -s -o /dev/null -w '%{http_code}' "$API_URL/api/counties?zip=33101" || true) + if [ "$code" = "200" ]; then echo "API answering"; exit 0; fi + sleep 10 + done + echo "API did not answer 200 within five minutes"; exit 1 + + frontend: + needs: [build, deploy] + runs-on: ubuntu-latest + environment: + name: production + url: ${{ vars.FRONTEND_URL }} + steps: + - uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 + with: + ref: ${{ needs.build.outputs.sha }} + + - uses: actions/setup-node@820762786026740c76f36085b0efc47a31fe5020 # v7.0.0 + with: + node-version: "24" + cache: npm + cache-dependency-path: frontend/package-lock.json + + - name: Build the browser app + working-directory: frontend + env: + # Baked in at build time: Vite has no runtime configuration. + VITE_API_BASE_URL: ${{ vars.API_URL }} + run: | + npm ci + npm run build + + - uses: Azure/static-web-apps-deploy@1a947af9992250f3bc2e68ad0754c0b0c11566c9 # v1.0.0 + with: + azure_static_web_apps_api_token: ${{ secrets.AZURE_STATIC_WEB_APPS_API_TOKEN }} + action: upload + app_location: frontend/dist + skip_app_build: true diff --git a/CHANGELOG.md b/CHANGELOG.md index 7fafe6a..ad001be 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -12,6 +12,15 @@ `/app/data` is not empty: the issuer PDFs are local-only (ADR 0016) and a `.dockerignore` regression would otherwise ship gigabytes of them unnoticed. Trivy fails the build on fixable HIGH or CRITICAL findings. +- CD to Azure (ADR 0023), closing the second of ADR 0002's deferred items: + `.github/workflows/cd.yml` builds and pushes the API image to ACR, runs + `alembic upgrade head` as a Container Apps job on that exact digest, updates + the Container App, checks it answers, and deploys the browser app to Static + Web Apps. It runs only after a green CI run on `main`, waits on a required + reviewer, deploys by digest rather than tag, and signs in with OIDC so no + Azure credential is stored in GitHub. Rollback is reactivating the previous + revision. `docs/runbooks/deploy.md` has the one-time setup and what to do + when a deploy goes wrong. - `.env.example`, which did not exist: the three settings with no default (`DATABASE_URL`, `OPENAI_API_KEY`, `JWT_SECRET_KEY`) and the handful worth setting, with no values in it. diff --git a/README.md b/README.md index 6136d63..8b05a5d 100644 --- a/README.md +++ b/README.md @@ -352,8 +352,23 @@ npm run build CI (`.github/workflows/ci.yml`) also runs `ruff`, `alembic check` (schema drift), `bandit` (SAST), `pip-audit`/`npm audit` (dependency CVEs), a -gitleaks secret scan, and a backend coverage floor (`--cov-fail-under=85`) -on every PR and push to `main`. +gitleaks secret scan, a backend coverage floor (`--cov-fail-under=85`), and a +Docker build scanned by Trivy — which also runs the image and fails if any +issuer PDF reached it. Every pull request, and every push to `main`. + +### Deploying + +`.github/workflows/cd.yml` deploys to Azure after a green CI run on `main`: +the API to Container Apps, the browser app to Static Web Apps, against a +PostgreSQL Flexible Server. Migrations run as a Container Apps job on the +exact image digest being deployed, so the database needs no public opening. +It waits on a required reviewer, and rollback is reactivating the previous +revision. Sign-in is OIDC, so no Azure credential is stored in GitHub. + +Setup, the deploy path, and what to do when one goes wrong are in +[`docs/runbooks/deploy.md`](docs/runbooks/deploy.md); the reasoning is in +[ADR 0023](docs/decisions/0023-deploying-to-azure.md). Ingestion stays on the +machine that holds `data/` — the issuer PDFs are never uploaded (ADR 0016). ### Retrieval quality eval diff --git a/docs/decisions/0002-ci-scope.md b/docs/decisions/0002-ci-scope.md index 8572e1e..65998bd 100644 --- a/docs/decisions/0002-ci-scope.md +++ b/docs/decisions/0002-ci-scope.md @@ -2,7 +2,9 @@ ## Status -Accepted +Accepted. All three deferrals below have since been closed: coverage +thresholds in Phase 3, the Docker build and image scan by ADR 0022, and CD to +Azure by ADR 0023. ## Context @@ -26,19 +28,18 @@ and shouldn't fabricate. `npm audit --audit-level=high`. - **secret-scan**: `gitleaks detect` over full git history. -**Deliberately not built yet:** - -- **Docker build + image vulnerability scan.** There is no Dockerfile for - the app. Writing a production one for a Flask service that pulls in - PyTorch + transformers (a multi-GB dependency tree) is its own real task — - base image choice, layer caching, non-root user, multi-stage build to - keep the final image lean — not something to bolt on to get a CI checkbox - green. -- **CD to Azure (ACR push, migrations, deploy).** Requires an Azure - subscription, service principal, and GitHub Environment secrets that - don't exist in this project yet. Configuring cloud deployment - infrastructure isn't something to set up unprompted or with placeholder - credentials. +**Deferred when this was written; all three have since been built:** + +- ~~**Docker build + image vulnerability scan.**~~ **Resolved by ADR 0022.** + It was its own real task, as expected: CPU-only torch, both models baked in + at their pinned revisions, a non-root multi-stage build, and a CI step that + runs the image to prove no issuer PDF reached it. 2.48 GB, scanned by Trivy + on every pull request. +- ~~**CD to Azure (ACR push, migrations, deploy).**~~ **Resolved by ADR 0023**, + once a subscription existed. Container Apps, Static Web Apps and a Flexible + Server, deployed by digest behind a required reviewer, with migrations run + as a job inside the environment so the database needs no public opening. + Sign-in is OIDC, so no Azure credential is stored here after all. - ~~**Coverage thresholds.**~~ Resolved: `pytest --cov=src --cov-fail-under=85` is now the backend test step. It is a regression floor set just under the measured baseline, not an arbitrary target — infrastructure glue wrapping diff --git a/docs/decisions/0023-deploying-to-azure.md b/docs/decisions/0023-deploying-to-azure.md new file mode 100644 index 0000000..fb52481 --- /dev/null +++ b/docs/decisions/0023-deploying-to-azure.md @@ -0,0 +1,111 @@ +# 0023 — Deploying to Azure: Container Apps, Static Web Apps, Flexible Server + +## Status + +Accepted. Closes the second of ADR 0002's two deferred items ("CD to Azure +(ACR push, migrations, deploy)"). Depends on ADR 0022 for the image, and on +ADR 0021 without which a deployed container cannot answer. + +## Context + +ADR 0002 deferred CD because it "requires an Azure subscription, service +principal, and GitHub Environment secrets that don't exist in this project +yet". A subscription now exists. + +One constraint shapes everything: **the issuer PDFs stay on the machine that +downloaded them** (ADR 0016), and ADR 0019 already records that "nothing here +can run in CI or in the cloud". Ingestion, refresh and the reports are local +operations. What deploys is the API and the browser app; what travels is the +text already extracted, not the documents it came from. + +## Decision + +### Container Apps for the API + +- **Scale-to-zero** suits a project with no steady traffic, and the revision + model gives rollback for free. +- **Revisions are the rollback.** Reactivating the previous revision is one + command, which is what ADR 0002's "keep rollback a single re-run" asks for. +- **One replica, for now.** ADR 0022 explains why: Flask-Limiter has no shared + storage, so a second replica would double every rate limit. Raising the + replica count and adding a Redis backend for the limiter are the same piece + of work, and neither happens by accident. + +App Service was the alternative. It costs more at idle for the same thing. +AKS is far more operational surface than one Flask service justifies. + +### Static Web Apps for the browser app + +A Vite build is static files. Static Web Apps serves them on a CDN with SPA +fallback routing, on a free tier. The API stays a separate origin, which is +what `frontend_origin` and the single-origin CORS rule already assume. + +`VITE_API_BASE_URL` is baked in at build time, because Vite has no runtime +configuration. The deployed frontend is therefore tied to one API origin, and +changing it is a rebuild. + +### The database is seeded once, by hand, and never by CI + +Azure Database for PostgreSQL Flexible Server with `pgvector`. + +- **`pg_dump` → `pg_restore`, once.** 75 MB: the corpus, its 1,568 chunks, the + 31,359 SBC chunks, the plan catalog, and the BM25 index row (ADR 0021). +- **Not a migration and not a workflow step.** Seeding is an operator action + with the local database in front of them, because that is where the data + lives and where ingestion will keep running. +- **The PDFs are not uploaded.** Nothing in this decision moves + `data/sbc/` anywhere. + +### Migrations run in Azure, on the image being deployed + +`alembic upgrade head` runs as a **Container Apps job** using the same image +and digest that is about to serve traffic. + +The alternative — running Alembic from the GitHub runner — needs the database +to accept connections from GitHub's IP ranges. That is a public opening on the +database for the sake of a deploy step. Running it inside the environment +keeps the server private, and guarantees the schema is migrated by exactly the +code that will then use it. + +### Deployed by digest, gated by a human + +- **The image is pushed with the commit as its tag and deployed by digest.** + A tag can be moved; a digest cannot. What a reviewer approves is exactly + what runs. +- **`workflow_run` on a successful CI run is the trigger.** A push that fails + CI never reaches the deploy workflow — "green CI" is a precondition, not a + convention. +- **`environment: production` with a required reviewer** is the manual + approval. Nothing reaches Azure until a person says so. +- **A post-deploy check** requests `/api/counties?zip=` until it returns 200, + so a revision that starts but cannot serve fails the run rather than sitting + there. + +### No Azure credential is stored in GitHub + +`azure/login` with **OIDC federated credentials**: the workflow presents a +short-lived GitHub token, and Azure trusts it for this repository and +environment. There is no client secret to rotate or to leak. `id-token: +write` is granted at the workflow level and `contents: read` everywhere else. + +`workflow_dispatch` takes a commit to deploy, which a human types, so it is +checked against `^[0-9a-f]{40}$` before it reaches `ref:` or an image tag. + +## Consequences + +- **Cold starts are slow**, because scale-to-zero means loading two models + from the image on the first request after idling. The trade is cost; the + fix, if it stops being acceptable, is a minimum of one replica. +- **A deploy needs a human.** Deliberate. This application answers questions + about people's health coverage, and there is no staging environment to catch + a bad revision first. +- **There is no staging environment.** One environment, one reviewer. Adding + staging is worth doing before the first user who is not the author. +- **The deployed data is a snapshot.** Ingestion runs locally, so the cloud + database is as current as its last restore. The runbook says how to refresh + it, and `checked_at` in `sbc_documents` says how old it is. +- **Costs are real but small**: Container Apps scaled to zero, a free Static + Web App, and a Flexible Server that cannot scale to zero and is therefore + the monthly floor. +- **Still not done:** a shared rate-limit store, a staging environment, and + alerting. Named here rather than discovered later. diff --git a/docs/runbooks/deploy.md b/docs/runbooks/deploy.md new file mode 100644 index 0000000..0e50959 --- /dev/null +++ b/docs/runbooks/deploy.md @@ -0,0 +1,189 @@ +# Deploying PolicyPal to Azure + +What to create once, what runs on every merge, and what to do when a deploy +goes wrong. The decision behind it is [ADR 0023](../decisions/0023-deploying-to-azure.md). + +**The issuer PDFs never leave this machine** (ADR 0016). Nothing below uploads +`data/sbc/`, and ingestion stays local. + +## What gets deployed + +| Piece | Where | Built by | +| --- | --- | --- | +| API | Container Apps, one replica | `Dockerfile` (ADR 0022) | +| Browser app | Static Web Apps | `npm run build` in CI | +| Database | PostgreSQL Flexible Server + `pgvector` | seeded once, by hand | +| Migrations | Container Apps job, same image | `alembic upgrade head` | + +## One-time setup + +Names below are placeholders; use your own and set them as repository +variables. + +### 1. Resource group, registry, database + +```sh +az group create --name policypal-rg --location eastus + +az acr create --resource-group policypal-rg --name policypalacr --sku Basic + +az postgres flexible-server create \ + --resource-group policypal-rg --name policypal-db \ + --tier Burstable --sku-name Standard_B1ms --version 16 \ + --storage-size 32 --public-access None +az postgres flexible-server parameter set \ + --resource-group policypal-rg --server-name policypal-db \ + --name azure.extensions --value VECTOR +``` + +`--public-access None` keeps the server off the internet. Migrations run +inside the environment (ADR 0023), so nothing needs to reach it from outside. + +### 2. Container Apps environment, the app, and the migration job + +```sh +az containerapp env create --resource-group policypal-rg \ + --name policypal-env --location eastus + +az containerapp create --resource-group policypal-rg --name policypal-api \ + --environment policypal-env --target-port 8000 --ingress external \ + --min-replicas 0 --max-replicas 1 \ + --registry-server policypalacr.azurecr.io --system-assigned \ + --image policypalacr.azurecr.io/policypal-api:bootstrap + +az containerapp job create --resource-group policypal-rg \ + --name policypal-migrate --environment policypal-env \ + --trigger-type Manual --replica-timeout 600 \ + --registry-server policypalacr.azurecr.io --system-assigned \ + --image policypalacr.azurecr.io/policypal-api:bootstrap \ + --command "/app/.venv/bin/alembic" --args "upgrade" "head" +``` + +**`--max-replicas 1` is load-bearing.** Flask-Limiter keeps its counters in +process, so a second replica doubles every rate limit — including the 5/min on +register and 10/min on login (ADR 0022). Raise it only together with a shared +store for the limiter. + +### 3. Settings and secrets on the app + +```sh +az containerapp secret set --resource-group policypal-rg --name policypal-api \ + --secrets database-url="postgresql+psycopg://..." \ + openai-key="sk-..." jwt-secret="$(python -c 'import secrets;print(secrets.token_urlsafe(48))')" + +az containerapp update --resource-group policypal-rg --name policypal-api \ + --set-env-vars ENVIRONMENT=production LOG_DIR=/tmp/logs \ + FRONTEND_ORIGIN="https://.azurestaticapps.net" \ + DATABASE_URL=secretref:database-url \ + OPENAI_API_KEY=secretref:openai-key \ + JWT_SECRET_KEY=secretref:jwt-secret +``` + +The migration job needs `DATABASE_URL` and the two settings with no default +(`OPENAI_API_KEY`, `JWT_SECRET_KEY`) as well — `Settings` will not construct +without them, even though a migration uses neither. + +`.env.example` lists every setting. Never put one in the repo. + +### 4. Sign-in without a stored credential + +Create an app registration, give it `AcrPush` on the registry and +`Contributor` on the resource group, then add **federated credentials** for +this repository — one for `environment:production`, one for `ref:refs/heads/main`. +No client secret is created (ADR 0023). + +### 5. GitHub Environment and variables + +Create an Environment named `production` with **yourself as a required +reviewer** — that is the manual approval gate. + +| Secret | | +| --- | --- | +| `AZURE_CLIENT_ID`, `AZURE_TENANT_ID`, `AZURE_SUBSCRIPTION_ID` | from the app registration | +| `AZURE_STATIC_WEB_APPS_API_TOKEN` | from the Static Web App | + +| Variable | Example | +| --- | --- | +| `ACR_NAME` | `policypalacr` | +| `AZURE_RESOURCE_GROUP` | `policypal-rg` | +| `CONTAINERAPP_NAME` | `policypal-api` | +| `MIGRATION_JOB_NAME` | `policypal-migrate` | +| `API_URL` | `https://policypal-api..azurecontainerapps.io` | +| `FRONTEND_URL` | `https://.azurestaticapps.net` | + +### 6. Seed the database, once + +The schema comes from migrations; the contents come from here, because +ingestion runs here (ADR 0019). + +```sh +pg_dump --no-owner --no-privileges --format=custom \ + -h localhost -U policypal policypal > policypal.dump # ~75 MB + +pg_restore --no-owner --no-privileges --clean --if-exists \ + -d "postgresql://...@policypal-db.postgres.database.azure.com/policypal" policypal.dump +``` + +This carries the corpus, its chunks, the SBC text, the plan catalog and the +BM25 index row. It does **not** carry the PDFs, and must not. + +Afterwards, check the index arrived: + +```sql +select name, chunks, built_at from search_indexes; -- bm25 | 1568 | ... +``` + +If it is missing, run `make build-index` against the Azure database rather +than re-ingesting: the index is derived from the chunks (ADR 0021). + +## Every merge to main + +1. CI runs. A failed run never reaches CD. +2. **build** — the image is pushed to ACR, tagged with the commit. +3. **deploy** — waits for your approval, then runs migrations as a job on that + exact digest, updates the Container App, and polls + `/api/counties?zip=33101` until it returns 200. +4. **frontend** — builds with `VITE_API_BASE_URL` and uploads to Static Web Apps. + +Deploying a specific commit by hand: **Actions → CD → Run workflow**, with a +full 40-character commit id. + +## When it goes wrong + +**A bad revision is serving.** Roll back to the previous one: + +```sh +az containerapp revision list --resource-group policypal-rg --name policypal-api \ + --query "[].{name:name,active:properties.active,created:properties.createdTime}" -o table +az containerapp revision activate --resource-group policypal-rg \ + --name policypal-api --revision +``` + +**Migrations failed.** The deploy stops before the app is updated, so the old +revision keeps serving — against a database that may be half migrated. Read +the job's logs, fix forward if you can: + +```sh +az containerapp job execution list --resource-group policypal-rg \ + --name policypal-migrate -o table +``` + +Alembic downgrades exist but are a last resort: reverting a migration that has +already dropped something does not bring it back. + +**The API deployed but answers 500.** Almost always the database: either the +restore has not happened, or `search_indexes` is empty. Retrieval says which — +`MissingSearchIndexError` names `make build-index`. + +**Cold start feels broken.** The first request after idle loads two models from +the image. It is slow, not stuck. If that stops being acceptable, set +`--min-replicas 1` and pay for it. + +## What this does not do + +- **No staging environment.** One environment, one reviewer. +- **No alerting.** Failures are seen in the Actions run, not pushed anywhere. +- **No shared rate-limit store**, which is what pins the app at one replica. +- **No scheduled ingestion.** The deployed data is as current as the last + restore; `checked_at` in `sbc_documents` says how old that is, and + [sbc.md](sbc.md) is how it is refreshed — locally.