feat(db): automated migrations, applied to prod in CI and runnable locally - #293
Merged
Conversation
…cally
The whole schema was one file, apps/backend/db/db_setup.sql, whose first line
was `DROP SCHEMA IF EXISTS branch CASCADE;`. There was no migration system --
the root package.json's TypeORM migration:* scripts pointed at
apps/backend/src/data-source.ts, which does not exist. Consequently nothing
applied schema changes to production: RDS was built by hand, and running the
actual file against it would have destroyed the database. Locally a schema
change needed `make down-v`, because db_setup.sql was mounted at
docker-entrypoint-initdb.d and only ran on an empty volume. Types could only be
regenerated by CI.
Migrations are now plain .sql files in apps/backend/db/migrations/, applied by
kysely's Migrator and tracked in branch.kysely_migration. Chosen over a
hand-rolled runner for the pg advisory lock, all-or-nothing transaction
semantics, and detection of deleted or renamed applied migrations; chosen over
kysely's TS migration files so contributors write the SQL they already know and
so the baseline is a copy of db_setup.sql rather than a hand-translation of it.
Contributor flow, two commands:
make new-migration NAME=add_thing # generates migrations/<utc>_add_thing.sql
make migrate # applies, seeds if empty, regenerates types
make migrate folds in type regeneration so it cannot be forgotten, and
make show-migrations reports applied vs pending from the ledger. Both run in the
migrator container rather than on the host: on the host, localhost:5432 may
resolve to a separate postgres install and silently generate types from the
wrong database.
In CI, a new `migrate` job in lambda-deploy.yml snapshots RDS and applies
pending migrations on merge to main, and `deploy` needs it -- so a failed
migration leaves production on the old code AND the old schema. It authenticates
with a new, narrowly scoped branch-ci-migrate OIDC role (assumable only from the
new production-db environment) and reads the DB connection off the deployed
branch-auth lambda config, the same trick preview-env.yml already uses, so no
copy of the production password is created anywhere. TLS is verified against the
AWS CA bundle. The environment has no required reviewers on purpose: a pending
approval would strand a merged PR with its schema applied and its code
undeployed.
PR-time gates fold into the existing lambda-tests check, so branch protection
needs no change: migrations-fresh applies every migration to an empty Postgres,
asserts idempotency, and fails if the committed types are stale;
migrations-guard rejects edits to already-merged migrations, bad filenames, and
statements that would break the currently deployed code. A sticky bot comment
tells the author to apply and test locally, keep the migration separate from
unrelated changes, and follow expand/contract.
Also here:
- db_setup.sql is split into migrations/0000_baseline_schema.sql (idempotent, so
it adopts the hand-built production database) and seed.sql (dev/test only,
never applied to prod). Verified lossless: pg_dump of the old file and of
migrations+seed are byte-identical for both schema and data, modulo
CURRENT_TIMESTAMP defaults.
- The 9 test files that re-ran db_setup.sql per test now use db/testkit.ts:
ensureSchema() in beforeAll rebuilds only when the migration files changed
(fingerprint stored as a schema comment), resetData() in beforeEach truncates
with RESTART IDENTITY and reseeds. Faster than before (no DDL per test), and
the truncate list is discovered from pg_tables so future tables reset
automatically.
- A one-shot `migrator` compose service applies migrations on every `make up`;
the six lambdas gate on service_completed_successfully. `make up` switches to
--abort-on-container-failure, since --abort-on-container-exit would tear the
stack down the moment the one-shot service succeeded.
- regenerate-db-types.yaml triggers off migrations/**, builds the schema via the
migrator, and calls the same `npm run types` a contributor runs -- the
kysely-ColumnType rewrite moves out of an inline node -e heredoc into
db/src/postprocess-types.ts, so local and CI output are identical by
construction. Node 23 -> 20 to match the nodejs20.x runtime.
- RDS gains backup_retention_period = 7 (it was unset, so PITR was whatever the
API defaulted to) — free at this storage size, and the backstop for automated
migrations.
- Removes the dead TypeORM scripts and the unused kysely-codegen.config.js,
whose outFile pointed at a nonexistent path.
Docs: new apps/backend/db/README.md covers the expand/contract rules and the
one-time production adoption procedure; AGENTS.md, READMEs, and the preview-env
comments are updated (previews still never migrate, and now say why, plus the
consequence that a schema PR cannot be fully previewed).
Verified locally: full flow from an empty volume; baseline adopts a pre-existing
schema as a no-op without touching its rows; a second migration applies, is
idempotent, and regenerates types; deleting an applied migration fails loudly;
452 tests pass across all six lambdas. The 4 remaining failures need the lambda
dev server on port 3000 and fail identically before and after this change.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Contributor
This PR contains a database migration
It will be applied to the production database automatically when this PR merges, before the new lambda code is deployed. Please confirm before requesting review:
This PR also changes 32 files outside |
- Auto-formatted .tf files with terraform fmt - Updated README.md with terraform-docs Co-authored-by: nourshoreibah <nourshoreibah@users.noreply.github.com>
Contributor
|
Database Types Check Complete The database schema files were modified, but the regenerated TypeScript types are identical to the existing ones. No changes were needed and the type definitions are already up to date. |
1 similar comment
Contributor
|
Database Types Check Complete The database schema files were modified, but the regenerated TypeScript types are identical to the existing ones. No changes were needed and the type definitions are already up to date. |
Manual snapshots never expire on their own and survive instance deletion, so the migrate job has to prune them. Five is plenty: anything older than the last few migrations is better served by the 7-day point-in-time recovery window that the same PR turns on, and neither lever is a one-click restore anyway. Also documents both rollback levers in db/README.md, since the snapshot and PITR cover different things (a named commit-tagged restore point vs any second in a 7-day window). Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Contributor
Terraform Plan 📖
|
Contributor
|
Database Types Check Complete The database schema files were modified, but the regenerated TypeScript types are identical to the existing ones. No changes were needed and the type definitions are already up to date. |
Contributor
Terraform Plan 📖
|
nourshoreibah
marked this pull request as ready for review
July 29, 2026 04:42
nourshoreibah
added a commit
that referenced
this pull request
Jul 29, 2026
terraform-apply failed on main after #293: InvalidParameterValue: The backup window and maintenance window must not overlap. `backup_retention_period = 0 -> 7` was the change that mattered — production had no point-in-time recovery. The `backup_window = "07:00-08:00"` next to it was an unnecessary timing tweak, and AWS assigns both the backup and maintenance windows without Terraform managing either, so pinning one guesses against the other. Dropping it. The windows AWS assigned already don't overlap, and a backup of a few MB on a db.t3.micro is seconds of I/O, so there was nothing to gain. If specific times are ever wanted, pin backup_window and maintenance_window together. The failed apply had already created aws_iam_role.ci_migrate before erroring; re-applying reconciles the rest, nothing needs manual cleanup. Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Why
The whole schema was one file,
apps/backend/db/db_setup.sql, whose first line wasDROP SCHEMA IF EXISTS branch CASCADE;. There was no migration system — the rootpackage.json's TypeORMmigration:*scripts pointed atapps/backend/src/data-source.ts, which doesn't exist.So:
make down-vlocally, becausedb_setup.sqlwas mounted atdocker-entrypoint-initdb.dand only ran on an empty volume.BACKEND_ANALYSIS.mditem 20 already flagged all of this; this PR closes it.What
Migrations are plain
.sqlfiles inapps/backend/db/migrations/, applied by kysely'sMigratorand tracked inbranch.kysely_migration(one row per applied migration).Contributor flow is two commands:
make migratefolds in type regeneration so it can't be forgotten.make show-migrationsreports applied vs pending. Both run inside the migrator container — on the host,localhost:5432can resolve to a separate postgres install and silently generate types from the wrong database (this bit me while building it).In CI, a new
migratejob inlambda-deploy.ymlsnapshots RDS and applies pending migrations on merge to main, anddeployneeds it — so a failed migration leaves production on the old code and the old schema. Schema-newer-than-code is possible; code-newer-than-schema is not.Review notes
Why kysely's Migrator and not a hand-rolled runner? The ~30-line provider buys
pg_advisory_xact_lock(two concurrent deploys can't interleave), all-or-nothing transaction semantics, and#ensureNoMissingMigrations. That last one I verified fires: deleting an applied migration errors withcorrupted migrations: previously executed migration ... is missingrather than silently re-applying.Why plain SQL and not kysely's TS migration files? Contributors write the SQL they already know, no TS toolchain is needed wherever migrations run, and the baseline is a copy of
db_setup.sqlrather than a hand-translation — a translation is a chance to silently diverge from the hand-built production database.Credentials. The migrate job uses a new, narrowly scoped
branch-ci-migrateOIDC role, assumable only from the newproduction-dbenvironment, and readsDB_HOST/USER/PASSWORD/NAMEoff the deployedbranch-authlambda config — the same trickpreview-env.ymlalready uses. No copy of the production password is created anywhere, so rotating it in Infisical needs no CI change. TLS is verified against the AWS CA bundle.production-dbdeliberately has no required reviewers:deploydepends onmigrate, so a pending approval would strand a merged PR with its schema applied and its code undeployed.allowUnorderedMigrations: trueis a deliberate call, not a default someone left. Out-of-order merges are routine (Alice authors first, Bob merges first); with the defaultfalsethe next production deploy fails withcorrupted migrations, fixable only by hand-editingkysely_migrationin RDS.The baseline is the one risky file.
0000_baseline_schema.sqlis the only migration allowed to useIF NOT EXISTS, which is what lets it adopt the existing production database. Please read it by hand — ifDROP SCHEMAhad survived the split, the first production migrate would destroy the database. Two guards exist anyway:migrations-guardrejects the statement, and the migrate job refuses to run if the baseline still shows as pending.One-time step before this can merge: production must be adopted by hand —
pg_dumpdiff against the baseline, thennpm run migrateagainst RDS with a human watching. Procedure is inapps/backend/db/README.md. Terraform (aws/oidc.tf,github/main.tf) must also be applied before the workflow runs on main, or the migrate job fails atAssumeRoleWithWebIdentity.Also in here
db_setup.sqlsplit intomigrations/0000_baseline_schema.sql(DDL, idempotent) andseed.sql(dev/test only, never applied to prod). Verified lossless:pg_dumpof the old file vs migrations+seed is byte-identical for both--schema-onlyand--data-only, moduloCURRENT_TIMESTAMPdefaults.db_setup.sqlper test now usedb/testkit.ts—ensureSchema()inbeforeAllrebuilds only when the migration files changed (fingerprint stored as a schema comment),resetData()inbeforeEachtruncates withRESTART IDENTITYand reseeds. Faster than before (no DDL per test), and the truncate list is discovered frompg_tablesso tables added by future migrations reset automatically.migratorcompose service applies migrations on everymake up; the six lambdas gate onservice_completed_successfully.make upswitches to--abort-on-container-failure—--abort-on-container-exitwould have torn the stack down the moment the one-shot service succeeded.lambda-testscheck, so branch protection needs no change and there's no risk of wedging the merge queue on a mistyped context.migrations-freshapplies everything to an empty Postgres, asserts idempotency, and fails on stale types;migrations-guardenforces append-only migrations, filenames, and no destructive SQL.regenerate-db-types.yamlnow triggers offmigrations/**and calls the samenpm run typesa contributor runs. The kysely-ColumnTyperewrite moves out of an inlinenode -eheredoc intodb/src/postprocess-types.ts, so local and CI output are identical by construction. Node 23 → 20 to match thenodejs20.xruntime.backup_retention_period = 7on RDS. It was unset, so PITR was whatever the API defaulted to — possibly 0. Free at this storage size, and the backstop for automated migrations.kysely-codegen.config.js(itsoutFilepointed at a nonexistent path).Verification
Run locally, from an empty volume:
make up→make new-migration→make migrate→make show-migrationsmake db-reset,seed --if-empty, and the deleted-migration guard all behave4 tests still fail on my machine (
auth3,donors1). Theyfetch("http://localhost:3000/...")and need the lambda dev server, which my own Next.js app occupies. I confirmed they fail identically on the unmodified files — same test names, same counts — so this change is neutral on them. CI starts the dev server properly, so they should pass there.Not verified, because it needs real AWS: the migrate job's OIDC assume, the snapshot, and the Slack thread step.
🤖 Generated with Claude Code