cesnimda/jobtrackingapp
1
0
Fork 0
Code Issues Pull Requests 1 Actions Packages Projects Releases Wiki Activity

fix(deploy): load production environment before backup

Browse Source 
deploy.sh symlinked /opt/job-tracker/shared/.env for docker compose but
never loaded it into its own shell. Its own decisions therefore ran
against an empty environment: DATABASE_PROVIDER fell back to sqlite on a
MariaDB host, so the deploy tarred the data volume, printed "Backup
verified" and continued with no database dump. The operator saw a green
backup line and a new file in the backups directory, and had no restore
point.

Load the shared env before any decision. Parsed line by line rather than
sourced, because a compose .env is not a shell script and an unquoted
value containing spaces would execute as a command. Values already in
the environment win, so CI-provided APP_VERSION and friends still
override the file. No value is echoed.

Remove the sqlite default. DATABASE_PROVIDER must be stated; missing or
unrecognised aborts the deploy.

Validate deployment configuration before the backup, and so before
anything is built, stopped or replaced: the connection string when the
provider needs one, AI_SERVICE_TOKEN (compose declares it with :?) and
AUTH_JWT_KEY (the backend throws on a blank key). Names in the output,
never values.

Verify each backup against its own format. A dump must be valid gzip,
contain CREATE TABLE, and carry the "Dump completed" trailer, so a dump
that died partway through is rejected. An archive must contain
jobtracker.db. A tar can no longer pass the dump check.

Also resolve the SQLite volume by its project-prefixed name and fail if
absent. The bare jobtracker_data name would have silently created an
empty volume and backed that up -- the same class of bug, found while
testing this fix.

Verified against a seeded MariaDB 11 container and real Docker volumes:
provider selection, all four validation failures, both backup formats
and their failure paths, truncated and trailer-stripped dumps, and zero
secret occurrences across every test's output.

Docs updated for the drift: deploy/README.md, deploy/first-production-
deployment.md, docs/release-candidate-review.md (B1 closed) and
.env.example, which now names the two database variables.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
This commit is contained in:
cesnimda
2026-07-19 18:44:23 +02:00
parent ab53582c71
commit 66b02bcab8
5 changed files with 380 additions and 49 deletions
Download Patch File Download Diff File Expand all files Collapse all files
docs/release-candidate-review.md
+63 -12
View File
@@ -9,8 +9,13 @@
> `docs/infrastructure/database-ownership.md` (who creates which table),
> `docs/release-checklist.md` (state of the build).
**Verdict: do not deploy yet.** Two blocking items, one of them new and material — the pre-deploy
database backup does not do what every other document assumes it does.
**Updated 2026-07-19 (second pass).** **B1 is fixed and verified** — `deploy.sh` now loads the shared
environment before it decides anything, requires `DATABASE_PROVIDER` explicitly, validates the rest of
the deployment configuration before touching the stack, and verifies each backup against its own
format. Details under *Blocking*.
**Verdict: one blocking item remains — CI (B2), which is external.** The deployment path itself is now
sound.
---
@@ -73,7 +78,9 @@ database backup does not do what every other document assumes it does.
## Blocking
### B1. The pre-deploy database backup does not back up the database — it silently backs up the wrong thing
### B1. ~~The pre-deploy database backup does not back up the database~~ — **CLOSED 2026-07-19**
*Original finding, kept because the failure mode is worth understanding:*
**Severity: critical. This invalidates the restore point that the entire deployment plan depends on.**
@@ -113,8 +120,47 @@ MariaDB dump. If the deploy then damages the schema, there is nothing to restore
`/opt/job-tracker/backups` is named `jobtracker-<dbname>-<stamp>.sql.gz`, not
`jobtracker-sqlite-<stamp>.tar.gz`. The filename alone distinguishes the two paths.
**Interim manual workaround if you deploy before this is fixed:** take the MariaDB dump by hand,
verify it restores into a scratch database, and treat `deploy.sh`'s backup line as meaningless.
**Closed.** `deploy/deploy.sh` now:
1. **Loads `/opt/job-tracker/shared/.env` into its own shell** before any decision. Parsed line by
line rather than sourced, because a compose `.env` is not a shell script. Variables already set in
the environment win, so CI-provided `APP_VERSION` and friends still override the file. No value is
echoed.
2. **Requires `DATABASE_PROVIDER` explicitly.** The `:-sqlite` default is gone. Missing means stop and
say which variable is missing; an unrecognised value means stop.
3. **Validates the rest of the deployment configuration before the backup**, and therefore before
anything is built, stopped or replaced: the connection string when the provider needs one,
`AI_SERVICE_TOKEN` (compose declares it with `:?`, so missing it would otherwise kill the stack
after the images are built) and `AUTH_JWT_KEY` (the backend throws at startup on a blank key, after
the containers have been replaced). Names only in the output, never values.
4. **Verifies each backup against its own format.** A MariaDB dump must be valid gzip, contain
`CREATE TABLE`, and carry the `Dump completed` trailer that `mariadb-dump` writes last — so a dump
that died partway through is rejected. A SQLite archive must be valid gzip and actually contain
`jobtracker.db`. A tar can no longer pass the dump check, which is precisely what went wrong.
5. **Resolves the SQLite volume by its real, project-prefixed name** and fails if it does not exist.
The old code named the bare `jobtracker_data`, which on a real deploy would have silently *created*
an empty volume and backed that up — a second instance of the same class of bug, found while
testing the fix.
Two side effects of the same root cause are also resolved: `APP_PUBLIC_BASE_URL` now reaches the
script, so the post-deploy public smoke check actually runs (and the script says so explicitly when it
is unset rather than skipping in silence), as does `OLLAMA_MODEL` for the warmup.
*Verified against a seeded MariaDB 11 container and real Docker volumes:*
| Scenario | Result |
|---|---|
| MariaDB production-style `.env` | ✅ env loaded, MariaDB path selected, `.sql.gz` written containing `CREATE TABLE`, the seeded row, and the `Dump completed` trailer |
| `DATABASE_PROVIDER` missing | ✅ exits 1 naming the variable; nothing built, stopped or replaced; no backup file |
| `DATABASE_PROVIDER` unrecognised | ✅ exits 1 |
| `AI_SERVICE_TOKEN` / `AUTH_JWT_KEY` missing | ✅ both reported in one pass, exits 1 |
| SQLite with a populated volume | ✅ `.tar.gz` written and verified |
| SQLite volume containing no `jobtracker.db` | ✅ rejected, file deleted |
| SQLite volume name not present | ✅ rejected before any container ran; no stray empty volume created |
| Broken database credentials | ✅ exits 1, no partial file left behind |
| Truncated dump (no `CREATE TABLE`) | ✅ rejected |
| Dump with the trailer stripped | ✅ rejected as truncated |
| Secret leakage across every test's output | ✅ zero occurrences of any password, token or key |
### B2. CI is red — deployment is gated on it
@@ -134,7 +180,7 @@ Known and accepted for the first release. None of these should stop a deploy; al
| # | Finding | Why it is not blocking |
|---|---|---|
| N1 | **`.env.example` omits `DATABASE_PROVIDER` and `JOBTRACKER_CONNECTION_STRING`** — both are consumed by `docker-compose.yml`, and the second is the only way to reach a database at all | The production `.env` already has them, and `deploy/README.md:48-49` documents both. Only bites someone building a new environment from the template |
| N1 | ~~**`.env.example` omits `DATABASE_PROVIDER` and `JOBTRACKER_CONNECTION_STRING`**~~ — **CLOSED 2026-07-19** | Both added to the template with the connection-string host caveat. `deploy.sh` now hard-fails without `DATABASE_PROVIDER`, so the template had to name it |
| N2 | **`/health` always reports `"version":"unknown"` under Docker** — the endpoint reads the `APP_VERSION` environment variable, but compose passes it as `App__Version` | Liveness is unaffected; only the version string is wrong. `deploy/first-production-deployment.md` shows a populated version in its expected output, which will not match reality |
| N3 | **The post-deploy gate in `deploy.sh` checks `.State == running`, not health** — a container can be `running` while `starting` or `unhealthy` | Harmless in practice: `compose up` already blocks on `service_healthy` for the frontend's dependency, so an unhealthy backend aborts the deploy before this check is reached. The check is weaker than it looks, not wrong |
| N4 | **Table-count discrepancy across documents** — `database-ownership.md` records 40 tables on MariaDB; `release-checklist.md` and the runbook say ~42 | Both were measured, at different points in Phase 5. Use "the tables listed in `database-ownership.md` all exist" as the check, not a number |
@@ -154,12 +200,17 @@ them — every automated check stops at the authentication boundary.
### Before deploying
- [ ] **Take a MariaDB dump by hand and restore it into a scratch database.** Given B1, this is not
optional and not a formality. `deploy.sh`'s backup line cannot currently be trusted on a MariaDB
host.
- [ ] **Confirm `/opt/job-tracker/shared/.env` contains `DATABASE_PROVIDER` and
`JOBTRACKER_CONNECTION_STRING`**, and that the `Server=` host resolves *from inside the backend
container* — `127.0.0.1` there means the container, not the host.
- [ ] **Take a MariaDB dump by hand and restore it into a scratch database.** B1 is fixed and the
automatic backup is verified against containers, but the first production deploy is still the
wrong moment to discover that a backup does not restore *on your data*.
- [ ] **Confirm `/opt/job-tracker/shared/.env` contains `DATABASE_PROVIDER=mariadb` and
`JOBTRACKER_CONNECTION_STRING`.** `deploy.sh` now aborts without them, so a missing value costs
an aborted deploy rather than a bad backup — but check first and skip the round trip.
- [ ] **Confirm the connection-string host resolves *from inside the backend container*** —
`127.0.0.1` there means the container, not the Docker host.
- [ ] **After the deploy, check the backup filename.** It must be
`jobtracker-<database>-<stamp>.sql.gz`. A `jobtracker-sqlite-<stamp>.tar.gz` means the
environment is wrong — though the provider check should now stop that before it happens.
- [ ] **Record the current commit** (`git rev-parse HEAD`) and the current row counts for
`JobApplications` and `Companies`. The row counts are the check that matters most afterwards.