Open
Conversation
Crispy1975
force-pushed
the
pjc/indata-403-admin-agent-pgbackrest
branch
from
8f44ad4 to
2370c92
Compare
hunleyd
requested changes
Mar 31, 2026
Crispy1975
force-pushed
the
pjc/indata-403-admin-agent-pgbackrest
branch
from
2370c92 to
d1b6678
Compare
tomashley
reviewed
Apr 7, 2026
tomashley
reviewed
Apr 7, 2026
tomashley
reviewed
Apr 7, 2026
tomashley
reviewed
Apr 7, 2026
- 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
- 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
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.
Co-authored-by: Tom Ashley <tom.ashley@gmail.com>
Co-authored-by: Tom Ashley <tom.ashley@gmail.com>
Co-authored-by: Tom Ashley <tom.ashley@gmail.com>
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.
pgdata-signal is called via sudo as postgres (other user), so it must be world-executable. 0700 would prevent postgres from running it.
Crispy1975
force-pushed
the
pjc/indata-403-admin-agent-pgbackrest
branch
from
b1a04b4 to
7425331
Compare
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.
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])
- 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
hunleyd
approved these changes
Apr 9, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What kind of change does this PR introduce?
Additional config for
pgBackRestbackups and restores. Details further in: https://linear.app/supabase/issue/INDATA-403/admin-agent-add-new-command-to-control-pgbackrest