diff --git a/src/backend/distributed/commands/role.c b/src/backend/distributed/commands/role.c index 99ce4fb9f87..f13edaf04f8 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 void ErrorIfGrantRoleWithInheritOrSetOption(GrantRoleStmt *stmt); /* controlled via GUC */ bool EnableCreateRolePropagation = true; @@ -1141,6 +1142,8 @@ PreprocessGrantRoleStmt(Node *node, const char *queryString, return NIL; } + ErrorIfGrantRoleWithInheritOrSetOption(stmt); + /* * 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 @@ -1190,6 +1193,29 @@ PostprocessGrantRoleStmt(Node *node, const char *queryString) } +/* + * ErrorIfGrantRoleWithInheritOrSetOption errors out if the given + * GrantRoleStmt has inherit or set option specified in its options + */ +static void +ErrorIfGrantRoleWithInheritOrSetOption(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) + { + ereport(ERROR, (errcode(ERRCODE_FEATURE_NOT_SUPPORTED), + errmsg( + "cannot propagate GRANT/REVOKE statements with" + " specified INHERIT/SET option"))); + } + } + #endif +} + + /* * 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/backend/distributed/commands/utility_hook.c b/src/backend/distributed/commands/utility_hook.c index 10e4246230a..6a7ca87bf8f 100644 --- a/src/backend/distributed/commands/utility_hook.c +++ b/src/backend/distributed/commands/utility_hook.c @@ -694,9 +694,9 @@ ProcessUtilityInternal(PlannedStmt *pstmt, } /* inform the user about potential caveats */ - if (IsA(parsetree, CreatedbStmt)) + if (EnableUnsupportedFeatureMessages) { - if (EnableUnsupportedFeatureMessages) + if (IsA(parsetree, CreatedbStmt)) { ereport(NOTICE, (errmsg("Citus partially supports CREATE DATABASE for " "distributed databases"), @@ -705,13 +705,14 @@ ProcessUtilityInternal(PlannedStmt *pstmt, errhint("You can manually create a database and its " "extensions on workers."))); } - } - else if (IsA(parsetree, CreateRoleStmt) && !EnableCreateRolePropagation) - { - ereport(NOTICE, (errmsg("not propagating CREATE ROLE/USER commands to worker" - " nodes"), - errhint("Connect to worker nodes directly to manually create all" + else if (IsA(parsetree, CreateRoleStmt) && !EnableCreateRolePropagation) + { + ereport(NOTICE, (errmsg("not propagating CREATE ROLE/USER commands to worker" + " nodes"), + errhint( + "Connect to worker nodes directly to manually create all" " necessary users and roles."))); + } } /* diff --git a/src/test/regress/expected/pg16.out b/src/test/regress/expected/pg16.out index 4d1b9bda843..f7504fe19ac 100644 --- a/src/test/regress/expected/pg16.out +++ b/src/test/regress/expected/pg16.out @@ -1009,6 +1009,50 @@ 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 +-- Error out for now in Citus +-- +GRANT role1 TO role2 WITH INHERIT FALSE; +ERROR: cannot propagate GRANT/REVOKE statements with specified INHERIT/SET option +GRANT role1 TO role2 WITH INHERIT TRUE; +ERROR: cannot propagate GRANT/REVOKE statements with specified INHERIT/SET option +GRANT role1 TO role2 WITH INHERIT OPTION; +ERROR: cannot propagate GRANT/REVOKE statements with specified INHERIT/SET option +GRANT role1 TO role2 WITH SET FALSE; +ERROR: cannot propagate GRANT/REVOKE statements with specified INHERIT/SET option +GRANT role1 TO role2 WITH SET TRUE; +ERROR: cannot propagate GRANT/REVOKE statements with specified INHERIT/SET option +GRANT role1 TO role2 WITH SET OPTION; +ERROR: cannot propagate GRANT/REVOKE statements with specified INHERIT/SET option +GRANT role1 TO role2 WITH ADMIN OPTION, INHERIT FALSE, SET FALSE; +ERROR: cannot propagate GRANT/REVOKE statements with specified INHERIT/SET option +-- test REVOKES as well +GRANT role1 TO role2; +REVOKE SET OPTION FOR role1 FROM role2; +ERROR: cannot propagate GRANT/REVOKE statements with specified INHERIT/SET option +REVOKE INHERIT OPTION FOR role1 FROM role2; +ERROR: cannot propagate GRANT/REVOKE statements with specified INHERIT/SET option +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/pg_regress_multi.pl b/src/test/regress/pg_regress_multi.pl index 4cc022198c4..a0a3ed932fc 100755 --- a/src/test/regress/pg_regress_multi.pl +++ b/src/test/regress/pg_regress_multi.pl @@ -509,6 +509,9 @@ sub generate_hba # we disable some restrictions for local objects like local views to not break postgres vanilla test behaviour. push(@pgOptions, "citus.enforce_object_restrictions_for_local_objects=false"); + + # we disable propagating create role statements to not have roles as distributed objects during vanilla tests. + push(@pgOptions, "citus.enable_create_role_propagation=false"); } if ($useMitmproxy) diff --git a/src/test/regress/sql/pg16.sql b/src/test/regress/sql/pg16.sql index fb18a1b589b..5e48ce65a49 100644 --- a/src/test/regress/sql/pg16.sql +++ b/src/test/regress/sql/pg16.sql @@ -591,6 +591,40 @@ 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 +-- Error out for now in Citus +-- +GRANT role1 TO role2 WITH INHERIT FALSE; +GRANT role1 TO role2 WITH INHERIT TRUE; +GRANT role1 TO role2 WITH INHERIT OPTION; +GRANT role1 TO role2 WITH SET FALSE; +GRANT role1 TO role2 WITH SET TRUE; +GRANT role1 TO role2 WITH SET OPTION; +GRANT role1 TO role2 WITH ADMIN OPTION, INHERIT FALSE, SET FALSE; + +-- 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;