Hello,
I wanted to resurface this thread.
The original intention I had with this patch I sent over a year ago
was to have the possibility for monitoring ROLEs like pg_monitor and
pg_read_all_stats to have read access for the replication origin
status. Seems the patch only got half way through
On Thu, Jul 02, 2020 at 03:03:22PM +0200, Daniel Gustafsson wrote:
> AFAICT from the thread there is nothing left of this changeset to consider, so
> I've marked the entry as committed in the CF app. Feel free to update in case
> I've missed something.
A second item discussed in this thread was
> On 14 Jun 2020, at 05:46, Michael Paquier wrote:
>
> On Wed, Jun 10, 2020 at 12:35:49PM +0900, Michael Paquier wrote:
>> On Tue, Jun 09, 2020 at 05:07:39PM +0900, Masahiko Sawada wrote:
>>> I agreed with the change you proposed.
>>
>> OK, thanks. Then let's wait a couple of days to see if
On Mon, Jun 15, 2020 at 12:44:02AM -0400, Tom Lane wrote:
> FWIW, I'd have included a catversion bump in this, to enforce that
> the modified backend functions are used with matching pg_proc entries.
> It's not terribly important at this phase of the devel cycle, but still
> somebody might wonder
Michael Paquier writes:
> On Wed, Jun 10, 2020 at 12:35:49PM +0900, Michael Paquier wrote:
>> OK, thanks. Then let's wait a couple of days to see if anybody has
>> any objections with the removal of the hardcoded superuser check
>> for those functions.
> Committed the part removing the
On Wed, Jun 10, 2020 at 12:35:49PM +0900, Michael Paquier wrote:
> On Tue, Jun 09, 2020 at 05:07:39PM +0900, Masahiko Sawada wrote:
>> I agreed with the change you proposed.
>
> OK, thanks. Then let's wait a couple of days to see if anybody has
> any objections with the removal of the hardcoded
On Tue, Jun 09, 2020 at 05:07:39PM +0900, Masahiko Sawada wrote:
> Oh, I see. There might be room for improvement but it's a separate issue.
>
> I agreed with the change you proposed.
OK, thanks. Then let's wait a couple of days to see if anybody has
any objections with the removal of the
At Tue, 9 Jun 2020 15:11:04 +0900, Michael Paquier wrote
in
> On Mon, Jun 08, 2020 at 05:44:56PM +0900, Kyotaro Horiguchi wrote:
> > Mmm. Right.
>
> Yep. I bumped on that myself. I am not sure about 0002 and 0004 yet,
> and IMO they are not mandatory pieces, but from what I can see in the
>
At Tue, 9 Jun 2020 16:35:55 +0900, Michael Paquier wrote
in
> On Tue, Jun 09, 2020 at 03:32:24PM +0900, Masahiko Sawada wrote:
> > One thing I'm concerned with this change is that we will end up
> > needing to grant both execute on pg_show_replication_origin_status()
> > and select on
On Tue, 9 Jun 2020 at 16:36, Michael Paquier wrote:
>
> On Tue, Jun 09, 2020 at 03:32:24PM +0900, Masahiko Sawada wrote:
> > One thing I'm concerned with this change is that we will end up
> > needing to grant both execute on pg_show_replication_origin_status()
> > and select on
On Tue, Jun 09, 2020 at 03:32:24PM +0900, Masahiko Sawada wrote:
> One thing I'm concerned with this change is that we will end up
> needing to grant both execute on pg_show_replication_origin_status()
> and select on pg_replication_origin_status view when we want a
> non-super user to access
On Tue, 9 Jun 2020 at 15:11, Michael Paquier wrote:
>
> On Mon, Jun 08, 2020 at 05:44:56PM +0900, Kyotaro Horiguchi wrote:
> > Mmm. Right.
>
> Yep. I bumped on that myself. I am not sure about 0002 and 0004 yet,
> and IMO they are not mandatory pieces, but from what I can see in the
> set 0001
On Mon, Jun 08, 2020 at 05:44:56PM +0900, Kyotaro Horiguchi wrote:
> Mmm. Right.
Yep. I bumped on that myself. I am not sure about 0002 and 0004 yet,
and IMO they are not mandatory pieces, but from what I can see in the
set 0001 and 0003 can just be squashed together to remove those
superuser
At Mon, 8 Jun 2020 16:21:45 +0900, Masahiko Sawada
wrote in
> I've looked at these patches and have one question:
>
> REVOKE ALL ON pg_replication_origin_status FROM public;
>
> +GRANT SELECT ON pg_replication_origin_status TO pg_read_all_stats;
>
> +REVOKE EXECUTE ON FUNCTION
Hello, Martín.
Thanks for the new version.
At Thu, 4 Jun 2020 09:17:18 -0300, Martín Marqués
wrote in
> > I'm not sure where to put the GRANT on
> > pg_show_replication_origin_status(), but maybe it also should be at
> > the same place.
>
> Yes, I agree that it makes the revoking/granting
On Thu, 4 Jun 2020 at 21:17, Martín Marqués wrote:
>
> Hi Kyotaro-san,
>
> > Sorry for not mentioning it at that time, but about the following diff:
> >
> > +GRANT SELECT ON pg_replication_origin_status TO pg_read_all_stats;
> >
> > system_views.sql already has a REVOKE command on the view. We
Hi Kyotaro-san,
> Sorry for not mentioning it at that time, but about the following diff:
>
> +GRANT SELECT ON pg_replication_origin_status TO pg_read_all_stats;
>
> system_views.sql already has a REVOKE command on the view. We should
> put the above just below the REVOKE command.
>
> I'm not
Hi, Martin.
At Wed, 3 Jun 2020 13:32:28 -0300, Martín Marqués
wrote in
> Hi Kyotaro-san,
>
> Thank you for taking the time to review my patches. Would you like to
> set yourself as a reviewer in the commit entry here?
> https://commitfest.postgresql.org/28/2577/
Done.
> > 0002:
> >
> > It
Hi Kyotaro-san,
Thank you for taking the time to review my patches. Would you like to
set yourself as a reviewer in the commit entry here?
https://commitfest.postgresql.org/28/2577/
> 0002:
>
> It is forgetting to grant pg_read_all_stats to execute
> pg_show_replication_origin_status. As the
At Tue, 2 Jun 2020 13:13:18 -0300, Martín Marqués
wrote in
> > > I have no problem adding it to this ROLE, but we'd have to amend the
> > > doc for default-roles to reflect that SELECT for this view is also
> > > granted to `pg_read_all_stats`.
> >
> > I agree in general that pg_monitor
Hi Stephen,
> > I have no problem adding it to this ROLE, but we'd have to amend the
> > doc for default-roles to reflect that SELECT for this view is also
> > granted to `pg_read_all_stats`.
>
> I agree in general that pg_monitor shouldn't have privileges granted
> directly to it. If this needs
Greetings,
* Martín Marqués (mar...@2ndquadrant.com) wrote:
> > > $ git diff
> > > diff --git a/src/backend/catalog/system_views.sql
> > > b/src/backend/catalog/system_views.sql
> > > index c16061f8f00..97ee72a9cfc 100644
> > > --- a/src/backend/catalog/system_views.sql
> > > +++
Hi,
> > $ git diff
> > diff --git a/src/backend/catalog/system_views.sql
> > b/src/backend/catalog/system_views.sql
> > index c16061f8f00..97ee72a9cfc 100644
> > --- a/src/backend/catalog/system_views.sql
> > +++ b/src/backend/catalog/system_views.sql
> > @@ -1494,9 +1494,6 @@ GRANT EXECUTE ON
Hi.
At Mon, 1 Jun 2020 21:41:13 -0300, Martín Marqués
wrote in
> Hi,
>
> > Took me a bit longer than expected, but here is a new version, now
> > with the idea of just removing the superuser() check and REVOKEing
> > execution of the functions from public. At the end I grant permission
> > to
On Mon, Jun 01, 2020 at 03:38:07PM -0300, Martín Marqués wrote:
> Took me a bit longer than expected, but here is a new version, now
> with the idea of just removing the superuser() check and REVOKEing
> execution of the functions from public. At the end I grant permission
> to functions and the
Hi,
> Took me a bit longer than expected, but here is a new version, now
> with the idea of just removing the superuser() check and REVOKEing
> execution of the functions from public. At the end I grant permission
> to functions and the pg_replication_origin_status view.
>
> I wonder now if I
Hi,
Took me a bit longer than expected, but here is a new version, now
with the idea of just removing the superuser() check and REVOKEing
execution of the functions from public. At the end I grant permission
to functions and the pg_replication_origin_status view.
I wonder now if I needed to
Hi Michael,
> Wouldn't it be just better to remove this hardcoded superuser check
> and replace it with equivalent ACLs by default? The trick is to make
> sure that any function calling replorigin_check_prerequisites() has
> its execution correctly revoked from public. See for example
>
On Fri, May 29, 2020 at 05:39:31PM -0300, Martín Marqués wrote:
> I believe we could skip the superuser() check for cases like
> pg_replication_origin_session_progress() and
> pg_replication_origin_progress().
>
> Once option could be to add a third bool argument check_superuser to
>
Hi,
> While working on some monitoring tasks I realised that the pg_monitor
> role doesn't have access to the pg_replication_origin_status.
>
> Are there any strong thoughts on not giving pg_monitor access to this
> view, or is it just something that nobody asked for yet? I can't find
> any
Hi all,
While working on some monitoring tasks I realised that the pg_monitor
role doesn't have access to the pg_replication_origin_status.
Are there any strong thoughts on not giving pg_monitor access to this
view, or is it just something that nobody asked for yet? I can't find
any reason for
31 matches
Mail list logo
