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

Add Grants to BigQuery Materializations #212

Merged
merged 31 commits into from
Jul 11, 2022
Merged
Show file tree
Hide file tree
Changes from 19 commits
Commits
Show all changes
31 commits
Select commit Hold shift + click to select a range
712d22b
Add get_show_grant_sql
emmyoop Jun 27, 2022
5bb3afd
first pass at granting with sql
emmyoop Jun 27, 2022
aeb19f3
more macro overrides
emmyoop Jun 29, 2022
911fed0
Merge branch 'main' of https://github.com/dbt-labs/dbt-bigquery into …
emmyoop Jul 7, 2022
42ae831
update macros, temp update to dev-req for CI
emmyoop Jul 7, 2022
a661549
tweak to sql and added some TODOs
emmyoop Jul 7, 2022
b0b06ef
move things around
emmyoop Jul 7, 2022
e0909ca
exclude session_user
emmyoop Jul 7, 2022
9f73a3a
Merge branch 'main' of https://github.com/dbt-labs/dbt-bigquery into …
emmyoop Jul 7, 2022
0733a5f
fix mypy error, fix exclude to be more than users
emmyoop Jul 7, 2022
ce094b7
simplify revoke logic
emmyoop Jul 7, 2022
83c73e0
small cleanup, point to ct-660 branch
emmyoop Jul 8, 2022
0cdeb1e
grant entire list in one statement
emmyoop Jul 8, 2022
8070891
wip
emmyoop Jul 8, 2022
d8f6e2d
wip, broken tests
emmyoop Jul 8, 2022
10cc952
Update dbt/include/bigquery/macros/materializations/incremental.sql
emmyoop Jul 8, 2022
cb42c81
working on getting tests working, very WIP
emmyoop Jul 8, 2022
2dde9ee
Merge branch 'main' of https://github.com/dbt-labs/dbt-bigquery into …
emmyoop Jul 8, 2022
9f51fab
Merge branch 'er/ct-717-grant-materialization' of https://github.com/…
emmyoop Jul 8, 2022
118d311
All tests passing locally
jtcohen6 Jul 10, 2022
2957696
Updated user / group names
dbeatty10 Jul 11, 2022
f61bd39
Ongoing test cleanup
jtcohen6 Jul 11, 2022
210830d
Merge branch 'dbeatty/grantee-env-vars-take-2' into er/ct-717-grant-m…
jtcohen6 Jul 11, 2022
c4ce8d6
Account for refactor in dbt-labs/dbt-core@c763601
jtcohen6 Jul 11, 2022
cee9fa1
Updated third value
dbeatty10 Jul 11, 2022
d6aedf8
Merge branch 'dbeatty/grantee-env-vars-take-2' into er/ct-717-grant-m…
dbeatty10 Jul 11, 2022
27c41ed
Alt approach for grant location (#218)
jtcohen6 Jul 11, 2022
3b9c9a4
Merge branch 'main' of https://github.com/dbt-labs/dbt-bigquery into …
emmyoop Jul 11, 2022
a6ef920
reset to point dbt-core to main instead of branch
emmyoop Jul 11, 2022
f47322f
fix examples in test.env.example
emmyoop Jul 11, 2022
d94b623
add changelog
emmyoop Jul 11, 2022
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
66 changes: 66 additions & 0 deletions dbt/include/bigquery/macros/adapters/apply_grants.sql
Original file line number Diff line number Diff line change
@@ -0,0 +1,66 @@
{% macro bigquery__get_show_grant_sql(relation) %}
{% if not target.location %}
{{ exceptions.raise_compiler_error("In order to use the grants feature, you must specify a location ") }}
{% endif %}

select privilege_type, grantee from {{ relation.project }}.`region-{{ target.location }}`.INFORMATION_SCHEMA.OBJECT_PRIVILEGES
emmyoop marked this conversation as resolved.
Show resolved Hide resolved
emmyoop marked this conversation as resolved.
Show resolved Hide resolved
where object_schema = "{{ relation.dataset }}" and object_name = "{{ relation.identifier }}" and split(grantee, ':')[offset(1)] != session_user()
{% endmacro %}


{%- macro bigquery__get_grant_sql(relation, grant_config) -%}
{%- set grant_statements = [] -%}
{%- for privilege in grant_config.keys() %}
{%- set grantees = grant_config[privilege] -%}
{%- if grantees %}
{% set grant_sql -%}
grant `{{ privilege }}` on {{ relation.type }} {{ relation }} to {{ '\"' + grantees|join('\", \"') + '\"' }}
{%- endset %}
{%- do grant_statements.append(grant_sql) -%}
{% endif -%}
{%- endfor -%}
{{ return(grant_statements) }}

{%- endmacro %}


{% macro bigquery__get_revoke_sql(relation, grant_config) %}

{%- set revoke_statements = [] -%}
{%- for privilege in grant_config.keys() -%}
{%- set grantees = grant_config[privilege] -%}
{%- if grantees %}
{% set revoke_sql -%}
revoke `{{ privilege }}` on {{ relation.type }} {{ relation }} from {{ '\"' + grantees|join('\", \"') + '\"' }}
{%- endset %}
{%- do revoke_statements.append(revoke_sql) -%}
{% endif -%}
{%- endfor -%}
{{ return(revoke_statements) }}

{%- endmacro -%}

{% macro bigquery__apply_grants(relation, grant_config, should_revoke=True) %}
{% if grant_config %}
{% if should_revoke %}
{% set current_grants_table = run_query(get_show_grant_sql(relation)) %}
{% set current_grants_dict = adapter.standardize_grants_dict(current_grants_table) %}
{% set needs_granting = diff_of_two_dicts_no_lower(grant_config, current_grants_dict) %}
{% set needs_revoking = diff_of_two_dicts_no_lower(current_grants_dict, grant_config) %}
emmyoop marked this conversation as resolved.
Show resolved Hide resolved
{% if not (needs_granting or needs_revoking) %}
{{ log('On ' ~ relation ~': All grants are in place, no revocation or granting needed.')}}
{% endif %}
{% else %}
{% set needs_revoking = {} %}
{% set needs_granting = grant_config %}
{% endif %}
{% if needs_granting or needs_revoking %}
{% set revoke_statement_list = get_revoke_sql(relation, needs_revoking) %}
{% set grant_statement_list = get_grant_sql(relation, needs_granting) %}
{% set grant_and_revoke_statement_list = revoke_statement_list + grant_statement_list %}
{% if grant_and_revoke_statement_list %}
{{ call_grant_revoke_statement_list(grant_and_revoke_statement_list) }}
{% endif %}
{% endif %}
{% endif %}
{% endmacro %}
3 changes: 2 additions & 1 deletion dbt/include/bigquery/macros/materializations/copy.sql
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@
{{ source_array.append(source(*src_table)) }}
{% endfor %}

{# Call adapter's copy_table function #}
{# Call adapter copy_table function #}
{%- set result_str = adapter.copy_table(
source_array,
destination,
Expand All @@ -26,6 +26,7 @@

{# Clean up #}
{{ run_hooks(post_hooks) }}
{%- do apply_grants(target_relation, grant_config) -%}
{{ adapter.commit() }}

{{ return({'relations': [destination]}) }}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -152,6 +152,9 @@

{% set on_schema_change = incremental_validate_on_schema_change(config.get('on_schema_change'), default='ignore') %}

-- grab current tables grants config for comparision later on
{% set grant_config = config.get('grants') %}

{{ run_hooks(pre_hooks) }}

{% if existing_relation is none %}
Expand Down Expand Up @@ -197,6 +200,8 @@

{% set target_relation = this.incorporate(type='table') %}

{% do apply_grants(target_relation, grant_config) %}

{% do persist_docs(target_relation, model) %}

{{ return({'relations': [target_relation]}) }}
Expand Down
5 changes: 5 additions & 0 deletions dbt/include/bigquery/macros/materializations/table.sql
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,9 @@
{%- set exists_not_as_table = (old_relation is not none and not old_relation.is_table) -%}
{%- set target_relation = api.Relation.create(database=database, schema=schema, identifier=identifier, type='table') -%}

-- grab current tables grants config for comparision later on
{%- set grant_config = config.get('grants') -%}

{{ run_hooks(pre_hooks) }}

{#
Expand All @@ -30,6 +33,8 @@

{{ run_hooks(post_hooks) }}

{%- do apply_grants(target_relation, grant_config) -%}
emmyoop marked this conversation as resolved.
Show resolved Hide resolved

{% do persist_docs(target_relation, model) %}

{{ return({'relations': [target_relation]}) }}
Expand Down
4 changes: 4 additions & 0 deletions dbt/include/bigquery/macros/materializations/view.sql
Original file line number Diff line number Diff line change
Expand Up @@ -9,9 +9,13 @@


{% materialization view, adapter='bigquery' -%}
-- grab current tables grants config for comparision later on
{% set grant_config = config.get('grants') %}

{% set to_return = create_or_replace_view() %}

{% set target_relation = this.incorporate(type='view') %}
{% do apply_grants(target_relation, grant_config) %}
emmyoop marked this conversation as resolved.
Show resolved Hide resolved
{% do persist_docs(target_relation, model) %}

{% if config.get('grant_access_to') %}
Expand Down
9 changes: 7 additions & 2 deletions dev-requirements.txt
Original file line number Diff line number Diff line change
@@ -1,7 +1,12 @@
# install latest changes in dbt-core
# TODO: how to automate switching from develop to version branches?
git+https://github.com/dbt-labs/dbt-core.git#egg=dbt-core&subdirectory=core
git+https://github.com/dbt-labs/dbt-core.git#egg=dbt-tests-adapter&subdirectory=tests/adapter

git+https://github.com/dbt-labs/dbt-core.git@ct-808-more_grant_adapter_tests#egg=dbt-core&subdirectory=core
git+https://github.com/dbt-labs/dbt-core.git@ct-808-more_grant_adapter_tests#egg=dbt-tests-adapter&subdirectory=tests/adapter

# TODO: replace above with this after dbt-core/#5263 gets merged to main
# git+https://github.com/dbt-labs/dbt-core.git#egg=dbt-core&subdirectory=core
# git+https://github.com/dbt-labs/dbt-core.git#egg=dbt-tests-adapter&subdirectory=tests/adapter
emmyoop marked this conversation as resolved.
Show resolved Hide resolved

black==22.6.0
bumpversion
Expand Down
4 changes: 4 additions & 0 deletions test.env.example
Original file line number Diff line number Diff line change
@@ -1,3 +1,7 @@
BIGQUERY_TEST_ALT_DATABASE=
BIGQUERY_TEST_NO_ACCESS_DATABASE=
BIGQUERY_TEST_SERVICE_ACCOUNT_JSON='{}'

DBT_TEST_USER_1="user:buildbot@dbtlabs.com"
DBT_TEST_USER_2="user:buildbot@fishtownanalytics.com"
emmyoop marked this conversation as resolved.
Show resolved Hide resolved
DBT_TEST_USER_3="group:dev-core@dbtlabs.com"
2 changes: 2 additions & 0 deletions tests/conftest.py
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@ def oauth_target():
'method': 'oauth',
'threads': 1,
# project isn't needed if you configure a default, via 'gcloud config set project'
'location': 'US'
emmyoop marked this conversation as resolved.
Show resolved Hide resolved
}


Expand All @@ -43,4 +44,5 @@ def service_account_target():
'threads': 1,
'project': project_id,
'keyfile_json': credentials,
'location': 'US'
}
44 changes: 44 additions & 0 deletions tests/functional/adapter/test_grants.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,44 @@
import pytest

from dbt.tests.adapter.grants.base_grants import BaseGrants
from dbt.tests.adapter.grants.test_model_grants import BaseModelGrants
from dbt.tests.adapter.grants.test_incremental_grants import BaseIncrementalGrants
from dbt.tests.adapter.grants.test_invalid_grants import BaseInvalidGrants
from dbt.tests.adapter.grants.test_seed_grants import BaseSeedGrants
from dbt.tests.adapter.grants.test_snapshot_grants import BaseSnapshotGrants

from dbt.context.base import BaseContext # diff_of_two_dicts_no_lower only


class BaseGrantsBigQuery(BaseGrants):
def privilege_names(self):
# TODO: what is insert equivalent?
return {"select": "roles/bigquery.dataViewer", "insert": "roles/bigquery.dataViewer", "fake_privilege": "roles/invalid"}
emmyoop marked this conversation as resolved.
Show resolved Hide resolved


def assert_expected_grants_match_actual(self, project, relation_name, expected_grants):
actual_grants = self.get_grants_on_relation(project, relation_name)
# need a case-insensitive comparison
# so just a simple "assert expected == actual_grants" won't work
diff_a = BaseContext.diff_of_two_dicts_no_lower(actual_grants, expected_grants)
diff_b = BaseContext.diff_of_two_dicts_no_lower(expected_grants, actual_grants)
emmyoop marked this conversation as resolved.
Show resolved Hide resolved
assert diff_a == diff_b == {}

class TestModelGrantsBigQuery(BaseGrantsBigQuery, BaseModelGrants):
pass


class TestIncrementalGrantsBigQuery(BaseGrantsBigQuery, BaseIncrementalGrants):
pass


class TestSeedGrantsBigQuery(BaseGrantsBigQuery, BaseSeedGrants):
pass


class TestSnapshotGrantsBigQuery(BaseGrantsBigQuery, BaseSnapshotGrants):
pass


class TestInvalidGrantsBigQuery(BaseGrantsBigQuery, BaseInvalidGrants):
pass
170 changes: 170 additions & 0 deletions tests/functional/adapter/test_model_grants_local.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,170 @@
import pytest
emmyoop marked this conversation as resolved.
Show resolved Hide resolved
from dbt.tests.util import (
run_dbt_and_capture,
get_manifest,
write_file,
)
from dbt.tests.adapter.grants.base_grants import BaseGrants

from dbt.context.base import BaseContext # diff_of_two_dicts_no_lower only


my_model_sql = """
select 1 as fun
"""

model_schema_yml = """
version: 2
models:
- name: my_model
config:
grants:
select: ["{{ env_var('DBT_TEST_USER_1') }}"]
"""

user2_model_schema_yml = """
version: 2
models:
- name: my_model
config:
grants:
select: ["{{ env_var('DBT_TEST_USER_2') }}"]
"""

table_model_schema_yml = """
version: 2
models:
- name: my_model
config:
materialized: table
grants:
select: ["{{ env_var('DBT_TEST_USER_1') }}"]
"""

user2_table_model_schema_yml = """
version: 2
models:
- name: my_model
config:
materialized: table
grants:
select: ["{{ env_var('DBT_TEST_USER_2') }}"]
"""

multiple_users_table_model_schema_yml = """
version: 2
models:
- name: my_model
config:
materialized: table
grants:
select: ["{{ env_var('DBT_TEST_USER_1') }}", "{{ env_var('DBT_TEST_USER_2') }}"]
"""

multiple_privileges_table_model_schema_yml = """
version: 2
models:
- name: my_model
config:
materialized: table
grants:
select: ["{{ env_var('DBT_TEST_USER_1') }}"]
insert: ["{{ env_var('DBT_TEST_USER_2') }}"]
"""

class BaseGrantsBigQuery(BaseGrants):
def privilege_names(self):
# TODO: what is insert equivalent?
return {"select": "roles/bigquery.dataViewer", "insert": "roles/bigquery.dataViewer", "fake_privilege": "roles/invalid"}

def assert_expected_grants_match_actual(self, project, relation_name, expected_grants):
actual_grants = self.get_grants_on_relation(project, relation_name)
# need a case-insensitive comparison
# so just a simple "assert expected == actual_grants" won't work
diff_a = BaseContext.diff_of_two_dicts_no_lower(actual_grants, expected_grants)
diff_b = BaseContext.diff_of_two_dicts_no_lower(expected_grants, actual_grants)
assert diff_a == diff_b == {}

class BaseModelGrants(BaseGrantsBigQuery):
@pytest.fixture(scope="class")
def models(self):
updated_schema = self.interpolate_privilege_names(model_schema_yml)
return {
"my_model.sql": my_model_sql,
"schema.yml": updated_schema,
}

def test_view_table_grants(self, project, get_test_users):
# we want the test to fail, not silently skip
test_users = get_test_users
select_privilege_name = self.privilege_names()["select"]
insert_privilege_name = self.privilege_names()["insert"]
assert len(test_users) == 3

# View materialization, single select grant
(results, log_output) = run_dbt_and_capture(["--debug", "run"])
assert len(results) == 1
manifest = get_manifest(project.project_root)
model_id = "model.test.my_model"
model = manifest.nodes[model_id]
expected = {select_privilege_name: [test_users[0]]}
assert model.config.grants == expected
assert model.config.materialized == "view"
self.assert_expected_grants_match_actual(project, "my_model", expected)

# View materialization, change select grant user
updated_yaml = self.interpolate_privilege_names(user2_model_schema_yml)
write_file(updated_yaml, project.project_root, "models", "schema.yml")
(results, log_output) = run_dbt_and_capture(["--debug", "run"])
assert len(results) == 1
expected = {select_privilege_name: [get_test_users[1]]}
self.assert_expected_grants_match_actual(project, "my_model", expected)

# Table materialization, single select grant
updated_yaml = self.interpolate_privilege_names(table_model_schema_yml)
write_file(updated_yaml, project.project_root, "models", "schema.yml")
(results, log_output) = run_dbt_and_capture(["--debug", "run"])
assert len(results) == 1
manifest = get_manifest(project.project_root)
model_id = "model.test.my_model"
model = manifest.nodes[model_id]
assert model.config.materialized == "table"
expected = {select_privilege_name: [test_users[0]]}
self.assert_expected_grants_match_actual(project, "my_model", expected)

# Table materialization, change select grant user
updated_yaml = self.interpolate_privilege_names(user2_table_model_schema_yml)
write_file(updated_yaml, project.project_root, "models", "schema.yml")
(results, log_output) = run_dbt_and_capture(["--debug", "run"])
assert len(results) == 1
manifest = get_manifest(project.project_root)
model = manifest.nodes[model_id]
assert model.config.materialized == "table"
expected = {select_privilege_name: [test_users[1]]}
self.assert_expected_grants_match_actual(project, "my_model", expected)

# Table materialization, multiple grantees
updated_yaml = self.interpolate_privilege_names(multiple_users_table_model_schema_yml)
write_file(updated_yaml, project.project_root, "models", "schema.yml")
(results, log_output) = run_dbt_and_capture(["--debug", "run"])
assert len(results) == 1
manifest = get_manifest(project.project_root)
model = manifest.nodes[model_id]
assert model.config.materialized == "table"
expected = {select_privilege_name: [test_users[0], test_users[1]]}
self.assert_expected_grants_match_actual(project, "my_model", expected)

# Table materialization, multiple privileges
updated_yaml = self.interpolate_privilege_names(multiple_privileges_table_model_schema_yml)
write_file(updated_yaml, project.project_root, "models", "schema.yml")
(results, log_output) = run_dbt_and_capture(["--debug", "run"])
assert len(results) == 1
manifest = get_manifest(project.project_root)
model = manifest.nodes[model_id]
assert model.config.materialized == "table"
expected = {select_privilege_name: [test_users[0]], insert_privilege_name: [test_users[1]]}
self.assert_expected_grants_match_actual(project, "my_model", expected)


class TestModelGrants(BaseModelGrants):
pass