Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Copy recreated object permissions on update #3109

Merged
merged 1 commit into from Apr 26, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
9 changes: 7 additions & 2 deletions scripts/test_functions.inc
Expand Up @@ -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
;;
Expand Down
19 changes: 6 additions & 13 deletions scripts/test_update_from_tag.sh
Expand Up @@ -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
Expand All @@ -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

Expand Down
12 changes: 6 additions & 6 deletions scripts/test_updates_pg11.sh
Expand Up @@ -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
6 changes: 3 additions & 3 deletions scripts/test_updates_pg12.sh
Expand Up @@ -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
43 changes: 43 additions & 0 deletions sql/updates/post-update.sql
Expand Up @@ -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
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you rewrite the comment a bit so that it is clear 1) we choose chunk table (but it could be any other catalog table
2) The comment about first version can be misinterpreted as version 1.0, and I did. Instead reword it as, -- the first time Timescaledb extension was installed --

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Updated the comment

-- 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;
22 changes: 22 additions & 0 deletions sql/updates/pre-update.sql
Expand Up @@ -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');


19 changes: 15 additions & 4 deletions test/sql/updates/post.catalog.sql
Expand Up @@ -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,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should check initprivs as well? Or is that being done some other place?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good point. No, this is probably the best place. Added a check.

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.*;
Expand Down
8 changes: 4 additions & 4 deletions test/sql/updates/post.continuous_aggs.sql
Expand Up @@ -26,27 +26,27 @@ 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)
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
Expand Down
23 changes: 23 additions & 0 deletions 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;
1 change: 1 addition & 0 deletions test/sql/updates/setup.roles.sql
Expand Up @@ -3,3 +3,4 @@
-- LICENSE-APACHE for a copy of the license.

CREATE ROLE cagg_user;
CREATE USER tsdbadmin;
1 change: 1 addition & 0 deletions test/sql/updates/setup.v2.sql
Expand Up @@ -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
Expand Down