From d5ebb86cae01df4ed6466a38ac0cb3ffc1a66685 Mon Sep 17 00:00:00 2001 From: Ricardo Mancinas Date: Thu, 30 Jul 2026 16:03:40 -0700 Subject: [PATCH] fix(deploy): dump from a dedicated container as root, and prove the dump is real MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The pre-migrate backup ran INSIDE the API container, which made it depend on that image's toolchain — and deadlocked: the running image shipped a MySQL client that could not authenticate, so the backup failed, which blocked the very deploy that would have replaced the broken image. A backup must not depend on the thing being deployed. The dump now runs in a throwaway container built from mysql:8.4 with the API's backup volume mounted. The volume name is discovered from the API container's mounts, so the file still lands where the Operaciones restore screen looks. As a container rather than an exec, its logs can simply be read — no more failures reported as a bare exit code. The image is pulled if the host lacks it, since a scope:app deploy never touches the db stack. Three further defects found while verifying, none of which would have surfaced without dumping against the real database: - The dump now runs as root. mysqldump --single-transaction issues FLUSH TABLES, needing the global RELOAD privilege; the MySQL image grants the application user only ALL ON ``.*, and --skip-lock-tables does not avoid it. Elevating the app's own runtime user would have been the worse trade. - --set-gtid-purged=OFF. galactus is the replication SOURCE with GTID on, so a default dump embeds SET @@GLOBAL.GTID_PURGED and is unrestorable onto the server it came from. Verified: 0 GTID_PURGED lines in the output. - Verification was too weak to be worth having. `test -s` plus `gzip -t` passes on a 372-byte gzip containing no tables, which is exactly what a dump that died on its first statement produces. It now asserts a CREATE TABLE count and logs it. A failed attempt also deletes its own output, so a truncated file never appears in the restore list. Verified against live prod, both paths: success writes a 31-table dump the API container can see; a wrong password fails with mysqldump's own error quoted and leaves the volume empty. Co-Authored-By: Claude Opus 5 --- .gitea/workflows/deploy-galactus.yml | 10 +- .gitea/workflows/deploy.yml | 8 +- deploy/scripts/pre-migrate-backup.mjs | 290 ++++++++++++++++---------- 3 files changed, 188 insertions(+), 120 deletions(-) diff --git a/.gitea/workflows/deploy-galactus.yml b/.gitea/workflows/deploy-galactus.yml index 2050244..972058a 100644 --- a/.gitea/workflows/deploy-galactus.yml +++ b/.gitea/workflows/deploy-galactus.yml @@ -122,11 +122,11 @@ jobs: PORTAINER_ENDPOINT_ID_GALACTUS PORTAINER_APP_STACK_NAME_GALACTUS DATABASE_URL_GALACTUS SESSION_SECRET_GALACTUS APP_API_ORIGIN_GALACTUS APP_WEB_ORIGIN_GALACTUS - APP_S3_ENDPOINT_GALACTUS MINIO_ROOT_USER MINIO_ROOT_PASSWORD" + APP_S3_ENDPOINT_GALACTUS MINIO_ROOT_USER MINIO_ROOT_PASSWORD + MYSQL_ROOT_PASSWORD" if [ "$SCOPE" = "full" ]; then REQUIRED="$REQUIRED PORTAINER_DB_STACK_NAME_GALACTUS - PORTAINER_MINIO_STACK_NAME_GALACTUS - MYSQL_PASSWORD MYSQL_ROOT_PASSWORD" + PORTAINER_MINIO_STACK_NAME_GALACTUS MYSQL_PASSWORD" fi missing="" for name in $REQUIRED; do @@ -189,6 +189,10 @@ jobs: PORTAINER_API_KEY: ${{ secrets.PORTAINER_API_KEY_GALACTUS }} PORTAINER_ENDPOINT_ID: ${{ secrets.PORTAINER_ENDPOINT_ID_GALACTUS }} DATABASE_URL: ${{ secrets.DATABASE_URL_GALACTUS }} + # The dump runs as root: --single-transaction issues FLUSH TABLES, + # which needs the global RELOAD privilege the application user + # deliberately does not have. + MYSQL_ROOT_PASSWORD: ${{ secrets.MYSQL_ROOT_PASSWORD }} BACKUP_TAG: ${{ github.event.inputs.tag }} ALLOW_MISSING_CONTAINER: ${{ github.event.inputs.bootstrap }} # Portainer serves a self-signed certificate. Scoped to this step diff --git a/.gitea/workflows/deploy.yml b/.gitea/workflows/deploy.yml index 0443f90..c52514c 100644 --- a/.gitea/workflows/deploy.yml +++ b/.gitea/workflows/deploy.yml @@ -126,10 +126,10 @@ jobs: REQUIRED="PORTAINER_URL PORTAINER_API_KEY PORTAINER_ENDPOINT_ID PORTAINER_APP_STACK_NAME DATABASE_URL SESSION_SECRET APP_API_ORIGIN APP_WEB_ORIGIN APP_S3_ENDPOINT - MINIO_ROOT_USER MINIO_ROOT_PASSWORD" + MINIO_ROOT_USER MINIO_ROOT_PASSWORD MYSQL_ROOT_PASSWORD" if [ "$SCOPE" = "full" ]; then REQUIRED="$REQUIRED PORTAINER_DB_STACK_NAME PORTAINER_MINIO_STACK_NAME - MYSQL_PASSWORD MYSQL_ROOT_PASSWORD" + MYSQL_PASSWORD" fi missing="" for name in $REQUIRED; do @@ -193,6 +193,10 @@ jobs: PORTAINER_API_KEY: ${{ secrets.PORTAINER_API_KEY }} PORTAINER_ENDPOINT_ID: ${{ secrets.PORTAINER_ENDPOINT_ID }} DATABASE_URL: ${{ secrets.DATABASE_URL }} + # The dump runs as root: --single-transaction issues FLUSH TABLES, + # which needs the global RELOAD privilege the application user + # deliberately does not have. + MYSQL_ROOT_PASSWORD: ${{ secrets.MYSQL_ROOT_PASSWORD }} BACKUP_TAG: ${{ github.event.inputs.tag }} ALLOW_MISSING_CONTAINER: ${{ github.event.inputs.bootstrap }} # Portainer serves a self-signed certificate. Scoped to this step diff --git a/deploy/scripts/pre-migrate-backup.mjs b/deploy/scripts/pre-migrate-backup.mjs index e02a9a3..b45c1d2 100644 --- a/deploy/scripts/pre-migrate-backup.mjs +++ b/deploy/scripts/pre-migrate-backup.mjs @@ -2,48 +2,53 @@ /** * Take a mysqldump immediately before a deploy runs `prisma migrate deploy`. * - * Why it is done THIS way and not with a plain `mysqldump` on the CI runner: - * a dump is only useful if a human can restore it, and the only restore path - * this platform has is the "Operaciones" admin screen, which lists and replays - * whatever `*.sql.gz` files sit in the API container's BACKUP_DIR volume - * (apps/api/src/ops/ops.service.ts — listBackups / RESTORE). A dump written on - * the runner would land nowhere anybody can reach. So we drive the dump INSIDE - * the still-running old API container, via Portainer's Docker API proxy: the - * container already has mysql-client baked in (docker/api.Dockerfile), and the - * file lands in the exact directory the restore UI reads. + * The dump runs in a DEDICATED, throwaway container built from the MySQL image, + * with the API's backup volume mounted — not inside the API container. Three + * reasons, each learned the hard way: * - * It must therefore run BEFORE the app stack is re-applied, while the previous - * container is still up. + * 1. Deadlock. Dumping inside the API container makes the backup depend on + * whatever toolchain that image happens to carry. When the image shipped a + * MySQL client that could not authenticate, the backup failed, which blocked + * the very deploy that would have replaced the broken image. The backup must + * not depend on the thing being deployed. + * 2. The right client. Alpine's `mysql-client` is MariaDB's and cannot perform + * caching_sha2_password (MySQL 8.4's default auth). The official MySQL image + * obviously can. + * 3. Diagnosability. A container's logs can simply be read, whereas a detached + * exec reports nothing but an exit code. + * + * The file still lands in the API's BACKUP_DIR volume, because the only restore + * path this platform has is the "Operaciones" admin screen, which lists whatever + * `*.sql.gz` sits there (apps/api/src/ops/ops.service.ts). + * + * It must run BEFORE the app stack is re-applied, while the old container is up + * — that container is how the backup volume's name is discovered. * * Required env: * PORTAINER_URL https://:9443 * PORTAINER_API_KEY Portainer access token * PORTAINER_ENDPOINT_ID numeric endpoint id (galactus = 3) - * DATABASE_URL mysql://user:pass@host:port/db + * DATABASE_URL mysql://user:pass@host:port/db — host/port/db only + * MYSQL_ROOT_PASSWORD the dump runs as root, see below * BACKUP_TAG label for the filename, e.g. the deployed tag * Optional env: * ALLOW_MISSING_CONTAINER=true exit 0 when no API container exists yet - * (first-ever deploy — nothing to back up) - * API_CONTAINER_LABEL=io.jorgecuadros.role=api - * EXEC_TIMEOUT_SECONDS=1800 + * BACKUP_VOLUME override the auto-discovered volume name + * DUMP_IMAGE default mysql:8.4 + * API_CONTAINER_LABEL default io.jorgecuadros.role=api + * TAILSCALE_DNS / FALLBACK_DNS / TAILNET_SUFFIX + * EXEC_TIMEOUT_SECONDS default 1800 * - * TLS: Portainer here uses a self-signed certificate. The caller is expected to - * set NODE_TLS_REJECT_UNAUTHORIZED=0 for this step; see the workflow. That - * disables verification for the whole process, so nothing else should run in it. + * Why root: mysqldump --single-transaction issues FLUSH TABLES, which needs the + * global RELOAD (or FLUSH_TABLES) privilege. The application user is granted + * only ALL ON ``.* by the MySQL image and deliberately has no global rights, + * so it cannot take a consistent dump. Backups are an administrative operation; + * elevating the app's own runtime user instead would be the worse trade. + * + * TLS: Portainer here is self-signed; the caller sets + * NODE_TLS_REJECT_UNAUTHORIZED=0 for this step. */ -const PORTAINER_URL = required("PORTAINER_URL").replace(/\/+$/, ""); -const API_KEY = required("PORTAINER_API_KEY"); -const ENDPOINT_ID = required("PORTAINER_ENDPOINT_ID"); -const DATABASE_URL = required("DATABASE_URL"); -const BACKUP_TAG = required("BACKUP_TAG"); - -const CONTAINER_LABEL = - process.env.API_CONTAINER_LABEL ?? "io.jorgecuadros.role=api"; -const ALLOW_MISSING = process.env.ALLOW_MISSING_CONTAINER === "true"; -const TIMEOUT_MS = - Number(process.env.EXEC_TIMEOUT_SECONDS ?? 1800) * 1000; - function required(name) { const v = process.env[name]; if (!v) { @@ -53,6 +58,24 @@ function required(name) { return v; } +const PORTAINER_URL = required("PORTAINER_URL").replace(/\/+$/, ""); +const API_KEY = required("PORTAINER_API_KEY"); +const ENDPOINT_ID = required("PORTAINER_ENDPOINT_ID"); +const DATABASE_URL = required("DATABASE_URL"); +const ROOT_PASSWORD = required("MYSQL_ROOT_PASSWORD"); +const BACKUP_TAG = required("BACKUP_TAG"); + +const CONTAINER_LABEL = + process.env.API_CONTAINER_LABEL ?? "io.jorgecuadros.role=api"; +const ALLOW_MISSING = process.env.ALLOW_MISSING_CONTAINER === "true"; +const DUMP_IMAGE = process.env.DUMP_IMAGE ?? "mysql:8.4"; +const DNS = [ + process.env.TAILSCALE_DNS ?? "100.100.100.100", + process.env.FALLBACK_DNS ?? "1.1.1.1", +]; +const DNS_SEARCH = [process.env.TAILNET_SUFFIX ?? "tail01aa2.ts.net"]; +const TIMEOUT_MS = Number(process.env.EXEC_TIMEOUT_SECONDS ?? 1800) * 1000; + const DOCKER = `${PORTAINER_URL}/api/endpoints/${ENDPOINT_ID}/docker`; async function docker(path, init = {}) { @@ -81,8 +104,6 @@ function parseDbUrl(raw) { return { host: u.hostname, port: u.port || "3306", - user: decodeURIComponent(u.username), - password: decodeURIComponent(u.password), database: u.pathname.replace(/^\//, ""), }; } @@ -98,7 +119,7 @@ function timestamp() { /** * ops.service.ts refuses to restore any name outside this character set, so a - * file we write with, say, a `+` in the tag would be permanently unrestorable + * file written with, say, a `/` in the tag would be permanently unrestorable * through the UI. Sanitise before writing, not after. */ function safeTag(tag) { @@ -114,68 +135,95 @@ async function findApiContainer() { return list.length ? list[0] : null; } +/** The named volume the API mounts at /data/backups — where restores look. */ +function backupVolumeOf(container) { + const mount = (container.Mounts ?? []).find( + (m) => m.Destination === "/data/backups", + ); + return mount?.Name ?? null; +} + /** - * Run a command in the container and return its exit code. Detach:true keeps - * this to plain HTTP — a non-detached exec start hijacks the connection into a - * raw stream, which fetch cannot read. The cost is that we get no stdout, so - * every check below has to be expressed as an exit code. + * Ensure the dump image is present. A `scope: app` deploy never touches the db + * stack, so a host can legitimately be missing it — and container/create fails + * with a bare 404 that reads like a Portainer problem rather than a missing + * image. The image is public, so no registry auth is involved. */ -async function execInContainer(containerId, cmd, env = []) { - const created = await docker(`/containers/${containerId}/exec`, { - method: "POST", - body: JSON.stringify({ - AttachStdout: false, - AttachStderr: false, - Tty: false, - Env: env, - Cmd: ["sh", "-c", cmd], - }), - }); - - await docker(`/exec/${created.Id}/start`, { - method: "POST", - body: JSON.stringify({ Detach: true, Tty: false }), - }); - - const deadline = Date.now() + TIMEOUT_MS; - for (;;) { - const info = await docker(`/exec/${created.Id}/json`); - if (!info.Running) return info.ExitCode ?? 1; - if (Date.now() > deadline) { - throw new Error(`exec timed out after ${TIMEOUT_MS / 1000}s`); +async function ensureDumpImage() { + const [repo, tag = "latest"] = DUMP_IMAGE.split(":"); + const existing = await docker(`/images/${encodeURIComponent(DUMP_IMAGE)}/json`) + .then(() => true) + .catch(() => false); + if (existing) return; + console.log(`pulling ${DUMP_IMAGE} (not present on the host)...`); + const res = await fetch( + `${DOCKER}/images/create?fromImage=${encodeURIComponent(repo)}&tag=${encodeURIComponent(tag)}`, + { method: "POST", headers: { "X-API-Key": API_KEY } }, + ); + const body = await res.text(); + if (!res.ok) { + throw new Error(`pull ${DUMP_IMAGE} -> HTTP ${res.status} ${body.slice(0, 300)}`); + } + for (const line of body.split("\n").filter((l) => l.trim())) { + try { + const obj = JSON.parse(line); + if (obj.error) throw new Error(`pull ${DUMP_IMAGE} failed: ${obj.error}`); + } catch (e) { + if (e.message.startsWith("pull ")) throw e; } - await new Promise((r) => setTimeout(r, 3000)); } } -/** - * Run a SHORT command and return its combined output. - * - * Detached exec cannot report why anything failed, which once reduced a real - * failure to the single line "mysqldump exited 2" and cost a manual - * reproduction on the host to discover it was a missing auth plugin. Tty:true - * makes the start response a plain (non-multiplexed) stream that can just be - * read, at the cost of holding the connection open — fine for reading a small - * error file, which is why the dump itself still runs detached. - */ -async function execCapture(containerId, cmd) { - const created = await docker(`/containers/${containerId}/exec`, { +async function runDumpContainer(cmd, env) { + await ensureDumpImage(); + const created = await docker(`/containers/create`, { method: "POST", body: JSON.stringify({ - AttachStdout: true, - AttachStderr: true, - Tty: true, - Cmd: ["sh", "-c", cmd], + Image: DUMP_IMAGE, + Entrypoint: ["sh", "-c"], + Cmd: [cmd], + Env: env, + HostConfig: { + AutoRemove: false, // we read the logs before removing it ourselves + Binds: [`${BACKUP_VOLUME}:/data/backups`], + Dns: DNS, + DnsSearch: DNS_SEARCH, + }, }), }); - const res = await fetch(`${DOCKER}/exec/${created.Id}/start`, { - method: "POST", - headers: { "X-API-Key": API_KEY, "Content-Type": "application/json" }, - body: JSON.stringify({ Detach: false, Tty: true }), - }); - return (await res.text()).trim(); + + const id = created.Id; + try { + await docker(`/containers/${id}/start`, { method: "POST" }); + + const deadline = Date.now() + TIMEOUT_MS; + for (;;) { + const info = await docker(`/containers/${id}/json`); + if (!info.State.Running) { + const logs = await fetch( + `${DOCKER}/containers/${id}/logs?stdout=true&stderr=true&tail=40`, + { headers: { "X-API-Key": API_KEY } }, + ).then((r) => r.text()); + // Strip Docker's 8-byte stream framing and any stray control bytes. + const clean = logs + .replace(/[\x00-\x08\x0b\x0c\x0e-\x1f]/g, "") + .trim(); + return { code: info.State.ExitCode ?? 1, logs: clean }; + } + if (Date.now() > deadline) { + throw new Error(`dump timed out after ${TIMEOUT_MS / 1000}s`); + } + await new Promise((r) => setTimeout(r, 3000)); + } + } finally { + await docker(`/containers/${id}?force=true`, { method: "DELETE" }).catch( + () => {}, + ); + } } +let BACKUP_VOLUME = process.env.BACKUP_VOLUME ?? null; + async function main() { const container = await findApiContainer(); if (!container) { @@ -190,52 +238,64 @@ async function main() { ); } + BACKUP_VOLUME = BACKUP_VOLUME ?? backupVolumeOf(container); + if (!BACKUP_VOLUME) { + throw new Error( + "could not determine the backup volume from the API container's mounts; " + + "set BACKUP_VOLUME explicitly", + ); + } + const conn = parseDbUrl(DATABASE_URL); const file = `pre-migrate-${safeTag(BACKUP_TAG)}-${timestamp()}.sql.gz`; const out = `/data/backups/${file}`; - console.log(`container : ${container.Id.slice(0, 12)}`); - console.log(`database : ${conn.user}@${conn.host}:${conn.port}/${conn.database}`); + console.log(`database : ${conn.host}:${conn.port}/${conn.database}`); + console.log(`volume : ${BACKUP_VOLUME}`); + console.log(`image : ${DUMP_IMAGE}`); console.log(`writing : ${out}`); - // Password via MYSQL_PWD in the exec env, never on the command line — argv is - // world-readable through `ps` inside the container. - const flags = `--host=${conn.host} --port=${conn.port} --user=${shq(conn.user)}`; - const errFile = "/tmp/pre-migrate-backup.err"; + // --set-gtid-purged=OFF because this server is the replication SOURCE with + // GTID on. Without it the dump embeds SET @@GLOBAL.GTID_PURGED, which makes + // the file unrestorable onto the very server it came from. + // + // pipefail is essential: without it the exit status is gzip's, so a dump that + // failed on the first statement still produces a small, perfectly valid .gz — + // a "successful" backup containing nothing. + // + // The table count is asserted for the same reason: valid gzip is not evidence + // of a usable dump. It is echoed so the log records how much was captured. + // + // A failed attempt deletes its own output. Otherwise every failure leaves a + // truncated .sql.gz sitting in the volume, and the Operaciones restore screen + // lists it as a perfectly ordinary restore point. const dump = - `set -o pipefail; mysqldump ${flags} --single-transaction --routines ` + - `--triggers --no-tablespaces ${shq(conn.database)} 2>${errFile} ` + - `| gzip -c > ${shq(out)}`; + `set -o pipefail; ` + + `( mysqldump --host=${conn.host} --port=${conn.port} --user=root ` + + `--single-transaction --routines --triggers --no-tablespaces ` + + `--set-gtid-purged=OFF ${shq(conn.database)} | gzip -c > ${shq(out)} && ` + + `gzip -t ${shq(out)} && ` + + `TABLES=$(gunzip -c ${shq(out)} | grep -c 'CREATE TABLE') && ` + + `echo "tables captured: $TABLES" && ` + + `[ "$TABLES" -ge 1 ] ); ` + + `rc=$?; ` + + `if [ $rc -ne 0 ]; then rm -f ${shq(out)}; ` + + `echo "removed incomplete backup ${file}"; fi; ` + + `exit $rc`; - const code = await execInContainer(container.Id, dump, [ - `MYSQL_PWD=${conn.password}`, + const { code, logs } = await runDumpContainer(dump, [ + // Password via MYSQL_PWD, never argv — argv is readable through `ps`. + `MYSQL_PWD=${ROOT_PASSWORD}`, ]); + + if (logs) console.log(logs); if (code !== 0) { - // An exit code alone is not actionable — surface what mysqldump actually - // said. Leave the truncated file behind for inspection, but never let the - // deploy proceed believing it has a restore point. - const stderr = await execCapture( - container.Id, - `tail -20 ${errFile} 2>/dev/null`, - ); throw new Error( - `mysqldump exited ${code} — refusing to migrate` + - (stderr ? `\n--- mysqldump stderr ---\n${stderr}` : ""), + `dump failed (exit ${code}) — refusing to migrate. See the output above.`, ); } - // Detached exec gives no stdout, so prove the artefact separately: non-empty - // file, and gzip that actually decompresses. A dump that fails mid-stream can - // still leave a plausible-looking file. - const verify = await execInContainer( - container.Id, - `test -s ${shq(out)} && gzip -t ${shq(out)}`, - ); - if (verify !== 0) { - throw new Error(`backup ${file} is empty or corrupt (check exited ${verify})`); - } - - console.log(`ok: ${file} written and verified in the API backup volume`); + console.log(`ok: ${file} written and verified in ${BACKUP_VOLUME}`); } main().catch((err) => {