From: | Anastasia Lubennikova <a(dot)lubennikova(at)postgrespro(dot)ru> |
---|---|
To: | Noah Misch <noah(at)leadboat(dot)com> |
Cc: | Arthur Zakirov <zaartur(at)gmail(dot)com>, Alvaro Herrera <alvherre(at)2ndquadrant(dot)com>, Michael Paquier <michael(at)paquier(dot)xyz>, Grigory Smolkin <g(dot)smolkin(at)postgrespro(dot)ru>, pgsql-hackers(at)lists(dot)postgresql(dot)org |
Subject: | Re: pg_upgrade fails with non-standard ACL |
Date: | 2021-01-25 19:14:43 |
Message-ID: | cf4f05c1-bdc2-8f75-61ef-8ce368e4edd1@postgrespro.ru |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
On 24.01.2021 11:39, Noah Misch wrote:
> On Thu, Jan 21, 2021 at 01:03:58AM +0300, Anastasia Lubennikova wrote:
>> On 03.01.2021 14:29, Noah Misch wrote:
>>> Overall, this patch predicts a subset of cases where pg_dump will emit a
>>> failing GRANT or REVOKE that targets a pg_catalog object. Can you write a
>>> code comment stating what it does and does not detect? I think it's okay to
>>> not predict every failure, but let's record the intended coverage. Here are a
>>> few examples I know; there may be more. The above query only finds GRANTs to
>>> non-pinned roles. pg_dump dumps the following, but the above query doesn't
>>> find them:
>>>
>>> REVOKE ALL ON FUNCTION pg_last_wal_replay_lsn FROM public;
>>> GRANT EXECUTE ON FUNCTION pg_reload_conf() TO pg_signal_backend;
> I see a new comment listing object types. Please also explain the lack of
> preventing REVOKE failures (first example query above) and the limitation
> around non-pinned roles (second example).
>
1) Could you please clarify, what do you mean by REVOKE failures?
I tried following example:
Start 9.6 cluster.
REVOKE ALL ON function pg_switch_xlog() FROM public;
REVOKE ALL ON function pg_switch_xlog() FROM backup;
The upgrade to the current master passes with and without patch.
It seems that current implementation of pg_upgrade doesn't take into
account revoke ACLs.
2) As for pinned roles, I think we should fix this behavior, rather than
adding a comment. Because such roles can have grants on system objects.
In earlier versions of the patch, I gathered ACLs directly from system
catalogs: pg_proc.proacl, pg_class.relacl pg_attribute.attacl and
pg_type.typacl.
The only downside of this approach is that it cannot be easily extended
to other object types, as we need to handle every object type separately.
I don't think it is a big deal, as we already do it in
check_for_changed_signatures()
And also the query to gather non-standard ACLs won't look as nice as
now, because of the need to parse arrays of aclitems.
What do you think?
--
Anastasia Lubennikova
Postgres Professional: http://www.postgrespro.com
The Russian Postgres Company
From | Date | Subject | |
---|---|---|---|
Next Message | Bruce Momjian | 2021-01-25 19:18:05 | Re: Key management with tests |
Previous Message | Heikki Linnakangas | 2021-01-25 19:11:51 | Re: cleaning up a few CLOG-related things |