From 29dbabbcf3c58f658e47907d545aae22e76350e0 Mon Sep 17 00:00:00 2001 From: Naisila Puka <37271756+naisila@users.noreply.github.com> Date: Mon, 11 Sep 2023 15:58:24 +0300 Subject: [PATCH] Fix WITH ADMIN FALSE propagation (#7191) --- .../distributed/deparser/deparse_role_stmts.c | 32 ++----- src/test/regress/expected/pg16.out | 89 ------------------- src/test/regress/sql/pg16.sql | 26 ++++++ 3 files changed, 33 insertions(+), 114 deletions(-) diff --git a/src/backend/distributed/deparser/deparse_role_stmts.c b/src/backend/distributed/deparser/deparse_role_stmts.c index 49756bde4..739cb737c 100644 --- a/src/backend/distributed/deparser/deparse_role_stmts.c +++ b/src/backend/distributed/deparser/deparse_role_stmts.c @@ -452,34 +452,16 @@ AppendGrantWithAdminOption(StringInfo buf, GrantRoleStmt *stmt) int opt_count = 0; foreach_ptr(opt, stmt->opt) { - switch (opt->defname) - { - case "admin": - appendStringInfo(buf, " WITH ADMIN OPTION"); - opt_count++; - break; - - case "inherit": - if (opt_count > 0) - { - appendStringInfo(buf, ", "); - } - appendStringInfo(buf, "INHERIT OPTION "); - opt_count++; + bool admin_option = false; + char *optval = defGetString(opt); + if (strcmp(opt->defname, "admin") == 0 && + parse_bool(optval, &admin_option) && admin_option) + { + appendStringInfo(buf, " WITH ADMIN OPTION"); break; - - - case "set": - if (opt_count > 0) - { - appendStringInfo(buf, ", "); - } - appendStringInfo(buf, "SET OPTION "); - opt_count++; - break; - } } } + } #else if (stmt->admin_opt) { diff --git a/src/test/regress/expected/pg16.out b/src/test/regress/expected/pg16.out index d1275230b..39fd5a29b 100644 --- a/src/test/regress/expected/pg16.out +++ b/src/test/regress/expected/pg16.out @@ -1009,95 +1009,6 @@ 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 230d12a37..58dc83daa 100644 --- a/src/test/regress/sql/pg16.sql +++ b/src/test/regress/sql/pg16.sql @@ -796,6 +796,32 @@ SELECT roleid::regrole::text AS role, member::regrole::text, admin_option, inher DROP ROLE role3, role4, role5; +-- +-- PG16 added WITH ADMIN FALSE option to GRANT ROLE +-- WITH ADMIN FALSE is the default, make sure we propagate correctly in Citus +-- Relevant PG commit: https://github.com/postgres/postgres/commit/e3ce2de +-- + +CREATE ROLE role1; +CREATE ROLE role2; + +SET citus.log_remote_commands TO on; +SET citus.grep_remote_commands = '%GRANT%'; +-- default admin option is false +GRANT role1 TO role2; +REVOKE role1 FROM role2; +-- should behave same as default +GRANT role1 TO role2 WITH ADMIN FALSE; +REVOKE role1 FROM role2; +-- with admin option and with admin true are the same +GRANT role1 TO role2 WITH ADMIN OPTION; +REVOKE role1 FROM role2; +GRANT role1 TO role2 WITH ADMIN TRUE; +REVOKE role1 FROM role2; + +RESET citus.log_remote_commands; +RESET citus.grep_remote_commands; + \set VERBOSITY terse SET client_min_messages TO ERROR; DROP EXTENSION postgres_fdw CASCADE;