Skip to content

Commit bf123dd

Browse files
Alena0704x4m
andcommitted
Add predefined role pg_manage_resource_groups
In Cloudberry/Greenplum only a superuser can manage resource groups. In managed-service deployments superuser cannot be granted to the client, so the cloud admin has no way to tune their own CPU and memory limits. Add a predefined role pg_manage_resource_groups (OID 6300) in pg_authid.dat, following the upstream convention used by pg_monitor and pg_read_all_data. Members of this role can CREATE, ALTER and DROP resource groups and call pg_resgroup_move_query() without being a superuser. Delegation is done with standard SQL: GRANT pg_manage_resource_groups TO <user>; The regression and isolation2 tests exercise the new role via GRANT. The basic CREATE/ALTER/DROP test cases in the regression test are adapted from open-gpdb/gpdb commit 3ac99962ad2. Co-authored-by: Andrey Borodin <x4mmm@yandex-team.ru>
1 parent a4ece88 commit bf123dd

12 files changed

Lines changed: 674 additions & 18 deletions

File tree

doc/src/sgml/user-manag.sgml

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -693,6 +693,16 @@ DROP ROLE doomed_role;
693693
database to issue
694694
<link linkend="sql-createsubscription"><command>CREATE SUBSCRIPTION</command></link>.</entry>
695695
</row>
696+
<row>
697+
<entry>pg_manage_resource_groups</entry>
698+
<entry>Allow
699+
<link linkend="sql-createresourcegroup"><command>CREATE</command></link>,
700+
<link linkend="sql-alterresourcegroup"><command>ALTER</command></link> and
701+
<link linkend="sql-dropresourcegroup"><command>DROP RESOURCE GROUP</command></link>,
702+
and execution of <function>pg_resgroup_move_query()</function>, normally
703+
restricted to superusers. The system <literal>admin_group</literal> can
704+
still only be altered or dropped by a superuser.</entry>
705+
</row>
696706
</tbody>
697707
</tgroup>
698708
</table>
@@ -727,6 +737,17 @@ DROP ROLE doomed_role;
727737
superuser. See <xref linkend="functions-admin-signal"/>.
728738
</para>
729739

740+
<para>
741+
The <literal>pg_manage_resource_groups</literal> role is intended to allow
742+
trusted, non-superuser administrators (for example, the database owner in
743+
managed deployments) to define and tune resource groups without holding
744+
full superuser privileges. Members of this role can create, alter and drop
745+
resource groups, and move running queries between groups with
746+
<function>pg_resgroup_move_query()</function>. The system
747+
<literal>admin_group</literal> is reserved for the cluster's control plane
748+
and can only be altered or dropped by a real superuser.
749+
</para>
750+
730751
<para>
731752
The <literal>pg_read_server_files</literal>, <literal>pg_write_server_files</literal> and
732753
<literal>pg_execute_server_program</literal> roles are intended to allow administrators to have

src/backend/commands/resgroupcmds.c

Lines changed: 37 additions & 12 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"
@@ -103,11 +104,15 @@ CreateResourceGroup(CreateResourceGroupStmt *stmt)
103104
int nResGroups;
104105
MemoryContext oldContext;
105106

106-
/* Permission check - only superuser can create groups. */
107-
if (!superuser())
107+
/*
108+
* Permission check - superusers and members of the predefined role
109+
* pg_manage_resource_groups can create resource groups.
110+
*/
111+
if (!has_privs_of_role(GetUserId(), ROLE_PG_MANAGE_RESOURCE_GROUPS))
108112
ereport(ERROR,
109113
(errcode(ERRCODE_INSUFFICIENT_PRIVILEGE),
110-
errmsg("must be superuser to create resource groups")));
114+
errmsg("permission denied to create resource group"),
115+
errhint("Must be superuser or have privileges of the pg_manage_resource_groups role.")));
111116

112117
/*
113118
* Check for an illegal name ('none' is used to signify no group in ALTER ROLE).
@@ -269,11 +274,15 @@ DropResourceGroup(DropResourceGroupStmt *stmt)
269274
Oid groupid;
270275
ResourceGroupCallbackContext *callbackCtx;
271276

272-
/* Permission check - only superuser can drop resource groups. */
273-
if (!superuser())
277+
/*
278+
* Permission check - superusers and members of the predefined role
279+
* pg_manage_resource_groups can drop resource groups.
280+
*/
281+
if (!has_privs_of_role(GetUserId(), ROLE_PG_MANAGE_RESOURCE_GROUPS))
274282
ereport(ERROR,
275283
(errcode(ERRCODE_INSUFFICIENT_PRIVILEGE),
276-
errmsg("must be superuser to drop resource groups")));
284+
errmsg("permission denied to drop resource group \"%s\"", stmt->name),
285+
errhint("Must be superuser or have privileges of the pg_manage_resource_groups role.")));
277286

278287
/*
279288
* Check the pg_resgroup relation to be certain the resource group already
@@ -375,11 +384,15 @@ AlterResourceGroup(AlterResourceGroupStmt *stmt)
375384
ResourceGroupCallbackContext *callbackCtx;
376385
MemoryContext oldContext;
377386

378-
/* Permission check - only superuser can alter resource groups. */
379-
if (!superuser())
387+
/*
388+
* Permission check - superusers and members of the predefined role
389+
* pg_manage_resource_groups can alter resource groups.
390+
*/
391+
if (!has_privs_of_role(GetUserId(), ROLE_PG_MANAGE_RESOURCE_GROUPS))
380392
ereport(ERROR,
381393
(errcode(ERRCODE_INSUFFICIENT_PRIVILEGE),
382-
errmsg("must be superuser to alter resource groups")));
394+
errmsg("permission denied to alter resource group \"%s\"", stmt->name),
395+
errhint("Must be superuser or have privileges of the pg_manage_resource_groups role.")));
383396

384397
/* Currently we only support to ALTER one limit at one time */
385398
Assert(list_length(stmt->options) == 1);
@@ -412,6 +425,18 @@ AlterResourceGroup(AlterResourceGroupStmt *stmt)
412425
*/
413426
groupid = get_resgroup_oid(stmt->name, false);
414427

428+
/*
429+
* The built-in system resource groups may only be altered by a real
430+
* superuser, never by a member of pg_manage_resource_groups
431+
*/
432+
if (!superuser() &&
433+
(groupid == ADMINRESGROUP_OID ||
434+
groupid == SYSTEMRESGROUP_OID))
435+
ereport(ERROR,
436+
(errcode(ERRCODE_INSUFFICIENT_PRIVILEGE),
437+
errmsg("permission denied to alter resource group \"%s\"", stmt->name),
438+
errhint("Must be superuser to alter a system resource group.")));
439+
415440
if (limitType == RESGROUP_LIMIT_TYPE_CONCURRENCY &&
416441
value == 0 &&
417442
groupid == ADMINRESGROUP_OID)
@@ -500,7 +525,7 @@ AlterResourceGroup(AlterResourceGroupStmt *stmt)
500525
RESGROUP_DEFAULT_CPU_WEIGHT, "");
501526

502527
updateResgroupCapabilityEntry(pg_resgroupcapability_rel,
503-
groupid, RESGROUP_LIMIT_TYPE_CPUSET,
528+
groupid, RESGROUP_LIMIT_TYPE_CPUSET,
504529
0, caps.cpuset);
505530
}
506531
else if (limitType == RESGROUP_LIMIT_TYPE_CPU)
@@ -1007,7 +1032,7 @@ parseStmtOptions(CreateResourceGroupStmt *stmt, ResGroupCaps *caps)
10071032
else
10081033
mask |= 1 << type;
10091034

1010-
if (type == RESGROUP_LIMIT_TYPE_CPUSET)
1035+
if (type == RESGROUP_LIMIT_TYPE_CPUSET)
10111036
{
10121037
const char *cpuset = defGetString(defel);
10131038
strlcpy(caps->cpuset, cpuset, sizeof(caps->cpuset));
@@ -1611,7 +1636,7 @@ checkCpuSetByRole(const char *cpuset)
16111636
* ex:
16121637
* cpuset = "1;4"
16131638
* then we should assign '1' to corrdinator and '4' to segment
1614-
*
1639+
*
16151640
* cpuset = "1"
16161641
* assign '1' to both coordinator and segment
16171642
*/

src/backend/utils/resgroup/resgroup_helper.c

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -15,12 +15,14 @@
1515
#include "funcapi.h"
1616
#include "libpq-fe.h"
1717
#include "miscadmin.h"
18+
#include "catalog/pg_authid.h"
1819
#include "catalog/pg_resgroup.h"
1920
#include "cdb/cdbdisp_query.h"
2021
#include "cdb/cdbdispatchresult.h"
2122
#include "cdb/cdbvars.h"
2223
#include "commands/resgroupcmds.h"
2324
#include "storage/procarray.h"
25+
#include "utils/acl.h"
2426
#include "utils/builtins.h"
2527
#include "utils/datetime.h"
2628
#include "utils/resgroup.h"
@@ -464,10 +466,15 @@ pg_resgroup_move_query(PG_FUNCTION_ARGS)
464466
(errcode(ERRCODE_INSUFFICIENT_PRIVILEGE),
465467
(errmsg("resource group is not enabled"))));
466468

467-
if (!superuser())
469+
/*
470+
* Superusers and members of the predefined role
471+
* pg_manage_resource_groups can move a query between resource groups.
472+
*/
473+
if (!has_privs_of_role(GetUserId(), ROLE_PG_MANAGE_RESOURCE_GROUPS))
468474
ereport(ERROR,
469475
(errcode(ERRCODE_INSUFFICIENT_PRIVILEGE),
470-
(errmsg("must be superuser to move query"))));
476+
errmsg("permission denied to move query between resource groups"),
477+
errhint("Must be superuser or have privileges of the pg_manage_resource_groups role.")));
471478

472479
if (Gp_role == GP_ROLE_DISPATCH)
473480
{

src/include/catalog/pg_authid.dat

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -89,6 +89,11 @@
8989
rolcreaterole => 'f', rolcreatedb => 'f', rolcanlogin => 'f',
9090
rolreplication => 'f', rolbypassrls => 'f', rolconnlimit => '-1',
9191
rolpassword => '_null_', rolvaliduntil => '_null_' },
92+
{ oid => '6312', oid_symbol => 'ROLE_PG_MANAGE_RESOURCE_GROUPS',
93+
rolname => 'pg_manage_resource_groups', rolsuper => 'f', rolinherit => 't',
94+
rolcreaterole => 'f', rolcreatedb => 'f', rolcanlogin => 'f',
95+
rolreplication => 'f', rolbypassrls => 'f', rolconnlimit => '-1',
96+
rolpassword => '_null_', rolvaliduntil => '_null_' },
9297
{ oid => '6304', oid_symbol => 'ROLE_PG_CREATE_SUBSCRIPTION',
9398
rolname => 'pg_create_subscription', rolsuper => 'f', rolinherit => 't',
9499
rolcreaterole => 'f', rolcreatedb => 'f', rolcanlogin => 'f',
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

0 commit comments

Comments
 (0)