From 1850fbeff54dab75878ee84f1d2fc4ceada9827f Mon Sep 17 00:00:00 2001 From: Jamo Date: Mon, 25 May 2026 01:26:58 +0200 Subject: [PATCH] fix(backup): pin pg_dump 18 in api images and expose download via SDK + CLI --- backend/Dockerfile | 12 +++- backend/Dockerfile.prod | 11 +++- backend/testjam/routers/settings.py | 7 ++- backend/testjam/services/backup.py | 18 +++++- testjam-client/testjam_client/cli/__init__.py | 2 + .../testjam_client/cli/commands/backup.py | 57 +++++++++++++++++++ testjam-client/testjam_client/client.py | 2 + .../testjam_client/resources/backup.py | 51 +++++++++++++++++ testjam-client/tests/test_backup.py | 45 +++++++++++++++ testjam-client/tests/test_cli_backup.py | 51 +++++++++++++++++ 10 files changed, 251 insertions(+), 5 deletions(-) create mode 100644 testjam-client/testjam_client/cli/commands/backup.py create mode 100644 testjam-client/testjam_client/resources/backup.py create mode 100644 testjam-client/tests/test_backup.py create mode 100644 testjam-client/tests/test_cli_backup.py diff --git a/backend/Dockerfile b/backend/Dockerfile index f26d2ac..d7ab989 100644 --- a/backend/Dockerfile +++ b/backend/Dockerfile @@ -1,8 +1,18 @@ # syntax=docker/dockerfile:1.7 FROM python:3.14-slim +# pg_dump must match the server major version or it refuses to run +# (server 18 vs client 17 → "aborting because of server version mismatch"). +# Debian's `postgresql-client` defaults to whatever the base image ships; +# pin to PGDG's `postgresql-client-18` so the version stays in lockstep +# with the postgres:18.x image used in docker-compose-dev.yml. RUN apt-get update \ - && apt-get install -y --no-install-recommends postgresql-client git \ + && apt-get install -y --no-install-recommends ca-certificates curl gnupg git \ + && install -d /usr/share/keyrings \ + && curl -fsSL https://www.postgresql.org/media/keys/ACCC4CF8.asc | gpg --dearmor -o /usr/share/keyrings/pgdg.gpg \ + && echo "deb [signed-by=/usr/share/keyrings/pgdg.gpg] http://apt.postgresql.org/pub/repos/apt $(. /etc/os-release && echo $VERSION_CODENAME)-pgdg main" > /etc/apt/sources.list.d/pgdg.list \ + && apt-get update \ + && apt-get install -y --no-install-recommends postgresql-client-18 \ && rm -rf /var/lib/apt/lists/* WORKDIR /app diff --git a/backend/Dockerfile.prod b/backend/Dockerfile.prod index 2c0c691..2138664 100644 --- a/backend/Dockerfile.prod +++ b/backend/Dockerfile.prod @@ -4,8 +4,17 @@ ENV PYTHONUNBUFFERED=1 \ PIP_NO_CACHE_DIR=1 \ PIP_DISABLE_PIP_VERSION_CHECK=1 +# pg_dump must match the server major version or `/settings/backup` +# refuses to run with "aborting because of server version mismatch". +# Pin to PGDG's `postgresql-client-18` so prod deployments stay in +# lockstep with the postgres 18.x they're expected to point at. RUN apt-get update \ - && apt-get install -y --no-install-recommends postgresql-client \ + && apt-get install -y --no-install-recommends ca-certificates curl gnupg \ + && install -d /usr/share/keyrings \ + && curl -fsSL https://www.postgresql.org/media/keys/ACCC4CF8.asc | gpg --dearmor -o /usr/share/keyrings/pgdg.gpg \ + && echo "deb [signed-by=/usr/share/keyrings/pgdg.gpg] http://apt.postgresql.org/pub/repos/apt $(. /etc/os-release && echo $VERSION_CODENAME)-pgdg main" > /etc/apt/sources.list.d/pgdg.list \ + && apt-get update \ + && apt-get install -y --no-install-recommends postgresql-client-18 \ && rm -rf /var/lib/apt/lists/* WORKDIR /app diff --git a/backend/testjam/routers/settings.py b/backend/testjam/routers/settings.py index 82af662..8cd3e15 100644 --- a/backend/testjam/routers/settings.py +++ b/backend/testjam/routers/settings.py @@ -10,7 +10,7 @@ AppSettingsPublicOut, AppSettingsUpdate, ) -from testjam.services.backup import cleanup_archive, create_backup +from testjam.services.backup import BackupFailed, cleanup_archive, create_backup from testjam.services.email import smtp_configured from testjam.services.log_flusher import configure_from_settings as configure_log_flusher from testjam.services.restore import RestoreSummary, restore_backup @@ -89,7 +89,10 @@ def download_backup( db: Session = Depends(get_db), _: User = Depends(require_admin), ): - artifact = create_backup(db, db.get_bind()) + try: + artifact = create_backup(db, db.get_bind()) + except BackupFailed as exc: + raise HTTPException(status_code=503, detail=str(exc)) background_tasks.add_task(cleanup_archive, artifact.path) return FileResponse( artifact.path, diff --git a/backend/testjam/services/backup.py b/backend/testjam/services/backup.py index 5c94388..55fb918 100644 --- a/backend/testjam/services/backup.py +++ b/backend/testjam/services/backup.py @@ -34,6 +34,15 @@ UPLOADS_PREFIX = "uploads/" +class BackupFailed(RuntimeError): + """Raised when the backup pipeline can't produce a usable archive. + + Carries the diagnostic the operator needs to act on — usually the + pg_dump stderr — so the API layer can surface it as a 503 detail + instead of swallowing it into a generic 500. + """ + + @dataclass class BackupArtifact: path: str @@ -85,6 +94,11 @@ def _dump_database(db: Session, engine: Engine) -> str: def _pg_dump_to(database_url: str, dump_path: str) -> None: + if not pg_dump_available(): + raise BackupFailed( + "pg_dump is not installed in the API container; backup endpoint " + "needs a matching postgresql-client.", + ) parsed = urlparse(database_url) env = os.environ.copy() if parsed.password: @@ -100,7 +114,9 @@ def _pg_dump_to(database_url: str, dump_path: str) -> None: with open(dump_path, "w") as out: result = subprocess.run(cmd, env=env, stdout=out, stderr=subprocess.PIPE, check=False) if result.returncode != 0: - raise RuntimeError(f"pg_dump failed: {result.stderr.decode('utf-8', 'replace')}") + raise BackupFailed( + f"pg_dump failed: {result.stderr.decode('utf-8', 'replace').strip()}", + ) def _sqlite_dump_to(db: Session, dump_path: str) -> None: diff --git a/testjam-client/testjam_client/cli/__init__.py b/testjam-client/testjam_client/cli/__init__.py index 0b69f92..c0f7296 100644 --- a/testjam-client/testjam_client/cli/__init__.py +++ b/testjam-client/testjam_client/cli/__init__.py @@ -6,6 +6,7 @@ import typer from testjam_client.cli.commands import auth as auth_commands +from testjam_client.cli.commands import backup as backup_commands from testjam_client.cli.commands import bugs as bug_commands from testjam_client.cli.commands import cases as case_commands from testjam_client.cli.commands import config as config_commands @@ -131,3 +132,4 @@ def repl_command(context: typer.Context) -> None: app.add_typer(execution_commands.build_app()) app.add_typer(run_commands.build_app()) app.add_typer(bug_commands.build_app()) +app.add_typer(backup_commands.build_app()) diff --git a/testjam-client/testjam_client/cli/commands/backup.py b/testjam-client/testjam_client/cli/commands/backup.py new file mode 100644 index 0000000..e792728 --- /dev/null +++ b/testjam-client/testjam_client/cli/commands/backup.py @@ -0,0 +1,57 @@ +"""`testjam backup` — download a full-instance backup archive.""" +from __future__ import annotations + +from datetime import datetime, timezone +from pathlib import Path + +import typer + +from testjam_client.cli import output, runtime +from testjam_client.cli.state import CliState + + +def build_app() -> typer.Typer: + app = typer.Typer( + name="backup", + help="Download full instance backup (admin only).", + no_args_is_help=False, + invoke_without_command=True, + ) + app.callback(invoke_without_command=True)(download_backup) + return app + + +def download_backup( + context: typer.Context, + out: Path | None = typer.Option( + None, "--out", "-o", + help="File or directory to write the archive into. Defaults to the current directory.", + ), +) -> None: + if context.invoked_subcommand is not None: + return + state: CliState = context.obj + target = out if out is not None else Path.cwd() + with output.progress("Requesting backup…", enabled=not state.json_output): + with runtime.build_client(state) as client: + written = client.backup.download(_resolve_destination(target)) + payload = {"path": str(written), "bytes": written.stat().st_size} + output.render(payload, columns=["path", "bytes"], json_mode=state.json_output) + + +def _resolve_destination(target: Path) -> Path: + if target.exists() and target.is_dir(): + return target + if str(target).endswith(("/", "\\")): + target.mkdir(parents=True, exist_ok=True) + return target + if target.suffix == "": + # Bare name with no extension — treat as directory. + target.mkdir(parents=True, exist_ok=True) + return target + return target + + +def _default_filename() -> str: + stamp = datetime.now(timezone.utc).strftime("%Y%m%dT%H%M%SZ") + return f"testjam-backup-{stamp}.zip" diff --git a/testjam-client/testjam_client/client.py b/testjam-client/testjam_client/client.py index 08c1e73..05bd86d 100644 --- a/testjam-client/testjam_client/client.py +++ b/testjam-client/testjam_client/client.py @@ -17,6 +17,7 @@ import httpx from testjam_client.errors import raise_for_status +from testjam_client.resources.backup import BackupResource from testjam_client.resources.bugs import BugsResource from testjam_client.resources.cases import CasesResource from testjam_client.resources.custom_fields import CustomFieldsResource @@ -83,6 +84,7 @@ def __init__( self.tokens = TokensResource(self) self.mentions = MentionsResource(self) self.public_reports = PublicReportsResource(self) + self.backup = BackupResource(self) def login(self, username: str, password: str) -> str: response = self._http.request( diff --git a/testjam-client/testjam_client/resources/backup.py b/testjam-client/testjam_client/resources/backup.py new file mode 100644 index 0000000..503a911 --- /dev/null +++ b/testjam-client/testjam_client/resources/backup.py @@ -0,0 +1,51 @@ +"""Admin-only backup download. + +The API ships the whole instance as a single zip (manifest + SQL dump + +uploads tree) over ``GET /settings/backup``. This resource exposes both +a "give me the bytes" path for callers that want to handle persistence +themselves and a "save to disk" path that takes care of streaming and +filename resolution. +""" +from __future__ import annotations + +import os +import re +from pathlib import Path + +from testjam_client.resources._base import Resource + + +class BackupResource(Resource): + def download_bytes(self) -> bytes: + response = self._request("GET", "/settings/backup") + return response.content + + def download(self, destination: str | os.PathLike[str]) -> Path: + """Save the backup archive to ``destination``. + + If ``destination`` is an existing directory (or a path that ends in + a separator), the filename is taken from the API's + ``Content-Disposition`` header when present, otherwise it falls + back to ``testjam-backup.zip``. Returns the full path written. + """ + response = self._request("GET", "/settings/backup") + target = Path(destination) + if target.is_dir() or str(destination).endswith(os.sep): + target.mkdir(parents=True, exist_ok=True) + filename = _filename_from_response(response, fallback="testjam-backup.zip") + target = target / filename + else: + target.parent.mkdir(parents=True, exist_ok=True) + target.write_bytes(response.content) + return target + + +def _filename_from_response(response, *, fallback: str) -> str: + disposition = "" + headers = getattr(response, "headers", None) + if headers is not None: + disposition = headers.get("content-disposition", "") or "" + match = re.search(r'filename="?([^";]+)"?', disposition) + if not match: + return fallback + return match.group(1).strip() or fallback diff --git a/testjam-client/tests/test_backup.py b/testjam-client/tests/test_backup.py new file mode 100644 index 0000000..25177b3 --- /dev/null +++ b/testjam-client/tests/test_backup.py @@ -0,0 +1,45 @@ +"""Backup download via the SDK. + +Hits the real `/settings/backup` endpoint through the in-process FastAPI +transport. SQLite mode is used in tests so the path exercises +`_sqlite_dump_to`, not `pg_dump`. +""" +from __future__ import annotations + +import io +import json +import zipfile + + +def test_download_bytes_returns_a_valid_zip(auth_client): + payload = auth_client.backup.download_bytes() + + with zipfile.ZipFile(io.BytesIO(payload)) as archive: + names = set(archive.namelist()) + assert "manifest.json" in names + assert "dump.sql" in names + manifest = json.loads(archive.read("manifest.json")) + assert manifest["format_version"] == 1 + + +def test_download_writes_archive_to_explicit_path(auth_client, tmp_path): + target = tmp_path / "snapshots" / "instance.zip" + + written = auth_client.backup.download(target) + + assert written == target + assert target.exists() + assert target.stat().st_size > 0 + with zipfile.ZipFile(target) as archive: + assert "manifest.json" in archive.namelist() + + +def test_download_into_directory_uses_content_disposition_filename(auth_client, tmp_path): + destination = tmp_path / "backups" + destination.mkdir() + + written = auth_client.backup.download(destination) + + assert written.parent == destination + assert written.name.startswith("testjam-backup-") + assert written.name.endswith(".zip") diff --git a/testjam-client/tests/test_cli_backup.py b/testjam-client/tests/test_cli_backup.py new file mode 100644 index 0000000..3d9b9dd --- /dev/null +++ b/testjam-client/tests/test_cli_backup.py @@ -0,0 +1,51 @@ +"""CLI smoke for `testjam backup`. + +Reuses the `auth_client` fixture (in-process FastAPI) and patches +`runtime.build_client` so the CLI talks to the same admin SDK instance. +""" +from __future__ import annotations + +import zipfile + +import pytest +from typer.testing import CliRunner + +from testjam_client.cli import app + +runner = CliRunner() + + +@pytest.fixture +def cli_admin(auth_client, tmp_path, monkeypatch): + config_file = tmp_path / "config.toml" + monkeypatch.setenv("TESTJAM_CONFIG", str(config_file)) + config_file.write_text('[profiles.default]\nurl = "http://testserver/api/v1"\n') + monkeypatch.setattr(auth_client, "close", lambda: None) + monkeypatch.setattr( + "testjam_client.cli.runtime.build_client", + lambda state: auth_client, + ) + return auth_client + + +def test_backup_writes_archive_to_directory(cli_admin, tmp_path): + target = tmp_path / "out" + + result = runner.invoke(app, ["--json", "backup", "--out", str(target)]) + + assert result.exit_code == 0, result.output + archives = list(target.glob("testjam-backup-*.zip")) + assert len(archives) == 1 + with zipfile.ZipFile(archives[0]) as archive: + assert "manifest.json" in archive.namelist() + + +def test_backup_writes_archive_to_explicit_file(cli_admin, tmp_path): + target = tmp_path / "snapshot.zip" + + result = runner.invoke(app, ["--json", "backup", "--out", str(target)]) + + assert result.exit_code == 0, result.output + assert target.exists() + with zipfile.ZipFile(target) as archive: + assert "manifest.json" in archive.namelist()