Adds metadata sync support for grant on parameter

src/backend/distributed/commands/parameter.c

@@ -1,10 +1,24 @@
 #include "postgres.h"
 
+
+#include "access/genam.h"
 #include "catalog/namespace.h"
+#include "catalog/pg_parameter_acl.h"
 #include "commands/defrem.h"
 #include "distributed/metadata_sync.h"
 #include "distributed/deparser.h"
 #include "distributed/commands.h"
+#include "distributed/grant_utils.h"
+#include "distributed/listutils.h"
+
+#include "utils/acl.h"
+#include "utils/builtins.h"
+#include "utils/syscache.h"
+
+static List *GenerateGrantOnParameterFromAclItem(char *parameterName, AclItem *aclItem);
+static bool HasAclGrantOption(AclItem *aclItem,AclMode aclMode);
+static void CheckPermissionsAndGrants(AclItem *aclItem, AclMode modes[], int numModes);
+static void CheckAndAppendQuery(List **queries, AclItem *aclItem, Oid granteeOid, char *parameterName, AclMode mode, char *modeStr);
 
 
 List *
@@ -25,3 +39,109 @@ PostprocessGrantParameterStmt(Node *node, const char *queryString)
 
 	return NontransactionalNodeDDLTaskList(REMOTE_NODES, commands);
 }
+
+
+/*
+ * GenerateGrantOnParameterFromAclItem generates a query string for replicating a users permissions
+ * on a database.
+ */
+static List *
+GenerateGrantOnParameterFromAclItem(char *parameterName, AclItem *aclItem)
+{
+	/*
+	 * seems unlikely but we check if there is a grant option in the list without the actual permission
+	 */
+	CheckPermissionsAndGrants(aclItem, (AclMode[]) {ACL_SET, ACL_ALTER_SYSTEM}, 2);
+	Oid granteeOid = aclItem->ai_grantee;
+	List *queries = NIL;
+
+	queries = lappend(queries, GenerateSetRoleQuery(aclItem->ai_grantor));
+
+	CheckAndAppendQuery(&queries, aclItem, granteeOid, parameterName, ACL_SET, "SET");
+	CheckAndAppendQuery(&queries, aclItem, granteeOid, parameterName, ACL_ALTER_SYSTEM, "ALTER SYSTEM");
+
+	queries = lappend(queries, "RESET ROLE");
+
+	return queries;
+}
+
+static void CheckAndAppendQuery(List **queries, AclItem *aclItem, Oid granteeOid, char *parameterName, AclMode mode, char *modeStr) {
+    AclResult aclresult = pg_parameter_aclcheck(parameterName, granteeOid, mode);
+    if (aclresult == ACLCHECK_OK)
+    {
+        char *query = DeparseTreeNode((Node *) GenerateGrantStmtForRightsWithObjectName(
+                                          OBJECT_PARAMETER_ACL, granteeOid, parameterName,
+                                          modeStr,
+                                          HasAclGrantOption(aclItem, mode)));
+
+		// remove the semicolon at the end of the query since it is already
+		// appended in metadata_sync phase
+		query[strlen(query) - 1] = '\0';
+
+        *queries = lappend(*queries, query);
+    }
+}
+
+static void CheckPermissionsAndGrants(AclItem *aclItem, AclMode modes[], int numModes) {
+    AclMode permissions = ACLITEM_GET_PRIVS(*aclItem) & ACL_ALL_RIGHTS_PARAMETER_ACL;
+    AclMode grants = ACLITEM_GET_GOPTIONS(*aclItem) & ACL_ALL_RIGHTS_PARAMETER_ACL;
+
+    for (int i = 0; i < numModes; i++) {
+        AclMode mode = modes[i];
+        Assert(!(grants & mode) || (permissions & mode));
+    }
+}
+
+static bool HasAclGrantOption(AclItem *aclItem,AclMode aclMode)
+{
+	return (aclItem->ai_privs & ACL_GRANT_OPTION_FOR(aclMode)) != 0;
+}
+
+List * GrantOnParameters(void)
+{
+	/* Open pg_shdescription catalog */
+	Relation paramPermissionRelation = table_open(ParameterAclRelationId, AccessShareLock);
+
+
+	int scanKeyCount = 0;
+	bool indexOk = false;
+	SysScanDesc scan = systable_beginscan(paramPermissionRelation, InvalidOid,
+										  indexOk, NULL, scanKeyCount,NULL);
+	HeapTuple tuple;
+	List *commands = NIL;
+	while ((tuple = systable_getnext(scan)) != NULL)
+	{
+
+		bool isNull = false;
+
+		TupleDesc tupdesc = RelationGetDescr(paramPermissionRelation);
+
+		Datum aclDatum = heap_getattr(tuple, Anum_pg_parameter_acl_paracl, tupdesc,
+									   &isNull);
+		Datum parameterNameDatum = heap_getattr(tuple, Anum_pg_parameter_acl_parname, tupdesc,
+									   &isNull);
+
+		char *parameterName = TextDatumGetCString(parameterNameDatum);
+
+		Acl *acl = DatumGetAclPCopy(aclDatum);
+		AclItem *aclDat = ACL_DAT(acl);
+		int aclNum = ACL_NUM(acl);
+
+
+
+		for (int i = 0; i < aclNum; i++)
+		{
+			commands = list_concat(commands,
+								GenerateGrantOnParameterFromAclItem(
+								parameterName, &aclDat[i]));
+		}
+
+
+	}
+
+	/* End the scan and close the catalog */
+	systable_endscan(scan);
+	table_close(paramPermissionRelation, AccessShareLock);
+
+	return commands;
+}

src/backend/distributed/metadata/metadata_sync.c

@@ -88,6 +88,7 @@
 #include "distributed/tenant_schema_metadata.h"
 #include "distributed/utils/array_type.h"
 #include "distributed/utils/function.h"
+#include "distributed/grant_utils.h"
 #include "distributed/version_compat.h"
 #include "distributed/worker_manager.h"
 #include "distributed/worker_protocol.h"
@@ -115,11 +116,6 @@ static bool SyncNodeMetadataSnapshotToNode(WorkerNode *workerNode, bool raiseOnE
 static void DropMetadataSnapshotOnNode(WorkerNode *workerNode);
 static char * CreateSequenceDependencyCommand(Oid relationId, Oid sequenceId,
 											  char *columnName);
-static GrantStmt * GenerateGrantStmtForRights(ObjectType objectType,
-											  Oid roleOid,
-											  Oid objectId,
-											  char *permission,
-											  bool withGrantOption);
 static List * GetObjectsForGrantStmt(ObjectType objectType, Oid objectId);
 static AccessPriv * GetAccessPrivObjectForGrantStmt(char *permission);
 static List * GenerateGrantOnSchemaQueriesFromAclItem(Oid schemaOid,
@@ -130,7 +126,6 @@ static List * GenerateGrantOnFunctionQueriesFromAclItem(Oid schemaOid,
 static List * GrantOnSequenceDDLCommands(Oid sequenceOid);
 static List * GenerateGrantOnSequenceQueriesFromAclItem(Oid sequenceOid,
 														AclItem *aclItem);
-static char * GenerateSetRoleQuery(Oid roleOid);
 static void MetadataSyncSigTermHandler(SIGNAL_ARGS);
 static void MetadataSyncSigAlrmHandler(SIGNAL_ARGS);
 
@@ -2159,18 +2154,51 @@ GenerateGrantOnDatabaseFromAclItem(Oid databaseOid, AclItem *aclItem)
  * The field `objects` of GrantStmt doesn't have a common structure for all types.
  * Make sure you have added your object type to GetObjectsForGrantStmt.
  */
-static GrantStmt *
+GrantStmt *
 GenerateGrantStmtForRights(ObjectType objectType,
 						   Oid roleOid,
 						   Oid objectId,
 						   char *permission,
 						   bool withGrantOption)
 {
+	return BaseGenerateGrantStmtForRights(objectType,roleOid,objectId,NULL,permission,withGrantOption);
+}
+
+GrantStmt *
+GenerateGrantStmtForRightsWithObjectName(ObjectType objectType,
+						   Oid roleOid,
+						   char *objectName,
+						   char *permission,
+						   bool withGrantOption)
+{
+	return BaseGenerateGrantStmtForRights(objectType,roleOid,InvalidOid,objectName,permission,withGrantOption);
+}
+
+
+GrantStmt *
+BaseGenerateGrantStmtForRights(ObjectType objectType,
+						   Oid roleOid,
+						   Oid objectId,
+						   char *objectName,
+						   char *permission,
+						   bool withGrantOption)
+{
+
+	//either objectId or objectName should be valid
+	Assert(objectId != InvalidOid || objectName != NULL);
+
 	GrantStmt *stmt = makeNode(GrantStmt);
 	stmt->is_grant = true;
 	stmt->targtype = ACL_TARGET_OBJECT;
 	stmt->objtype = objectType;
-	stmt->objects = GetObjectsForGrantStmt(objectType, objectId);
+	if (objectId != InvalidOid)
+	{
+		stmt->objects = GetObjectsForGrantStmt(objectType, objectId);
+	}
+	else
+	{
+		stmt->objects = list_make1(makeString(objectName));
+	}
 	stmt->privileges = list_make1(GetAccessPrivObjectForGrantStmt(permission));
 	stmt->grantees = list_make1(GetRoleSpecObjectForUser(roleOid));
 	stmt->grant_option = withGrantOption;
@@ -2179,6 +2207,7 @@ GenerateGrantStmtForRights(ObjectType objectType,
 }
 
 
+
 /*
  * GetObjectsForGrantStmt takes an object type and object id and returns the 'objects'
  * field to be used when creating GrantStmt. We have only one object here (the one with
@@ -2230,6 +2259,7 @@ GetObjectsForGrantStmt(ObjectType objectType, Oid objectId)
 			return list_make1(makeString(get_database_name(objectId)));
 		}
 
+
 		default:
 		{
 			elog(ERROR, "unsupported object type for GRANT");
@@ -2563,7 +2593,7 @@ SetLocalEnableMetadataSync(bool state)
 }
 
 
-static char *
+char *
 GenerateSetRoleQuery(Oid roleOid)
 {
 	StringInfo buf = makeStringInfo();
@@ -4682,6 +4712,8 @@ PropagateNodeWideObjectsCommandList(void)
 		List *alterRoleSetCommands = GenerateAlterRoleSetCommandForRole(InvalidOid);
 		ddlCommands = list_concat(ddlCommands, alterRoleSetCommands);
 	}
+	List *grantOnParameterCommands = GrantOnParameters();
+	ddlCommands = list_concat(ddlCommands, grantOnParameterCommands);
 
 	return ddlCommands;
 }

src/include/distributed/grant_utils.h

@@ -0,0 +1,34 @@
+/*-------------------------------------------------------------------------
+ *
+ * grant_utils.h
+ *
+ * Routines for grant operations.
+ *
+ *-------------------------------------------------------------------------
+ */
+#ifndef CITUS_GRANT_UTILS_H
+#define CITUS_GRANT_UTILS_H
+#include "postgres.h"
+#include "nodes/parsenodes.h"
+
+extern List * GrantOnParameters(void);
+extern char * GenerateSetRoleQuery(Oid roleOid);
+extern GrantStmt * GenerateGrantStmtForRights(ObjectType objectType,
+						   Oid roleOid,
+						   Oid objectId,
+						   char *permission,
+						   bool withGrantOption);
+extern GrantStmt *GenerateGrantStmtForRightsWithObjectName(ObjectType objectType,
+						   Oid roleOid,
+						   char *objectName,
+						   char *permission,
+						   bool withGrantOption);
+extern GrantStmt *BaseGenerateGrantStmtForRights(ObjectType objectType,
+						   Oid roleOid,
+						   Oid objectId,
+						   char *objectName,
+						   char *permission,
+						   bool withGrantOption);
+
+
+#endif   /* CITUS_GRANT_UTILS_H */

src/test/regress/expected/grant_on_parameter_propagation.out

@@ -169,13 +169,75 @@ SELECT check_parameter_privileges(ARRAY['grant_param_user4'],ARRAY['max_connecti
  (f,grant_param_user4,shared_buffers,"ALTER SYSTEM")
 (6 rows)
 
+--test metadata_sync
+SELECT 1 FROM citus_remove_node('localhost', :worker_2_port);
+ ?column?
+---------------------------------------------------------------------
+        1
+(1 row)
+
+GRANT SET,ALTER SYSTEM ON PARAMETER max_connections,shared_buffers TO grant_param_user3,"grant_param_user5-\!" WITH GRANT OPTION GRANTED BY CURRENT_USER;
+SELECT check_parameter_privileges(ARRAY['grant_param_user3','grant_param_user5-\!'],ARRAY['max_connections','shared_buffers'], ARRAY['SET','ALTER SYSTEM']);
+                 check_parameter_privileges
+---------------------------------------------------------------------
+ (t,grant_param_user3,max_connections,SET)
+ (t,grant_param_user3,max_connections,SET)
+ (t,grant_param_user3,max_connections,"ALTER SYSTEM")
+ (t,grant_param_user3,max_connections,"ALTER SYSTEM")
+ (t,grant_param_user3,shared_buffers,SET)
+ (t,grant_param_user3,shared_buffers,SET)
+ (t,grant_param_user3,shared_buffers,"ALTER SYSTEM")
+ (t,grant_param_user3,shared_buffers,"ALTER SYSTEM")
+ (t,"grant_param_user5-\\!",max_connections,SET)
+ (t,"grant_param_user5-\\!",max_connections,SET)
+ (t,"grant_param_user5-\\!",max_connections,"ALTER SYSTEM")
+ (t,"grant_param_user5-\\!",max_connections,"ALTER SYSTEM")
+ (t,"grant_param_user5-\\!",shared_buffers,SET)
+ (t,"grant_param_user5-\\!",shared_buffers,SET)
+ (t,"grant_param_user5-\\!",shared_buffers,"ALTER SYSTEM")
+ (t,"grant_param_user5-\\!",shared_buffers,"ALTER SYSTEM")
+(16 rows)
+
+SELECT 1 FROM citus_add_node('localhost', :worker_2_port);
+ ?column?
+---------------------------------------------------------------------
+        1
+(1 row)
+
+SELECT check_parameter_privileges(ARRAY['grant_param_user3','grant_param_user5-\!'],ARRAY['max_connections','shared_buffers'], ARRAY['SET','ALTER SYSTEM']);
+                 check_parameter_privileges
+---------------------------------------------------------------------
+ (t,grant_param_user3,max_connections,SET)
+ (t,grant_param_user3,max_connections,SET)
+ (t,grant_param_user3,max_connections,SET)
+ (t,grant_param_user3,max_connections,"ALTER SYSTEM")
+ (t,grant_param_user3,max_connections,"ALTER SYSTEM")
+ (t,grant_param_user3,max_connections,"ALTER SYSTEM")
+ (t,grant_param_user3,shared_buffers,SET)
+ (t,grant_param_user3,shared_buffers,SET)
+ (t,grant_param_user3,shared_buffers,SET)
+ (t,grant_param_user3,shared_buffers,"ALTER SYSTEM")
+ (t,grant_param_user3,shared_buffers,"ALTER SYSTEM")
+ (t,grant_param_user3,shared_buffers,"ALTER SYSTEM")
+ (t,"grant_param_user5-\\!",max_connections,SET)
+ (t,"grant_param_user5-\\!",max_connections,SET)
+ (t,"grant_param_user5-\\!",max_connections,SET)
+ (t,"grant_param_user5-\\!",max_connections,"ALTER SYSTEM")
+ (t,"grant_param_user5-\\!",max_connections,"ALTER SYSTEM")
+ (t,"grant_param_user5-\\!",max_connections,"ALTER SYSTEM")
+ (t,"grant_param_user5-\\!",shared_buffers,SET)
+ (t,"grant_param_user5-\\!",shared_buffers,SET)
+ (t,"grant_param_user5-\\!",shared_buffers,SET)
+ (t,"grant_param_user5-\\!",shared_buffers,"ALTER SYSTEM")
+ (t,"grant_param_user5-\\!",shared_buffers,"ALTER SYSTEM")
+ (t,"grant_param_user5-\\!",shared_buffers,"ALTER SYSTEM")
+(24 rows)
+
+REVOKE SET,ALTER SYSTEM ON PARAMETER max_connections,shared_buffers FROM grant_param_user3,"grant_param_user5-\!" cascade;
 --clean all resources
 DROP USER grant_param_user1;
 DROP USER grant_param_user2;
 DROP USER grant_param_user3;
-ERROR:  role "grant_param_user3" cannot be dropped because some objects depend on it
-DETAIL:  privileges for parameter max_connections
-privileges for parameter shared_buffers
 DROP USER grant_param_user4;
 DROP USER "grant_param_user5-\!";
 reset  citus.log_remote_commands;

src/test/regress/multi_1_schedule

@@ -38,6 +38,7 @@ test: create_single_shard_table
 test: create_drop_database_propagation
 test: create_drop_database_propagation_pg15
 test: create_drop_database_propagation_pg16
+test: grant_on_parameter_propagation
 # don't parallelize single_shard_table_udfs to make sure colocation ids are sequential
 test: single_shard_table_udfs
 test: schema_based_sharding
@@ -60,7 +61,6 @@ test: alter_database_propagation
 
 test: citus_shards
 test: reassign_owned
-test: grant_on_parameter_propagation
 
 # ----------
 # multi_citus_tools tests utility functions written for citus tools

src/test/regress/sql/grant_on_parameter_propagation.sql

@@ -47,6 +47,20 @@ GRANT ALTER SYSTEM ON PARAMETER max_connections,shared_buffers TO grant_param_us
 
 SELECT check_parameter_privileges(ARRAY['grant_param_user4'],ARRAY['max_connections','shared_buffers'], ARRAY['ALTER SYSTEM']);
 
+--test metadata_sync
+
+SELECT 1 FROM citus_remove_node('localhost', :worker_2_port);
+GRANT SET,ALTER SYSTEM ON PARAMETER max_connections,shared_buffers TO grant_param_user3,"grant_param_user5-\!" WITH GRANT OPTION GRANTED BY CURRENT_USER;
+
+SELECT check_parameter_privileges(ARRAY['grant_param_user3','grant_param_user5-\!'],ARRAY['max_connections','shared_buffers'], ARRAY['SET','ALTER SYSTEM']);
+
+SELECT 1 FROM citus_add_node('localhost', :worker_2_port);
+
+SELECT check_parameter_privileges(ARRAY['grant_param_user3','grant_param_user5-\!'],ARRAY['max_connections','shared_buffers'], ARRAY['SET','ALTER SYSTEM']);
+
+REVOKE SET,ALTER SYSTEM ON PARAMETER max_connections,shared_buffers FROM grant_param_user3,"grant_param_user5-\!" cascade;
+
+
 --clean all resources
 DROP USER grant_param_user1;
 DROP USER grant_param_user2;