fix(postgresql): preserve pgdump restore owners and privileges

addons/postgresql/dataprotection/pgdump-restore.sh

@@ -15,11 +15,16 @@ if [ -z "$jobs" ]; then
   jobs=4
 fi
 
-# Build pg_dump parameters
-# Roles are not dumped by pg_dump; the target cluster (e.g. a freshly
-# restored one) usually does not have the roles referenced by ownership and
-# ACL statements, so skip restoring owners and privileges altogether.
-params="-j $jobs -Fd -v -C -d postgres --no-owner --no-privileges"
+# Build pg_restore parameters. Preserve archive ownership and privileges by
+# default; users restoring into a cluster without the referenced roles can
+# explicitly opt out of either behavior.
+params="-j $jobs -Fd -v -C -d postgres"
+if [ "$skip_owner" == "true" ]; then
+  params="$params --no-owner"
+fi
+if [ "$skip_privileges" == "true" ]; then
+  params="$params --no-privileges"
+fi
 if [ -n "$database" ]; then
     $psql_cmd -d postgres -Atc "create database $database" || echo "Failed to create database $database"
 fi

addons/postgresql/scripts-ut-spec/pgdump_restore_spec.sh

@@ -0,0 +1,165 @@
+# shellcheck shell=sh
+
+Describe "dataprotection/pgdump-restore.sh owner and privilege contract"
+
+  script_path() {
+    printf "%s" "../dataprotection/pgdump-restore.sh"
+  }
+
+  actionset_path() {
+    printf "%s" "../templates/actionset-pgdump.yaml"
+  }
+
+  setup() {
+    tmpdir=$(mktemp -d -t pg-dump-restore-XXXXXX)
+    bindir="${tmpdir}/bin"
+    mkdir -p "${bindir}"
+    PATH="${bindir}:${PATH}"
+    CALL_LOG="${tmpdir}/calls.log"
+    : > "${CALL_LOG}"
+    DP_DATASAFED_BIN_PATH="${bindir}"
+    DP_BACKUP_BASE_PATH="/backup"
+    DP_BACKUP_NAME="backup-test"
+    POSTGRES_PASSWORD="secret"
+    POSTGRES_USER="postgres"
+    DP_DB_HOST="localhost"
+    DP_DB_PORT="5432"
+    BACKUP_DIR="${tmpdir}/restore-workdir"
+    export PATH CALL_LOG DP_DATASAFED_BIN_PATH DP_BACKUP_BASE_PATH \
+      DP_BACKUP_NAME POSTGRES_PASSWORD POSTGRES_USER DP_DB_HOST DP_DB_PORT BACKUP_DIR
+    unset PG_RESTORE_EXIT PG_RESTORE_STDERR jobs database schemas tables \
+      schema_only conflict_policy skip_owner skip_privileges 2>/dev/null || true
+    write_stubs
+  }
+
+  cleanup() {
+    rm -rf "${tmpdir}"
+    rm -f /tmp/pg_restore.log
+  }
+
+  BeforeEach 'setup'
+  AfterEach 'cleanup'
+
+  write_stubs() {
+    cat > "${bindir}/datasafed" <<'EOF'
+#!/bin/sh
+printf 'datasafed %s\n' "$*" >> "${CALL_LOG}"
+printf '%s\n' '-- dump data'
+EOF
+    cat > "${bindir}/tar" <<'EOF'
+#!/bin/sh
+printf 'tar %s\n' "$*" >> "${CALL_LOG}"
+cat > /dev/null
+EOF
+    cat > "${bindir}/psql" <<'EOF'
+#!/bin/sh
+printf 'psql %s\n' "$*" >> "${CALL_LOG}"
+exit 0
+EOF
+    cat > "${bindir}/pg_restore" <<'EOF'
+#!/bin/sh
+printf 'pg_restore %s\n' "$*" >> "${CALL_LOG}"
+if [ -n "${PG_RESTORE_STDERR:-}" ]; then
+  printf '%s\n' "${PG_RESTORE_STDERR}" >&2
+fi
+exit "${PG_RESTORE_EXIT:-0}"
+EOF
+    chmod +x "${bindir}/datasafed" "${bindir}/tar" "${bindir}/psql" "${bindir}/pg_restore"
+  }
+
+  call_log() {
+    cat "${CALL_LOG}"
+  }
+
+  restore_parameters() {
+    grep '^pg_restore ' "${CALL_LOG}"
+  }
+
+  actionset_schema() {
+    cat "$(actionset_path)"
+  }
+
+  restore_parameter_names() {
+    awk '/^  restore:/{restore=1} restore && /^    withParameters:/{params=1; next} params && /^    [a-zA-Z]/{exit} params{print}' "$(actionset_path)"
+  }
+
+  restore_parameter_contract() {
+    awk -v parameter="$1" '
+      $0 == "        " parameter ":" { found=1; next }
+      found && /^        [a-zA-Z0-9_]+:$/ { exit }
+      found && /^          type:/ {
+        sub(/^          type:[[:space:]]*/, "type=")
+        print
+      }
+      found && /^          default:/ {
+        sub(/^          default:[[:space:]]*/, "default=")
+        print
+      }
+    ' "$(actionset_path)"
+  }
+
+  skip_owner_contract() {
+    restore_parameter_contract skip_owner
+  }
+
+  skip_privileges_contract() {
+    restore_parameter_contract skip_privileges
+  }
+
+  It "restores archive owners and privileges by default"
+    When run bash "$(script_path)"
+    The status should eq 0
+    The result of function restore_parameters should not include "--no-owner"
+    The result of function restore_parameters should not include "--no-privileges"
+  End
+
+  It "skips only owners when skip_owner is true"
+    export skip_owner="true"
+    When run bash "$(script_path)"
+    The status should eq 0
+    The result of function restore_parameters should include "--no-owner"
+    The result of function restore_parameters should not include "--no-privileges"
+  End
+
+  It "skips only privileges when skip_privileges is true"
+    export skip_privileges="true"
+    When run bash "$(script_path)"
+    The status should eq 0
+    The result of function restore_parameters should not include "--no-owner"
+    The result of function restore_parameters should include "--no-privileges"
+  End
+
+  It "supports explicitly skipping owners and privileges together"
+    export skip_owner="true"
+    export skip_privileges="true"
+    When run bash "$(script_path)"
+    The status should eq 0
+    The result of function restore_parameters should include "--no-owner"
+    The result of function restore_parameters should include "--no-privileges"
+  End
+
+  It "does not downgrade a missing archive role to success"
+    export PG_RESTORE_EXIT=1
+    export PG_RESTORE_STDERR='pg_restore: error: could not execute query: ERROR: role "app" does not exist
+pg_restore: warning: errors ignored on restore: 1'
+    When run bash "$(script_path)"
+    The status should be failure
+    The output should include "parameters:"
+    The error should include 'role "app" does not exist'
+    The error should include "pg_restore reported non-conflict errors; failing restore"
+  End
+
+  It "declares and passes the two opt-out parameters"
+    When call actionset_schema
+    The output should include "skip_owner:"
+    The output should include "skip_privileges:"
+    The output should include 'description: "Skip restoring object owners.'
+    The output should include 'description: "Skip restoring object privileges.'
+    The result of function restore_parameter_names should include "- skip_owner"
+    The result of function restore_parameter_names should include "- skip_privileges"
+    The result of function skip_owner_contract should include "type=boolean"
+    The result of function skip_owner_contract should include "default=false"
+    The result of function skip_privileges_contract should include "type=boolean"
+    The result of function skip_privileges_contract should include "default=false"
+  End
+End

addons/postgresql/templates/actionset-pgdump.yaml

@@ -33,6 +33,14 @@ spec:
           type: boolean
           description: "Backup only the schema, no data."
           default: false
+        skip_owner:
+          type: boolean
+          description: "Skip restoring object owners. By default, archived owners are restored."
+          default: false
+        skip_privileges:
+          type: boolean
+          description: "Skip restoring object privileges. By default, archived GRANT and REVOKE statements are restored."
+          default: false
         conflict_policy:
           type: string
           description: "Conflict policy for restore. If not specified, the default is 'CONTINUE'."
@@ -69,6 +77,8 @@ spec:
       - schemas
       - tables
       - schema_only
+      - skip_owner
+      - skip_privileges
     postReady:
     - job:
         image: {{ .Values.image.registry | default "docker.io" }}/{{ .Values.image.repository }}:$(IMAGE_TAG)