diff --git a/.unreleased/pr_6987 b/.unreleased/pr_6987 new file mode 100644 index 000000000..e149545ad --- /dev/null +++ b/.unreleased/pr_6987 @@ -0,0 +1 @@ +Fixes: #6987 Fix REASSIGN OWNED BY for background jobs diff --git a/src/bgw/job.h b/src/bgw/job.h index 7850feac2..98e370ade 100644 --- a/src/bgw/job.h +++ b/src/bgw/job.h @@ -73,3 +73,4 @@ extern TSDLLEXPORT void ts_bgw_job_validate_schedule_interval(Interval *schedule extern TSDLLEXPORT char *ts_bgw_job_validate_timezone(Datum timezone); extern TSDLLEXPORT bool ts_is_telemetry_job(BgwJob *job); +ScanTupleResult ts_bgw_job_change_owner(TupleInfo *ti, void *data); diff --git a/src/process_utility.c b/src/process_utility.c index 571549645..7b8b4b238 100644 --- a/src/process_utility.c +++ b/src/process_utility.c @@ -24,6 +24,7 @@ #include #include #include +#include #include #include #include @@ -4061,6 +4062,60 @@ process_create_rule_start(ProcessUtilityArgs *args) return DDL_CONTINUE; } +/* + * Update the owner of a background job given by a heap tuple. + * + * Note that there is no check for correct privileges here and this is the + * responsibility of the caller. + */ +static void +ts_bgw_job_update_owner(Relation rel, HeapTuple tuple, TupleDesc tupledesc, Oid newrole_oid) +{ + bool isnull[Natts_bgw_job]; + Datum values[Natts_bgw_job]; + bool replace[Natts_bgw_job] = { false }; + HeapTuple new_tuple; + + heap_deform_tuple(tuple, tupledesc, values, isnull); + + if (DatumGetObjectId(values[AttrNumberGetAttrOffset(Anum_bgw_job_owner)]) != newrole_oid) + { + values[AttrNumberGetAttrOffset(Anum_bgw_job_owner)] = Int32GetDatum(newrole_oid); + replace[AttrNumberGetAttrOffset(Anum_bgw_job_owner)] = true; + new_tuple = heap_modify_tuple(tuple, tupledesc, values, isnull, replace); + ts_catalog_update(rel, new_tuple); + heap_freetuple(new_tuple); + } +} + +static DDLResult +process_reassign_owned_start(ProcessUtilityArgs *args) +{ + ReassignOwnedStmt *stmt = (ReassignOwnedStmt *) args->parsetree; + List *role_ids = roleSpecsToIds(stmt->roles); + ScanIterator iterator = + ts_scan_iterator_create(BGW_JOB, RowExclusiveLock, CurrentMemoryContext); + ts_scanner_foreach(&iterator) + { + bool should_free, isnull; + TupleInfo *ti = ts_scan_iterator_tuple_info(&iterator); + Datum value = slot_getattr(ti->slot, Anum_bgw_job_owner, &isnull); + if (!isnull && list_member_oid(role_ids, DatumGetObjectId(value))) + { + Oid newrole_oid = get_rolespec_oid(stmt->newrole, false); + HeapTuple tuple = ts_scanner_fetch_heap_tuple(ti, false, &should_free); + + /* We do not need to check privileges here since ReassignOwnedObjects() will check the + * privileges and error out if they are not correct. */ + ts_bgw_job_update_owner(ti->scanrel, tuple, ts_scanner_get_tupledesc(ti), newrole_oid); + + if (should_free) + heap_freetuple(tuple); + } + } + return DDL_CONTINUE; +} + /* ALTER TABLE SET ( timescaledb.compress, ...) */ static DDLResult process_altertable_set_options(AlterTableCmd *cmd, Hypertable *ht) @@ -4241,6 +4296,9 @@ process_ddl_command_start(ProcessUtilityArgs *args) case T_RuleStmt: handler = process_create_rule_start; break; + case T_ReassignOwnedStmt: + handler = process_reassign_owned_start; + break; case T_DropStmt: /* * Drop associated metadata/chunks but also continue on to drop diff --git a/tsl/test/expected/bgw_db_scheduler.out b/tsl/test/expected/bgw_db_scheduler.out index c8e830478..df8fa029b 100644 --- a/tsl/test/expected/bgw_db_scheduler.out +++ b/tsl/test/expected/bgw_db_scheduler.out @@ -1619,35 +1619,6 @@ consecutive_crashes | 0 flags | 0 \x off --- Test renaming a user and see that the owner of the job changes. -\c :TEST_DBNAME :ROLE_SUPERUSER -CREATE USER another_user; -SET ROLE another_user; -SELECT insert_job('another_one', 'bgw_test_job_1', INTERVAL '100ms', INTERVAL '100s', INTERVAL '1s') AS job_id \gset -SELECT proc_name, owner FROM _timescaledb_config.bgw_job WHERE id = :job_id; - proc_name | owner -----------------+-------------- - bgw_test_job_1 | another_user -(1 row) - -RESET ROLE; -ALTER USER another_user RENAME TO renamed_user; -SELECT proc_name, owner FROM _timescaledb_config.bgw_job WHERE id = :job_id; - proc_name | owner -----------------+-------------- - bgw_test_job_1 | renamed_user -(1 row) - --- This should fail since the job is dependent on the owner -\set VERBOSITY default -\set ON_ERROR_STOP 0 -DROP USER renamed_user; -ERROR: role "renamed_user" cannot be dropped because some objects depend on it -DETAIL: owner of job 1026 -\set ON_ERROR_STOP 1 -DELETE FROM _timescaledb_config.bgw_job WHERE id = :job_id; --- This should succeed -DROP USER renamed_user; -- -- Test without retry -- @@ -1670,7 +1641,7 @@ DELETE FROM _timescaledb_config.bgw_job; INSERT INTO _timescaledb_config.bgw_job(application_name, schedule_interval, max_runtime, max_retries, retry_period, proc_schema, proc_name) VALUES('bgw_test_job_2_error', INTERVAL '5000ms', INTERVAL '20ms', 0, INTERVAL '20ms', 'public', 'bgw_test_job_2_error') RETURNING id; id ------ - 1027 + 1026 (1 row) \c :TEST_DBNAME :ROLE_DEFAULT_PERM_USER @@ -1684,7 +1655,7 @@ SELECT ts_bgw_db_scheduler_test_run_and_wait_for_scheduler_finish(25); SELECT job_id, last_run_success, total_runs, total_successes, total_failures, total_crashes FROM _timescaledb_internal.bgw_job_stat; job_id | last_run_success | total_runs | total_successes | total_failures | total_crashes --------+------------------+------------+-----------------+----------------+--------------- - 1027 | f | 1 | 0 | 1 | 0 + 1026 | f | 1 | 0 | 1 | 0 (1 row) SELECT * FROM sorted_bgw_log; @@ -1692,8 +1663,8 @@ SELECT * FROM sorted_bgw_log; --------+----------------------+----------------------------------------------------------- 0 | DB Scheduler | [TESTING] Registered new background worker 1 | DB Scheduler | [TESTING] Wait until (RANDOM), started at (RANDOM) - 1 | bgw_test_job_2_error | job 1027 reached max_retries after 1 consecutive failures - 2 | bgw_test_job_2_error | job 1027 threw an error + 1 | bgw_test_job_2_error | job 1026 reached max_retries after 1 consecutive failures + 2 | bgw_test_job_2_error | job 1026 threw an error 3 | bgw_test_job_2_error | Error job 2 2 | DB Scheduler | [TESTING] Wait until (RANDOM), started at (RANDOM) (6 rows) @@ -1714,7 +1685,7 @@ SELECT ts_bgw_db_scheduler_test_run_and_wait_for_scheduler_finish(100, 50); SELECT job_id, last_run_success, total_runs, total_successes, total_failures, total_crashes FROM _timescaledb_internal.bgw_job_stat; job_id | last_run_success | total_runs | total_successes | total_failures | total_crashes --------+------------------+------------+-----------------+----------------+--------------- - 1027 | f | 1 | 0 | 1 | 0 + 1026 | f | 1 | 0 | 1 | 0 (1 row) -- We increase the mock time a lot to ensure the job does not get restarted. However, the amount of scheduler sleep/wakeup cycles @@ -1723,8 +1694,8 @@ SELECT * FROM sorted_bgw_log WHERE msg NOT LIKE '[TESTING] Wait until%'; msg_no | application_name | msg --------+----------------------+----------------------------------------------------------- 0 | DB Scheduler | [TESTING] Registered new background worker - 1 | bgw_test_job_2_error | job 1027 reached max_retries after 1 consecutive failures - 2 | bgw_test_job_2_error | job 1027 threw an error + 1 | bgw_test_job_2_error | job 1026 reached max_retries after 1 consecutive failures + 2 | bgw_test_job_2_error | job 1026 threw an error 3 | bgw_test_job_2_error | Error job 2 (4 rows) diff --git a/tsl/test/expected/bgw_job_ddl.out b/tsl/test/expected/bgw_job_ddl.out new file mode 100644 index 000000000..bf7f6d37f --- /dev/null +++ b/tsl/test/expected/bgw_job_ddl.out @@ -0,0 +1,91 @@ +-- This file and its contents are licensed under the Timescale License. +-- Please see the included NOTICE for copyright information and +-- LICENSE-TIMESCALE for a copy of the license. +-- Test for DDL-like functionality +\c :TEST_DBNAME :ROLE_SUPERUSER +CREATE OR REPLACE FUNCTION insert_job( + application_name NAME, + job_type NAME, + schedule_interval INTERVAL, + max_runtime INTERVAL, + retry_period INTERVAL, + owner regrole DEFAULT CURRENT_ROLE::regrole, + scheduled BOOL DEFAULT true, + fixed_schedule BOOL DEFAULT false +) RETURNS INT LANGUAGE SQL SECURITY DEFINER AS +$$ + INSERT INTO _timescaledb_config.bgw_job(application_name,schedule_interval,max_runtime,max_retries, + retry_period,proc_name,proc_schema,owner,scheduled,fixed_schedule) + VALUES($1,$3,$4,5,$5,$2,'public',$6,$7,$8) RETURNING id; +$$; +CREATE USER another_user; +SET ROLE another_user; +SELECT insert_job('another_one', 'bgw_test_job_1', INTERVAL '100ms', INTERVAL '100s', INTERVAL '1s') AS job_id \gset +SELECT proc_name, owner FROM _timescaledb_config.bgw_job WHERE id = :job_id; + proc_name | owner +----------------+-------------- + bgw_test_job_1 | another_user +(1 row) + +-- Test that reassigning to another user privileges does not work for +-- a normal user. We test both users with superuser privileges and +-- default permissions. +\set ON_ERROR_STOP 0 +REASSIGN OWNED BY another_user TO :ROLE_CLUSTER_SUPERUSER; +ERROR: permission denied to reassign objects +REASSIGN OWNED BY another_user TO :ROLE_DEFAULT_PERM_USER; +ERROR: permission denied to reassign objects +\set ON_ERROR_STOP 1 +RESET ROLE; +-- Test that renaming a user changes keeps the job assigned to that user. +ALTER USER another_user RENAME TO renamed_user; +SELECT proc_name, owner FROM _timescaledb_config.bgw_job WHERE id = :job_id; + proc_name | owner +----------------+-------------- + bgw_test_job_1 | renamed_user +(1 row) + +\set VERBOSITY default +\set ON_ERROR_STOP 0 +-- Test that dropping a user owning a job fails. +DROP USER renamed_user; +ERROR: role "renamed_user" cannot be dropped because some objects depend on it +DETAIL: owner of job 1000 +SELECT proc_name, owner FROM _timescaledb_config.bgw_job WHERE id = :job_id; + proc_name | owner +----------------+-------------- + bgw_test_job_1 | renamed_user +(1 row) + +-- Test that re-assigning objects owned by an unknown user still fails +REASSIGN OWNED BY renamed_user, unknown_user TO :ROLE_DEFAULT_PERM_USER; +ERROR: role "unknown_user" does not exist +SELECT proc_name, owner FROM _timescaledb_config.bgw_job WHERE id = :job_id; + proc_name | owner +----------------+-------------- + bgw_test_job_1 | renamed_user +(1 row) + +\set ON_ERROR_STOP 1 +-- Test that reassigning the owned job actually changes the owner of +-- the job. +START TRANSACTION; +REASSIGN OWNED BY renamed_user TO :ROLE_DEFAULT_PERM_USER; +SELECT proc_name, owner FROM _timescaledb_config.bgw_job WHERE id = :job_id; + proc_name | owner +----------------+------------------- + bgw_test_job_1 | default_perm_user +(1 row) + +ROLLBACK; +-- Test that reassigning to postgres works +REASSIGN OWNED BY renamed_user TO :ROLE_CLUSTER_SUPERUSER; +SELECT proc_name, owner FROM _timescaledb_config.bgw_job WHERE id = :job_id; + proc_name | owner +----------------+-------------------- + bgw_test_job_1 | cluster_super_user +(1 row) + +-- Dropping the user now should work. +DROP USER renamed_user; +DELETE FROM _timescaledb_config.bgw_job WHERE id = :job_id; diff --git a/tsl/test/sql/CMakeLists.txt b/tsl/test/sql/CMakeLists.txt index 044770c3b..05c09d952 100644 --- a/tsl/test/sql/CMakeLists.txt +++ b/tsl/test/sql/CMakeLists.txt @@ -6,6 +6,7 @@ set(TEST_FILES agg_partials_pushdown.sql bgw_security.sql bgw_policy.sql + bgw_job_ddl.sql cagg_deprecated_bucket_ng.sql cagg_errors.sql cagg_invalidation.sql diff --git a/tsl/test/sql/bgw_db_scheduler.sql b/tsl/test/sql/bgw_db_scheduler.sql index 1d95492d5..dd562c494 100644 --- a/tsl/test/sql/bgw_db_scheduler.sql +++ b/tsl/test/sql/bgw_db_scheduler.sql @@ -674,32 +674,6 @@ SELECT * FROM sorted_bgw_log; SELECT * FROM _timescaledb_internal.bgw_job_stat; \x off --- Test renaming a user and see that the owner of the job changes. -\c :TEST_DBNAME :ROLE_SUPERUSER -CREATE USER another_user; - -SET ROLE another_user; -SELECT insert_job('another_one', 'bgw_test_job_1', INTERVAL '100ms', INTERVAL '100s', INTERVAL '1s') AS job_id \gset - -SELECT proc_name, owner FROM _timescaledb_config.bgw_job WHERE id = :job_id; - -RESET ROLE; -ALTER USER another_user RENAME TO renamed_user; - -SELECT proc_name, owner FROM _timescaledb_config.bgw_job WHERE id = :job_id; - --- This should fail since the job is dependent on the owner -\set VERBOSITY default -\set ON_ERROR_STOP 0 -DROP USER renamed_user; -\set ON_ERROR_STOP 1 - -DELETE FROM _timescaledb_config.bgw_job WHERE id = :job_id; - --- This should succeed -DROP USER renamed_user; - - -- -- Test without retry -- diff --git a/tsl/test/sql/bgw_job_ddl.sql b/tsl/test/sql/bgw_job_ddl.sql new file mode 100644 index 000000000..2e335d8ac --- /dev/null +++ b/tsl/test/sql/bgw_job_ddl.sql @@ -0,0 +1,74 @@ +-- This file and its contents are licensed under the Timescale License. +-- Please see the included NOTICE for copyright information and +-- LICENSE-TIMESCALE for a copy of the license. + +-- Test for DDL-like functionality + +\c :TEST_DBNAME :ROLE_SUPERUSER +CREATE OR REPLACE FUNCTION insert_job( + application_name NAME, + job_type NAME, + schedule_interval INTERVAL, + max_runtime INTERVAL, + retry_period INTERVAL, + owner regrole DEFAULT CURRENT_ROLE::regrole, + scheduled BOOL DEFAULT true, + fixed_schedule BOOL DEFAULT false +) RETURNS INT LANGUAGE SQL SECURITY DEFINER AS +$$ + INSERT INTO _timescaledb_config.bgw_job(application_name,schedule_interval,max_runtime,max_retries, + retry_period,proc_name,proc_schema,owner,scheduled,fixed_schedule) + VALUES($1,$3,$4,5,$5,$2,'public',$6,$7,$8) RETURNING id; +$$; + +CREATE USER another_user; + +SET ROLE another_user; +SELECT insert_job('another_one', 'bgw_test_job_1', INTERVAL '100ms', INTERVAL '100s', INTERVAL '1s') AS job_id \gset + +SELECT proc_name, owner FROM _timescaledb_config.bgw_job WHERE id = :job_id; + +-- Test that reassigning to another user privileges does not work for +-- a normal user. We test both users with superuser privileges and +-- default permissions. +\set ON_ERROR_STOP 0 +REASSIGN OWNED BY another_user TO :ROLE_CLUSTER_SUPERUSER; +REASSIGN OWNED BY another_user TO :ROLE_DEFAULT_PERM_USER; +\set ON_ERROR_STOP 1 + +RESET ROLE; + +-- Test that renaming a user changes keeps the job assigned to that user. +ALTER USER another_user RENAME TO renamed_user; +SELECT proc_name, owner FROM _timescaledb_config.bgw_job WHERE id = :job_id; + +\set VERBOSITY default +\set ON_ERROR_STOP 0 + +-- Test that dropping a user owning a job fails. +DROP USER renamed_user; +SELECT proc_name, owner FROM _timescaledb_config.bgw_job WHERE id = :job_id; + +-- Test that re-assigning objects owned by an unknown user still fails +REASSIGN OWNED BY renamed_user, unknown_user TO :ROLE_DEFAULT_PERM_USER; +SELECT proc_name, owner FROM _timescaledb_config.bgw_job WHERE id = :job_id; + +\set ON_ERROR_STOP 1 + +-- Test that reassigning the owned job actually changes the owner of +-- the job. +START TRANSACTION; +REASSIGN OWNED BY renamed_user TO :ROLE_DEFAULT_PERM_USER; +SELECT proc_name, owner FROM _timescaledb_config.bgw_job WHERE id = :job_id; +ROLLBACK; + +-- Test that reassigning to postgres works +REASSIGN OWNED BY renamed_user TO :ROLE_CLUSTER_SUPERUSER; +SELECT proc_name, owner FROM _timescaledb_config.bgw_job WHERE id = :job_id; + +-- Dropping the user now should work. +DROP USER renamed_user; + +DELETE FROM _timescaledb_config.bgw_job WHERE id = :job_id; + +