From a1ec09c2573ba27713bc0fa5f82325646ecf663e Mon Sep 17 00:00:00 2001 From: Soksamnang Lim Date: Tue, 3 Oct 2023 22:43:41 -0700 Subject: [PATCH 01/14] Support Privilege Grants to Roles and Groups in Materialization --- dbt/adapters/redshift/impl.py | 93 +++++- .../redshift/macros/adapters/apply_grants.sql | 106 ++++++- test.env.example | 6 + .../functional/adapter/grants/base_grants.py | 47 +++ .../adapter/grants/test_incremental_grants.py | 171 ++++++++++ .../adapter/grants/test_model_grants.py | 293 ++++++++++++++++++ .../adapter/grants/test_seed_grants.py | 171 ++++++++++ .../adapter/grants/test_snapshot_grants.py | 129 ++++++++ tests/functional/adapter/test_grants.py | 20 +- 9 files changed, 1022 insertions(+), 14 deletions(-) create mode 100644 tests/functional/adapter/grants/base_grants.py create mode 100644 tests/functional/adapter/grants/test_incremental_grants.py create mode 100644 tests/functional/adapter/grants/test_model_grants.py create mode 100644 tests/functional/adapter/grants/test_seed_grants.py create mode 100644 tests/functional/adapter/grants/test_snapshot_grants.py diff --git a/dbt/adapters/redshift/impl.py b/dbt/adapters/redshift/impl.py index ae9f18392..cfea6237f 100644 --- a/dbt/adapters/redshift/impl.py +++ b/dbt/adapters/redshift/impl.py @@ -9,15 +9,12 @@ from dbt.contracts.graph.nodes import ConstraintType from dbt.events import AdapterLogger - import dbt.exceptions from dbt.adapters.redshift import RedshiftConnectionManager, RedshiftRelation - logger = AdapterLogger("Redshift") - GET_RELATIONS_MACRO_NAME = "redshift__get_relations" @@ -168,3 +165,93 @@ def generate_python_submission_response(self, submission_result: Any) -> Adapter def debug_query(self): """Override for DebugTask method""" self.execute("select 1 as id") + + ## grant-related methods ## + @available + def standardize_grants_dict(self, grants_table): + """ + Override for standardize_grants_dict + """ + grants_dict = {} # Dict[str, Dict[str, List[str]]] + for row in grants_table: + grantee_type = row["grantee_type"] + grantee = row["grantee"] + privilege = row["privilege_type"] + if privilege not in grants_dict: + grants_dict[privilege] = {} + + if grantee_type not in grants_dict[privilege]: + grants_dict[privilege][grantee_type] = [] + + grants_dict[privilege][grantee_type].append(grantee) + + return grants_dict + + @available + def diff_of_two_nested_dicts(self, dict_a, dict_b): + """ + Given two dictionaries of type Dict[str, Dict[str, List[str]]]: + dict_a = {'key_x': {'key_a': 'VALUE_1'}, 'KEY_Y': {'key_b': value_2'}} + dict_b = {'key_x': {'key_a': 'VALUE_1'}, 'KEY_Y': {'key_b': value_2'}} + Return the same dictionary representation of dict_a MINUS dict_b, + performing a case-insensitive comparison between the strings in each. + All keys returned will be in the original case of dict_a. + returns {'key_x': ['VALUE_2'], 'KEY_Y': ['value_3']} + """ + + dict_diff = {} + + for k, v_a in dict_a.items(): + if k.casefold() in dict_b: + v_b = dict_b[k.casefold()] + + for sub_key, values_a in v_a.items(): + if sub_key in v_b: + values_b = v_b[sub_key] + diff_values = [v for v in values_a if v.casefold() not in values_b] + if diff_values: + if k in dict_diff: + dict_diff[k][sub_key] = diff_values + else: + dict_diff[k] = {sub_key: diff_values} + else: + if k in dict_diff: + if values_a: + dict_diff[k][sub_key] = values_a + else: + if values_a: + dict_diff[k] = {sub_key: values_a} + else: + dict_diff[k] = v_a + + return dict_diff + + @available + def process_grant_dicts(self, unknown_dict): + """ + Given a dictionary where the type can either be of type: + - Dict[str, List[str]] + - Dict[str, List[Dict[str, List[str]] + Return a processed dictionary of the type Dict[str, Dict[str, List[str]] + """ + first_value = next(iter(unknown_dict.values())) + if first_value: + is_dict = isinstance(first_value[0], dict) + else: + is_dict = False + + temp = {} + if not is_dict: + for privilege, grantees in unknown_dict.items(): + if grantees: + temp[privilege] = {"user": grantees} + else: + for privilege, grantee_map in unknown_dict.items(): + grantees_map_temp = {} + for grantee_type, grantees in grantee_map[0].items(): + if grantees: + grantees_map_temp[grantee_type] = grantees + if grantees_map_temp: + temp[privilege] = grantees_map_temp + + return temp \ No newline at end of file diff --git a/dbt/include/redshift/macros/adapters/apply_grants.sql b/dbt/include/redshift/macros/adapters/apply_grants.sql index fa6523a26..ed51aaa2a 100644 --- a/dbt/include/redshift/macros/adapters/apply_grants.sql +++ b/dbt/include/redshift/macros/adapters/apply_grants.sql @@ -1,5 +1,42 @@ -{% macro redshift__get_show_grant_sql(relation) %} +{# ------- DCL STATEMENT TEMPLATES ------- #} + +{%- macro redshift__get_grant_sql(relation, privilege, grantee_dict) -%} + {#-- generates a multiple-grantees grant privilege statement --#} + grant {{privilege}} on {{relation}} to + {%- for grantee_type, grantees in grantee_dict.items() -%} + {%- if grantee_type=='user' and grantees -%} + {{ " " + (grantees | join(', ')) }} + {%- elif grantee_type=='group' and grantees -%} + {{ " " +("group " + grantees | join(', group ')) }} + {%- elif grantee_type=='role' and grantees -%} + {{ " " + ("role " + grantees | join(', role ')) }} + {%- endif -%} + {%- if not loop.last -%} + , + {%- endif -%} + {%- endfor -%} +{%- endmacro -%} + +{%- macro redshift__get_revoke_sql(relation, privilege, revokee_dict) -%} + {#-- generates a multiple-grantees revoke privilege statement --#} + revoke {{privilege}} on {{relation}} from + {%- for revokee_type, revokees in revokee_dict.items() -%} + {%- if revokee_type=='user' and revokees -%} + {{ " " + (revokees | join(', ')) }} + {%- elif revokee_type=='group' and revokees -%} + {{ " " +("group " + revokees | join(', group ')) }} + {%- elif revokee_type=='role' and revokees -%} + {{ " " + ("role " + revokees | join(', role ')) }} + {%- endif -%} + {%- if not loop.last -%} + , + {%- endif -%} + {%- endfor -%} +{%- endmacro -%} + +{% macro redshift__get_show_grant_sql(relation) %} +{#-- shows the privilege grants on a table for users, groups, and roles --#} with privileges as ( -- valid options per https://docs.aws.amazon.com/redshift/latest/dg/r_HAS_TABLE_PRIVILEGE.html @@ -16,6 +53,7 @@ with privileges as ( ) select + 'user' as grantee_type, u.usename as grantee, p.privilege_type from pg_user u @@ -24,4 +62,70 @@ where has_table_privilege(u.usename, '{{ relation }}', privilege_type) and u.usename != current_user and not u.usesuper +union all +-- check that group has table privilege +select + 'group' as grantee_type, + g.groname as grantee, + p.privilege_type +from pg_group g +cross join privileges p +where exists( + select * + from information_schema.table_privileges tp + where tp.grantee=g.groname + and tp.table_name=replace(split_part('{{ relation }}', '.', 3), '"', '') + and LOWER(tp.privilege_type)=p.privilege_type +) + +union all +-- check that role has table privilege +select + 'role' as grantee_type, + r.role_name as rolename, + p.privilege_type +from svv_roles r +cross join privileges p +where exists( + select * + from svv_relation_privileges rp + where rp.identity_name=r.role_name + and rp.relation_name=replace(split_part('{{ relation }}', '.', 3), '"', '') + and LOWER(rp.privilege_type)=p.privilege_type +) + +{% endmacro %} + +{% macro redshift__apply_grants(relation, grant_config, should_revoke=True) %} + {#-- Override for apply grants --#} + {#-- If grant_config is {} or None, this is a no-op --#} + {% if grant_config %} + {#-- change grant_config to Dict[str, Dict[str, List[str]] format --#} + {% set grant_config = adapter.process_grant_dicts(grant_config) %} + + {% if should_revoke %} + {#-- We think that there is a chance that grants are carried over. --#} + {#-- Show the current grants for users, roles, and groups and calculate the diffs. --#} + {% 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 = adapter.diff_of_two_nested_dicts(grant_config, current_grants_dict) %} + {% set needs_revoking = adapter.diff_of_two_nested_dicts(current_grants_dict, grant_config) %} + {% if not (needs_granting or needs_revoking) %} + {{ log('On ' ~ relation ~': All grants are in place, no revocation or granting needed.')}} + {% endif %} + {% else %} + {#-- We don't think there's any chance of previous grants having carried over. --#} + {#-- Jump straight to granting what the user has configured. --#} + {% set needs_revoking = {} %} + {% set needs_granting = grant_config %} + {% endif %} + {% if needs_granting or needs_revoking %} + {% set revoke_statement_list = get_dcl_statement_list(relation, needs_revoking, get_revoke_sql) %} + {% set grant_statement_list = get_dcl_statement_list(relation, needs_granting, get_grant_sql) %} + {% set dcl_statement_list = revoke_statement_list + grant_statement_list %} + {% if dcl_statement_list %} + {{ call_dcl_statements(dcl_statement_list) }} + {% endif %} + {% endif %} + {% endif %} {% endmacro %} diff --git a/test.env.example b/test.env.example index 4de05edab..8d645f83b 100644 --- a/test.env.example +++ b/test.env.example @@ -16,3 +16,9 @@ REDSHIFT_TEST_DBNAME= DBT_TEST_USER_1=dbt_test_user_1 DBT_TEST_USER_2=dbt_test_user_2 DBT_TEST_USER_3=dbt_test_user_3 +DBT_TEST_GROUP_1=dbt_test_group_1 +DBT_TEST_GROUP_2=dbt_test_group_2 +DBT_TEST_GROUP_3=dbt_test_group_3 +DBT_TEST_ROLE_1=dbt_test_role_1 +DBT_TEST_ROLE_2=dbt_test_role_2 +DBT_TEST_ROLE_3=dbt_test_role_3 diff --git a/tests/functional/adapter/grants/base_grants.py b/tests/functional/adapter/grants/base_grants.py new file mode 100644 index 000000000..740faff92 --- /dev/null +++ b/tests/functional/adapter/grants/base_grants.py @@ -0,0 +1,47 @@ +from dbt.tests.adapter.grants.base_grants import BaseGrants +import pytest +import os +from dbt.tests.util import ( + relation_from_name, + get_connection, +) + +TEST_USER_ENV_VARS = ["DBT_TEST_USER_1", "DBT_TEST_USER_2", "DBT_TEST_USER_3"] +TEST_GROUP_ENV_VARS = ["DBT_TEST_GROUP_1", "DBT_TEST_GROUP_2", "DBT_TEST_GROUP_3"] +TEST_ROLE_ENV_VARS = ["DBT_TEST_ROLE_1", "DBT_TEST_ROLE_2", "DBT_TEST_ROLE_3"] + + +def replace_all(text, dic): + for i, j in dic.items(): + text = text.replace(i, j) + return text + + +def get_test_permissions(permission_env_vars): + test_permissions = [] + for env_var in permission_env_vars: + permission_name = os.getenv(env_var) + if permission_name: + test_permissions.append(permission_name) + return test_permissions + + +class BaseGrantsRedshift(BaseGrants): + + @pytest.fixture(scope="class", autouse=True) + def get_test_groups(self, project): + return get_test_permissions(TEST_GROUP_ENV_VARS) + + @pytest.fixture(scope="class", autouse=True) + def get_test_roles(self, project): + return get_test_permissions(TEST_ROLE_ENV_VARS) + + # This is an override of the BaseGrants class + def assert_expected_grants_match_actual(self, project, relation_name, expected_grants): + actual_grants = self.get_grants_on_relation(project, relation_name) + adapter = project.adapter + # need a case-insensitive comparison + # so just a simple "assert expected == actual_grants" won't work + diff_a = adapter.diff_of_two_nested_dicts(actual_grants, expected_grants) + diff_b = adapter.diff_of_two_nested_dicts(expected_grants, actual_grants) + assert diff_a == diff_b == {} diff --git a/tests/functional/adapter/grants/test_incremental_grants.py b/tests/functional/adapter/grants/test_incremental_grants.py new file mode 100644 index 000000000..8cee6d1e3 --- /dev/null +++ b/tests/functional/adapter/grants/test_incremental_grants.py @@ -0,0 +1,171 @@ +import pytest +from dbt.tests.util import ( + run_dbt, + run_dbt_and_capture, + get_manifest, + write_file, + relation_from_name, + get_connection, +) + +from tests.functional.adapter.grants.base_grants import BaseGrantsRedshift +# from tests.functional.adapter.grants import BaseGrantsRedshift + +my_incremental_model_sql = """ + select 1 as fun +""" + +incremental_model_schema_yml = """ +version: 2 +models: + - name: my_incremental_model + config: + materialized: incremental + grants: + select: ["{{ env_var('DBT_TEST_USER_1') }}"] +""" + +user2_incremental_model_schema_yml = """ +version: 2 +models: + - name: my_incremental_model + config: + materialized: incremental + grants: + select: ["{{ env_var('DBT_TEST_USER_2') }}"] +""" + +extended_incremental_model_schema_yml = """ +version: 2 +models: + - name: my_incremental_model + config: + materialized: incremental + grants: + select: + user: ["{{ env_var('DBT_TEST_USER_1') }}"] + group: ["{{ env_var('DBT_TEST_GROUP_1') }}"] + role: ["{{ env_var('DBT_TEST_ROLE_1') }}"] +""" + +extended2_incremental_model_schema_yml = """ +version: 2 +models: + - name: my_incremental_model + config: + materialized: incremental + grants: + select: + user: ["{{ env_var('DBT_TEST_USER_2') }}"] + group: ["{{ env_var('DBT_TEST_GROUP_2') }}"] + role: ["{{ env_var('DBT_TEST_ROLE_2') }}"] +""" + +class BaseIncrementalGrantsRedshift(BaseGrantsRedshift): + @pytest.fixture(scope="class") + def models(self): + updated_schema = self.interpolate_name_overrides(incremental_model_schema_yml) + return { + "my_incremental_model.sql": my_incremental_model_sql, + "schema.yml": updated_schema, + } + + def test_incremental_grants(self, project, get_test_users, get_test_groups, get_test_roles): + # for debugging + print("incremental test") + + # we want the test to fail, not silently skip + test_users = get_test_users + test_groups = get_test_groups + test_roles = get_test_roles + select_privilege_name = self.privilege_grantee_name_overrides()["select"] + assert len(test_users) == 3 + assert len(test_groups) == 3 + assert len(test_roles) == 3 + + # Incremental 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_incremental_model" + model = manifest.nodes[model_id] + assert model.config.materialized == "incremental" + expected = {select_privilege_name: {"user": [test_users[0]]}} + self.assert_expected_grants_match_actual(project, "my_incremental_model", expected) + + # Incremental materialization, run again without changes + (results, log_output) = run_dbt_and_capture(["--debug", "run"]) + assert len(results) == 1 + assert "revoke " not in log_output + assert "grant " not in log_output # with space to disambiguate from 'show grants' + self.assert_expected_grants_match_actual(project, "my_incremental_model", expected) + + # Incremental materialization, change select grant user + updated_yaml = self.interpolate_name_overrides(user2_incremental_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 + assert "revoke " in log_output + manifest = get_manifest(project.project_root) + model = manifest.nodes[model_id] + assert model.config.materialized == "incremental" + expected = {select_privilege_name: {"user": [test_users[1]]}} + self.assert_expected_grants_match_actual(project, "my_incremental_model", expected) + + # Incremental materialization, same config, now with --full-refresh + run_dbt(["--debug", "run", "--full-refresh"]) + assert len(results) == 1 + # whether grants or revokes happened will vary by adapter + self.assert_expected_grants_match_actual(project, "my_incremental_model", expected) + + # Now drop the schema (with the table in it) + adapter = project.adapter + relation = relation_from_name(adapter, "my_incremental_model") + with get_connection(adapter): + adapter.drop_schema(relation) + + # Incremental materialization, same config, rebuild now that table is missing + (results, log_output) = run_dbt_and_capture(["--debug", "run"]) + assert len(results) == 1 + assert "grant " in log_output + assert "revoke " not in log_output + self.assert_expected_grants_match_actual(project, "my_incremental_model", expected) + + ## Additional tests for privilege grants to extended permission types + + # Incremental materialization, single select grant + updated_yaml = self.interpolate_name_overrides(extended_incremental_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 + # select grant to *_1 users, groups, and roles, select revoke from DBT_TEST_USER_2 + assert "grant " in log_output + assert "revoke " in log_output + manifest = get_manifest(project.project_root) + model = manifest.nodes[model_id] + assert model.config.materialized == "incremental" + expected = {select_privilege_name: { + "user": [test_users[0]], + "group": [test_groups[0]], + "role": [test_roles[0]]} + } + self.assert_expected_grants_match_actual(project, "my_incremental_model", expected) + + # Incremental materialization, change select grant + updated_yaml = self.interpolate_name_overrides(extended2_incremental_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 + # select grant to *_2 users,groups, and roles, select revoke from *_1 users, groups, and roles + assert "grant " in log_output + assert "revoke " in log_output + manifest = get_manifest(project.project_root) + model = manifest.nodes[model_id] + assert model.config.materialized == "incremental" + expected = {select_privilege_name: { + "user": [test_users[1]], + "group": [test_groups[1]], + "role": [test_roles[1]]} + } + self.assert_expected_grants_match_actual(project, "my_incremental_model", expected) + diff --git a/tests/functional/adapter/grants/test_model_grants.py b/tests/functional/adapter/grants/test_model_grants.py new file mode 100644 index 000000000..f7260f575 --- /dev/null +++ b/tests/functional/adapter/grants/test_model_grants.py @@ -0,0 +1,293 @@ +import pytest +from dbt.tests.util import ( + run_dbt_and_capture, + get_manifest, + write_file, +) +from tests.functional.adapter.grants.base_grants import BaseGrantsRedshift + +# from tests.functional.adapter.grants import BaseGrantsRedshift + +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') }}"] +""" + +# table materialization single select +extended_table_model_schema_yml = """ +version: 2 +models: + - name: my_model + config: + materialized: table + grants: + select: + user: ["{{ env_var('DBT_TEST_USER_1') }}"] + group: ["{{ env_var('DBT_TEST_GROUP_1') }}"] + role: ["{{ env_var('DBT_TEST_ROLE_1') }}"] +""" + +# table materialization change select +extended2_table_model_schema_yml = """ +version: 2 +models: + - name: my_model + config: + materialized: table + grants: + select: + user: ["{{ env_var('DBT_TEST_USER_2') }}"] + group: ["{{ env_var('DBT_TEST_GROUP_2') }}"] + role: ["{{ env_var('DBT_TEST_ROLE_2') }}"] +""" + +# table materialization multiple grantees +extended_multiple_grantees_table_model_schema_yml = """ +version: 2 +models: + - name: my_model + config: + materialized: table + grants: + select: + user: ["{{ env_var('DBT_TEST_USER_1') }}", "{{ env_var('DBT_TEST_USER_2') }}"] + group: ["{{ env_var('DBT_TEST_GROUP_1') }}", "{{ env_var('DBT_TEST_GROUP_2') }}"] + role: ["{{ env_var('DBT_TEST_ROLE_1') }}", "{{ env_var('DBT_TEST_ROLE_2') }}"] +""" +# table materialization multiple privileges +extended_multiple_privileges_table_model_schema_yml = """ +version: 2 +models: + - name: my_model + config: + materialized: table + grants: + select: + user: ["{{ env_var('DBT_TEST_USER_1') }}"] + group: ["{{ env_var('DBT_TEST_GROUP_1') }}"] + role: ["{{ env_var('DBT_TEST_ROLE_1') }}"] + insert: + user: ["{{ env_var('DBT_TEST_USER_2') }}"] + group: ["{{ env_var('DBT_TEST_GROUP_2') }}"] + role: ["{{ env_var('DBT_TEST_ROLE_2') }}"] +""" + + +class BaseModelGrantsRedshift(BaseGrantsRedshift): + @pytest.fixture(scope="class") + def models(self): + updated_schema = self.interpolate_name_overrides(model_schema_yml) + return { + "my_model.sql": my_model_sql, + "schema.yml": updated_schema, + } + + def test_view_table_grants(self, project, get_test_users, get_test_groups, get_test_roles): + ## Override/refactor the tests from dbt-core ## + + # we want the test to fail, not silently skip + test_users = get_test_users + test_groups = get_test_groups + test_roles = get_test_roles + select_privilege_name = self.privilege_grantee_name_overrides()["select"] + insert_privilege_name = self.privilege_grantee_name_overrides()["insert"] + assert len(test_users) == 3 + assert len(test_groups) == 3 + assert len(test_roles) == 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] + # user configuration for grants + user_expected = {select_privilege_name: [test_users[0]]} + assert model.config.grants == user_expected + assert model.config.materialized == "view" + # new configuration for grants + expected = {select_privilege_name: {"user": [test_users[0]]}} + self.assert_expected_grants_match_actual(project, "my_model", expected) + + # View materialization, change select grant user + updated_yaml = self.interpolate_name_overrides(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: {"user": [test_users[1]]}} + self.assert_expected_grants_match_actual(project, "my_model", expected) + + # Table materialization, single select grant + updated_yaml = self.interpolate_name_overrides(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: {"user": [test_users[0]]}} + self.assert_expected_grants_match_actual(project, "my_model", expected) + + # Table materialization, change select grant user + updated_yaml = self.interpolate_name_overrides(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: {"user": [test_users[1]]}} + self.assert_expected_grants_match_actual(project, "my_model", expected) + + # Table materialization, multiple grantees + updated_yaml = self.interpolate_name_overrides(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: {"user": [test_users[0], test_users[1]]}} + self.assert_expected_grants_match_actual(project, "my_model", expected) + + # Table materialization, multiple privileges + updated_yaml = self.interpolate_name_overrides(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] + user_expected = {select_privilege_name: [test_users[0]], insert_privilege_name: [test_users[1]]} + assert model.config.grants == user_expected + assert model.config.materialized == "table" + expected = {select_privilege_name: {"user": [test_users[0]]}, insert_privilege_name: {"user": [test_users[1]]}} + self.assert_expected_grants_match_actual(project, "my_model", expected) + + ## Additional tests for privilege grants to extended permission types + + # Table materialization, single select grant + updated_yaml = self.interpolate_name_overrides(extended_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: { + "user": [test_users[0]], + "group": [test_groups[0]], + "role": [test_roles[0]]} + } + self.assert_expected_grants_match_actual(project, "my_model", expected) + + # Table materialization, change select grant + updated_yaml = self.interpolate_name_overrides(extended2_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: { + "user": [test_users[1]], + "group": [test_groups[1]], + "role": [test_roles[1]]} + } + self.assert_expected_grants_match_actual(project, "my_model", expected) + + # Table materialization, multiple grantees + updated_yaml = self.interpolate_name_overrides(extended_multiple_grantees_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: { + "user": [test_users[0], test_users[1]], + "group": [test_groups[0], test_groups[1]], + "role": [test_roles[0], test_roles[1]]} + } + self.assert_expected_grants_match_actual(project, "my_model", expected) + + # Table materialization, multiple privileges + updated_yaml = self.interpolate_name_overrides(extended_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: { + "user": [test_users[0]], + "group": [test_groups[0]], + "role": [test_roles[0]] + }, + insert_privilege_name: { + "user": [test_users[1]], + "group": [test_groups[1]], + "role": [test_roles[1]] + } + } + self.assert_expected_grants_match_actual(project, "my_model", expected) diff --git a/tests/functional/adapter/grants/test_seed_grants.py b/tests/functional/adapter/grants/test_seed_grants.py new file mode 100644 index 000000000..c826b8e15 --- /dev/null +++ b/tests/functional/adapter/grants/test_seed_grants.py @@ -0,0 +1,171 @@ +import pytest +from dbt.tests.util import ( + run_dbt, + run_dbt_and_capture, + get_manifest, + write_file, +) +from tests.functional.adapter.grants.base_grants import BaseGrantsRedshift +# from dbt.tests.adapter.grants.base_grants import BaseGrantsRedshift + +seeds__my_seed_csv = """ +id,name,some_date +1,Easton,1981-05-20T06:46:51 +2,Lillian,1978-09-03T18:10:33 +""".lstrip() + +# rewrite this +schema_base_yml = """ +version: 2 +seeds: + - name: my_seed + config: + grants: + select: ["{{ env_var('DBT_TEST_USER_1') }}"] +""" + +# rewrite this +user2_schema_base_yml = """ +version: 2 +seeds: + - name: my_seed + config: + grants: + select: ["{{ env_var('DBT_TEST_USER_2') }}"] +""" + +ignore_grants_yml = """ +version: 2 +seeds: + - name: my_seed + config: + grants: {} +""" + +zero_grants_yml = """ +version: 2 +seeds: + - name: my_seed + config: + grants: + select: [] +""" + +extended_zero_grants_yml = """ +version: 2 +seeds: + - name: my_seed + config: + grants: + select: + user: [] + group: [] + role: [] +""" + + +class BaseSeedGrantsRedshift(BaseGrantsRedshift): + def seeds_support_partial_refresh(self): + return True + + @pytest.fixture(scope="class") + def seeds(self): + updated_schema = self.interpolate_name_overrides(schema_base_yml) + return { + "my_seed.csv": seeds__my_seed_csv, + "schema.yml": updated_schema, + } + + def test_seed_grants(self, project, get_test_users, get_test_groups, get_test_roles): + # debugging for seeds + print("seed testing") + + test_users = get_test_users + test_groups = get_test_groups + test_roles = get_test_roles + select_privilege_name = self.privilege_grantee_name_overrides()["select"] + + # seed command + (results, log_output) = run_dbt_and_capture(["--debug", "seed"]) + assert len(results) == 1 + manifest = get_manifest(project.project_root) + seed_id = "seed.test.my_seed" + seed = manifest.nodes[seed_id] + user_expected = {select_privilege_name: [test_users[0]]} + assert seed.config.grants == user_expected + assert "grant " in log_output + expected = {select_privilege_name: {"user": [test_users[0]]}} + self.assert_expected_grants_match_actual(project, "my_seed", expected) + + # run it again, with no config changes + (results, log_output) = run_dbt_and_capture(["--debug", "seed"]) + assert len(results) == 1 + if self.seeds_support_partial_refresh(): + # grants carried over -- nothing should have changed + assert "revoke " not in log_output + assert "grant " not in log_output + else: + # seeds are always full-refreshed on this adapter, so we need to re-grant + assert "grant " in log_output + self.assert_expected_grants_match_actual(project, "my_seed", expected) + + # change the grantee, assert it updates + updated_yaml = self.interpolate_name_overrides(user2_schema_base_yml) + write_file(updated_yaml, project.project_root, "seeds", "schema.yml") + (results, log_output) = run_dbt_and_capture(["--debug", "seed"]) + assert len(results) == 1 + expected = {select_privilege_name: {"user": [test_users[1]]}} + self.assert_expected_grants_match_actual(project, "my_seed", expected) + + # run it again, with --full-refresh, grants should be the same + run_dbt(["seed", "--full-refresh"]) + self.assert_expected_grants_match_actual(project, "my_seed", expected) + + # change config to 'grants: {}' -- should be completely ignored + updated_yaml = self.interpolate_name_overrides(ignore_grants_yml) + write_file(updated_yaml, project.project_root, "seeds", "schema.yml") + (results, log_output) = run_dbt_and_capture(["--debug", "seed"]) + assert len(results) == 1 + assert "revoke " not in log_output + assert "grant " not in log_output + manifest = get_manifest(project.project_root) + seed_id = "seed.test.my_seed" + seed = manifest.nodes[seed_id] + expected_config = {} + expected_actual = {select_privilege_name: {"user": [test_users[1]]}} + assert seed.config.grants == expected_config + if self.seeds_support_partial_refresh(): + # ACTUAL grants will NOT match expected grants + self.assert_expected_grants_match_actual(project, "my_seed", expected_actual) + else: + # there should be ZERO grants on the seed + self.assert_expected_grants_match_actual(project, "my_seed", expected_config) + + # now run with ZERO grants -- all grants should be removed + # whether explicitly (revoke) or implicitly (recreated without any grants added on) + updated_yaml = self.interpolate_name_overrides(zero_grants_yml) + write_file(updated_yaml, project.project_root, "seeds", "schema.yml") + (results, log_output) = run_dbt_and_capture(["--debug", "seed"]) + assert len(results) == 1 + if self.seeds_support_partial_refresh(): + assert "revoke " in log_output + expected = {} + self.assert_expected_grants_match_actual(project, "my_seed", expected) + + # run it again -- dbt shouldn't try to grant or revoke anything + (results, log_output) = run_dbt_and_capture(["--debug", "seed"]) + assert len(results) == 1 + assert "revoke " not in log_output + assert "grant " not in log_output + self.assert_expected_grants_match_actual(project, "my_seed", expected) + + # run with ZERO grants -- since all grants were removed previously + # nothing should have changed + updated_yaml = self.interpolate_name_overrides(extended_zero_grants_yml) + write_file(updated_yaml, project.project_root, "seeds", "schema.yml") + (results, log_output) = run_dbt_and_capture(["--debug", "seed"]) + assert len(results) == 1 + assert "grant " not in log_output + assert "revoke " not in log_output + expected = {} + self.assert_expected_grants_match_actual(project, "my_seed", expected) diff --git a/tests/functional/adapter/grants/test_snapshot_grants.py b/tests/functional/adapter/grants/test_snapshot_grants.py new file mode 100644 index 000000000..db6cf02ba --- /dev/null +++ b/tests/functional/adapter/grants/test_snapshot_grants.py @@ -0,0 +1,129 @@ +import pytest +from dbt.tests.util import ( + run_dbt, + run_dbt_and_capture, + get_manifest, + write_file, +) + +from tests.functional.adapter.grants.base_grants import BaseGrantsRedshift +# from tests.functional.adapter.grants import BaseGrantsRedshift + +my_snapshot_sql = """ +{% snapshot my_snapshot %} + {{ config( + check_cols='all', unique_key='id', strategy='check', + target_database=database, target_schema=schema + ) }} + select 1 as id, cast('blue' as {{ type_string() }}) as color +{% endsnapshot %} +""".strip() + +snapshot_schema_yml = """ +version: 2 +snapshots: + - name: my_snapshot + config: + grants: + select: ["{{ env_var('DBT_TEST_USER_1') }}"] +""" + +user2_snapshot_schema_yml = """ +version: 2 +snapshots: + - name: my_snapshot + config: + grants: + select: ["{{ env_var('DBT_TEST_USER_2') }}"] +""" + +extended_snapshot_schema_yml = """ +version: 2 +snapshots: + - name: my_snapshot + config: + grants: + select: + user: ["{{ env_var('DBT_TEST_USER_1') }}"] + group: ["{{ env_var('DBT_TEST_GROUP_1') }}"] + role: ["{{ env_var('DBT_TEST_ROLE_1') }}"] +""" + +extended2_snapshot_schema_yml = """ +version: 2 +snapshots: + - name: my_snapshot + config: + grants: + select: + user: ["{{ env_var('DBT_TEST_USER_2') }}"] + group: ["{{ env_var('DBT_TEST_GROUP_2') }}"] + role: ["{{ env_var('DBT_TEST_ROLE_2') }}"] +""" + + +class BaseSnapshotGrantsRedshift(BaseGrantsRedshift): + @pytest.fixture(scope="class") + def snapshots(self): + return { + "my_snapshot.sql": my_snapshot_sql, + "schema.yml": self.interpolate_name_overrides(snapshot_schema_yml), + } + + def test_snapshot_grants(self, project, get_test_users, get_test_groups, get_test_roles): + + print("snapshot testing") + test_users = get_test_users + test_groups = get_test_groups + test_roles = get_test_roles + select_privilege_name = self.privilege_grantee_name_overrides()["select"] + + # run the snapshot + results = run_dbt(["snapshot"]) + assert len(results) == 1 + manifest = get_manifest(project.project_root) + snapshot_id = "snapshot.test.my_snapshot" + snapshot = manifest.nodes[snapshot_id] + user_expected = {select_privilege_name: [test_users[0]]} + assert snapshot.config.grants == user_expected + expected = {select_privilege_name: {"user": [test_users[0]]}} + self.assert_expected_grants_match_actual(project, "my_snapshot", expected) + + # run it again, nothing should have changed + (results, log_output) = run_dbt_and_capture(["--debug", "snapshot"]) + assert len(results) == 1 + assert "revoke " not in log_output + assert "grant " not in log_output + self.assert_expected_grants_match_actual(project, "my_snapshot", expected) + + # change the grantee, assert it updates + updated_yaml = self.interpolate_name_overrides(user2_snapshot_schema_yml) + write_file(updated_yaml, project.project_root, "snapshots", "schema.yml") + (results, log_output) = run_dbt_and_capture(["--debug", "snapshot"]) + assert len(results) == 1 + expected = {select_privilege_name: {"user": [test_users[1]]}} + self.assert_expected_grants_match_actual(project, "my_snapshot", expected) + + # change the grants, assert that it updates + updated_yaml = self.interpolate_name_overrides(extended_snapshot_schema_yml) + write_file(updated_yaml, project.project_root, "snapshots", "schema.yml") + (results, log_output) = run_dbt_and_capture(["--debug", "snapshot"]) + assert len(results) == 1 + expected = {select_privilege_name: { + "user": [test_users[0]], + "group": [test_groups[0]], + "role": [test_roles[0]]} + } + self.assert_expected_grants_match_actual(project, "my_snapshot", expected) + + # change the grants again, assert that it updates + updated_yaml = self.interpolate_name_overrides(extended2_snapshot_schema_yml) + write_file(updated_yaml, project.project_root, "snapshots", "schema.yml") + (results, log_output) = run_dbt_and_capture(["--debug", "snapshot"]) + assert len(results) == 1 + expected = {select_privilege_name: { + "user": [test_users[1]], + "group": [test_groups[1]], + "role": [test_roles[1]]} + } + self.assert_expected_grants_match_actual(project, "my_snapshot", expected) diff --git a/tests/functional/adapter/test_grants.py b/tests/functional/adapter/test_grants.py index b627e450a..fe9ab87b9 100644 --- a/tests/functional/adapter/test_grants.py +++ b/tests/functional/adapter/test_grants.py @@ -1,24 +1,24 @@ -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_seed_grants import BaseSeedGrants -from dbt.tests.adapter.grants.test_snapshot_grants import BaseSnapshotGrants +from grants.test_model_grants import BaseModelGrantsRedshift +from grants.test_snapshot_grants import BaseSnapshotGrantsRedshift +from grants.test_seed_grants import BaseSeedGrantsRedshift +from grants.test_incremental_grants import BaseIncrementalGrantsRedshift -class TestModelGrantsRedshift(BaseModelGrants): +class TestModelGrantsRedshift(BaseModelGrantsRedshift): pass -class TestIncrementalGrantsRedshift(BaseIncrementalGrants): +class TestIncrementalGrantsRedshift(BaseIncrementalGrantsRedshift): pass -class TestSeedGrantsRedshift(BaseSeedGrants): +class TestSeedGrantsRedshift(BaseSeedGrantsRedshift): pass -class TestSnapshotGrantsRedshift(BaseSnapshotGrants): +class TestSnapshotGrantsRedshift(BaseSnapshotGrantsRedshift): pass -class TestInvalidGrantsRedshift(BaseModelGrants): - pass +class TestInvalidGrantsRedshift(BaseModelGrantsRedshift): + pass \ No newline at end of file From a0ca37d930301a7108e4b5cd3a554cdccf583737 Mon Sep 17 00:00:00 2001 From: Soksamnang Lim Date: Wed, 4 Oct 2023 10:39:55 -0700 Subject: [PATCH 02/14] Support Privilege Grants to Roles and Groups in Materialization --- .changes/unreleased/Features-20231004-103938.yaml | 6 ++++++ 1 file changed, 6 insertions(+) create mode 100644 .changes/unreleased/Features-20231004-103938.yaml diff --git a/.changes/unreleased/Features-20231004-103938.yaml b/.changes/unreleased/Features-20231004-103938.yaml new file mode 100644 index 000000000..7c9090171 --- /dev/null +++ b/.changes/unreleased/Features-20231004-103938.yaml @@ -0,0 +1,6 @@ +kind: Features +body: 'Support groups and roles for grants ' +time: 2023-10-04T10:39:38.680813-07:00 +custom: + Author: soksamnanglim + Issue: "415" From ca046c98b025daf3ef1be42139416ef1328d51b6 Mon Sep 17 00:00:00 2001 From: Soksamnang Lim Date: Wed, 4 Oct 2023 13:00:47 -0700 Subject: [PATCH 03/14] rectified code with pre-commit --- dbt/adapters/redshift/impl.py | 4 +- .../functional/adapter/grants/base_grants.py | 5 -- .../adapter/grants/test_incremental_grants.py | 34 +++++----- .../adapter/grants/test_model_grants.py | 62 +++++++++++-------- .../adapter/grants/test_seed_grants.py | 8 +-- .../adapter/grants/test_snapshot_grants.py | 31 +++++----- tests/functional/adapter/test_grants.py | 2 +- 7 files changed, 78 insertions(+), 68 deletions(-) diff --git a/dbt/adapters/redshift/impl.py b/dbt/adapters/redshift/impl.py index cfea6237f..4a497fa03 100644 --- a/dbt/adapters/redshift/impl.py +++ b/dbt/adapters/redshift/impl.py @@ -166,7 +166,7 @@ def debug_query(self): """Override for DebugTask method""" self.execute("select 1 as id") - ## grant-related methods ## + # grant-related methods @available def standardize_grants_dict(self, grants_table): """ @@ -254,4 +254,4 @@ def process_grant_dicts(self, unknown_dict): if grantees_map_temp: temp[privilege] = grantees_map_temp - return temp \ No newline at end of file + return temp diff --git a/tests/functional/adapter/grants/base_grants.py b/tests/functional/adapter/grants/base_grants.py index 740faff92..22a614d01 100644 --- a/tests/functional/adapter/grants/base_grants.py +++ b/tests/functional/adapter/grants/base_grants.py @@ -1,10 +1,6 @@ from dbt.tests.adapter.grants.base_grants import BaseGrants import pytest import os -from dbt.tests.util import ( - relation_from_name, - get_connection, -) TEST_USER_ENV_VARS = ["DBT_TEST_USER_1", "DBT_TEST_USER_2", "DBT_TEST_USER_3"] TEST_GROUP_ENV_VARS = ["DBT_TEST_GROUP_1", "DBT_TEST_GROUP_2", "DBT_TEST_GROUP_3"] @@ -27,7 +23,6 @@ def get_test_permissions(permission_env_vars): class BaseGrantsRedshift(BaseGrants): - @pytest.fixture(scope="class", autouse=True) def get_test_groups(self, project): return get_test_permissions(TEST_GROUP_ENV_VARS) diff --git a/tests/functional/adapter/grants/test_incremental_grants.py b/tests/functional/adapter/grants/test_incremental_grants.py index 8cee6d1e3..77f982d97 100644 --- a/tests/functional/adapter/grants/test_incremental_grants.py +++ b/tests/functional/adapter/grants/test_incremental_grants.py @@ -9,7 +9,6 @@ ) from tests.functional.adapter.grants.base_grants import BaseGrantsRedshift -# from tests.functional.adapter.grants import BaseGrantsRedshift my_incremental_model_sql = """ select 1 as fun @@ -41,8 +40,8 @@ - name: my_incremental_model config: materialized: incremental - grants: - select: + grants: + select: user: ["{{ env_var('DBT_TEST_USER_1') }}"] group: ["{{ env_var('DBT_TEST_GROUP_1') }}"] role: ["{{ env_var('DBT_TEST_ROLE_1') }}"] @@ -54,13 +53,14 @@ - name: my_incremental_model config: materialized: incremental - grants: - select: + grants: + select: user: ["{{ env_var('DBT_TEST_USER_2') }}"] group: ["{{ env_var('DBT_TEST_GROUP_2') }}"] role: ["{{ env_var('DBT_TEST_ROLE_2') }}"] """ + class BaseIncrementalGrantsRedshift(BaseGrantsRedshift): @pytest.fixture(scope="class") def models(self): @@ -131,8 +131,7 @@ def test_incremental_grants(self, project, get_test_users, get_test_groups, get_ assert "revoke " not in log_output self.assert_expected_grants_match_actual(project, "my_incremental_model", expected) - ## Additional tests for privilege grants to extended permission types - + # Additional tests for privilege grants to extended permission types # Incremental materialization, single select grant updated_yaml = self.interpolate_name_overrides(extended_incremental_model_schema_yml) write_file(updated_yaml, project.project_root, "models", "schema.yml") @@ -144,10 +143,12 @@ def test_incremental_grants(self, project, get_test_users, get_test_groups, get_ manifest = get_manifest(project.project_root) model = manifest.nodes[model_id] assert model.config.materialized == "incremental" - expected = {select_privilege_name: { - "user": [test_users[0]], - "group": [test_groups[0]], - "role": [test_roles[0]]} + expected = { + select_privilege_name: { + "user": [test_users[0]], + "group": [test_groups[0]], + "role": [test_roles[0]], + } } self.assert_expected_grants_match_actual(project, "my_incremental_model", expected) @@ -162,10 +163,11 @@ def test_incremental_grants(self, project, get_test_users, get_test_groups, get_ manifest = get_manifest(project.project_root) model = manifest.nodes[model_id] assert model.config.materialized == "incremental" - expected = {select_privilege_name: { - "user": [test_users[1]], - "group": [test_groups[1]], - "role": [test_roles[1]]} + expected = { + select_privilege_name: { + "user": [test_users[1]], + "group": [test_groups[1]], + "role": [test_roles[1]], + } } self.assert_expected_grants_match_actual(project, "my_incremental_model", expected) - diff --git a/tests/functional/adapter/grants/test_model_grants.py b/tests/functional/adapter/grants/test_model_grants.py index f7260f575..1e7ff5f9f 100644 --- a/tests/functional/adapter/grants/test_model_grants.py +++ b/tests/functional/adapter/grants/test_model_grants.py @@ -6,8 +6,6 @@ ) from tests.functional.adapter.grants.base_grants import BaseGrantsRedshift -# from tests.functional.adapter.grants import BaseGrantsRedshift - my_model_sql = """ select 1 as fun """ @@ -141,8 +139,7 @@ def models(self): } def test_view_table_grants(self, project, get_test_users, get_test_groups, get_test_roles): - ## Override/refactor the tests from dbt-core ## - + # Override/refactor the tests from dbt-core # # we want the test to fail, not silently skip test_users = get_test_users test_groups = get_test_groups @@ -217,14 +214,19 @@ def test_view_table_grants(self, project, get_test_users, get_test_groups, get_t assert len(results) == 1 manifest = get_manifest(project.project_root) model = manifest.nodes[model_id] - user_expected = {select_privilege_name: [test_users[0]], insert_privilege_name: [test_users[1]]} + user_expected = { + select_privilege_name: [test_users[0]], + insert_privilege_name: [test_users[1]], + } assert model.config.grants == user_expected assert model.config.materialized == "table" - expected = {select_privilege_name: {"user": [test_users[0]]}, insert_privilege_name: {"user": [test_users[1]]}} + expected = { + select_privilege_name: {"user": [test_users[0]]}, + insert_privilege_name: {"user": [test_users[1]]}, + } self.assert_expected_grants_match_actual(project, "my_model", expected) - ## Additional tests for privilege grants to extended permission types - + # Additional tests for privilege grants to extended permission types # Table materialization, single select grant updated_yaml = self.interpolate_name_overrides(extended_table_model_schema_yml) write_file(updated_yaml, project.project_root, "models", "schema.yml") @@ -233,10 +235,12 @@ def test_view_table_grants(self, project, get_test_users, get_test_groups, get_t manifest = get_manifest(project.project_root) model = manifest.nodes[model_id] assert model.config.materialized == "table" - expected = {select_privilege_name: { - "user": [test_users[0]], - "group": [test_groups[0]], - "role": [test_roles[0]]} + expected = { + select_privilege_name: { + "user": [test_users[0]], + "group": [test_groups[0]], + "role": [test_roles[0]], + } } self.assert_expected_grants_match_actual(project, "my_model", expected) @@ -248,30 +252,38 @@ def test_view_table_grants(self, project, get_test_users, get_test_groups, get_t manifest = get_manifest(project.project_root) model = manifest.nodes[model_id] assert model.config.materialized == "table" - expected = {select_privilege_name: { - "user": [test_users[1]], - "group": [test_groups[1]], - "role": [test_roles[1]]} + expected = { + select_privilege_name: { + "user": [test_users[1]], + "group": [test_groups[1]], + "role": [test_roles[1]], + } } self.assert_expected_grants_match_actual(project, "my_model", expected) # Table materialization, multiple grantees - updated_yaml = self.interpolate_name_overrides(extended_multiple_grantees_table_model_schema_yml) + updated_yaml = self.interpolate_name_overrides( + extended_multiple_grantees_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: { - "user": [test_users[0], test_users[1]], - "group": [test_groups[0], test_groups[1]], - "role": [test_roles[0], test_roles[1]]} + expected = { + select_privilege_name: { + "user": [test_users[0], test_users[1]], + "group": [test_groups[0], test_groups[1]], + "role": [test_roles[0], test_roles[1]], + } } self.assert_expected_grants_match_actual(project, "my_model", expected) # Table materialization, multiple privileges - updated_yaml = self.interpolate_name_overrides(extended_multiple_privileges_table_model_schema_yml) + updated_yaml = self.interpolate_name_overrides( + extended_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 @@ -282,12 +294,12 @@ def test_view_table_grants(self, project, get_test_users, get_test_groups, get_t select_privilege_name: { "user": [test_users[0]], "group": [test_groups[0]], - "role": [test_roles[0]] + "role": [test_roles[0]], }, insert_privilege_name: { "user": [test_users[1]], "group": [test_groups[1]], - "role": [test_roles[1]] - } + "role": [test_roles[1]], + }, } self.assert_expected_grants_match_actual(project, "my_model", expected) diff --git a/tests/functional/adapter/grants/test_seed_grants.py b/tests/functional/adapter/grants/test_seed_grants.py index c826b8e15..c87c68716 100644 --- a/tests/functional/adapter/grants/test_seed_grants.py +++ b/tests/functional/adapter/grants/test_seed_grants.py @@ -6,7 +6,7 @@ write_file, ) from tests.functional.adapter.grants.base_grants import BaseGrantsRedshift -# from dbt.tests.adapter.grants.base_grants import BaseGrantsRedshift + seeds__my_seed_csv = """ id,name,some_date @@ -57,7 +57,7 @@ - name: my_seed config: grants: - select: + select: user: [] group: [] role: [] @@ -76,13 +76,11 @@ def seeds(self): "schema.yml": updated_schema, } - def test_seed_grants(self, project, get_test_users, get_test_groups, get_test_roles): + def test_seed_grants(self, project, get_test_users): # debugging for seeds print("seed testing") test_users = get_test_users - test_groups = get_test_groups - test_roles = get_test_roles select_privilege_name = self.privilege_grantee_name_overrides()["select"] # seed command diff --git a/tests/functional/adapter/grants/test_snapshot_grants.py b/tests/functional/adapter/grants/test_snapshot_grants.py index db6cf02ba..b2ff5e6f1 100644 --- a/tests/functional/adapter/grants/test_snapshot_grants.py +++ b/tests/functional/adapter/grants/test_snapshot_grants.py @@ -7,7 +7,7 @@ ) from tests.functional.adapter.grants.base_grants import BaseGrantsRedshift -# from tests.functional.adapter.grants import BaseGrantsRedshift + my_snapshot_sql = """ {% snapshot my_snapshot %} @@ -42,8 +42,8 @@ snapshots: - name: my_snapshot config: - grants: - select: + grants: + select: user: ["{{ env_var('DBT_TEST_USER_1') }}"] group: ["{{ env_var('DBT_TEST_GROUP_1') }}"] role: ["{{ env_var('DBT_TEST_ROLE_1') }}"] @@ -54,8 +54,8 @@ snapshots: - name: my_snapshot config: - grants: - select: + grants: + select: user: ["{{ env_var('DBT_TEST_USER_2') }}"] group: ["{{ env_var('DBT_TEST_GROUP_2') }}"] role: ["{{ env_var('DBT_TEST_ROLE_2') }}"] @@ -71,7 +71,6 @@ def snapshots(self): } def test_snapshot_grants(self, project, get_test_users, get_test_groups, get_test_roles): - print("snapshot testing") test_users = get_test_users test_groups = get_test_groups @@ -109,10 +108,12 @@ def test_snapshot_grants(self, project, get_test_users, get_test_groups, get_tes write_file(updated_yaml, project.project_root, "snapshots", "schema.yml") (results, log_output) = run_dbt_and_capture(["--debug", "snapshot"]) assert len(results) == 1 - expected = {select_privilege_name: { - "user": [test_users[0]], - "group": [test_groups[0]], - "role": [test_roles[0]]} + expected = { + select_privilege_name: { + "user": [test_users[0]], + "group": [test_groups[0]], + "role": [test_roles[0]], + } } self.assert_expected_grants_match_actual(project, "my_snapshot", expected) @@ -121,9 +122,11 @@ def test_snapshot_grants(self, project, get_test_users, get_test_groups, get_tes write_file(updated_yaml, project.project_root, "snapshots", "schema.yml") (results, log_output) = run_dbt_and_capture(["--debug", "snapshot"]) assert len(results) == 1 - expected = {select_privilege_name: { - "user": [test_users[1]], - "group": [test_groups[1]], - "role": [test_roles[1]]} + expected = { + select_privilege_name: { + "user": [test_users[1]], + "group": [test_groups[1]], + "role": [test_roles[1]], + } } self.assert_expected_grants_match_actual(project, "my_snapshot", expected) diff --git a/tests/functional/adapter/test_grants.py b/tests/functional/adapter/test_grants.py index fe9ab87b9..9dcc63c77 100644 --- a/tests/functional/adapter/test_grants.py +++ b/tests/functional/adapter/test_grants.py @@ -21,4 +21,4 @@ class TestSnapshotGrantsRedshift(BaseSnapshotGrantsRedshift): class TestInvalidGrantsRedshift(BaseModelGrantsRedshift): - pass \ No newline at end of file + pass From 2bb40b8eefc87c6c2351cbdb6bdd0a5e7f71fa8e Mon Sep 17 00:00:00 2001 From: Soksamnang Lim Date: Wed, 4 Oct 2023 13:04:08 -0700 Subject: [PATCH 04/14] Update CI workflow to include new env vars --- .github/workflows/integration.yml | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/.github/workflows/integration.yml b/.github/workflows/integration.yml index 9d1fe0807..371cbc291 100644 --- a/.github/workflows/integration.yml +++ b/.github/workflows/integration.yml @@ -185,6 +185,12 @@ jobs: DBT_TEST_USER_1: dbt_test_user_1 DBT_TEST_USER_2: dbt_test_user_2 DBT_TEST_USER_3: dbt_test_user_3 + DBT_TEST_GROUP_1: dbt_test_group_1 + DBT_TEST_GROUP_2: dbt_test_group_2 + DBT_TEST_GROUP_3: dbt_test_group_3 + DBT_TEST_ROLE_1: dbt_test_role_1 + DBT_TEST_ROLE_2: dbt_test_role_2 + DBT_TEST_ROLE_3: dbt_test_role_3 run: tox -- --ddtrace - uses: actions/upload-artifact@v3 From f464d442c5c7808c23f584dde040cf467524d161 Mon Sep 17 00:00:00 2001 From: Colin Date: Mon, 9 Oct 2023 14:23:45 -0700 Subject: [PATCH 05/14] Add fixture to create grants/roles for test purposes --- tests/functional/adapter/conftest.py | 32 +++++++++++++++++++ .../adapter/grants/test_model_grants.py | 2 +- 2 files changed, 33 insertions(+), 1 deletion(-) diff --git a/tests/functional/adapter/conftest.py b/tests/functional/adapter/conftest.py index c5c980154..6cf4557e4 100644 --- a/tests/functional/adapter/conftest.py +++ b/tests/functional/adapter/conftest.py @@ -1,4 +1,36 @@ import pytest +import os + +from dbt.exceptions import DbtDatabaseError + +# This is a hack to prevent the fixture from running more than once +GRANTS_AND_ROLES_SETUP = False + + +@pytest.fixture(scope="class", autouse=True) +def setup_grants_and_roles(project): + global GRANTS_AND_ROLES_SETUP + groups = [ + os.environ[env_var] for env_var in os.environ if env_var.startswith("DBT_TEST_GROUP_") + ] + roles = [os.environ[env_var] for env_var in os.environ if env_var.startswith("DBT_TEST_ROLE_")] + if not GRANTS_AND_ROLES_SETUP: + with project.adapter.connection_named("__test"): + for group in groups: + try: + project.adapter.execute(f"CREATE GROUP {group}") + except DbtDatabaseError: + # This is expected if the group already exists + pass + + for role in roles: + try: + project.adapter.execute(f"CREATE ROLE {role}") + except DbtDatabaseError: + # This is expected if the group already exists + pass + + GRANTS_AND_ROLES_SETUP = True @pytest.fixture diff --git a/tests/functional/adapter/grants/test_model_grants.py b/tests/functional/adapter/grants/test_model_grants.py index 1e7ff5f9f..1841ff645 100644 --- a/tests/functional/adapter/grants/test_model_grants.py +++ b/tests/functional/adapter/grants/test_model_grants.py @@ -129,7 +129,7 @@ """ -class BaseModelGrantsRedshift(BaseGrantsRedshift): +class TestModelGrantsRedshift(BaseGrantsRedshift): @pytest.fixture(scope="class") def models(self): updated_schema = self.interpolate_name_overrides(model_schema_yml) From 72a24104a8e688d88116f03f07bbefd568c9455b Mon Sep 17 00:00:00 2001 From: Soksamnang Lim Date: Wed, 11 Oct 2023 17:07:30 -0700 Subject: [PATCH 06/14] Fixed `ImportError` related to misnaming BaseModelGrantsRedshift --- tests/functional/adapter/grants/test_model_grants.py | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/tests/functional/adapter/grants/test_model_grants.py b/tests/functional/adapter/grants/test_model_grants.py index 1841ff645..3f0220eae 100644 --- a/tests/functional/adapter/grants/test_model_grants.py +++ b/tests/functional/adapter/grants/test_model_grants.py @@ -129,7 +129,7 @@ """ -class TestModelGrantsRedshift(BaseGrantsRedshift): +class BaseModelGrantsRedshift(BaseGrantsRedshift): @pytest.fixture(scope="class") def models(self): updated_schema = self.interpolate_name_overrides(model_schema_yml) @@ -303,3 +303,7 @@ def test_view_table_grants(self, project, get_test_users, get_test_groups, get_t }, } self.assert_expected_grants_match_actual(project, "my_model", expected) + + +class TestModelGrantsRedshift(BaseModelGrantsRedshift): + pass From 0f6442eb69374286b4a35a2c544b0e32f228ad84 Mon Sep 17 00:00:00 2001 From: Soksamnang Lim Date: Wed, 11 Oct 2023 17:08:07 -0700 Subject: [PATCH 07/14] Remove debugging comment --- tests/functional/adapter/grants/test_incremental_grants.py | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/tests/functional/adapter/grants/test_incremental_grants.py b/tests/functional/adapter/grants/test_incremental_grants.py index 77f982d97..3fa1acdf1 100644 --- a/tests/functional/adapter/grants/test_incremental_grants.py +++ b/tests/functional/adapter/grants/test_incremental_grants.py @@ -71,9 +71,6 @@ def models(self): } def test_incremental_grants(self, project, get_test_users, get_test_groups, get_test_roles): - # for debugging - print("incremental test") - # we want the test to fail, not silently skip test_users = get_test_users test_groups = get_test_groups @@ -171,3 +168,7 @@ def test_incremental_grants(self, project, get_test_users, get_test_groups, get_ } } self.assert_expected_grants_match_actual(project, "my_incremental_model", expected) + + +class TestIncrementalGrantsRedshift(BaseIncrementalGrantsRedshift): + pass From bfc08f6855c8b2d85cc462684f8fc1e4e590bd4f Mon Sep 17 00:00:00 2001 From: Soksamnang Lim Date: Wed, 11 Oct 2023 17:08:33 -0700 Subject: [PATCH 08/14] Change away from inheritance of BaseGrants --- .../functional/adapter/grants/base_grants.py | 60 +++++++++++++++---- 1 file changed, 49 insertions(+), 11 deletions(-) diff --git a/tests/functional/adapter/grants/base_grants.py b/tests/functional/adapter/grants/base_grants.py index 22a614d01..3e94d3678 100644 --- a/tests/functional/adapter/grants/base_grants.py +++ b/tests/functional/adapter/grants/base_grants.py @@ -1,7 +1,11 @@ -from dbt.tests.adapter.grants.base_grants import BaseGrants import pytest import os +from dbt.tests.util import ( + relation_from_name, + get_connection, +) + TEST_USER_ENV_VARS = ["DBT_TEST_USER_1", "DBT_TEST_USER_2", "DBT_TEST_USER_3"] TEST_GROUP_ENV_VARS = ["DBT_TEST_GROUP_1", "DBT_TEST_GROUP_2", "DBT_TEST_GROUP_3"] TEST_ROLE_ENV_VARS = ["DBT_TEST_ROLE_1", "DBT_TEST_ROLE_2", "DBT_TEST_ROLE_3"] @@ -13,23 +17,57 @@ def replace_all(text, dic): return text -def get_test_permissions(permission_env_vars): - test_permissions = [] - for env_var in permission_env_vars: - permission_name = os.getenv(env_var) - if permission_name: - test_permissions.append(permission_name) - return test_permissions +class BaseGrantsRedshift: + def privilege_grantee_name_overrides(self): + # these privilege and grantee names are valid on most databases, but not all! + # looking at you, BigQuery + # optionally use this to map from "select" --> "other_select_name", "insert" --> ... + return { + "select": "select", + "insert": "insert", + "fake_privilege": "fake_privilege", + "invalid_user": "invalid_user", + } + def interpolate_name_overrides(self, yaml_text): + return replace_all(yaml_text, self.privilege_grantee_name_overrides()) -class BaseGrantsRedshift(BaseGrants): @pytest.fixture(scope="class", autouse=True) def get_test_groups(self, project): - return get_test_permissions(TEST_GROUP_ENV_VARS) + test_groups = [] + for env_var in TEST_GROUP_ENV_VARS: + group_name = os.getenv(env_var) + if group_name: + test_groups.append(group_name) + return test_groups @pytest.fixture(scope="class", autouse=True) def get_test_roles(self, project): - return get_test_permissions(TEST_ROLE_ENV_VARS) + test_roles = [] + for env_var in TEST_ROLE_ENV_VARS: + role_name = os.getenv(env_var) + if role_name: + test_roles.append(role_name) + return test_roles + + @pytest.fixture(scope="class", autouse=True) + def get_test_users(self, project): + test_users = [] + for env_var in TEST_USER_ENV_VARS: + user_name = os.getenv(env_var) + if user_name: + test_users.append(user_name) + return test_users + + def get_grants_on_relation(self, project, relation_name): + relation = relation_from_name(project.adapter, relation_name) + adapter = project.adapter + with get_connection(adapter): + kwargs = {"relation": relation} + show_grant_sql = adapter.execute_macro("get_show_grant_sql", kwargs=kwargs) + _, grant_table = adapter.execute(show_grant_sql, fetch=True) + actual_grants = adapter.standardize_grants_dict(grant_table) + return actual_grants # This is an override of the BaseGrants class def assert_expected_grants_match_actual(self, project, relation_name, expected_grants): From b546431a66b2128fbe90329ee6fea1139a964901 Mon Sep 17 00:00:00 2001 From: Colin Date: Fri, 13 Oct 2023 11:29:23 -0700 Subject: [PATCH 09/14] clean up duplicate tests --- .../adapter/grants/test_incremental_grants.py | 6 +--- .../adapter/grants/test_model_grants.py | 29 +++++++++---------- .../adapter/grants/test_seed_grants.py | 2 +- .../adapter/grants/test_snapshot_grants.py | 2 +- tests/functional/adapter/test_grants.py | 24 --------------- 5 files changed, 16 insertions(+), 47 deletions(-) delete mode 100644 tests/functional/adapter/test_grants.py diff --git a/tests/functional/adapter/grants/test_incremental_grants.py b/tests/functional/adapter/grants/test_incremental_grants.py index 3fa1acdf1..17ee34c87 100644 --- a/tests/functional/adapter/grants/test_incremental_grants.py +++ b/tests/functional/adapter/grants/test_incremental_grants.py @@ -61,7 +61,7 @@ """ -class BaseIncrementalGrantsRedshift(BaseGrantsRedshift): +class TestIncrementalGrantsRedshift(BaseGrantsRedshift): @pytest.fixture(scope="class") def models(self): updated_schema = self.interpolate_name_overrides(incremental_model_schema_yml) @@ -168,7 +168,3 @@ def test_incremental_grants(self, project, get_test_users, get_test_groups, get_ } } self.assert_expected_grants_match_actual(project, "my_incremental_model", expected) - - -class TestIncrementalGrantsRedshift(BaseIncrementalGrantsRedshift): - pass diff --git a/tests/functional/adapter/grants/test_model_grants.py b/tests/functional/adapter/grants/test_model_grants.py index 3f0220eae..bbf3cd485 100644 --- a/tests/functional/adapter/grants/test_model_grants.py +++ b/tests/functional/adapter/grants/test_model_grants.py @@ -1,6 +1,6 @@ import pytest from dbt.tests.util import ( - run_dbt_and_capture, + run_dbt, get_manifest, write_file, ) @@ -129,7 +129,7 @@ """ -class BaseModelGrantsRedshift(BaseGrantsRedshift): +class TestModelGrantsRedshift(BaseGrantsRedshift): @pytest.fixture(scope="class") def models(self): updated_schema = self.interpolate_name_overrides(model_schema_yml) @@ -151,7 +151,7 @@ def test_view_table_grants(self, project, get_test_users, get_test_groups, get_t assert len(test_roles) == 3 # View materialization, single select grant - (results, log_output) = run_dbt_and_capture(["--debug", "run"]) + results = run_dbt(["run"]) assert len(results) == 1 manifest = get_manifest(project.project_root) model_id = "model.test.my_model" @@ -167,7 +167,7 @@ def test_view_table_grants(self, project, get_test_users, get_test_groups, get_t # View materialization, change select grant user updated_yaml = self.interpolate_name_overrides(user2_model_schema_yml) write_file(updated_yaml, project.project_root, "models", "schema.yml") - (results, log_output) = run_dbt_and_capture(["--debug", "run"]) + results = run_dbt(["run"]) assert len(results) == 1 expected = {select_privilege_name: {"user": [test_users[1]]}} @@ -176,7 +176,7 @@ def test_view_table_grants(self, project, get_test_users, get_test_groups, get_t # Table materialization, single select grant updated_yaml = self.interpolate_name_overrides(table_model_schema_yml) write_file(updated_yaml, project.project_root, "models", "schema.yml") - (results, log_output) = run_dbt_and_capture(["--debug", "run"]) + results = run_dbt(["run"]) assert len(results) == 1 manifest = get_manifest(project.project_root) model_id = "model.test.my_model" @@ -188,7 +188,7 @@ def test_view_table_grants(self, project, get_test_users, get_test_groups, get_t # Table materialization, change select grant user updated_yaml = self.interpolate_name_overrides(user2_table_model_schema_yml) write_file(updated_yaml, project.project_root, "models", "schema.yml") - (results, log_output) = run_dbt_and_capture(["--debug", "run"]) + results = run_dbt(["run"]) assert len(results) == 1 manifest = get_manifest(project.project_root) model = manifest.nodes[model_id] @@ -199,7 +199,7 @@ def test_view_table_grants(self, project, get_test_users, get_test_groups, get_t # Table materialization, multiple grantees updated_yaml = self.interpolate_name_overrides(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"]) + results = run_dbt(["run"]) assert len(results) == 1 manifest = get_manifest(project.project_root) model = manifest.nodes[model_id] @@ -210,7 +210,7 @@ def test_view_table_grants(self, project, get_test_users, get_test_groups, get_t # Table materialization, multiple privileges updated_yaml = self.interpolate_name_overrides(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"]) + results = run_dbt(["run"]) assert len(results) == 1 manifest = get_manifest(project.project_root) model = manifest.nodes[model_id] @@ -230,7 +230,7 @@ def test_view_table_grants(self, project, get_test_users, get_test_groups, get_t # Table materialization, single select grant updated_yaml = self.interpolate_name_overrides(extended_table_model_schema_yml) write_file(updated_yaml, project.project_root, "models", "schema.yml") - (results, log_output) = run_dbt_and_capture(["--debug", "run"]) + results = run_dbt(["run"]) assert len(results) == 1 manifest = get_manifest(project.project_root) model = manifest.nodes[model_id] @@ -242,12 +242,13 @@ def test_view_table_grants(self, project, get_test_users, get_test_groups, get_t "role": [test_roles[0]], } } + breakpoint() self.assert_expected_grants_match_actual(project, "my_model", expected) # Table materialization, change select grant updated_yaml = self.interpolate_name_overrides(extended2_table_model_schema_yml) write_file(updated_yaml, project.project_root, "models", "schema.yml") - (results, log_output) = run_dbt_and_capture(["--debug", "run"]) + results = run_dbt(["run"]) assert len(results) == 1 manifest = get_manifest(project.project_root) model = manifest.nodes[model_id] @@ -266,7 +267,7 @@ def test_view_table_grants(self, project, get_test_users, get_test_groups, get_t extended_multiple_grantees_table_model_schema_yml ) write_file(updated_yaml, project.project_root, "models", "schema.yml") - (results, log_output) = run_dbt_and_capture(["--debug", "run"]) + results = run_dbt(["run"]) assert len(results) == 1 manifest = get_manifest(project.project_root) model = manifest.nodes[model_id] @@ -285,7 +286,7 @@ def test_view_table_grants(self, project, get_test_users, get_test_groups, get_t extended_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"]) + results = run_dbt(["run"]) assert len(results) == 1 manifest = get_manifest(project.project_root) model = manifest.nodes[model_id] @@ -303,7 +304,3 @@ def test_view_table_grants(self, project, get_test_users, get_test_groups, get_t }, } self.assert_expected_grants_match_actual(project, "my_model", expected) - - -class TestModelGrantsRedshift(BaseModelGrantsRedshift): - pass diff --git a/tests/functional/adapter/grants/test_seed_grants.py b/tests/functional/adapter/grants/test_seed_grants.py index c87c68716..01bb59594 100644 --- a/tests/functional/adapter/grants/test_seed_grants.py +++ b/tests/functional/adapter/grants/test_seed_grants.py @@ -64,7 +64,7 @@ """ -class BaseSeedGrantsRedshift(BaseGrantsRedshift): +class TestSeedGrantsRedshift(BaseGrantsRedshift): def seeds_support_partial_refresh(self): return True diff --git a/tests/functional/adapter/grants/test_snapshot_grants.py b/tests/functional/adapter/grants/test_snapshot_grants.py index b2ff5e6f1..c9e3e2947 100644 --- a/tests/functional/adapter/grants/test_snapshot_grants.py +++ b/tests/functional/adapter/grants/test_snapshot_grants.py @@ -62,7 +62,7 @@ """ -class BaseSnapshotGrantsRedshift(BaseGrantsRedshift): +class TestSnapshotGrantsRedshift(BaseGrantsRedshift): @pytest.fixture(scope="class") def snapshots(self): return { diff --git a/tests/functional/adapter/test_grants.py b/tests/functional/adapter/test_grants.py deleted file mode 100644 index 9dcc63c77..000000000 --- a/tests/functional/adapter/test_grants.py +++ /dev/null @@ -1,24 +0,0 @@ -from grants.test_model_grants import BaseModelGrantsRedshift -from grants.test_snapshot_grants import BaseSnapshotGrantsRedshift -from grants.test_seed_grants import BaseSeedGrantsRedshift -from grants.test_incremental_grants import BaseIncrementalGrantsRedshift - - -class TestModelGrantsRedshift(BaseModelGrantsRedshift): - pass - - -class TestIncrementalGrantsRedshift(BaseIncrementalGrantsRedshift): - pass - - -class TestSeedGrantsRedshift(BaseSeedGrantsRedshift): - pass - - -class TestSnapshotGrantsRedshift(BaseSnapshotGrantsRedshift): - pass - - -class TestInvalidGrantsRedshift(BaseModelGrantsRedshift): - pass From 0aa8fb1c754c22ebe031e81469e4c0ccce45dad4 Mon Sep 17 00:00:00 2001 From: Soksamnang Lim Date: Wed, 25 Oct 2023 13:21:45 -0700 Subject: [PATCH 10/14] Split model grants tests into view and table tests --- ...l_grants.py => test_model_table_grants.py} | 85 +++++-------------- .../adapter/grants/test_model_view_grants.py | 80 +++++++++++++++++ 2 files changed, 101 insertions(+), 64 deletions(-) rename tests/functional/adapter/grants/{test_model_grants.py => test_model_table_grants.py} (80%) create mode 100644 tests/functional/adapter/grants/test_model_view_grants.py diff --git a/tests/functional/adapter/grants/test_model_grants.py b/tests/functional/adapter/grants/test_model_table_grants.py similarity index 80% rename from tests/functional/adapter/grants/test_model_grants.py rename to tests/functional/adapter/grants/test_model_table_grants.py index bbf3cd485..7ea8e2893 100644 --- a/tests/functional/adapter/grants/test_model_grants.py +++ b/tests/functional/adapter/grants/test_model_table_grants.py @@ -10,28 +10,10 @@ 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 + - name: my_model_table config: materialized: table grants: @@ -41,7 +23,7 @@ user2_table_model_schema_yml = """ version: 2 models: - - name: my_model + - name: my_model_table config: materialized: table grants: @@ -51,7 +33,7 @@ multiple_users_table_model_schema_yml = """ version: 2 models: - - name: my_model + - name: my_model_table config: materialized: table grants: @@ -61,7 +43,7 @@ multiple_privileges_table_model_schema_yml = """ version: 2 models: - - name: my_model + - name: my_model_table config: materialized: table grants: @@ -73,7 +55,7 @@ extended_table_model_schema_yml = """ version: 2 models: - - name: my_model + - name: my_model_table config: materialized: table grants: @@ -87,7 +69,7 @@ extended2_table_model_schema_yml = """ version: 2 models: - - name: my_model + - name: my_model_table config: materialized: table grants: @@ -101,7 +83,7 @@ extended_multiple_grantees_table_model_schema_yml = """ version: 2 models: - - name: my_model + - name: my_model_table config: materialized: table grants: @@ -114,7 +96,7 @@ extended_multiple_privileges_table_model_schema_yml = """ version: 2 models: - - name: my_model + - name: my_model_table config: materialized: table grants: @@ -132,13 +114,13 @@ class TestModelGrantsRedshift(BaseGrantsRedshift): @pytest.fixture(scope="class") def models(self): - updated_schema = self.interpolate_name_overrides(model_schema_yml) + updated_schema = self.interpolate_name_overrides(table_model_schema_yml) return { - "my_model.sql": my_model_sql, + "my_model_table.sql": my_model_sql, "schema.yml": updated_schema, } - def test_view_table_grants(self, project, get_test_users, get_test_groups, get_test_roles): + def test_table_grants(self, project, get_test_users, get_test_groups, get_test_roles): # Override/refactor the tests from dbt-core # # we want the test to fail, not silently skip test_users = get_test_users @@ -150,40 +132,15 @@ def test_view_table_grants(self, project, get_test_users, get_test_groups, get_t assert len(test_groups) == 3 assert len(test_roles) == 3 - # View materialization, single select grant - results = run_dbt(["run"]) - assert len(results) == 1 - manifest = get_manifest(project.project_root) - model_id = "model.test.my_model" - model = manifest.nodes[model_id] - # user configuration for grants - user_expected = {select_privilege_name: [test_users[0]]} - assert model.config.grants == user_expected - assert model.config.materialized == "view" - # new configuration for grants - expected = {select_privilege_name: {"user": [test_users[0]]}} - self.assert_expected_grants_match_actual(project, "my_model", expected) - - # View materialization, change select grant user - updated_yaml = self.interpolate_name_overrides(user2_model_schema_yml) - write_file(updated_yaml, project.project_root, "models", "schema.yml") - results = run_dbt(["run"]) - assert len(results) == 1 - - expected = {select_privilege_name: {"user": [test_users[1]]}} - self.assert_expected_grants_match_actual(project, "my_model", expected) - # Table materialization, single select grant - updated_yaml = self.interpolate_name_overrides(table_model_schema_yml) - write_file(updated_yaml, project.project_root, "models", "schema.yml") results = run_dbt(["run"]) assert len(results) == 1 manifest = get_manifest(project.project_root) - model_id = "model.test.my_model" + model_id = "model.test.my_model_table" model = manifest.nodes[model_id] assert model.config.materialized == "table" expected = {select_privilege_name: {"user": [test_users[0]]}} - self.assert_expected_grants_match_actual(project, "my_model", expected) + self.assert_expected_grants_match_actual(project, "my_model_table", expected) # Table materialization, change select grant user updated_yaml = self.interpolate_name_overrides(user2_table_model_schema_yml) @@ -194,7 +151,7 @@ def test_view_table_grants(self, project, get_test_users, get_test_groups, get_t model = manifest.nodes[model_id] assert model.config.materialized == "table" expected = {select_privilege_name: {"user": [test_users[1]]}} - self.assert_expected_grants_match_actual(project, "my_model", expected) + self.assert_expected_grants_match_actual(project, "my_model_table", expected) # Table materialization, multiple grantees updated_yaml = self.interpolate_name_overrides(multiple_users_table_model_schema_yml) @@ -205,7 +162,7 @@ def test_view_table_grants(self, project, get_test_users, get_test_groups, get_t model = manifest.nodes[model_id] assert model.config.materialized == "table" expected = {select_privilege_name: {"user": [test_users[0], test_users[1]]}} - self.assert_expected_grants_match_actual(project, "my_model", expected) + self.assert_expected_grants_match_actual(project, "my_model_table", expected) # Table materialization, multiple privileges updated_yaml = self.interpolate_name_overrides(multiple_privileges_table_model_schema_yml) @@ -224,7 +181,7 @@ def test_view_table_grants(self, project, get_test_users, get_test_groups, get_t select_privilege_name: {"user": [test_users[0]]}, insert_privilege_name: {"user": [test_users[1]]}, } - self.assert_expected_grants_match_actual(project, "my_model", expected) + self.assert_expected_grants_match_actual(project, "my_model_table", expected) # Additional tests for privilege grants to extended permission types # Table materialization, single select grant @@ -242,8 +199,8 @@ def test_view_table_grants(self, project, get_test_users, get_test_groups, get_t "role": [test_roles[0]], } } - breakpoint() - self.assert_expected_grants_match_actual(project, "my_model", expected) + # breakpoint() + self.assert_expected_grants_match_actual(project, "my_model_table", expected) # Table materialization, change select grant updated_yaml = self.interpolate_name_overrides(extended2_table_model_schema_yml) @@ -260,7 +217,7 @@ def test_view_table_grants(self, project, get_test_users, get_test_groups, get_t "role": [test_roles[1]], } } - self.assert_expected_grants_match_actual(project, "my_model", expected) + self.assert_expected_grants_match_actual(project, "my_model_table", expected) # Table materialization, multiple grantees updated_yaml = self.interpolate_name_overrides( @@ -279,7 +236,7 @@ def test_view_table_grants(self, project, get_test_users, get_test_groups, get_t "role": [test_roles[0], test_roles[1]], } } - self.assert_expected_grants_match_actual(project, "my_model", expected) + self.assert_expected_grants_match_actual(project, "my_model_table", expected) # Table materialization, multiple privileges updated_yaml = self.interpolate_name_overrides( @@ -303,4 +260,4 @@ def test_view_table_grants(self, project, get_test_users, get_test_groups, get_t "role": [test_roles[1]], }, } - self.assert_expected_grants_match_actual(project, "my_model", expected) + self.assert_expected_grants_match_actual(project, "my_model_table", expected) diff --git a/tests/functional/adapter/grants/test_model_view_grants.py b/tests/functional/adapter/grants/test_model_view_grants.py new file mode 100644 index 000000000..099175d79 --- /dev/null +++ b/tests/functional/adapter/grants/test_model_view_grants.py @@ -0,0 +1,80 @@ +from base_grants import BaseGrantsRedshift +import pytest +from dbt.tests.util import ( + run_dbt, + get_manifest, + write_file, +) + +my_model_sql = """ + select 1 as fun +""" + +model_schema_yml = """ +version: 2 +models: + - name: my_model_view + config: + materialized: view + grants: + select: ["{{ env_var('DBT_TEST_USER_1') }}"] +""" + +user2_model_schema_yml = """ +version: 2 +models: + - name: my_model_view + config: + materialized: view + grants: + select: ["{{ env_var('DBT_TEST_USER_2') }}"] +""" + + +# class BaseModelGrants(BaseGrantsRedshift): + +class TestModelGrantsView(BaseGrantsRedshift): + @pytest.fixture(scope="class") + def models(self): + updated_schema = self.interpolate_name_overrides(model_schema_yml) + return { + "my_model_view.sql": my_model_sql, + "schema.yml": updated_schema, + } + + def test_view_table_grants(self, project, get_test_users, get_test_groups, get_test_roles): + # Override/refactor the tests from dbt-core # + # we want the test to fail, not silently skip + test_users = get_test_users + test_groups = get_test_groups + test_roles = get_test_roles + select_privilege_name = self.privilege_grantee_name_overrides()["select"] + assert len(test_users) == 3 + assert len(test_groups) == 3 + assert len(test_roles) == 3 + + # View materialization, single select grant + updated_yaml = self.interpolate_name_overrides(model_schema_yml) + write_file(updated_yaml, project.project_root, "models", "schema.yml") + results = run_dbt(["run"]) + assert len(results) == 1 + + manifest = get_manifest(project.project_root) + model_id = "model.test.my_model_view" + model = manifest.nodes[model_id] + # user configuration for grants + user_expected = {select_privilege_name: [test_users[0]]} + assert model.config.grants == user_expected + assert model.config.materialized == "view" + # new configuration for grants + expected = {select_privilege_name: {"user": [test_users[0]]}} + self.assert_expected_grants_match_actual(project, "my_model_view", expected) + + # View materialization, change select grant user + updated_yaml = self.interpolate_name_overrides(user2_model_schema_yml) + write_file(updated_yaml, project.project_root, "models", "schema.yml") + results = run_dbt(["run"]) + assert len(results) == 1 + + expected = {select_privilege_name: {"user": [test_users[1]]}} + self.assert_expected_grants_match_actual(project, "my_model_view", expected) From c0235bab25f92c60fa39ad0a07888bbc4efdfc6f Mon Sep 17 00:00:00 2001 From: Soksamnang Lim Date: Wed, 25 Oct 2023 13:45:03 -0700 Subject: [PATCH 11/14] Renamed model view and model table test suites --- tests/functional/adapter/grants/test_model_table_grants.py | 2 +- tests/functional/adapter/grants/test_model_view_grants.py | 4 +--- 2 files changed, 2 insertions(+), 4 deletions(-) diff --git a/tests/functional/adapter/grants/test_model_table_grants.py b/tests/functional/adapter/grants/test_model_table_grants.py index 7ea8e2893..455da6177 100644 --- a/tests/functional/adapter/grants/test_model_table_grants.py +++ b/tests/functional/adapter/grants/test_model_table_grants.py @@ -111,7 +111,7 @@ """ -class TestModelGrantsRedshift(BaseGrantsRedshift): +class TestModelGrantsTableRedshift(BaseGrantsRedshift): @pytest.fixture(scope="class") def models(self): updated_schema = self.interpolate_name_overrides(table_model_schema_yml) diff --git a/tests/functional/adapter/grants/test_model_view_grants.py b/tests/functional/adapter/grants/test_model_view_grants.py index 099175d79..5b0cc83fe 100644 --- a/tests/functional/adapter/grants/test_model_view_grants.py +++ b/tests/functional/adapter/grants/test_model_view_grants.py @@ -31,9 +31,7 @@ """ -# class BaseModelGrants(BaseGrantsRedshift): - -class TestModelGrantsView(BaseGrantsRedshift): +class TestModelGrantsViewRedshift(BaseGrantsRedshift): @pytest.fixture(scope="class") def models(self): updated_schema = self.interpolate_name_overrides(model_schema_yml) From bdc61779b19e96eb253bd0055ac858642a0112bd Mon Sep 17 00:00:00 2001 From: Soksamnang Lim Date: Wed, 25 Oct 2023 13:51:02 -0700 Subject: [PATCH 12/14] added groups and roles to env-setup --- scripts/env-setup.sh | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/scripts/env-setup.sh b/scripts/env-setup.sh index 866d8f749..fb56f9a80 100644 --- a/scripts/env-setup.sh +++ b/scripts/env-setup.sh @@ -8,3 +8,9 @@ echo "INTEGRATION_TESTS_SECRETS_PREFIX=REDSHIFT_TEST" >> $GITHUB_ENV echo "DBT_TEST_USER_1=dbt_test_user_1" >> $GITHUB_ENV echo "DBT_TEST_USER_2=dbt_test_user_2" >> $GITHUB_ENV echo "DBT_TEST_USER_3=dbt_test_user_3" >> $GITHUB_ENV +echo "DBT_TEST_GROUP_1=dbt_test_group_1" >> $GITHUB_ENV +echo "DBT_TEST_GROUP_2=dbt_test_group_2" >> $GITHUB_ENV +echo "DBT_TEST_GROUP_3=dbt_test_group_3" >> $GITHUB_ENV +echo "DBT_TEST_ROLE_1=dbt_test_role_1" >> $GITHUB_ENV +echo "DBT_TEST_ROLE_2=dbt_test_role_2" >> $GITHUB_ENV +echo "DBT_TEST_ROLE_3=dbt_test_role_3" >> $GITHUB_ENV \ No newline at end of file From ead3a2ee6e70d4bb2ca313feef95c0d3997f4a1e Mon Sep 17 00:00:00 2001 From: Soksamnang Lim Date: Wed, 25 Oct 2023 13:55:29 -0700 Subject: [PATCH 13/14] fix pre-commit EOF space --- scripts/env-setup.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/scripts/env-setup.sh b/scripts/env-setup.sh index fb56f9a80..844958b08 100644 --- a/scripts/env-setup.sh +++ b/scripts/env-setup.sh @@ -13,4 +13,4 @@ echo "DBT_TEST_GROUP_2=dbt_test_group_2" >> $GITHUB_ENV echo "DBT_TEST_GROUP_3=dbt_test_group_3" >> $GITHUB_ENV echo "DBT_TEST_ROLE_1=dbt_test_role_1" >> $GITHUB_ENV echo "DBT_TEST_ROLE_2=dbt_test_role_2" >> $GITHUB_ENV -echo "DBT_TEST_ROLE_3=dbt_test_role_3" >> $GITHUB_ENV \ No newline at end of file +echo "DBT_TEST_ROLE_3=dbt_test_role_3" >> $GITHUB_ENV From 3cee6e20d6bdb0f3c2b3e8017942b57aa2cc82a0 Mon Sep 17 00:00:00 2001 From: Colin Date: Tue, 31 Oct 2023 11:33:57 -0700 Subject: [PATCH 14/14] set env vars in conftest.py --- .github/workflows/integration.yml | 6 ------ scripts/env-setup.sh | 6 ------ test.env.example | 6 ------ tests/functional/adapter/conftest.py | 23 +++++++++++++++++------ 4 files changed, 17 insertions(+), 24 deletions(-) diff --git a/.github/workflows/integration.yml b/.github/workflows/integration.yml index 371cbc291..9d1fe0807 100644 --- a/.github/workflows/integration.yml +++ b/.github/workflows/integration.yml @@ -185,12 +185,6 @@ jobs: DBT_TEST_USER_1: dbt_test_user_1 DBT_TEST_USER_2: dbt_test_user_2 DBT_TEST_USER_3: dbt_test_user_3 - DBT_TEST_GROUP_1: dbt_test_group_1 - DBT_TEST_GROUP_2: dbt_test_group_2 - DBT_TEST_GROUP_3: dbt_test_group_3 - DBT_TEST_ROLE_1: dbt_test_role_1 - DBT_TEST_ROLE_2: dbt_test_role_2 - DBT_TEST_ROLE_3: dbt_test_role_3 run: tox -- --ddtrace - uses: actions/upload-artifact@v3 diff --git a/scripts/env-setup.sh b/scripts/env-setup.sh index 844958b08..866d8f749 100644 --- a/scripts/env-setup.sh +++ b/scripts/env-setup.sh @@ -8,9 +8,3 @@ echo "INTEGRATION_TESTS_SECRETS_PREFIX=REDSHIFT_TEST" >> $GITHUB_ENV echo "DBT_TEST_USER_1=dbt_test_user_1" >> $GITHUB_ENV echo "DBT_TEST_USER_2=dbt_test_user_2" >> $GITHUB_ENV echo "DBT_TEST_USER_3=dbt_test_user_3" >> $GITHUB_ENV -echo "DBT_TEST_GROUP_1=dbt_test_group_1" >> $GITHUB_ENV -echo "DBT_TEST_GROUP_2=dbt_test_group_2" >> $GITHUB_ENV -echo "DBT_TEST_GROUP_3=dbt_test_group_3" >> $GITHUB_ENV -echo "DBT_TEST_ROLE_1=dbt_test_role_1" >> $GITHUB_ENV -echo "DBT_TEST_ROLE_2=dbt_test_role_2" >> $GITHUB_ENV -echo "DBT_TEST_ROLE_3=dbt_test_role_3" >> $GITHUB_ENV diff --git a/test.env.example b/test.env.example index 8d645f83b..4de05edab 100644 --- a/test.env.example +++ b/test.env.example @@ -16,9 +16,3 @@ REDSHIFT_TEST_DBNAME= DBT_TEST_USER_1=dbt_test_user_1 DBT_TEST_USER_2=dbt_test_user_2 DBT_TEST_USER_3=dbt_test_user_3 -DBT_TEST_GROUP_1=dbt_test_group_1 -DBT_TEST_GROUP_2=dbt_test_group_2 -DBT_TEST_GROUP_3=dbt_test_group_3 -DBT_TEST_ROLE_1=dbt_test_role_1 -DBT_TEST_ROLE_2=dbt_test_role_2 -DBT_TEST_ROLE_3=dbt_test_role_3 diff --git a/tests/functional/adapter/conftest.py b/tests/functional/adapter/conftest.py index 6cf4557e4..32c8b5e1e 100644 --- a/tests/functional/adapter/conftest.py +++ b/tests/functional/adapter/conftest.py @@ -6,24 +6,35 @@ # This is a hack to prevent the fixture from running more than once GRANTS_AND_ROLES_SETUP = False +GROUPS = { + "DBT_TEST_GROUP_1": "dbt_test_group_1", + "DBT_TEST_GROUP_2": "dbt_test_group_2", + "DBT_TEST_GROUP_3": "dbt_test_group_3", +} +ROLES = { + "DBT_TEST_ROLE_1": "dbt_test_role_1", + "DBT_TEST_ROLE_2": "dbt_test_role_2", + "DBT_TEST_ROLE_3": "dbt_test_role_3", +} + @pytest.fixture(scope="class", autouse=True) def setup_grants_and_roles(project): global GRANTS_AND_ROLES_SETUP - groups = [ - os.environ[env_var] for env_var in os.environ if env_var.startswith("DBT_TEST_GROUP_") - ] - roles = [os.environ[env_var] for env_var in os.environ if env_var.startswith("DBT_TEST_ROLE_")] + for env_name, env_var in GROUPS.items(): + os.environ[env_name] = env_var + for env_name, env_var in ROLES.items(): + os.environ[env_name] = env_var if not GRANTS_AND_ROLES_SETUP: with project.adapter.connection_named("__test"): - for group in groups: + for group in GROUPS.values(): try: project.adapter.execute(f"CREATE GROUP {group}") except DbtDatabaseError: # This is expected if the group already exists pass - for role in roles: + for role in ROLES.values(): try: project.adapter.execute(f"CREATE ROLE {role}") except DbtDatabaseError: