Skip to content

Commit

Permalink
PG16 - Don't propagate GRANT ROLE with INHERIT/SET option (#7190)
Browse files Browse the repository at this point in the history
We currently don't support propagating these options in Citus
Relevant PG commits:
postgres/postgres@e3ce2de
postgres/postgres@3d14e17

Limitation:
We also need to take care of generated GRANT statements by dependencies
in attempt to distribute something else. Specifically, this part of the
code in `GenerateGrantRoleStmtsOfRole`:
```
grantRoleStmt->admin_opt = membership->admin_option;
```
In PG16, membership also has `inherit_option` and `set_option` which
need to properly be part of the `grantRoleStmt`. We can skip for now
since #7164 will take care of this soon, and also this is not an
expected use-case.
  • Loading branch information
naisila authored and francisjodi committed Nov 13, 2023
1 parent 8de0b4a commit efd2492
Show file tree
Hide file tree
Showing 3 changed files with 187 additions and 0 deletions.
35 changes: 35 additions & 0 deletions src/backend/distributed/commands/role.c
Original file line number Diff line number Diff line change
Expand Up @@ -78,6 +78,7 @@ static const char * WrapQueryInAlterRoleIfExistsCall(const char *query, RoleSpec
static VariableSetStmt * MakeVariableSetStmt(const char *config);
static int ConfigGenericNameCompare(const void *lhs, const void *rhs);
static List * RoleSpecToObjectAddress(RoleSpec *role, bool missing_ok);
static bool IsGrantRoleWithInheritOrSetOption(GrantRoleStmt *stmt);

/* controlled via GUC */
bool EnableCreateRolePropagation = true;
Expand Down Expand Up @@ -1153,6 +1154,19 @@ PreprocessGrantRoleStmt(Node *node, const char *queryString,
return NIL;
}

if (IsGrantRoleWithInheritOrSetOption(stmt))
{
if (EnableUnsupportedFeatureMessages)
{
ereport(NOTICE, (errmsg("not propagating GRANT/REVOKE commands with specified"
" INHERIT/SET options to worker nodes"),
errhint(
"Connect to worker nodes directly to manually run the same"
" GRANT/REVOKE command after disabling DDL propagation.")));
}
return NIL;
}

/*
* Postgres don't seem to use the grantor. Even dropping the grantor doesn't
* seem to affect the membership. If this changes, we might need to add grantors
Expand Down Expand Up @@ -1202,6 +1216,27 @@ PostprocessGrantRoleStmt(Node *node, const char *queryString)
}


/*
* IsGrantRoleWithInheritOrSetOption returns true if the given
* GrantRoleStmt has inherit or set option specified in its options
*/
static bool
IsGrantRoleWithInheritOrSetOption(GrantRoleStmt *stmt)
{
#if PG_VERSION_NUM >= PG_VERSION_16
DefElem *opt = NULL;
foreach_ptr(opt, stmt->opt)
{
if (strcmp(opt->defname, "inherit") == 0 || strcmp(opt->defname, "set") == 0)
{
return true;
}
}
#endif
return false;
}


/*
* ConfigGenericNameCompare compares two config_generic structs based on their
* name fields. If the name fields contain the same strings two structs are
Expand Down
89 changes: 89 additions & 0 deletions src/test/regress/expected/pg16.out
Original file line number Diff line number Diff line change
Expand Up @@ -1009,6 +1009,95 @@ DETAIL: on server postgres@localhost:xxxxx connectionId: xxxxxxx
REVOKE role1 FROM role2;
RESET citus.log_remote_commands;
RESET citus.grep_remote_commands;
--
-- PG16 added new options to GRANT ROLE
-- inherit: https://github.com/postgres/postgres/commit/e3ce2de
-- set: https://github.com/postgres/postgres/commit/3d14e17
-- We don't propagate for now in Citus
--
GRANT role1 TO role2 WITH INHERIT FALSE;
NOTICE: not propagating GRANT/REVOKE commands with specified INHERIT/SET options to worker nodes
HINT: Connect to worker nodes directly to manually run the same GRANT/REVOKE command after disabling DDL propagation.
REVOKE role1 FROM role2;
GRANT role1 TO role2 WITH INHERIT TRUE;
NOTICE: not propagating GRANT/REVOKE commands with specified INHERIT/SET options to worker nodes
HINT: Connect to worker nodes directly to manually run the same GRANT/REVOKE command after disabling DDL propagation.
REVOKE role1 FROM role2;
GRANT role1 TO role2 WITH INHERIT OPTION;
NOTICE: not propagating GRANT/REVOKE commands with specified INHERIT/SET options to worker nodes
HINT: Connect to worker nodes directly to manually run the same GRANT/REVOKE command after disabling DDL propagation.
REVOKE role1 FROM role2;
GRANT role1 TO role2 WITH SET FALSE;
NOTICE: not propagating GRANT/REVOKE commands with specified INHERIT/SET options to worker nodes
HINT: Connect to worker nodes directly to manually run the same GRANT/REVOKE command after disabling DDL propagation.
REVOKE role1 FROM role2;
GRANT role1 TO role2 WITH SET TRUE;
NOTICE: not propagating GRANT/REVOKE commands with specified INHERIT/SET options to worker nodes
HINT: Connect to worker nodes directly to manually run the same GRANT/REVOKE command after disabling DDL propagation.
REVOKE role1 FROM role2;
GRANT role1 TO role2 WITH SET OPTION;
NOTICE: not propagating GRANT/REVOKE commands with specified INHERIT/SET options to worker nodes
HINT: Connect to worker nodes directly to manually run the same GRANT/REVOKE command after disabling DDL propagation.
REVOKE role1 FROM role2;
-- connect to worker node
GRANT role1 TO role2 WITH ADMIN OPTION, INHERIT FALSE, SET FALSE;
NOTICE: not propagating GRANT/REVOKE commands with specified INHERIT/SET options to worker nodes
HINT: Connect to worker nodes directly to manually run the same GRANT/REVOKE command after disabling DDL propagation.
SELECT roleid::regrole::text AS role, member::regrole::text,
admin_option, inherit_option, set_option FROM pg_auth_members
WHERE roleid::regrole::text = 'role1' ORDER BY 1, 2;
role | member | admin_option | inherit_option | set_option
---------------------------------------------------------------------
role1 | role2 | t | f | f
(1 row)

\c - - - :worker_1_port
SELECT roleid::regrole::text AS role, member::regrole::text,
admin_option, inherit_option, set_option FROM pg_auth_members
WHERE roleid::regrole::text = 'role1' ORDER BY 1, 2;
role | member | admin_option | inherit_option | set_option
---------------------------------------------------------------------
(0 rows)

SET citus.enable_ddl_propagation TO off;
GRANT role1 TO role2 WITH ADMIN OPTION, INHERIT FALSE, SET FALSE;
RESET citus.enable_ddl_propagation;
SELECT roleid::regrole::text AS role, member::regrole::text,
admin_option, inherit_option, set_option FROM pg_auth_members
WHERE roleid::regrole::text = 'role1' ORDER BY 1, 2;
role | member | admin_option | inherit_option | set_option
---------------------------------------------------------------------
role1 | role2 | t | f | f
(1 row)

\c - - - :master_port
REVOKE role1 FROM role2;
-- test REVOKES as well
GRANT role1 TO role2;
REVOKE SET OPTION FOR role1 FROM role2;
NOTICE: not propagating GRANT/REVOKE commands with specified INHERIT/SET options to worker nodes
HINT: Connect to worker nodes directly to manually run the same GRANT/REVOKE command after disabling DDL propagation.
REVOKE INHERIT OPTION FOR role1 FROM role2;
NOTICE: not propagating GRANT/REVOKE commands with specified INHERIT/SET options to worker nodes
HINT: Connect to worker nodes directly to manually run the same GRANT/REVOKE command after disabling DDL propagation.
DROP ROLE role1, role2;
-- test that everything works fine for roles that are not propagated
SET citus.enable_ddl_propagation TO off;
CREATE ROLE role3;
CREATE ROLE role4;
CREATE ROLE role5;
RESET citus.enable_ddl_propagation;
-- by default, admin option is false, inherit is true, set is true
GRANT role3 TO role4;
GRANT role3 TO role5 WITH ADMIN TRUE, INHERIT FALSE, SET FALSE;
SELECT roleid::regrole::text AS role, member::regrole::text, admin_option, inherit_option, set_option FROM pg_auth_members WHERE roleid::regrole::text = 'role3' ORDER BY 1, 2;
role | member | admin_option | inherit_option | set_option
---------------------------------------------------------------------
role3 | role4 | f | t | t
role3 | role5 | t | f | f
(2 rows)

DROP ROLE role3, role4, role5;
\set VERBOSITY terse
SET client_min_messages TO ERROR;
DROP EXTENSION postgres_fdw CASCADE;
Expand Down
63 changes: 63 additions & 0 deletions src/test/regress/sql/pg16.sql
Original file line number Diff line number Diff line change
Expand Up @@ -591,6 +591,69 @@ REVOKE role1 FROM role2;
RESET citus.log_remote_commands;
RESET citus.grep_remote_commands;

--
-- PG16 added new options to GRANT ROLE
-- inherit: https://github.com/postgres/postgres/commit/e3ce2de
-- set: https://github.com/postgres/postgres/commit/3d14e17
-- We don't propagate for now in Citus
--
GRANT role1 TO role2 WITH INHERIT FALSE;
REVOKE role1 FROM role2;
GRANT role1 TO role2 WITH INHERIT TRUE;
REVOKE role1 FROM role2;
GRANT role1 TO role2 WITH INHERIT OPTION;
REVOKE role1 FROM role2;
GRANT role1 TO role2 WITH SET FALSE;
REVOKE role1 FROM role2;
GRANT role1 TO role2 WITH SET TRUE;
REVOKE role1 FROM role2;
GRANT role1 TO role2 WITH SET OPTION;
REVOKE role1 FROM role2;

-- connect to worker node
GRANT role1 TO role2 WITH ADMIN OPTION, INHERIT FALSE, SET FALSE;

SELECT roleid::regrole::text AS role, member::regrole::text,
admin_option, inherit_option, set_option FROM pg_auth_members
WHERE roleid::regrole::text = 'role1' ORDER BY 1, 2;

\c - - - :worker_1_port

SELECT roleid::regrole::text AS role, member::regrole::text,
admin_option, inherit_option, set_option FROM pg_auth_members
WHERE roleid::regrole::text = 'role1' ORDER BY 1, 2;

SET citus.enable_ddl_propagation TO off;
GRANT role1 TO role2 WITH ADMIN OPTION, INHERIT FALSE, SET FALSE;
RESET citus.enable_ddl_propagation;

SELECT roleid::regrole::text AS role, member::regrole::text,
admin_option, inherit_option, set_option FROM pg_auth_members
WHERE roleid::regrole::text = 'role1' ORDER BY 1, 2;

\c - - - :master_port
REVOKE role1 FROM role2;

-- test REVOKES as well
GRANT role1 TO role2;
REVOKE SET OPTION FOR role1 FROM role2;
REVOKE INHERIT OPTION FOR role1 FROM role2;

DROP ROLE role1, role2;

-- test that everything works fine for roles that are not propagated
SET citus.enable_ddl_propagation TO off;
CREATE ROLE role3;
CREATE ROLE role4;
CREATE ROLE role5;
RESET citus.enable_ddl_propagation;
-- by default, admin option is false, inherit is true, set is true
GRANT role3 TO role4;
GRANT role3 TO role5 WITH ADMIN TRUE, INHERIT FALSE, SET FALSE;
SELECT roleid::regrole::text AS role, member::regrole::text, admin_option, inherit_option, set_option FROM pg_auth_members WHERE roleid::regrole::text = 'role3' ORDER BY 1, 2;

DROP ROLE role3, role4, role5;

\set VERBOSITY terse
SET client_min_messages TO ERROR;
DROP EXTENSION postgres_fdw CASCADE;
Expand Down

0 comments on commit efd2492

Please sign in to comment.