diff --git a/.github/copilot-instructions.md b/.github/copilot-instructions.md index a9157f45..d22e02a2 100644 --- a/.github/copilot-instructions.md +++ b/.github/copilot-instructions.md @@ -90,7 +90,6 @@ Always test backup and restore functionality after making changes using function ## Troubleshooting Known Issues - **Network Timeouts**: Package installations may timeout due to network connectivity. Retry commands if needed. -- **SQLite Restore Warnings**: Warnings about "UNIQUE constraint failed" during restore operations are normal for test scenarios - **Memory Database Issues**: If you see "no such table" errors, ensure you run migrations first in the appropriate environment - **Linting Temporarily Disabled**: CI linting checks are temporarily set to pass (marked with `|| true`) pending resolution in future PR - **Environment Isolation**: Each hatch environment is isolated - dependencies are automatically managed per environment @@ -102,7 +101,7 @@ Modern development process using Hatch: 1. **Bootstrap environment**: `pip install --upgrade pip hatch uv` 2. **Make your changes** to the codebase 3. **Run unit tests**: `hatch test` (≈30s) **All must pass - failures are never expected or allowed.** -4. **Run functional tests**: `hatch run functional:all` (≈10–15s) +4. **Run functional tests**: `hatch run functional:all -v` (≈10–15s) 5. **Run linting**: `hatch run lint:check` (5 seconds) 6. **Auto-format code**: `hatch run lint:format` (2 seconds) 7. **Test documentation**: `hatch run docs:build` (2 seconds) diff --git a/CHANGELOG.md b/CHANGELOG.md index f02766e4..e693252a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -50,6 +50,7 @@ Don't forget to remove deprecated code on each major release! - Fix SQLite `index`/`trigger`/`view` ` already exists` errors. - Fixed `pg_dump` error when backing up PostgreSQL databases with row-level security policies enabled. - Fix PostgreSQL restore errors with identity columns by automatically enabling `--if-exists` when using `--clean` in `PgDumpBinaryConnector`. +- Fix PostgreSQL permission errors when restoring with non-superuser accounts by automatically adding `--no-owner` flag to `pg_restore` in `PgDumpBinaryConnector`. ### Security diff --git a/dbbackup/db/postgresql.py b/dbbackup/db/postgresql.py index 993652a6..05ee8cb1 100644 --- a/dbbackup/db/postgresql.py +++ b/dbbackup/db/postgresql.py @@ -117,6 +117,7 @@ class PgDumpBinaryConnector(PgDumpConnector): single_transaction = True drop = True if_exists = False + no_owner = True enable_row_security = False pg_options = None @@ -177,6 +178,9 @@ def _restore_dump(self, dump: str): if self.if_exists or self.drop: cmd.extend(["--if-exists"]) + if self.no_owner: + cmd.extend(["--no-owner"]) + if self.restore_suffix: cmd.extend(self.restore_suffix if isinstance(self.restore_suffix, list) else [self.restore_suffix]) diff --git a/docs/src/databases.md b/docs/src/databases.md index 2ed25476..047c4a27 100644 --- a/docs/src/databases.md +++ b/docs/src/databases.md @@ -109,12 +109,13 @@ All PostgreSQL connectors have the following settings: #### Settings -| Setting | Description | Default | -| ------------------ | --------------------------------------------------------------------------------------------------------------------------------------- | ------- | -| SINGLE_TRANSACTION | Wrap restore in a single transaction so errors cause full rollback (`--single-transaction` for `psql` / `pg_restore`). | `True` | -| DROP | Include / execute drop statements when restoring (`--clean` with `pg_dump` / `pg_restore`). In binary mode drops happen during restore. | `True` | -| IF_EXISTS | Add `IF EXISTS` to destructive statements in clean mode. Automatically enabled when `DROP=True` to prevent identity column errors. | `False` | -| ENABLE_ROW_SECURITY | Enable row-level security for dumping data (`--enable-row-security` with `pg_dump`). Required for databases with row-level security policies. | `False` | +| Setting | Description | Default | +| ------------------- | --------------------------------------------------------------------------------------------------------------------------------------------------------------- | ------- | +| SINGLE_TRANSACTION | Wrap restore in a single transaction so errors cause full rollback (`--single-transaction` for `psql` / `pg_restore`). | `True` | +| DROP | Include / execute drop statements when restoring (`--clean` with `pg_dump` / `pg_restore`). In binary mode drops happen during restore. | `True` | +| IF_EXISTS | Add `IF EXISTS` to destructive statements in clean mode. Automatically enabled when `DROP=True` to prevent identity column errors. | `False` | +| NO_OWNER | Skip restoration of object ownership (`--no-owner` with `pg_restore`). Prevents permission errors when restoring with non-superuser accounts. Binary mode only. | `True` | +| ENABLE_ROW_SECURITY | Enable row-level security for dumping data (`--enable-row-security` with `pg_dump`). Required for databases with row-level security policies. | `False` | Example configuration for databases with row-level security: @@ -126,17 +127,19 @@ DBBACKUP_CONNECTORS = { } ``` -#### PgDumpConnector +#### PgDumpBinaryConnector -The `dbbackup.db.postgresql.PgDumpConnector` uses `pg_dump` to create RAW SQL files and `psql` to restore them. +The `dbbackup.db.postgresql.PgDumpBinaryConnector` is similar to PgDumpConnector, but it uses `pg_dump` in binary mode and restores using `pg_restore`. -This is the default connector for PostgreSQL databases, however, it is recommended to use the binary connector for better performance. +This is the default connector for PostgreSQL databases, and it allows for faster and parallel-capable restores. This connector may invoke `psql` for administrative tasks. -#### PgDumpBinaryConnector +#### PgDumpConnector -The `dbbackup.db.postgresql.PgDumpBinaryConnector` is similar to PgDumpConnector, but it uses `pg_dump` in binary mode and restores using `pg_restore`. +The `dbbackup.db.postgresql.PgDumpConnector` uses `pg_dump` to create RAW SQL files and `psql` to restore them. + +It is recommended to use the binary connector for better performance. -This allows for faster and parallel-capable restores. It may still invoke `psql` for administrative tasks. +**Permission Handling**: By default, this connector uses `NO_OWNER=True` to prevent permission errors when restoring with non-superuser database accounts. This adds the `--no-owner` flag to `pg_restore`, allowing any database user to restore backups without needing ownership of system extensions or objects. ### PostGIS diff --git a/tests/test_connectors/test_postgresql.py b/tests/test_connectors/test_postgresql.py index d03fb930..c56c7d18 100644 --- a/tests/test_connectors/test_postgresql.py +++ b/tests/test_connectors/test_postgresql.py @@ -286,6 +286,19 @@ def test_restore_suffix(self, mock_run_command): cmd_args = mock_run_command.call_args[0][0] self.assertTrue(cmd_args.endswith(" foo")) + def test_no_owner_default_behavior(self, mock_run_command): + """Test that --no-owner is added by default to prevent permission issues.""" + dump = self.connector.create_dump() + self.connector.restore_dump(dump) + cmd_args = mock_run_command.call_args[0][0] + self.assertIn(" --no-owner", cmd_args) + + # Test that no_owner can be disabled + self.connector.no_owner = False + self.connector.restore_dump(dump) + cmd_args = mock_run_command.call_args[0][0] + self.assertNotIn(" --no-owner", cmd_args) + @patch( "dbbackup.db.postgresql.PgDumpBinaryConnector.run_command", return_value=(BytesIO(), BytesIO()),