Skip to content

Commit

Permalink
Address final reviews
Browse files Browse the repository at this point in the history
  • Loading branch information
@naisila
naisila committed Nov 16, 2023
1 parent cdc97f0 commit c36a71a
Show file tree
Hide file tree
Showing 3 changed files with 54 additions and 54 deletions.
2 changes: 0 additions & 2 deletions src/backend/distributed/shared_library_init.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -582,8 +582,6 @@ _PG_init(void)
*/
if (RunningUnderCitusTestSuite)
{
register_label_provider("citus_tests_label_provider",
citus_test_object_relabel);
register_label_provider("citus '!tests_label_provider",
citus_test_object_relabel);
}
Expand Down
81 changes: 41 additions & 40 deletions src/test/regress/expected/seclabel.out
View file
Original file line number Diff line number Diff line change
Expand Up @@ -15,15 +15,15 @@ SELECT citus_remove_node('localhost', :worker_2_port);
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';
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';
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
node_type | result
---------------------------------------------------------------------
coordinator | {"label": "citus_unclassified", "objtype": "role", "provider": "citus_tests_label_provider"}
coordinator | {"label": "citus_unclassified", "objtype": "role", "provider": "citus '!tests_label_provider"}
worker_1 |
(2 rows)

Expand All @@ -43,60 +43,60 @@ 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 FOR citus_tests_label_provider ON TABLE a IS 'citus_classified';
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 FOR citus_tests_label_provider ON FUNCTION notice IS 'citus_unclassified';
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 FOR citus_tests_label_provider ON VIEW v_dist IS 'citus_classified';
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
node_type | result
---------------------------------------------------------------------
coordinator | {"label": "citus_classified", "objtype": "table", "provider": "citus_tests_label_provider"}
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
node_type | result
---------------------------------------------------------------------
coordinator | {"label": "citus_unclassified", "objtype": "function", "provider": "citus_tests_label_provider"}
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
node_type | result
---------------------------------------------------------------------
coordinator | {"label": "citus_classified", "objtype": "view", "provider": "citus_tests_label_provider"}
coordinator | {"label": "citus_classified", "objtype": "view", "provider": "citus '!tests_label_provider"}
worker_1 |
(2 rows)

\c - - - :worker_1_port
SECURITY LABEL FOR citus_tests_label_provider ON TABLE a IS 'citus_classified';
SECURITY LABEL FOR citus_tests_label_provider ON FUNCTION notice IS 'citus_unclassified';
SECURITY LABEL FOR citus_tests_label_provider ON VIEW v_dist IS 'citus_classified';
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
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"}
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
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"}
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
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"}
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;
Expand All @@ -105,30 +105,31 @@ 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%';
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'
-- 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 for citus_tests_label_provider ON ROLE user1 IS NULL;
NOTICE: issuing SECURITY LABEL FOR citus_tests_label_provider ON ROLE user1 IS NULL
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 for citus_tests_label_provider ON ROLE user1 IS 'citus_unclassified';
NOTICE: issuing SECURITY LABEL FOR citus_tests_label_provider ON ROLE user1 IS 'citus_unclassified'
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';
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
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"}
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;
Expand All @@ -142,7 +143,7 @@ SELECT node_type, result FROM get_citus_tests_label_provider_labels('"user 2"')
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'
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
Expand All @@ -152,11 +153,11 @@ DETAIL: on server postgres@localhost:xxxxx connectionId: xxxxxxx
(1 row)

SELECT node_type, result FROM get_citus_tests_label_provider_labels('user1') ORDER BY node_type;
node_type | result
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"}
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;
Expand Down
25 changes: 13 additions & 12 deletions src/test/regress/sql/seclabel.sql
View file
Original file line number Diff line number Diff line change
Expand Up @@ -13,11 +13,11 @@ 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';
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';
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;
Expand All @@ -33,18 +33,18 @@ CREATE VIEW v_dist AS SELECT * FROM a;
CREATE FUNCTION notice(text) RETURNS void LANGUAGE plpgsql AS $$
BEGIN RAISE NOTICE '%', $1; END; $$;

SECURITY LABEL FOR citus_tests_label_provider ON TABLE a IS 'citus_classified';
SECURITY LABEL FOR citus_tests_label_provider ON FUNCTION notice IS 'citus_unclassified';
SECURITY LABEL FOR citus_tests_label_provider ON VIEW v_dist IS 'citus_classified';
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 FOR citus_tests_label_provider ON TABLE a IS 'citus_classified';
SECURITY LABEL FOR citus_tests_label_provider ON FUNCTION notice IS 'citus_unclassified';
SECURITY LABEL FOR citus_tests_label_provider ON VIEW v_dist IS 'citus_classified';
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;
Expand All @@ -58,14 +58,15 @@ DROP FUNCTION notice;
SET citus.log_remote_commands TO on;
SET citus.grep_remote_commands = '%SECURITY LABEL%';

SECURITY LABEL for citus_tests_label_provider ON ROLE user1 IS 'citus_classified';
SECURITY LABEL for citus_tests_label_provider ON ROLE user1 IS NULL;
SECURITY LABEL for citus_tests_label_provider ON ROLE user1 IS 'citus_unclassified';
-- 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';
SECURITY LABEL for "citus '!tests_label_provider" ON ROLE user1 IS 'citus ''!unclassified';

\c - - - :master_port
RESET citus.log_remote_commands;
Expand Down

0 comments on commit c36a71a

Please sign in to comment.