Merge branch 'main' into failure-handling-create-db

src/backend/distributed/commands/role.c

@@ -886,6 +886,14 @@ GenerateGrantRoleStmtsOfRole(Oid roleid)
 	{
 		Form_pg_auth_members membership = (Form_pg_auth_members) GETSTRUCT(tuple);
 
+		ObjectAddress *roleAddress = palloc0(sizeof(ObjectAddress));
+		ObjectAddressSet(*roleAddress, AuthIdRelationId, membership->grantor);
+		if (!IsAnyObjectDistributed(list_make1(roleAddress)))
+		{
+			/* we only need to propagate the grant if the grantor is distributed */
+			continue;
+		}
+
 		GrantRoleStmt *grantRoleStmt = makeNode(GrantRoleStmt);
 		grantRoleStmt->is_grant = true;
 
@@ -901,7 +909,11 @@ GenerateGrantRoleStmtsOfRole(Oid roleid)
 		granteeRole->rolename = GetUserNameFromId(membership->member, true);
 		grantRoleStmt->grantee_roles = list_make1(granteeRole);
 
-		grantRoleStmt->grantor = NULL;
+		RoleSpec *grantorRole = makeNode(RoleSpec);
+		grantorRole->roletype = ROLESPEC_CSTRING;
+		grantorRole->location = -1;
+		grantorRole->rolename = GetUserNameFromId(membership->grantor, false);
+		grantRoleStmt->grantor = grantorRole;
 
 #if PG_VERSION_NUM >= PG_VERSION_16
 
@@ -1241,12 +1253,6 @@ PreprocessGrantRoleStmt(Node *node, const char *queryString,
 		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
-	 * to the dependency resolution too. For now we just don't propagate it.
-	 */
-	stmt->grantor = NULL;
 	stmt->grantee_roles = distributedGranteeRoles;
 	char *sql = DeparseTreeNode((Node *) stmt);
 	stmt->grantee_roles = allGranteeRoles;

src/backend/distributed/deparser/deparse_role_stmts.c

@@ -486,7 +486,6 @@ AppendGrantRoleStmt(StringInfo buf, GrantRoleStmt *stmt)
 	appendStringInfo(buf, "%s ", stmt->is_grant ? " TO " : " FROM ");
 	AppendRoleList(buf, stmt->grantee_roles);
 	AppendGrantWithAdminOption(buf, stmt);
-	AppendGrantedByInGrantForRoleSpec(buf, stmt->grantor, stmt->is_grant);
 	AppendGrantRestrictAndCascadeForRoleSpec(buf, stmt->behavior, stmt->is_grant);
 	AppendGrantedByInGrantForRoleSpec(buf, stmt->grantor, stmt->is_grant);
 	appendStringInfo(buf, ";");

src/test/regress/expected/create_role_propagation.out

@@ -196,6 +196,7 @@ SELECT roleid::regrole::text AS role, member::regrole::text, grantor::regrole::t
 (1 row)
 
 \c - - - :master_port
+create role test_admin_role;
 -- test grants with distributed and non-distributed roles
 SELECT master_remove_node('localhost', :worker_2_port);
  master_remove_node
@@ -221,29 +222,55 @@ CREATE ROLE non_dist_role_4;
 NOTICE:  not propagating CREATE ROLE/USER commands to other nodes
 HINT:  Connect to other nodes directly to manually create all necessary users and roles.
 SET citus.enable_create_role_propagation TO ON;
+grant dist_role_3,dist_role_1 to test_admin_role with admin option;
 SET ROLE dist_role_1;
 GRANT non_dist_role_1 TO non_dist_role_2;
 SET citus.enable_create_role_propagation TO OFF;
+grant dist_role_1 to non_dist_role_1 with admin option;
 SET ROLE non_dist_role_1;
-GRANT dist_role_1 TO dist_role_2;
+GRANT dist_role_1 TO dist_role_2 granted by non_dist_role_1;
 RESET ROLE;
 SET citus.enable_create_role_propagation TO ON;
-GRANT dist_role_3 TO non_dist_role_3;
+GRANT dist_role_3 TO non_dist_role_3 granted by test_admin_role;
 GRANT non_dist_role_4 TO dist_role_4;
+GRANT dist_role_3 TO dist_role_4 granted by test_admin_role;
 SELECT 1 FROM master_add_node('localhost', :worker_2_port);
  ?column?
 ---------------------------------------------------------------------
         1
 (1 row)
 
-SELECT roleid::regrole::text AS role, member::regrole::text, (grantor::regrole::text IN ('postgres', 'non_dist_role_1', 'dist_role_1')) AS grantor, admin_option FROM pg_auth_members WHERE roleid::regrole::text LIKE '%dist\_%' ORDER BY 1, 2;
+SELECT result FROM run_command_on_all_nodes(
+  $$
+  SELECT json_agg(q.* ORDER BY member) FROM (
+    SELECT member::regrole::text, roleid::regrole::text AS role, grantor::regrole::text, admin_option
+    FROM pg_auth_members WHERE roleid::regrole::text = 'dist_role_3'
+  ) q;
+  $$
+);
+                                                result
+---------------------------------------------------------------------
+ [{"member":"dist_role_4","role":"dist_role_3","grantor":"test_admin_role","admin_option":false},     +
+  {"member":"non_dist_role_3","role":"dist_role_3","grantor":"test_admin_role","admin_option":false}, +
+  {"member":"test_admin_role","role":"dist_role_3","grantor":"postgres","admin_option":true}]
+ [{"member":"dist_role_4","role":"dist_role_3","grantor":"test_admin_role","admin_option":false},     +
+  {"member":"test_admin_role","role":"dist_role_3","grantor":"postgres","admin_option":true}]
+ [{"member":"dist_role_4","role":"dist_role_3","grantor":"test_admin_role","admin_option":false},     +
+  {"member":"test_admin_role","role":"dist_role_3","grantor":"postgres","admin_option":true}]
+(3 rows)
+
+REVOKE dist_role_3 from dist_role_4 granted by test_admin_role;
+SELECT roleid::regrole::text AS role, member::regrole::text, (grantor::regrole::text IN ('postgres', 'non_dist_role_1', 'dist_role_1','test_admin_role')) AS grantor, admin_option FROM pg_auth_members WHERE roleid::regrole::text LIKE '%dist\_%' ORDER BY 1, 2;
       role       |     member      | grantor | admin_option
 ---------------------------------------------------------------------
  dist_role_1     | dist_role_2     | t       | f
+ dist_role_1     | non_dist_role_1 | t       | t
+ dist_role_1     | test_admin_role | t       | t
  dist_role_3     | non_dist_role_3 | t       | f
+ dist_role_3     | test_admin_role | t       | t
  non_dist_role_1 | non_dist_role_2 | t       | f
  non_dist_role_4 | dist_role_4     | t       | f
-(4 rows)
+(7 rows)
 
 SELECT objid::regrole FROM pg_catalog.pg_dist_object WHERE classid='pg_authid'::regclass::oid AND objid::regrole::text LIKE '%dist\_%' ORDER BY 1;
       objid
@@ -255,6 +282,9 @@ SELECT objid::regrole FROM pg_catalog.pg_dist_object WHERE classid='pg_authid'::
  non_dist_role_4
 (5 rows)
 
+REVOKE dist_role_3 from non_dist_role_3 granted by test_admin_role;
+revoke dist_role_3,dist_role_1 from test_admin_role cascade;
+drop role test_admin_role;
 \c - - - :worker_1_port
 SELECT roleid::regrole::text AS role, member::regrole::text, grantor::regrole::text, admin_option FROM pg_auth_members WHERE roleid::regrole::text LIKE '%dist\_%' ORDER BY 1, 2;
       role       |   member    | grantor  | admin_option
@@ -276,9 +306,8 @@ SELECT rolname FROM pg_authid WHERE rolname LIKE '%dist\_%' ORDER BY 1;
 SELECT roleid::regrole::text AS role, member::regrole::text, grantor::regrole::text, admin_option FROM pg_auth_members WHERE roleid::regrole::text LIKE '%dist\_%' ORDER BY 1, 2;
       role       |   member    | grantor  | admin_option
 ---------------------------------------------------------------------
- dist_role_1     | dist_role_2 | postgres | f
  non_dist_role_4 | dist_role_4 | postgres | f
-(2 rows)
+(1 row)
 
 SELECT rolname FROM pg_authid WHERE rolname LIKE '%dist\_%' ORDER BY 1;
      rolname

src/test/regress/sql/create_role_propagation.sql

@@ -75,6 +75,8 @@ SELECT roleid::regrole::text AS role, member::regrole::text, grantor::regrole::t
 
 \c - - - :master_port
 
+create role test_admin_role;
+
 -- test grants with distributed and non-distributed roles
 
 SELECT master_remove_node('localhost', :worker_2_port);
@@ -84,6 +86,8 @@ CREATE ROLE dist_role_2;
 CREATE ROLE dist_role_3;
 CREATE ROLE dist_role_4;
 
+
+
 SET citus.enable_create_role_propagation TO OFF;
 
 CREATE ROLE non_dist_role_1 SUPERUSER;
@@ -93,28 +97,51 @@ CREATE ROLE non_dist_role_4;
 
 SET citus.enable_create_role_propagation TO ON;
 
+
+grant dist_role_3,dist_role_1 to test_admin_role with admin option;
+
 SET ROLE dist_role_1;
 
 GRANT non_dist_role_1 TO non_dist_role_2;
 
 SET citus.enable_create_role_propagation TO OFF;
 
+grant dist_role_1 to non_dist_role_1 with admin option;
 SET ROLE non_dist_role_1;
 
-GRANT dist_role_1 TO dist_role_2;
+GRANT dist_role_1 TO dist_role_2 granted by non_dist_role_1;
 
 RESET ROLE;
 
 SET citus.enable_create_role_propagation TO ON;
 
-GRANT dist_role_3 TO non_dist_role_3;
+
+GRANT dist_role_3 TO non_dist_role_3 granted by test_admin_role;
 GRANT non_dist_role_4 TO dist_role_4;
+GRANT dist_role_3 TO dist_role_4 granted by test_admin_role;
+
 
 SELECT 1 FROM master_add_node('localhost', :worker_2_port);
 
-SELECT roleid::regrole::text AS role, member::regrole::text, (grantor::regrole::text IN ('postgres', 'non_dist_role_1', 'dist_role_1')) AS grantor, admin_option FROM pg_auth_members WHERE roleid::regrole::text LIKE '%dist\_%' ORDER BY 1, 2;
+SELECT result FROM run_command_on_all_nodes(
+  $$
+  SELECT json_agg(q.* ORDER BY member) FROM (
+    SELECT member::regrole::text, roleid::regrole::text AS role, grantor::regrole::text, admin_option
+    FROM pg_auth_members WHERE roleid::regrole::text = 'dist_role_3'
+  ) q;
+  $$
+);
+
+REVOKE dist_role_3 from dist_role_4 granted by test_admin_role;
+
+SELECT roleid::regrole::text AS role, member::regrole::text, (grantor::regrole::text IN ('postgres', 'non_dist_role_1', 'dist_role_1','test_admin_role')) AS grantor, admin_option FROM pg_auth_members WHERE roleid::regrole::text LIKE '%dist\_%' ORDER BY 1, 2;
 SELECT objid::regrole FROM pg_catalog.pg_dist_object WHERE classid='pg_authid'::regclass::oid AND objid::regrole::text LIKE '%dist\_%' ORDER BY 1;
 
+REVOKE dist_role_3 from non_dist_role_3 granted by test_admin_role;
+
+revoke dist_role_3,dist_role_1 from test_admin_role cascade;
+drop role test_admin_role;
+
 \c - - - :worker_1_port
 SELECT roleid::regrole::text AS role, member::regrole::text, grantor::regrole::text, admin_option FROM pg_auth_members WHERE roleid::regrole::text LIKE '%dist\_%' ORDER BY 1, 2;
 SELECT rolname FROM pg_authid WHERE rolname LIKE '%dist\_%' ORDER BY 1;