This is an automated email from the ASF dual-hosted git repository. leborchuk pushed a commit to branch PG14_ARCHIVE_REBASED in repository https://gitbox.apache.org/repos/asf/cloudberry.git
commit f6b7727da597277a9a0e091c09d1d70e09770e90 Author: Alena Rybakina <[email protected]> AuthorDate: Tue Jun 16 18:46:11 2026 +0300 Allow non-superuser role to manage resource groups. Historically Yandex Greenplum allows non-superuser no managed resource groups. So, a regular non-superuser role allowed to run pg_resgroup_move_query(), and tune CPU/memory limits if granted with mdb_admin. Such feature was introduced as early as 6.22, see also gpdb commit 3ac99962. This commit introduces same feature for managed Cloudberry. To disallow altering predefined roles, fixed-OID hardening is used, reserving 8067 OID to be an mdb_admin role OID. We choose this (efficiently a catalog change) over complex bookkeeping what CREATEROLE can do and what is disallowed. We use Yandex managed predefined roles bootstrap util via auxiliary contrib extension, based on what Yandex Postgres fork does, see also pg-sharding/cpg repo. Co-authored-by: Andrey Borodin <[email protected]> Co-authored-by: reshke <[email protected]> --- .github/workflows/build-cloudberry-rocky8.yml | 1 + .github/workflows/build-cloudberry.yml | 1 + .github/workflows/build-deb-cloudberry.yml | 1 + contrib/Makefile | 1 + contrib/pg_aux_catalog/.gitignore | 6 + contrib/pg_aux_catalog/Makefile | 37 +++++++ contrib/pg_aux_catalog/README.md | 71 ++++++++++++ contrib/pg_aux_catalog/expected/pg_aux_catalog.out | 46 ++++++++ .../isolation2/expected/resgroup_mdb_admin.out | 123 +++++++++++++++++++++ .../pg_aux_catalog/isolation2/isolation2_schedule | 1 + .../isolation2/sql/resgroup_mdb_admin.sql | 91 +++++++++++++++ contrib/pg_aux_catalog/pg_aux_catalog--1.0.sql | 10 ++ contrib/pg_aux_catalog/pg_aux_catalog.c | 88 +++++++++++++++ contrib/pg_aux_catalog/pg_aux_catalog.control | 5 + contrib/pg_aux_catalog/sql/pg_aux_catalog.sql | 38 +++++++ pom.xml | 3 + 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 ++ 22 files changed, 631 insertions(+), 23 deletions(-) diff --git a/.github/workflows/build-cloudberry-rocky8.yml b/.github/workflows/build-cloudberry-rocky8.yml index 2abf88060e3..c8068f098cb 100644 --- a/.github/workflows/build-cloudberry-rocky8.yml +++ b/.github/workflows/build-cloudberry-rocky8.yml @@ -311,6 +311,7 @@ jobs: "contrib/pgstattuple:installcheck", "contrib/tablefunc:installcheck", "contrib/passwordcheck:installcheck", + "contrib/pg_aux_catalog:installcheck", "contrib/pg_buffercache:installcheck", "contrib/sslinfo:installcheck"] }, diff --git a/.github/workflows/build-cloudberry.yml b/.github/workflows/build-cloudberry.yml index ca75f7b42e7..99bc67c99b7 100644 --- a/.github/workflows/build-cloudberry.yml +++ b/.github/workflows/build-cloudberry.yml @@ -304,6 +304,7 @@ jobs: "contrib/pgstattuple:installcheck", "contrib/tablefunc:installcheck", "contrib/passwordcheck:installcheck", + "contrib/pg_aux_catalog:installcheck", "contrib/pg_buffercache:installcheck", "contrib/sslinfo:installcheck"] }, diff --git a/.github/workflows/build-deb-cloudberry.yml b/.github/workflows/build-deb-cloudberry.yml index 85d917b8ff0..fee69b073f7 100644 --- a/.github/workflows/build-deb-cloudberry.yml +++ b/.github/workflows/build-deb-cloudberry.yml @@ -243,6 +243,7 @@ jobs: "contrib/pgstattuple:installcheck", "contrib/tablefunc:installcheck", "contrib/passwordcheck:installcheck", + "contrib/pg_aux_catalog:installcheck", "contrib/pg_buffercache:installcheck", "contrib/sslinfo:installcheck"] }, diff --git a/contrib/Makefile b/contrib/Makefile index b14600e3557..01315b1f6f8 100644 --- a/contrib/Makefile +++ b/contrib/Makefile @@ -34,6 +34,7 @@ SUBDIRS = \ old_snapshot \ pageinspect \ passwordcheck \ + pg_aux_catalog \ postgres_fdw \ pg_buffercache \ pg_freespacemap \ diff --git a/contrib/pg_aux_catalog/.gitignore b/contrib/pg_aux_catalog/.gitignore new file mode 100644 index 00000000000..c4ec060cefb --- /dev/null +++ b/contrib/pg_aux_catalog/.gitignore @@ -0,0 +1,6 @@ +# Generated test output +/log/ +/results/ +/tmp_check/ +/isolation2/results/ +/isolation2/output_iso/ diff --git a/contrib/pg_aux_catalog/Makefile b/contrib/pg_aux_catalog/Makefile new file mode 100644 index 00000000000..439ecf31a76 --- /dev/null +++ b/contrib/pg_aux_catalog/Makefile @@ -0,0 +1,37 @@ +# contrib/pg_aux_catalog/Makefile + +MODULE_big = pg_aux_catalog +OBJS = \ + $(WIN32RES) \ + pg_aux_catalog.o + +EXTENSION = pg_aux_catalog +DATA = pg_aux_catalog--1.0.sql + +PGFILEDESC = "pg_aux_catalog - auxiliary catalog management" + +REGRESS = pg_aux_catalog +REGRESS_OPTS = --init-file=$(top_srcdir)/src/test/regress/init_file + +ifdef USE_PGXS +PG_CONFIG = pg_config +PGXS := $(shell $(PG_CONFIG) --pgxs) +include $(PGXS) +else +subdir = contrib/pg_aux_catalog +top_builddir = ../.. +include $(top_builddir)/src/Makefile.global +include $(top_srcdir)/contrib/contrib-global.mk +endif + +# Multi-session resource-group permission tests, run with the GPDB isolation2 +# harness. Requires a running cluster with resource groups enabled +# (gp_resource_manager=group); see installcheck-resgroup in +# src/test/isolation2. Not part of the default "installcheck". +installcheck-isolation2: install + $(pg_isolation2_regress_installcheck) \ + --init-file=$(top_builddir)/src/test/regress/init_file \ + --inputdir=$(srcdir)/isolation2 \ + --outputdir=isolation2 \ + --schedule=$(srcdir)/isolation2/isolation2_schedule + diff --git a/contrib/pg_aux_catalog/README.md b/contrib/pg_aux_catalog/README.md new file mode 100644 index 00000000000..a89535838ee --- /dev/null +++ b/contrib/pg_aux_catalog/README.md @@ -0,0 +1,71 @@ +# pg_aux_catalog + +Auxiliary catalog management for Apache Cloudberry. + +This extension provisions the **`mdb_admin`** privilege role, which lets a +non-superuser manage resource groups in managed-service deployments where the +client is never given superuser. + +## Background + +In Greenplum/Cloudberry only a superuser may `CREATE`/`ALTER`/`DROP` resource +groups or move a running query between groups with `pg_resgroup_move_query()`. +The server gates those four entry points on membership of `mdb_admin`, +identified by a **fixed OID (8067)** rather than by name, so the privilege is +recognised reliably across the coordinator and all segments. + +A fixed OID cannot be obtained from a plain `CREATE ROLE` (that assigns an +ordinary OID). This extension provides the one supported way to create the +role at OID 8067. + +## Functions + +### `pg_create_mdb_admin_role() returns oid` + +Creates the `mdb_admin` role with its fixed OID (8067). +Returns the OID of the created role (8067). Errors if a role with that OID or +the name `mdb_admin` already exists. The OID assignment is dispatched to the +segments, so the role has the same OID cluster-wide. + +## Usage + +```sql +CREATE EXTENSION pg_aux_catalog; + +-- Provision the role (the control plane does this once per cluster). +SELECT pg_create_mdb_admin_role(); + +-- Grant the capability to a tenant admin. +GRANT mdb_admin TO cloud_admin; + +-- cloud_admin can now manage resource groups without superuser: +SET ROLE cloud_admin; +CREATE RESOURCE GROUP rg_tenant WITH (concurrency = 4, cpu_max_percent = 20); +ALTER RESOURCE GROUP rg_tenant SET cpu_max_percent 30; +DROP RESOURCE GROUP rg_tenant; +``` + +`admin_group` and `system_group` remain superuser-only for `ALTER`/`DROP`: +they are infrastructure, not user-tunable groups. + +## Building and testing + +```sh +make -C contrib/pg_aux_catalog install +make -C contrib/pg_aux_catalog installcheck +``` + +`installcheck` runs a single-session regression test (role creation and the +resource-group permission gate). A multi-session isolation2 test covering the +dispatched / cross-session behaviour lives under `isolation2/` and is run +separately, against a cluster with resource groups enabled +(`gp_resource_manager=group`): + +```sh +make -C contrib/pg_aux_catalog installcheck-isolation2 +``` + +## Credits + +Based on [pg-sharding/cpg](https://github.com/pg-sharding/cpg) commit +`7b8c912`. Some tests are adapted from open-gpdb/gpdb commit `3ac99962ad2`. diff --git a/contrib/pg_aux_catalog/expected/pg_aux_catalog.out b/contrib/pg_aux_catalog/expected/pg_aux_catalog.out new file mode 100644 index 00000000000..32e52b9728d --- /dev/null +++ b/contrib/pg_aux_catalog/expected/pg_aux_catalog.out @@ -0,0 +1,46 @@ +-- Tests for the pg_aux_catalog extension: creation of the fixed-OID +-- mdb_admin role and the resource-group permission gate it enables. +CREATE EXTENSION pg_aux_catalog; +-- --------------------------------------------------------------------- +-- pg_create_mdb_admin_role() creates the mdb_admin role with its fixed OID. +-- --------------------------------------------------------------------- +SELECT 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 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; +DROP EXTENSION pg_aux_catalog; diff --git a/contrib/pg_aux_catalog/isolation2/expected/resgroup_mdb_admin.out b/contrib/pg_aux_catalog/isolation2/expected/resgroup_mdb_admin.out new file mode 100644 index 00000000000..c8fd232faea --- /dev/null +++ b/contrib/pg_aux_catalog/isolation2/expected/resgroup_mdb_admin.out @@ -0,0 +1,123 @@ +-- 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. The mdb_admin role is not predefined in the catalog; it is +-- created here the same way the control plane provisions it at runtime. +-- --------------------------------------------------------------------- +CREATE RESOURCE GROUP rg_perm_test WITH (concurrency=2, cpu_max_percent=10); +CREATE +CREATE ROLE mdb_admin; +CREATE +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: ... <quitting> + +-- --------------------------------------------------------------------- +-- 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: ... <quitting> + +-- --------------------------------------------------------------------- +-- 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: ... <quitting> + +-- --------------------------------------------------------------------- +-- 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: ... <quitting> +5q: ... <quitting> + +-- --------------------------------------------------------------------- +-- 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/contrib/pg_aux_catalog/isolation2/isolation2_schedule b/contrib/pg_aux_catalog/isolation2/isolation2_schedule new file mode 100644 index 00000000000..73b2a8a95a5 --- /dev/null +++ b/contrib/pg_aux_catalog/isolation2/isolation2_schedule @@ -0,0 +1 @@ +test: resgroup_mdb_admin diff --git a/contrib/pg_aux_catalog/isolation2/sql/resgroup_mdb_admin.sql b/contrib/pg_aux_catalog/isolation2/sql/resgroup_mdb_admin.sql new file mode 100644 index 00000000000..1b7ea19fb3e --- /dev/null +++ b/contrib/pg_aux_catalog/isolation2/sql/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 contrib/pg_aux_catalog (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 EXTENSION IF NOT EXISTS pg_aux_catalog; +SELECT 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; diff --git a/contrib/pg_aux_catalog/pg_aux_catalog--1.0.sql b/contrib/pg_aux_catalog/pg_aux_catalog--1.0.sql new file mode 100644 index 00000000000..a1e1b00fcce --- /dev/null +++ b/contrib/pg_aux_catalog/pg_aux_catalog--1.0.sql @@ -0,0 +1,10 @@ +/* contrib/pg_aux_catalog/pg_aux_catalog--1.0.sql */ + +-- complain if script is sourced in psql, rather than via CREATE EXTENSION +\echo Use "CREATE EXTENSION pg_aux_catalog" to load this file. \quit + +-- Create the mdb_admin role with fixed OID 8067 +CREATE FUNCTION pg_create_mdb_admin_role() +RETURNS OID +AS 'MODULE_PATHNAME', 'pg_create_mdb_admin_role' +LANGUAGE C PARALLEL SAFE STRICT; diff --git a/contrib/pg_aux_catalog/pg_aux_catalog.c b/contrib/pg_aux_catalog/pg_aux_catalog.c new file mode 100644 index 00000000000..91685561cca --- /dev/null +++ b/contrib/pg_aux_catalog/pg_aux_catalog.c @@ -0,0 +1,88 @@ +/*------------------------------------------------------------------------- + * + * pg_aux_catalog.c + * Extension for auxiliary catalog management + * + * contrib/pg_aux_catalog/pg_aux_catalog.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" + +PG_MODULE_MAGIC; + +/* 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/contrib/pg_aux_catalog/pg_aux_catalog.control b/contrib/pg_aux_catalog/pg_aux_catalog.control new file mode 100644 index 00000000000..aff4205f7eb --- /dev/null +++ b/contrib/pg_aux_catalog/pg_aux_catalog.control @@ -0,0 +1,5 @@ +# pg_aux_catalog extension +comment = 'auxiliary catalog management (mdb_admin role creation)' +default_version = '1.0' +module_pathname = '$libdir/pg_aux_catalog' +relocatable = false diff --git a/contrib/pg_aux_catalog/sql/pg_aux_catalog.sql b/contrib/pg_aux_catalog/sql/pg_aux_catalog.sql new file mode 100644 index 00000000000..aa16ffe4e13 --- /dev/null +++ b/contrib/pg_aux_catalog/sql/pg_aux_catalog.sql @@ -0,0 +1,38 @@ +-- Tests for the pg_aux_catalog extension: creation of the fixed-OID +-- mdb_admin role and the resource-group permission gate it enables. + +CREATE EXTENSION pg_aux_catalog; + +-- --------------------------------------------------------------------- +-- pg_create_mdb_admin_role() creates the mdb_admin role with its fixed OID. +-- --------------------------------------------------------------------- +SELECT 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 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; +DROP EXTENSION pg_aux_catalog; diff --git a/pom.xml b/pom.xml index 0e000093399..6eaa095fa37 100644 --- a/pom.xml +++ b/pom.xml @@ -352,6 +352,9 @@ code or new licensing patterns. <exclude>contrib/indexscan/indexscan.c</exclude> <exclude>contrib/indexscan/indexscan.sql.in</exclude> + <exclude>contrib/pg_aux_catalog/pg_aux_catalog.c</exclude> + <exclude>contrib/pg_aux_catalog/isolation2/isolation2_schedule</exclude> + <exclude>contrib/file_fdw/init_file</exclude> <exclude>contrib/file_fdw/data/**</exclude> diff --git a/src/backend/catalog/oid_dispatch.c b/src/backend/catalog/oid_dispatch.c index 6f39a07857e..eaa2e099876 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 contrib/pg_aux_catalog + * 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 contrib/pg_aux_catalog) + * 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..a1bf330b762 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 contrib/pg_aux_catalog). 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..d9c543bee5d 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 + * contrib/pg_aux_catalog 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..e3949dd8769 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 + * contrib/pg_aux_catalog (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); --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
