diff --git a/.gitignore b/.gitignore index df447746a..e636392ac 100644 --- a/.gitignore +++ b/.gitignore @@ -55,3 +55,6 @@ lib*.pc # style related temporary outputs *.uncrustify .venv + +# added output when modifying check_gucs_are_alphabetically_sorted.sh +guc.out diff --git a/CHANGELOG.md b/CHANGELOG.md index 686e78dd1..8d979c104 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,10 +1,10 @@ ### citus v12.1.1 (November 9, 2023) ### * Fixes leaking of memory and memory contexts in Citus foreign key cache - (#7219) + (#7236) * Makes sure to disallow creating a replicated distributed table concurrently - (#7236) + (#7219) ### citus v12.1.0 (September 12, 2023) ### diff --git a/ci/check_gucs_are_alphabetically_sorted.sh b/ci/check_gucs_are_alphabetically_sorted.sh index a769ae4fb..763b5305f 100755 --- a/ci/check_gucs_are_alphabetically_sorted.sh +++ b/ci/check_gucs_are_alphabetically_sorted.sh @@ -5,6 +5,6 @@ set -euo pipefail source ci/ci_helpers.sh # extract citus gucs in the form of "citus.X" -grep -o -E "(\.*\"citus.\w+\")," src/backend/distributed/shared_library_init.c > gucs.out +grep -o -E "(\.*\"citus\.\w+\")," src/backend/distributed/shared_library_init.c > gucs.out sort -c gucs.out rm gucs.out diff --git a/src/backend/distributed/commands/distribute_object_ops.c b/src/backend/distributed/commands/distribute_object_ops.c index 4c27ba086..06390f65d 100644 --- a/src/backend/distributed/commands/distribute_object_ops.c +++ b/src/backend/distributed/commands/distribute_object_ops.c @@ -374,6 +374,15 @@ static DistributeObjectOps Any_Rename = { .address = NULL, .markDistributed = false, }; +static DistributeObjectOps Any_SecLabel = { + .deparse = DeparseSecLabelStmt, + .qualify = NULL, + .preprocess = NULL, + .postprocess = PostprocessSecLabelStmt, + .operationType = DIST_OPS_ALTER, + .address = SecLabelStmtObjectAddress, + .markDistributed = false, +}; static DistributeObjectOps Attribute_Rename = { .deparse = DeparseRenameAttributeStmt, .qualify = QualifyRenameAttributeStmt, @@ -2052,6 +2061,11 @@ GetDistributeObjectOps(Node *node) return &Vacuum_Analyze; } + case T_SecLabelStmt: + { + return &Any_SecLabel; + } + case T_RenameStmt: { RenameStmt *stmt = castNode(RenameStmt, node); diff --git a/src/backend/distributed/commands/role.c b/src/backend/distributed/commands/role.c index f3ac7b4ff..407659aec 100644 --- a/src/backend/distributed/commands/role.c +++ b/src/backend/distributed/commands/role.c @@ -23,6 +23,7 @@ #include "catalog/pg_auth_members.h" #include "catalog/pg_authid.h" #include "catalog/pg_db_role_setting.h" +#include "catalog/pg_shseclabel.h" #include "catalog/pg_type.h" #include "catalog/objectaddress.h" #include "commands/dbcommands.h" @@ -65,6 +66,7 @@ static DefElem * makeDefElemBool(char *name, bool value); static List * GenerateRoleOptionsList(HeapTuple tuple); static List * GenerateGrantRoleStmtsFromOptions(RoleSpec *roleSpec, List *options); static List * GenerateGrantRoleStmtsOfRole(Oid roleid); +static List * GenerateSecLabelOnRoleStmts(Oid roleid, char *rolename); static void EnsureSequentialModeForRoleDDL(void); static char * GetRoleNameFromDbRoleSetting(HeapTuple tuple, @@ -515,13 +517,14 @@ GenerateCreateOrAlterRoleCommand(Oid roleOid) { HeapTuple roleTuple = SearchSysCache1(AUTHOID, ObjectIdGetDatum(roleOid)); Form_pg_authid role = ((Form_pg_authid) GETSTRUCT(roleTuple)); + char *rolename = pstrdup(NameStr(role->rolname)); CreateRoleStmt *createRoleStmt = NULL; if (EnableCreateRolePropagation) { createRoleStmt = makeNode(CreateRoleStmt); createRoleStmt->stmt_type = ROLESTMT_ROLE; - createRoleStmt->role = pstrdup(NameStr(role->rolname)); + createRoleStmt->role = rolename; createRoleStmt->options = GenerateRoleOptionsList(roleTuple); } @@ -532,7 +535,7 @@ GenerateCreateOrAlterRoleCommand(Oid roleOid) alterRoleStmt->role = makeNode(RoleSpec); alterRoleStmt->role->roletype = ROLESPEC_CSTRING; alterRoleStmt->role->location = -1; - alterRoleStmt->role->rolename = pstrdup(NameStr(role->rolname)); + alterRoleStmt->role->rolename = rolename; alterRoleStmt->action = 1; alterRoleStmt->options = GenerateRoleOptionsList(roleTuple); } @@ -544,7 +547,7 @@ GenerateCreateOrAlterRoleCommand(Oid roleOid) { /* add a worker_create_or_alter_role command if any of them are set */ char *createOrAlterRoleQuery = CreateCreateOrAlterRoleCommand( - pstrdup(NameStr(role->rolname)), + rolename, createRoleStmt, alterRoleStmt); @@ -566,6 +569,20 @@ GenerateCreateOrAlterRoleCommand(Oid roleOid) { completeRoleList = lappend(completeRoleList, DeparseTreeNode(stmt)); } + + /* + * append SECURITY LABEL ON ROLE commands for this specific user + * When we propagate user creation, we also want to make sure that we propagate + * all the security labels it has been given. For this, we check pg_shseclabel + * for the ROLE entry corresponding to roleOid, and generate the relevant + * SecLabel stmts to be run in the new node. + */ + List *secLabelOnRoleStmts = GenerateSecLabelOnRoleStmts(roleOid, rolename); + stmt = NULL; + foreach_ptr(stmt, secLabelOnRoleStmts) + { + completeRoleList = lappend(completeRoleList, DeparseTreeNode(stmt)); + } } return completeRoleList; @@ -895,6 +912,54 @@ GenerateGrantRoleStmtsOfRole(Oid roleid) } +/* + * GenerateSecLabelOnRoleStmts generates the SecLabelStmts for the role + * whose oid is roleid. + */ +static List * +GenerateSecLabelOnRoleStmts(Oid roleid, char *rolename) +{ + List *secLabelStmts = NIL; + + /* + * Note that roles are shared database objects, therefore their + * security labels are stored in pg_shseclabel instead of pg_seclabel. + */ + Relation pg_shseclabel = table_open(SharedSecLabelRelationId, AccessShareLock); + ScanKeyData skey[1]; + ScanKeyInit(&skey[0], Anum_pg_shseclabel_objoid, BTEqualStrategyNumber, F_OIDEQ, + ObjectIdGetDatum(roleid)); + SysScanDesc scan = systable_beginscan(pg_shseclabel, SharedSecLabelObjectIndexId, + true, NULL, 1, &skey[0]); + + HeapTuple tuple = NULL; + while (HeapTupleIsValid(tuple = systable_getnext(scan))) + { + SecLabelStmt *secLabelStmt = makeNode(SecLabelStmt); + secLabelStmt->objtype = OBJECT_ROLE; + secLabelStmt->object = (Node *) makeString(pstrdup(rolename)); + + Datum datumArray[Natts_pg_shseclabel]; + bool isNullArray[Natts_pg_shseclabel]; + + heap_deform_tuple(tuple, RelationGetDescr(pg_shseclabel), datumArray, + isNullArray); + + secLabelStmt->provider = TextDatumGetCString( + datumArray[Anum_pg_shseclabel_provider - 1]); + secLabelStmt->label = TextDatumGetCString( + datumArray[Anum_pg_shseclabel_label - 1]); + + secLabelStmts = lappend(secLabelStmts, secLabelStmt); + } + + systable_endscan(scan); + table_close(pg_shseclabel, AccessShareLock); + + return secLabelStmts; +} + + /* * PreprocessCreateRoleStmt creates a worker_create_or_alter_role query for the * role that is being created. With that query we can create the role in the diff --git a/src/backend/distributed/commands/seclabel.c b/src/backend/distributed/commands/seclabel.c new file mode 100644 index 000000000..208d186c7 --- /dev/null +++ b/src/backend/distributed/commands/seclabel.c @@ -0,0 +1,125 @@ +/*------------------------------------------------------------------------- + * + * seclabel.c + * + * This file contains the logic of SECURITY LABEL statement propagation. + * + * Copyright (c) Citus Data, Inc. + * + *------------------------------------------------------------------------- + */ + +#include "postgres.h" + +#include "distributed/commands.h" +#include "distributed/commands/utility_hook.h" +#include "distributed/coordinator_protocol.h" +#include "distributed/deparser.h" +#include "distributed/log_utils.h" +#include "distributed/metadata_sync.h" +#include "distributed/metadata/distobject.h" + + +/* + * PostprocessSecLabelStmt prepares the commands that need to be run on all workers to assign + * security labels on distributed objects, currently supporting just Role objects. + * It also ensures that all object dependencies exist on all + * nodes for the object in the SecLabelStmt. + */ +List * +PostprocessSecLabelStmt(Node *node, const char *queryString) +{ + if (!ShouldPropagate()) + { + return NIL; + } + + SecLabelStmt *secLabelStmt = castNode(SecLabelStmt, node); + + List *objectAddresses = GetObjectAddressListFromParseTree(node, false, true); + if (!IsAnyObjectDistributed(objectAddresses)) + { + return NIL; + } + + if (secLabelStmt->objtype != OBJECT_ROLE) + { + /* + * If we are not in the coordinator, we don't want to interrupt the security + * label command with notices, the user expects that from the worker node + * the command will not be propagated + */ + if (EnableUnsupportedFeatureMessages && IsCoordinator()) + { + ereport(NOTICE, (errmsg("not propagating SECURITY LABEL commands whose " + "object type is not role"), + errhint("Connect to worker nodes directly to manually " + "run the same SECURITY LABEL command."))); + } + return NIL; + } + + if (!EnableCreateRolePropagation) + { + return NIL; + } + + EnsureCoordinator(); + EnsureAllObjectDependenciesExistOnAllNodes(objectAddresses); + + const char *sql = DeparseTreeNode((Node *) secLabelStmt); + + List *commandList = list_make3(DISABLE_DDL_PROPAGATION, + (void *) sql, + ENABLE_DDL_PROPAGATION); + + return NodeDDLTaskList(NON_COORDINATOR_NODES, commandList); +} + + +/* + * SecLabelStmtObjectAddress returns the object address of the object on + * which this statement operates (secLabelStmt->object). Note that it has no limitation + * on the object type being OBJECT_ROLE. This is intentionally implemented like this + * since it is fairly simple to implement and we might extend SECURITY LABEL propagation + * in the future to include more object types. + */ +List * +SecLabelStmtObjectAddress(Node *node, bool missing_ok, bool isPostprocess) +{ + SecLabelStmt *secLabelStmt = castNode(SecLabelStmt, node); + + Relation rel = NULL; + ObjectAddress address = get_object_address(secLabelStmt->objtype, + secLabelStmt->object, &rel, + AccessShareLock, missing_ok); + if (rel != NULL) + { + relation_close(rel, AccessShareLock); + } + + ObjectAddress *addressPtr = palloc0(sizeof(ObjectAddress)); + *addressPtr = address; + return list_make1(addressPtr); +} + + +/* + * citus_test_object_relabel is a dummy function for check_object_relabel_type hook. + * It is meant to be used in tests combined with citus_test_register_label_provider + */ +void +citus_test_object_relabel(const ObjectAddress *object, const char *seclabel) +{ + if (seclabel == NULL || + strcmp(seclabel, "citus_unclassified") == 0 || + strcmp(seclabel, "citus_classified") == 0 || + strcmp(seclabel, "citus '!unclassified") == 0) + { + return; + } + + ereport(ERROR, + (errcode(ERRCODE_INVALID_NAME), + errmsg("'%s' is not a valid security label for Citus tests.", seclabel))); +} diff --git a/src/backend/distributed/deparser/deparse_seclabel_stmts.c b/src/backend/distributed/deparser/deparse_seclabel_stmts.c new file mode 100644 index 000000000..a1aa047cc --- /dev/null +++ b/src/backend/distributed/deparser/deparse_seclabel_stmts.c @@ -0,0 +1,78 @@ +/*------------------------------------------------------------------------- + * + * deparse_seclabel_stmts.c + * All routines to deparse SECURITY LABEL statements. + * + * Copyright (c), Citus Data, Inc. + * + *------------------------------------------------------------------------- + */ + +#include "postgres.h" + +#include "distributed/deparser.h" +#include "nodes/parsenodes.h" +#include "utils/builtins.h" + +static void AppendSecLabelStmt(StringInfo buf, SecLabelStmt *stmt); + +/* + * DeparseSecLabelStmt builds and returns a string representing of the + * SecLabelStmt for application on a remote server. + */ +char * +DeparseSecLabelStmt(Node *node) +{ + SecLabelStmt *secLabelStmt = castNode(SecLabelStmt, node); + StringInfoData buf = { 0 }; + initStringInfo(&buf); + + AppendSecLabelStmt(&buf, secLabelStmt); + + return buf.data; +} + + +/* + * AppendSecLabelStmt generates the string representation of the + * SecLabelStmt and appends it to the buffer. + */ +static void +AppendSecLabelStmt(StringInfo buf, SecLabelStmt *stmt) +{ + appendStringInfoString(buf, "SECURITY LABEL "); + + if (stmt->provider != NULL) + { + appendStringInfo(buf, "FOR %s ", quote_identifier(stmt->provider)); + } + + appendStringInfoString(buf, "ON "); + + switch (stmt->objtype) + { + case OBJECT_ROLE: + { + appendStringInfo(buf, "ROLE %s ", quote_identifier(strVal(stmt->object))); + break; + } + + /* normally, we shouldn't reach this */ + default: + { + ereport(ERROR, (errmsg("unsupported security label statement for" + " deparsing"))); + } + } + + appendStringInfoString(buf, "IS "); + + if (stmt->label != NULL) + { + appendStringInfo(buf, "%s", quote_literal_cstr(stmt->label)); + } + else + { + appendStringInfoString(buf, "NULL"); + } +} diff --git a/src/backend/distributed/operations/shard_rebalancer.c b/src/backend/distributed/operations/shard_rebalancer.c index d339ac56a..213d135e6 100644 --- a/src/backend/distributed/operations/shard_rebalancer.c +++ b/src/backend/distributed/operations/shard_rebalancer.c @@ -317,7 +317,7 @@ PG_FUNCTION_INFO_V1(citus_rebalance_start); PG_FUNCTION_INFO_V1(citus_rebalance_stop); PG_FUNCTION_INFO_V1(citus_rebalance_wait); -bool RunningUnderIsolationTest = false; +bool RunningUnderCitusTestSuite = false; int MaxRebalancerLoggedIgnoredMoves = 5; int RebalancerByDiskSizeBaseCost = 100 * 1024 * 1024; bool PropagateSessionSettingsForLoopbackConnection = false; diff --git a/src/backend/distributed/replication/multi_logical_replication.c b/src/backend/distributed/replication/multi_logical_replication.c index 97f6fdb3d..48571d7c4 100644 --- a/src/backend/distributed/replication/multi_logical_replication.c +++ b/src/backend/distributed/replication/multi_logical_replication.c @@ -1143,7 +1143,7 @@ ConflictWithIsolationTestingBeforeCopy(void) const bool sessionLock = false; const bool dontWait = false; - if (RunningUnderIsolationTest) + if (RunningUnderCitusTestSuite) { SET_LOCKTAG_ADVISORY(tag, MyDatabaseId, SHARD_MOVE_ADVISORY_LOCK_SECOND_KEY, @@ -1177,7 +1177,7 @@ ConflictWithIsolationTestingAfterCopy(void) const bool sessionLock = false; const bool dontWait = false; - if (RunningUnderIsolationTest) + if (RunningUnderCitusTestSuite) { SET_LOCKTAG_ADVISORY(tag, MyDatabaseId, SHARD_MOVE_ADVISORY_LOCK_FIRST_KEY, diff --git a/src/backend/distributed/shared_library_init.c b/src/backend/distributed/shared_library_init.c index f06e0f2b0..dccc30e15 100644 --- a/src/backend/distributed/shared_library_init.c +++ b/src/backend/distributed/shared_library_init.c @@ -29,6 +29,7 @@ #include "citus_version.h" #include "commands/explain.h" #include "commands/extension.h" +#include "commands/seclabel.h" #include "common/string.h" #include "executor/executor.h" #include "distributed/backend_data.h" @@ -574,6 +575,16 @@ _PG_init(void) INIT_COLUMNAR_SYMBOL(PGFunction, columnar_storage_info); INIT_COLUMNAR_SYMBOL(PGFunction, columnar_store_memory_stats); INIT_COLUMNAR_SYMBOL(PGFunction, test_columnar_storage_write_new_page); + + /* + * This part is only for SECURITY LABEL tests + * mimicking what an actual security label provider would do + */ + if (RunningUnderCitusTestSuite) + { + register_label_provider("citus '!tests_label_provider", + citus_test_object_relabel); + } } @@ -2316,13 +2327,14 @@ RegisterCitusConfigVariables(void) WarnIfReplicationModelIsSet, NULL, NULL); DefineCustomBoolVariable( - "citus.running_under_isolation_test", + "citus.running_under_citus_test_suite", gettext_noop( "Only useful for testing purposes, when set to true, Citus does some " - "tricks to implement useful isolation tests with rebalancing. Should " + "tricks to implement useful isolation tests with rebalancing. It also " + "registers a dummy label provider for SECURITY LABEL tests. Should " "never be set to true on production systems "), gettext_noop("for details of the tricks implemented, refer to the source code"), - &RunningUnderIsolationTest, + &RunningUnderCitusTestSuite, false, PGC_SUSET, GUC_SUPERUSER_ONLY | GUC_NO_SHOW_ALL | GUC_NOT_IN_SAMPLE, diff --git a/src/include/distributed/commands.h b/src/include/distributed/commands.h index aaee98d1c..4391015cb 100644 --- a/src/include/distributed/commands.h +++ b/src/include/distributed/commands.h @@ -533,6 +533,11 @@ extern List * AlterSchemaOwnerStmtObjectAddress(Node *node, bool missing_ok, extern List * AlterSchemaRenameStmtObjectAddress(Node *node, bool missing_ok, bool isPostprocess); +/* seclabel.c - forward declarations*/ +extern List * PostprocessSecLabelStmt(Node *node, const char *queryString); +extern List * SecLabelStmtObjectAddress(Node *node, bool missing_ok, bool isPostprocess); +extern void citus_test_object_relabel(const ObjectAddress *object, const char *seclabel); + /* sequence.c - forward declarations */ extern List * PreprocessAlterSequenceStmt(Node *node, const char *queryString, ProcessUtilityContext processUtilityContext); diff --git a/src/include/distributed/deparser.h b/src/include/distributed/deparser.h index 5159f4c9d..f4bd6864e 100644 --- a/src/include/distributed/deparser.h +++ b/src/include/distributed/deparser.h @@ -285,6 +285,9 @@ extern void QualifyRenameTextSearchDictionaryStmt(Node *node); extern void QualifyTextSearchConfigurationCommentStmt(Node *node); extern void QualifyTextSearchDictionaryCommentStmt(Node *node); +/* forward declarations for deparse_seclabel_stmts.c */ +extern char * DeparseSecLabelStmt(Node *node); + /* forward declarations for deparse_sequence_stmts.c */ extern char * DeparseDropSequenceStmt(Node *node); extern char * DeparseRenameSequenceStmt(Node *node); diff --git a/src/include/distributed/shard_rebalancer.h b/src/include/distributed/shard_rebalancer.h index 38ce4f485..345748ced 100644 --- a/src/include/distributed/shard_rebalancer.h +++ b/src/include/distributed/shard_rebalancer.h @@ -189,7 +189,7 @@ typedef struct RebalancePlanFunctions extern char *VariablesToBePassedToNewConnections; extern int MaxRebalancerLoggedIgnoredMoves; extern int RebalancerByDiskSizeBaseCost; -extern bool RunningUnderIsolationTest; +extern bool RunningUnderCitusTestSuite; extern bool PropagateSessionSettingsForLoopbackConnection; extern int MaxBackgroundTaskExecutorsPerNode; diff --git a/src/test/regress/expected/multi_test_helpers.out b/src/test/regress/expected/multi_test_helpers.out index 1d077bb11..4b621b968 100644 --- a/src/test/regress/expected/multi_test_helpers.out +++ b/src/test/regress/expected/multi_test_helpers.out @@ -526,73 +526,33 @@ BEGIN RETURN result; END; $func$ LANGUAGE plpgsql; --- For all nodes, returns database properties of given database, except --- oid, datfrozenxid and datminmxid. --- --- Also returns whether the node has a pg_dist_object record for the database --- and whether there are any stale pg_dist_object records for a database. -CREATE OR REPLACE FUNCTION check_database_on_all_nodes(p_database_name text) -RETURNS TABLE (node_type text, result text) +-- Returns pg_seclabels entries from all nodes in the cluster for which +-- the object name is the input. +CREATE OR REPLACE FUNCTION get_citus_tests_label_provider_labels(object_name text, + master_port INTEGER DEFAULT 57636, + worker_1_port INTEGER DEFAULT 57637, + worker_2_port INTEGER DEFAULT 57638) +RETURNS TABLE ( + node_type text, + result text +) AS $func$ DECLARE - pg_ge_15_options text := ''; - pg_ge_16_options text := ''; + pg_seclabels_cmd TEXT := 'SELECT to_jsonb(q.*) FROM (' || + 'SELECT provider, objtype, label FROM pg_seclabels ' || + 'WHERE objname = ''' || object_name || ''') q'; BEGIN - IF EXISTS (SELECT 1 FROM pg_attribute WHERE attrelid = 'pg_database'::regclass AND attname = 'datlocprovider') THEN - pg_ge_15_options := ', daticulocale, datcollversion, datlocprovider'; - ELSE - pg_ge_15_options := $$, null as daticulocale, null as datcollversion, 'c' as datlocprovider$$; - END IF; - - IF EXISTS (SELECT 1 FROM pg_attribute WHERE attrelid = 'pg_database'::regclass AND attname = 'daticurules') THEN - pg_ge_16_options := ', daticurules'; - ELSE - pg_ge_16_options := ', null as daticurules'; - END IF; - - RETURN QUERY + RETURN QUERY SELECT - CASE WHEN (groupid = 0 AND groupid = (SELECT groupid FROM pg_dist_local_group)) THEN 'coordinator (local)' - WHEN (groupid = 0) THEN 'coordinator (remote)' - WHEN (groupid = (SELECT groupid FROM pg_dist_local_group)) THEN 'worker node (local)' - ELSE 'worker node (remote)' - END AS node_type, - q2.result - FROM run_command_on_all_nodes( - format( - $$ - SELECT to_jsonb(q.*) - FROM ( - SELECT - ( - SELECT to_jsonb(database_properties.*) - FROM ( - SELECT datname, pa.rolname as database_owner, - pg_encoding_to_char(pd.encoding) as encoding, - datistemplate, datallowconn, datconnlimit, datacl, - pt.spcname AS tablespace, datcollate, datctype - %2$s -- >= pg15 options - %3$s -- >= pg16 options - FROM pg_database pd - JOIN pg_authid pa ON pd.datdba = pa.oid - JOIN pg_tablespace pt ON pd.dattablespace = pt.oid - WHERE datname = '%1$s' - ) database_properties - ) AS database_properties, - ( - SELECT COUNT(*)=1 - FROM pg_dist_object WHERE objid = (SELECT oid FROM pg_database WHERE datname = '%1$s') - ) AS pg_dist_object_record_for_db_exists, - ( - SELECT COUNT(*) > 0 - FROM pg_dist_object - WHERE classid = 1262 AND objid NOT IN (SELECT oid FROM pg_database) - ) AS stale_pg_dist_object_record_for_a_db_exists - ) q - $$, - p_database_name, pg_ge_15_options, pg_ge_16_options - ) - ) q2 - JOIN pg_dist_node USING (nodeid); + CASE + WHEN nodeport = master_port THEN 'coordinator' + WHEN nodeport = worker_1_port THEN 'worker_1' + WHEN nodeport = worker_2_port THEN 'worker_2' + ELSE 'unexpected_node' + END AS node_type, + a.result + FROM run_command_on_all_nodes(pg_seclabels_cmd) a + JOIN pg_dist_node USING (nodeid) + ORDER BY node_type; END; $func$ LANGUAGE plpgsql; diff --git a/src/test/regress/expected/seclabel.out b/src/test/regress/expected/seclabel.out new file mode 100644 index 000000000..f826de44b --- /dev/null +++ b/src/test/regress/expected/seclabel.out @@ -0,0 +1,173 @@ +-- +-- SECLABEL +-- +-- Test suite for SECURITY LABEL ON ROLE statements +-- +-- first we remove one of the worker nodes to be able to test +-- citus_add_node later +SELECT citus_remove_node('localhost', :worker_2_port); + citus_remove_node +--------------------------------------------------------------------- + +(1 row) + +-- create two roles, one with characters that need escaping +CREATE ROLE user1; +CREATE ROLE "user 2"; +-- check an invalid label for our current dummy hook citus_test_object_relabel +SECURITY LABEL FOR "citus '!tests_label_provider" ON ROLE user1 IS 'invalid_label'; +ERROR: 'invalid_label' is not a valid security label for Citus tests. +-- if we disable metadata_sync, the command will not be propagated +SET citus.enable_metadata_sync TO off; +SECURITY LABEL FOR "citus '!tests_label_provider" ON ROLE user1 IS 'citus_unclassified'; +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 | +(2 rows) + +RESET citus.enable_metadata_sync; +-- check that we only support propagating for roles +SET citus.shard_replication_factor to 1; +-- distributed table +CREATE TABLE a (a int); +SELECT create_distributed_table('a', 'a'); + create_distributed_table +--------------------------------------------------------------------- + +(1 row) + +-- distributed view +CREATE VIEW v_dist AS SELECT * FROM a; +-- distributed function +CREATE FUNCTION notice(text) RETURNS void LANGUAGE plpgsql AS $$ + BEGIN RAISE NOTICE '%', $1; END; $$; +SECURITY LABEL ON TABLE a IS 'citus_classified'; +NOTICE: not propagating SECURITY LABEL commands whose object type is not role +HINT: Connect to worker nodes directly to manually run the same SECURITY LABEL command. +SECURITY LABEL ON FUNCTION notice IS 'citus_unclassified'; +NOTICE: not propagating SECURITY LABEL commands whose object type is not role +HINT: Connect to worker nodes directly to manually run the same SECURITY LABEL command. +SECURITY LABEL ON VIEW v_dist IS 'citus_classified'; +NOTICE: not propagating SECURITY LABEL commands whose object type is not role +HINT: Connect to worker nodes directly to manually run the same SECURITY LABEL command. +SELECT node_type, result FROM get_citus_tests_label_provider_labels('a') ORDER BY node_type; + node_type | result +--------------------------------------------------------------------- + coordinator | {"label": "citus_classified", "objtype": "table", "provider": "citus '!tests_label_provider"} + worker_1 | +(2 rows) + +SELECT node_type, result FROM get_citus_tests_label_provider_labels('notice(text)') ORDER BY node_type; + node_type | result +--------------------------------------------------------------------- + coordinator | {"label": "citus_unclassified", "objtype": "function", "provider": "citus '!tests_label_provider"} + worker_1 | +(2 rows) + +SELECT node_type, result FROM get_citus_tests_label_provider_labels('v_dist') ORDER BY node_type; + node_type | result +--------------------------------------------------------------------- + coordinator | {"label": "citus_classified", "objtype": "view", "provider": "citus '!tests_label_provider"} + worker_1 | +(2 rows) + +\c - - - :worker_1_port +SECURITY LABEL ON TABLE a IS 'citus_classified'; +SECURITY LABEL ON FUNCTION notice IS 'citus_unclassified'; +SECURITY LABEL ON VIEW v_dist IS 'citus_classified'; +\c - - - :master_port +SELECT node_type, result FROM get_citus_tests_label_provider_labels('a') ORDER BY node_type; + node_type | result +--------------------------------------------------------------------- + coordinator | {"label": "citus_classified", "objtype": "table", "provider": "citus '!tests_label_provider"} + worker_1 | {"label": "citus_classified", "objtype": "table", "provider": "citus '!tests_label_provider"} +(2 rows) + +SELECT node_type, result FROM get_citus_tests_label_provider_labels('notice(text)') ORDER BY node_type; + node_type | result +--------------------------------------------------------------------- + coordinator | {"label": "citus_unclassified", "objtype": "function", "provider": "citus '!tests_label_provider"} + worker_1 | {"label": "citus_unclassified", "objtype": "function", "provider": "citus '!tests_label_provider"} +(2 rows) + +SELECT node_type, result FROM get_citus_tests_label_provider_labels('v_dist') ORDER BY node_type; + node_type | result +--------------------------------------------------------------------- + coordinator | {"label": "citus_classified", "objtype": "view", "provider": "citus '!tests_label_provider"} + worker_1 | {"label": "citus_classified", "objtype": "view", "provider": "citus '!tests_label_provider"} +(2 rows) + +DROP TABLE a CASCADE; +NOTICE: drop cascades to view v_dist +DROP FUNCTION notice; +-- test that SECURITY LABEL statement is actually propagated for ROLES +SET citus.log_remote_commands TO on; +SET citus.grep_remote_commands = '%SECURITY LABEL%'; +-- we have exactly one provider loaded, so we may not include the provider in the command +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 +SECURITY LABEL ON ROLE user1 IS NULL; +NOTICE: issuing SECURITY LABEL ON ROLE user1 IS NULL +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' +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; +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) + +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) + +-- add a new node and check that it also propagates the SECURITY LABEL statement to the new node +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' +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 + ?column? +--------------------------------------------------------------------- + 1 +(1 row) + +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"} +(3 rows) + +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"} + worker_2 | {"label": "citus '!unclassified", "objtype": "role", "provider": "citus '!tests_label_provider"} +(3 rows) + +-- cleanup +RESET citus.log_remote_commands; +DROP ROLE user1, "user 2"; diff --git a/src/test/regress/multi_1_schedule b/src/test/regress/multi_1_schedule index 21b7a27fd..41672ae07 100644 --- a/src/test/regress/multi_1_schedule +++ b/src/test/regress/multi_1_schedule @@ -32,6 +32,7 @@ test: propagate_extension_commands test: escape_extension_name test: ref_citus_local_fkeys test: alter_database_owner +test: seclabel test: distributed_triggers test: create_single_shard_table test: create_drop_database_propagation diff --git a/src/test/regress/pg_regress_multi.pl b/src/test/regress/pg_regress_multi.pl index ba2251e37..c4b59ef0b 100755 --- a/src/test/regress/pg_regress_multi.pl +++ b/src/test/regress/pg_regress_multi.pl @@ -510,6 +510,12 @@ if($vanillatest) # 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"); } +else +{ + # We currently need this config for isolation tests and security label tests + # this option loads a security label provider, which we don't want in vanilla tests + push(@pgOptions, "citus.running_under_citus_test_suite=true"); +} if ($useMitmproxy) { @@ -560,7 +566,6 @@ if($isolationtester) push(@pgOptions, "citus.metadata_sync_interval=1000"); push(@pgOptions, "citus.metadata_sync_retry_interval=100"); push(@pgOptions, "client_min_messages='warning'"); # pg12 introduced notice showing during isolation tests - push(@pgOptions, "citus.running_under_isolation_test=true"); # Disable all features of the maintenance daemon. Otherwise queries might # randomly show temporarily as "waiting..." because they are waiting for the diff --git a/src/test/regress/sql/multi_test_helpers.sql b/src/test/regress/sql/multi_test_helpers.sql index 835174169..7f0346d14 100644 --- a/src/test/regress/sql/multi_test_helpers.sql +++ b/src/test/regress/sql/multi_test_helpers.sql @@ -551,74 +551,33 @@ BEGIN END; $func$ LANGUAGE plpgsql; - --- For all nodes, returns database properties of given database, except --- oid, datfrozenxid and datminmxid. --- --- Also returns whether the node has a pg_dist_object record for the database --- and whether there are any stale pg_dist_object records for a database. -CREATE OR REPLACE FUNCTION check_database_on_all_nodes(p_database_name text) -RETURNS TABLE (node_type text, result text) +-- Returns pg_seclabels entries from all nodes in the cluster for which +-- the object name is the input. +CREATE OR REPLACE FUNCTION get_citus_tests_label_provider_labels(object_name text, + master_port INTEGER DEFAULT 57636, + worker_1_port INTEGER DEFAULT 57637, + worker_2_port INTEGER DEFAULT 57638) +RETURNS TABLE ( + node_type text, + result text +) AS $func$ DECLARE - pg_ge_15_options text := ''; - pg_ge_16_options text := ''; + pg_seclabels_cmd TEXT := 'SELECT to_jsonb(q.*) FROM (' || + 'SELECT provider, objtype, label FROM pg_seclabels ' || + 'WHERE objname = ''' || object_name || ''') q'; BEGIN - IF EXISTS (SELECT 1 FROM pg_attribute WHERE attrelid = 'pg_database'::regclass AND attname = 'datlocprovider') THEN - pg_ge_15_options := ', daticulocale, datcollversion, datlocprovider'; - ELSE - pg_ge_15_options := $$, null as daticulocale, null as datcollversion, 'c' as datlocprovider$$; - END IF; - - IF EXISTS (SELECT 1 FROM pg_attribute WHERE attrelid = 'pg_database'::regclass AND attname = 'daticurules') THEN - pg_ge_16_options := ', daticurules'; - ELSE - pg_ge_16_options := ', null as daticurules'; - END IF; - - RETURN QUERY + RETURN QUERY SELECT - CASE WHEN (groupid = 0 AND groupid = (SELECT groupid FROM pg_dist_local_group)) THEN 'coordinator (local)' - WHEN (groupid = 0) THEN 'coordinator (remote)' - WHEN (groupid = (SELECT groupid FROM pg_dist_local_group)) THEN 'worker node (local)' - ELSE 'worker node (remote)' - END AS node_type, - q2.result - FROM run_command_on_all_nodes( - format( - $$ - SELECT to_jsonb(q.*) - FROM ( - SELECT - ( - SELECT to_jsonb(database_properties.*) - FROM ( - SELECT datname, pa.rolname as database_owner, - pg_encoding_to_char(pd.encoding) as encoding, - datistemplate, datallowconn, datconnlimit, datacl, - pt.spcname AS tablespace, datcollate, datctype - %2$s -- >= pg15 options - %3$s -- >= pg16 options - FROM pg_database pd - JOIN pg_authid pa ON pd.datdba = pa.oid - JOIN pg_tablespace pt ON pd.dattablespace = pt.oid - WHERE datname = '%1$s' - ) database_properties - ) AS database_properties, - ( - SELECT COUNT(*)=1 - FROM pg_dist_object WHERE objid = (SELECT oid FROM pg_database WHERE datname = '%1$s') - ) AS pg_dist_object_record_for_db_exists, - ( - SELECT COUNT(*) > 0 - FROM pg_dist_object - WHERE classid = 1262 AND objid NOT IN (SELECT oid FROM pg_database) - ) AS stale_pg_dist_object_record_for_a_db_exists - ) q - $$, - p_database_name, pg_ge_15_options, pg_ge_16_options - ) - ) q2 - JOIN pg_dist_node USING (nodeid); + CASE + WHEN nodeport = master_port THEN 'coordinator' + WHEN nodeport = worker_1_port THEN 'worker_1' + WHEN nodeport = worker_2_port THEN 'worker_2' + ELSE 'unexpected_node' + END AS node_type, + a.result + FROM run_command_on_all_nodes(pg_seclabels_cmd) a + JOIN pg_dist_node USING (nodeid) + ORDER BY node_type; END; $func$ LANGUAGE plpgsql; diff --git a/src/test/regress/sql/seclabel.sql b/src/test/regress/sql/seclabel.sql new file mode 100644 index 000000000..e523fc1da --- /dev/null +++ b/src/test/regress/sql/seclabel.sql @@ -0,0 +1,87 @@ +-- +-- SECLABEL +-- +-- Test suite for SECURITY LABEL ON ROLE statements +-- + +-- first we remove one of the worker nodes to be able to test +-- citus_add_node later +SELECT citus_remove_node('localhost', :worker_2_port); + +-- create two roles, one with characters that need escaping +CREATE ROLE user1; +CREATE ROLE "user 2"; + +-- check an invalid label for our current dummy hook citus_test_object_relabel +SECURITY LABEL FOR "citus '!tests_label_provider" ON ROLE user1 IS 'invalid_label'; + +-- if we disable metadata_sync, the command will not be propagated +SET citus.enable_metadata_sync TO off; +SECURITY LABEL FOR "citus '!tests_label_provider" 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_metadata_sync; + +-- check that we only support propagating for roles +SET citus.shard_replication_factor to 1; +-- distributed table +CREATE TABLE a (a int); +SELECT create_distributed_table('a', 'a'); +-- distributed view +CREATE VIEW v_dist AS SELECT * FROM a; +-- distributed function +CREATE FUNCTION notice(text) RETURNS void LANGUAGE plpgsql AS $$ + BEGIN RAISE NOTICE '%', $1; END; $$; + +SECURITY LABEL ON TABLE a IS 'citus_classified'; +SECURITY LABEL ON FUNCTION notice IS 'citus_unclassified'; +SECURITY LABEL ON VIEW v_dist IS 'citus_classified'; + +SELECT node_type, result FROM get_citus_tests_label_provider_labels('a') ORDER BY node_type; +SELECT node_type, result FROM get_citus_tests_label_provider_labels('notice(text)') ORDER BY node_type; +SELECT node_type, result FROM get_citus_tests_label_provider_labels('v_dist') ORDER BY node_type; + +\c - - - :worker_1_port +SECURITY LABEL ON TABLE a IS 'citus_classified'; +SECURITY LABEL ON FUNCTION notice IS 'citus_unclassified'; +SECURITY LABEL ON VIEW v_dist IS 'citus_classified'; + +\c - - - :master_port +SELECT node_type, result FROM get_citus_tests_label_provider_labels('a') ORDER BY node_type; +SELECT node_type, result FROM get_citus_tests_label_provider_labels('notice(text)') ORDER BY node_type; +SELECT node_type, result FROM get_citus_tests_label_provider_labels('v_dist') ORDER BY node_type; + +DROP TABLE a CASCADE; +DROP FUNCTION notice; + +-- test that SECURITY LABEL statement is actually propagated for ROLES +SET citus.log_remote_commands TO on; +SET citus.grep_remote_commands = '%SECURITY LABEL%'; + +-- we have exactly one provider loaded, so we may not include the provider in the command +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'; + +\c - - - :worker_1_port +-- command not allowed from worker node +SECURITY LABEL for "citus '!tests_label_provider" ON ROLE user1 IS 'citus ''!unclassified'; + +\c - - - :master_port +RESET citus.log_remote_commands; + +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; + +-- add a new node and check that it also propagates the SECURITY LABEL statement to the new node +SET citus.log_remote_commands TO on; +SET citus.grep_remote_commands = '%SECURITY LABEL%'; +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; + +-- cleanup +RESET citus.log_remote_commands; +DROP ROLE user1, "user 2";