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?=
<[email protected]>
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 <[email protected]>
---
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
