From 22805c1bc1b06a6d15282cd24d2f55428868eb34 Mon Sep 17 00:00:00 2001 From: Alena Rybakina Date: Tue, 14 Jul 2026 13:43:15 +0300 Subject: [PATCH] Let mdb_admin manage resource groups MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Add gp_toolkit.pg_create_mdb_admin_role() (gp_toolkit 1.7) to provision the role with its fixed OID. Allow members of mdb_admin — not just superusers — to CREATE/ALTER/DROP resource groups and run pg_resgroup_move_query(), so a cloud admin can tune their own CPU/memory limits. Based on pg-sharding/cpg 7b8c912. Some tests are adapted from open-gpdb/gpdb 3ac99962ad2. Co-authored-by: reshke Co-authored-by: Andrey Borodin --- gpcontrib/gp_toolkit/Makefile | 8 +- .../expected/gp_toolkit_mdb_admin.out | 46 +++++++ gpcontrib/gp_toolkit/gp_toolkit--1.6--1.7.sql | 21 +++ gpcontrib/gp_toolkit/gp_toolkit.control | 2 +- gpcontrib/gp_toolkit/mdb_admin.c | 86 ++++++++++++ .../gp_toolkit/sql/gp_toolkit_mdb_admin.sql | 37 +++++ src/backend/catalog/oid_dispatch.c | 50 ++++++- src/backend/commands/resgroupcmds.c | 51 ++++--- src/backend/utils/resgroup/resgroup_helper.c | 5 +- src/include/access/transam.h | 10 ++ src/include/catalog/oid_dispatch.h | 7 + src/include/utils/acl.h | 8 ++ .../expected/resgroup/resgroup_mdb_admin.out | 128 ++++++++++++++++++ .../isolation2_resgroup_v1_schedule | 3 + .../isolation2_resgroup_v2_schedule | 3 + .../sql/resgroup/resgroup_mdb_admin.sql | 91 +++++++++++++ 16 files changed, 528 insertions(+), 28 deletions(-) create mode 100644 gpcontrib/gp_toolkit/expected/gp_toolkit_mdb_admin.out create mode 100644 gpcontrib/gp_toolkit/gp_toolkit--1.6--1.7.sql create mode 100644 gpcontrib/gp_toolkit/mdb_admin.c create mode 100644 gpcontrib/gp_toolkit/sql/gp_toolkit_mdb_admin.sql create mode 100644 src/test/isolation2/expected/resgroup/resgroup_mdb_admin.out create mode 100644 src/test/isolation2/sql/resgroup/resgroup_mdb_admin.sql diff --git a/gpcontrib/gp_toolkit/Makefile b/gpcontrib/gp_toolkit/Makefile index 82fc7f7dee9..edb63558593 100644 --- a/gpcontrib/gp_toolkit/Makefile +++ b/gpcontrib/gp_toolkit/Makefile @@ -1,15 +1,15 @@ EXTENSION = gp_toolkit DATA = gp_toolkit--1.1--1.2.sql gp_toolkit--1.0--1.1.sql gp_toolkit--1.0.sql \ gp_toolkit--1.2--1.3.sql gp_toolkit--1.3.sql gp_toolkit--1.3--1.4.sql \ - gp_toolkit--1.4--1.5.sql gp_toolkit--1.5--1.6.sql + gp_toolkit--1.4--1.5.sql gp_toolkit--1.5--1.6.sql gp_toolkit--1.6--1.7.sql MODULE_big = gp_toolkit ifeq ($(shell uname -s), Linux) -OBJS = resgroup.o gp_partition_maint.o +OBJS = resgroup.o gp_partition_maint.o mdb_admin.o else -OBJS = resgroup-dummy.o gp_partition_maint.o +OBJS = resgroup-dummy.o gp_partition_maint.o mdb_admin.o endif -REGRESS = resource_manager_restore_to_none gp_toolkit resource_manager_switch_to_queue gp_toolkit_resqueue gp_toolkit_ao_funcs gp_partition_maint +REGRESS = resource_manager_restore_to_none gp_toolkit gp_toolkit_mdb_admin resource_manager_switch_to_queue gp_toolkit_resqueue gp_toolkit_ao_funcs gp_partition_maint EXTRA_REGRESS_OPTS = --init-file=$(top_builddir)/src/test/regress/init_file ifdef USE_PGXS diff --git a/gpcontrib/gp_toolkit/expected/gp_toolkit_mdb_admin.out b/gpcontrib/gp_toolkit/expected/gp_toolkit_mdb_admin.out new file mode 100644 index 00000000000..aefc4a9b3e8 --- /dev/null +++ b/gpcontrib/gp_toolkit/expected/gp_toolkit_mdb_admin.out @@ -0,0 +1,46 @@ +-- Tests for gp_toolkit.pg_create_mdb_admin_role(): creation of the +-- fixed-OID mdb_admin role and the resource-group permission gate it +-- enables. gp_toolkit is installed in template1 by gpinitsystem, so the +-- extension is already present in this database. +-- --------------------------------------------------------------------- +-- pg_create_mdb_admin_role() creates the mdb_admin role with its fixed OID. +-- --------------------------------------------------------------------- +SELECT gp_toolkit.pg_create_mdb_admin_role() AS mdb_admin_oid; + mdb_admin_oid +--------------- + 8067 +(1 row) + +-- The role exists with the fixed OID and is a non-login, non-superuser, +-- connection-limited role. +SELECT oid = 8067 AS has_fixed_oid, rolcanlogin, rolsuper, + rolcreaterole, rolcreatedb, rolconnlimit + FROM pg_authid WHERE rolname = 'mdb_admin'; + has_fixed_oid | rolcanlogin | rolsuper | rolcreaterole | rolcreatedb | rolconnlimit +---------------+-------------+----------+---------------+-------------+-------------- + t | f | f | f | f | 0 +(1 row) + +-- Creating it a second time is rejected. +SELECT gp_toolkit.pg_create_mdb_admin_role(); +ERROR: role with OID 8067 already exists +-- --------------------------------------------------------------------- +-- Resource-group permission gate: a role that is not a member of mdb_admin +-- is rejected on every entry point. These checks run before the "resource +-- group is enabled" check, so they are deterministic regardless of the +-- resource manager in use. +-- --------------------------------------------------------------------- +CREATE ROLE regress_rg_noadmin; +SET ROLE regress_rg_noadmin; +CREATE RESOURCE GROUP regress_rg_x WITH (concurrency=1, cpu_max_percent=5); +ERROR: must be mdb_admin to create resource groups +ALTER RESOURCE GROUP regress_rg_x SET cpu_max_percent 6; +ERROR: must be mdb_admin to alter resource groups +DROP RESOURCE GROUP regress_rg_x; +ERROR: must be mdb_admin to drop resource groups +RESET ROLE; +DROP ROLE regress_rg_noadmin; +-- --------------------------------------------------------------------- +-- Cleanup. +-- --------------------------------------------------------------------- +DROP ROLE mdb_admin; diff --git a/gpcontrib/gp_toolkit/gp_toolkit--1.6--1.7.sql b/gpcontrib/gp_toolkit/gp_toolkit--1.6--1.7.sql new file mode 100644 index 00000000000..32af2e25abe --- /dev/null +++ b/gpcontrib/gp_toolkit/gp_toolkit--1.6--1.7.sql @@ -0,0 +1,21 @@ +/* gpcontrib/gp_toolkit/gp_toolkit--1.6--1.7.sql */ + +-- complain if script is sourced in psql, rather than via ALTER EXTENSION +\echo Use "ALTER EXTENSION gp_toolkit UPDATE TO '1.7'" to load this file. \quit + +-- Create the mdb_admin privilege role with its fixed OID (8067). +-- +-- The server lets members of mdb_admin CREATE/ALTER/DROP resource groups and +-- run pg_resgroup_move_query() without superuser. The role is identified by +-- a fixed, well-known OID rather than by name, so it must be created through +-- this function (a plain CREATE ROLE would assign an ordinary OID and the +-- permission checks would not recognise its members). The OID assignment is +-- dispatched to the segments, so the role has the same OID cluster-wide. +-- +-- Typical usage (once per cluster, as superuser): +-- SELECT gp_toolkit.pg_create_mdb_admin_role(); +-- GRANT mdb_admin TO cloud_admin; +CREATE FUNCTION gp_toolkit.pg_create_mdb_admin_role() +RETURNS OID +AS 'gp_toolkit.so', 'pg_create_mdb_admin_role' +LANGUAGE C STRICT; diff --git a/gpcontrib/gp_toolkit/gp_toolkit.control b/gpcontrib/gp_toolkit/gp_toolkit.control index 0d9b47e7e6d..c8c0a4bacad 100644 --- a/gpcontrib/gp_toolkit/gp_toolkit.control +++ b/gpcontrib/gp_toolkit/gp_toolkit.control @@ -1,5 +1,5 @@ # gp_toolkit extension comment = 'various GPDB administrative views/functions' -default_version = '1.6' +default_version = '1.7' schema = gp_toolkit diff --git a/gpcontrib/gp_toolkit/mdb_admin.c b/gpcontrib/gp_toolkit/mdb_admin.c new file mode 100644 index 00000000000..6ffc1eb0ad7 --- /dev/null +++ b/gpcontrib/gp_toolkit/mdb_admin.c @@ -0,0 +1,86 @@ +/*------------------------------------------------------------------------- + * + * mdb_admin.c + * Provisioning of the mdb_admin privilege role + * + * gpcontrib/gp_toolkit/mdb_admin.c + * + *------------------------------------------------------------------------- + */ +#include "postgres.h" + +#include "catalog/oid_dispatch.h" +#include "catalog/pg_authid.h" +#include "commands/user.h" +#include "fmgr.h" +#include "miscadmin.h" +#include "nodes/makefuncs.h" +#include "nodes/parsenodes.h" +#include "utils/acl.h" +#include "utils/builtins.h" +#include "utils/syscache.h" + +/* Name of the mdb_admin role; its OID is MDB_ADMIN_ROLEID (see acl.h). */ +#define MDB_ADMIN_ROLE_NAME "mdb_admin" + +PG_FUNCTION_INFO_V1(pg_create_mdb_admin_role); + +/* + * Create the mdb_admin role with its fixed OID (MDB_ADMIN_ROLEID, 8067). + * + * The core privilege checks identify mdb_admin by this fixed OID (see acl.c + * and resgroupcmds.c), so the role must always be created with it. On a + * Cloudberry cluster the OID is dispatched to the segments so the role ends + * up with the same OID everywhere. Returns the new role's OID. + */ +Datum +pg_create_mdb_admin_role(PG_FUNCTION_ARGS) +{ + CreateRoleStmt stmt; + List *options = NIL; + Oid roleid; + + /* + * Only a superuser may establish the mdb_admin privilege role. Otherwise + * a CREATEROLE user could drop mdb_admin and re-create it (CreateRole only + * requires CREATEROLE), taking ownership of the fixed-OID role and + * granting the capability to itself. + */ + if (!superuser()) + ereport(ERROR, + (errcode(ERRCODE_INSUFFICIENT_PRIVILEGE), + errmsg("must be superuser to create the mdb_admin role"))); + + /* Check if a role with the fixed OID already exists. */ + if (SearchSysCacheExists1(AUTHOID, ObjectIdGetDatum(MDB_ADMIN_ROLEID))) + ereport(ERROR, + (errcode(ERRCODE_DUPLICATE_OBJECT), + errmsg("role with OID %u already exists", MDB_ADMIN_ROLEID))); + + /* Check if a role named "mdb_admin" already exists. */ + if (SearchSysCacheExists1(AUTHNAME, CStringGetDatum(MDB_ADMIN_ROLE_NAME))) + ereport(ERROR, + (errcode(ERRCODE_DUPLICATE_OBJECT), + errmsg("role \"%s\" already exists", MDB_ADMIN_ROLE_NAME))); + + /* Build options for CreateRole: connection limit = 0. */ + options = list_make1(makeDefElem("connectionlimit", + (Node *) makeInteger(0), -1)); + + /* Prepare the CreateRoleStmt. */ + memset(&stmt, 0, sizeof(stmt)); + stmt.type = T_CreateRoleStmt; + stmt.stmt_type = ROLESTMT_ROLE; + stmt.role = MDB_ADMIN_ROLE_NAME; + stmt.options = options; + + /* + * Request the fixed OID for the role. GetNewOidForAuthId() consumes and + * clears this override. + */ + next_aux_pg_authid_oid = MDB_ADMIN_ROLEID; + + roleid = CreateRole(NULL, &stmt); + + PG_RETURN_OID(roleid); +} diff --git a/gpcontrib/gp_toolkit/sql/gp_toolkit_mdb_admin.sql b/gpcontrib/gp_toolkit/sql/gp_toolkit_mdb_admin.sql new file mode 100644 index 00000000000..6643b1c218c --- /dev/null +++ b/gpcontrib/gp_toolkit/sql/gp_toolkit_mdb_admin.sql @@ -0,0 +1,37 @@ +-- Tests for gp_toolkit.pg_create_mdb_admin_role(): creation of the +-- fixed-OID mdb_admin role and the resource-group permission gate it +-- enables. gp_toolkit is installed in template1 by gpinitsystem, so the +-- extension is already present in this database. + +-- --------------------------------------------------------------------- +-- pg_create_mdb_admin_role() creates the mdb_admin role with its fixed OID. +-- --------------------------------------------------------------------- +SELECT gp_toolkit.pg_create_mdb_admin_role() AS mdb_admin_oid; + +-- The role exists with the fixed OID and is a non-login, non-superuser, +-- connection-limited role. +SELECT oid = 8067 AS has_fixed_oid, rolcanlogin, rolsuper, + rolcreaterole, rolcreatedb, rolconnlimit + FROM pg_authid WHERE rolname = 'mdb_admin'; + +-- Creating it a second time is rejected. +SELECT gp_toolkit.pg_create_mdb_admin_role(); + +-- --------------------------------------------------------------------- +-- Resource-group permission gate: a role that is not a member of mdb_admin +-- is rejected on every entry point. These checks run before the "resource +-- group is enabled" check, so they are deterministic regardless of the +-- resource manager in use. +-- --------------------------------------------------------------------- +CREATE ROLE regress_rg_noadmin; +SET ROLE regress_rg_noadmin; +CREATE RESOURCE GROUP regress_rg_x WITH (concurrency=1, cpu_max_percent=5); +ALTER RESOURCE GROUP regress_rg_x SET cpu_max_percent 6; +DROP RESOURCE GROUP regress_rg_x; +RESET ROLE; +DROP ROLE regress_rg_noadmin; + +-- --------------------------------------------------------------------- +-- Cleanup. +-- --------------------------------------------------------------------- +DROP ROLE mdb_admin; diff --git a/src/backend/catalog/oid_dispatch.c b/src/backend/catalog/oid_dispatch.c index 6f39a07857e..af49156bdde 100644 --- a/src/backend/catalog/oid_dispatch.c +++ b/src/backend/catalog/oid_dispatch.c @@ -156,6 +156,18 @@ static MemoryContext oids_context = NULL; static bool preserve_oids_on_commit = false; +/* + * OID to assign to the next auxiliary pg_authid role created through + * GetNewOidForAuthId(), or InvalidOid for the normal allocation path. + * + * This is the GPDB analogue of upstream PostgreSQL's + * binary_upgrade_next_pg_authid_oid (which is disabled here in favour of the + * generic OID pre-assignment machinery). It lets gp_toolkit + * create roles such as mdb_admin with a fixed, well-known OID. It is reset + * to InvalidOid as soon as it is consumed. + */ +Oid next_aux_pg_authid_oid = InvalidOid; + /* * These will be used by the schema restoration process during binary upgrade, * so any new object must not use any Oid in this structure or else there will @@ -423,6 +435,7 @@ GetNewOrPreassignedOid(Relation relation, Oid indexId, AttrNumber oidcolumn, OidAssignment *searchkey) { Oid oid; + Oid forcedOid = searchkey->oid; searchkey->catalog = RelationGetRelid(relation); @@ -461,8 +474,18 @@ GetNewOrPreassignedOid(Relation relation, Oid indexId, AttrNumber oidcolumn, { MemoryContext oldcontext; - /* Assign a new oid, and memorize it in the list of OIDs to dispatch */ - oid = GetNewOidWithIndex(relation, indexId, oidcolumn); + /* + * Assign a new oid, and memorize it in the list of OIDs to dispatch. + * + * A caller may request a fixed, well-known OID by passing it in + * searchkey->oid (e.g. the mdb_admin auxiliary role, see + * GetNewOidForAuthId()). In that case use the requested OID instead + * of allocating a fresh one, but still record it for dispatch so the + * QEs end up with the same OID. + */ + oid = OidIsValid(forcedOid) + ? forcedOid + : GetNewOidWithIndex(relation, indexId, oidcolumn); oldcontext = MemoryContextSwitchTo(get_oids_context()); searchkey->oid = oid; @@ -479,7 +502,9 @@ GetNewOrPreassignedOid(Relation relation, Oid indexId, AttrNumber oidcolumn, } else { - oid = GetNewOidWithIndex(relation, indexId, oidcolumn); + oid = OidIsValid(forcedOid) + ? forcedOid + : GetNewOidWithIndex(relation, indexId, oidcolumn); } return oid; @@ -572,6 +597,25 @@ GetNewOidForAuthId(Relation relation, Oid indexId, AttrNumber oidcolumn, memset(&key, 0, sizeof(OidAssignment)); key.type = T_OidAssignment; key.objname = rolname; + + /* + * Allow auxiliary roles (such as mdb_admin, see gpcontrib/gp_toolkit) + * to be created with a fixed, well-known OID. The OID is supplied through + * the next_aux_pg_authid_oid override, mirroring how upstream PostgreSQL + * assigns role OIDs during binary upgrade. We only honor it for OIDs in + * the auxiliary range to avoid clashing with normal OID allocation, and + * reset it immediately so it affects a single role only. + */ + if (OidIsValid(next_aux_pg_authid_oid)) + { + if (!IsAuxOid(next_aux_pg_authid_oid)) + elog(ERROR, "pre-assigned auxiliary role OID %u is out of the auxiliary OID range", + next_aux_pg_authid_oid); + + key.oid = next_aux_pg_authid_oid; + next_aux_pg_authid_oid = InvalidOid; + } + return GetNewOrPreassignedOid(relation, indexId, oidcolumn, &key); } diff --git a/src/backend/commands/resgroupcmds.c b/src/backend/commands/resgroupcmds.c index 384675edb7f..b746a3db49b 100644 --- a/src/backend/commands/resgroupcmds.c +++ b/src/backend/commands/resgroupcmds.c @@ -34,6 +34,7 @@ #include "commands/resgroupcmds.h" #include "miscadmin.h" #include "nodes/pg_list.h" +#include "utils/acl.h" #include "utils/builtins.h" #include "utils/datetime.h" #include "utils/fmgroids.h" @@ -103,11 +104,11 @@ CreateResourceGroup(CreateResourceGroupStmt *stmt) int nResGroups; MemoryContext oldContext; - /* Permission check - only superuser can create groups. */ - if (!superuser()) + /* Permission check - only superuser or mdb_admin can create groups. */ + if (!is_member_of_role(GetUserId(), MDB_ADMIN_ROLEID)) ereport(ERROR, (errcode(ERRCODE_INSUFFICIENT_PRIVILEGE), - errmsg("must be superuser to create resource groups"))); + errmsg("must be mdb_admin to create resource groups"))); /* * Check for an illegal name ('none' is used to signify no group in ALTER ROLE). @@ -269,11 +270,11 @@ DropResourceGroup(DropResourceGroupStmt *stmt) Oid groupid; ResourceGroupCallbackContext *callbackCtx; - /* Permission check - only superuser can drop resource groups. */ - if (!superuser()) + /* Permission check - only superuser or mdb_admin can drop resource groups. */ + if (!is_member_of_role(GetUserId(), MDB_ADMIN_ROLEID)) ereport(ERROR, (errcode(ERRCODE_INSUFFICIENT_PRIVILEGE), - errmsg("must be superuser to drop resource groups"))); + errmsg("must be mdb_admin to drop resource groups"))); /* * Check the pg_resgroup relation to be certain the resource group already @@ -302,6 +303,13 @@ DropResourceGroup(DropResourceGroupStmt *stmt) */ groupid = ((Form_pg_resgroup) GETSTRUCT(tuple))->oid; + /* Permission check - only superuser can drop the admin/system resource groups. */ + if (!superuser() && (groupid == ADMINRESGROUP_OID || groupid == SYSTEMRESGROUP_OID)) + ereport(ERROR, + (errcode(ERRCODE_INSUFFICIENT_PRIVILEGE), + errmsg("must be superuser to drop resource group \"%s\"", + stmt->name))); + /* cannot DROP default resource groups */ if (groupid == DEFAULTRESGROUP_OID || groupid == ADMINRESGROUP_OID @@ -375,11 +383,24 @@ AlterResourceGroup(AlterResourceGroupStmt *stmt) ResourceGroupCallbackContext *callbackCtx; MemoryContext oldContext; - /* Permission check - only superuser can alter resource groups. */ - if (!superuser()) + /* Permission check - only mdb_admin can alter resource groups. */ + if (!is_member_of_role(GetUserId(), MDB_ADMIN_ROLEID)) + ereport(ERROR, + (errcode(ERRCODE_INSUFFICIENT_PRIVILEGE), + errmsg("must be mdb_admin to alter resource groups"))); + + /* + * Check the pg_resgroup relation to be certain the resource group already + * exists. + */ + groupid = get_resgroup_oid(stmt->name, false); + + /* Permission check - only superuser can alter the admin/system resource groups. */ + if (!superuser() && (groupid == ADMINRESGROUP_OID || groupid == SYSTEMRESGROUP_OID)) ereport(ERROR, (errcode(ERRCODE_INSUFFICIENT_PRIVILEGE), - errmsg("must be superuser to alter resource groups"))); + errmsg("must be superuser to alter resource group \"%s\"", + stmt->name))); /* Currently we only support to ALTER one limit at one time */ Assert(list_length(stmt->options) == 1); @@ -406,12 +427,6 @@ AlterResourceGroup(AlterResourceGroupStmt *stmt) checkResgroupCapLimit(limitType, value); } - /* - * Check the pg_resgroup relation to be certain the resource group already - * exists. - */ - groupid = get_resgroup_oid(stmt->name, false); - if (limitType == RESGROUP_LIMIT_TYPE_CONCURRENCY && value == 0 && groupid == ADMINRESGROUP_OID) @@ -500,7 +515,7 @@ AlterResourceGroup(AlterResourceGroupStmt *stmt) RESGROUP_DEFAULT_CPU_WEIGHT, ""); updateResgroupCapabilityEntry(pg_resgroupcapability_rel, - groupid, RESGROUP_LIMIT_TYPE_CPUSET, + groupid, RESGROUP_LIMIT_TYPE_CPUSET, 0, caps.cpuset); } else if (limitType == RESGROUP_LIMIT_TYPE_CPU) @@ -1007,7 +1022,7 @@ parseStmtOptions(CreateResourceGroupStmt *stmt, ResGroupCaps *caps) else mask |= 1 << type; - if (type == RESGROUP_LIMIT_TYPE_CPUSET) + if (type == RESGROUP_LIMIT_TYPE_CPUSET) { const char *cpuset = defGetString(defel); strlcpy(caps->cpuset, cpuset, sizeof(caps->cpuset)); @@ -1611,7 +1626,7 @@ checkCpuSetByRole(const char *cpuset) * ex: * cpuset = "1;4" * then we should assign '1' to corrdinator and '4' to segment - * + * * cpuset = "1" * assign '1' to both coordinator and segment */ diff --git a/src/backend/utils/resgroup/resgroup_helper.c b/src/backend/utils/resgroup/resgroup_helper.c index 00aaded168d..5acca0be5e7 100644 --- a/src/backend/utils/resgroup/resgroup_helper.c +++ b/src/backend/utils/resgroup/resgroup_helper.c @@ -21,6 +21,7 @@ #include "cdb/cdbvars.h" #include "commands/resgroupcmds.h" #include "storage/procarray.h" +#include "utils/acl.h" #include "utils/builtins.h" #include "utils/datetime.h" #include "utils/resgroup.h" @@ -464,10 +465,10 @@ pg_resgroup_move_query(PG_FUNCTION_ARGS) (errcode(ERRCODE_INSUFFICIENT_PRIVILEGE), (errmsg("resource group is not enabled")))); - if (!superuser()) + if (!is_member_of_role(GetUserId(), MDB_ADMIN_ROLEID)) ereport(ERROR, (errcode(ERRCODE_INSUFFICIENT_PRIVILEGE), - (errmsg("must be superuser to move query")))); + (errmsg("must be mdb_admin to move query")))); if (Gp_role == GP_ROLE_DISPATCH) { diff --git a/src/include/access/transam.h b/src/include/access/transam.h index 687799bec9f..0fa8e02ece5 100644 --- a/src/include/access/transam.h +++ b/src/include/access/transam.h @@ -208,6 +208,16 @@ FullTransactionIdAdvance(FullTransactionId *dest) #define FirstBinaryUpgradeReservedObjectId 9000 #define LastBinaryUpgradeReservedObjectId 9100 +/* + * Reserve a block of OIDs for auxiliary catalog objects (such as the + * mdb_admin role created by gp_toolkit). These need fixed, + * well-known OIDs that are stable across clusters, so they live in their own + * range below FirstBinaryUpgradeReservedObjectId. + */ +#define FirstAuxObjectId 8000 +#define LastAuxObjectId 9000 +#define IsAuxOid(oid) ((oid) >= FirstAuxObjectId && (oid) < LastAuxObjectId) + /* * VariableCache is a data structure in shared memory that is used to track * OID and XID assignment state. For largely historical reasons, there is diff --git a/src/include/catalog/oid_dispatch.h b/src/include/catalog/oid_dispatch.h index fbb7a14f59e..324997a399f 100644 --- a/src/include/catalog/oid_dispatch.h +++ b/src/include/catalog/oid_dispatch.h @@ -16,6 +16,13 @@ #include "utils/relcache.h" #include "access/htup.h" +/* + * OID to assign to the next auxiliary pg_authid role created through + * GetNewOidForAuthId(), or InvalidOid for normal allocation. Set by + * gp_toolkit to create roles such as mdb_admin with a fixed OID. + */ +extern PGDLLIMPORT Oid next_aux_pg_authid_oid; + /* Functions used in master */ extern List *GetAssignedOidsForDispatch(void); diff --git a/src/include/utils/acl.h b/src/include/utils/acl.h index 49068f04b2f..a8e0b63f93d 100644 --- a/src/include/utils/acl.h +++ b/src/include/utils/acl.h @@ -213,6 +213,14 @@ extern bool is_member_of_role_nosuper(Oid member, Oid role); extern bool is_admin_of_role(Oid member, Oid role); // -- non-upstream patch begin +/* + * Fixed, well-known OID of the mdb_admin role. The role is created by + * gp_toolkit (pg_create_mdb_admin_role()) with this OID, and the + * resource-group permission checks identify it by OID. It lives in the + * auxiliary OID range (see IsAuxOid()). + */ +#define MDB_ADMIN_ROLEID 8067 + extern bool mdb_admin_allow_bypass_owner_checks(Oid userId, Oid ownerId); extern void check_mdb_admin_is_member_of_role(Oid member, Oid role); diff --git a/src/test/isolation2/expected/resgroup/resgroup_mdb_admin.out b/src/test/isolation2/expected/resgroup/resgroup_mdb_admin.out new file mode 100644 index 00000000000..bc9749cee0e --- /dev/null +++ b/src/test/isolation2/expected/resgroup/resgroup_mdb_admin.out @@ -0,0 +1,128 @@ +-- Tests permission checks for the mdb_admin role with +-- resource groups enabled. + +-- start_matchsubs +-- m/ERROR: cannot find process: \d+/ +-- s/\d+/XXX/g +-- end_matchsubs + +DROP ROLE IF EXISTS role_rg_admin; +DROP +DROP ROLE IF EXISTS role_rg_noadmin; +DROP +DROP ROLE IF EXISTS mdb_admin; +DROP +-- start_ignore +DROP RESOURCE GROUP rg_perm_admin1; +DROP RESOURCE GROUP rg_perm_admin2; +DROP RESOURCE GROUP rg_perm_revoke1; +DROP RESOURCE GROUP rg_perm_revoke2; +DROP RESOURCE GROUP rg_perm_test; +-- end_ignore + +-- --------------------------------------------------------------------- +-- Setup. mdb_admin is identified by its fixed OID, so it must be created +-- through gp_toolkit.pg_create_mdb_admin_role() (a plain CREATE ROLE would +-- assign a different OID and the permission checks would not recognise its +-- members). +-- --------------------------------------------------------------------- +CREATE RESOURCE GROUP rg_perm_test WITH (concurrency=2, cpu_max_percent=10); +CREATE +SELECT gp_toolkit.pg_create_mdb_admin_role(); + pg_create_mdb_admin_role +-------------------------- + 8067 +(1 row) +CREATE ROLE role_rg_admin RESOURCE GROUP rg_perm_test; +CREATE +CREATE ROLE role_rg_noadmin RESOURCE GROUP rg_perm_test; +CREATE +GRANT mdb_admin TO role_rg_admin; +GRANT + +-- --------------------------------------------------------------------- +-- 1. Member of mdb_admin can CREATE/ALTER/DROP resource groups +-- (statements are dispatched to segments). +-- --------------------------------------------------------------------- +1: SET ROLE role_rg_admin; +SET +1: CREATE RESOURCE GROUP rg_perm_admin1 WITH (concurrency=1, cpu_max_percent=5); +CREATE +1: ALTER RESOURCE GROUP rg_perm_admin1 SET cpu_max_percent 6; +ALTER +1: DROP RESOURCE GROUP rg_perm_admin1; +DROP + +-- 2. Even a member cannot ALTER or DROP the system admin_group. +1: ALTER RESOURCE GROUP admin_group SET cpu_max_percent 99; +ERROR: must be superuser to alter resource group "admin_group" +1: DROP RESOURCE GROUP admin_group; +ERROR: must be superuser to drop resource group "admin_group" +1q: ... + +-- --------------------------------------------------------------------- +-- 3. A non-member is rejected on every entry point. +-- --------------------------------------------------------------------- +2: SET ROLE role_rg_noadmin; +SET +2: CREATE RESOURCE GROUP rg_perm_admin2 WITH (concurrency=1, cpu_max_percent=5); +ERROR: must be mdb_admin to create resource groups +2: ALTER RESOURCE GROUP rg_perm_test SET cpu_max_percent 7; +ERROR: must be mdb_admin to alter resource groups +2: DROP RESOURCE GROUP rg_perm_test; +ERROR: must be mdb_admin to drop resource groups +2q: ... + +-- --------------------------------------------------------------------- +-- 4. pg_resgroup_move_query() honours the same permission check. +-- The first call (non-member) must fail with "must be mdb_admin". +-- The second call (member) gets past the permission gate and +-- fails on the pid lookup (masked by start_matchsubs above). +-- --------------------------------------------------------------------- +3: SET ROLE role_rg_noadmin; +SET +3: SELECT pg_resgroup_move_query(999999999, 'admin_group'); +ERROR: must be mdb_admin to move query +3: RESET ROLE; +RESET +3: SET ROLE role_rg_admin; +SET +3: SELECT pg_resgroup_move_query(999999999, 'admin_group'); +ERROR: cannot find process: XXX +3q: ... + +-- --------------------------------------------------------------------- +-- 5. Cross-session REVOKE takes effect on the granted session's +-- next statement (the privilege is re-checked per command, not +-- cached at SET ROLE time). +-- --------------------------------------------------------------------- +4: SET ROLE role_rg_admin; +SET +4: CREATE RESOURCE GROUP rg_perm_revoke1 WITH (concurrency=1, cpu_max_percent=5); +CREATE +5: REVOKE mdb_admin FROM role_rg_admin; +REVOKE +4: CREATE RESOURCE GROUP rg_perm_revoke2 WITH (concurrency=1, cpu_max_percent=5); +ERROR: must be mdb_admin to create resource groups +4: DROP RESOURCE GROUP rg_perm_revoke1; +ERROR: must be mdb_admin to drop resource groups +4q: ... +5q: ... + +-- --------------------------------------------------------------------- +-- Cleanup. Roles must be dropped before the resource group they +-- reference, otherwise DROP RESOURCE GROUP fails with +-- "resource group is used by at least one role". +-- --------------------------------------------------------------------- +RESET ROLE; +RESET +DROP ROLE role_rg_admin; +DROP +DROP ROLE role_rg_noadmin; +DROP +DROP ROLE mdb_admin; +DROP +DROP RESOURCE GROUP rg_perm_revoke1; +DROP +DROP RESOURCE GROUP rg_perm_test; +DROP diff --git a/src/test/isolation2/isolation2_resgroup_v1_schedule b/src/test/isolation2/isolation2_resgroup_v1_schedule index f5dea2d4012..792ab2642bc 100644 --- a/src/test/isolation2/isolation2_resgroup_v1_schedule +++ b/src/test/isolation2/isolation2_resgroup_v1_schedule @@ -35,6 +35,9 @@ test: resgroup/resgroup_move_query test: resgroup/resgroup_recreate test: resgroup/resgroup_functions +# mdb_admin permission checks +test: resgroup/resgroup_mdb_admin + # dump info test: resgroup/resgroup_dumpinfo diff --git a/src/test/isolation2/isolation2_resgroup_v2_schedule b/src/test/isolation2/isolation2_resgroup_v2_schedule index 0190efcd3d8..a422e0115f3 100644 --- a/src/test/isolation2/isolation2_resgroup_v2_schedule +++ b/src/test/isolation2/isolation2_resgroup_v2_schedule @@ -40,6 +40,9 @@ test: resgroup/resgroup_functions #test: resgroup/restore_default_resgroup #test: resgroup/resgroup_parallel_queries +# mdb_admin permission checks +test: resgroup/resgroup_mdb_admin + # dump info test: resgroup/resgroup_dumpinfo diff --git a/src/test/isolation2/sql/resgroup/resgroup_mdb_admin.sql b/src/test/isolation2/sql/resgroup/resgroup_mdb_admin.sql new file mode 100644 index 00000000000..9db82b44906 --- /dev/null +++ b/src/test/isolation2/sql/resgroup/resgroup_mdb_admin.sql @@ -0,0 +1,91 @@ +-- Tests permission checks for the mdb_admin role with +-- resource groups enabled. + +-- start_matchsubs +-- m/ERROR: cannot find process: \d+/ +-- s/\d+/XXX/g +-- end_matchsubs + +DROP ROLE IF EXISTS role_rg_admin; +DROP ROLE IF EXISTS role_rg_noadmin; +DROP ROLE IF EXISTS mdb_admin; +-- start_ignore +DROP RESOURCE GROUP rg_perm_admin1; +DROP RESOURCE GROUP rg_perm_admin2; +DROP RESOURCE GROUP rg_perm_revoke1; +DROP RESOURCE GROUP rg_perm_revoke2; +DROP RESOURCE GROUP rg_perm_test; +-- end_ignore + +-- --------------------------------------------------------------------- +-- Setup. mdb_admin is identified by its fixed OID, so it must be created +-- through gp_toolkit.pg_create_mdb_admin_role() (a plain CREATE ROLE would +-- assign a different OID and the permission checks would not recognise its +-- members). +-- --------------------------------------------------------------------- +CREATE RESOURCE GROUP rg_perm_test WITH (concurrency=2, cpu_max_percent=10); +SELECT gp_toolkit.pg_create_mdb_admin_role(); +CREATE ROLE role_rg_admin RESOURCE GROUP rg_perm_test; +CREATE ROLE role_rg_noadmin RESOURCE GROUP rg_perm_test; +GRANT mdb_admin TO role_rg_admin; + +-- --------------------------------------------------------------------- +-- 1. Member of mdb_admin can CREATE/ALTER/DROP resource groups +-- (statements are dispatched to segments). +-- --------------------------------------------------------------------- +1: SET ROLE role_rg_admin; +1: CREATE RESOURCE GROUP rg_perm_admin1 WITH (concurrency=1, cpu_max_percent=5); +1: ALTER RESOURCE GROUP rg_perm_admin1 SET cpu_max_percent 6; +1: DROP RESOURCE GROUP rg_perm_admin1; + +-- 2. Even a member cannot ALTER or DROP the system admin_group. +1: ALTER RESOURCE GROUP admin_group SET cpu_max_percent 99; +1: DROP RESOURCE GROUP admin_group; +1q: + +-- --------------------------------------------------------------------- +-- 3. A non-member is rejected on every entry point. +-- --------------------------------------------------------------------- +2: SET ROLE role_rg_noadmin; +2: CREATE RESOURCE GROUP rg_perm_admin2 WITH (concurrency=1, cpu_max_percent=5); +2: ALTER RESOURCE GROUP rg_perm_test SET cpu_max_percent 7; +2: DROP RESOURCE GROUP rg_perm_test; +2q: + +-- --------------------------------------------------------------------- +-- 4. pg_resgroup_move_query() honours the same permission check. +-- The first call (non-member) must fail with "must be mdb_admin". +-- The second call (member) gets past the permission gate and +-- fails on the pid lookup (masked by start_matchsubs above). +-- --------------------------------------------------------------------- +3: SET ROLE role_rg_noadmin; +3: SELECT pg_resgroup_move_query(999999999, 'admin_group'); +3: RESET ROLE; +3: SET ROLE role_rg_admin; +3: SELECT pg_resgroup_move_query(999999999, 'admin_group'); +3q: + +-- --------------------------------------------------------------------- +-- 5. Cross-session REVOKE takes effect on the granted session's +-- next statement (the privilege is re-checked per command, not +-- cached at SET ROLE time). +-- --------------------------------------------------------------------- +4: SET ROLE role_rg_admin; +4: CREATE RESOURCE GROUP rg_perm_revoke1 WITH (concurrency=1, cpu_max_percent=5); +5: REVOKE mdb_admin FROM role_rg_admin; +4: CREATE RESOURCE GROUP rg_perm_revoke2 WITH (concurrency=1, cpu_max_percent=5); +4: DROP RESOURCE GROUP rg_perm_revoke1; +4q: +5q: + +-- --------------------------------------------------------------------- +-- Cleanup. Roles must be dropped before the resource group they +-- reference, otherwise DROP RESOURCE GROUP fails with +-- "resource group is used by at least one role". +-- --------------------------------------------------------------------- +RESET ROLE; +DROP ROLE role_rg_admin; +DROP ROLE role_rg_noadmin; +DROP ROLE mdb_admin; +DROP RESOURCE GROUP rg_perm_revoke1; +DROP RESOURCE GROUP rg_perm_test;