Skip to content

Commit 2ea6898

Browse files
Alena0704Alena0704
committed
Let mdb_admin manage resource groups
In Greenplum/Cloudberry only a superuser may CREATE/ALTER/DROP resource groups. In managed-service deployments superuser is never handed to the client, so a cloud admin has no way to tune their own CPU/memory limits. The upstream-shaped fix is a predefined role pinned in pg_authid.dat. That is correct for master, but a new bootstrap catalog entry bumps CATALOG_VERSION_NO and thus needs a fresh initdb / gpupgrade. We must be able to grant this capability between minor versions. So instead of pinning an OID, gate the four entry points on membership of the mdb_admin role, resolved by name at runtime: role = get_role_oid("mdb_admin", true); if (!is_member_of_role(GetUserId(), role)) ereport(ERROR, (errcode(ERRCODE_INSUFFICIENT_PRIVILEGE), ...)); mdb_admin is an ordinary role created by the control plane with a plain CREATE ROLE, not a built-in catalog role. If the role does not exist, get_role_oid returns InvalidOid, so on a stock cluster only superusers pass the check and nothing changes. The capability is enabled only where the control plane creates the role. admin_group stays superuser-only for ALTER/DROP: it is infrastructure, not a user-tunable group. This commit is adapted from open-gpdb/gpdb commit 3ac99962ad2. Some tests are added from apache/cloudberry PR #1763.
1 parent ae58802 commit 2ea6898

8 files changed

Lines changed: 407 additions & 24 deletions

File tree

‎src/backend/commands/resgroupcmds.c‎

Lines changed: 38 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,7 @@
3434
#include "commands/resgroupcmds.h"
3535
#include "miscadmin.h"
3636
#include "nodes/pg_list.h"
37+
#include "utils/acl.h"
3738
#include "utils/builtins.h"
3839
#include "utils/datetime.h"
3940
#include "utils/fmgroids.h"
@@ -102,12 +103,14 @@ CreateResourceGroup(CreateResourceGroupStmt *stmt)
102103
ResGroupCaps caps;
103104
int nResGroups;
104105
MemoryContext oldContext;
106+
Oid role;
105107

106-
/* Permission check - only superuser can create groups. */
107-
if (!superuser())
108+
/* Permission check - only superuser or mdb_admin can create groups. */
109+
role = get_role_oid("mdb_admin", true);
110+
if (!is_member_of_role(GetUserId(), role))
108111
ereport(ERROR,
109112
(errcode(ERRCODE_INSUFFICIENT_PRIVILEGE),
110-
errmsg("must be superuser to create resource groups")));
113+
errmsg("must be mdb_admin to create resource groups")));
111114

112115
/*
113116
* Check for an illegal name ('none' is used to signify no group in ALTER ROLE).
@@ -268,12 +271,20 @@ DropResourceGroup(DropResourceGroupStmt *stmt)
268271
SysScanDesc sscan;
269272
Oid groupid;
270273
ResourceGroupCallbackContext *callbackCtx;
274+
Oid role;
271275

272-
/* Permission check - only superuser can drop resource groups. */
273-
if (!superuser())
276+
/* Permission check - only superuser or mdb_admin can drop resource groups. */
277+
role = get_role_oid("mdb_admin", true);
278+
if (!is_member_of_role(GetUserId(), role))
274279
ereport(ERROR,
275280
(errcode(ERRCODE_INSUFFICIENT_PRIVILEGE),
276-
errmsg("must be superuser to drop resource groups")));
281+
errmsg("must be mdb_admin to drop resource groups")));
282+
283+
/* Permission check - only superuser can drop resource group admin_group. */
284+
if (!superuser() && (groupid == ADMINRESGROUP_OID || groupid == SYSTEMRESGROUP_OID))
285+
ereport(ERROR,
286+
(errcode(ERRCODE_INSUFFICIENT_PRIVILEGE),
287+
errmsg("must be superuser to drop resource group admin_group")));
277288

278289
/*
279290
* Check the pg_resgroup relation to be certain the resource group already
@@ -374,12 +385,27 @@ AlterResourceGroup(AlterResourceGroupStmt *stmt)
374385
char *io_limit = NULL;
375386
ResourceGroupCallbackContext *callbackCtx;
376387
MemoryContext oldContext;
388+
Oid role;
377389

378-
/* Permission check - only superuser can alter resource groups. */
379-
if (!superuser())
390+
/* Permission check - only mdb_admin can alter resource groups. */
391+
role = get_role_oid("mdb_admin", true);
392+
if (!is_member_of_role(GetUserId(), role))
380393
ereport(ERROR,
381394
(errcode(ERRCODE_INSUFFICIENT_PRIVILEGE),
382-
errmsg("must be superuser to alter resource groups")));
395+
errmsg("must be mdb_admin to alter resource groups")));
396+
397+
/*
398+
* Check the pg_resgroup relation to be certain the resource group already
399+
* exists.
400+
*/
401+
groupid = get_resgroup_oid(stmt->name, false);
402+
403+
/* Permission check - only superuser can alter the admin/system resource groups. */
404+
if (!superuser() && (groupid == ADMINRESGROUP_OID || groupid == SYSTEMRESGROUP_OID))
405+
ereport(ERROR,
406+
(errcode(ERRCODE_INSUFFICIENT_PRIVILEGE),
407+
errmsg("must be superuser to alter resource group \"%s\"",
408+
stmt->name)));
383409

384410
/* Currently we only support to ALTER one limit at one time */
385411
Assert(list_length(stmt->options) == 1);
@@ -406,12 +432,6 @@ AlterResourceGroup(AlterResourceGroupStmt *stmt)
406432
checkResgroupCapLimit(limitType, value);
407433
}
408434

409-
/*
410-
* Check the pg_resgroup relation to be certain the resource group already
411-
* exists.
412-
*/
413-
groupid = get_resgroup_oid(stmt->name, false);
414-
415435
if (limitType == RESGROUP_LIMIT_TYPE_CONCURRENCY &&
416436
value == 0 &&
417437
groupid == ADMINRESGROUP_OID)
@@ -500,7 +520,7 @@ AlterResourceGroup(AlterResourceGroupStmt *stmt)
500520
RESGROUP_DEFAULT_CPU_WEIGHT, "");
501521

502522
updateResgroupCapabilityEntry(pg_resgroupcapability_rel,
503-
groupid, RESGROUP_LIMIT_TYPE_CPUSET,
523+
groupid, RESGROUP_LIMIT_TYPE_CPUSET,
504524
0, caps.cpuset);
505525
}
506526
else if (limitType == RESGROUP_LIMIT_TYPE_CPU)
@@ -1007,7 +1027,7 @@ parseStmtOptions(CreateResourceGroupStmt *stmt, ResGroupCaps *caps)
10071027
else
10081028
mask |= 1 << type;
10091029

1010-
if (type == RESGROUP_LIMIT_TYPE_CPUSET)
1030+
if (type == RESGROUP_LIMIT_TYPE_CPUSET)
10111031
{
10121032
const char *cpuset = defGetString(defel);
10131033
strlcpy(caps->cpuset, cpuset, sizeof(caps->cpuset));
@@ -1611,7 +1631,7 @@ checkCpuSetByRole(const char *cpuset)
16111631
* ex:
16121632
* cpuset = "1;4"
16131633
* then we should assign '1' to corrdinator and '4' to segment
1614-
*
1634+
*
16151635
* cpuset = "1"
16161636
* assign '1' to both coordinator and segment
16171637
*/

‎src/backend/utils/resgroup/resgroup_helper.c‎

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@
2121
#include "cdb/cdbvars.h"
2222
#include "commands/resgroupcmds.h"
2323
#include "storage/procarray.h"
24+
#include "utils/acl.h"
2425
#include "utils/builtins.h"
2526
#include "utils/datetime.h"
2627
#include "utils/resgroup.h"
@@ -458,16 +459,18 @@ pg_resgroup_move_query(PG_FUNCTION_ARGS)
458459
int sessionId;
459460
Oid groupId;
460461
const char *groupName;
462+
Oid role;
461463

462464
if (!IsResGroupEnabled())
463465
ereport(ERROR,
464466
(errcode(ERRCODE_INSUFFICIENT_PRIVILEGE),
465467
(errmsg("resource group is not enabled"))));
466468

467-
if (!superuser())
469+
role = get_role_oid("mdb_admin", true);
470+
if (!is_member_of_role(GetUserId(), role))
468471
ereport(ERROR,
469472
(errcode(ERRCODE_INSUFFICIENT_PRIVILEGE),
470-
(errmsg("must be superuser to move query"))));
473+
(errmsg("must be mdb_admin to move query"))));
471474

472475
if (Gp_role == GP_ROLE_DISPATCH)
473476
{

‎src/test/isolation2/expected/resgroup/resgroup_mdb_admin.out‎

Lines changed: 123 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,123 @@
1+
-- Tests permission checks for the mdb_admin role with
2+
-- resource groups enabled.
3+
4+
-- start_matchsubs
5+
-- m/ERROR: cannot find process: \d+/
6+
-- s/\d+/XXX/g
7+
-- end_matchsubs
8+
9+
DROP ROLE IF EXISTS role_rg_admin;
10+
DROP
11+
DROP ROLE IF EXISTS role_rg_noadmin;
12+
DROP
13+
DROP ROLE IF EXISTS mdb_admin;
14+
DROP
15+
-- start_ignore
16+
DROP RESOURCE GROUP rg_perm_admin1;
17+
DROP RESOURCE GROUP rg_perm_admin2;
18+
DROP RESOURCE GROUP rg_perm_revoke1;
19+
DROP RESOURCE GROUP rg_perm_revoke2;
20+
DROP RESOURCE GROUP rg_perm_test;
21+
-- end_ignore
22+
23+
-- ---------------------------------------------------------------------
24+
-- Setup. The mdb_admin role is not predefined in the catalog; it is
25+
-- created here the same way the control plane provisions it at runtime.
26+
-- ---------------------------------------------------------------------
27+
CREATE RESOURCE GROUP rg_perm_test WITH (concurrency=2, cpu_max_percent=10);
28+
CREATE
29+
CREATE ROLE mdb_admin;
30+
CREATE
31+
CREATE ROLE role_rg_admin RESOURCE GROUP rg_perm_test;
32+
CREATE
33+
CREATE ROLE role_rg_noadmin RESOURCE GROUP rg_perm_test;
34+
CREATE
35+
GRANT mdb_admin TO role_rg_admin;
36+
GRANT
37+
38+
-- ---------------------------------------------------------------------
39+
-- 1. Member of mdb_admin can CREATE/ALTER/DROP resource groups
40+
-- (statements are dispatched to segments).
41+
-- ---------------------------------------------------------------------
42+
1: SET ROLE role_rg_admin;
43+
SET
44+
1: CREATE RESOURCE GROUP rg_perm_admin1 WITH (concurrency=1, cpu_max_percent=5);
45+
CREATE
46+
1: ALTER RESOURCE GROUP rg_perm_admin1 SET cpu_max_percent 6;
47+
ALTER
48+
1: DROP RESOURCE GROUP rg_perm_admin1;
49+
DROP
50+
51+
-- 2. Even a member cannot ALTER or DROP the system admin_group.
52+
1: ALTER RESOURCE GROUP admin_group SET cpu_max_percent 99;
53+
ERROR: must be superuser to alter resource group "admin_group"
54+
1: DROP RESOURCE GROUP admin_group;
55+
ERROR: must be superuser to drop resource group admin_group
56+
1q: ... <quitting>
57+
58+
-- ---------------------------------------------------------------------
59+
-- 3. A non-member is rejected on every entry point.
60+
-- ---------------------------------------------------------------------
61+
2: SET ROLE role_rg_noadmin;
62+
SET
63+
2: CREATE RESOURCE GROUP rg_perm_admin2 WITH (concurrency=1, cpu_max_percent=5);
64+
ERROR: must be mdb_admin to create resource groups
65+
2: ALTER RESOURCE GROUP rg_perm_test SET cpu_max_percent 7;
66+
ERROR: must be mdb_admin to alter resource groups
67+
2: DROP RESOURCE GROUP rg_perm_test;
68+
ERROR: must be mdb_admin to drop resource groups
69+
2q: ... <quitting>
70+
71+
-- ---------------------------------------------------------------------
72+
-- 4. pg_resgroup_move_query() honours the same permission check.
73+
-- The first call (non-member) must fail with "must be mdb_admin".
74+
-- The second call (member) gets past the permission gate and
75+
-- fails on the pid lookup (masked by start_matchsubs above).
76+
-- ---------------------------------------------------------------------
77+
3: SET ROLE role_rg_noadmin;
78+
SET
79+
3: SELECT pg_resgroup_move_query(999999999, 'admin_group');
80+
ERROR: must be mdb_admin to move query
81+
3: RESET ROLE;
82+
RESET
83+
3: SET ROLE role_rg_admin;
84+
SET
85+
3: SELECT pg_resgroup_move_query(999999999, 'admin_group');
86+
ERROR: cannot find process: XXX
87+
3q: ... <quitting>
88+
89+
-- ---------------------------------------------------------------------
90+
-- 5. Cross-session REVOKE takes effect on the granted session's
91+
-- next statement (the privilege is re-checked per command, not
92+
-- cached at SET ROLE time).
93+
-- ---------------------------------------------------------------------
94+
4: SET ROLE role_rg_admin;
95+
SET
96+
4: CREATE RESOURCE GROUP rg_perm_revoke1 WITH (concurrency=1, cpu_max_percent=5);
97+
CREATE
98+
5: REVOKE mdb_admin FROM role_rg_admin;
99+
REVOKE
100+
4: CREATE RESOURCE GROUP rg_perm_revoke2 WITH (concurrency=1, cpu_max_percent=5);
101+
ERROR: must be mdb_admin to create resource groups
102+
4: DROP RESOURCE GROUP rg_perm_revoke1;
103+
ERROR: must be mdb_admin to drop resource groups
104+
4q: ... <quitting>
105+
5q: ... <quitting>
106+
107+
-- ---------------------------------------------------------------------
108+
-- Cleanup. Roles must be dropped before the resource group they
109+
-- reference, otherwise DROP RESOURCE GROUP fails with
110+
-- "resource group is used by at least one role".
111+
-- ---------------------------------------------------------------------
112+
RESET ROLE;
113+
RESET
114+
DROP ROLE role_rg_admin;
115+
DROP
116+
DROP ROLE role_rg_noadmin;
117+
DROP
118+
DROP ROLE mdb_admin;
119+
DROP
120+
DROP RESOURCE GROUP rg_perm_revoke1;
121+
DROP
122+
DROP RESOURCE GROUP rg_perm_test;
123+
DROP

‎src/test/isolation2/isolation2_resgroup_v1_schedule‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,7 @@ test: resgroup/resgroup_move_query
3434
# regression tests
3535
test: resgroup/resgroup_recreate
3636
test: resgroup/resgroup_functions
37+
test: resgroup/resgroup_mdb_admin
3738

3839
# dump info
3940
test: resgroup/resgroup_dumpinfo

‎src/test/isolation2/isolation2_resgroup_v2_schedule‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -35,6 +35,7 @@ test: resgroup/resgroup_io_limit
3535
# regression tests
3636
test: resgroup/resgroup_recreate
3737
test: resgroup/resgroup_functions
38+
test: resgroup/resgroup_mdb_admin
3839

3940
# parallel tests
4041
#test: resgroup/restore_default_resgroup

‎src/test/isolation2/sql/resgroup/resgroup_mdb_admin.sql‎

Lines changed: 89 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,89 @@
1+
-- Tests permission checks for the mdb_admin role with
2+
-- resource groups enabled.
3+
4+
-- start_matchsubs
5+
-- m/ERROR: cannot find process: \d+/
6+
-- s/\d+/XXX/g
7+
-- end_matchsubs
8+
9+
DROP ROLE IF EXISTS role_rg_admin;
10+
DROP ROLE IF EXISTS role_rg_noadmin;
11+
DROP ROLE IF EXISTS mdb_admin;
12+
-- start_ignore
13+
DROP RESOURCE GROUP rg_perm_admin1;
14+
DROP RESOURCE GROUP rg_perm_admin2;
15+
DROP RESOURCE GROUP rg_perm_revoke1;
16+
DROP RESOURCE GROUP rg_perm_revoke2;
17+
DROP RESOURCE GROUP rg_perm_test;
18+
-- end_ignore
19+
20+
-- ---------------------------------------------------------------------
21+
-- Setup. The mdb_admin role is not predefined in the catalog; it is
22+
-- created here the same way the control plane provisions it at runtime.
23+
-- ---------------------------------------------------------------------
24+
CREATE RESOURCE GROUP rg_perm_test WITH (concurrency=2, cpu_max_percent=10);
25+
CREATE ROLE mdb_admin;
26+
CREATE ROLE role_rg_admin RESOURCE GROUP rg_perm_test;
27+
CREATE ROLE role_rg_noadmin RESOURCE GROUP rg_perm_test;
28+
GRANT mdb_admin TO role_rg_admin;
29+
30+
-- ---------------------------------------------------------------------
31+
-- 1. Member of mdb_admin can CREATE/ALTER/DROP resource groups
32+
-- (statements are dispatched to segments).
33+
-- ---------------------------------------------------------------------
34+
1: SET ROLE role_rg_admin;
35+
1: CREATE RESOURCE GROUP rg_perm_admin1 WITH (concurrency=1, cpu_max_percent=5);
36+
1: ALTER RESOURCE GROUP rg_perm_admin1 SET cpu_max_percent 6;
37+
1: DROP RESOURCE GROUP rg_perm_admin1;
38+
39+
-- 2. Even a member cannot ALTER or DROP the system admin_group.
40+
1: ALTER RESOURCE GROUP admin_group SET cpu_max_percent 99;
41+
1: DROP RESOURCE GROUP admin_group;
42+
1q:
43+
44+
-- ---------------------------------------------------------------------
45+
-- 3. A non-member is rejected on every entry point.
46+
-- ---------------------------------------------------------------------
47+
2: SET ROLE role_rg_noadmin;
48+
2: CREATE RESOURCE GROUP rg_perm_admin2 WITH (concurrency=1, cpu_max_percent=5);
49+
2: ALTER RESOURCE GROUP rg_perm_test SET cpu_max_percent 7;
50+
2: DROP RESOURCE GROUP rg_perm_test;
51+
2q:
52+
53+
-- ---------------------------------------------------------------------
54+
-- 4. pg_resgroup_move_query() honours the same permission check.
55+
-- The first call (non-member) must fail with "must be mdb_admin".
56+
-- The second call (member) gets past the permission gate and
57+
-- fails on the pid lookup (masked by start_matchsubs above).
58+
-- ---------------------------------------------------------------------
59+
3: SET ROLE role_rg_noadmin;
60+
3: SELECT pg_resgroup_move_query(999999999, 'admin_group');
61+
3: RESET ROLE;
62+
3: SET ROLE role_rg_admin;
63+
3: SELECT pg_resgroup_move_query(999999999, 'admin_group');
64+
3q:
65+
66+
-- ---------------------------------------------------------------------
67+
-- 5. Cross-session REVOKE takes effect on the granted session's
68+
-- next statement (the privilege is re-checked per command, not
69+
-- cached at SET ROLE time).
70+
-- ---------------------------------------------------------------------
71+
4: SET ROLE role_rg_admin;
72+
4: CREATE RESOURCE GROUP rg_perm_revoke1 WITH (concurrency=1, cpu_max_percent=5);
73+
5: REVOKE mdb_admin FROM role_rg_admin;
74+
4: CREATE RESOURCE GROUP rg_perm_revoke2 WITH (concurrency=1, cpu_max_percent=5);
75+
4: DROP RESOURCE GROUP rg_perm_revoke1;
76+
4q:
77+
5q:
78+
79+
-- ---------------------------------------------------------------------
80+
-- Cleanup. Roles must be dropped before the resource group they
81+
-- reference, otherwise DROP RESOURCE GROUP fails with
82+
-- "resource group is used by at least one role".
83+
-- ---------------------------------------------------------------------
84+
RESET ROLE;
85+
DROP ROLE role_rg_admin;
86+
DROP ROLE role_rg_noadmin;
87+
DROP ROLE mdb_admin;
88+
DROP RESOURCE GROUP rg_perm_revoke1;
89+
DROP RESOURCE GROUP rg_perm_test;

0 commit comments

Comments
 (0)