search

EnterpriseDB / barman

Barman - Backup and Recovery Manager for PostgreSQL
https://www.pgbarman.org/
GNU General Public License v3.0
2.14k stars 193 forks source link

allows to run 'barman switch-wal --force' when the user has the 'pg_checkpoint' role #845

Closed toydarian closed 1 year ago

toydarian commented 1 year ago

Since CHECKPOINT; can be run by regular users that have the pg_checkpoint-role, we can use this to run switch-wal --force without superuser-privileges.

I implemented a method that will check if the current user has that role (or is a superuser) for PostgreSQL 14 and above and just checks for superuser-privileges for older versions of PostgreSQL.

Related issue: #844

mikewallace1979 commented 1 year ago

Thanks for filing #844 and making this PR.

I tested locally and this appears to work as advertised - here using PostgreSQL 15 and a non-superuser:

$ barman switch-wal main --force
ERROR: Barman switch-wal --force requires superuser rights or the 'pg_checkpoint' role

After granting the pg_checkpoint role:

$ barman switch-wal main --force
The WAL file 000000010000000100000098 has been closed on server 'main'

The patch itself looks solid however the unit tests will need updating before we can merge it - the test case tests.test_postgres.TestPostgres.test_checkpoint is currently failing because it is testing for the old PostgresSuperuserRequired exception instead of the new PostgresCheckpointPrivilegesRequired exception, and the test case will also need to set server_version on the server.postgres mock so that the code under test gets an int instead of a mock.

Ideally that test case could be expanded so that it covers both the PG < 14 and the PG >= 14 behaviour.

Are you happy to update this test case to cover the patch?

Thanks again for the contribution - it'll be great to get this merged.

toydarian commented 1 year ago

Sorry for breaking unit-tests. I ran pytest on tests, but it seems it didn't pick up all tests, my bad.
I fixed the failing test and I wrote an additional one for the has_checkpoint_privileges property.

I also ran tox this time and it seems there are additional tests failing (in test_cli.py and test_fs.py), but I don't think those are related to my changes.

mikewallace1979 commented 1 year ago

Sorry for breaking unit-tests. I ran pytest on tests, but it seems it didn't pick up all tests, my bad. I fixed the failing test and I wrote an additional one for the has_checkpoint_privileges property.

No worries and thank you for following up with them so quickly.