>
> - There is no need for the initial DROP ROLE commands as those already
> get dropped at the end of the tests.
>
Removed.

- There is already rolenames.sql which has a tiny coverage for default
> roles, why not just using it?
>
Moved changes to rolenames.sql.


> +-- should fail because regress_role_nopriv has not CONNECT permission
> on this db
> +SELECT pg_database_size('regression') > 0 AS canread;
> +ERROR:  permission denied for database regression
> +-- should fail because regress_role_nopriv has not CREATE permission on
> this tablespace
> +SELECT pg_tablespace_size('pg_global') > 0 AS canread;
> +ERROR:  permission denied for tablespace pg_global
> Why is that part of a test suite for default roles?
>
Just to check if changes broke something. I haven't find these checks in
other
regress tests. In other way we get only positive tests. If this is not
needed then
should I remove all the checks for regress_role_nopriv role or negative
regress_role_nopriv tests only?

2) is easy to be triggered as a negative test (which fails), less as a
> positive test.  In order to make a positive test failure-proof with
> installcheck you would need to have a parameter which can be changed by
> a superuser at session level which gets updated to a certain value, and
> would fail to show for another user, so you could use one which is
> GUC_SUPERUSER_ONLY and of category PGC_SUSET, like
> session_preload_libraries or dynamic_preload_libraries.  Still that's
> pretty restrictive, and would only test one out of the three code paths
> available.
>
Changed to use session_preload_libraries.

Alexandra
From 9d16cdb419b8cea547a40bf4f188b0bd744de310 Mon Sep 17 00:00:00 2001
From: Alexandra Ryzhevich <aryzhev...@google.com>
Date: Tue, 21 Aug 2018 16:01:30 +0100
Subject: [PATCH 1/1] Add regress tests for default monitoring roles

---
 src/test/regress/expected/rolenames.out | 98 +++++++++++++++++++++++++
 src/test/regress/sql/rolenames.sql      | 70 ++++++++++++++++++
 2 files changed, 168 insertions(+)

diff --git a/src/test/regress/expected/rolenames.out b/src/test/regress/expected/rolenames.out
index 7daba9fc12..67a9cb75a2 100644
--- a/src/test/regress/expected/rolenames.out
+++ b/src/test/regress/expected/rolenames.out
@@ -944,9 +944,107 @@ SELECT proname, proacl FROM pg_proc WHERE proname LIKE 'testagg_';
  testagg9 | 
 (9 rows)
 
+-- DEFAULT MONITORING ROLES
+CREATE ROLE regress_role_haspriv;
+CREATE ROLE regress_role_nopriv;
+-- pg_read_all_stats
+REVOKE CONNECT ON DATABASE regression FROM public;
+GRANT pg_read_all_stats TO regress_role_haspriv;
+SET SESSION AUTHORIZATION regress_role_haspriv;
+-- should not fail because regress_role_haspriv is a member of pg_read_all_stats
+SELECT pg_database_size('regression') > 0 AS canread;
+ canread 
+---------
+ t
+(1 row)
+
+-- should not fail because regress_role_haspriv is a member of pg_read_all_stats
+SELECT pg_tablespace_size('pg_global') > 0 AS canread;
+ canread 
+---------
+ t
+(1 row)
+
+-- should not fail because it is a default tablespace
+SELECT pg_tablespace_size('pg_default') > 0 AS canread;
+ canread 
+---------
+ t
+(1 row)
+
+-- should be true because regress_role_haspriv is member of pg_read_all_stats
+SELECT COUNT(*) = 0 AS haspriv FROM pg_stat_activity WHERE "query" = '<insufficient privilege>';
+ haspriv 
+---------
+ t
+(1 row)
+
+SET SESSION AUTHORIZATION regress_role_nopriv;
+-- should fail because regress_role_nopriv has not CONNECT permission on this db
+SELECT pg_database_size('regression') > 0 AS canread;
+ERROR:  permission denied for database regression
+-- should fail because regress_role_nopriv has not CREATE permission on this tablespace
+SELECT pg_tablespace_size('pg_global') > 0 AS canread;
+ERROR:  permission denied for tablespace pg_global
+-- should not fail because it is a default tablespace
+SELECT pg_tablespace_size('pg_default') > 0 AS canread;
+ canread 
+---------
+ t
+(1 row)
+
+-- should be false because current session belongs to superuser
+SELECT COUNT(*) = 0 AS haspriv FROM pg_stat_activity WHERE "query" = '<insufficient privilege>';
+ haspriv 
+---------
+ f
+(1 row)
+
+RESET SESSION AUTHORIZATION;
+GRANT CONNECT ON DATABASE regression TO public;
+REVOKE pg_read_all_stats FROM regress_role_haspriv;
+-- pg_read_all_settings
+GRANT pg_read_all_settings TO regress_role_haspriv;
+BEGIN;
+-- session_preload_libraries is of PGC_SUSET category
+SET LOCAL session_preload_libraries TO '/tmp/';
+-- vacuum_cost_delay is of PGC_USERSET category
+SET LOCAL vacuum_cost_delay TO 40;
+SET SESSION AUTHORIZATION regress_role_haspriv;
+-- should not fail because regress_role_haspriv is a member of pg_read_all_settings
+SHOW session_preload_libraries;
+ session_preload_libraries 
+---------------------------
+ "/tmp/"
+(1 row)
+
+-- should not fail because this config option has not GUC_SUPERUSER_ONLY flag
+SHOW vacuum_cost_delay;
+ vacuum_cost_delay 
+-------------------
+ 40ms
+(1 row)
+
+SET SESSION AUTHORIZATION regress_role_nopriv;
+-- should not fail because this config option has not GUC_SUPERUSER_ONLY flag
+SHOW vacuum_cost_delay;
+ vacuum_cost_delay 
+-------------------
+ 40ms
+(1 row)
+
+RESET SESSION AUTHORIZATION;
+ROLLBACK;
+SET SESSION AUTHORIZATION regress_role_nopriv;
+-- should fail because regress_role_nopriv is not superuser and is not a member of pg_read_all_settings
+SHOW session_preload_libraries;
+ERROR:  must be superuser or a member of pg_read_all_settings to examine "session_preload_libraries"
+RESET SESSION AUTHORIZATION;
+REVOKE pg_read_all_settings FROM regress_role_haspriv;
 -- clean up
 \c
 DROP SCHEMA test_roles_schema;
 DROP OWNED BY regress_testrol0, "Public", "current_user", regress_testrol1, regress_testrol2, regress_testrolx CASCADE;
 DROP ROLE regress_testrol0, regress_testrol1, regress_testrol2, regress_testrolx;
 DROP ROLE "Public", "None", "current_user", "session_user", "user";
+DROP ROLE regress_role_haspriv, regress_role_nopriv;
diff --git a/src/test/regress/sql/rolenames.sql b/src/test/regress/sql/rolenames.sql
index 5fe8bc8bca..d8bcd24791 100644
--- a/src/test/regress/sql/rolenames.sql
+++ b/src/test/regress/sql/rolenames.sql
@@ -438,6 +438,75 @@ REVOKE ALL PRIVILEGES ON FUNCTION testagg9(int2) FROM "none"; --error
 
 SELECT proname, proacl FROM pg_proc WHERE proname LIKE 'testagg_';
 
+
+-- DEFAULT MONITORING ROLES
+
+CREATE ROLE regress_role_haspriv;
+CREATE ROLE regress_role_nopriv;
+
+-- pg_read_all_stats
+
+REVOKE CONNECT ON DATABASE regression FROM public;
+
+GRANT pg_read_all_stats TO regress_role_haspriv;
+
+SET SESSION AUTHORIZATION regress_role_haspriv;
+-- should not fail because regress_role_haspriv is a member of pg_read_all_stats
+SELECT pg_database_size('regression') > 0 AS canread;
+-- should not fail because regress_role_haspriv is a member of pg_read_all_stats
+SELECT pg_tablespace_size('pg_global') > 0 AS canread;
+-- should not fail because it is a default tablespace
+SELECT pg_tablespace_size('pg_default') > 0 AS canread;
+-- should be true because regress_role_haspriv is member of pg_read_all_stats
+SELECT COUNT(*) = 0 AS haspriv FROM pg_stat_activity WHERE "query" = '<insufficient privilege>';
+
+SET SESSION AUTHORIZATION regress_role_nopriv;
+-- should fail because regress_role_nopriv has not CONNECT permission on this db
+SELECT pg_database_size('regression') > 0 AS canread;
+-- should fail because regress_role_nopriv has not CREATE permission on this tablespace
+SELECT pg_tablespace_size('pg_global') > 0 AS canread;
+-- should not fail because it is a default tablespace
+SELECT pg_tablespace_size('pg_default') > 0 AS canread;
+-- should be false because current session belongs to superuser
+SELECT COUNT(*) = 0 AS haspriv FROM pg_stat_activity WHERE "query" = '<insufficient privilege>';
+
+RESET SESSION AUTHORIZATION;
+
+GRANT CONNECT ON DATABASE regression TO public;
+
+REVOKE pg_read_all_stats FROM regress_role_haspriv;
+
+-- pg_read_all_settings
+
+GRANT pg_read_all_settings TO regress_role_haspriv;
+
+BEGIN;
+-- session_preload_libraries is of PGC_SUSET category
+SET LOCAL session_preload_libraries TO '/tmp/';
+-- vacuum_cost_delay is of PGC_USERSET category
+SET LOCAL vacuum_cost_delay TO 40;
+
+SET SESSION AUTHORIZATION regress_role_haspriv;
+-- should not fail because regress_role_haspriv is a member of pg_read_all_settings
+SHOW session_preload_libraries;
+-- should not fail because this config option has not GUC_SUPERUSER_ONLY flag
+SHOW vacuum_cost_delay;
+
+SET SESSION AUTHORIZATION regress_role_nopriv;
+-- should not fail because this config option has not GUC_SUPERUSER_ONLY flag
+SHOW vacuum_cost_delay;
+
+RESET SESSION AUTHORIZATION;
+ROLLBACK;
+
+SET SESSION AUTHORIZATION regress_role_nopriv;
+-- should fail because regress_role_nopriv is not superuser and is not a member of pg_read_all_settings
+SHOW session_preload_libraries;
+
+RESET SESSION AUTHORIZATION;
+
+REVOKE pg_read_all_settings FROM regress_role_haspriv;
+
 -- clean up
 \c
 
@@ -445,3 +514,4 @@ DROP SCHEMA test_roles_schema;
 DROP OWNED BY regress_testrol0, "Public", "current_user", regress_testrol1, regress_testrol2, regress_testrolx CASCADE;
 DROP ROLE regress_testrol0, regress_testrol1, regress_testrol2, regress_testrolx;
 DROP ROLE "Public", "None", "current_user", "session_user", "user";
+DROP ROLE regress_role_haspriv, regress_role_nopriv;
-- 
2.18.0.865.gffc8e1a3cd6-goog

Reply via email to