From 430026ebe287037058318325e153a504695abbac Mon Sep 17 00:00:00 2001 From: Vishal Rana Date: Wed, 19 Aug 2026 08:37:50 -0700 Subject: [PATCH 01/30] feat(backup): make PostgreSQL point-in-time recovery executable MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Protection was fully described and entirely inert. The project schema accepted a policy, the lifecycle catalogue declared postgres qualified, the artifact set, locks, fences, journal, and scheduled-runner machinery were all built and unit-tested — and none of it had a production caller. `ob validate` accepted a 15-minute-RPO PITR policy and `ob preview` rendered a plain `postgres:17` with no archive command and no repository. A database could be declared protected and be backed up by nothing. This wires the vertical slice end to end for the postgres driver: - a derived image (images/postgres-pgbackrest) carrying pgBackRest, whose entrypoint assembles pgBackRest's configuration from the credential file on the host; - rendering that turns an established policy into archive_mode, an archive_command, and a repository configuration, gated on the durable lifecycle state rather than on the policy's presence; - `ob backup enable`, which pins the protected image by registry digest, restarts the service under it, creates the stanza, proves the archive path, and does not report success until a base backup exists; - `ob backup create` and `ob backup status`, the latter reading what the repository can recover rather than what the project claims. archive_timeout is derived from maximum_data_loss, which is what makes the declared RPO a bound rather than an aspiration: a quiet database otherwise archives only when a 16MB segment fills. Validated end to end against a real S3 repository: 2000 rows written, a recovery target marked, the table truncated, then recovered to that target in a clean volume with all 2000 rows intact while the live database stayed empty. Four defects found only by running it, each of which would have shipped: - PGBACKREST_REPO_PASSWORD is parsed by pgBackRest as a `repo-password` option that does not exist. The credential entry is now OB_REPOSITORY_PASSPHRASE, outside that namespace. - pgBackRest connects as the OS user, which is `postgres`; the driver creates `onebox`. pg1-user and pg1-database are now written. - Generated files are replaced atomically by rename, which gives the path a new inode — and a Docker bind-mount of a *file* is bound to the inode, so the configuration vanished from inside the running container. The configuration directory is mounted instead. - `tls: insecure` was rendered as unverified TLS over a plaintext endpoint. pgBackRest has no plaintext mode for S3, so a plaintext endpoint is now refused at render with what to do instead. Credentials never enter the project, the rendered document, or its digest: the document carries only the *names* of the credential entries, and the image's entrypoint reads the values from the mode-0600 file on the host. Restore, scheduling, and drills are not in this commit. `ob backup enable`, `create`, and `status` are. Co-Authored-By: Claude Opus 5 --- cmd/ob/backup.go | 124 +++++++ cmd/ob/main.go | 1 + cmd/ob/output.go | 3 + cmd/ob/output_test.go | 3 + images/postgres-pgbackrest/Dockerfile | 37 ++ .../ob-protected-entrypoint.sh | 69 ++++ internal/app/errors.go | 1 + internal/app/generate.go | 12 +- internal/app/names.go | 26 ++ internal/app/protection_artifacts.go | 14 + internal/app/protection_pgbackrest.go | 333 ++++++++++++++++++ internal/app/service_image_state_test.go | 38 +- internal/app/service_lifecycle.go | 19 + internal/app/service_lifecycle_records.go | 2 +- internal/app/services.go | 46 ++- internal/engine/protection_postgres.go | 163 +++++++++ internal/engine/protection_postgres_ops.go | 287 +++++++++++++++ internal/engine/service_apply.go | 10 + internal/engine/services.go | 4 + internal/onebox/binding.go | 5 +- internal/onebox/execute.go | 9 + internal/onebox/execution_types.go | 19 +- .../onebox/protection_credentials_test.go | 2 +- internal/onebox/protection_enable.go | 138 ++++++++ internal/onebox/protection_read.go | 31 ++ site/src/content/docs/reference/cli.mdx | 111 ++++++ site/src/content/docs/reference/errors.mdx | 1 + site/src/content/docs/reference/policies.mdx | 4 +- 28 files changed, 1492 insertions(+), 20 deletions(-) create mode 100644 cmd/ob/backup.go create mode 100644 images/postgres-pgbackrest/Dockerfile create mode 100644 images/postgres-pgbackrest/ob-protected-entrypoint.sh create mode 100644 internal/app/protection_pgbackrest.go create mode 100644 internal/engine/protection_postgres.go create mode 100644 internal/engine/protection_postgres_ops.go create mode 100644 internal/onebox/protection_enable.go create mode 100644 internal/onebox/protection_read.go diff --git a/cmd/ob/backup.go b/cmd/ob/backup.go new file mode 100644 index 00000000..a8201150 --- /dev/null +++ b/cmd/ob/backup.go @@ -0,0 +1,124 @@ +package main + +import ( + "encoding/json" + "fmt" + "text/tabwriter" + "time" + + "github.com/spf13/cobra" + + "github.com/labstack/onebox/internal/onebox" +) + +// `ob backup` is the operator's whole view of protection. +// +// Three verbs, and the split between them is the point. `enable` is the one +// that changes what the server is — it restarts the database under the +// protected image and does not return until a recoverable backup exists. +// `create` takes another one. `status` asks the repository, not the project, +// what can actually be recovered; everything it prints comes from pgBackRest. +// +// There is deliberately no verb that reports protection as established from the +// project alone. A policy in `ob.yml` is a request, and until `enable` has +// succeeded the service renders as an ordinary unprotected server. +func addBackupCommands(root *cobra.Command, g *globalFlags) { + backupCmd := &cobra.Command{ + Use: "backup", + Short: "protect a data service and inspect what can be recovered", + Long: "Backup and recovery for the data services this project declares.\n\n" + + "Protection is physical: a base backup plus continuous WAL archiving to the\n" + + "off-host repository the project's backup_targets name, which is what makes\n" + + "recovery to a point in time possible rather than recovery to last night.\n\n" + + "Declaring a policy does not establish it. `ob backup enable` restarts the\n" + + "service under the protected image, creates the repository stanza, and takes\n" + + "the first base backup; only then does the service render as protected.", + Args: cobra.NoArgs, RunE: showCommandHelp, + } + + var enableBreakLock bool + enableCmd := &cobra.Command{ + Use: "enable ", + Short: "establish protection — restarts the service, creates the stanza, takes the first backup", + Long: "Make a declared protection policy real.\n\n" + + "The order is forced by PostgreSQL: a server cannot archive to a stanza that\n" + + "does not exist, and a stanza cannot be created against a server that is not\n" + + "already archiving. So this pins the protected image, writes the repository\n" + + "configuration, restarts the server with archiving on, creates the stanza,\n" + + "and takes a full backup.\n\n" + + "The restart is a real restart of the database. It is not complete until the\n" + + "first backup exists, because a stanza with no backup can recover nothing.", + Args: cobra.ExactArgs(1), + RunE: func(cmd *cobra.Command, args []string) error { + return runMutation(cmd, g, onebox.ExecuteRequest{ + Kind: onebox.KindProtectionEnable, Service: args[0], BreakLock: enableBreakLock, + }, "backup enable") + }, + } + enableCmd.Flags().BoolVar(&enableBreakLock, "break-lock", false, "break a stale operation lock after inspecting its holder") + backupCmd.AddCommand(enableCmd) + + var backupType string + var createBreakLock bool + createCmd := &cobra.Command{ + Use: "create ", + Short: "take a base backup now", + Long: "Take a base backup of a protected service.\n\n" + + "Retention counts full generations, so only --type full starts a new one; a\n" + + "diff or incr backup extends the newest full and is expired with it.\n\n" + + "WAL archiving runs continuously and is not this command: between backups the\n" + + "recoverable point keeps advancing on its own.", + Args: cobra.ExactArgs(1), + RunE: func(cmd *cobra.Command, args []string) error { + return runMutation(cmd, g, onebox.ExecuteRequest{ + Kind: onebox.KindBackupCreate, Service: args[0], + BackupType: backupType, BreakLock: createBreakLock, + }, "backup create") + }, + } + createCmd.Flags().StringVar(&backupType, "type", "full", "full, diff, or incr") + createCmd.Flags().BoolVar(&createBreakLock, "break-lock", false, "break a stale operation lock after inspecting its holder") + backupCmd.AddCommand(createCmd) + + statusCmd := &cobra.Command{ + Use: "status ", + Short: "what the repository can recover, read from the repository", + Long: "Report what is actually recoverable.\n\n" + + "Every figure comes from the repository rather than from the project: the\n" + + "policy states what should be true, and this states what is. A service whose\n" + + "policy is declared but never enabled has no repository to ask, and says so.", + Args: cobra.ExactArgs(1), + RunE: func(cmd *cobra.Command, args []string) error { + status, err := operationsService(cmd, g).ProtectionStatus(cmd.Context(), args[0]) + if err != nil { + return err + } + if isStructuredOutput(g) { + encoder := json.NewEncoder(cmd.OutOrStdout()) + encoder.SetIndent("", " ") + return encoder.Encode(status) + } + out := cmd.OutOrStdout() + fmt.Fprintf(out, "service %s\nstanza %s\nstate %s\n", status.Service, status.Stanza, status.State) + if status.ArchiveMin != "" { + fmt.Fprintf(out, "wal %s .. %s\n", status.ArchiveMin, status.ArchiveMax) + } + if len(status.Generations) == 0 { + fmt.Fprintln(out, "\nno recoverable generation yet") + return nil + } + fmt.Fprintln(out) + w := tabwriter.NewWriter(out, 0, 0, 2, ' ', 0) + fmt.Fprintln(w, "LABEL\tTYPE\tCOMPLETED\tWAL") + for _, generation := range status.Generations { + fmt.Fprintf(w, "%s\t%s\t%s\t%s..%s\n", generation.Label, generation.Type, + time.Unix(generation.StoppedAt, 0).UTC().Format(time.RFC3339), + generation.WALStart, generation.WALStop) + } + return w.Flush() + }, + } + backupCmd.AddCommand(statusCmd) + + root.AddCommand(backupCmd) +} diff --git a/cmd/ob/main.go b/cmd/ob/main.go index f1e4a8cd..d1bea48d 100644 --- a/cmd/ob/main.go +++ b/cmd/ob/main.go @@ -62,6 +62,7 @@ func newRootCmd() *cobra.Command { addJobCommand(root, g) addInitCommand(root, g) addOpsCommands(root, g) + addBackupCommands(root, g) addPreviewCommand(root, g) addSchemaCommand(root, g) addPreflightCommand(root, g) diff --git a/cmd/ob/output.go b/cmd/ob/output.go index 37ebb0f6..b0989f43 100644 --- a/cmd/ob/output.go +++ b/cmd/ob/output.go @@ -87,6 +87,9 @@ var cliOutputMatrix = map[string]cliOutputClass{ "ob eject": {Class: cliClassFiniteEnvelope, JSON: true}, "ob exec": {Class: cliClassOperatorPassthrough, NDJSON: true}, "ob init": {Class: cliClassFiniteEnvelope, JSON: true}, + "ob backup create": {Class: cliClassFiniteStream, JSON: true, NDJSON: true}, + "ob backup enable": {Class: cliClassFiniteStream, JSON: true, NDJSON: true}, + "ob backup status": {Class: cliClassFiniteEnvelope, JSON: true}, "ob job plan": {Class: cliClassFiniteEnvelope, JSON: true}, "ob job run": {Class: cliClassFiniteStream, JSON: true, NDJSON: true}, "ob logs": {Class: cliClassOperatorPassthrough, JSON: true, NDJSON: true}, diff --git a/cmd/ob/output_test.go b/cmd/ob/output_test.go index 2c1ef4fe..d81b01d8 100644 --- a/cmd/ob/output_test.go +++ b/cmd/ob/output_test.go @@ -502,6 +502,9 @@ func TestLeafOutputMatrixIsClosedAndHasNoAliases(t *testing.T) { "ob exec": {Class: "operator_passthrough", NDJSON: true}, "ob init": {Class: "finite_envelope", JSON: true}, "ob job plan": {Class: "finite_envelope", JSON: true}, + "ob backup create": {Class: "finite_stream", JSON: true, NDJSON: true}, + "ob backup enable": {Class: "finite_stream", JSON: true, NDJSON: true}, + "ob backup status": {Class: "finite_envelope", JSON: true}, "ob job run": {Class: "finite_stream", JSON: true, NDJSON: true}, "ob logs": {Class: "operator_passthrough", JSON: true, NDJSON: true}, "ob plan": {Class: "finite_envelope", JSON: true}, diff --git a/images/postgres-pgbackrest/Dockerfile b/images/postgres-pgbackrest/Dockerfile new file mode 100644 index 00000000..7bedc617 --- /dev/null +++ b/images/postgres-pgbackrest/Dockerfile @@ -0,0 +1,37 @@ +# The protected PostgreSQL image. +# +# Onebox runs pgBackRest in the same container as the server, because that is +# what physical backup requires: pgBackRest reads and writes the data directory +# directly, and the server's archive_command invokes it synchronously for every +# completed WAL segment. A sidecar cannot do either. +# +# The base is the official image at an exact major, and pgBackRest comes from +# PGDG — the same apt repository the base image already trusts for the server. +# The Alpine package was tried first and rejected: its pgbackrest pulls in a +# complete second PostgreSQL server (18) next to the 17 the image runs, so the +# image would ship two majors and a `postgresql-common` default pointing at the +# wrong one. +ARG POSTGRES_VERSION=17 +FROM postgres:${POSTGRES_VERSION}-bookworm + +# The base image already has the PGDG repository and its key configured, which +# is what makes this a one-package addition rather than a repository setup. +RUN set -eux; \ + apt-get update; \ + apt-get install -y --no-install-recommends pgbackrest; \ + rm -rf /var/lib/apt/lists/*; \ + pgbackrest version + +# pgBackRest's runtime directories must exist and be owned by postgres before +# the first archive-push: it creates none of them, and the failure surfaces +# later as a WAL archive that silently never drains. +RUN set -eux; \ + mkdir -p /etc/pgbackrest/conf.d /etc/onebox /var/log/pgbackrest /var/spool/pgbackrest; \ + chown -R postgres:postgres /etc/pgbackrest /var/log/pgbackrest /var/spool/pgbackrest; \ + chmod 0750 /var/log/pgbackrest /var/spool/pgbackrest + +COPY ob-protected-entrypoint.sh /usr/local/bin/ob-protected-entrypoint.sh +RUN chmod 0755 /usr/local/bin/ob-protected-entrypoint.sh + +ENTRYPOINT ["/usr/local/bin/ob-protected-entrypoint.sh"] +CMD ["postgres"] diff --git a/images/postgres-pgbackrest/ob-protected-entrypoint.sh b/images/postgres-pgbackrest/ob-protected-entrypoint.sh new file mode 100644 index 00000000..34c601a8 --- /dev/null +++ b/images/postgres-pgbackrest/ob-protected-entrypoint.sh @@ -0,0 +1,69 @@ +#!/bin/sh +# Assembles pgBackRest's configuration, then hands over to the official +# entrypoint unchanged. +# +# Why this exists at all, and why it is not a wrapper around the pgbackrest +# binary: pgBackRest is invoked along three paths that share nothing. +# +# 1. The server's archive_command, a child of postmaster. +# 2. The restore_command pgBackRest writes into postgresql.auto.conf during a +# restore. That line is generated as the *absolute resolved path* of the +# real binary and cannot be configured, so no wrapper — by name, by PATH, +# or by argv[0] — is reachable from it. +# 3. `docker exec ... pgbackrest`, which inherits the container's configured +# environment but nothing an entrypoint exported. +# +# A credential passed through the environment covers 1 and 3 and misses 2, which +# means it works until the moment it is needed. So the credentials are put where +# every path finds them regardless of who invoked the binary or how: pgBackRest's +# own configuration, in the include directory it reads by default. +# +# Onebox's generated configuration is mounted read-only elsewhere and symlinked +# in rather than copied, so regenerating it on the host takes effect without +# rebuilding or restarting anything. +set -eu + +ob_indirect() { + # POSIX sh has no ${!name}. eval on a name already checked against the + # variable-name grammar is the portable equivalent. + case "$1" in + [A-Za-z_][A-Za-z0-9_]*) ;; + *) echo "ob: $1 is not a variable name" >&2; exit 1 ;; + esac + eval "printf '%s' \"\${$1-}\"" +} + +if [ -f /etc/onebox/pgbackrest.conf ]; then + mkdir -p /etc/pgbackrest/conf.d + ln -sfn /etc/onebox/pgbackrest.conf /etc/pgbackrest/pgbackrest.conf + + # Written with a restrictive umask *before* any content, so the file is + # never briefly readable with the credentials already in it. + credentials=/etc/pgbackrest/conf.d/ob-credentials.conf + ( + umask 077 + { + echo "# Generated at container start from the credential file on the host." + echo "[global]" + [ -n "${OB_S3_KEY_ENTRY-}" ] && \ + echo "repo1-s3-key=$(ob_indirect "$OB_S3_KEY_ENTRY")" + [ -n "${OB_S3_SECRET_ENTRY-}" ] && \ + echo "repo1-s3-key-secret=$(ob_indirect "$OB_S3_SECRET_ENTRY")" + [ -n "${OB_S3_SESSION_TOKEN_ENTRY-}" ] && \ + echo "repo1-s3-token=$(ob_indirect "$OB_S3_SESSION_TOKEN_ENTRY")" + # The repository passphrase has a fixed name: it is Onebox's own + # requirement rather than a property of the destination, so there is + # no backup_targets field to indirect through. The OB_ prefix is not + # decoration — pgBackRest reads every PGBACKREST_