Skip to content

Commit

Permalink
Removes granted by statement to fix flaky test errors (#7526)
Browse files Browse the repository at this point in the history 
Fix for the #7519
In metadata sync phase, grant statements for roles are being fetched and
propagated from catalog tables.
However, in some cases grant .. with admin option clauses executes after
the granted by statements which causes #7519 error.
We will fix this issue with the grantor propagation task in the project
  • Loading branch information
@gurkanindibay
gurkanindibay authored Feb 21, 2024
1 parent 683e10a commit 211415d
Show file tree
Hide file tree
Showing 2 changed files with 18 additions and 8 deletions.
17 changes: 11 additions & 6 deletions src/test/regress/expected/metadata_sync_from_non_maindb.out
View file
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,12 @@ select 1 from citus_remove_node('localhost', :worker_2_port);

\c metadata_sync_2pc_db
grant "grant_role2pc'_user1","grant_role2pc'_user2" to "grant_role2pc'_user3" WITH ADMIN OPTION;
grant "grant_role2pc'_user1","grant_role2pc'_user2" to grant_role2pc_user4,grant_role2pc_user5 granted by "grant_role2pc'_user3";
-- This section was originally testing a scenario where a user with the 'admin option' grants the same role to another user, also with the 'admin option'.
-- However, we encountered inconsistent errors because the 'admin option' grant is executed after the grant below.
-- Once we establish the correct order of granting, we will reintroduce the 'granted by' clause.
-- For now, we are commenting out the grant below that includes 'granted by', and instead, we are adding a grant without the 'granted by' clause.
-- grant "grant_role2pc'_user1","grant_role2pc'_user2" to grant_role2pc_user4,grant_role2pc_user5 granted by "grant_role2pc'_user3";
grant "grant_role2pc'_user1","grant_role2pc'_user2" to grant_role2pc_user4,grant_role2pc_user5;
--test for grant on database
\c metadata_sync_2pc_db - - :master_port
grant create on database metadata_sync_2pc_db to "grant_role2pc'_user1";
Expand Down Expand Up @@ -76,11 +81,11 @@ FROM (
order by member::regrole::text
) t
$$);
result
result
---------------------------------------------------------------------
[{"member":"\"grant_role2pc'_user3\"","role":"\"grant_role2pc'_user1\"","grantor":"postgres","admin_option":true},{"member":"\"grant_role2pc'_user3\"","role":"\"grant_role2pc'_user2\"","grantor":"postgres","admin_option":true},{"member":"grant_role2pc_user4","role":"\"grant_role2pc'_user1\"","grantor":"\"grant_role2pc'_user3\"","admin_option":false},{"member":"grant_role2pc_user4","role":"\"grant_role2pc'_user2\"","grantor":"\"grant_role2pc'_user3\"","admin_option":false},{"member":"grant_role2pc_user5","role":"\"grant_role2pc'_user1\"","grantor":"\"grant_role2pc'_user3\"","admin_option":false},{"member":"grant_role2pc_user5","role":"\"grant_role2pc'_user2\"","grantor":"\"grant_role2pc'_user3\"","admin_option":false}]
[{"member":"\"grant_role2pc'_user3\"","role":"\"grant_role2pc'_user1\"","grantor":"postgres","admin_option":true},{"member":"\"grant_role2pc'_user3\"","role":"\"grant_role2pc'_user2\"","grantor":"postgres","admin_option":true},{"member":"grant_role2pc_user4","role":"\"grant_role2pc'_user1\"","grantor":"\"grant_role2pc'_user3\"","admin_option":false},{"member":"grant_role2pc_user4","role":"\"grant_role2pc'_user2\"","grantor":"\"grant_role2pc'_user3\"","admin_option":false},{"member":"grant_role2pc_user5","role":"\"grant_role2pc'_user1\"","grantor":"\"grant_role2pc'_user3\"","admin_option":false},{"member":"grant_role2pc_user5","role":"\"grant_role2pc'_user2\"","grantor":"\"grant_role2pc'_user3\"","admin_option":false}]
[{"member":"\"grant_role2pc'_user3\"","role":"\"grant_role2pc'_user1\"","grantor":"postgres","admin_option":true},{"member":"\"grant_role2pc'_user3\"","role":"\"grant_role2pc'_user2\"","grantor":"postgres","admin_option":true},{"member":"grant_role2pc_user4","role":"\"grant_role2pc'_user1\"","grantor":"\"grant_role2pc'_user3\"","admin_option":false},{"member":"grant_role2pc_user4","role":"\"grant_role2pc'_user2\"","grantor":"\"grant_role2pc'_user3\"","admin_option":false},{"member":"grant_role2pc_user5","role":"\"grant_role2pc'_user1\"","grantor":"\"grant_role2pc'_user3\"","admin_option":false},{"member":"grant_role2pc_user5","role":"\"grant_role2pc'_user2\"","grantor":"\"grant_role2pc'_user3\"","admin_option":false}]
[{"member":"\"grant_role2pc'_user3\"","role":"\"grant_role2pc'_user1\"","grantor":"postgres","admin_option":true},{"member":"\"grant_role2pc'_user3\"","role":"\"grant_role2pc'_user2\"","grantor":"postgres","admin_option":true},{"member":"grant_role2pc_user4","role":"\"grant_role2pc'_user1\"","grantor":"postgres","admin_option":false},{"member":"grant_role2pc_user4","role":"\"grant_role2pc'_user2\"","grantor":"postgres","admin_option":false},{"member":"grant_role2pc_user5","role":"\"grant_role2pc'_user1\"","grantor":"postgres","admin_option":false},{"member":"grant_role2pc_user5","role":"\"grant_role2pc'_user2\"","grantor":"postgres","admin_option":false}]
[{"member":"\"grant_role2pc'_user3\"","role":"\"grant_role2pc'_user1\"","grantor":"postgres","admin_option":true},{"member":"\"grant_role2pc'_user3\"","role":"\"grant_role2pc'_user2\"","grantor":"postgres","admin_option":true},{"member":"grant_role2pc_user4","role":"\"grant_role2pc'_user1\"","grantor":"postgres","admin_option":false},{"member":"grant_role2pc_user4","role":"\"grant_role2pc'_user2\"","grantor":"postgres","admin_option":false},{"member":"grant_role2pc_user5","role":"\"grant_role2pc'_user1\"","grantor":"postgres","admin_option":false},{"member":"grant_role2pc_user5","role":"\"grant_role2pc'_user2\"","grantor":"postgres","admin_option":false}]
[{"member":"\"grant_role2pc'_user3\"","role":"\"grant_role2pc'_user1\"","grantor":"postgres","admin_option":true},{"member":"\"grant_role2pc'_user3\"","role":"\"grant_role2pc'_user2\"","grantor":"postgres","admin_option":true},{"member":"grant_role2pc_user4","role":"\"grant_role2pc'_user1\"","grantor":"postgres","admin_option":false},{"member":"grant_role2pc_user4","role":"\"grant_role2pc'_user2\"","grantor":"postgres","admin_option":false},{"member":"grant_role2pc_user5","role":"\"grant_role2pc'_user1\"","grantor":"postgres","admin_option":false},{"member":"grant_role2pc_user5","role":"\"grant_role2pc'_user2\"","grantor":"postgres","admin_option":false}]
(3 rows)

select check_database_privileges('grant_role2pc''_user1','metadata_sync_2pc_db',ARRAY['CREATE']);
Expand Down Expand Up @@ -117,7 +122,7 @@ select check_database_privileges('grant_role2pc''_user3','metadata_sync_2pc_db',
(12 rows)

\c metadata_sync_2pc_db
revoke "grant_role2pc'_user1","grant_role2pc'_user2" from grant_role2pc_user4,grant_role2pc_user5 granted by "grant_role2pc'_user3";
revoke "grant_role2pc'_user1","grant_role2pc'_user2" from grant_role2pc_user4,grant_role2pc_user5 ;
revoke admin option for "grant_role2pc'_user1","grant_role2pc'_user2" from "grant_role2pc'_user3";
revoke "grant_role2pc'_user1","grant_role2pc'_user2" from "grant_role2pc'_user3";
revoke ALL on database metadata_sync_2pc_db from "grant_role2pc'_user3";
Expand Down
9 changes: 7 additions & 2 deletions src/test/regress/sql/metadata_sync_from_non_maindb.sql
View file
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,12 @@ select 1 from citus_remove_node('localhost', :worker_2_port);

\c metadata_sync_2pc_db
grant "grant_role2pc'_user1","grant_role2pc'_user2" to "grant_role2pc'_user3" WITH ADMIN OPTION;
grant "grant_role2pc'_user1","grant_role2pc'_user2" to grant_role2pc_user4,grant_role2pc_user5 granted by "grant_role2pc'_user3";
-- This section was originally testing a scenario where a user with the 'admin option' grants the same role to another user, also with the 'admin option'.
-- However, we encountered inconsistent errors because the 'admin option' grant is executed after the grant below.
-- Once we establish the correct order of granting, we will reintroduce the 'granted by' clause.
-- For now, we are commenting out the grant below that includes 'granted by', and instead, we are adding a grant without the 'granted by' clause.
-- grant "grant_role2pc'_user1","grant_role2pc'_user2" to grant_role2pc_user4,grant_role2pc_user5 granted by "grant_role2pc'_user3";
grant "grant_role2pc'_user1","grant_role2pc'_user2" to grant_role2pc_user4,grant_role2pc_user5;

--test for grant on database
\c metadata_sync_2pc_db - - :master_port
Expand Down Expand Up @@ -52,7 +57,7 @@ select check_database_privileges('grant_role2pc''_user2','metadata_sync_2pc_db',
select check_database_privileges('grant_role2pc''_user3','metadata_sync_2pc_db',ARRAY['CREATE','CONNECT','TEMP','TEMPORARY']);

\c metadata_sync_2pc_db
revoke "grant_role2pc'_user1","grant_role2pc'_user2" from grant_role2pc_user4,grant_role2pc_user5 granted by "grant_role2pc'_user3";
revoke "grant_role2pc'_user1","grant_role2pc'_user2" from grant_role2pc_user4,grant_role2pc_user5 ;

revoke admin option for "grant_role2pc'_user1","grant_role2pc'_user2" from "grant_role2pc'_user3";

Expand Down

0 comments on commit 211415d

Please sign in to comment.