Switch TAP tests of pg_rewind to use role with only function permissions

classic Classic list List threaded Threaded
3 messages Options
Reply | Threaded
Open this post in threaded view
|

Switch TAP tests of pg_rewind to use role with only function permissions

Michael Paquier-2
Hi all,

Recent commit bfc80683 has added some documentation in pg_rewind about
the fact that it is possible to do the operation with a non-superuser,
assuming that this role has sufficient grant rights to execute the
functions used by pg_rewind.

Peter Eisentraut has suggested to have some tests for this kind of
user here:
https://www.postgresql.org/message-id/e1570ba6-4459-d9b2-1321-9449adaaef4c@...

Attached is a patch which switches all the TAP tests of pg_rewind to
do that.  As of now, the tests depend on a superuser for everything,
and it seems to me that it makes little sense to make the tests more
pluggable by being able to switch the roles used on-the-fly (the
invocation of pg_rewind is stuck into RewindTest.pm) as a superuser
has no restrictions.

Any thoughts?
--
Michael

rewind-user-tap.patch (1K) Download Attachment
signature.asc (849 bytes) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: Switch TAP tests of pg_rewind to use role with only function permissions

Magnus Hagander-2


On Thu, Apr 11, 2019 at 6:13 AM Michael Paquier <[hidden email]> wrote:
Hi all,

Recent commit bfc80683 has added some documentation in pg_rewind about
the fact that it is possible to do the operation with a non-superuser,
assuming that this role has sufficient grant rights to execute the
functions used by pg_rewind.

Peter Eisentraut has suggested to have some tests for this kind of
user here:
https://www.postgresql.org/message-id/e1570ba6-4459-d9b2-1321-9449adaaef4c@...

Attached is a patch which switches all the TAP tests of pg_rewind to
do that.  As of now, the tests depend on a superuser for everything,
and it seems to me that it makes little sense to make the tests more
pluggable by being able to switch the roles used on-the-fly (the
invocation of pg_rewind is stuck into RewindTest.pm) as a superuser
has no restrictions.

Any thoughts?

+1.

I definitely think having tests for this is good, otherwise we'll just end up making a change at some point that then suddenly breaks it and we won't notice.

If we haven't already (and knowing you it wouldn't surprise me if you had :P), we should probably look through the rest of the tests to see if we have other similar cases. In general I think any case where "can be run by non-superuser with specific permissions or a superuser" is the case, we should be testing it with the "non-superuser with permissions". Because, well, superusers will never have permission problems (and they will both test the functionality).

I do think it's perfectly reasonable to have that hardcoded in the RewindTest.pm module. It doesn't have to be pluggable. 

--
Reply | Threaded
Open this post in threaded view
|

Re: Switch TAP tests of pg_rewind to use role with only function permissions

Michael Paquier-2
On Thu, Apr 11, 2019 at 09:40:36AM +0200, Magnus Hagander wrote:
> If we haven't already (and knowing you it wouldn't surprise me if you had
> :P), we should probably look through the rest of the tests to see if we
> have other similar cases. In general I think any case where "can be run by
> non-superuser with specific permissions or a superuser" is the case, we
> should be testing it with the "non-superuser with permissions". Because,
> well, superusers will never have permission problems (and they will both
> test the functionality).

I am ready to bet that we have other problems lying around.

> I do think it's perfectly reasonable to have that hardcoded in the
> RewindTest.pm module. It doesn't have to be pluggable.

Thanks, I have committed the patch to do so (d4e2a84), after rewording
a bit the comments.  And particularly thanks to Peter to mention that
having more tests with such properties would be nicer.
--
Michael

signature.asc (849 bytes) Download Attachment