Copy recreated object permissions on update

Tables, indexes, and sequences that are recreated as part of an update
does not propagate permissions to the recreated object. This commit
fixes that by saving away the permissions in `pg_class` temporarily and
then copying them back into the `pg_class` table.

If internal objects are created or re-created, they get the wrong
initial privileges, which result in privileges not being dumped when
using `pg_dump`. Save away the privileges before starting the update
and restore them afterwards to make sure that the privileges are
maintained over the update.

For new objects, we use the initial privileges of the `chunk` metadata
table, which should always have correct initial privileges.

Fixes #3078

scripts/test_functions.inc

@@ -6,18 +6,23 @@ SCRIPT_DIR=$(dirname $0)
 #
 # Options:
 #   -r    Run repair tests as a separate pass (optional)
+#   -k    Keep temporary directory
 #   -vN   Use version N of the update tests (required)
 run_tests() (
     export TEST_VERSION
     export TEST_REPAIR=false
-
+    export DO_CLEANUP=true
+    
     OPTIND=1
-    while getopts "v:r" opt;
+    while getopts "kv:r" opt;
     do
         case $opt in
              v)
                  TEST_VERSION=v$OPTARG
                  ;;
+	     k)
+		 DO_CLEANUP=false
+		 ;;
              r)
                  TEST_REPAIR=true
                  ;;

scripts/test_update_from_tag.sh

@@ -16,7 +16,7 @@ UPDATE_FROM_IMAGE=${UPDATE_FROM_IMAGE:-timescale/timescaledb}
 UPDATE_FROM_TAG=${UPDATE_FROM_TAG:-0.1.0}
 UPDATE_TO_IMAGE=${UPDATE_TO_IMAGE:-update_test}
 UPDATE_TO_TAG=${UPDATE_TO_TAG:-${GIT_ID}}
-DO_CLEANUP=true
+DO_CLEANUP=${DO_CLEANUP:-true}
 PGOPTS="-v TEST_VERSION=${TEST_VERSION} -v TEST_REPAIR=${TEST_REPAIR} -v WITH_SUPERUSER=${WITH_SUPERUSER}"
 
 # PID of the current shell
@@ -30,18 +30,11 @@ CONTAINER_CLEAN_RERUN=timescaledb-clean-rerun-${PID}
 
 export PG_VERSION
 
-while getopts "d" opt;
-do
-    case $opt in
-        d)
-            DO_CLEANUP=false
-            echo "!!Debug mode: Containers and temporary directory will be left on disk"
-            echo
-            ;;
-    esac
-done
-
-shift $((OPTIND-1))
+if [[ "$DO_CLEANUP" = "false" ]]; then
+    echo "!!Debug mode: Containers and temporary directory will be left on disk"
+else
+    echo "!!Containers and temporary directory will be cleaned up"
+fi
 
 trap cleanup EXIT
 

scripts/test_updates_pg11.sh

@@ -11,16 +11,16 @@ source ${SCRIPT_DIR}/test_functions.inc
 #    2.0.0-rc1 and 2.0.0-rc2
 #
 # Please extend this list if repairs are needed between more steps.
-run_tests -r -v2 \
+run_tests "$@" -r -v2 \
           1.1.0-pg11 1.1.1-pg11 1.2.0-pg11 1.2.1-pg11 1.2.2-pg11
-run_tests -r -v4 \
+run_tests "$@" -r -v4 \
           1.3.0-pg11 1.3.1-pg11 1.3.2-pg11 1.4.0-pg11 1.4.1-pg11 1.4.2-pg11
-run_tests -r -v5 \
+run_tests "$@" -r -v5 \
           1.5.0-pg11 1.5.1-pg11 1.6.0-pg11 1.6.1-pg11
-run_tests -r -v6 \
+run_tests "$@" -r -v6 \
           1.7.0-pg11 1.7.1-pg11 1.7.2-pg11 1.7.3-pg11 1.7.4-pg11 1.7.5-pg11
-run_tests -r -v7 \
+run_tests "$@" -r -v7 \
           2.0.0-rc1-pg11 
-run_tests -v7 \
+run_tests "$@" -v7 \
           2.0.0-rc2-pg11 2.0.0-rc3-pg11 2.0.0-rc4-pg11 2.0.0-pg11 2.0.1-pg11 2.0.2-pg11 2.1.0-pg11 \
           2.1.1-pg11 2.2.0-pg11

scripts/test_updates_pg12.sh

@@ -11,10 +11,10 @@ source ${SCRIPT_DIR}/test_functions.inc
 #    2.0.0-rc1 and 2.0.0-rc2
 #
 # Please extend this list if repairs are needed between more steps.
-run_tests -r -v6 \
+run_tests "$@" -r -v6 \
           1.7.0-pg12 1.7.1-pg12 1.7.2-pg12 1.7.3-pg12 1.7.4-pg12 1.7.5-pg12
-run_tests -r -v7 \
+run_tests "$@" -r -v7 \
           2.0.0-rc1-pg12 
-run_tests -v7 \
+run_tests "$@" -v7 \
           2.0.0-rc2-pg12 2.0.0-rc3-pg12 2.0.0-rc4-pg12 2.0.0-pg12 2.0.1-pg12 2.0.2-pg12 2.1.0-pg12 \
           2.1.1-pg12 2.2.0-pg12

sql/updates/post-update.sql

@@ -30,3 +30,46 @@ $$;
 
 -- can only be dropped after views have been rebuilt
 DROP FUNCTION IF EXISTS _timescaledb_internal.cagg_watermark(oid);
+
+-- For objects that are newly created, we need to set the initprivs to
+-- the initprivs for some table that was created in the installation
+-- of the TimescaleDB extension and not as part of any update.
+--
+-- We chose the "chunk" catalog table for this since that is created
+-- in the first version of TimescaleDB and should have the correct
+-- initprivs, but we could use any other table that existed in the
+-- first installation.
+INSERT INTO saved_privs
+     SELECT nspname, relname, relacl,
+ 	    (SELECT tmpini FROM saved_privs
+	      WHERE tmpnsp = '_timescaledb_catalog' AND tmpname = 'chunk')
+       FROM pg_class JOIN pg_namespace ns ON ns.oid = relnamespace
+		LEFT JOIN saved_privs ON tmpnsp = nspname AND tmpname = relname
+      WHERE nspname IN ('_timescaledb_catalog', '_timescaledb_config')
+ 	 OR nspname = '_timescaledb_internal'
+        AND relname IN ('hypertable_chunk_local_size', 'compressed_chunk_stats',
+                        'bgw_job_stat', 'bgw_policy_chunk_stats')
+ON CONFLICT DO NOTHING;
+
+-- We can now copy back saved initprivs.
+WITH to_update AS (
+     SELECT objoid, tmpini
+     FROM pg_class cl JOIN pg_namespace ns ON ns.oid = relnamespace
+   		      JOIN pg_init_privs ip ON ip.objoid = cl.oid
+		      JOIN saved_privs ON tmpnsp = nspname AND tmpname = relname)
+UPDATE pg_init_privs
+   SET initprivs = tmpini
+  FROM to_update
+ WHERE to_update.objoid = pg_init_privs.objoid
+   AND classoid = 'pg_class'::regclass;
+
+-- Can only restore permissions on views after they have been rebuilt,
+-- so we restore for all types of objects here.
+WITH to_update AS (
+     SELECT cl.oid, tmpacl
+     FROM pg_class cl JOIN pg_namespace ns ON ns.oid = relnamespace
+                      JOIN saved_privs ON tmpnsp = nspname AND tmpname = relname)
+UPDATE pg_class cl SET relacl = tmpacl
+  FROM to_update WHERE cl.oid = to_update.oid;
+
+DROP TABLE saved_privs;

sql/updates/pre-update.sql

@@ -27,3 +27,25 @@ AS '@LOADER_PATHNAME@', 'ts_bgw_db_workers_restart'
 LANGUAGE C VOLATILE;
 
 SELECT _timescaledb_internal.restart_background_workers();
+
+-- Table for ACL and initprivs of tables. 
+CREATE TABLE saved_privs(
+       tmpnsp name,
+       tmpname name,
+       tmpacl aclitem[],
+       tmpini aclitem[],
+       UNIQUE (tmpnsp, tmpname));
+
+-- We save away both the ACL and the initprivs for all tables and
+-- views in the extension (but not for chunks and internal objects) so
+-- that we can restore them to the proper state after the update.
+INSERT INTO saved_privs
+SELECT nspname, relname, relacl, initprivs
+  FROM pg_class cl JOIN pg_namespace ns ON ns.oid = relnamespace
+                   JOIN pg_init_privs ip ON ip.objoid = cl.oid
+WHERE nspname IN ('_timescaledb_catalog', '_timescaledb_config')
+   OR nspname = '_timescaledb_internal'
+  AND relname IN ('hypertable_chunk_local_size', 'compressed_chunk_stats',
+      	      	  'bgw_job_stat', 'bgw_policy_chunk_stats');
+
+

test/sql/updates/post.catalog.sql

@@ -10,10 +10,21 @@
 \d+ _timescaledb_catalog.chunk_index
 \d+ _timescaledb_catalog.tablespace
 
-\z _timescaledb_cache.*
-\z _timescaledb_catalog.*
-\z _timescaledb_config.*
-\z _timescaledb_internal.*
+SELECT nspname AS Schema,
+       relname AS Name,
+       unnest(relacl)::text as ACL
+FROM pg_class JOIN pg_namespace ns ON relnamespace = ns.oid
+WHERE nspname IN ('_timescaledb_catalog', '_timescaledb_config')
+ORDER BY Schema, Name, ACL;
+
+SELECT nspname AS schema,
+       relname AS name,
+       unnest(initprivs)::text AS initpriv
+FROM pg_class cl JOIN pg_namespace ns ON ns.oid = relnamespace
+            LEFT JOIN pg_init_privs ON objoid = cl.oid
+WHERE classoid = 'pg_class'::regclass
+  AND nspname IN ('_timescaledb_catalog', '_timescaledb_config')
+ORDER BY schema, name, initpriv;
 
 \di _timescaledb_catalog.*
 \ds+ _timescaledb_catalog.*;

test/sql/updates/post.continuous_aggs.sql

@@ -26,7 +26,7 @@ SELECT * FROM mat_before ORDER BY bucket, location;
 
 -- Output the ACLs for each internal cagg object
 SELECT cl.oid::regclass::text AS reloid,
-       relacl
+       unnest(relacl)::text AS relacl
 FROM _timescaledb_catalog.continuous_agg ca
 JOIN _timescaledb_catalog.hypertable h
 ON (ca.mat_hypertable_id = h.id)
@@ -34,19 +34,19 @@ JOIN pg_class cl
 ON (cl.oid IN (format('%I.%I', h.schema_name, h.table_name)::regclass,
                format('%I.%I', direct_view_schema, direct_view_name)::regclass,
                format('%I.%I', partial_view_schema, partial_view_name)::regclass))
-ORDER BY reloid;
+ORDER BY reloid, relacl;
 
 -- Output ACLs for chunks on materialized hypertables
 SELECT inhparent::regclass::text AS parent,
        cl.oid::regclass::text AS chunk,
-       relacl
+       unnest(relacl)::text AS acl
 FROM _timescaledb_catalog.continuous_agg ca
 JOIN _timescaledb_catalog.hypertable h
 ON (ca.mat_hypertable_id = h.id)
 JOIN pg_inherits inh ON (inh.inhparent = format('%I.%I', h.schema_name, h.table_name)::regclass)
 JOIN pg_class cl
 ON (cl.oid = inh.inhrelid)
-ORDER BY parent, chunk;
+ORDER BY parent, chunk, acl;
 
 -- Verify privileges on internal cagg objects.  The privileges on the
 -- materialized hypertable, partial view, and direct view should match

test/sql/updates/setup.catalog.sql

@@ -0,0 +1,23 @@
+-- This file and its contents are licensed under the Apache License 2.0.
+-- Please see the included NOTICE for copyright information and
+-- LICENSE-APACHE for a copy of the license.
+
+-- Catalog tables are occationally rewritten as part of updates, so
+-- this is to test that privileges are maintained over updates of the
+-- extension. We could verify that other properties (e.g., comments)
+-- are maintained here as well, but this is not something we use right
+-- now.
+--
+-- We do not alter the privileges on _timescaledb_internal since this
+-- affects both internal objects and two tables that are metadata
+-- placed in the _timescaledb_internal schema.
+
+GRANT SELECT ON ALL TABLES IN SCHEMA _timescaledb_catalog TO tsdbadmin;
+GRANT SELECT ON ALL TABLES IN SCHEMA _timescaledb_config TO tsdbadmin;
+GRANT SELECT ON ALL SEQUENCES IN SCHEMA _timescaledb_catalog TO tsdbadmin;
+GRANT SELECT ON ALL SEQUENCES IN SCHEMA _timescaledb_config TO tsdbadmin;
+
+ALTER DEFAULT PRIVILEGES IN SCHEMA _timescaledb_catalog
+      GRANT SELECT ON tables TO tsdbadmin;
+ALTER DEFAULT PRIVILEGES IN SCHEMA _timescaledb_config
+      GRANT SELECT ON tables TO tsdbadmin;

test/sql/updates/setup.roles.sql

@@ -3,3 +3,4 @@
 -- LICENSE-APACHE for a copy of the license.
 
 CREATE ROLE cagg_user;
+CREATE USER tsdbadmin;

test/sql/updates/setup.v2.sql

@@ -2,6 +2,7 @@
 -- Please see the included NOTICE for copyright information and
 -- LICENSE-APACHE for a copy of the license.
 
+\ir setup.catalog.sql
 \ir setup.bigint.sql
 \ir setup.constraints.sql
 \ir setup.insert_bigint.v2.sql