diff --git a/.github/workflows/lambda-deploy.yml b/.github/workflows/lambda-deploy.yml index 38bf5406..cfef17e2 100644 --- a/.github/workflows/lambda-deploy.yml +++ b/.github/workflows/lambda-deploy.yml @@ -15,6 +15,10 @@ on: description: 'Apply pending DB migrations without redeploying lambdas' type: boolean default: false + adopt_baseline: + description: 'One-time: let Flyway adopt a populated database that has no schema history' + type: boolean + default: false jobs: detect-changes: @@ -162,7 +166,7 @@ jobs: # reviewers: a pending approval here would strand a merged PR with its schema # applied and its lambda code undeployed. environment: production-db - # kysely takes a pg advisory lock, but serialize anyway so two merges in a row + # Flyway locks the history table, but serialize anyway so two merges in a row # queue instead of racing -- and never cancel a half-run migration. concurrency: group: db-migrate-prod @@ -172,6 +176,8 @@ jobs: contents: read actions: read pull-requests: read + env: + FLYWAY_BASELINE_ON_MIGRATE: ${{ inputs.adopt_baseline == true }} steps: - uses: actions/checkout@v4 - name: Setup Node.js @@ -221,22 +227,36 @@ jobs: - name: Pending migrations (read-only preflight) id: status run: | - set -o pipefail - npm run migrate:status --prefix apps/backend/db 2>&1 | tee status.log - pending=$(grep -oE '[0-9]+ pending' status.log | tail -1 | cut -d' ' -f1) - # Unknown (unexpected output) falls through as "1" so we take the safe - # path and snapshot before touching anything. - echo "pending=${pending:-1}" >> "$GITHUB_OUTPUT" + set -euo pipefail + npm run --silent migrate:info --prefix apps/backend/db -- -outputType=json > info.json + # `jq -e` on malformed JSON exits non-zero, which inside `if` reads as "condition absent". + jq -e 'type == "object" and has("migrations") and has("allSchemasEmpty")' \ + info.json > /dev/null \ + || { echo "::error::could not parse the output of flyway info"; exit 1; } + + jq -r '.migrations[] | "\(.state)\t\(.version)\t\(.description)"' info.json | tee status.log + pending=$(jq '[.migrations[] | select(.state == "Pending")] | length' info.json) + echo "pending=$pending" >> "$GITHUB_OUTPUT" echo "$pending migration(s) pending" - # 0000_baseline_schema is idempotent, but "idempotent" is one typo away from - # DROP SCHEMA. It is adopted once by hand against production (see - # apps/backend/db/README.md); if it still shows as pending here, we are - # pointed at a database we did not expect -- refuse rather than guess. + # Empty schema means the wrong DB_HOST; populated with no history means a snapshot or clone -- refuse both. - name: Refuse to execute the baseline against production + env: + ADOPT: ${{ inputs.adopt_baseline }} run: | - if grep -E '0000_baseline_schema' status.log | grep -q 'PENDING'; then - echo "::error::0000_baseline_schema is PENDING on the target database. It was never adopted (or DB_HOST is wrong). Refusing to run -- see apps/backend/db/README.md." + set -euo pipefail + if [ "$(jq -r '.allSchemasEmpty' info.json)" = 'true' ]; then + echo "::error::schema \"branch\" is empty on the target database. Refusing to build production from scratch -- check DB_HOST." + exit 1 + fi + tracked=$(jq -r '.schemaVersion' info.json) + if [ "$tracked" = 'null' ] && [ "$ADOPT" != 'true' ]; then + echo "::error::the target database has a populated schema \"branch\" but no Flyway history. Refusing to adopt it -- re-run this workflow manually with adopt_baseline if that is genuinely production." + exit 1 + fi + if [ "$tracked" != 'null' ] \ + && jq -e 'any(.migrations[]; .version == "0000" and .state == "Pending")' info.json > /dev/null; then + echo "::error::V0000__baseline_schema is PENDING on a database Flyway already tracks. The schema history and the schema have drifted -- refusing to run." exit 1 fi diff --git a/.github/workflows/lambda-tests.yml b/.github/workflows/lambda-tests.yml index 4b15f590..9a9ff629 100644 --- a/.github/workflows/lambda-tests.yml +++ b/.github/workflows/lambda-tests.yml @@ -44,13 +44,9 @@ jobs: uses: actions/setup-node@v4 with: node-version: '20' - # Build the schema the same way production does -- by applying - # db/migrations -- then load the dev seed rows the tests expect. The tests - # also call ensureSchema()/resetData() from db/testkit.ts themselves, so - # this doubles as a per-PR smoke test that migrations apply to a virgin DB. - - name: Apply migrations and seed + - name: Build the schema and seed working-directory: apps/backend/db - run: npm ci --no-audit --no-fund && npm run migrate && npm run seed + run: npm ci --no-audit --no-fund && npm run reset env: DATABASE_URL: postgres://branch_dev:password@localhost:5432/branch_db - name: Build shared packages @@ -120,6 +116,7 @@ jobs: esac echo "sha=$base" >> "$GITHUB_OUTPUT" + # Keyed by version, not path: that is what Flyway's history pins. - name: Applied migrations are immutable if: steps.base.outputs.sha != '' env: @@ -127,17 +124,42 @@ jobs: run: | set -euo pipefail MB=$(git merge-base "$BASE" HEAD) - # Anything but A(dded) under migrations/ is a violation: M modified, - # D deleted, R renamed, C copied, T typechange. --find-renames is - # explicit so a rename reports R (not D+A) regardless of the runner's - # diff.renames default. - violations=$(git diff --name-status --find-renames "$MB" HEAD -- "$DIR" | grep -vE '^A[[:space:]]' || true) - if [ -n "$violations" ]; then - echo "::error::Migrations already on main can never be modified, renamed or deleted -- someone has already run them. Add a NEW migration that fixes the old one." - printf '%s\n' "$violations" - exit 1 + TAB=$(printf '\t') + + # versionblobname, sorted because `join` requires sorted input. + list() { + git ls-tree -r "$1" -- "$DIR" | while read -r _ _ sha path; do + name=$(basename "$path") + printf '%s\t%s\t%s\n' \ + "$(printf '%s' "$name" | sed -E 's/^V?0*([0-9]+)_+.*/\1/')" \ + "$sha" "$name" + done | sort + } + list "$MB" > "$RUNNER_TEMP/was.tsv" + list HEAD > "$RUNNER_TEMP/now.tsv" + + fail=0 + gone=$(join -t"$TAB" -v1 -j1 "$RUNNER_TEMP/was.tsv" "$RUNNER_TEMP/now.tsv" | cut -f3) + if [ -n "$gone" ]; then + echo "::error::deleted, but already run on production -- add a NEW migration that undoes it: $(echo $gone)" + fail=1 fi + # Rename check only for V-names: adopting Flyway renamed every file once. + join -t"$TAB" -j1 "$RUNNER_TEMP/was.tsv" "$RUNNER_TEMP/now.tsv" > "$RUNNER_TEMP/both.tsv" + awk -F'\t' -v dir="$DIR" ' + $2 != $4 { + printf "::error file=%s/%s::contents changed. Flyway checksums every applied migration, so this fails the next deploy. Add a NEW migration that fixes the old one.\n", dir, $5 + bad = 1 + } + $3 ~ /^V/ && $3 != $5 { + printf "::error file=%s/%s::renamed from %s. Production'"'"'s schema history records the old name, and Flyway rejects the mismatch.\n", dir, $5, $3 + bad = 1 + } + END { exit bad } + ' "$RUNNER_TEMP/both.tsv" || fail=1 + exit $fail + - name: New migrations must be named correctly if: steps.base.outputs.sha != '' env: @@ -150,14 +172,44 @@ jobs: last=$(git ls-tree -r --name-only "$MB" -- "$DIR" | xargs -r -n1 basename | sort | tail -1 || true) fail=0 for f in $added; do - if ! echo "$f" | grep -qE '^(0000_baseline_schema|[0-9]{14}_[a-z0-9_]+)\.sql$'; then - echo "::error file=$DIR/$f::name must be YYYYMMDDHHMMSS_lower_snake_case.sql -- use 'make new-migration NAME=...' so it is generated for you" + if ! echo "$f" | grep -qE '^V(0000|[0-9]{14})__[a-z0-9_]+\.sql$'; then + echo "::error file=$DIR/$f::name must be VYYYYMMDDHHMMSS__lower_snake_case.sql -- use 'make new-migration NAME=...' so it is generated for you" fail=1 fi - # Out-of-order merges are allowed at runtime (the migrator sets - # allowUnorderedMigrations), so this is advice, not a failure. + # Warning only: outOfOrder is set at runtime; the next step fails anything at or below the baseline. if [ -n "${last:-}" ] && [ "$f" \< "$last" ]; then - echo "::warning file=$DIR/$f::sorts before $last, which is already on main, so it will be applied out of order. Harmless, but renaming it to a current timestamp keeps the history readable." + echo "::warning file=$DIR/$f::sorts before $last, which is already on main, so it will be applied out of order. Renaming it to a current UTC timestamp keeps the history readable." + fi + done + exit $fail + + # Flyway skips anything at or below the baseline as "Below Baseline" with exit 0; CI's empty schema never baselines, so nothing else catches it. + - name: New migrations must sort above the adoption baseline + if: steps.base.outputs.sha != '' + env: + BASE: ${{ steps.base.outputs.sha }} + run: | + set -euo pipefail + baseline=$(sed -n 's/^BASELINE_VERSION=//p' apps/backend/db/flyway.sh) + case "$baseline" in + '' | *[!0-9]*) + echo "::error::could not read a numeric BASELINE_VERSION from apps/backend/db/flyway.sh" + exit 1 ;; + esac + MB=$(git merge-base "$BASE" HEAD) + version_of() { basename "$1" | sed -E 's/^V0*([0-9]+)_+.*/\1/'; } + + # Renames show up as additions, so skip versions already on main. + git ls-tree -r --name-only "$MB" -- "$DIR" | while read -r p; do version_of "$p"; done \ + | sort -u > "$RUNNER_TEMP/known.txt" + + fail=0 + for f in $(git diff --diff-filter=A --name-only "$MB" HEAD -- "$DIR"); do + v=$(version_of "$f") + grep -qxF "$v" "$RUNNER_TEMP/known.txt" && continue + if [ "$v" -le "$baseline" ]; then + echo "::error file=$f::version $v is at or below the adoption baseline $baseline, so production would report it \"Below Baseline\" and never run it -- while CI stays green. Rename it with a current UTC timestamp." + fail=1 fi done exit $fail @@ -189,7 +241,7 @@ jobs: fail=1 fi if printf '%s\n' "$sql" | grep -inE 'CONCURRENTLY'; then - echo "::error file=$f::CONCURRENTLY cannot run inside a transaction, and the migrator wraps the run in one. This database is tiny -- use a plain CREATE INDEX." + echo "::error file=$f::CONCURRENTLY cannot run inside a transaction, and Flyway wraps each migration in one. This database is tiny -- use a plain CREATE INDEX." fail=1 fi if printf '%s\n' "$sql" | grep -inE '^[[:space:]]*INSERT[[:space:]]+INTO[[:space:]]+(branch\.)?users\b'; then @@ -198,6 +250,26 @@ jobs: done exit $fail + # Advisory: the existing corpus still holds plpgsql the DSQL cutover drops. + - name: Aurora DSQL compatibility (advisory) + if: steps.base.outputs.sha != '' + continue-on-error: true + env: + BASE: ${{ steps.base.outputs.sha }} + run: | + set -euo pipefail + MB=$(git merge-base "$BASE" HEAD) + files=$(git diff --diff-filter=A --name-only "$MB" HEAD -- "$DIR" || true) + [ -n "$files" ] || { echo "no new migrations"; exit 0; } + npm install --prefix "$RUNNER_TEMP/dsql-lint" --no-audit --no-fund @aws/dsql-lint + lint=$RUNNER_TEMP/dsql-lint/node_modules/@aws/dsql-lint/bin/dsql-lint + for f in $files; do + [ -f "$f" ] || continue + echo "--- $f" + sed 's/${async}/ASYNC/g' "$f" > "$RUNNER_TEMP/$(basename "$f")" + node "$lint" "$RUNNER_TEMP/$(basename "$f")" || true + done + - name: Build and test shared packages uses: ./.github/actions/build-shared-packages with: @@ -209,20 +281,27 @@ jobs: - name: Apply every migration from scratch run: npm run migrate --prefix apps/backend/db - - name: Ledger must be clean afterwards + - name: Schema history must be clean afterwards run: | set -o pipefail - npm run migrate:status --prefix apps/backend/db | tee status.log - if grep -q 'PENDING' status.log; then - echo "::error::migrations still pending after a successful migrate run" + npm run --silent migrate:info --prefix apps/backend/db -- -outputType=json > info.json + pending=$(jq '[.migrations[] | select(.state == "Pending")] | length' info.json) + if [ "$pending" != '0' ]; then + echo "::error::$pending migration(s) still pending after a successful migrate run" + jq -r '.migrations[] | select(.state == "Pending") | .filepath' info.json exit 1 fi - name: Re-running migrations must be a no-op run: npm run migrate --prefix apps/backend/db + - name: Applied migrations must still match their checksums + run: npm run migrate:validate --prefix apps/backend/db + - name: Seeds must apply on top of the migrated schema - run: npm run seed --prefix apps/backend/db + run: | + npm run fingerprint --prefix apps/backend/db + npm run seed --prefix apps/backend/db # Drift check: the committed types must be what these migrations produce. - name: Committed types must match the migrations diff --git a/.github/workflows/regenerate-db-types.yaml b/.github/workflows/regenerate-db-types.yaml index 22e7b94f..d1585d47 100644 --- a/.github/workflows/regenerate-db-types.yaml +++ b/.github/workflows/regenerate-db-types.yaml @@ -139,6 +139,9 @@ jobs: echo "Applying db/migrations" npm run migrate --prefix apps/backend/db + # Type generation refuses a schema without this marker. + npm run fingerprint --prefix apps/backend/db + echo "Verifying schema application" psql -h localhost -U postgres -d testdb -c "\dt branch.*" env: diff --git a/apps/backend/AGENTS.md b/apps/backend/AGENTS.md index a8959e09..a189cc7c 100644 --- a/apps/backend/AGENTS.md +++ b/apps/backend/AGENTS.md @@ -47,8 +47,8 @@ http://localhost:3000//health - Schema built from `db/migrations/*.sql` (Postgres schema `branch`); dev/test rows in `db/seed.sql`. Tables: `users`, `projects`, `project_memberships` (roles: Director/Student), `donors`, `project_donations`, `expenditures` (status: approved/pending/denied/needs_more_info), `reports` (report_type: technical/narrative), plus the trigger-maintained rollups `expenditure_rollup` and `project_rollup`. - **Analytics reads go through the rollups, never the base tables.** `expenditure_rollup` (grain: project × month × category × status) and `project_rollup` (per project: member/donation/report counts and `total_donated`) are maintained by `AFTER INSERT OR UPDATE OR DELETE` row triggers, so they are exact rather than eventually consistent — there is nothing to refresh and no scheduler. Spend lives **only** in `expenditure_rollup`; do not add a `total_spent` column to `project_rollup`. Two rules when touching the triggers: increments upsert, decrements are a plain `UPDATE` (a project cascade-delete reaches `expenditures` and the rollups in an undefined order, so a zero-row `UPDATE` must be a safe no-op), and emptied buckets are left at zero rather than deleted. `TRUNCATE` fires no row triggers, so each base table also has an `AFTER TRUNCATE ... FOR EACH STATEMENT` trigger — a new base table needs both kinds. Time-relative figures (the active-project count, "spend on active projects") cannot be rolled up and still read `projects` live. - Kysely connects via `db.ts` in each lambda (`Kysely` + `pg.Pool`, env `DB_HOST/DB_PORT/DB_USER/DB_PASSWORD/DB_NAME`). Query with the `branch.` schema prefix: `db.selectFrom('branch.users')`. -- **Changing the schema:** `make new-migration NAME=add_thing` → write SQL → `make migrate` (applies it and regenerates `shared/types/db-types.d.ts`). `make show-migrations` shows applied vs pending; the ledger is `branch.kysely_migration`. Forward-only — never edit a merged migration, and never hand-edit `db-types.d.ts`. Migrations apply to prod automatically on merge **before** the lambda deploy, so a single PR may only contain additive changes; see `db/README.md` for expand/contract. -- The migration runner is kysely's `Migrator` over plain `.sql` files (`db/src/`). Root `package.json` still carries NestJS/TypeORM **dependencies** from the original scaffold, but nothing uses them; its dead `migration:*` scripts have been removed. +- **Changing the schema:** `make new-migration NAME=add_thing` → write SQL → `make migrate` (applies it and regenerates `shared/types/db-types.d.ts`). `make show-migrations` shows applied vs pending; the ledger is `branch.flyway_schema_history`. Forward-only — never edit a merged migration, and never hand-edit `db-types.d.ts`. Migrations apply to prod automatically on merge **before** the lambda deploy, so a single PR may only contain additive changes; see `db/README.md` for expand/contract. +- The migration runner is Flyway over plain `.sql` files, configured once in `db/flyway.sh`; `db/src/` holds seeding, the schema rebuild and type generation. Root `package.json` still carries NestJS/TypeORM **dependencies** from the original scaffold, but nothing uses them; its dead `migration:*` scripts have been removed. ## Shared packages diff --git a/apps/backend/Makefile b/apps/backend/Makefile index 60095c09..8134e5de 100644 --- a/apps/backend/Makefile +++ b/apps/backend/Makefile @@ -108,6 +108,7 @@ grant-admin: migrate: @docker compose --env-file $(ENV_FILE) build migrator @docker compose --env-file $(ENV_FILE) run --rm migrator npm run migrate + @docker compose --env-file $(ENV_FILE) run --rm migrator npm run fingerprint @docker compose --env-file $(ENV_FILE) run --rm migrator npm run seed -- --if-empty @$(MAKE) types @$(MAKE) show-migrations @@ -116,10 +117,10 @@ migrate: # List every migration and whether it has been applied. Read-only. # -# The source of truth is the branch.kysely_migration table -- one row per applied -# migration -- diffed against the .sql files in db/migrations. +# The source of truth is the branch.flyway_schema_history table -- one row per +# applied migration -- diffed against the .sql files in db/migrations. show-migrations: - @docker compose --env-file $(ENV_FILE) run --rm migrator npm run migrate:status + @docker compose --env-file $(ENV_FILE) run --rm migrator npm run migrate:info # Create an empty timestamped migration file, then say what to do next. # usage: make new-migration NAME=add_expenditure_project_id_index diff --git a/apps/backend/db/Dockerfile b/apps/backend/db/Dockerfile index 30b3f55f..8a513a3d 100644 --- a/apps/backend/db/Dockerfile +++ b/apps/backend/db/Dockerfile @@ -1,18 +1,17 @@ # One-shot migration runner, built from the repo root like the lambdas. -# -# A Dockerfile rather than bind-mounting the repo into node:20-alpine and running -# `npm ci` on every `up`: that would clobber the host's apps/backend/db/node_modules -# with alpine-built artifacts and cost ~30s per start. This way docker caches the -# install layer. -FROM node:20-alpine +FROM flyway/flyway:13.5.0-alpine + +RUN apk add --no-cache nodejs npm WORKDIR /db COPY apps/backend/db/package.json apps/backend/db/package-lock.json ./ RUN npm ci --no-audit --no-fund -COPY apps/backend/db/tsconfig.json apps/backend/db/testkit.ts apps/backend/db/seed.sql ./ +COPY apps/backend/db/tsconfig.json apps/backend/db/testkit.ts apps/backend/db/seed.sql apps/backend/db/flyway.sh ./ COPY apps/backend/db/src ./src COPY apps/backend/db/migrations ./migrations +# The base image entrypoint would prepend `flyway` to every compose `command:`. +ENTRYPOINT [] CMD ["npm", "run", "migrate"] diff --git a/apps/backend/db/README.md b/apps/backend/db/README.md index 01303f0a..6f355738 100644 --- a/apps/backend/db/README.md +++ b/apps/backend/db/README.md @@ -1,8 +1,8 @@ # Database migrations -Schema changes are plain `.sql` files in `migrations/`, applied by [kysely's -`Migrator`](https://kysely.dev/docs/migrations). They are applied to **production -automatically when your PR merges**. +Schema changes are plain `.sql` files in `migrations/`, applied by +[Flyway](https://documentation.red-gate.com/flyway). They are applied to +**production automatically when your PR merges**. ## Changing the schema @@ -10,7 +10,7 @@ automatically when your PR merges**. cd apps/backend make up # if the stack isn't already running -make new-migration NAME=add_expenditure_notes # creates migrations/_add_expenditure_notes.sql +make new-migration NAME=add_expenditure_notes # creates migrations/V__add_expenditure_notes.sql # ...write your SQL... make migrate # applies it, reseeds if empty, regenerates types, prints status ``` @@ -59,31 +59,53 @@ and CI will reject the change. ## How it works -- `migrations/*.sql` — applied in filename order. `make new-migration` generates a - UTC `YYYYMMDDHHMMSS_` prefix so concurrent PRs can't collide. +- `migrations/V__.sql` — applied in version order. + `make new-migration` generates a UTC `VYYYYMMDDHHMMSS__` prefix so concurrent PRs + can't collide. The double underscore is Flyway's separator and is not optional. - `seed.sql` — dev/test data only, **never applied to production**. - `testkit.ts` — `ensureSchema()` / `resetData()` used by the lambda tests. -- `src/` — the runner CLI, the type generator, and the shared post-processing that - keeps local and CI type output byte-identical. +- `flyway.sh` — the only place Flyway is configured. Uses the flyway on `PATH`, + otherwise the pinned image, so docker is the only prerequisite. +- `src/` — seeding and schema-rebuild commands, the type generator, and the shared + post-processing that keeps local and CI type output byte-identical. -**What's applied is tracked in the database**, in `branch.kysely_migration` — one row -per applied migration (`name`, `timestamp`). "Pending" is just the `.sql` files on -disk minus the rows in that table. It's an ordinary table, so in any environment: +**What's applied is tracked in the database**, in `branch.flyway_schema_history` — +one row per applied migration, including a checksum of the file. "Pending" is the +`.sql` files on disk minus the rows in that table. It's an ordinary table, so in any +environment: ```sql -select * from branch.kysely_migration order by name; +select version, description, checksum, success from branch.flyway_schema_history + order by installed_rank; ``` -All pending migrations run inside a **single transaction** with +The checksum is why a merged migration can never be edited: Flyway compares the file +against the recorded checksum on the next run and fails the deploy. + +Each migration file runs inside its **own transaction** with `search_path = branch, public`, so table names can be unqualified and a failure -part-way through rolls the whole run back. That's also why `CREATE INDEX +part-way through a file rolls that file back. That's also why `CREATE INDEX CONCURRENTLY` and `VACUUM` don't work here — they can't run in a transaction. This database is tiny; a plain `CREATE INDEX` is fine. -Out-of-order merges are allowed (`allowUnorderedMigrations`): if your migration -merges after someone whose timestamp is later, it simply applies late. The -alternative — the default — is a production deploy that fails with `corrupted -migrations` and can only be unblocked by hand-editing `kysely_migration` in RDS. +Out-of-order merges are allowed (`outOfOrder`): if your migration merges after +someone whose timestamp is later, it simply applies late. The alternative — the +default — is a production deploy that fails and can only be unblocked by +hand-editing `flyway_schema_history` in RDS. + +**`outOfOrder` does not extend below the baseline.** `BASELINE_VERSION` in +`flyway.sh` is the version production was adopted at, and Flyway reports anything at +or below it as `Below Baseline` — skipped, permanently, with nothing pending, a +passing `validate` and a green deploy. CI builds from an empty schema, where +baselining never happens, so CI cannot see it either. The `checks` job therefore +fails any new migration whose version is not strictly above `BASELINE_VERSION`. Let +`make new-migration` generate the timestamp and this never comes up. + +`${async}` in a migration is a Flyway placeholder. It expands to nothing on +PostgreSQL and to `ASYNC` on Aurora DSQL, which has no synchronous `CREATE INDEX`. +Placeholders are Flyway's, so `testkit.ts` substitutes them itself when it applies +the files directly — add any new one to `PLACEHOLDERS` there as well as to +`flyway.sh`, or the tests fail on a postgres syntax error. ## In CI @@ -108,9 +130,9 @@ manual restore-into-a-new-instance procedure, not a button: ## One-time: adopting an existing database -`0000_baseline_schema.sql` is the schema as it existed before migrations, and is the -only migration allowed to use `IF NOT EXISTS` — that's what lets it be applied to a -database that already has these tables. +`V0000__baseline_schema.sql` is the schema as it existed before migrations, and is +the only migration allowed to use `IF NOT EXISTS` — that's what lets it be applied to +a database that already has these tables. `IF NOT EXISTS` skips the **entire** `CREATE TABLE` when the table exists, so it cannot detect a column or constraint that differs. Before running the migrator @@ -119,16 +141,24 @@ against a pre-existing database, diff it: ```bash # use a pg_dump matching the server's major version pg_dump --schema-only --schema=branch --no-owner --no-privileges --no-comments \ - -T 'branch.kysely_migration*' -d "$URL" | grep -v '^--' + -T 'branch.flyway_schema_history' -d "$URL" | grep -v '^--' ``` Run that against a local database with only the baseline applied, and against the -target; `diff -u` the two. Once it's empty, run `npm run migrate` against the target -with a human watching — the baseline no-ops and records itself in the ledger. - -If the schemas genuinely diverge, use `npm run db -- stamp 0000_baseline_schema` to -record it without executing, then write a follow-up migration reconciling the -difference. - -The `migrate` job refuses to run if `0000_baseline_schema` is still pending, on the -assumption that it means `DB_HOST` is pointing somewhere unexpected. +target; `diff -u` the two. + +Once it's empty, adopt the database: Flyway writes a single baseline row at +`BASELINE_VERSION` from `flyway.sh` instead of replaying the files, and everything at +or below that version counts as applied. If the schemas genuinely diverge, write a +follow-up migration reconciling the difference — there is no way to record one file +as applied on its own. + +Adoption is **off by default and never happens on a push**: a populated schema with +no history table is far more often a restored snapshot or a clone than the real +production database, and adopting one silently is how you end up migrating the wrong +thing. Run the `Lambda Deploy` workflow by hand with `adopt_baseline` checked, once, +with a human watching. Locally, compose sets `FLYWAY_BASELINE_ON_MIGRATE=true` so a +dev database built before Flyway is adopted without ceremony. + +The `migrate` job refuses to run against an **empty** target, on the assumption that +it means `DB_HOST` is pointing somewhere unexpected. diff --git a/apps/backend/db/flyway.sh b/apps/backend/db/flyway.sh new file mode 100755 index 00000000..1f42b8d6 --- /dev/null +++ b/apps/backend/db/flyway.sh @@ -0,0 +1,94 @@ +#!/bin/sh +set -eu + +FLYWAY_IMAGE=flyway/flyway:13.5.0-alpine +DB_DIR=$(CDPATH= cd -- "$(dirname -- "$0")" && pwd) + +# Last migration applied under kysely; already on production, so Flyway baselines here. +# Also a permanent floor: anything at or below it is "Below Baseline" and never runs. +BASELINE_VERSION=20260907213524 + +# Off by default: on production, baselining would silently adopt an unrecognised database. +BASELINE_ON_MIGRATE=${FLYWAY_BASELINE_ON_MIGRATE:-false} + +# Percent-encoded credentials in DATABASE_URL are not decoded -- use DB_USER/DB_PASSWORD. +if [ -n "${DATABASE_URL:-}" ]; then + rest=${DATABASE_URL#*://} + case "$rest" in + *@*) creds=${rest%%@*} ; hostpath=${rest#*@} ;; + *) creds='' ; hostpath=$rest ;; + esac + db_user=${creds%%:*} + case "$creds" in *:*) db_password=${creds#*:} ;; *) db_password='' ;; esac + hostport=${hostpath%%/*} + db_name=${hostpath#*/} + db_name=${db_name%%\?*} + db_host=${hostport%%:*} + case "$hostport" in *:*) db_port=${hostport#*:} ;; *) db_port=5432 ;; esac +else + db_user=${DB_USER:-branch_dev} + db_password=${DB_PASSWORD:-password} + db_host=${DB_HOST:-localhost} + db_port=${DB_PORT:-5432} + db_name=${DB_NAME:-branch_db} +fi + +ca_path=${DB_SSL_CA:-} +if [ -n "$ca_path" ]; then + ssl='?sslmode=verify-full&sslrootcert=' +elif [ "${DB_SSL:-}" = 'true' ]; then + ssl='?sslmode=require' +else + ssl='' +fi + +# outOfOrder: PRs merge out of timestamp order. async: Aurora DSQL has no synchronous CREATE INDEX. +add_flags() { + set -- \ + "-locations=filesystem:$1" \ + -schemas=branch \ + -defaultSchema=branch \ + -createSchemas=true \ + -outOfOrder=true \ + "-baselineOnMigrate=$BASELINE_ON_MIGRATE" \ + "-baselineVersion=$BASELINE_VERSION" \ + -validateMigrationNaming=true \ + -cleanDisabled=true \ + "-placeholders.async=${FLYWAY_PLACEHOLDER_ASYNC:-}" + printf '%s\n' "$@" +} + +if command -v flyway >/dev/null 2>&1; then + [ -z "$ca_path" ] || ssl="${ssl}${ca_path}" + # shellcheck disable=SC2046 # deliberate word splitting: one flag per line + set -- flyway $(add_flags "$DB_DIR/migrations") "$@" +else + ca_mount='' + if [ -n "$ca_path" ]; then + ssl="${ssl}/rds-ca.pem" + ca_mount="--volume=$ca_path:/rds-ca.pem:ro" + fi + # Docker Desktop ignores --network=host unless it is switched on, so a loopback + # host has to go via the gateway. Only there: on a native daemon --network=host + # is real, and the gateway address misses a server bound to loopback only. + if docker info --format '{{.OperatingSystem}}' 2>/dev/null | grep -qi 'docker desktop'; then + case "$db_host" in + localhost | 127.0.0.1 | ::1) db_host=host.docker.internal ;; + esac + fi + # shellcheck disable=SC2046 + set -- docker run --rm --network=host \ + --add-host=host.docker.internal:host-gateway \ + --volume="$DB_DIR/migrations:/db/migrations:ro" \ + --env=FLYWAY_URL --env=FLYWAY_USER --env=FLYWAY_PASSWORD \ + ${ca_mount:+"$ca_mount"} "$FLYWAY_IMAGE" \ + $(add_flags /db/migrations) "$@" +fi + +# Credentials via environment, never argv: `docker run` args are world-visible. +FLYWAY_URL="jdbc:postgresql://${db_host}:${db_port}/${db_name}${ssl}" +FLYWAY_USER=$db_user +FLYWAY_PASSWORD=$db_password +export FLYWAY_URL FLYWAY_USER FLYWAY_PASSWORD + +exec "$@" diff --git a/apps/backend/db/migrations/0000_baseline_schema.sql b/apps/backend/db/migrations/V0000__baseline_schema.sql similarity index 100% rename from apps/backend/db/migrations/0000_baseline_schema.sql rename to apps/backend/db/migrations/V0000__baseline_schema.sql diff --git a/apps/backend/db/migrations/20260812011405_rename_project_roles.sql b/apps/backend/db/migrations/V20260812011405__rename_project_roles.sql similarity index 100% rename from apps/backend/db/migrations/20260812011405_rename_project_roles.sql rename to apps/backend/db/migrations/V20260812011405__rename_project_roles.sql diff --git a/apps/backend/db/migrations/20260812012951_add_expenditure_admin_notes.sql b/apps/backend/db/migrations/V20260812012951__add_expenditure_admin_notes.sql similarity index 100% rename from apps/backend/db/migrations/20260812012951_add_expenditure_admin_notes.sql rename to apps/backend/db/migrations/V20260812012951__add_expenditure_admin_notes.sql diff --git a/apps/backend/db/migrations/20260812022651_add_access_pattern_indexes.sql b/apps/backend/db/migrations/V20260812022651__add_access_pattern_indexes.sql similarity index 100% rename from apps/backend/db/migrations/20260812022651_add_access_pattern_indexes.sql rename to apps/backend/db/migrations/V20260812022651__add_access_pattern_indexes.sql diff --git a/apps/backend/db/migrations/20260823054531_add_followup_indexes.sql b/apps/backend/db/migrations/V20260823054531__add_followup_indexes.sql similarity index 100% rename from apps/backend/db/migrations/20260823054531_add_followup_indexes.sql rename to apps/backend/db/migrations/V20260823054531__add_followup_indexes.sql diff --git a/apps/backend/db/migrations/20260823055243_add_analytics_rollups.sql b/apps/backend/db/migrations/V20260823055243__add_analytics_rollups.sql similarity index 100% rename from apps/backend/db/migrations/20260823055243_add_analytics_rollups.sql rename to apps/backend/db/migrations/V20260823055243__add_analytics_rollups.sql diff --git a/apps/backend/db/migrations/20260825023851_drop_project_admin_role.sql b/apps/backend/db/migrations/V20260825023851__drop_project_admin_role.sql similarity index 100% rename from apps/backend/db/migrations/20260825023851_drop_project_admin_role.sql rename to apps/backend/db/migrations/V20260825023851__drop_project_admin_role.sql diff --git a/apps/backend/db/migrations/20260906215733_move_rollups_to_application.sql b/apps/backend/db/migrations/V20260906215733__move_rollups_to_application.sql similarity index 100% rename from apps/backend/db/migrations/20260906215733_move_rollups_to_application.sql rename to apps/backend/db/migrations/V20260906215733__move_rollups_to_application.sql diff --git a/apps/backend/db/migrations/20260907213524_project_rollup_bump_reports_hit.sql b/apps/backend/db/migrations/V20260907213524__project_rollup_bump_reports_hit.sql similarity index 100% rename from apps/backend/db/migrations/20260907213524_project_rollup_bump_reports_hit.sql rename to apps/backend/db/migrations/V20260907213524__project_rollup_bump_reports_hit.sql diff --git a/apps/backend/db/package-lock.json b/apps/backend/db/package-lock.json index 649f6761..835bcfac 100644 --- a/apps/backend/db/package-lock.json +++ b/apps/backend/db/package-lock.json @@ -9,12 +9,12 @@ "version": "1.0.0", "license": "ISC", "dependencies": { - "kysely": "^0.28.8", "pg": "^8.16.3" }, "devDependencies": { "@types/node": "^20.11.30", "@types/pg": "^8.15.6", + "kysely": "^0.28.8", "kysely-codegen": "^0.19.0", "ts-node": "^10.9.2", "typescript": "^5.9.3" @@ -663,6 +663,7 @@ "version": "0.28.8", "resolved": "https://registry.npmjs.org/kysely/-/kysely-0.28.8.tgz", "integrity": "sha512-QUOgl5ZrS9IRuhq5FvOKFSsD/3+IA6MLE81/bOOTRA/YQpKDza2sFdN5g6JCB9BOpqMJDGefLCQ9F12hRS13TA==", + "dev": true, "license": "MIT", "engines": { "node": ">=20.0.0" diff --git a/apps/backend/db/package.json b/apps/backend/db/package.json index 98472569..d5efeb91 100644 --- a/apps/backend/db/package.json +++ b/apps/backend/db/package.json @@ -5,21 +5,23 @@ "description": "Database migrations, seed data and type generation for BRANCH.", "scripts": { "db": "ts-node --transpile-only src/cli.ts", - "migrate": "npm run db -- up", - "migrate:status": "npm run db -- status", + "migrate": "./flyway.sh migrate", + "migrate:info": "./flyway.sh info", + "migrate:validate": "./flyway.sh validate", "migrate:new": "npm run db -- new", "seed": "npm run db -- seed", "reset": "npm run db -- reset", + "fingerprint": "npm run db -- fingerprint", "types": "ts-node --transpile-only src/generate-types.ts" }, "license": "ISC", "dependencies": { - "kysely": "^0.28.8", "pg": "^8.16.3" }, "devDependencies": { "@types/node": "^20.11.30", "@types/pg": "^8.15.6", + "kysely": "^0.28.8", "kysely-codegen": "^0.19.0", "ts-node": "^10.9.2", "typescript": "^5.9.3" diff --git a/apps/backend/db/src/cli.ts b/apps/backend/db/src/cli.ts index 1db07d8b..1fb3bd9b 100644 --- a/apps/backend/db/src/cli.ts +++ b/apps/backend/db/src/cli.ts @@ -1,25 +1,22 @@ /** - * db CLI -- the single entrypoint for everything schema related. + * db CLI -- the schema-adjacent commands that are not migrations themselves. * - * npm run migrate apply every pending migration - * npm run migrate:status list applied/pending - * npm run migrate:new -- x scaffold migrations/_x.sql + * npm run migrate apply every pending migration (flyway.sh migrate) + * npm run migrate:info list applied/pending (flyway.sh info) + * npm run migrate:new -- x scaffold migrations/V__x.sql * npm run seed truncate + re-apply seed.sql * npm run seed -- --if-empty seed only an empty database * npm run reset rebuild the schema from migrations, then seed - * npm run db -- stamp NAME record a migration as applied WITHOUT running it + * npm run fingerprint record which migrations built the live schema * - * The Makefile in apps/backend wraps these; CI calls them directly. + * Migrations are Flyway's job -- see flyway.sh. The Makefile in apps/backend + * wraps all of this; CI calls it directly. */ import fs from 'node:fs'; import path from 'node:path'; -import { sql } from 'kysely'; import type { PoolClient } from 'pg'; -import { SCHEMA, createPool, describeTarget } from './config'; -import { createDb, createMigrator } from './migrator'; -import { MIGRATIONS_DIR } from './provider'; +import { MIGRATIONS_DIR, createPool, describeTarget } from './config'; import { - migrationsFingerprint, rebuildSchema, resetData, seedIfEmpty, @@ -28,9 +25,9 @@ import { const TEMPLATE = `-- __NAME__ -- --- Every pending migration runs inside a SINGLE transaction, with --- search_path = branch, public -- so table names can be unqualified, and --- CREATE INDEX CONCURRENTLY / VACUUM will not work here. +-- Flyway runs this file inside a SINGLE transaction, with search_path = +-- branch, public -- so table names can be unqualified, and CREATE INDEX +-- CONCURRENTLY / VACUUM will not work here. -- -- This migration is applied to PRODUCTION automatically when the PR merges, -- BEFORE the new lambda code is deployed. It must be safe for the code that is @@ -38,68 +35,12 @@ const TEMPLATE = `-- __NAME__ -- expand/contract rules that destructive changes need. -- -- Forward-only: there is no rollback. Fix a mistake with a new migration, and --- never edit a migration that has been merged -- someone has already run it. --- Do not use IF NOT EXISTS: you want a failure, not silent drift. +-- never edit a migration that has been merged -- Flyway checksums it, and +-- someone has already run it. Do not use IF NOT EXISTS: you want a failure, not +-- silent drift. `; -async function up(): Promise { - const db = createDb(); - try { - console.log(`migrating ${describeTarget()}`); - // migrateToLatest NEVER throws -- it returns the error. Not checking it is - // the classic way to ship a green deploy that migrated nothing. - const { error, results } = await createMigrator(db).migrateToLatest(); - - for (const result of results ?? []) { - console.log(`${result.status.padEnd(11)} ${result.migrationName}`); - } - - if (error) { - console.error('\nmigration failed, nothing was applied:'); - console.error(error); - process.exitCode = 1; - return; - } - - if (!results?.length) console.log('database is already up to date'); - - // Lets db/testkit.ts skip its rebuild when the live schema already matches - // the migration files on disk. Value is a hex digest, safe to inline. - await sql - .raw( - `comment on schema ${SCHEMA} is 'migrations:${migrationsFingerprint()}'`, - ) - .execute(db); - } finally { - await db.destroy(); - } -} - -async function status(): Promise { - const db = createDb(); - try { - const migrations = await createMigrator(db).getMigrations(); - if (!migrations.length) { - console.log(`no *.sql files in ${MIGRATIONS_DIR}`); - return; - } - - console.log(`${describeTarget()}\n`); - for (const migration of migrations) { - const applied = migration.executedAt?.toISOString() ?? 'PENDING'; - console.log(`${applied.padEnd(26)} ${migration.name}`); - } - - const pending = migrations.filter( - (migration) => !migration.executedAt, - ).length; - console.log(`\n${migrations.length - pending} applied, ${pending} pending`); - } finally { - await db.destroy(); - } -} - function newMigration(name?: string): void { if (!name || !/^[a-z0-9]+(_[a-z0-9]+)*$/.test(name)) { console.error( @@ -113,65 +54,14 @@ function newMigration(name?: string): void { // UTC YYYYMMDDHHMMSS, generated so nobody hand-types one: collisions between // concurrent PRs are then effectively impossible. const stamp = new Date().toISOString().replace(/[-:T]/g, '').slice(0, 14); - const file = path.join(MIGRATIONS_DIR, `${stamp}_${name}.sql`); + const file = path.join(MIGRATIONS_DIR, `V${stamp}__${name}.sql`); - fs.writeFileSync(file, TEMPLATE.replace('__NAME__', `${stamp}_${name}`), { + fs.writeFileSync(file, TEMPLATE.replace('__NAME__', `V${stamp}__${name}`), { flag: 'wx', }); console.log(`created ${path.relative(process.cwd(), file)}`); } -/** - * Record a migration as applied WITHOUT running it. Only for adopting a database - * whose schema was built by hand -- verify with pg_dump first. The table - * definitions below are copied from kysely's Migrator so a later migrate run - * finds exactly what it expects. - */ -async function stamp(name?: string): Promise { - if (!name) { - console.error('usage: npm run db -- stamp 0000_baseline_schema'); - process.exitCode = 1; - return; - } - - const db = createDb(); - try { - await sql.raw(`create schema if not exists ${SCHEMA}`).execute(db); - await sql - .raw( - `create table if not exists ${SCHEMA}.kysely_migration ( - name varchar(255) not null primary key, - "timestamp" varchar(255) not null)`, - ) - .execute(db); - await sql - .raw( - `create table if not exists ${SCHEMA}.kysely_migration_lock ( - id varchar(255) not null primary key, - is_locked integer not null default 0)`, - ) - .execute(db); - await sql - .raw( - `insert into ${SCHEMA}.kysely_migration_lock (id, is_locked) - values ('migration_lock', 0) on conflict (id) do nothing`, - ) - .execute(db); - await sql - .raw( - `insert into ${SCHEMA}.kysely_migration (name, "timestamp") - values ('${name}', '${new Date().toISOString()}')`, - ) - .execute(db); - - console.log( - `stamped ${name} as applied (not executed) on ${describeTarget()}`, - ); - } finally { - await db.destroy(); - } -} - async function withClient( fn: (client: PoolClient) => Promise, ): Promise { @@ -189,14 +79,8 @@ async function main(): Promise { const [command, ...args] = process.argv.slice(2); switch (command) { - case 'up': - return up(); - case 'status': - return status(); case 'new': return newMigration(args[0]); - case 'stamp': - return stamp(args[0]); case 'seed': return withClient(async (client) => { if (args.includes('--if-empty')) { @@ -214,13 +98,15 @@ async function main(): Promise { return withClient(async (client) => { await rebuildSchema(client); await resetData(client); - await stampFingerprint(client); console.log('schema rebuilt from migrations and reseeded'); }); + case 'fingerprint': + return withClient(async (client) => { + await stampFingerprint(client); + console.log(`fingerprinted ${describeTarget()}`); + }); default: - console.error( - 'usage: db ', - ); + console.error('usage: db '); process.exitCode = 1; } } diff --git a/apps/backend/db/src/config.ts b/apps/backend/db/src/config.ts index f856de51..a2b95271 100644 --- a/apps/backend/db/src/config.ts +++ b/apps/backend/db/src/config.ts @@ -1,9 +1,13 @@ import fs from 'node:fs'; +import path from 'node:path'; import { Pool } from 'pg'; /** Every table lives in this schema; the generated DB types key off it. */ export const SCHEMA = 'branch'; +/** Flyway is pointed at the same directory by flyway.sh. */ +export const MIGRATIONS_DIR = path.join(__dirname, '..', 'migrations'); + /** * The one place that knows how to reach the database. * diff --git a/apps/backend/db/src/generate-types.ts b/apps/backend/db/src/generate-types.ts index c41dc5a6..6b5becf6 100644 --- a/apps/backend/db/src/generate-types.ts +++ b/apps/backend/db/src/generate-types.ts @@ -12,7 +12,7 @@ import fs from 'node:fs'; import os from 'node:os'; import path from 'node:path'; import { createPool, databaseUrl } from './config'; -import { assertSchemaIsCurrent } from '../testkit'; +import { HISTORY_TABLE, assertSchemaIsCurrent } from '../testkit'; import { postprocess } from './postprocess-types'; /** @@ -47,14 +47,16 @@ async function main(): Promise { databaseUrl(), '--dialect', 'postgres', - // Ignore anything created outside schema "branch". kysely already excludes - // kysely_migration / kysely_migration_lock by name. + // Ignore anything created outside schema "branch". // // Deliberately NOT --default-schema: that would strip the `branch.` prefix // from the generated DB keys and break every db.selectFrom('branch.users') // in all six lambdas. '--include-pattern', 'branch.*', + // kysely-codegen only skips kysely_migration% by name, not Flyway's table. + '--exclude-pattern', + `branch.${HISTORY_TABLE}`, '--out-file', tmp, ], diff --git a/apps/backend/db/src/migrator.ts b/apps/backend/db/src/migrator.ts deleted file mode 100644 index b7d6393f..00000000 --- a/apps/backend/db/src/migrator.ts +++ /dev/null @@ -1,52 +0,0 @@ -import { CompiledQuery, Kysely, Migrator, PostgresDialect } from 'kysely'; -import { SCHEMA, createPool } from './config'; -import { SqlFileMigrationProvider } from './provider'; - -export function createDb(): Kysely { - return new Kysely({ - dialect: new PostgresDialect({ - pool: createPool(), - // Awaited by PostgresDriver before the connection is handed out, so every - // migration runs with search_path=branch and may use unqualified table - // names. Doing this with a `SET` inside a migration file instead would - // leak the setting onto a pooled connection. - onCreateConnection: async (connection) => { - await connection.executeQuery( - CompiledQuery.raw(`set search_path to ${SCHEMA}, public`), - ); - }, - }), - }); -} - -export function createMigrator(db: Kysely): Migrator { - return new Migrator({ - db, - provider: new SqlFileMigrationProvider(), - - // Bookkeeping lives beside the tables it tracks: branch.kysely_migration - // (one row per applied migration) and branch.kysely_migration_lock. - // - // Keep the DEFAULT table names. kysely-codegen's introspector skips tables - // named kysely_migration / kysely_migration_lock in every schema, so they - // stay out of shared/types/db-types.d.ts for free. Renaming them to - // something like schema_migrations would leak the interfaces into the - // shared types package. - migrationTableSchema: SCHEMA, - - // Two contributors' migrations will sometimes merge out of timestamp order - // (Alice authors first, Bob merges first). With the default `false` that - // makes the next production deploy fail with "corrupted migrations", - // fixable only by renaming the file and hand-editing kysely_migration in - // RDS. With `true` the late migration is simply applied late. Deletions and - // renames of already-applied migrations are still rejected either way, which - // is the guard that actually matters. - allowUnorderedMigrations: true, - - // Byte-order comparison, matching the .sort() the provider uses to order - // files. The default is localeCompare, whose handling of `_` can disagree - // with byte order and raise a spurious ordering error when two migrations - // share a timestamp. - nameComparator: (a, b) => (a < b ? -1 : a > b ? 1 : 0), - }); -} diff --git a/apps/backend/db/src/provider.ts b/apps/backend/db/src/provider.ts deleted file mode 100644 index 375ce792..00000000 --- a/apps/backend/db/src/provider.ts +++ /dev/null @@ -1,52 +0,0 @@ -import fs from 'node:fs'; -import path from 'node:path'; -import { sql } from 'kysely'; -import type { Kysely, Migration, MigrationProvider } from 'kysely'; - -export const MIGRATIONS_DIR = path.join(__dirname, '..', 'migrations'); - -/** Byte-order sort, matching what kysely uses internally to order migrations. */ -export function migrationFilenames(dir: string = MIGRATIONS_DIR): string[] { - return fs - .readdirSync(dir) - .filter((file) => file.endsWith('.sql')) - .sort(); -} - -/** - * Serves every `*.sql` file in db/migrations to kysely's Migrator. The migration - * name is the filename without `.sql`, and kysely orders by that name, so the - * numeric prefix defines the order. - * - * Forward-only by design: kysely's migrateDown silently does nothing for a - * migration with no `down` (it leaves the tracking row in place), so we never - * define one and never expose a rollback command. Undo by writing a new - * migration. - */ -export class SqlFileMigrationProvider implements MigrationProvider { - constructor(private readonly dir: string = MIGRATIONS_DIR) {} - - async getMigrations(): Promise> { - const migrations: Record = {}; - - for (const file of migrationFilenames(this.dir)) { - const name = file.slice(0, -'.sql'.length); - if (name.length > 255) { - throw new Error( - `migration name is longer than kysely_migration.name (varchar(255)): ${file}`, - ); - } - - const contents = fs.readFileSync(path.join(this.dir, file), 'utf8'); - migrations[name] = { - // node-postgres sends parameterless queries over the simple query - // protocol, so one file may hold many `;`-separated statements. - up: async (db: Kysely) => { - await sql.raw(contents).execute(db); - }, - }; - } - - return migrations; - } -} diff --git a/apps/backend/db/testkit.ts b/apps/backend/db/testkit.ts index 9546f960..43266b8e 100644 --- a/apps/backend/db/testkit.ts +++ b/apps/backend/db/testkit.ts @@ -26,9 +26,9 @@ export interface Queryable { } /** - * Byte-order sort, matching the sort kysely's Migrator uses to order migrations. - * If these two ever disagree, tests and production apply migrations in different - * orders. + * Byte-order sort. Flyway orders by parsed version rather than by filename, but + * `V__` sorts identically either way. If that ever stops + * being true, tests and production apply migrations in different orders. */ function migrationFiles(): string[] { return fs @@ -37,14 +37,29 @@ function migrationFiles(): string[] { .sort(); } +// Must stay in step with the `-placeholders.*` flags in flyway.sh; tests apply the SQL directly. +const PLACEHOLDERS: Record = { async: '' }; + +function substitutePlaceholders(sql: string, file: string): string { + return sql.replace(/\$\{(\w+)\}/g, (match, name: string) => { + const value = PLACEHOLDERS[name]; + if (value === undefined) { + throw new Error( + `${file}: unknown Flyway placeholder ${match} -- define it in flyway.sh and in PLACEHOLDERS here`, + ); + } + return value; + }); +} + let allSql: string | undefined; function allMigrationSql(): string { allSql ??= migrationFiles() .map( (file) => - `-- ${file}\n${fs.readFileSync( - path.join(MIGRATIONS_DIR, file), - 'utf8', + `-- ${file}\n${substitutePlaceholders( + fs.readFileSync(path.join(MIGRATIONS_DIR, file), 'utf8'), + file, )}`, ) .join('\n'); @@ -62,41 +77,56 @@ export function migrationsFingerprint(): string { return crypto.createHash('sha256').update(allMigrationSql()).digest('hex'); } +export const HISTORY_TABLE = 'flyway_schema_history'; + /** - * Writes kysely's migration ledger as if every migration file had just been - * applied by the Migrator: branch.kysely_migration with one row per file, plus - * branch.kysely_migration_lock. The DDL is copied from kysely's Migrator so a - * later `npm run migrate` finds exactly what it expects. + * Writes Flyway's history as if every migration file had just been applied. The + * DDL is copied from what Flyway itself creates on PostgreSQL, so a later + * `npm run migrate` finds exactly what it expects. * * Without this, rebuildSchema() would leave a schema whose tables exist but - * whose ledger is empty -- and since the dev stack and the tests share one local - * database, the next `make migrate` would try to re-apply every migration and - * fail on "already exists". + * whose history is empty -- and since the dev stack and the tests share one + * local database, the next `make migrate` would baseline at the adoption + * version and re-apply every migration written since, failing on "already + * exists". */ export async function stampLedger(client: Queryable): Promise { await client.query( - `CREATE TABLE IF NOT EXISTS ${SCHEMA}.kysely_migration ( - name varchar(255) NOT NULL PRIMARY KEY, - "timestamp" varchar(255) NOT NULL)`, + `CREATE TABLE IF NOT EXISTS ${SCHEMA}.${HISTORY_TABLE} ( + installed_rank integer NOT NULL PRIMARY KEY, + version varchar(50), + description varchar(200) NOT NULL, + type varchar(20) NOT NULL, + script varchar(1000) NOT NULL, + checksum integer, + installed_by varchar(100) NOT NULL, + installed_on timestamp NOT NULL DEFAULT now(), + execution_time integer NOT NULL, + success boolean NOT NULL)`, ); await client.query( - `CREATE TABLE IF NOT EXISTS ${SCHEMA}.kysely_migration_lock ( - id varchar(255) NOT NULL PRIMARY KEY, - is_locked integer NOT NULL DEFAULT 0)`, - ); - await client.query( - `INSERT INTO ${SCHEMA}.kysely_migration_lock (id, is_locked) - VALUES ('migration_lock', 0) ON CONFLICT (id) DO NOTHING`, + `CREATE INDEX IF NOT EXISTS ${HISTORY_TABLE}_s_idx + ON ${SCHEMA}.${HISTORY_TABLE} (success)`, ); - const now = new Date().toISOString(); + // Flyway stores the description with the underscores turned back into spaces. const rows = migrationFiles() - .map((file) => `('${file.slice(0, -'.sql'.length)}', '${now}')`) + .map((file, index) => { + const [version, description] = file + .slice(1, -'.sql'.length) + .split('__', 2); + return ( + `(${index + 1}, '${version}', '${description.replace(/_/g, ' ')}',` + + ` 'SQL', '${file}', NULL, current_user, 0, TRUE)` + ); + }) .join(', '); if (rows) { await client.query( - `INSERT INTO ${SCHEMA}.kysely_migration (name, "timestamp") - VALUES ${rows} ON CONFLICT (name) DO NOTHING`, + `INSERT INTO ${SCHEMA}.${HISTORY_TABLE} + (installed_rank, version, description, type, script, checksum, + installed_by, execution_time, success) + VALUES ${rows} ON CONFLICT (installed_rank) DO NOTHING`, ); } } @@ -114,8 +144,8 @@ export async function rebuildSchema(client: Queryable): Promise { await client.query(`SET search_path TO ${SCHEMA}, public`); try { // A parameterless multi-statement query goes over the simple query protocol - // and runs as one implicit transaction -- the same all-or-nothing semantics - // kysely's Migrator gives us in production. + // and runs as one implicit transaction. Flyway commits per file rather than + // per run, so this is stricter than production, not looser. await client.query(allMigrationSql()); } finally { await client.query('RESET search_path'); @@ -168,7 +198,7 @@ async function truncateAll(client: Queryable): Promise { const { rows } = await client.query( `SELECT quote_ident(schemaname) || '.' || quote_ident(tablename) AS t FROM pg_tables - WHERE schemaname = '${SCHEMA}' AND tablename NOT LIKE 'kysely_migration%' + WHERE schemaname = '${SCHEMA}' AND tablename <> '${HISTORY_TABLE}' ORDER BY tablename`, ); const tables = (rows ?? []).map((row) => row.t as string); diff --git a/apps/backend/docker-compose.yml b/apps/backend/docker-compose.yml index c2d83a59..27656482 100644 --- a/apps/backend/docker-compose.yml +++ b/apps/backend/docker-compose.yml @@ -43,7 +43,9 @@ services: DB_USER: ${DB_USER:-branch_dev} DB_PASSWORD: ${DB_PASSWORD:-password} DB_NAME: ${DB_NAME:-branch_db} - command: sh -c "npm run migrate && npm run seed -- --if-empty" + # Local only: adopt a dev database built before Flyway. + FLYWAY_BASELINE_ON_MIGRATE: 'true' + command: sh -c "npm run migrate && npm run fingerprint && npm run seed -- --if-empty" depends_on: postgres: condition: service_healthy