feat: additional config for pgbackrest (#2099)

* feat: additional config for pgbackrest

- pgdata-signal: add remove-pid action to remove stale postmaster.pid
  via the constrained wrapper rather than a broad sudo rm entry,
  keeping the sudoers scope limited to this script

* fix: address pgbackrest PR review feedback

- pgdata-chown: simplify case; use group=postgres consistently for both
  ownership targets (pgbackrest:postgres and postgres:postgres)
- pgdata-signal: consolidate recovery/standby case into single pattern
- pgdata-signal: deploy at mode 0755 so postgres can execute via sudo -u
- setup-pgbackrest.yml: combine dir creation into single task with dict
  loop; conf.d gets 02770 setgid, others get default 0770
- setup-pgbackrest.yml: sort logrotate task keys alphabetically

* fix: add missing pgbackrest sudoers entries and pre-create log files

Three gaps found by cross-referencing SAA commands against Ansible:

1. adminapi.sudoers.conf: add two entries so adminapi can call the
   pgbackrest binary via the wrapper.
   - NewRunner() path: wrapper calls sudo -u pgbackrest <real binary>,
     requires adminapi -> pgbackrest NOPASSWD for the real binary path.
   - NewRunnerAs("pgbackrest") path: SAA does sudo -n -u pgbackrest
     /usr/bin/pgbackrest, requires adminapi -> pgbackrest NOPASSWD for
     the wrapper path.

2. setup-pgbackrest.yml: add pgbackrest -> pgbackrest sudoers entry for
   the real binary. When NewRunnerAs runs the wrapper as the pgbackrest
   user, the wrapper still calls sudo -u pgbackrest internally; without
   this entry that inner sudo fails.

3. setup-pgbackrest.yml: pre-create the three SAA log files
   (saa-pgb.log, wal-push.log, wal-fetch.log) as pgbackrest:postgres
   0660. SAA opens them with O_APPEND|O_WRONLY (no O_CREATE) — a missing
   file causes enable to fail immediately before any pgBackRest work.
   modification_time/access_time: preserve means the task is idempotent.

* Update ansible/files/supabase_admin_agent_config/pgdata-signal

Co-authored-by: Tom Ashley <tom.ashley@gmail.com>

* Update ansible/files/supabase_admin_agent_config/pgdata-signal

Co-authored-by: Tom Ashley <tom.ashley@gmail.com>

* Update ansible/tasks/internal/supabase-admin-agent.yml

Co-authored-by: Tom Ashley <tom.ashley@gmail.com>

* fix: use peer auth with saa_map for supabase_admin local connections

Replace trust with peer map=saa_map for the supabase_admin pg_hba rule.
Add saa_map entries in pg_ident.conf mapping adminapi and root OS users
to the supabase_admin PG user, as required by supabase-admin-agent.

* fix: restore pgdata-signal mode to 0755

pgdata-signal is called via sudo as postgres (other user), so it must
be world-executable. 0700 would prevent postgres from running it.

* fix: revert supabase_admin auth to trust, remove saa_map from pg_ident

peer map=saa_map cannot be cleanly implemented in pg_hba.conf.j2 while
the Dockerfiles copy it raw — pg_hba include_if_exists (PG 16+) is
needed to separate Docker from production, and that work is in progress
on a separate branch.

Trust is appropriate for this local Unix-socket-only connection; the
security boundary is OS-level access to the machine. Revisit once the
include directive infrastructure lands.

* docs: add comment to supabase_admin trust rule in pg_hba.conf

* docs: add explanatory comments to pgBackRest Ansible changes

Clarify the intent and rationale behind the less-obvious changes:
- adminapi.sudoers.conf: explain the two pgbackrest sudo chains
  (NewRunner vs NewRunnerAs) and the wrapper/real-binary split
- supabase-admin-agent.yml: explain why pgdata-chown is 0700 and
  pgdata-signal is 0755
- setup-pgbackrest.yml: explain the pgbackrest self-sudo entry,
  why conf.d needs setgid (02770), and why SAA log files must be
  pre-created before the agent runs
- pgbackrest.conf: explain expire-auto change and why the [supabase]
  stanza was removed (SAA owns it via conf.d to avoid error [031])

* fix: move pgbackrest logrotate config to follow repo convention

- Move from pgbackrest_config/ to logrotate_config/ to match the
  existing pattern for all other logrotate configs in the repo
- Deploy via the finalize-ami.yml loop rather than setup-pgbackrest.yml
- Add size 50M cap to prevent logs growing unbounded between daily
  rotations during large backup/restore operations

* fix: run pgBackRest directory and log file setup in stage2_nix pass

The directory and log file creation tasks were gated on nixpkg_mode
only, so they were skipped during the stage 2 Nix provision pass
(nixpkg_mode=false, stage2_nix=true). SAA is installed in stage 2 but
had no log directory or pre-created log files to write to, causing
pgbackrest enable to fail immediately.

Extend both conditions to nixpkg_mode or stage2_nix so the paths are
created in whichever pass installs the software.

* fix: set data_directory to canonical path to resolve pgBackRest [058]

pgBackRest stanza-create compares the live cluster's data_directory
against the DataDir stored in pg_control. initdb resolves symlinks
internally so pg_control stores /data/pgdata, but postgresql.conf
explicitly set data_directory to /var/lib/postgresql/data (the symlink),
causing a path mismatch and [058] error on every enable attempt.

Patch data_directory to /data/pgdata immediately after deploying the
postgresql.conf template, scoped to the debpkg_mode/nixpkg_mode block
where the /data volume and symlink are always present. Docker images and
Nix test environments are unaffected as they do not use this Ansible path.

* fix: allow SAA execution from postgres AppArmor confinement

Remove explicit `deny /** x,` from postgres_shell and pgbackrest_shell
sub-profiles — the deny keyword has absolute precedence in AppArmor,
overriding all specific ix allow rules and blocking SAA execution with
exit 126.

Add SAA to the parent sbpostgres profile with Pix -> postgres_shell so
that the archive_command path (shell staying in sbpostgres due to Pix
fallback) can exec SAA and transition it into postgres_shell. Also add
/nix/store/*/bin/sh to the shell transition rules to cover nix-built
postgres popen() behaviour.

* chore: bump postgres release versions for pgbackrest AMI

* fix: align AppArmor profile with salt managed version

Sync sbpostgres_apparmor with the salt repo (PR #526, merged):
- Add WAL dir rwkl entries to parent and both sub-profiles
- Consolidate systemd notify to single /{,var/}run/systemd/notify rule
- Add /usr/bin/kill and signal hup rule to parent
- Add pg_archivecleanup ix to postgres_shell

---------

Co-authored-by: Douglas J Hunley <doug.hunley@gmail.com>
Co-authored-by: Tom Ashley <tom.ashley@gmail.com>

Author: 3 people

ansible/files/adminapi.sudoers.conf

@@ -15,6 +15,21 @@ Cmnd_Alias PGBOUNCER = /bin/systemctl start pgbouncer.service, /bin/systemctl st
 %adminapi ALL= NOPASSWD: /etc/adminapi/pg_upgrade_scripts/common.sh
 %adminapi ALL= NOPASSWD: /etc/adminapi/pg_upgrade_scripts/pgsodium_getkey.sh
 %adminapi ALL= NOPASSWD: /usr/bin/systemctl daemon-reload
+# pgBackRest wrapper scripts: constrained helpers called by supabase-admin-agent.
+# pgdata-chown runs as root (default); pgdata-signal runs as postgres so it can
+# create/remove signal files owned by that user.
+%adminapi ALL= NOPASSWD: /usr/local/lib/supabase-admin-agent/pgdata-chown
+%adminapi ALL=(postgres) NOPASSWD: /usr/local/lib/supabase-admin-agent/pgdata-signal
+# pgBackRest binary entries support two sudo chains used by supabase-admin-agent:
+# NewRunner()      — adminapi → /usr/bin/pgbackrest wrapper → sudo -u pgbackrest real_binary
+# NewRunnerAs()    — adminapi → sudo -u pgbackrest /usr/bin/pgbackrest → sudo -u pgbackrest real_binary
+# Both paths require the wrapper entry; NewRunner() additionally needs the real binary entry
+# because the wrapper itself calls sudo to drop privileges to the pgbackrest user.
+%adminapi ALL=(pgbackrest) NOPASSWD: /var/lib/pgbackrest/.nix-profile/bin/pgbackrest
+%adminapi ALL=(pgbackrest) NOPASSWD: /usr/bin/pgbackrest
+# pgBackRest restore stops PostgreSQL, restores PGDATA, then starts PostgreSQL.
+%adminapi ALL= NOPASSWD: /usr/bin/systemctl start postgresql.service
+%adminapi ALL= NOPASSWD: /usr/bin/systemctl stop postgresql.service
 %adminapi ALL= NOPASSWD: /usr/bin/systemctl reload postgresql.service
 %adminapi ALL= NOPASSWD: /usr/bin/systemctl restart postgresql.service
 %adminapi ALL= NOPASSWD: /usr/bin/systemctl show -p NRestarts postgresql.service

ansible/files/logrotate_config/logrotate-pgbackrest.conf

@@ -0,0 +1,10 @@
+/var/log/pgbackrest/*.log {
+    daily
+    size 50M
+    rotate 7
+    compress
+    delaycompress
+    missingok
+    notifempty
+    create 0660 pgbackrest postgres
+}

ansible/files/pgbackrest_config/pgbackrest.conf

@@ -4,15 +4,17 @@ archive-copy = y
 backup-standby = prefer
 compress-type = zst
 delta = y
-expire-auto = n
+# expire-auto=y allows pgBackRest to expire old backups automatically after
+# each new backup completes, keeping retention in sync without a separate
+# expire command. Previously n; changed to work correctly with SAA-managed backups.
+expire-auto = y
 link-all = y
 log-level-console = info
 log-level-file = detail
 log-subprocess = y
 resume = n
 start-fast = y
-
-[supabase]
-pg1-path = /var/lib/postgresql/data
-pg1-socket-path = /run/postgresql
-pg1-user = supabase_admin
+# Note: the [supabase] stanza (pg1-path, pg1-socket-path, pg1-user) has been
+# removed from this file. supabase-admin-agent owns that stanza and writes it
+# to /etc/pgbackrest/conf.d/ at enable time. Having the stanza in both places
+# causes pgBackRest error [031] (duplicate option).

ansible/files/postgresql_config/pg_hba.conf.j2

@@ -79,7 +79,10 @@
 # TYPE  DATABASE        USER            ADDRESS                 METHOD
 
 # trust local connections
-local all  supabase_admin     scram-sha-256
+# supabase_admin: trust over Unix socket only — no network exposure. Access
+# requires OS-level shell access to the host, which is already a stronger
+# security boundary than a database password.
+local all  supabase_admin     trust
 local all  all                peer map=supabase_map
 host  all  all  127.0.0.1/32  trust
 host  all  all  ::1/128       trust

ansible/files/postgresql_config/sbpostgres_apparmor

@@ -24,14 +24,13 @@ profile sbpostgres flags=(attach_disconnected) {
     # lock permission for pgroonga
     /data/pgdata/pgroonga.log k,
 
+    # WAL directory - needs link permission for segment recycling
+    /data/pgdata/pg_wal/** rwkl,
+    /data/pgdata/pg_wal/  rw,
+
     # Systemd notification socket (for Type=notify services)
-    /run/systemd/notify w,
     /{,var/}run/systemd/notify w,
 
-    # Allow disconnected paths (for mount namespaces)
-    @{run}/systemd/notify w,
-    /run/systemd/notify w,
-
     # Full network access
     network inet stream,
     network inet6 stream,
@@ -50,6 +49,8 @@ profile sbpostgres flags=(attach_disconnected) {
     /usr/bin/false ix,
     # kill needed for postgresql to reload itself
     /bin/kill ix,
+    /usr/bin/kill ix,
+    signal (send) set=(hup) peer=unconfined,
 
     # When parent executes shell, transition to restricted profile
     # This accounts for popen, which postgres uses for any child process
@@ -71,7 +72,15 @@ profile sbpostgres flags=(attach_disconnected) {
     /usr/bin/nix Pix -> pgbackrest_shell,
     /usr/bin/sudo Pix -> pgbackrest_shell,
 
-    profile postgres_shell {
+    # nix-built postgres popen() may use a nix store sh instead of /bin/sh
+    /nix/store/*/bin/sh Pix -> postgres_shell,
+
+    # SAA called directly from sbpostgres context (e.g. shell Pix fallback)
+    /opt/supabase-admin-agent/supabase-admin-agent Pix -> postgres_shell,
+    /opt/supabase-admin-agent/supabase-admin-agent-linux-arm64 Pix -> postgres_shell,
+    /opt/supabase-admin-agent/supabase-admin-agent-linux-amd64 Pix -> postgres_shell,
+
+  profile postgres_shell {
         #include <abstractions/base>
 
         /usr/bin/* m,
@@ -93,17 +102,23 @@ profile sbpostgres flags=(attach_disconnected) {
         # backup things
         /usr/lib/postgresql/bin/pgsodium_getkey.sh ix,
         /usr/bin/admin-mgr ix,
+        /opt/supabase-admin-agent/supabase-admin-agent ix,
+        /opt/supabase-admin-agent/supabase-admin-agent-linux-arm64 ix,
+        /opt/supabase-admin-agent/supabase-admin-agent-linux-amd64 ix,
         /nix/store/*/bin/.postgres-wrapped ix,
         /nix/store/*/bin/wal-g-2 ix,
         /nix/store/*/bin/pgbackrest ix,
         /nix/store/*/bin/pg_dump ix,
+        /nix/store/*/bin/pg_archivecleanup ix,
 
         # file path permissions
         /** r,
         /data/wal_fetch_dir/ rw,
         /tmp/wal_fetch_dir/ rw,
         /var/lib/postgresql/data rw,
         /data/pgdata rw,
+        /data/pgdata/pg_wal/** rwkl,
+        /data/pgdata/pg_wal/  rw,
         /data/latest-lsn-checkpoint-v2 rw,
         /data/previous-lsn-checkpoint-v2 rw,
         /var/lib/postgresql/data/recovery.signal rw,
@@ -136,11 +151,9 @@ profile sbpostgres flags=(attach_disconnected) {
         # tools have network access
         network,
 
-        # Block everything else
-        deny /** x,
     }
 
-    profile pgbackrest_shell {
+  profile pgbackrest_shell {
         #include <abstractions/base>
 
         /usr/bin/* m,
@@ -167,6 +180,8 @@ profile sbpostgres flags=(attach_disconnected) {
         /tmp/wal_fetch_dir/ rw,
         /var/lib/postgresql/data rw,
         /data/pgdata rw,
+        /data/pgdata/pg_wal/** rwkl,
+        /data/pgdata/pg_wal/  rw,
         /data/latest-lsn-checkpoint-v2 rw,
         /data/previous-lsn-checkpoint-v2 rw,
         /var/lib/postgresql/data/recovery.signal rw,
@@ -196,7 +211,5 @@ profile sbpostgres flags=(attach_disconnected) {
         # tools have network access
         network,
 
-        # Block everything else
-        deny /** x,
     }
 }

ansible/files/supabase_admin_agent_config/pgdata-chown

@@ -0,0 +1,34 @@
+#!/usr/bin/env bash
+# pgdata-chown — transfers PGDATA ownership for pgBackRest restore operations.
+#
+# Called via sudo by supabase-admin-agent (running as adminapi).  Only two
+# actions are accepted, and the target path must resolve to /data/pgdata or a
+# path beneath it.  realpath(1) is used to expand symlinks before the check,
+# which prevents directory-traversal attacks (e.g. /data/pgdata/../../etc/sudoers).
+#
+# Usage: pgdata-chown <to-pgbackrest|to-postgres> <path>
+set -euo pipefail
+
+if [[ $# -ne 2 ]]; then
+  echo "usage: pgdata-chown <to-pgbackrest|to-postgres> <path>" >&2
+  exit 1
+fi
+
+ACTION="$1"
+TARGET="$2"
+
+REAL=$(realpath "$TARGET")
+if [[ "$REAL" != "/data/pgdata" && "$REAL" != /data/pgdata/* ]]; then
+  echo "error: '${TARGET}' resolves to '${REAL}', which is not under /data/pgdata" >&2
+  exit 1
+fi
+
+case "$ACTION" in
+  to-pgbackrest|to-postgres)
+    exec /usr/bin/chown -R "${ACTION:3}:postgres" "$REAL"
+    ;;
+  *)
+    echo "error: unknown action '${ACTION}'; expected to-pgbackrest or to-postgres" >&2
+    exit 1
+    ;;
+esac

ansible/files/supabase_admin_agent_config/pgdata-signal

@@ -0,0 +1,44 @@
+#!/usr/bin/env bash
+# pgdata-signal — creates or removes PostgreSQL signal files and stale pid files.
+# Called via sudo (as postgres) by supabase-admin-agent (running as adminapi).
+#
+# All file paths are hardcoded to prevent path injection.  No external
+# path argument is accepted.
+#
+# Usage: pgdata-signal <create|remove> <recovery|standby>
+#        pgdata-signal remove-pid
+set -euo pipefail
+
+# Special-case: remove-pid removes the stale postmaster.pid file that would
+# prevent PostgreSQL from starting after a restore.  Handled as a single-arg
+# command to keep the sudoers entry scoped to this script rather than allowing
+# a broad "rm" entry.
+if [[ $# -eq 1 && "$1" == "remove-pid" ]]; then
+  exec /usr/bin/rm -f "/data/pgdata/postmaster.pid"
+fi
+
+if [[ $# -ne 2 ]]; then
+  echo "usage: pgdata-signal <create|remove> <recovery|standby>" >&2
+  echo "       pgdata-signal remove-pid" >&2
+  exit 1
+fi
+
+ACTION="$1"
+SIGNAL_TYPE="$2"
+
+case "$SIGNAL_TYPE" in
+  recovery|standby) FILE="/data/pgdata/${SIGNAL_TYPE}.signal" ;;
+  *)
+    echo "error: unknown signal type '${SIGNAL_TYPE}'; expected recovery or standby" >&2
+    exit 1
+    ;;
+esac
+
+case "$ACTION" in
+  create) exec /usr/bin/touch "${FILE}" ;;
+  remove) exec /usr/bin/rm -f "${FILE}" ;;
+  *)
+    echo "error: unknown action '${ACTION}'; expected create or remove" >&2
+    exit 1
+    ;;
+esac

ansible/tasks/finalize-ami.yml

@@ -61,6 +61,7 @@
     - { file: 'logrotate-postgres-auth.conf' }
     - { file: 'logrotate-postgres-csv.conf' }
     - { file: 'logrotate-walg.conf' }
+    - { file: 'logrotate-pgbackrest.conf' }
   loop_control:
     loop_var: 'logrotate_item'
 

ansible/tasks/internal/supabase-admin-agent.yml

@@ -31,6 +31,33 @@
     dest: /etc/sudoers.d/supabase-admin-agent
     mode: "0440"
 
+- name: supabase-admin-agent - pgbackrest helper scripts dir
+  file:
+    path: /usr/local/lib/supabase-admin-agent
+    state: directory
+    owner: root
+    group: root
+    mode: "0755"
+
+- name: supabase-admin-agent - pgdata-chown script
+  copy:
+    src: files/supabase_admin_agent_config/pgdata-chown
+    dest: /usr/local/lib/supabase-admin-agent/pgdata-chown
+    owner: root
+    group: root
+    # 0700: always invoked via "sudo" (as root); no other user needs execute.
+    mode: "0700"
+
+- name: supabase-admin-agent - pgdata-signal script
+  copy:
+    src: files/supabase_admin_agent_config/pgdata-signal
+    dest: /usr/local/lib/supabase-admin-agent/pgdata-signal
+    owner: root
+    group: root
+    # 0755: invoked via "sudo -u postgres"; the postgres OS user needs the
+    # world-execute bit since it is neither owner nor group member.
+    mode: "0755"
+
 - name: Setting arch (amd64)
   set_fact:
     arch: "amd64"

ansible/tasks/setup-pgbackrest.yml

@@ -30,6 +30,11 @@
     - 'postgres ALL=(pgbackrest) NOPASSWD: /usr/bin/bash'
     - 'postgres ALL=(pgbackrest) NOPASSWD: /usr/bin/nix'
     - 'pgbackrest ALL=(pgbackrest) NOPASSWD: /usr/bin/bash'
+    # Required for the NewRunnerAs() path: supabase-admin-agent runs the
+    # /usr/bin/pgbackrest wrapper as the pgbackrest user, but the wrapper
+    # internally calls "sudo -u pgbackrest <real_binary>". Without this entry
+    # that inner sudo fails even though the outer caller is already pgbackrest.
+    - 'pgbackrest ALL=(pgbackrest) NOPASSWD: /var/lib/pgbackrest/.nix-profile/bin/pgbackrest'
 
 - name: Install pgBackRest
   ansible.builtin.shell: |
@@ -43,19 +48,42 @@
 - name: Create needed directories for pgBackRest
   ansible.legacy.file:
     group: postgres
-    mode: '0770'
+    mode: "{{ backrest_dir['mode'] | default('0770', true) }}"
     owner: pgbackrest
-    path: "{{ backrest_dir }}"
+    path: "{{ backrest_dir['dir'] }}"
     state: directory
   loop:
-    - /etc/pgbackrest/conf.d
-    - /var/lib/pgbackrest
-    - /var/spool/pgbackrest
-    - /var/log/pgbackrest
+    # conf.d is setgid (02770) so files written by adminapi (postgres group)
+    # automatically inherit the postgres group. Without setgid, pgbackrest
+    # (running as the pgbackrest user) cannot read conf files created by adminapi.
+    - {dir: /etc/pgbackrest/conf.d, mode: '02770'}
+    - {dir: /var/lib/pgbackrest}
+    - {dir: /var/spool/pgbackrest}
+    - {dir: /var/log/pgbackrest}
   loop_control:
     loop_var: backrest_dir
   when:
-    - nixpkg_mode
+    - nixpkg_mode or stage2_nix
+
+# supabase-admin-agent opens these log files with O_APPEND|O_WRONLY (no
+# O_CREATE). They must exist before SAA runs; a missing file causes the
+# pgBackRest enable command to fail before any backup work is attempted.
+# access_time/modification_time: preserve keeps this task idempotent.
+- name: Pre-create pgBackRest SAA log files
+  ansible.builtin.file:
+    access_time: preserve
+    group: postgres
+    mode: '0660'
+    modification_time: preserve
+    owner: pgbackrest
+    path: "{{ item }}"
+    state: touch
+  loop:
+    - /var/log/pgbackrest/saa-pgb.log
+    - /var/log/pgbackrest/wal-push.log
+    - /var/log/pgbackrest/wal-fetch.log
+  when:
+    - nixpkg_mode or stage2_nix
 
 - name: Symlink pgbackrest.conf
   ansible.legacy.file: