Support SECURITY LABEL ON ROLE from any node (#7508)

DESCRIPTION: Propagates SECURITY LABEL ON ROLE statement from any node
citusdata · Feb 15, 2024 · 15a3ade · 15a3ade
1 parent 59da063
commit 15a3ade
Show file tree

Hide file tree

Showing 4 changed files with 100 additions and 25 deletions.
diff --git a/src/backend/distributed/commands/seclabel.c b/src/backend/distributed/commands/seclabel.c
@@ -29,7 +29,7 @@
 List *
 PostprocessSecLabelStmt(Node *node, const char *queryString)
 {
-	if (!ShouldPropagate())
+	if (!EnableAlterRolePropagation || !ShouldPropagate())
 	{
 		return NIL;
 	}
@@ -59,21 +59,17 @@ PostprocessSecLabelStmt(Node *node, const char *queryString)
 		return NIL;
 	}
 
-	if (!EnableCreateRolePropagation)
-	{
-		return NIL;
-	}
 
-	EnsureCoordinator();
+	EnsurePropagationToCoordinator();
 	EnsureAllObjectDependenciesExistOnAllNodes(objectAddresses);
 
-	const char *sql = DeparseTreeNode((Node *) secLabelStmt);
+	const char *secLabelCommands = DeparseTreeNode((Node *) secLabelStmt);
 
 	List *commandList = list_make3(DISABLE_DDL_PROPAGATION,
-								   (void *) sql,
+								   (void *) secLabelCommands,
 								   ENABLE_DDL_PROPAGATION);
 
-	return NodeDDLTaskList(NON_COORDINATOR_NODES, commandList);
+	return NodeDDLTaskList(REMOTE_NODES, commandList);
 }
 
 

diff --git a/src/backend/distributed/commands/utility_hook.c b/src/backend/distributed/commands/utility_hook.c
@@ -738,6 +738,13 @@ citus_ProcessUtilityInternal(PlannedStmt *pstmt,
 						 errhint("Connect to other nodes directly to manually create all"
 								 " necessary users and roles.")));
 	}
+	else if (IsA(parsetree, SecLabelStmt) && !EnableAlterRolePropagation)
+	{
+		ereport(NOTICE, (errmsg("not propagating SECURITY LABEL commands to other"
+								" nodes"),
+						 errhint("Connect to other nodes directly to manually assign"
+								 " necessary labels.")));
+	}
 
 	/*
 	 * Make sure that on DROP EXTENSION we terminate the background daemon

diff --git a/src/test/regress/expected/seclabel.out b/src/test/regress/expected/seclabel.out
@@ -115,23 +115,47 @@ DETAIL:  on server postgres@localhost:xxxxx connectionId: xxxxxxx
 SECURITY LABEL ON ROLE user1 IS 'citus_unclassified';
 NOTICE:  issuing SECURITY LABEL ON ROLE user1 IS 'citus_unclassified'
 DETAIL:  on server postgres@localhost:xxxxx connectionId: xxxxxxx
-SECURITY LABEL for "citus '!tests_label_provider" ON ROLE "user 2" IS 'citus ''!unclassified';
-NOTICE:  issuing SECURITY LABEL FOR "citus '!tests_label_provider" ON ROLE "user 2" IS 'citus ''!unclassified'
+SECURITY LABEL for "citus '!tests_label_provider" ON ROLE "user 2" IS 'citus_classified';
+NOTICE:  issuing SECURITY LABEL FOR "citus '!tests_label_provider" ON ROLE "user 2" IS 'citus_classified'
 DETAIL:  on server postgres@localhost:xxxxx connectionId: xxxxxxx
 \c - - - :worker_1_port
--- command not allowed from worker node
-SECURITY LABEL for "citus '!tests_label_provider" ON ROLE user1 IS 'citus ''!unclassified';
-ERROR:  operation is not allowed on this node
-HINT:  Connect to the coordinator and run it again.
-\c - - - :master_port
-RESET citus.log_remote_commands;
+SET citus.log_remote_commands TO on;
+SET citus.grep_remote_commands = '%SECURITY LABEL%';
+-- command from the worker node should be propagated to the coordinator
 SELECT node_type, result FROM get_citus_tests_label_provider_labels('user1') ORDER BY node_type;
   node_type  |                                             result
 ---------------------------------------------------------------------
  coordinator | {"label": "citus_unclassified", "objtype": "role", "provider": "citus '!tests_label_provider"}
  worker_1    | {"label": "citus_unclassified", "objtype": "role", "provider": "citus '!tests_label_provider"}
 (2 rows)
 
+SECURITY LABEL for "citus '!tests_label_provider" ON ROLE user1 IS 'citus_classified';
+NOTICE:  issuing SECURITY LABEL FOR "citus '!tests_label_provider" ON ROLE user1 IS 'citus_classified'
+DETAIL:  on server postgres@localhost:xxxxx connectionId: xxxxxxx
+SELECT node_type, result FROM get_citus_tests_label_provider_labels('user1') ORDER BY node_type;
+  node_type  |                                            result
+---------------------------------------------------------------------
+ coordinator | {"label": "citus_classified", "objtype": "role", "provider": "citus '!tests_label_provider"}
+ worker_1    | {"label": "citus_classified", "objtype": "role", "provider": "citus '!tests_label_provider"}
+(2 rows)
+
+RESET citus.log_remote_commands;
+SECURITY LABEL for "citus '!tests_label_provider" ON ROLE "user 2" IS 'citus ''!unclassified';
+SELECT node_type, result FROM get_citus_tests_label_provider_labels('"user 2"') ORDER BY node_type;
+  node_type  |                                              result
+---------------------------------------------------------------------
+ coordinator | {"label": "citus '!unclassified", "objtype": "role", "provider": "citus '!tests_label_provider"}
+ worker_1    | {"label": "citus '!unclassified", "objtype": "role", "provider": "citus '!tests_label_provider"}
+(2 rows)
+
+\c - - - :master_port
+SELECT node_type, result FROM get_citus_tests_label_provider_labels('user1') ORDER BY node_type;
+  node_type  |                                             result
+---------------------------------------------------------------------
+ coordinator | {"label": "citus_classified", "objtype": "role", "provider": "citus '!tests_label_provider"}
+ worker_1    | {"label": "citus_classified", "objtype": "role", "provider": "citus '!tests_label_provider"}
+(2 rows)
+
 SELECT node_type, result FROM get_citus_tests_label_provider_labels('"user 2"') ORDER BY node_type;
   node_type  |                                              result
 ---------------------------------------------------------------------
@@ -143,7 +167,7 @@ SELECT node_type, result FROM get_citus_tests_label_provider_labels('"user 2"')
 SET citus.log_remote_commands TO on;
 SET citus.grep_remote_commands = '%SECURITY LABEL%';
 SELECT 1 FROM citus_add_node('localhost', :worker_2_port);
-NOTICE:  issuing SELECT worker_create_or_alter_role('user1', 'CREATE ROLE user1 NOSUPERUSER NOCREATEDB NOCREATEROLE INHERIT NOLOGIN NOREPLICATION NOBYPASSRLS CONNECTION LIMIT -1 PASSWORD NULL VALID UNTIL ''infinity''', 'ALTER ROLE user1 NOSUPERUSER NOCREATEDB NOCREATEROLE INHERIT NOLOGIN NOREPLICATION NOBYPASSRLS CONNECTION LIMIT -1 PASSWORD NULL VALID UNTIL ''infinity''');SECURITY LABEL FOR "citus '!tests_label_provider" ON ROLE user1 IS 'citus_unclassified'
+NOTICE:  issuing SELECT worker_create_or_alter_role('user1', 'CREATE ROLE user1 NOSUPERUSER NOCREATEDB NOCREATEROLE INHERIT NOLOGIN NOREPLICATION NOBYPASSRLS CONNECTION LIMIT -1 PASSWORD NULL VALID UNTIL ''infinity''', 'ALTER ROLE user1 NOSUPERUSER NOCREATEDB NOCREATEROLE INHERIT NOLOGIN NOREPLICATION NOBYPASSRLS CONNECTION LIMIT -1 PASSWORD NULL VALID UNTIL ''infinity''');SECURITY LABEL FOR "citus '!tests_label_provider" ON ROLE user1 IS 'citus_classified'
 DETAIL:  on server postgres@localhost:xxxxx connectionId: xxxxxxx
 NOTICE:  issuing SELECT worker_create_or_alter_role('user 2', 'CREATE ROLE "user 2" NOSUPERUSER NOCREATEDB NOCREATEROLE INHERIT NOLOGIN NOREPLICATION NOBYPASSRLS CONNECTION LIMIT -1 PASSWORD NULL VALID UNTIL ''infinity''', 'ALTER ROLE "user 2" NOSUPERUSER NOCREATEDB NOCREATEROLE INHERIT NOLOGIN NOREPLICATION NOBYPASSRLS CONNECTION LIMIT -1 PASSWORD NULL VALID UNTIL ''infinity''');SECURITY LABEL FOR "citus '!tests_label_provider" ON ROLE "user 2" IS 'citus ''!unclassified'
 DETAIL:  on server postgres@localhost:xxxxx connectionId: xxxxxxx
@@ -155,9 +179,9 @@ DETAIL:  on server postgres@localhost:xxxxx connectionId: xxxxxxx
 SELECT node_type, result FROM get_citus_tests_label_provider_labels('user1') ORDER BY node_type;
   node_type  |                                             result
 ---------------------------------------------------------------------
- coordinator | {"label": "citus_unclassified", "objtype": "role", "provider": "citus '!tests_label_provider"}
- worker_1    | {"label": "citus_unclassified", "objtype": "role", "provider": "citus '!tests_label_provider"}
- worker_2    | {"label": "citus_unclassified", "objtype": "role", "provider": "citus '!tests_label_provider"}
+ coordinator | {"label": "citus_classified", "objtype": "role", "provider": "citus '!tests_label_provider"}
+ worker_1    | {"label": "citus_classified", "objtype": "role", "provider": "citus '!tests_label_provider"}
+ worker_2    | {"label": "citus_classified", "objtype": "role", "provider": "citus '!tests_label_provider"}
 (3 rows)
 
 SELECT node_type, result FROM get_citus_tests_label_provider_labels('"user 2"') ORDER BY node_type;
@@ -168,6 +192,35 @@ SELECT node_type, result FROM get_citus_tests_label_provider_labels('"user 2"')
  worker_2    | {"label": "citus '!unclassified", "objtype": "role", "provider": "citus '!tests_label_provider"}
 (3 rows)
 
+-- disable the GUC and check that the command is not propagated
+SET citus.enable_alter_role_propagation TO off;
+SECURITY LABEL ON ROLE user1 IS 'citus_unclassified';
+NOTICE:  not propagating SECURITY LABEL commands to other nodes
+HINT:  Connect to other nodes directly to manually assign necessary labels.
+SELECT node_type, result FROM get_citus_tests_label_provider_labels('user1') ORDER BY node_type;
+  node_type  |                                             result
+---------------------------------------------------------------------
+ coordinator | {"label": "citus_unclassified", "objtype": "role", "provider": "citus '!tests_label_provider"}
+ worker_1    | {"label": "citus_classified", "objtype": "role", "provider": "citus '!tests_label_provider"}
+ worker_2    | {"label": "citus_classified", "objtype": "role", "provider": "citus '!tests_label_provider"}
+(3 rows)
+
+\c - - - :worker_2_port
+SET citus.log_remote_commands TO on;
+SET citus.grep_remote_commands = '%SECURITY LABEL%';
+SET citus.enable_alter_role_propagation TO off;
+SECURITY LABEL ON ROLE user1 IS 'citus ''!unclassified';
+NOTICE:  not propagating SECURITY LABEL commands to other nodes
+HINT:  Connect to other nodes directly to manually assign necessary labels.
+SELECT node_type, result FROM get_citus_tests_label_provider_labels('user1') ORDER BY node_type;
+  node_type  |                                              result
+---------------------------------------------------------------------
+ coordinator | {"label": "citus_unclassified", "objtype": "role", "provider": "citus '!tests_label_provider"}
+ worker_1    | {"label": "citus_classified", "objtype": "role", "provider": "citus '!tests_label_provider"}
+ worker_2    | {"label": "citus '!unclassified", "objtype": "role", "provider": "citus '!tests_label_provider"}
+(3 rows)
+
+RESET citus.enable_alter_role_propagation;
 -- cleanup
 RESET citus.log_remote_commands;
 DROP ROLE user1, "user 2";
diff --git a/src/test/regress/sql/seclabel.sql b/src/test/regress/sql/seclabel.sql
@@ -62,14 +62,20 @@ SET citus.grep_remote_commands = '%SECURITY LABEL%';
 SECURITY LABEL for "citus '!tests_label_provider" ON ROLE user1 IS 'citus_classified';
 SECURITY LABEL ON ROLE user1 IS NULL;
 SECURITY LABEL ON ROLE user1 IS 'citus_unclassified';
-SECURITY LABEL for "citus '!tests_label_provider" ON ROLE "user 2" IS 'citus ''!unclassified';
+SECURITY LABEL for "citus '!tests_label_provider" ON ROLE "user 2" IS 'citus_classified';
 
 \c - - - :worker_1_port
--- command not allowed from worker node
-SECURITY LABEL for "citus '!tests_label_provider" ON ROLE user1 IS 'citus ''!unclassified';
+SET citus.log_remote_commands TO on;
+SET citus.grep_remote_commands = '%SECURITY LABEL%';
+-- command from the worker node should be propagated to the coordinator
+SELECT node_type, result FROM get_citus_tests_label_provider_labels('user1') ORDER BY node_type;
+SECURITY LABEL for "citus '!tests_label_provider" ON ROLE user1 IS 'citus_classified';
+SELECT node_type, result FROM get_citus_tests_label_provider_labels('user1') ORDER BY node_type;
 
-\c - - - :master_port
 RESET citus.log_remote_commands;
+SECURITY LABEL for "citus '!tests_label_provider" ON ROLE "user 2" IS 'citus ''!unclassified';
+SELECT node_type, result FROM get_citus_tests_label_provider_labels('"user 2"') ORDER BY node_type;
+\c - - - :master_port
 
 SELECT node_type, result FROM get_citus_tests_label_provider_labels('user1') ORDER BY node_type;
 SELECT node_type, result FROM get_citus_tests_label_provider_labels('"user 2"') ORDER BY node_type;
@@ -82,6 +88,19 @@ SELECT 1 FROM citus_add_node('localhost', :worker_2_port);
 SELECT node_type, result FROM get_citus_tests_label_provider_labels('user1') ORDER BY node_type;
 SELECT node_type, result FROM get_citus_tests_label_provider_labels('"user 2"') ORDER BY node_type;
 
+-- disable the GUC and check that the command is not propagated
+SET citus.enable_alter_role_propagation TO off;
+SECURITY LABEL ON ROLE user1 IS 'citus_unclassified';
+SELECT node_type, result FROM get_citus_tests_label_provider_labels('user1') ORDER BY node_type;
+
+\c - - - :worker_2_port
+SET citus.log_remote_commands TO on;
+SET citus.grep_remote_commands = '%SECURITY LABEL%';
+SET citus.enable_alter_role_propagation TO off;
+SECURITY LABEL ON ROLE user1 IS 'citus ''!unclassified';
+SELECT node_type, result FROM get_citus_tests_label_provider_labels('user1') ORDER BY node_type;
+RESET citus.enable_alter_role_propagation;
+
 -- cleanup
 RESET citus.log_remote_commands;
 DROP ROLE user1, "user 2";