From 5c9b9305f0466f0a5951cbc83e558cf92b06b6f4 Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 9 Nov 2026 04:22:08 +0000 Subject: [PATCH] RV-S1: align Terraform, deployer and entrypoint on the prod database (#172) Terraform created te_prod for pgadmin; build/deploy_all.sh created te_mgmt_prod for postgres, hardcoded; infra/entrypoint.sh needed an env_prod.sql that a freshly built image does not contain, so a first deploy would stop at "Environment file not found". Canonical (product owner, 9 Nov): database te_prod, owner pgadmin. - build/environments/env_prod.example.sql is the single statement of db_name and db_owner. deploy_all.sh now reads both from the launcher it runs instead of keeping its own copy, which is what drifted. db_owner was declared in every launcher but never used. - deploy_all.sh creates the owner role NOLOGIN when it does not exist (CI, dev servers, the offline install; on Azure it is the server admin), and reports, but does not change, an existing database with another owner. - infra/entrypoint.sh generates env_.sql at run time from the image's .example.sql template and APP_USER_PASSWORD, writes it 0600, and deletes it on exit. The image still holds no launcher with a password (RV-S4, #174). It refuses to run without APP_USER_PASSWORD, and refuses when PGDATABASE is not the launcher's db_name, before changing anything. - infra/terraform-prod: Key Vault secret and job wiring for APP_USER_PASSWORD, mirroring pg_password; comment tying the database to the launcher. - Consumers of the old name follow: CI, evals Tier E, parity and env tests, config examples, provisioning script, offline installer, docs. CI now creates te_prod owned by pgadmin, as Azure has it. - Dockerfile default PGDATABASE was te_dev, a schema name, not a database. tests/test_prod_db_naming.py: static checks that the artefacts agree, and a behavioural run of the real entrypoint from committed files only. DECISIONS/2026-11-09-prod-db-naming.md records the choice. Test and staging have the same Terraform/deployer disagreement and are not changed here. Co-Authored-By: Claude Sonnet 5.5 Claude-Session: https://claude.ai/code/session_011T6WqjTEDRRQGLEYPPFVXx --- .github/workflows/quality-gate.yml | 11 +- DECISIONS/2026-11-09-prod-db-naming.md | 60 +++++++ README.md | 2 +- build/config.env.example | 2 +- build/deploy_all.sh | 42 ++++- build/environments/env_prod.example.sql | 6 +- docs/DATABASE_ENVIRONMENTS.md | 9 +- docs/ENV_VARIABLES.md | 26 ++- docs/PROD_DEPLOY.md | 6 + docs/offline-install.md | 2 +- evals/runner.py | 2 +- infra/Dockerfile | 11 +- infra/entrypoint.sh | 82 +++++++++- infra/terraform-prod/main.tf | 36 +++++ scripts/offline/install.sh | 11 +- scripts/provision_full_test_env.sh | 2 +- tests/environments/test_env_prod.py | 2 +- tests/run_tests.sh | 2 +- tests/test_parity.py | 2 +- tests/test_prod_db_naming.py | 200 ++++++++++++++++++++++++ 20 files changed, 484 insertions(+), 32 deletions(-) create mode 100644 DECISIONS/2026-11-09-prod-db-naming.md create mode 100644 tests/test_prod_db_naming.py diff --git a/.github/workflows/quality-gate.yml b/.github/workflows/quality-gate.yml index 821ae9a..b0189a8 100644 --- a/.github/workflows/quality-gate.yml +++ b/.github/workflows/quality-gate.yml @@ -117,7 +117,7 @@ jobs: PG_SCHEMA_TEST="te_test" PG_DB_STAGING="te_mgmt_staging" PG_SCHEMA_STAGING="te_staging" - PG_DB_PROD="te_mgmt_prod" + PG_DB_PROD="te_prod" PG_SCHEMA_PROD="te_prod" EOF @@ -133,12 +133,17 @@ jobs: done ls -1 build/environments/ + # Prod's database is te_prod owned by pgadmin, as in Azure (#172); the + # others are te_mgmt_ owned by postgres. The owners must match each + # build/environments/env_.example.sql, or deploy_all.sh warns. - name: Create environment databases run: | - for db in te_mgmt_dev te_mgmt_test te_mgmt_staging te_mgmt_prod; do + psql -v ON_ERROR_STOP=1 -d postgres -c "CREATE ROLE pgadmin NOLOGIN" + for db in te_mgmt_dev te_mgmt_test te_mgmt_staging te_prod; do + owner=postgres; [ "$db" = te_prod ] && owner=pgadmin if [ -z "$(psql -d postgres -tA -c "SELECT 1 FROM pg_database WHERE datname = '$db'")" ]; then psql -v ON_ERROR_STOP=1 -d postgres -c \ - "CREATE DATABASE \"$db\" WITH OWNER = postgres ENCODING = 'UTF8' TEMPLATE = template0 CONNECTION LIMIT = -1" + "CREATE DATABASE \"$db\" WITH OWNER = $owner ENCODING = 'UTF8' TEMPLATE = template0 CONNECTION LIMIT = -1" fi done diff --git a/DECISIONS/2026-11-09-prod-db-naming.md b/DECISIONS/2026-11-09-prod-db-naming.md new file mode 100644 index 0000000..e6bf861 --- /dev/null +++ b/DECISIONS/2026-11-09-prod-db-naming.md @@ -0,0 +1,60 @@ +# Decision: Production database is `te_prod`, owned by `pgadmin` + +Status: Decided and applied +Date: 2026-11-09 +Review: On trigger (see below) + +## Choice + +The production database is **`te_prod`** and its owner is **`pgadmin`**, in all three +artefacts that describe it: `infra/terraform-prod/main.tf`, `build/deploy_all.sh` and +`infra/entrypoint.sh`. Option (a) of #172. The schema inside it stays `te_prod`. + +`build/environments/env_prod.example.sql` is the single place that states the name and +owner (`db_name`, `db_owner`). The deployer reads them from the launcher it is about to +run instead of keeping its own copy. + +## Context + +#172 (RV-S1) found three artefacts disagreeing: Terraform created `te_prod` for `pgadmin`; +the deployer created `te_mgmt_prod` for `postgres`; the entrypoint needed an +`env_prod.sql` that a freshly built image does not contain. Nothing was deployed yet, so +nothing was broken; the first Azure deploy would have failed at the entrypoint with a +cryptic error. The fix had to land before Azure plumbing (#31, #32, #34) resumed. + +The 3 Nov decision was to fix all three together in one PR. The product owner chose the +canonical name on 9 Nov 2026. + +## Reasoning + +- Terraform is the source of truth for the Azure database; matching it is the + conventional direction for infrastructure-as-code. +- `postgres` owning an application database is poor practice on its own, and + `te_mgmt_prod` would have kept it. +- The hardcoded copies in the deployer were what drifted. Reading the launcher removes the + second copy rather than aligning two values by hand. + +## Trade-offs accepted + +- Prod now differs from dev, test and staging (`te_mgmt_`, owner `postgres`). The + reading code does not care; the asymmetry is in the data. +- Where `pgadmin` does not exist (CI, a dev server, the offline install), `deploy_all.sh` + creates it as `NOLOGIN`. It owns the database and nothing else, so it adds no credential, + but a stray role appears on those servers. +- The offline installer (`scripts/offline/install.sh`) follows: its prod database is now + `te_prod`, not `te_mgmt_prod`. Its #60 dry run (due 15 Nov) has not exercised this change. +- A database that already exists with a different owner is reported, not altered. +- `deploy` now requires `APP_USER_PASSWORD`. Without it the application role would be + created with the template's placeholder password. + +## Not decided here + +`infra/terraform-test` and `infra/terraform-staging` create `te_test` and `te_staging`, +while the deployer builds `te_mgmt_test` and `te_mgmt_staging`: the same disagreement. +Those stacks are code only (#31). Settle their names before #31 resumes. + +## Reopen trigger + +Reconsider if test and staging are given the Azure-style names (then prod no longer +stands apart and the `te_mgmt_` prefix may be dropped everywhere), or if the application +is ever to run as a role other than the database owner in Azure. diff --git a/README.md b/README.md index 74bf629..39fba1c 100644 --- a/README.md +++ b/README.md @@ -332,7 +332,7 @@ organisations ──< personnel | Setting | Dev | Test | Staging | Prod | |---|---|---|---|---| -| Database | `te_mgmt_dev` | `te_mgmt_test` | `te_mgmt_staging` | `te_mgmt_prod` | +| Database | `te_mgmt_dev` | `te_mgmt_test` | `te_mgmt_staging` | `te_prod` | | Schema | `te_dev` | `te_test` | `te_staging` | `te_prod` | | App User | `te_dev_user` | `te_test_user` | `te_stg_user` | `te_prod_user` | | Connection Limit | 10 | 15 | 25 | 50 | diff --git a/build/config.env.example b/build/config.env.example index e53368d..facd282 100644 --- a/build/config.env.example +++ b/build/config.env.example @@ -38,7 +38,7 @@ PG_APP_USER_STAGING="te_stg_user" PG_APP_PASSWORD_STAGING="" PG_CONN_LIMIT_STAGING="20" -PG_DB_PROD="te_mgmt_prod" +PG_DB_PROD="te_prod" PG_SCHEMA_PROD="te_prod" PG_APP_USER_PROD="te_prod_user" PG_APP_PASSWORD_PROD="" diff --git a/build/deploy_all.sh b/build/deploy_all.sh index be338e9..c2de4e3 100644 --- a/build/deploy_all.sh +++ b/build/deploy_all.sh @@ -44,6 +44,14 @@ warn() { echo -e "${YELLOW}[⚠]${NC} $*"; } error() { echo -e "${RED}[✗]${NC} $*" >&2; } separator(){ echo "────────────────────────────────────────────────────────────"; } +# Read one \set variable from an environment launcher (RV-S1, #172). +# env_.sql is the single place that says which database an environment +# deploys to and who owns it. This script used to keep its own copy +# (te_mgmt_, owner postgres) which drifted from Terraform's. +launcher_var() { + tr -d '\r' < "$1" | sed -n "s/^\\\\set[[:space:]][[:space:]]*$2[[:space:]][[:space:]]*'\{0,1\}\([^' ]*\)'\{0,1\}.*/\1/p" | head -n1 +} + # ── Parse arguments ─────────────────────────────────────────────────────────── ENVS_TO_DEPLOY=("${@:-${ALL_ENVS[@]}}") @@ -75,7 +83,14 @@ for env in "${ENVS_TO_DEPLOY[@]}"; do continue fi - DB_NAME="te_mgmt_${env}" + DB_NAME="$(launcher_var "${env_file}" db_name)" + DB_OWNER="$(launcher_var "${env_file}" db_owner)" + # Both land inside quoted SQL identifiers/literals below, so insist on plain names. + if [[ ! "${DB_NAME}" =~ ^[A-Za-z_][A-Za-z0-9_]*$ || ! "${DB_OWNER}" =~ ^[A-Za-z_][A-Za-z0-9_]*$ ]]; then + error "${env_file}: db_name ('${DB_NAME}') and db_owner ('${DB_OWNER}') must both be set to plain identifiers." + FAILED+=("${env}") + continue + fi echo "" warn "Deploying environment: ${env^^} (database: ${DB_NAME})" @@ -86,10 +101,24 @@ for env in "${ENVS_TO_DEPLOY[@]}"; do DB_EXISTS=$(${PSQL} -d postgres -tA -c \ "SELECT 1 FROM pg_database WHERE datname = '${DB_NAME}'" 2>/dev/null || true) if [[ -z "${DB_EXISTS}" ]]; then - log "Creating database: ${DB_NAME}" + # The owner has to exist before it can own anything. On Azure it is the + # server admin and already does. Anywhere else (CI, the offline install, a + # fresh dev server) it is created NOLOGIN: it owns the database and nothing + # else, so it adds no credential and no way in. + OWNER_EXISTS=$(${PSQL} -d postgres -tA -c \ + "SELECT 1 FROM pg_roles WHERE rolname = '${DB_OWNER}'" 2>/dev/null || true) + if [[ -z "${OWNER_EXISTS}" ]]; then + log "Creating database owner role: ${DB_OWNER} (NOLOGIN)" + ${PSQL} -d postgres -c "CREATE ROLE \"${DB_OWNER}\" NOLOGIN" || { + error "Failed to create owner role ${DB_OWNER}" + FAILED+=("${env}") + continue + } + fi + log "Creating database: ${DB_NAME} (owner: ${DB_OWNER})" ${PSQL} -d postgres -c \ "CREATE DATABASE \"${DB_NAME}\" - WITH OWNER = postgres + WITH OWNER = \"${DB_OWNER}\" ENCODING = 'UTF8' TEMPLATE = template0 CONNECTION LIMIT = -1" || { @@ -100,6 +129,13 @@ for env in "${ENVS_TO_DEPLOY[@]}"; do log "Database ${DB_NAME} created." else log "Database ${DB_NAME} already exists — skipping CREATE." + # Report drift, never fix it: changing the owner of a database that already + # holds data is not a decision a deploy script should take silently. + ACTUAL_OWNER=$(${PSQL} -d postgres -tA -c \ + "SELECT pg_get_userbyid(datdba) FROM pg_database WHERE datname = '${DB_NAME}'" 2>/dev/null || true) + if [[ -n "${ACTUAL_OWNER}" && "${ACTUAL_OWNER}" != "${DB_OWNER}" ]]; then + warn "${DB_NAME} is owned by '${ACTUAL_OWNER}', but ${env_file##*/} says '${DB_OWNER}'. Left unchanged." + fi fi if ${PSQL} \ diff --git a/build/environments/env_prod.example.sql b/build/environments/env_prod.example.sql index 13b213e..8493703 100644 --- a/build/environments/env_prod.example.sql +++ b/build/environments/env_prod.example.sql @@ -1,8 +1,10 @@ -- env_prod.example.sql (TEMPLATE - safe to commit; no real secrets) -- Copy to env_prod.sql (gitignored) and fill in, or inject app_password at deploy. +-- db_name and db_owner are read by build/deploy_all.sh and must match infra/terraform-prod +-- (database te_prod, server admin pgadmin). RV-S1, #172; DECISIONS/2026-11-09-prod-db-naming.md. \set env_label PROD -\set db_name te_mgmt_prod -\set db_owner postgres +\set db_name te_prod +\set db_owner pgadmin \set schema_name te_prod \set app_user te_prod_user \set app_password '__INJECT_AT_DEPLOY__' diff --git a/docs/DATABASE_ENVIRONMENTS.md b/docs/DATABASE_ENVIRONMENTS.md index 4a53816..6adb482 100644 --- a/docs/DATABASE_ENVIRONMENTS.md +++ b/docs/DATABASE_ENVIRONMENTS.md @@ -14,7 +14,14 @@ for names: `build/config.env.example` (template) / `build/config.local.env` | **dev** | `te_mgmt_dev` | `te_dev` | `te_dev_user` | 10 | yes (`SEED_DEV=true`) | | **test** | `te_mgmt_test` | `te_test` | `te_test_user` | 15 | yes (`SEED_TEST=true`) | | **staging** | `te_mgmt_staging` | `te_staging` | `te_stg_user` | 25 | no (`SEED_STAGING=false`) | -| **prod** | `te_mgmt_prod` | `te_prod` | `te_prod_user` | 50 | no (`SEED_PROD=false`) | +| **prod** | `te_prod` | `te_prod` | `te_prod_user` | 50 | no (`SEED_PROD=false`) | + +The database name and owner come from `build/environments/env_.sql` (`db_name`, +`db_owner`), which `deploy_all.sh` reads; nothing else keeps its own copy. Prod's +database is `te_prod`, owned by `pgadmin`, to match `infra/terraform-prod` (#172; +`DECISIONS/2026-11-09-prod-db-naming.md`). Where `pgadmin` does not exist yet (a dev +server, CI, the offline install) `deploy_all.sh` creates it `NOLOGIN`: it owns the +database and nothing else. Dev, test and staging keep `te_mgmt_` owned by `postgres`. Every environment also gets a `csv_uploads` schema in its own database (same name across environments — no collision, since each environment is a diff --git a/docs/ENV_VARIABLES.md b/docs/ENV_VARIABLES.md index da8868d..b5bea1c 100644 --- a/docs/ENV_VARIABLES.md +++ b/docs/ENV_VARIABLES.md @@ -60,7 +60,7 @@ the environment SQL templates. Copy `build/config.env.example` to | Variable | Dev | Test | Staging | Prod | |---|---|---|---|---| -| `PG_DB_*` | `te_mgmt_dev` | `te_mgmt_test` | `te_mgmt_staging` | `te_mgmt_prod` | +| `PG_DB_*` | `te_mgmt_dev` | `te_mgmt_test` | `te_mgmt_staging` | `te_prod` | | `PG_SCHEMA_*` | `te_dev` | `te_test` | `te_staging` | `te_prod` | | `PG_APP_USER_*` | `te_dev_user` | `te_test_user` | `te_stg_user` | `te_prod_user` | | `PG_APP_PASSWORD_*` | _(empty)_ | _(empty)_ | _(empty)_ | _(empty)_ | @@ -162,14 +162,32 @@ $env:PGPASSWORD = "changeme_local_only" python -m pytest tests\ -m "unit or regression or security or snapshot" ``` -### Deploying to Azure +### Running the migration job (`infra/entrypoint.sh`) +Terraform sets these on the Container Apps Job (`infra/terraform-prod/main.tf`): +``` +TARGET_ENV = prod +PGHOST = your-pg-host.postgres.database.azure.com +PGPORT = 5432 +PGUSER = pgadmin # the server admin, and the owner of te_prod +PGPASSWORD = +PGDATABASE = te_prod # must equal db_name in env_prod.example.sql +APP_USER_PASSWORD = +``` +`APP_USER_PASSWORD` is the password for the `te_prod_user` application role. The +image carries only `env_prod.example.sql`; `deploy` fills the password into a +launcher at run time and deletes it on exit, and refuses to run without it. +`deploy` also refuses to run if `PGDATABASE` differs from the launcher's +`db_name`. `MIGRATION_ROOT` (default `/opt/migration`) moves where the script +looks for the runner; it exists so the script can be tested outside a container. + +### Deploying the API to Azure Set these in the Container App environment (not in code): ``` PGHOST = your-pg-host.postgres.database.azure.com PGPORT = 5432 -PGUSER = postgres +PGUSER = pgadmin PGPASSWORD = -PGDATABASE = te_mgmt_prod +PGDATABASE = te_prod API_KEY = CORS_ORIGINS = https://your-frontend.azurestaticapps.net TE_SCHEMA = te_prod diff --git a/docs/PROD_DEPLOY.md b/docs/PROD_DEPLOY.md index 84606a3..d25c35e 100644 --- a/docs/PROD_DEPLOY.md +++ b/docs/PROD_DEPLOY.md @@ -154,6 +154,12 @@ branch) — this binds the SP to GitHub Environment approval. - Triggers a single execution of the Job - Streams logs from Log Analytics - Exits 0 only if the Job status is `Succeeded` +5. What the Job does on a fresh server: `infra/entrypoint.sh` generates + `env_prod.sql` from the image's `env_prod.example.sql` and the + `pg-app-user-password` secret, `deploy_all.sh` deploys into the `te_prod` + database Terraform created, and the generated file is deleted on exit. It fails + fast, naming the cause, if `APP_USER_PASSWORD` is missing or `PGDATABASE` is not + the launcher's database (#172). ### Step 3 — Verify diff --git a/docs/offline-install.md b/docs/offline-install.md index 231767c..75814c1 100644 --- a/docs/offline-install.md +++ b/docs/offline-install.md @@ -66,7 +66,7 @@ The script runs six steps: 1. Checks every file against `SHA256SUMS`. 2. Installs the Ubuntu packages, using the bundle as the only apt source. The VM's own apt sources are ignored. 3. Copies the app to `/opt/te-migration`, creates the `te-app` system user, and builds a Python virtual environment from `wheels/` without network access. -4. Deploys database `te_mgmt_prod` with schema `te_prod` and its owner role `te_prod_user`. It also creates the runtime role `te_prod_runtime`, which serves requests and can only read and insert the audit and upload tables (M3d, #107). Both get generated passwords. +4. Deploys database `te_prod` with schema `te_prod` and its owner role `te_prod_user`. It also creates the runtime role `te_prod_runtime`, which serves requests and can only read and insert the audit and upload tables (M3d, #107). Both get generated passwords. 5. Writes `/opt/te-migration/env` (mode 600) with both passwords and a generated API key, and starts the `te-api` (port 8000) and `te-frontend` (port 3000) services under systemd. The API key is for scripts only. The frontend holds no key; people sign in. 6. Creates the first tenant and its tenant admin, then runs the smoke test. diff --git a/evals/runner.py b/evals/runner.py index 5a9c0a0..48822e4 100644 --- a/evals/runner.py +++ b/evals/runner.py @@ -559,7 +559,7 @@ def _run_fresh_deploy_then_tests( ("dev", "te_mgmt_dev", "te_dev"), ("test", "te_mgmt_test", "te_test"), ("staging", "te_mgmt_staging", "te_staging"), - ("prod", "te_mgmt_prod", "te_prod"), + ("prod", "te_prod", "te_prod"), ] diff --git a/infra/Dockerfile b/infra/Dockerfile index 91893c9..cfb33ac 100644 --- a/infra/Dockerfile +++ b/infra/Dockerfile @@ -16,9 +16,16 @@ # Run locally (against a PG container on the same network): # docker run --rm \ # -e PGHOST=postgres -e PGPORT=5432 -e PGUSER=postgres \ -# -e PGPASSWORD=devpass -e PGDATABASE=te_dev \ +# -e PGPASSWORD=devpass -e PGDATABASE=te_mgmt_dev \ +# -e APP_USER_PASSWORD=change-me \ # -e TARGET_ENV=dev \ # te-migration:dev deploy +# +# The image holds only the env_.example.sql templates. `deploy` generates +# env_.sql from the template and APP_USER_PASSWORD at run time and deletes it +# on exit, so no launcher with a password is ever in an image (RV-S4, #174). +# PGDATABASE must equal the db_name in that launcher (prod: te_prod, matching +# infra/terraform-prod); `deploy` refuses to run if it does not (RV-S1, #172). FROM python:3.11-slim AS base @@ -65,7 +72,7 @@ USER app ENV PGHOST=localhost \ PGPORT=5432 \ PGUSER=postgres \ - PGDATABASE=te_dev \ + PGDATABASE=te_mgmt_dev \ TARGET_ENV=dev \ WAIT_FOR_DB_SECONDS=60 diff --git a/infra/entrypoint.sh b/infra/entrypoint.sh index e8a6c43..553bfae 100644 --- a/infra/entrypoint.sh +++ b/infra/entrypoint.sh @@ -1,10 +1,19 @@ #!/usr/bin/env bash # entrypoint.sh - dispatches to the correct workflow inside the container. # Usage (passed as the container CMD): deploy | load | evals | full | shell +# +# deploy needs APP_USER_PASSWORD: the password for the environment's application +# role (te_prod_user for prod). The image holds only the committed +# env_.example.sql template, so the launcher is generated here at run time +# from that template and the password. It is never in the image (RV-S4, #174) and +# is deleted when the script exits (RV-S1, #172). set -euo pipefail ACTION="${1:-deploy}" +# Where the image keeps the runner. Overridable so the script can be tested +# outside a container; inside one it is always /opt/migration. +ROOT="${MIGRATION_ROOT:-/opt/migration}" echo "============================================================" echo " T&E Migration container" @@ -30,16 +39,81 @@ wait_for_pg() { echo "PostgreSQL is ready." } +# \set from a launcher; same parsing as build/deploy_all.sh. +launcher_var() { + tr -d '\r' < "$1" | sed -n "s/^\\\\set[[:space:]][[:space:]]*$2[[:space:]][[:space:]]*'\{0,1\}\([^' ]*\)'\{0,1\}.*/\1/p" | head -n1 +} + +GENERATED_LAUNCHER="" +cleanup() { [ -z "$GENERATED_LAUNCHER" ] || rm -f "$GENERATED_LAUNCHER"; } +trap cleanup EXIT + +# Make sure build/environments/env_.sql exists. An operator-supplied one +# (mounted in, or left by a developer) is used as is; otherwise build it from the +# template. A clean image has only the template, so without this a first deploy +# stops at "Environment file not found". +prepare_launcher() { + case "$TARGET_ENV" in + dev|test|staging|prod) ;; + *) echo "ERROR: TARGET_ENV must be dev, test, staging or prod (got '${TARGET_ENV}')." >&2; exit 2 ;; + esac + local dir="$ROOT/build/environments" + local template="$dir/env_${TARGET_ENV}.example.sql" + local launcher="$dir/env_${TARGET_ENV}.sql" + + if [ -f "$launcher" ]; then + echo "--- Using existing launcher ${launcher##*/} ---" + return 0 + fi + if [ ! -f "$template" ]; then + echo "ERROR: neither ${launcher##*/} nor ${template##*/} is in $dir." >&2 + exit 1 + fi + if [ -z "${APP_USER_PASSWORD:-}" ]; then + echo "ERROR: APP_USER_PASSWORD is not set. It is the password for the ${TARGET_ENV} application role;" >&2 + echo " without it the role would be created with the placeholder password in the template." >&2 + exit 1 + fi + + # Written 0600 and removed on exit by the trap above. Escaping is for psql's + # single-quoted \set argument: '' for a quote, \\ for a backslash. + GENERATED_LAUNCHER="$launcher" + ( umask 077 + python3 - "$template" "$launcher" <<'EOF' +import os, sys +src, dst = sys.argv[1:3] +pw = os.environ["APP_USER_PASSWORD"].replace("\\", "\\\\").replace("'", "''") +text = open(src, encoding="utf-8-sig").read() +if "__INJECT_AT_DEPLOY__" not in text: + sys.exit("template has no __INJECT_AT_DEPLOY__ placeholder: " + src) +open(dst, "w", encoding="utf-8").write(text.replace("__INJECT_AT_DEPLOY__", pw)) +EOF + ) + echo "--- Generated ${launcher##*/} from ${template##*/} ---" +} + run_deploy() { wait_for_pg + prepare_launcher + # The deployer creates and fills the database the launcher names, whatever the + # job's PGDATABASE says. If the two differ the evals would then run against a + # different database from the one just deployed, which is how Terraform and + # the deployer drifted apart (#172). Fail before changing anything. + local launcher_db + launcher_db="$(launcher_var "$ROOT/build/environments/env_${TARGET_ENV}.sql" db_name)" + if [ "$launcher_db" != "$PGDATABASE" ]; then + echo "ERROR: PGDATABASE is '${PGDATABASE}' but env_${TARGET_ENV}.sql deploys to '${launcher_db}'." >&2 + echo " Point the job at ${launcher_db}, or change db_name in the launcher template." >&2 + exit 1 + fi echo "--- Deploying schema for env: ${TARGET_ENV} ---" - cd /opt/migration + cd "$ROOT" bash build/deploy_all.sh "${TARGET_ENV}" } run_load() { wait_for_pg - if [ ! -f /opt/migration/input_data/load_input_data.sql ]; then + if [ ! -f "$ROOT/input_data/load_input_data.sql" ]; then echo "--- SKIP load: no input_data/load_input_data.sql in image ---" echo " (mount input_data as a volume or rebuild the image with it included)" return 0 @@ -47,13 +121,13 @@ run_load() { echo "--- Loading input data ---" psql -h "$PGHOST" -p "$PGPORT" -U "$PGUSER" -d "$PGDATABASE" \ -v ON_ERROR_STOP=1 \ - -f /opt/migration/input_data/load_input_data.sql + -f "$ROOT/input_data/load_input_data.sql" } run_evals() { wait_for_pg echo "--- Running eval suite (tiers p,i,s) ---" - cd /opt/migration + cd "$ROOT" python3 evals/runner.py --tiers p,i,s } diff --git a/infra/terraform-prod/main.tf b/infra/terraform-prod/main.tf index 427fe5b..e8a88f3 100644 --- a/infra/terraform-prod/main.tf +++ b/infra/terraform-prod/main.tf @@ -227,6 +227,27 @@ resource "azurerm_key_vault_secret" "pg_password" { depends_on = [azurerm_role_assignment.kv_admin_terraform] } +# Password for te_prod_user, the application role te_core_schema.sql creates. +# The migration job passes it to infra/entrypoint.sh as APP_USER_PASSWORD, which +# writes it into the launcher at run time. Without it the role would be created +# with the template's placeholder password (RV-S1, #172). +resource "random_password" "app_user_password" { + length = 32 + special = true + min_lower = 4 + min_upper = 4 + min_numeric = 4 + override_special = "!@#$%^&*()-_=+" +} + +resource "azurerm_key_vault_secret" "app_user_password" { + name = "pg-app-user-password" + value = random_password.app_user_password.result + key_vault_id = azurerm_key_vault.kv.id + + depends_on = [azurerm_role_assignment.kv_admin_terraform] +} + # --- PostgreSQL Flexible Server (managed, HA-capable) ------------------- resource "azurerm_postgresql_flexible_server" "pg" { @@ -271,6 +292,11 @@ resource "azurerm_postgresql_flexible_server" "pg" { } } +# The production database. Its name and the server admin ("pgadmin", the job's +# PGUSER and the database owner) must equal db_name and db_owner in +# build/environments/env_prod.example.sql: that file is what build/deploy_all.sh +# reads. tests/test_prod_db_naming.py fails if they drift (RV-S1, #172; +# DECISIONS/2026-11-09-prod-db-naming.md). resource "azurerm_postgresql_flexible_server_database" "te_prod" { name = "te_prod" server_id = azurerm_postgresql_flexible_server.pg.id @@ -477,6 +503,10 @@ resource "azurerm_container_app_job" "migration" { name = "PGPASSWORD" secret_name = "pg-password" } + env { + name = "APP_USER_PASSWORD" + secret_name = "app-user-password" + } } } @@ -486,6 +516,12 @@ resource "azurerm_container_app_job" "migration" { key_vault_secret_id = azurerm_key_vault_secret.pg_password.id } + secret { + name = "app-user-password" + identity = azurerm_user_assigned_identity.runner.id + key_vault_secret_id = azurerm_key_vault_secret.app_user_password.id + } + tags = local.tags lifecycle { diff --git a/scripts/offline/install.sh b/scripts/offline/install.sh index 1393908..310e509 100755 --- a/scripts/offline/install.sh +++ b/scripts/offline/install.sh @@ -7,7 +7,7 @@ # The first tenant and its admin come from TE_TENANT (default: main), # TE_ADMIN_USER (default: admin) and TE_ADMIN_PASSWORD (default: generated and # written to /root/te-initial-admin, readable by root only). -# Installs to /opt/te-migration, deploys database te_mgmt_prod (schema te_prod), +# Installs to /opt/te-migration, deploys database te_prod (schema te_prod), # writes /opt/te-migration/env, and installs systemd units te-api and te-frontend. set -euo pipefail @@ -38,7 +38,8 @@ python3.12 -m venv "$DEST/venv" echo "== 4/6 database, T&E schema and application role" DB_PASS="$(openssl rand -hex 24)"; API_KEY="$(openssl rand -hex 32)" -# env_prod.sql creates te_mgmt_prod, schema te_prod and login role te_prod_user. +# env_prod.sql creates database te_prod (owned by NOLOGIN role pgadmin, which deploy_all.sh +# creates when absent: the same name Azure uses), schema te_prod and login role te_prod_user. sed "s/__INJECT_AT_DEPLOY__/$DB_PASS/" "$DEST/app/build/environments/env_prod.example.sql" \ > "$DEST/app/build/environments/env_prod.sql" ( cd "$DEST/app" && sudo -u postgres env PGHOST=/var/run/postgresql bash build/deploy_all.sh prod >/dev/null ) @@ -46,11 +47,11 @@ rm "$DEST/app/build/environments/env_prod.sql" # it holds the password # M3d (#107): a separate runtime role for requests; it can only read and insert the # audit and upload tables. te_prod_user stays the owner and runs migrations at startup. APP_DB_PASS="$(openssl rand -hex 24)" -sudo -u postgres psql -q -v ON_ERROR_STOP=1 -d te_mgmt_prod -c "SET client_min_messages = warning" \ +sudo -u postgres psql -q -v ON_ERROR_STOP=1 -d te_prod -c "SET client_min_messages = warning" \ -c "DO \$\$BEGIN IF NOT EXISTS (SELECT FROM pg_roles WHERE rolname = 'te_prod_runtime') THEN CREATE ROLE te_prod_runtime LOGIN; END IF; END\$\$" \ -c "ALTER ROLE te_prod_runtime LOGIN NOSUPERUSER NOCREATEDB NOCREATEROLE PASSWORD '$APP_DB_PASS'" \ -c "GRANT te_prod_runtime TO te_prod_user" \ - -c "GRANT CONNECT ON DATABASE te_mgmt_prod TO te_prod_runtime" >/dev/null + -c "GRANT CONNECT ON DATABASE te_prod TO te_prod_runtime" >/dev/null echo "== 5/6 configuration and services" cat > "$DEST/env" <.example.sql`` is the single +place that says so; the deployer reads it rather than keeping its own copy. + +``ProductionArtefactsAgree`` is the regression guard: static, no database, runs in CI's +free-tier job. ``EntrypointOnAFreshServer`` is the behavioural check: it runs the real +entrypoint from a tree holding only committed files (what a clean image contains) against +a live server with no pre-existing owner role. +""" + +from __future__ import annotations + +import os +import re +import shutil +import subprocess +import tempfile +import unittest +import uuid +from pathlib import Path + +import psycopg2 +import pytest +from psycopg2 import sql + +from api.config import settings + +ROOT = Path(__file__).resolve().parents[1] +ENV_DIR = ROOT / "build" / "environments" +CANONICAL_DB = "te_prod" +CANONICAL_OWNER = "pgadmin" + + +def launcher_var(path: Path, name: str) -> str: + """Value of ``\\set `` in an environment launcher.""" + for line in path.read_text(encoding="utf-8-sig").splitlines(): + m = re.match(rf"\\set\s+{name}\s+'?([^'\s]*)'?", line) + if m: + return m.group(1) + raise AssertionError(f"\\set {name} not found in {path}") + + +@pytest.mark.unit +class ProductionArtefactsAgree(unittest.TestCase): + prod_template = ENV_DIR / "env_prod.example.sql" + tf = (ROOT / "infra" / "terraform-prod" / "main.tf").read_text(encoding="utf-8") + deployer = (ROOT / "build" / "deploy_all.sh").read_text(encoding="utf-8") + entrypoint = (ROOT / "infra" / "entrypoint.sh").read_text(encoding="utf-8") + + def test_the_launcher_names_the_canonical_database_and_owner(self): + self.assertEqual(launcher_var(self.prod_template, "db_name"), CANONICAL_DB) + self.assertEqual(launcher_var(self.prod_template, "db_owner"), CANONICAL_OWNER) + + def test_terraform_creates_the_same_database(self): + names = re.findall( + r'resource\s+"azurerm_postgresql_flexible_server_database"\s+"\w+"\s*\{[^}]*?name\s*=\s*"([^"]+)"', + self.tf) + self.assertEqual(names, [launcher_var(self.prod_template, "db_name")], + "Terraform's database differs from the one the deployer will deploy to") + + def test_terraform_admin_is_the_launchers_owner(self): + admin = re.search(r'administrator_login\s*=\s*"([^"]+)"', self.tf).group(1) + self.assertEqual(admin, launcher_var(self.prod_template, "db_owner")) + + def test_the_migration_job_runs_as_that_owner_against_that_database(self): + job_user = re.search(r'name\s*=\s*"PGUSER"\s*value\s*=\s*"([^"]+)"', self.tf).group(1) + self.assertEqual(job_user, launcher_var(self.prod_template, "db_owner")) + self.assertRegex(self.tf, r'name\s*=\s*"PGDATABASE"\s*value\s*=\s*azurerm_postgresql_flexible_server_database\.\w+\.name', + "PGDATABASE must come from the database resource, not a second literal") + + def test_every_launcher_declares_both_so_the_deployer_can_read_them(self): + for env in ("dev", "test", "staging", "prod"): + path = ENV_DIR / f"env_{env}.example.sql" + self.assertTrue(launcher_var(path, "db_name"), path) + self.assertTrue(launcher_var(path, "db_owner"), path) + + def test_the_deployer_keeps_no_name_or_owner_of_its_own(self): + """The hardcoded copies are what drifted.""" + code = "\n".join(l for l in self.deployer.splitlines() if not l.lstrip().startswith("#")) + self.assertNotRegex(code, r"te_mgmt_", "deploy_all.sh hardcodes a database name") + self.assertNotRegex(code, r"OWNER\s*=\s*postgres", "deploy_all.sh hardcodes the database owner") + + def test_the_entrypoint_generates_its_launcher_from_the_committed_template(self): + self.assertIn("example.sql", self.entrypoint, + "entrypoint.sh must build env_.sql from the template; a clean image has only the template") + + def test_the_template_is_not_a_populated_launcher(self): + """RV-S4 (#174): the fix must not bake a real password into the image.""" + self.assertEqual(launcher_var(self.prod_template, "app_password"), "__INJECT_AT_DEPLOY__") + + +@pytest.mark.integration +class EntrypointOnAFreshServer(unittest.TestCase): + """Run the real entrypoint, from committed files only, against a live server. + + The launcher's database, schema, owner and app user are rewritten to throwaway + names so the run cannot touch a real environment; the machinery under test (name + and owner read from the launcher, launcher generated at runtime, owner role + created when absent) is identical. The names themselves are pinned by + ``ProductionArtefactsAgree``. + """ + + PASSWORD = "a'b\\c&d/e\"f g" # every character the generation step must survive + + @classmethod + def setUpClass(cls): + tag = uuid.uuid4().hex[:8] + cls.db, cls.schema = f"te_ept_{tag}", f"ept_{tag}" + cls.owner, cls.app_user = f"ept_owner_{tag}", f"ept_app_{tag}" + cls.tmp = tempfile.TemporaryDirectory() + cls.root = Path(cls.tmp.name) + tracked = subprocess.run(["git", "ls-files", "--", "build", "infra/entrypoint.sh"], cwd=ROOT, + capture_output=True, text=True, check=True).stdout.split() + for rel in tracked: # committed files only: what a clean image holds + dest = cls.root / rel + dest.parent.mkdir(parents=True, exist_ok=True) + shutil.copy2(ROOT / rel, dest) + shutil.copy2(ROOT / "infra" / "entrypoint.sh", cls.root / "entrypoint.sh") + tpl = cls.root / "build" / "environments" / "env_prod.example.sql" + text = tpl.read_text(encoding="utf-8-sig") + for var, value in (("db_name", cls.db), ("schema_name", cls.schema), + ("db_owner", cls.owner), ("app_user", cls.app_user)): + text = re.sub(rf"(\\set\s+{var}\s+)\S+", rf"\g<1>{value}", text) + tpl.write_text(text, encoding="utf-8") + + @classmethod + def tearDownClass(cls): + conn = cls._admin() + try: + conn.autocommit = True # DROP DATABASE cannot run in a transaction + with conn.cursor() as cur: + cur.execute(sql.SQL("DROP DATABASE IF EXISTS {}").format(sql.Identifier(cls.db))) + for role in (cls.app_user, cls.owner): + cur.execute(sql.SQL("DROP ROLE IF EXISTS {}").format(sql.Identifier(role))) + finally: + conn.close() + cls.tmp.cleanup() + + @staticmethod + def _admin(): + return psycopg2.connect(host=settings.PG_HOST, port=settings.PG_PORT, user=settings.PG_USER, + password=settings.PG_PASSWORD, dbname="postgres") + + def _entrypoint(self, **extra): + env = {k: v for k, v in os.environ.items() if k != "APP_USER_PASSWORD"} + env.update(MIGRATION_ROOT=str(self.root), TARGET_ENV="prod", PGHOST=settings.PG_HOST, + PGPORT=str(settings.PG_PORT), PGUSER=settings.PG_USER, PGPASSWORD=settings.PG_PASSWORD, + PGDATABASE=self.db, WAIT_FOR_DB_SECONDS="10") + env.update(extra) + return subprocess.run(["bash", str(self.root / "entrypoint.sh"), "deploy"], env=env, + capture_output=True, text=True, timeout=300) + + def test_1_it_refuses_to_deploy_without_a_password_and_says_why(self): + run = self._entrypoint() + self.assertNotEqual(run.returncode, 0) + self.assertIn("APP_USER_PASSWORD", run.stderr, run.stderr) + self.assertFalse((self.root / "build" / "environments" / "env_prod.sql").exists()) + + def test_2_it_refuses_a_job_pointed_at_a_different_database(self): + run = self._entrypoint(APP_USER_PASSWORD=self.PASSWORD, PGDATABASE="somewhere_else") + self.assertNotEqual(run.returncode, 0) + self.assertIn(self.db, run.stderr, run.stderr) + + def test_3_a_clean_tree_deploys_to_the_launchers_database_and_owner(self): + self.assertFalse((self.root / "build" / "environments" / "env_prod.sql").exists(), + "the tree must start with only the template, like a clean image") + run = self._entrypoint(APP_USER_PASSWORD=self.PASSWORD) + self.assertEqual(run.returncode, 0, run.stdout[-1500:] + run.stderr[-1500:]) + with self._admin() as conn, conn.cursor() as cur: + cur.execute("SELECT pg_get_userbyid(datdba) FROM pg_database WHERE datname = %s", (self.db,)) + row = cur.fetchone() + self.assertIsNotNone(row, f"database {self.db} was not created") + self.assertEqual(row[0], self.owner, "database owner is not the launcher's db_owner") + cur.execute("SELECT rolcanlogin FROM pg_roles WHERE rolname = %s", (self.owner,)) + self.assertEqual(cur.fetchone(), (False,), "the owner role must exist and be NOLOGIN") + with psycopg2.connect(host=settings.PG_HOST, port=settings.PG_PORT, user=self.app_user, + password=self.PASSWORD, dbname=self.db) as conn, conn.cursor() as cur: + cur.execute("SELECT count(*) FROM information_schema.tables WHERE table_schema = %s", (self.schema,)) + self.assertGreaterEqual(cur.fetchone()[0], 12, "the T&E tables were not created") + + def test_4_no_launcher_with_a_password_is_left_behind(self): + self.assertFalse((self.root / "build" / "environments" / "env_prod.sql").exists(), + "the generated launcher holds the app password and must be removed") + + def test_5_a_second_run_is_idempotent(self): + run = self._entrypoint(APP_USER_PASSWORD=self.PASSWORD) + self.assertEqual(run.returncode, 0, run.stdout[-1500:] + run.stderr[-1500:]) + + +if __name__ == "__main__": + unittest.main()