Adds alter user missing features (#7204)

DESCRIPTION: Adds alter user rename propagation and enriches alter user
tests

---------

Co-authored-by: Jelte Fennema <github-tech@jeltef.nl>
pull/7227/head^2
Gürkan İndibay 2023-09-26 12:28:07 +03:00 committed by GitHub
parent a9d28ca96f
commit 7fa109c977
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
11 changed files with 275 additions and 54 deletions

View File

@ -150,6 +150,17 @@ static DistributeObjectOps Any_AlterRole = {
.address = AlterRoleStmtObjectAddress,
.markDistributed = false,
};
static DistributeObjectOps Any_AlterRoleRename = {
.deparse = DeparseRenameRoleStmt,
.qualify = NULL,
.preprocess = PreprocessAlterRoleRenameStmt,
.postprocess = NULL,
.operationType = DIST_OPS_ALTER,
.address = RenameRoleStmtObjectAddress,
.markDistributed = false,
};
static DistributeObjectOps Any_AlterRoleSet = {
.deparse = DeparseAlterRoleSetStmt,
.qualify = QualifyAlterRoleSetStmt,
@ -2059,6 +2070,11 @@ GetDistributeObjectOps(Node *node)
return &Publication_Rename;
}
case OBJECT_ROLE:
{
return &Any_AlterRoleRename;
}
case OBJECT_ROUTINE:
{
return &Routine_Rename;

View File

@ -1306,3 +1306,54 @@ EnsureSequentialModeForRoleDDL(void)
"use only one connection for all future commands")));
SetLocalMultiShardModifyModeToSequential();
}
/*
* PreprocessAlterDatabaseSetStmt is executed before the statement is applied to the local
* postgres instance.
*
* In this stage we can prepare the commands that need to be run on all workers to grant
* on databases.
*/
List *
PreprocessAlterRoleRenameStmt(Node *node, const char *queryString,
ProcessUtilityContext processUtilityContext)
{
if (!ShouldPropagate())
{
return NIL;
}
if (!EnableAlterRolePropagation)
{
return NIL;
}
RenameStmt *stmt = castNode(RenameStmt, node);
Assert(stmt->renameType == OBJECT_ROLE);
EnsureCoordinator();
char *sql = DeparseTreeNode((Node *) stmt);
List *commands = list_make3(DISABLE_DDL_PROPAGATION,
(void *) sql,
ENABLE_DDL_PROPAGATION);
return NodeDDLTaskList(NON_COORDINATOR_NODES, commands);
}
List *
RenameRoleStmtObjectAddress(Node *node, bool missing_ok, bool isPostprocess)
{
RenameStmt *stmt = castNode(RenameStmt, node);
Assert(stmt->renameType == OBJECT_ROLE);
Oid roleOid = get_role_oid(stmt->subname, missing_ok);
ObjectAddress *address = palloc0(sizeof(ObjectAddress));
ObjectAddressSet(*address, AuthIdRelationId, roleOid);
return list_make1(address);
}

View File

@ -817,19 +817,6 @@ ProcessUtilityInternal(PlannedStmt *pstmt,
ddlJobs = processJobs;
}
}
if (IsA(parsetree, RenameStmt) && ((RenameStmt *) parsetree)->renameType ==
OBJECT_ROLE && EnableAlterRolePropagation)
{
if (EnableUnsupportedFeatureMessages)
{
ereport(NOTICE, (errmsg(
"not propagating ALTER ROLE ... RENAME TO commands "
"to worker nodes"),
errhint("Connect to worker nodes directly to manually "
"rename the role")));
}
}
}
if (IsA(parsetree, CreateStmt))

View File

@ -201,17 +201,51 @@ DeparseCreateRoleStmt(Node *node)
}
static void
AppendSysIdStatement(StringInfo buf, ListCell *optionCell)
{
DefElem *option = (DefElem *) lfirst(optionCell);
if (strcmp(option->defname, "sysid") == 0)
{
appendStringInfo(buf, " SYSID %d", intVal(option->arg));
}
}
/*
* AppendCreateRoleStmt generates the string representation of the
* CreateRoleStmt and appends it to the buffer.
* AppendInlinePriviliges generates the string representation for the inline
* privileges of the role in create statement and appends it to the buffer.
*/
static void
AppendCreateRoleStmt(StringInfo buf, CreateRoleStmt *stmt)
AppendInlinePriviliges(StringInfo buf, ListCell *optionCell)
{
ListCell *optionCell = NULL;
DefElem *option = (DefElem *) lfirst(optionCell);
appendStringInfo(buf, "CREATE ");
if (strcmp(option->defname, "adminmembers") == 0)
{
appendStringInfo(buf, " ADMIN ");
AppendRoleList(buf, (List *) option->arg);
}
else if (strcmp(option->defname, "rolemembers") == 0)
{
appendStringInfo(buf, " ROLE ");
AppendRoleList(buf, (List *) option->arg);
}
else if (strcmp(option->defname, "addroleto") == 0)
{
appendStringInfo(buf, " IN ROLE ");
AppendRoleList(buf, (List *) option->arg);
}
}
/*
* AppendStatementType generates the string representation for the statement
* type (role, user or group) in alter/create statement and appends it to the buffer.
*/
static void
AppendStatementType(StringInfo buf, CreateRoleStmt *stmt)
{
switch (stmt->stmt_type)
{
case ROLESTMT_ROLE:
@ -232,34 +266,29 @@ AppendCreateRoleStmt(StringInfo buf, CreateRoleStmt *stmt)
break;
}
}
}
/*
* AppendCreateRoleStmt generates the string representation of the
* CreateRoleStmt and appends it to the buffer.
*/
static void
AppendCreateRoleStmt(StringInfo buf, CreateRoleStmt *stmt)
{
ListCell *optionCell = NULL;
appendStringInfo(buf, "CREATE ");
AppendStatementType(buf, stmt);
appendStringInfo(buf, "%s", quote_identifier(stmt->role));
foreach(optionCell, stmt->options)
{
AppendRoleOption(buf, optionCell);
DefElem *option = (DefElem *) lfirst(optionCell);
if (strcmp(option->defname, "sysid") == 0)
{
appendStringInfo(buf, " SYSID %d", intVal(option->arg));
}
else if (strcmp(option->defname, "adminmembers") == 0)
{
appendStringInfo(buf, " ADMIN ");
AppendRoleList(buf, (List *) option->arg);
}
else if (strcmp(option->defname, "rolemembers") == 0)
{
appendStringInfo(buf, " ROLE ");
AppendRoleList(buf, (List *) option->arg);
}
else if (strcmp(option->defname, "addroleto") == 0)
{
appendStringInfo(buf, " IN ROLE ");
AppendRoleList(buf, (List *) option->arg);
}
AppendInlinePriviliges(buf, optionCell);
AppendSysIdStatement(buf, optionCell);
}
}
@ -326,6 +355,22 @@ AppendRoleList(StringInfo buf, List *roleList)
}
char *
DeparseRenameRoleStmt(Node *node)
{
RenameStmt *stmt = castNode(RenameStmt, node);
StringInfoData str = { 0 };
initStringInfo(&str);
Assert(stmt->renameType == OBJECT_ROLE);
appendStringInfo(&str, "ALTER ROLE %s RENAME TO %s;",
quote_identifier(stmt->subname), quote_identifier(stmt->newname));
return str.data;
}
/*
* DeparseGrantRoleStmt builds and returns a string representing of the
* GrantRoleStmt for application on a remote server.

View File

@ -479,6 +479,10 @@ extern List * PreprocessRenameAttributeStmt(Node *stmt, const char *queryString,
extern List * PostprocessAlterRoleStmt(Node *stmt, const char *queryString);
extern List * PreprocessAlterRoleSetStmt(Node *stmt, const char *queryString,
ProcessUtilityContext processUtilityContext);
extern List * PreprocessAlterRoleRenameStmt(Node *stmt, const char *queryString,
ProcessUtilityContext processUtilityContext);
extern List * GenerateAlterRoleSetCommandForRole(Oid roleid);
extern List * AlterRoleStmtObjectAddress(Node *node,
bool missing_ok, bool isPostprocess);
@ -494,6 +498,10 @@ extern List * PostprocessGrantRoleStmt(Node *stmt, const char *queryString);
extern List * GenerateCreateOrAlterRoleCommand(Oid roleOid);
extern List * CreateRoleStmtObjectAddress(Node *stmt, bool missing_ok, bool
isPostprocess);
extern List * RenameRoleStmtObjectAddress(Node *stmt, bool missing_ok, bool
isPostprocess);
extern void UnmarkRolesDistributed(List *roles);
extern List * FilterDistributedRoles(List *roles);

View File

@ -201,6 +201,7 @@ extern void QualifyAlterFunctionDependsStmt(Node *stmt);
/* forward declarations for deparse_role_stmts.c */
extern char * DeparseAlterRoleStmt(Node *stmt);
extern char * DeparseAlterRoleSetStmt(Node *stmt);
extern char * DeparseRenameRoleStmt(Node *stmt);
extern List * MakeSetStatementArguments(char *configurationName,
char *configurationValue);

View File

@ -108,6 +108,9 @@ DEPS = {
"minimal_cluster_management": TestDeps(
None, ["multi_test_helpers_superuser"], repeatable=False
),
"multi_behavioral_analytics_create_table": TestDeps(
"minimal_schedule", ["multi_test_helpers_superuser"], repeatable=False
),
"create_role_propagation": TestDeps(None, ["multi_cluster_management"]),
"single_node_enterprise": TestDeps(None),
"single_node": TestDeps(None, ["multi_test_helpers"]),
@ -131,6 +134,7 @@ DEPS = {
"alter_distributed_table": TestDeps(
"minimal_schedule", ["multi_behavioral_analytics_create_table"]
),
"alter_role_propagation": TestDeps("minimal_schedule"),
"background_rebalance": TestDeps(
None,
[
@ -149,6 +153,7 @@ DEPS = {
),
"function_propagation": TestDeps("minimal_schedule"),
"grant_on_foreign_server_propagation": TestDeps("minimal_schedule"),
"multi_modifying_xacts": TestDeps("minimal_schedule"),
"multi_mx_modifying_xacts": TestDeps(None, ["multi_mx_create_table"]),
"multi_mx_router_planner": TestDeps(None, ["multi_mx_create_table"]),
"multi_mx_copy_data": TestDeps(None, ["multi_mx_create_table"]),

View File

@ -1,7 +1,7 @@
CREATE SCHEMA alter_role;
CREATE SCHEMA ",CitUs,.TeeN!?";
-- test if the passowrd of the extension owner can be upgraded
ALTER ROLE CURRENT_USER PASSWORD 'password123' VALID UNTIL 'infinity';
ALTER ROLE CURRENT_USER CONNECTION LIMIT -1 PASSWORD 'password123' VALID UNTIL 'infinity';
SELECT run_command_on_workers($$SELECT row(rolname, rolsuper, rolinherit, rolcreaterole, rolcreatedb, rolcanlogin, rolreplication, rolbypassrls, rolconnlimit, EXTRACT (year FROM rolvaliduntil)) FROM pg_authid WHERE rolname = current_user$$);
run_command_on_workers
---------------------------------------------------------------------
@ -356,5 +356,56 @@ SELECT workers.result AS worker_password, pg_authid.rolpassword AS coord_passwor
RESET password_encryption;
DROP ROLE new_role;
drop user if exists test1 ;
NOTICE: role "test1" does not exist, skipping
create user test1;
SELECT run_command_on_workers($$SELECT row() FROM pg_roles WHERE rolname = 'test1'$$);
run_command_on_workers
---------------------------------------------------------------------
(localhost,57637,t,"()")
(localhost,57638,t,"()")
(2 rows)
alter user test1 with encrypted password 'test1' nosuperuser noinherit nocreaterole nocreatedb nologin noreplication nobypassrls connection limit -1 valid until 'infinity';
SELECT run_command_on_workers($$SELECT row(rolname, rolsuper, rolinherit, rolcreaterole, rolcreatedb, rolcanlogin, rolreplication, rolbypassrls, rolconnlimit, EXTRACT (year FROM rolvaliduntil)) FROM pg_authid WHERE rolname = 'test1'$$);
run_command_on_workers
---------------------------------------------------------------------
(localhost,57637,t,"(test1,f,f,f,f,f,f,f,-1,Infinity)")
(localhost,57638,t,"(test1,f,f,f,f,f,f,f,-1,Infinity)")
(2 rows)
alter user test1 with password NULL superuser inherit createrole createdb login replication bypassrls connection limit 10 valid until '2019-01-01';
SELECT run_command_on_workers($$SELECT row(rolname, rolsuper, rolinherit, rolcreaterole, rolcreatedb, rolcanlogin, rolreplication, rolbypassrls, rolconnlimit, EXTRACT (year FROM rolvaliduntil)) FROM pg_authid WHERE rolname = 'test1'$$);
run_command_on_workers
---------------------------------------------------------------------
(localhost,57637,t,"(test1,t,t,t,t,t,t,t,10,2019)")
(localhost,57638,t,"(test1,t,t,t,t,t,t,t,10,2019)")
(2 rows)
SET citus.enable_alter_role_set_propagation TO on;
SET citus.log_remote_commands = true;
set citus.grep_remote_commands = '%ALTER ROLE%';
ALTER USER test1 SET timezone TO 'America/New_York';
NOTICE: issuing ALTER ROLE test1 SET timezone = 'America/New_York'
DETAIL: on server postgres@localhost:xxxxx connectionId: xxxxxxx
NOTICE: issuing ALTER ROLE test1 SET timezone = 'America/New_York'
DETAIL: on server postgres@localhost:xxxxx connectionId: xxxxxxx
ALTER USER test1 SET work_mem TO '64MB';
NOTICE: issuing ALTER ROLE test1 SET work_mem = '64MB'
DETAIL: on server postgres@localhost:xxxxx connectionId: xxxxxxx
NOTICE: issuing ALTER ROLE test1 SET work_mem = '64MB'
DETAIL: on server postgres@localhost:xxxxx connectionId: xxxxxxx
ALTER USER test1 SET random_page_cost TO 1.5;
NOTICE: issuing ALTER ROLE test1 SET random_page_cost = 1.5
DETAIL: on server postgres@localhost:xxxxx connectionId: xxxxxxx
NOTICE: issuing ALTER ROLE test1 SET random_page_cost = 1.5
DETAIL: on server postgres@localhost:xxxxx connectionId: xxxxxxx
alter user test1 rename to test2;
NOTICE: issuing ALTER ROLE test1 RENAME TO test2;
DETAIL: on server postgres@localhost:xxxxx connectionId: xxxxxxx
NOTICE: issuing ALTER ROLE test1 RENAME TO test2;
DETAIL: on server postgres@localhost:xxxxx connectionId: xxxxxxx
drop user test2;
SET citus.log_remote_commands = false;
DROP TABLE test_search_path;
DROP SCHEMA alter_role, ",CitUs,.TeeN!?", test_sp CASCADE;

View File

@ -1200,8 +1200,9 @@ SET citus.override_table_visibility TO false;
-- and rename the existing user
\c - :default_user - :worker_1_port
SET search_path TO multi_modifying_xacts;
set citus.enable_alter_role_propagation=false;
ALTER USER test_user RENAME TO test_user_new;
NOTICE: not propagating ALTER ROLE ... RENAME TO commands to worker nodes
set citus.enable_alter_role_propagation=true;
-- connect back to master and query the reference table
\c - test_user - :master_port
SET search_path TO multi_modifying_xacts;
@ -1320,8 +1321,9 @@ WARNING: connection to the remote node localhost:xxxxx failed with the followin
-- break the other node as well
\c - :default_user - :worker_2_port
SET search_path TO multi_modifying_xacts;
set citus.enable_alter_role_propagation=false;
ALTER USER test_user RENAME TO test_user_new;
NOTICE: not propagating ALTER ROLE ... RENAME TO commands to worker nodes
set citus.enable_alter_role_propagation=true;
\c - test_user - :master_port
SET search_path TO multi_modifying_xacts;
-- fails on all shard placements
@ -1330,16 +1332,21 @@ ERROR: connection to the remote node localhost:xxxxx failed with the following
-- connect back to the master with the proper user to continue the tests
\c - :default_user - :master_port
SET search_path TO multi_modifying_xacts;
-- unbreak both nodes by renaming the user back to the original name
\c - :default_user - :worker_2_port
SET search_path TO multi_modifying_xacts;
set citus.enable_alter_role_propagation=false;
ALTER USER test_user_new RENAME TO test_user;
set citus.enable_alter_role_propagation=true;
\c - :default_user - :worker_1_port
SET search_path TO multi_modifying_xacts;
set citus.enable_alter_role_propagation=false;
ALTER USER test_user_new RENAME TO test_user;
set citus.enable_alter_role_propagation=true;
\c - :default_user - :master_port
SET search_path TO multi_modifying_xacts;
SET citus.next_shard_id TO 1200020;
SET citus.next_placement_id TO 1200033;
-- unbreak both nodes by renaming the user back to the original name
SELECT * FROM run_command_on_workers('ALTER USER test_user_new RENAME TO test_user');
nodename | nodeport | success | result
---------------------------------------------------------------------
localhost | 57637 | t | ALTER ROLE
localhost | 57638 | t | ALTER ROLE
(2 rows)
DROP TABLE reference_modifying_xacts, hash_modifying_xacts, hash_modifying_xacts_second,
reference_failure_test, numbers_hash_failure_test;
REVOKE ALL ON SCHEMA multi_modifying_xacts FROM test_user;

View File

@ -2,7 +2,7 @@ CREATE SCHEMA alter_role;
CREATE SCHEMA ",CitUs,.TeeN!?";
-- test if the passowrd of the extension owner can be upgraded
ALTER ROLE CURRENT_USER PASSWORD 'password123' VALID UNTIL 'infinity';
ALTER ROLE CURRENT_USER CONNECTION LIMIT -1 PASSWORD 'password123' VALID UNTIL 'infinity';
SELECT run_command_on_workers($$SELECT row(rolname, rolsuper, rolinherit, rolcreaterole, rolcreatedb, rolcanlogin, rolreplication, rolbypassrls, rolconnlimit, EXTRACT (year FROM rolvaliduntil)) FROM pg_authid WHERE rolname = current_user$$);
SELECT workers.result = pg_authid.rolpassword AS password_is_same FROM run_command_on_workers($$SELECT rolpassword FROM pg_authid WHERE rolname = current_user$$) workers, pg_authid WHERE pg_authid.rolname = current_user;
@ -79,6 +79,7 @@ SELECT run_command_on_workers('SHOW enable_hashagg');
-- check that ALTER ROLE SET is not propagated when scoped to a different database
-- also test case sensitivity
CREATE DATABASE "REGRESSION";
ALTER ROLE CURRENT_USER IN DATABASE "REGRESSION" SET public.myguc TO "Hello from coordinator only";
SELECT d.datname, r.setconfig FROM pg_db_role_setting r LEFT JOIN pg_database d ON r.setdatabase=d.oid WHERE r.setconfig::text LIKE '%Hello from coordinator only%';
SELECT run_command_on_workers($$SELECT json_agg((d.datname, r.setconfig)) FROM pg_db_role_setting r LEFT JOIN pg_database d ON r.setdatabase=d.oid WHERE r.setconfig::text LIKE '%Hello from coordinator only%'$$);
@ -119,5 +120,35 @@ SELECT workers.result AS worker_password, pg_authid.rolpassword AS coord_passwor
RESET password_encryption;
DROP ROLE new_role;
drop user if exists test1 ;
create user test1;
SELECT run_command_on_workers($$SELECT row() FROM pg_roles WHERE rolname = 'test1'$$);
alter user test1 with encrypted password 'test1' nosuperuser noinherit nocreaterole nocreatedb nologin noreplication nobypassrls connection limit -1 valid until 'infinity';
SELECT run_command_on_workers($$SELECT row(rolname, rolsuper, rolinherit, rolcreaterole, rolcreatedb, rolcanlogin, rolreplication, rolbypassrls, rolconnlimit, EXTRACT (year FROM rolvaliduntil)) FROM pg_authid WHERE rolname = 'test1'$$);
alter user test1 with password NULL superuser inherit createrole createdb login replication bypassrls connection limit 10 valid until '2019-01-01';
SELECT run_command_on_workers($$SELECT row(rolname, rolsuper, rolinherit, rolcreaterole, rolcreatedb, rolcanlogin, rolreplication, rolbypassrls, rolconnlimit, EXTRACT (year FROM rolvaliduntil)) FROM pg_authid WHERE rolname = 'test1'$$);
SET citus.enable_alter_role_set_propagation TO on;
SET citus.log_remote_commands = true;
set citus.grep_remote_commands = '%ALTER ROLE%';
ALTER USER test1 SET timezone TO 'America/New_York';
ALTER USER test1 SET work_mem TO '64MB';
ALTER USER test1 SET random_page_cost TO 1.5;
alter user test1 rename to test2;
drop user test2;
SET citus.log_remote_commands = false;
DROP TABLE test_search_path;
DROP SCHEMA alter_role, ",CitUs,.TeeN!?", test_sp CASCADE;

View File

@ -981,7 +981,9 @@ SET citus.override_table_visibility TO false;
-- and rename the existing user
\c - :default_user - :worker_1_port
SET search_path TO multi_modifying_xacts;
set citus.enable_alter_role_propagation=false;
ALTER USER test_user RENAME TO test_user_new;
set citus.enable_alter_role_propagation=true;
-- connect back to master and query the reference table
\c - test_user - :master_port
@ -1050,7 +1052,9 @@ SELECT count(*) FROM numbers_hash_failure_test;
-- break the other node as well
\c - :default_user - :worker_2_port
SET search_path TO multi_modifying_xacts;
set citus.enable_alter_role_propagation=false;
ALTER USER test_user RENAME TO test_user_new;
set citus.enable_alter_role_propagation=true;
\c - test_user - :master_port
SET search_path TO multi_modifying_xacts;
@ -1061,10 +1065,25 @@ INSERT INTO numbers_hash_failure_test VALUES (2,2);
-- connect back to the master with the proper user to continue the tests
\c - :default_user - :master_port
SET search_path TO multi_modifying_xacts;
-- unbreak both nodes by renaming the user back to the original name
\c - :default_user - :worker_2_port
SET search_path TO multi_modifying_xacts;
set citus.enable_alter_role_propagation=false;
ALTER USER test_user_new RENAME TO test_user;
set citus.enable_alter_role_propagation=true;
\c - :default_user - :worker_1_port
SET search_path TO multi_modifying_xacts;
set citus.enable_alter_role_propagation=false;
ALTER USER test_user_new RENAME TO test_user;
set citus.enable_alter_role_propagation=true;
\c - :default_user - :master_port
SET search_path TO multi_modifying_xacts;
SET citus.next_shard_id TO 1200020;
SET citus.next_placement_id TO 1200033;
-- unbreak both nodes by renaming the user back to the original name
SELECT * FROM run_command_on_workers('ALTER USER test_user_new RENAME TO test_user');
DROP TABLE reference_modifying_xacts, hash_modifying_xacts, hash_modifying_xacts_second,
reference_failure_test, numbers_hash_failure_test;