Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
21 changes: 21 additions & 0 deletions doc/src/sgml/user-manag.sgml
Original file line number Diff line number Diff line change
Expand Up @@ -693,6 +693,16 @@ DROP ROLE doomed_role;
database to issue
<link linkend="sql-createsubscription"><command>CREATE SUBSCRIPTION</command></link>.</entry>
</row>
<row>
<entry>pg_manage_resource_groups</entry>
<entry>Allow
<link linkend="sql-createresourcegroup"><command>CREATE</command></link>,
<link linkend="sql-alterresourcegroup"><command>ALTER</command></link> and
<link linkend="sql-dropresourcegroup"><command>DROP RESOURCE GROUP</command></link>,
and execution of <function>pg_resgroup_move_query()</function>, normally
restricted to superusers. The system <literal>admin_group</literal> can
still only be altered or dropped by a superuser.</entry>
</row>
</tbody>
</tgroup>
</table>
Expand Down Expand Up @@ -727,6 +737,17 @@ DROP ROLE doomed_role;
superuser. See <xref linkend="functions-admin-signal"/>.
</para>

<para>
The <literal>pg_manage_resource_groups</literal> role is intended to allow
trusted, non-superuser administrators (for example, the database owner in
managed deployments) to define and tune resource groups without holding
full superuser privileges. Members of this role can create, alter and drop
resource groups, and move running queries between groups with
<function>pg_resgroup_move_query()</function>. The system
<literal>admin_group</literal> is reserved for the cluster's control plane
and can only be altered or dropped by a real superuser.
</para>

<para>
The <literal>pg_read_server_files</literal>, <literal>pg_write_server_files</literal> and
<literal>pg_execute_server_program</literal> roles are intended to allow administrators to have
Expand Down
49 changes: 37 additions & 12 deletions src/backend/commands/resgroupcmds.c
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -103,11 +104,15 @@ CreateResourceGroup(CreateResourceGroupStmt *stmt)
int nResGroups;
MemoryContext oldContext;

/* Permission check - only superuser can create groups. */
if (!superuser())
/*
* Permission check - superusers and members of the predefined role
* pg_manage_resource_groups can create resource groups.
*/
if (!has_privs_of_role(GetUserId(), ROLE_PG_MANAGE_RESOURCE_GROUPS))
ereport(ERROR,
(errcode(ERRCODE_INSUFFICIENT_PRIVILEGE),
errmsg("must be superuser to create resource groups")));
errmsg("permission denied to create resource group"),
errhint("Must be superuser or have privileges of the pg_manage_resource_groups role.")));

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

/* Permission check - only superuser can drop resource groups. */
if (!superuser())
/*
* Permission check - superusers and members of the predefined role
* pg_manage_resource_groups can drop resource groups.
*/
if (!has_privs_of_role(GetUserId(), ROLE_PG_MANAGE_RESOURCE_GROUPS))
ereport(ERROR,
(errcode(ERRCODE_INSUFFICIENT_PRIVILEGE),
errmsg("must be superuser to drop resource groups")));
errmsg("permission denied to drop resource group \"%s\"", stmt->name),
errhint("Must be superuser or have privileges of the pg_manage_resource_groups role.")));

/*
* Check the pg_resgroup relation to be certain the resource group already
Expand Down Expand Up @@ -375,11 +384,15 @@ AlterResourceGroup(AlterResourceGroupStmt *stmt)
ResourceGroupCallbackContext *callbackCtx;
MemoryContext oldContext;

/* Permission check - only superuser can alter resource groups. */
if (!superuser())
/*
* Permission check - superusers and members of the predefined role
* pg_manage_resource_groups can alter resource groups.
*/
if (!has_privs_of_role(GetUserId(), ROLE_PG_MANAGE_RESOURCE_GROUPS))
ereport(ERROR,
(errcode(ERRCODE_INSUFFICIENT_PRIVILEGE),
errmsg("must be superuser to alter resource groups")));
errmsg("permission denied to alter resource group \"%s\"", stmt->name),
errhint("Must be superuser or have privileges of the pg_manage_resource_groups role.")));

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

/*
* The built-in system resource groups may only be altered by a real
* superuser, never by a member of pg_manage_resource_groups
*/
if (!superuser() &&
(groupid == ADMINRESGROUP_OID ||
groupid == SYSTEMRESGROUP_OID))
ereport(ERROR,
(errcode(ERRCODE_INSUFFICIENT_PRIVILEGE),
errmsg("permission denied to alter resource group \"%s\"", stmt->name),
errhint("Must be superuser to alter a system resource group.")));

if (limitType == RESGROUP_LIMIT_TYPE_CONCURRENCY &&
value == 0 &&
groupid == ADMINRESGROUP_OID)
Expand Down Expand Up @@ -500,7 +525,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)
Expand Down Expand Up @@ -1007,7 +1032,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));
Expand Down Expand Up @@ -1611,7 +1636,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
*/
Expand Down
11 changes: 9 additions & 2 deletions src/backend/utils/resgroup/resgroup_helper.c
Original file line number Diff line number Diff line change
Expand Up @@ -15,12 +15,14 @@
#include "funcapi.h"
#include "libpq-fe.h"
#include "miscadmin.h"
#include "catalog/pg_authid.h"
#include "catalog/pg_resgroup.h"
#include "cdb/cdbdisp_query.h"
#include "cdb/cdbdispatchresult.h"
#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"
Expand Down Expand Up @@ -464,10 +466,15 @@ pg_resgroup_move_query(PG_FUNCTION_ARGS)
(errcode(ERRCODE_INSUFFICIENT_PRIVILEGE),
(errmsg("resource group is not enabled"))));

if (!superuser())
/*
* Superusers and members of the predefined role
* pg_manage_resource_groups can move a query between resource groups.
*/
if (!has_privs_of_role(GetUserId(), ROLE_PG_MANAGE_RESOURCE_GROUPS))
ereport(ERROR,
(errcode(ERRCODE_INSUFFICIENT_PRIVILEGE),
(errmsg("must be superuser to move query"))));
errmsg("permission denied to move query between resource groups"),
errhint("Must be superuser or have privileges of the pg_manage_resource_groups role.")));

if (Gp_role == GP_ROLE_DISPATCH)
{
Expand Down
5 changes: 5 additions & 0 deletions src/include/catalog/pg_authid.dat
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

bump catversion

Original file line number Diff line number Diff line change
Expand Up @@ -89,6 +89,11 @@
rolcreaterole => 'f', rolcreatedb => 'f', rolcanlogin => 'f',
rolreplication => 'f', rolbypassrls => 'f', rolconnlimit => '-1',
rolpassword => '_null_', rolvaliduntil => '_null_' },
{ oid => '6312', oid_symbol => 'ROLE_PG_MANAGE_RESOURCE_GROUPS',
rolname => 'pg_manage_resource_groups', rolsuper => 'f', rolinherit => 't',
rolcreaterole => 'f', rolcreatedb => 'f', rolcanlogin => 'f',
rolreplication => 'f', rolbypassrls => 'f', rolconnlimit => '-1',
rolpassword => '_null_', rolvaliduntil => '_null_' },
{ oid => '6304', oid_symbol => 'ROLE_PG_CREATE_SUBSCRIPTION',
rolname => 'pg_create_subscription', rolsuper => 'f', rolinherit => 't',
rolcreaterole => 'f', rolcreatedb => 'f', rolcanlogin => 'f',
Expand Down
123 changes: 123 additions & 0 deletions src/test/isolation2/expected/resgroup/resgroup_mdb_admin.out
Original file line number Diff line number Diff line change
@@ -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
Loading