From efd2492a3eaf1552e34ca7dce7504d68b5bcc43b Mon Sep 17 00:00:00 2001 From: Naisila Puka <37271756+naisila@users.noreply.github.com> Date: Tue, 12 Sep 2023 12:47:37 +0300 Subject: [PATCH] PG16 - Don't propagate GRANT ROLE with INHERIT/SET option (#7190) We currently don't support propagating these options in Citus Relevant PG commits: https://github.com/postgres/postgres/commit/e3ce2de https://github.com/postgres/postgres/commit/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. --- src/backend/distributed/commands/role.c | 35 ++++++++++ src/test/regress/expected/pg16.out | 89 +++++++++++++++++++++++++ src/test/regress/sql/pg16.sql | 63 +++++++++++++++++ 3 files changed, 187 insertions(+) diff --git a/src/backend/distributed/commands/role.c b/src/backend/distributed/commands/role.c index 0ea4569c652..40b138e11de 100644 --- a/src/backend/distributed/commands/role.c +++ b/src/backend/distributed/commands/role.c @@ -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; @@ -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 @@ -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 diff --git a/src/test/regress/expected/pg16.out b/src/test/regress/expected/pg16.out index 39fd5a29b44..d1275230bde 100644 --- a/src/test/regress/expected/pg16.out +++ b/src/test/regress/expected/pg16.out @@ -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; diff --git a/src/test/regress/sql/pg16.sql b/src/test/regress/sql/pg16.sql index c45f7d96031..557efc9e750 100644 --- a/src/test/regress/sql/pg16.sql +++ b/src/test/regress/sql/pg16.sql @@ -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;