From 19e1feeff54d5d9448a5e8a827591aac7c7cdf7f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Erik=20Nordstr=C3=B6m?= Date: Wed, 20 Jan 2021 11:26:11 +0100 Subject: [PATCH] Fix continuous aggregate privileges during update Copy ACL privileges (grants) from the query view (user-facing object) to the internal objects (e.g., materialized hypertable, direct view, and partial view) when updating the extension to the new version. A previous change added such propagation of privileges when executing `GRANT` statements, but didn't apply it retrospectively to the internal objects of existing continuous aggregates. Having the right permissions on internal objects is also necessary for the watermark function used by real-time aggregation since it queries the materialized hypertable directly. The update script copies the ACL information from the user-facing view of every continuous aggregate to its internal objects (including the materialized hypertable and its chunks). This is done by direct insert into `pg_class` instead of executing a `GRANT` statement in the update script, since the latter will record the grant/ACL as an init privilege (i.e., the system believes the GRANT is for an extension object). The init privilege will prevent this ACL from being included in future dump files, since `pg_dump` only includes non-init privileges as it believes such privileges will be recreated with the extension. Fixes #2825 --- CHANGELOG.md | 1 + scripts/test_update_from_tag.sh | 10 ++- sql/updates/latest-dev.sql | 48 +++++++++++++ test/sql/updates/post.continuous_aggs.sql | 69 +++++++++++++++++++ test/sql/updates/setup.continuous_aggs.sql | 2 + test/sql/updates/setup.continuous_aggs.v2.sql | 9 ++- test/sql/updates/setup.roles.sql | 5 ++ 7 files changed, 142 insertions(+), 2 deletions(-) create mode 100644 test/sql/updates/setup.roles.sql diff --git a/CHANGELOG.md b/CHANGELOG.md index c424642b4bf..53b7f4b0ca1 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,6 +8,7 @@ accidentally triggering the load of a previous DB version.** **Bugfixes** * #2842 Do not mark job as started when seting next_start field +* #2845 Fix continuous aggregate privileges during upgrade **Minor features** * #2736 Support adding columns to hypertables with compression enabled diff --git a/scripts/test_update_from_tag.sh b/scripts/test_update_from_tag.sh index 30aa3c4a190..25ab178522a 100755 --- a/scripts/test_update_from_tag.sh +++ b/scripts/test_update_from_tag.sh @@ -172,6 +172,13 @@ docker_run ${CONTAINER_ORIG} ${UPDATE_FROM_IMAGE}:${UPDATE_FROM_TAG} docker_run ${CONTAINER_CLEAN_RESTORE} ${UPDATE_TO_IMAGE}:${UPDATE_TO_TAG} docker_run ${CONTAINER_CLEAN_RERUN} ${UPDATE_TO_IMAGE}:${UPDATE_TO_TAG} +# Create roles for test. Roles must be created outside of regular +# setup scripts; they must be added separately to each instance since +# roles are not dumped by pg_dump. +docker_pgscript ${CONTAINER_ORIG} /src/test/sql/updates/setup.roles.sql "postgres" +docker_pgscript ${CONTAINER_CLEAN_RESTORE} /src/test/sql/updates/setup.roles.sql "postgres" +docker_pgscript ${CONTAINER_CLEAN_RERUN} /src/test/sql/updates/setup.roles.sql "postgres" + CLEAN_VOLUME=$(docker inspect ${CONTAINER_CLEAN_RESTORE} --format='{{range .Mounts }}{{.Name}}{{end}}') UPDATE_VOLUME=$(docker inspect ${CONTAINER_ORIG} --format='{{range .Mounts }}{{.Name}}{{end}}') @@ -185,7 +192,7 @@ docker rm -f ${CONTAINER_ORIG} echo "Running update container" docker_run_vol ${CONTAINER_UPDATED} ${UPDATE_VOLUME}:/var/lib/postgresql/data ${UPDATE_TO_IMAGE}:${UPDATE_TO_TAG} -echo "Executing ALTER EXTENSION timescaledb UPDATE" +echo "Executing ALTER EXTENSION timescaledb UPDATE ($UPDATE_FROM_TAG -> $UPDATE_TO_TAG)" docker_pgcmd ${CONTAINER_UPDATED} "ALTER EXTENSION timescaledb UPDATE" "single" docker_pgcmd ${CONTAINER_UPDATED} "ALTER EXTENSION timescaledb UPDATE" "dn1" # Need to update also postgres DB since add_data_node may connect to @@ -225,4 +232,5 @@ docker_pgcmd ${CONTAINER_CLEAN_RESTORE} "ALTER DATABASE dn1 SET timescaledb.rest docker_exec ${CONTAINER_CLEAN_RESTORE} "pg_restore -h localhost -U postgres -d dn1 /tmp/dn1.sql" docker_pgcmd ${CONTAINER_CLEAN_RESTORE} "ALTER DATABASE dn1 SET timescaledb.restoring='off'" +echo "Comparing upgraded ($UPDATE_FROM_TAG -> $UPDATE_TO_TAG) with clean install ($UPDATE_TO_TAG)" docker_pgdiff_all /src/test/sql/updates/post.${TEST_VERSION}.sql "single" diff --git a/sql/updates/latest-dev.sql b/sql/updates/latest-dev.sql index e69de29bb2d..7d30789a313 100644 --- a/sql/updates/latest-dev.sql +++ b/sql/updates/latest-dev.sql @@ -0,0 +1,48 @@ +-- For continuous aggregates: Copy ACL privileges (grants) from the +-- query view (user-facing object) to the internal objects (e.g., +-- materialized hypertable, direct, and partial views). We want to +-- maintain the abstraction that a continuous aggregates is similar to +-- a materialized view (which is one object), so privileges on the +-- user-facing object should apply also to the internal objects that +-- implement the continuous aggregate. Having the right permissions on +-- internal objects is necessary for the watermark function used by +-- real-time aggregation since it queries the materialized hypertable +-- directly. +DO $$ +DECLARE + rels regclass[]; + rel regclass; + acl aclitem[]; +BEGIN + FOR rels, acl IN + -- For each cagg, collect an array of all relations (including + -- chunks) to copy the ACL to + SELECT array_cat(ARRAY[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], + (SELECT array_agg(inhrelid::regclass) + FROM pg_inherits + WHERE inhparent = format('%I.%I', h.schema_name, h.table_name)::regclass)), + relacl + FROM _timescaledb_catalog.continuous_agg ca + LEFT JOIN pg_class cl + ON (cl.oid = format('%I.%I', user_view_schema, user_view_name)::regclass) + LEFT JOIN _timescaledb_catalog.hypertable h + ON (ca.mat_hypertable_id = h.id) + WHERE relacl IS NOT NULL + LOOP + -- Set the ACL on all internal cagg relations, including + -- chunks. Note that we cannot use GRANT statements because + -- such statements are recorded as privileges on extension + -- objects when run in an update script. The result is that + -- the privileges will become init privileges, which will then + -- be ignored by, e.g., pg_dump. + FOR rel IN + SELECT * FROM unnest(rels) + LOOP + UPDATE pg_class SET relacl = acl + WHERE oid = rel; + END LOOP; + END LOOP; +END +$$ LANGUAGE PLPGSQL; diff --git a/test/sql/updates/post.continuous_aggs.sql b/test/sql/updates/post.continuous_aggs.sql index e6bea87a03c..427f2c847bf 100644 --- a/test/sql/updates/post.continuous_aggs.sql +++ b/test/sql/updates/post.continuous_aggs.sql @@ -17,3 +17,72 @@ CALL refresh_continuous_aggregate('mat_before',NULL,NULL); --the max of the temp for the POR should now be 165 SELECT * FROM mat_before ORDER BY bucket, location; + +SET ROLE cagg_user; +--should be able to query as cagg_user +SELECT * FROM mat_before ORDER BY bucket, location; +RESET ROLE; + +-- Output the ACLs for each internal cagg object +SELECT cl.oid::regclass::text, + 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 1; + +-- Output ACLs for chunks on materialized hypertables +SELECT inhparent::regclass::text AS parent, + cl.oid::regclass::text AS chunk, + relacl +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 1,2; + +-- Verify privileges on internal cagg objects. The privileges on the +-- materialized hypertable, partial view, and direct view should match +-- the user-facing user view. +DO $$ +DECLARE + user_view_rel regclass; + user_view_acl aclitem[]; + rel regclass; + acl aclitem[]; + acl_matches boolean; +BEGIN + FOR user_view_rel, user_view_acl IN + SELECT cl.oid, cl.relacl + FROM pg_class cl + JOIN _timescaledb_catalog.continuous_agg ca + ON (format('%I.%I', ca.user_view_schema, ca.user_view_name)::regclass = cl.oid) + LOOP + FOR rel, acl, acl_matches IN + SELECT cl.oid, + cl.relacl, + COALESCE(cl.relacl, ARRAY[]::aclitem[]) @> COALESCE(user_view_acl, ARRAY[]::aclitem[]) + 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)) + WHERE format('%I.%I', ca.user_view_schema, ca.user_view_name)::regclass = user_view_rel + LOOP + IF NOT acl_matches THEN + RAISE EXCEPTION 'privileges mismatch for continuous aggregate "%"', user_view_rel + USING DETAIL = format('Privileges for internal object "%s" are [%s], expected [%s].', + rel, acl, user_view_acl); + END IF; + END LOOP; + END LOOP; +END +$$ LANGUAGE PLPGSQL; diff --git a/test/sql/updates/setup.continuous_aggs.sql b/test/sql/updates/setup.continuous_aggs.sql index 80aa1e9d064..3cd9746b713 100644 --- a/test/sql/updates/setup.continuous_aggs.sql +++ b/test/sql/updates/setup.continuous_aggs.sql @@ -130,6 +130,8 @@ BEGIN END IF; END $$; +GRANT SELECT ON mat_before TO cagg_user WITH GRANT OPTION; + -- have to use psql conditional here because the procedure call can't be in transaction SELECT extversion < '2.0.0' AS has_refresh_mat_view from pg_extension WHERE extname = 'timescaledb' \gset \if :has_refresh_mat_view diff --git a/test/sql/updates/setup.continuous_aggs.v2.sql b/test/sql/updates/setup.continuous_aggs.v2.sql index e0eaa273242..174181f08b9 100644 --- a/test/sql/updates/setup.continuous_aggs.v2.sql +++ b/test/sql/updates/setup.continuous_aggs.v2.sql @@ -49,7 +49,6 @@ DECLARE ts_version TEXT; BEGIN SELECT extversion INTO ts_version FROM pg_extension WHERE extname = 'timescaledb'; - IF ts_version < '2.0.0' THEN CREATE VIEW mat_before WITH ( timescaledb.continuous, timescaledb.materialized_only=true, timescaledb.refresh_lag='-30 day', timescaledb.max_interval_per_job ='1000 day') @@ -143,6 +142,8 @@ BEGIN END IF; END $$; +GRANT SELECT ON mat_before TO cagg_user WITH GRANT OPTION; + \if :has_refresh_mat_view REFRESH MATERIALIZED VIEW mat_before; \else @@ -251,6 +252,8 @@ BEGIN END IF; END $$; +GRANT SELECT ON cagg.realtime_mat TO cagg_user; + \if :has_refresh_mat_view REFRESH MATERIALIZED VIEW cagg.realtime_mat; \else @@ -450,6 +453,10 @@ BEGIN END IF; END $$; +GRANT SELECT, TRIGGER, UPDATE +ON mat_conflict TO cagg_user +WITH GRANT OPTION; + -- Test that calling drop chunks on the hypertable does not break the -- update process when chunks are marked as dropped rather than -- removed. This happens when a continuous aggregate is defined on the diff --git a/test/sql/updates/setup.roles.sql b/test/sql/updates/setup.roles.sql new file mode 100644 index 00000000000..f19dfe9891c --- /dev/null +++ b/test/sql/updates/setup.roles.sql @@ -0,0 +1,5 @@ +-- 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. + +CREATE ROLE cagg_user;