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 reason for pg_monitor not to have access to it.
Further looking into this, I can see that the requirement of a superuser to access/monify the replication origins is hardcoded in backend/replication/logical/origin.c, so it's not a question of GRANTing access to the pg_monitor user. ``` static void replorigin_check_prerequisites(bool check_slots, bool recoveryOK) { if (!superuser()) ereport(ERROR, (errcode(ERRCODE_INSUFFICIENT_PRIVILEGE), errmsg("only superusers can query or manipulate replication origins"))); if (check_slots && max_replication_slots == 0) ereport(ERROR, (errcode(ERRCODE_OBJECT_NOT_IN_PREREQUISITE_STATE), errmsg("cannot query or manipulate replication origin when max_replication_slots = 0"))); if (!recoveryOK && RecoveryInProgress()) ereport(ERROR, (errcode(ERRCODE_READ_ONLY_SQL_TRANSACTION), errmsg("cannot manipulate replication origins during recovery"))); } ``` 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 replorigin_check_prerequisites() and have it set to false for the functions which a none superuser could execute. Patch attached -- Martín Marqués http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training & Services
From ade8b9a70afed900dddc4e93616499bb80966592 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mart=C3=ADn=20Marqu=C3=A9s?= <martin.marq...@2ndquadrant.com> Date: Fri, 29 May 2020 15:45:27 -0300 Subject: [PATCH v1] First version of patch to give permission to pg_replication_origin_status() MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Martín Marqués <martin.marq...@2ndquadrant.com> --- src/backend/replication/logical/origin.c | 28 ++++++++++++------------ 1 file changed, 14 insertions(+), 14 deletions(-) diff --git a/src/backend/replication/logical/origin.c b/src/backend/replication/logical/origin.c index 981d60f135d..82698617236 100644 --- a/src/backend/replication/logical/origin.c +++ b/src/backend/replication/logical/origin.c @@ -180,9 +180,9 @@ static ReplicationState *session_replication_state = NULL; #define REPLICATION_STATE_MAGIC ((uint32) 0x1257DADE) static void -replorigin_check_prerequisites(bool check_slots, bool recoveryOK) +replorigin_check_prerequisites(bool check_slots, bool recoveryOK, bool check_superuser) { - if (!superuser()) + if (check_superuser && !superuser()) ereport(ERROR, (errcode(ERRCODE_INSUFFICIENT_PRIVILEGE), errmsg("only superusers can query or manipulate replication origins"))); @@ -1225,7 +1225,7 @@ pg_replication_origin_create(PG_FUNCTION_ARGS) char *name; RepOriginId roident; - replorigin_check_prerequisites(false, false); + replorigin_check_prerequisites(false, false, true); name = text_to_cstring((text *) DatumGetPointer(PG_GETARG_DATUM(0))); @@ -1262,7 +1262,7 @@ pg_replication_origin_drop(PG_FUNCTION_ARGS) char *name; RepOriginId roident; - replorigin_check_prerequisites(false, false); + replorigin_check_prerequisites(false, false, true); name = text_to_cstring((text *) DatumGetPointer(PG_GETARG_DATUM(0))); @@ -1285,7 +1285,7 @@ pg_replication_origin_oid(PG_FUNCTION_ARGS) char *name; RepOriginId roident; - replorigin_check_prerequisites(false, false); + replorigin_check_prerequisites(false, false, true); name = text_to_cstring((text *) DatumGetPointer(PG_GETARG_DATUM(0))); roident = replorigin_by_name(name, true); @@ -1306,7 +1306,7 @@ pg_replication_origin_session_setup(PG_FUNCTION_ARGS) char *name; RepOriginId origin; - replorigin_check_prerequisites(true, false); + replorigin_check_prerequisites(true, false, true); name = text_to_cstring((text *) DatumGetPointer(PG_GETARG_DATUM(0))); origin = replorigin_by_name(name, false); @@ -1325,7 +1325,7 @@ pg_replication_origin_session_setup(PG_FUNCTION_ARGS) Datum pg_replication_origin_session_reset(PG_FUNCTION_ARGS) { - replorigin_check_prerequisites(true, false); + replorigin_check_prerequisites(true, false, true); replorigin_session_reset(); @@ -1342,7 +1342,7 @@ pg_replication_origin_session_reset(PG_FUNCTION_ARGS) Datum pg_replication_origin_session_is_setup(PG_FUNCTION_ARGS) { - replorigin_check_prerequisites(false, false); + replorigin_check_prerequisites(false, false, true); PG_RETURN_BOOL(replorigin_session_origin != InvalidRepOriginId); } @@ -1361,7 +1361,7 @@ pg_replication_origin_session_progress(PG_FUNCTION_ARGS) XLogRecPtr remote_lsn = InvalidXLogRecPtr; bool flush = PG_GETARG_BOOL(0); - replorigin_check_prerequisites(true, false); + replorigin_check_prerequisites(true, false, false); if (session_replication_state == NULL) ereport(ERROR, @@ -1381,7 +1381,7 @@ pg_replication_origin_xact_setup(PG_FUNCTION_ARGS) { XLogRecPtr location = PG_GETARG_LSN(0); - replorigin_check_prerequisites(true, false); + replorigin_check_prerequisites(true, false, true); if (session_replication_state == NULL) ereport(ERROR, @@ -1397,7 +1397,7 @@ pg_replication_origin_xact_setup(PG_FUNCTION_ARGS) Datum pg_replication_origin_xact_reset(PG_FUNCTION_ARGS) { - replorigin_check_prerequisites(true, false); + replorigin_check_prerequisites(true, false, true); replorigin_session_origin_lsn = InvalidXLogRecPtr; replorigin_session_origin_timestamp = 0; @@ -1413,7 +1413,7 @@ pg_replication_origin_advance(PG_FUNCTION_ARGS) XLogRecPtr remote_commit = PG_GETARG_LSN(1); RepOriginId node; - replorigin_check_prerequisites(true, false); + replorigin_check_prerequisites(true, false, true); /* lock to prevent the replication origin from vanishing */ LockRelationOid(ReplicationOriginRelationId, RowExclusiveLock); @@ -1449,7 +1449,7 @@ pg_replication_origin_progress(PG_FUNCTION_ARGS) RepOriginId roident; XLogRecPtr remote_lsn = InvalidXLogRecPtr; - replorigin_check_prerequisites(true, true); + replorigin_check_prerequisites(true, true, false); name = text_to_cstring((text *) DatumGetPointer(PG_GETARG_DATUM(0))); flush = PG_GETARG_BOOL(1); @@ -1478,7 +1478,7 @@ pg_show_replication_origin_status(PG_FUNCTION_ARGS) #define REPLICATION_ORIGIN_PROGRESS_COLS 4 /* we want to return 0 rows if slot is set to zero */ - replorigin_check_prerequisites(false, true); + replorigin_check_prerequisites(false, true, false); if (rsinfo == NULL || !IsA(rsinfo, ReturnSetInfo)) ereport(ERROR, base-commit: 5b1c61e8b8f98f4a1c42856819b6dea600669f47 -- 2.21.3