From 52f48e5ea60db940182366b4ac59df74a0d16a5f Mon Sep 17 00:00:00 2001 From: Duc Nguyen Date: Sat, 11 Apr 2026 21:22:04 +0700 Subject: [PATCH] fix(backup): detect pg server major for version-aware pg_dump hints (#829) (#830) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * fix(backup): detect pg server major for version-aware pg_dump hints pg_dump aborts when its major version is older than the server's, so backup preflight now runs SHOW server_version_num against the live PG server and uses the detected major to drive: - the missing-pg_dump hint (names the exact postgresqlNN-client) - a new compat check that flags an installed-but-too-old pg_dump as not-ready, instead of letting backup fail partway through the dump Adds ParsePgDumpMajor helper for Debian/Homebrew/EDB output shapes, covered by table-driven unit tests. Normalizes nil ctx once at RunPreflight boundary so downstream exec.CommandContext calls are safe. Stubs detectPGServerMajor + checkPgDumpServerCompat for the sqliteonly build. UI: removes the duplicate static hint from the preflight alert box so the dynamic, actionable warning is the single source of truth. Drops the obsolete pgDumpHint i18n key from en/vi/zh locale files. Refs #829 * fix(docker): bump runtime base alpine 3.22 → 3.23 Alpine 3.23 main ships postgresql16-client, postgresql17-client, and postgresql18-client simultaneously. Alpine 3.22 only shipped up to postgresql17-client, so the backup preflight's dynamic hint to install postgresql18-client previously failed with "no such package" on PG 18 deployments. No client package is pre-bundled: the on-demand install via the Packages page now resolves for any supported PG major. Refs #829 --- Dockerfile | 2 +- internal/backup/db_dump.go | 28 ++++++++ internal/backup/db_dump_test.go | 31 +++++++++ internal/backup/preflight.go | 46 ++++++++++++- internal/backup/preflight_pg.go | 68 +++++++++++++++++++ internal/backup/preflight_sqlite.go | 12 ++++ ui/web/src/i18n/locales/en/backup.json | 3 +- ui/web/src/i18n/locales/vi/backup.json | 3 +- ui/web/src/i18n/locales/zh/backup.json | 3 +- .../backup-restore/backup-preflight-panel.tsx | 1 - 10 files changed, 186 insertions(+), 11 deletions(-) create mode 100644 internal/backup/db_dump_test.go diff --git a/Dockerfile b/Dockerfile index 9d42ca70..b7ab822c 100644 --- a/Dockerfile +++ b/Dockerfile @@ -67,7 +67,7 @@ RUN set -eux; \ go build -ldflags="-s -w" -o /out/pkg-helper ./cmd/pkg-helper # ── Stage 2: Runtime ── -FROM alpine:3.22 +FROM alpine:3.23 ARG ENABLE_SANDBOX=false ARG ENABLE_PYTHON=false diff --git a/internal/backup/db_dump.go b/internal/backup/db_dump.go index cc04f46f..91c26075 100644 --- a/internal/backup/db_dump.go +++ b/internal/backup/db_dump.go @@ -9,6 +9,7 @@ import ( "io" "os" "os/exec" + "strconv" "strings" ) @@ -74,3 +75,30 @@ func PgDumpVersion(ctx context.Context) (string, error) { } return strings.TrimSpace(string(out)), nil } + +// ParsePgDumpMajor extracts the PostgreSQL major version number from a +// pg_dump --version string. Returns 0 if parsing fails. +// Example inputs: +// +// "pg_dump (PostgreSQL) 17.9 (Debian 17.9-1.pgdg12+1)" -> 17 +// "pg_dump (PostgreSQL) 18.3" -> 18 +func ParsePgDumpMajor(version string) int { + const marker = "(PostgreSQL) " + idx := strings.Index(version, marker) + if idx < 0 { + return 0 + } + rest := version[idx+len(marker):] + end := 0 + for end < len(rest) && rest[end] >= '0' && rest[end] <= '9' { + end++ + } + if end == 0 { + return 0 + } + major, err := strconv.Atoi(rest[:end]) + if err != nil { + return 0 + } + return major +} diff --git a/internal/backup/db_dump_test.go b/internal/backup/db_dump_test.go new file mode 100644 index 00000000..b66a5b16 --- /dev/null +++ b/internal/backup/db_dump_test.go @@ -0,0 +1,31 @@ +//go:build !sqliteonly + +package backup + +import "testing" + +func TestParsePgDumpMajor(t *testing.T) { + cases := []struct { + in string + want int + }{ + {"pg_dump (PostgreSQL) 17.9 (Debian 17.9-1.pgdg12+1)", 17}, + {"pg_dump (PostgreSQL) 18.3", 18}, + {"pg_dump (PostgreSQL) 18.3 (Homebrew)", 18}, + {"pg_dump (PostgreSQL) 10.21", 10}, + {"pg_dump (PostgreSQL) 9.6.24", 9}, + {"pg_dump (PostgreSQL) 18", 18}, + // Parse failures → 0 + {"", 0}, + {"pg_dump unknown", 0}, + {"pg_dump (PostgreSQL) vNext", 0}, + } + for _, tc := range cases { + t.Run(tc.in, func(t *testing.T) { + got := ParsePgDumpMajor(tc.in) + if got != tc.want { + t.Errorf("ParsePgDumpMajor(%q) = %d, want %d", tc.in, got, tc.want) + } + }) + } +} diff --git a/internal/backup/preflight.go b/internal/backup/preflight.go index a53ee4a8..b3c09d1a 100644 --- a/internal/backup/preflight.go +++ b/internal/backup/preflight.go @@ -35,12 +35,44 @@ type PreflightResult struct { // Checks: pg_dump binary, free disk space, estimated DB size (PG builds only). // A missing pg_dump makes ready=false, but filesystem-only backup may still work. func RunPreflight(ctx context.Context, dsn, dataDir, workspace string) *PreflightResult { + // Normalize a nil ctx once so every downstream helper (including + // exec.CommandContext, which panics on nil) is safe. Historical callers + // pass nil from tests; defensive at the boundary is cheaper than + // per-helper guards. + if ctx == nil { + ctx = context.Background() + } + var checks []PreflightCheck ready := true - pgDumpCheck := checkPgDump(ctx) + // Detect the server's PostgreSQL major version once (if a DSN is + // configured and reachable). We use it to tailor both the "pg_dump + // missing" hint and the "pg_dump incompatible" hint so the user always + // sees the exact postgresqlNN-client package to install. Returns 0 on + // any error or for SQLite builds (stub). + serverMajor := 0 + if dsn != "" { + serverMajor = detectPGServerMajor(ctx, dsn) + } + + pgDumpCheck := checkPgDump(ctx, serverMajor) checks = append(checks, pgDumpCheck) pgDumpAvail := pgDumpCheck.Status != "missing" + + // If pg_dump is present and we know the server major, verify its major + // version is compatible. pg_dump aborts at runtime when client major < + // server major, so we surface this up front as an actionable "not + // available" state rather than letting the backup fail partway through. + if pgDumpAvail && serverMajor > 0 { + compatCheck, compatOK := checkPgDumpServerCompat(ctx, serverMajor) + if compatCheck.Name != "" { // sqlite stub returns empty check + checks = append(checks, compatCheck) + if !compatOK { + pgDumpAvail = false + } + } + } if !pgDumpAvail { ready = false } @@ -83,17 +115,25 @@ func RunPreflight(ctx context.Context, dsn, dataDir, workspace string) *Prefligh } } -func checkPgDump(ctx context.Context) PreflightCheck { +// checkPgDump verifies pg_dump is on PATH. When serverMajor > 0 (detected +// from the live PG server), the missing-hint names the exact package to +// install (e.g. postgresql18-client). When serverMajor is 0 (no DSN, or +// server unreachable, or SQLite build), we fall back to a generic hint. +func checkPgDump(ctx context.Context, serverMajor int) PreflightCheck { if ctx == nil { ctx = context.Background() } path, err := exec.LookPath("pg_dump") if err != nil { + hint := "Install a PostgreSQL client package whose major version matches your server, or add pg_dump to PATH. Filesystem-only backup still works with --exclude-db." + if serverMajor > 0 { + hint = fmt.Sprintf("Install postgresql%d-client to match your PostgreSQL %d server, or add pg_dump to PATH. Filesystem-only backup still works with --exclude-db.", serverMajor, serverMajor) + } return PreflightCheck{ Name: "pg_dump", Status: "missing", Detail: "pg_dump not found on PATH", - Hint: "Install postgresql-client or add pg_dump to PATH. Filesystem-only backup still works with --exclude-db.", + Hint: hint, } } ver, verErr := PgDumpVersion(ctx) diff --git a/internal/backup/preflight_pg.go b/internal/backup/preflight_pg.go index 96a8efeb..93d65dcb 100644 --- a/internal/backup/preflight_pg.go +++ b/internal/backup/preflight_pg.go @@ -10,6 +10,74 @@ import ( _ "github.com/jackc/pgx/v5/stdlib" ) +// detectPGServerMajor queries the live PostgreSQL server for its major +// version number. Returns 0 on any error (no DSN, unreachable, parse +// failure) or for exotic values below the PG 8.x floor, so callers can +// fall back to a generic hint. Uses server_version_num which encodes +// major*10000 + minor for PG 10+ (e.g. 180003 = 18.3, 170009 = 17.9). +func detectPGServerMajor(ctx context.Context, dsn string) int { + if ctx == nil { + ctx = context.Background() + } + db, err := sql.Open("pgx", dsn) + if err != nil { + return 0 + } + defer db.Close() + + var serverNum int + if err := db.QueryRowContext(ctx, "SHOW server_version_num").Scan(&serverNum); err != nil { + return 0 + } + major := serverNum / 10000 + // Clamp implausible values (negative, zero, or pre-8 output from an + // exotic pooler) so we never render a hint like "postgresql-1-client". + if major < 8 { + return 0 + } + return major +} + +// checkPgDumpServerCompat verifies that the installed pg_dump can dump a +// PostgreSQL server of the given major version. pg_dump's major version must +// be >= the server's; otherwise pg_dump aborts at runtime with a version +// mismatch error. Returns (check, compatible). When the check cannot read +// pg_dump's version, the result is a non-fatal warning and compatible=true. +func checkPgDumpServerCompat(ctx context.Context, serverMajor int) (PreflightCheck, bool) { + name := "pg_version_compat" + + pgDumpVer, err := PgDumpVersion(ctx) + if err != nil { + return PreflightCheck{ + Name: name, + Status: "warning", + Detail: fmt.Sprintf("could not read pg_dump version: %v", err), + }, true + } + clientMajor := ParsePgDumpMajor(pgDumpVer) + if clientMajor == 0 { + return PreflightCheck{ + Name: name, + Status: "warning", + Detail: fmt.Sprintf("could not parse pg_dump version: %q", pgDumpVer), + }, true + } + + if clientMajor < serverMajor { + return PreflightCheck{ + Name: name, + Status: "missing", + Detail: fmt.Sprintf("pg_dump %d cannot dump PostgreSQL %d server (major version mismatch)", clientMajor, serverMajor), + Hint: fmt.Sprintf("Install postgresql%d-client to match the server major version %d.", serverMajor, serverMajor), + }, false + } + return PreflightCheck{ + Name: name, + Status: "ok", + Detail: fmt.Sprintf("pg_dump %d, server %d", clientMajor, serverMajor), + }, true +} + func checkDBSize(ctx context.Context, dsn string) (PreflightCheck, int64) { creds, err := ParseDSN(dsn) if err != nil { diff --git a/internal/backup/preflight_sqlite.go b/internal/backup/preflight_sqlite.go index 0f91ae89..8a04776f 100644 --- a/internal/backup/preflight_sqlite.go +++ b/internal/backup/preflight_sqlite.go @@ -8,6 +8,18 @@ import ( "os" ) +// detectPGServerMajor is a no-op for SQLite builds; always returns 0 so +// RunPreflight falls back to a generic pg_dump hint. +func detectPGServerMajor(_ context.Context, _ string) int { + return 0 +} + +// checkPgDumpServerCompat is a no-op for SQLite builds (no pg_dump involved). +// Returns an empty check (Name=="") so RunPreflight skips appending it. +func checkPgDumpServerCompat(_ context.Context, _ int) (PreflightCheck, bool) { + return PreflightCheck{}, true +} + func checkDBSize(ctx context.Context, dsn string) (PreflightCheck, int64) { dbPath := parseSQLitePath(dsn) if dbPath == "" { diff --git a/ui/web/src/i18n/locales/en/backup.json b/ui/web/src/i18n/locales/en/backup.json index b506710b..94106da6 100644 --- a/ui/web/src/i18n/locales/en/backup.json +++ b/ui/web/src/i18n/locales/en/backup.json @@ -18,8 +18,7 @@ "freeDisk": "Free disk space", "warnings": "Warnings", "refresh": "Refresh", - "pgDumpMissing": "pg_dump not available", - "pgDumpHint": "Install postgresql-client to enable database backups.", + "pgDumpMissing": "pg_dump not available or incompatible", "goToPackages": "Go to Packages" }, "options": { diff --git a/ui/web/src/i18n/locales/vi/backup.json b/ui/web/src/i18n/locales/vi/backup.json index af7f49f1..8e325078 100644 --- a/ui/web/src/i18n/locales/vi/backup.json +++ b/ui/web/src/i18n/locales/vi/backup.json @@ -18,8 +18,7 @@ "freeDisk": "Dung lượng trống", "warnings": "Cảnh báo", "refresh": "Làm mới", - "pgDumpMissing": "pg_dump không khả dụng", - "pgDumpHint": "Cài đặt postgresql-client để bật sao lưu cơ sở dữ liệu.", + "pgDumpMissing": "pg_dump không khả dụng hoặc không tương thích", "goToPackages": "Đi đến Gói cài đặt" }, "options": { diff --git a/ui/web/src/i18n/locales/zh/backup.json b/ui/web/src/i18n/locales/zh/backup.json index 76a2ebc1..9731506d 100644 --- a/ui/web/src/i18n/locales/zh/backup.json +++ b/ui/web/src/i18n/locales/zh/backup.json @@ -18,8 +18,7 @@ "freeDisk": "可用磁盘空间", "warnings": "警告", "refresh": "刷新", - "pgDumpMissing": "pg_dump 不可用", - "pgDumpHint": "安装 postgresql-client 以启用数据库备份。", + "pgDumpMissing": "pg_dump 不可用或版本不兼容", "goToPackages": "前往软件包" }, "options": { diff --git a/ui/web/src/pages/backup-restore/backup-preflight-panel.tsx b/ui/web/src/pages/backup-restore/backup-preflight-panel.tsx index 2a475f1c..7adbdc5b 100644 --- a/ui/web/src/pages/backup-restore/backup-preflight-panel.tsx +++ b/ui/web/src/pages/backup-restore/backup-preflight-panel.tsx @@ -91,7 +91,6 @@ export function BackupPreflightPanel() { {t("backup.preflight.pgDumpMissing")} - {t("backup.preflight.pgDumpHint")}{" "} {t("backup.preflight.goToPackages")}