Skip to content

Commit

Permalink
Fix continuous aggregate privileges during update
Browse files Browse the repository at this point in the history
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
  • Loading branch information
erimatnor committed Jan 22, 2021
1 parent 41cfabd commit f21aad0
Show file tree
Hide file tree
Showing 6 changed files with 139 additions and 2 deletions.
10 changes: 9 additions & 1 deletion scripts/test_update_from_tag.sh
Expand Up @@ -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}}')

Expand All @@ -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
Expand Down Expand Up @@ -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"
46 changes: 46 additions & 0 deletions sql/updates/latest-dev.sql
@@ -0,0 +1,46 @@
-- 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;
69 changes: 69 additions & 0 deletions test/sql/updates/post.continuous_aggs.sql
Expand Up @@ -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;
2 changes: 2 additions & 0 deletions test/sql/updates/setup.continuous_aggs.sql
Expand Up @@ -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
Expand Down
9 changes: 8 additions & 1 deletion test/sql/updates/setup.continuous_aggs.v2.sql
Expand Up @@ -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')
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down
5 changes: 5 additions & 0 deletions 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;

0 comments on commit f21aad0

Please sign in to comment.