From d9a609d255fc7cb483eab3c8242eed0846481922 Mon Sep 17 00:00:00 2001 From: kubikb Date: Sun, 2 Feb 2025 10:50:28 +0100 Subject: [PATCH 1/6] Add test to grant access to materialized view, extend table materialization SQL --- .../macros/materializations/table.sql | 6 +++++ .../adapter/test_grant_access_to.py | 23 +++++++++++++++++-- 2 files changed, 27 insertions(+), 2 deletions(-) diff --git a/dbt/include/bigquery/macros/materializations/table.sql b/dbt/include/bigquery/macros/materializations/table.sql index 41bb69770..25f944c5c 100644 --- a/dbt/include/bigquery/macros/materializations/table.sql +++ b/dbt/include/bigquery/macros/materializations/table.sql @@ -41,6 +41,12 @@ {% do persist_docs(target_relation, model) %} + {% if config.get('materialized') == "materialized_view" and config.get('grant_access_to') %} + {% for grant_target_dict in config.get('grant_access_to') %} + {% do adapter.grant_access_to(this, 'view', None, grant_target_dict) %} + {% endfor %} + {% endif %} + {{ return({'relations': [target_relation]}) }} {% endmaterialization %} diff --git a/tests/functional/adapter/test_grant_access_to.py b/tests/functional/adapter/test_grant_access_to.py index 633cebe92..bcc49c310 100644 --- a/tests/functional/adapter/test_grant_access_to.py +++ b/tests/functional/adapter/test_grant_access_to.py @@ -21,6 +21,24 @@ def select_1(dataset: str, materialized: str): ) +def select_1_materialized_view(dataset: str): + config = f"""config( + materialized='materialized_view', + grant_access_to=[ + {{'project': 'dbt-test-env', 'dataset': '{dataset}'}}, + ] + )""" + return ( + "{{" + + config + + "}}" + + """ + SELECT one, COUNT(1) AS count_one + FROM {{ ref('select_1_table') }} + GROUP BY one""" + ) + + BAD_CONFIG_TABLE_NAME = "bad_view" BAD_CONFIG_TABLE = """ {{ config( @@ -75,15 +93,16 @@ def models(self, unique_schema): return { "select_1.sql": select_1(dataset=dataset, materialized="view"), "select_1_table.sql": select_1(dataset=dataset, materialized="table"), + "select_1_materialized_view.sql": select_1_materialized_view(dataset=dataset), } def test_grant_access_succeeds(self, project, setup_grant_schema, teardown_grant_schema): # Need to run twice to validate idempotency results = run_dbt(["run"]) - assert len(results) == 2 + assert len(results) == 3 time.sleep(10) results = run_dbt(["run"]) - assert len(results) == 2 + assert len(results) == 3 class TestAccessGrantFails: From a7cdf880c7ff5b40960b971f2e97d3cfa1212731 Mon Sep 17 00:00:00 2001 From: kubikb Date: Mon, 3 Feb 2025 11:32:32 +0100 Subject: [PATCH 2/6] Add test to validate whether view and materialized view have been authorized --- .../functional/adapter/test_grant_access_to.py | 18 +++++++++++++++++- 1 file changed, 17 insertions(+), 1 deletion(-) diff --git a/tests/functional/adapter/test_grant_access_to.py b/tests/functional/adapter/test_grant_access_to.py index bcc49c310..17536c90b 100644 --- a/tests/functional/adapter/test_grant_access_to.py +++ b/tests/functional/adapter/test_grant_access_to.py @@ -96,13 +96,29 @@ def models(self, unique_schema): "select_1_materialized_view.sql": select_1_materialized_view(dataset=dataset), } - def test_grant_access_succeeds(self, project, setup_grant_schema, teardown_grant_schema): + def test_grant_access_succeeds(self, project, setup_grant_schema, teardown_grant_schema, unique_schema): # Need to run twice to validate idempotency results = run_dbt(["run"]) assert len(results) == 3 time.sleep(10) results = run_dbt(["run"]) assert len(results) == 3 + time.sleep(3) + + with project.adapter.connection_named("__test_grants"): + client = project.adapter.connections.get_thread_connection().handle + dataset_name = get_schema_name(unique_schema) + dataset_id = "{}.{}".format("dbt-test-env", dataset_name) + bq_dataset = client.get_dataset(dataset_id) + + authorized_view_names = [] + for access_entry in bq_dataset.access_entries: + if access_entry.entity_type != "view": + continue + + authorized_view_names.append(access_entry.entity_id["tableId"]) + + assert set(authorized_view_names) == set(["select_1", "select_1_materialized_view"]) class TestAccessGrantFails: From ae1decc718fccc36cdbce38d6cc6ac22ecade9e8 Mon Sep 17 00:00:00 2001 From: kubikb Date: Mon, 3 Feb 2025 11:40:20 +0100 Subject: [PATCH 3/6] Exclude materialized view from second run --- tests/functional/adapter/test_grant_access_to.py | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/tests/functional/adapter/test_grant_access_to.py b/tests/functional/adapter/test_grant_access_to.py index 17536c90b..b4ce00670 100644 --- a/tests/functional/adapter/test_grant_access_to.py +++ b/tests/functional/adapter/test_grant_access_to.py @@ -101,8 +101,9 @@ def test_grant_access_succeeds(self, project, setup_grant_schema, teardown_grant results = run_dbt(["run"]) assert len(results) == 3 time.sleep(10) - results = run_dbt(["run"]) - assert len(results) == 3 + # Materialized view excluded since it would produce an error since source table is replaced + results = run_dbt(["run", "--exclude", "select_1_materialized_view"]) + assert len(results) == 2 time.sleep(3) with project.adapter.connection_named("__test_grants"): From 8dd82aadec1ede8c0627e648619dbe426d48d129 Mon Sep 17 00:00:00 2001 From: kubikb Date: Tue, 4 Feb 2025 08:00:04 +0100 Subject: [PATCH 4/6] Move access granting to relation .sql files --- dbt/include/bigquery/macros/materializations/table.sql | 6 ------ .../bigquery/macros/relations/materialized_view/create.sql | 6 ++++++ .../bigquery/macros/relations/materialized_view/replace.sql | 6 ++++++ 3 files changed, 12 insertions(+), 6 deletions(-) diff --git a/dbt/include/bigquery/macros/materializations/table.sql b/dbt/include/bigquery/macros/materializations/table.sql index 25f944c5c..41bb69770 100644 --- a/dbt/include/bigquery/macros/materializations/table.sql +++ b/dbt/include/bigquery/macros/materializations/table.sql @@ -41,12 +41,6 @@ {% do persist_docs(target_relation, model) %} - {% if config.get('materialized') == "materialized_view" and config.get('grant_access_to') %} - {% for grant_target_dict in config.get('grant_access_to') %} - {% do adapter.grant_access_to(this, 'view', None, grant_target_dict) %} - {% endfor %} - {% endif %} - {{ return({'relations': [target_relation]}) }} {% endmaterialization %} diff --git a/dbt/include/bigquery/macros/relations/materialized_view/create.sql b/dbt/include/bigquery/macros/relations/materialized_view/create.sql index d3e8c7685..13eaac666 100644 --- a/dbt/include/bigquery/macros/relations/materialized_view/create.sql +++ b/dbt/include/bigquery/macros/relations/materialized_view/create.sql @@ -2,6 +2,12 @@ {%- set materialized_view = adapter.Relation.materialized_view_from_relation_config(config.model) -%} + {% if config.get('grant_access_to') %} + {% for grant_target_dict in config.get('grant_access_to') %} + {% do adapter.grant_access_to(this, 'view', None, grant_target_dict) %} + {% endfor %} + {% endif %} + create materialized view if not exists {{ relation }} {% if materialized_view.partition %}{{ partition_by(materialized_view.partition) }}{% endif %} {% if materialized_view.cluster %}{{ cluster_by(materialized_view.cluster.fields) }}{% endif %} diff --git a/dbt/include/bigquery/macros/relations/materialized_view/replace.sql b/dbt/include/bigquery/macros/relations/materialized_view/replace.sql index 2e4a0b69f..5be66443c 100644 --- a/dbt/include/bigquery/macros/relations/materialized_view/replace.sql +++ b/dbt/include/bigquery/macros/relations/materialized_view/replace.sql @@ -2,6 +2,12 @@ {%- set materialized_view = adapter.Relation.materialized_view_from_relation_config(config.model) -%} + {% if config.get('grant_access_to') %} + {% for grant_target_dict in config.get('grant_access_to') %} + {% do adapter.grant_access_to(this, 'view', None, grant_target_dict) %} + {% endfor %} + {% endif %} + create or replace materialized view if not exists {{ relation }} {% if materialized_view.partition %}{{ partition_by(materialized_view.partition) }}{% endif %} {% if materialized_view.cluster %}{{ cluster_by(materialized_view.cluster.fields) }}{% endif %} From c5cb4768648a6f08df84cc82320cb1c50822f5be Mon Sep 17 00:00:00 2001 From: kubikb Date: Tue, 4 Feb 2025 08:23:35 +0100 Subject: [PATCH 5/6] Add file to changes with fixes --- .changes/unreleased/Fixes-20250202-105422.yaml | 6 ++++++ 1 file changed, 6 insertions(+) create mode 100644 .changes/unreleased/Fixes-20250202-105422.yaml diff --git a/.changes/unreleased/Fixes-20250202-105422.yaml b/.changes/unreleased/Fixes-20250202-105422.yaml new file mode 100644 index 000000000..af8c00760 --- /dev/null +++ b/.changes/unreleased/Fixes-20250202-105422.yaml @@ -0,0 +1,6 @@ +kind: Fixes +body: Fix silently ignoring materialized view authorization +time: 2025-02-02T10:54:22.12198+01:00 +custom: + Author: kubikb + Issue: "1471" From b83afd677add9c7762b17b65162e770f8e641543 Mon Sep 17 00:00:00 2001 From: kubikb Date: Tue, 4 Feb 2025 08:24:44 +0100 Subject: [PATCH 6/6] Remove unnecessary sleep in grant_access_to test --- tests/functional/adapter/test_grant_access_to.py | 1 - 1 file changed, 1 deletion(-) diff --git a/tests/functional/adapter/test_grant_access_to.py b/tests/functional/adapter/test_grant_access_to.py index b4ce00670..08c17cda1 100644 --- a/tests/functional/adapter/test_grant_access_to.py +++ b/tests/functional/adapter/test_grant_access_to.py @@ -104,7 +104,6 @@ def test_grant_access_succeeds(self, project, setup_grant_schema, teardown_grant # Materialized view excluded since it would produce an error since source table is replaced results = run_dbt(["run", "--exclude", "select_1_materialized_view"]) assert len(results) == 2 - time.sleep(3) with project.adapter.connection_named("__test_grants"): client = project.adapter.connections.get_thread_connection().handle