From a1ec09c2573ba27713bc0fa5f82325646ecf663e Mon Sep 17 00:00:00 2001 From: Soksamnang Lim Date: Tue, 3 Oct 2023 22:43:41 -0700 Subject: [PATCH 01/33] 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/33] 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/33] 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/33] 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/33] 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/33] 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/33] 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/33] 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/33] 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/33] 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/33] 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/33] 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/33] 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 f691812d0c1804c1b7741778fbe33e66342e4e4c Mon Sep 17 00:00:00 2001 From: Soksamnang Lim Date: Fri, 27 Oct 2023 11:18:14 -0700 Subject: [PATCH 14/33] adding CI_flags override to makefile --- Makefile | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/Makefile b/Makefile index efd23b806..776992ef1 100644 --- a/Makefile +++ b/Makefile @@ -1,5 +1,19 @@ .DEFAULT_GOAL:=help +CI_FLAGS =\ + DBT_TEST_USER_1=$(if $(DBT_TEST_USER_1),$(DBT_TEST_USER_1),dbt_test_user_1)\ + DBT_TEST_USER_2=$(if $(DBT_TEST_USER_2),$(DBT_TEST_USER_2),dbt_test_user_2)\ + DBT_TEST_USER_3=$(if $(DBT_TEST_USER_3),$(DBT_TEST_USER_3),dbt_test_user_3)\ + DBT_TEST_GROUP_1=$(if $(DBT_TEST_GROUP_1),$(DBT_TEST_GROUP_1),dbt_test_group_1)\ + DBT_TEST_GROUP_2=$(if $(DBT_TEST_GROUP_2),$(DBT_TEST_GROUP_2),dbt_test_group_2)\ + DBT_TEST_GROUP_3=$(if $(DBT_TEST_GROUP_3),$(DBT_TEST_GROUP_3),dbt_test_group_3)\ + DBT_TEST_ROLE_1=$(if $(DBT_TEST_ROLE_1),$(DBT_TEST_ROLE_1),dbt_test_role_1)\ + DBT_TEST_ROLE_2=$(if $(DBT_TEST_ROLE_2),$(DBT_TEST_ROLE_2),dbt_test_role_2)\ + DBT_TEST_ROLE_3=$(if $(DBT_TEST_ROLE_3),$(DBT_TEST_ROLE_3),dbt_test_role_3)\ + RUSTFLAGS=$(if $(RUSTFLAGS),$(RUSTFLAGS),"-D warnings")\ + LOG_DIR=$(if $(LOG_DIR),$(LOG_DIR),./logs)\ + DBT_LOG_FORMAT=$(if $(DBT_LOG_FORMAT),$(DBT_LOG_FORMAT),json) + .PHONY: dev dev: ## Installs adapter in develop mode along with development dependencies @\ From 3cee6e20d6bdb0f3c2b3e8017942b57aa2cc82a0 Mon Sep 17 00:00:00 2001 From: Colin Date: Tue, 31 Oct 2023 11:33:57 -0700 Subject: [PATCH 15/33] 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: From fad2ef8873383caf388975b68def17180e5fb6a3 Mon Sep 17 00:00:00 2001 From: Soksamnang Lim Date: Tue, 31 Oct 2023 12:46:20 -0700 Subject: [PATCH 16/33] delete breakpoint and add comment to CONTRIBUTING.md --- CONTRIBUTING.md | 2 +- tests/functional/adapter/grants/test_model_table_grants.py | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index ff08b6190..164e1d9e7 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -57,7 +57,7 @@ To confirm you have correct `dbt-core` and adapter versions installed please run `dbt-redshift` contains [unit](https://github.com/dbt-labs/dbt-redshift/tree/main/tests/unit) and [functional](https://github.com/dbt-labs/dbt-redshift/tree/main/tests/functional) tests. Functional tests require testing against an actual Redshift warehouse. We have CI set up to test against a Redshift warehouse during PR checks. -In order to run functional tests locally, you will need a `test.env` file in the root of the repository that contains credentials for your Redshift warehouse. +In order to run functional tests locally, you will need a `test.env` file in the root of the repository that contains credentials for your Redshift warehouse. You'll need all the objects provided in `test.env.example` in `test.env` for all the tests to pass. Note: This `test.env` file is git-ignored, but please be extra careful to never check in credentials or other sensitive information when developing. To create your `test.env` file, copy the provided example file, then supply your relevant credentials. diff --git a/tests/functional/adapter/grants/test_model_table_grants.py b/tests/functional/adapter/grants/test_model_table_grants.py index 455da6177..334915b9f 100644 --- a/tests/functional/adapter/grants/test_model_table_grants.py +++ b/tests/functional/adapter/grants/test_model_table_grants.py @@ -199,7 +199,7 @@ def test_table_grants(self, project, get_test_users, get_test_groups, get_test_r "role": [test_roles[0]], } } - # breakpoint() + self.assert_expected_grants_match_actual(project, "my_model_table", expected) # Table materialization, change select grant From aeadcac26f9d05321219459e0d084d320c7563ed Mon Sep 17 00:00:00 2001 From: Soksamnang Lim Date: Tue, 31 Oct 2023 18:34:07 -0700 Subject: [PATCH 17/33] fix get_show_grant_sql macro --- dbt/include/redshift/macros/adapters/apply_grants.sql | 2 ++ 1 file changed, 2 insertions(+) diff --git a/dbt/include/redshift/macros/adapters/apply_grants.sql b/dbt/include/redshift/macros/adapters/apply_grants.sql index ed51aaa2a..c6dedfa3f 100644 --- a/dbt/include/redshift/macros/adapters/apply_grants.sql +++ b/dbt/include/redshift/macros/adapters/apply_grants.sql @@ -74,6 +74,7 @@ where exists( select * from information_schema.table_privileges tp where tp.grantee=g.groname + and tp.table_schema=replace(split_part('{{ relation }}', '.', 2), '"', '') and tp.table_name=replace(split_part('{{ relation }}', '.', 3), '"', '') and LOWER(tp.privilege_type)=p.privilege_type ) @@ -90,6 +91,7 @@ where exists( select * from svv_relation_privileges rp where rp.identity_name=r.role_name + and rp.namespace_name=replace(split_part('{{ relation }}', '.', 2), '"', '') and rp.relation_name=replace(split_part('{{ relation }}', '.', 3), '"', '') and LOWER(rp.privilege_type)=p.privilege_type ) From cee0637fca7f383b96c4f3908f9afea0c21b8d87 Mon Sep 17 00:00:00 2001 From: Soksamnang Lim Date: Wed, 1 Nov 2023 14:58:34 -0700 Subject: [PATCH 18/33] Refactor assert_expected_grants_match_actual for debugging --- .../functional/adapter/grants/base_grants.py | 3 +-- .../adapter/grants/test_incremental_grants.py | 22 +++++++++++----- .../adapter/grants/test_model_table_grants.py | 24 +++++++++++------ .../adapter/grants/test_model_view_grants.py | 7 +++-- .../adapter/grants/test_seed_grants.py | 26 ++++++++++++------- .../adapter/grants/test_snapshot_grants.py | 15 +++++++---- 6 files changed, 64 insertions(+), 33 deletions(-) diff --git a/tests/functional/adapter/grants/base_grants.py b/tests/functional/adapter/grants/base_grants.py index 3e94d3678..a65e3e26e 100644 --- a/tests/functional/adapter/grants/base_grants.py +++ b/tests/functional/adapter/grants/base_grants.py @@ -70,8 +70,7 @@ def get_grants_on_relation(self, project, relation_name): return actual_grants # 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) + def assert_expected_grants_match_actual(self, project, actual_grants, expected_grants): adapter = project.adapter # need a case-insensitive comparison # so just a simple "assert expected == actual_grants" won't work diff --git a/tests/functional/adapter/grants/test_incremental_grants.py b/tests/functional/adapter/grants/test_incremental_grants.py index 17ee34c87..1e47d4360 100644 --- a/tests/functional/adapter/grants/test_incremental_grants.py +++ b/tests/functional/adapter/grants/test_incremental_grants.py @@ -88,14 +88,16 @@ def test_incremental_grants(self, project, get_test_users, get_test_groups, get_ 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) + actual_grants = self.get_grants_on_relation(project, "my_incremental_model") + self.assert_expected_grants_match_actual(project, actual_grants, 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) + actual_grants = self.get_grants_on_relation(project, "my_incremental_model") + self.assert_expected_grants_match_actual(project, actual_grants, expected) # Incremental materialization, change select grant user updated_yaml = self.interpolate_name_overrides(user2_incremental_model_schema_yml) @@ -107,13 +109,15 @@ def test_incremental_grants(self, project, get_test_users, get_test_groups, get_ 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) + actual_grants = self.get_grants_on_relation(project, "my_incremental_model") + self.assert_expected_grants_match_actual(project, actual_grants, 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) + actual_grants = self.get_grants_on_relation(project, "my_incremental_model") + self.assert_expected_grants_match_actual(project, actual_grants, expected) # Now drop the schema (with the table in it) adapter = project.adapter @@ -126,7 +130,8 @@ def test_incremental_grants(self, project, get_test_users, get_test_groups, get_ 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) + actual_grants = self.get_grants_on_relation(project, "my_incremental_model") + self.assert_expected_grants_match_actual(project, actual_grants, expected) # Additional tests for privilege grants to extended permission types # Incremental materialization, single select grant @@ -147,7 +152,9 @@ def test_incremental_grants(self, project, get_test_users, get_test_groups, get_ "role": [test_roles[0]], } } - self.assert_expected_grants_match_actual(project, "my_incremental_model", expected) + + actual_grants = self.get_grants_on_relation(project, "my_incremental_model") + self.assert_expected_grants_match_actual(project, actual_grants, expected) # Incremental materialization, change select grant updated_yaml = self.interpolate_name_overrides(extended2_incremental_model_schema_yml) @@ -167,4 +174,5 @@ def test_incremental_grants(self, project, get_test_users, get_test_groups, get_ "role": [test_roles[1]], } } - self.assert_expected_grants_match_actual(project, "my_incremental_model", expected) + actual_grants = self.get_grants_on_relation(project, "my_incremental_model") + self.assert_expected_grants_match_actual(project, actual_grants, expected) diff --git a/tests/functional/adapter/grants/test_model_table_grants.py b/tests/functional/adapter/grants/test_model_table_grants.py index 334915b9f..300117615 100644 --- a/tests/functional/adapter/grants/test_model_table_grants.py +++ b/tests/functional/adapter/grants/test_model_table_grants.py @@ -140,7 +140,8 @@ def test_table_grants(self, project, get_test_users, get_test_groups, get_test_r 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_table", expected) + actual_grants = self.get_grants_on_relation(project, "my_model_table") + self.assert_expected_grants_match_actual(project, actual_grants, expected) # Table materialization, change select grant user updated_yaml = self.interpolate_name_overrides(user2_table_model_schema_yml) @@ -151,7 +152,8 @@ def test_table_grants(self, project, get_test_users, get_test_groups, get_test_r 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_table", expected) + actual_grants = self.get_grants_on_relation(project, "my_model_table") + self.assert_expected_grants_match_actual(project, actual_grants, expected) # Table materialization, multiple grantees updated_yaml = self.interpolate_name_overrides(multiple_users_table_model_schema_yml) @@ -162,7 +164,8 @@ def test_table_grants(self, project, get_test_users, get_test_groups, get_test_r 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_table", expected) + actual_grants = self.get_grants_on_relation(project, "my_model_table") + self.assert_expected_grants_match_actual(project, actual_grants, expected) # Table materialization, multiple privileges updated_yaml = self.interpolate_name_overrides(multiple_privileges_table_model_schema_yml) @@ -181,7 +184,8 @@ def test_table_grants(self, project, get_test_users, get_test_groups, get_test_r select_privilege_name: {"user": [test_users[0]]}, insert_privilege_name: {"user": [test_users[1]]}, } - self.assert_expected_grants_match_actual(project, "my_model_table", expected) + actual_grants = self.get_grants_on_relation(project, "my_model_table") + self.assert_expected_grants_match_actual(project, actual_grants, expected) # Additional tests for privilege grants to extended permission types # Table materialization, single select grant @@ -200,7 +204,8 @@ def test_table_grants(self, project, get_test_users, get_test_groups, get_test_r } } - self.assert_expected_grants_match_actual(project, "my_model_table", expected) + actual_grants = self.get_grants_on_relation(project, "my_model_table") + self.assert_expected_grants_match_actual(project, actual_grants, expected) # Table materialization, change select grant updated_yaml = self.interpolate_name_overrides(extended2_table_model_schema_yml) @@ -217,7 +222,8 @@ def test_table_grants(self, project, get_test_users, get_test_groups, get_test_r "role": [test_roles[1]], } } - self.assert_expected_grants_match_actual(project, "my_model_table", expected) + actual_grants = self.get_grants_on_relation(project, "my_model_table") + self.assert_expected_grants_match_actual(project, actual_grants, expected) # Table materialization, multiple grantees updated_yaml = self.interpolate_name_overrides( @@ -236,7 +242,8 @@ def test_table_grants(self, project, get_test_users, get_test_groups, get_test_r "role": [test_roles[0], test_roles[1]], } } - self.assert_expected_grants_match_actual(project, "my_model_table", expected) + actual_grants = self.get_grants_on_relation(project, "my_model_table") + self.assert_expected_grants_match_actual(project, actual_grants, expected) # Table materialization, multiple privileges updated_yaml = self.interpolate_name_overrides( @@ -260,4 +267,5 @@ def test_table_grants(self, project, get_test_users, get_test_groups, get_test_r "role": [test_roles[1]], }, } - self.assert_expected_grants_match_actual(project, "my_model_table", expected) + actual_grants = self.get_grants_on_relation(project, "my_model_table") + self.assert_expected_grants_match_actual(project, actual_grants, expected) diff --git a/tests/functional/adapter/grants/test_model_view_grants.py b/tests/functional/adapter/grants/test_model_view_grants.py index 5b0cc83fe..ebf1055a6 100644 --- a/tests/functional/adapter/grants/test_model_view_grants.py +++ b/tests/functional/adapter/grants/test_model_view_grants.py @@ -66,7 +66,9 @@ def test_view_table_grants(self, project, get_test_users, get_test_groups, get_t 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) + + actual_grants = self.get_grants_on_relation(project, "my_model_view") + self.assert_expected_grants_match_actual(project, actual_grants, expected) # View materialization, change select grant user updated_yaml = self.interpolate_name_overrides(user2_model_schema_yml) @@ -75,4 +77,5 @@ def test_view_table_grants(self, project, get_test_users, get_test_groups, get_t assert len(results) == 1 expected = {select_privilege_name: {"user": [test_users[1]]}} - self.assert_expected_grants_match_actual(project, "my_model_view", expected) + actual_grants = self.get_grants_on_relation(project, "my_model_view") + self.assert_expected_grants_match_actual(project, actual_grants, expected) diff --git a/tests/functional/adapter/grants/test_seed_grants.py b/tests/functional/adapter/grants/test_seed_grants.py index 01bb59594..feb5cda2f 100644 --- a/tests/functional/adapter/grants/test_seed_grants.py +++ b/tests/functional/adapter/grants/test_seed_grants.py @@ -93,7 +93,8 @@ def test_seed_grants(self, project, get_test_users): 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) + actual_grants = self.get_grants_on_relation(project, "my_seed") + self.assert_expected_grants_match_actual(project, actual_grants, expected) # run it again, with no config changes (results, log_output) = run_dbt_and_capture(["--debug", "seed"]) @@ -105,7 +106,8 @@ def test_seed_grants(self, project, get_test_users): 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) + actual_grants = self.get_grants_on_relation(project, "my_seed") + self.assert_expected_grants_match_actual(project, actual_grants, expected) # change the grantee, assert it updates updated_yaml = self.interpolate_name_overrides(user2_schema_base_yml) @@ -113,11 +115,13 @@ def test_seed_grants(self, project, get_test_users): (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) + actual_grants = self.get_grants_on_relation(project, "my_seed") + self.assert_expected_grants_match_actual(project, actual_grants, 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) + actual_grants = self.get_grants_on_relation(project, "my_seed") + self.assert_expected_grants_match_actual(project, actual_grants, expected) # change config to 'grants: {}' -- should be completely ignored updated_yaml = self.interpolate_name_overrides(ignore_grants_yml) @@ -132,12 +136,13 @@ def test_seed_grants(self, project, get_test_users): expected_config = {} expected_actual = {select_privilege_name: {"user": [test_users[1]]}} assert seed.config.grants == expected_config + actual_grants = self.get_grants_on_relation(project, "my_seed") if self.seeds_support_partial_refresh(): # ACTUAL grants will NOT match expected grants - self.assert_expected_grants_match_actual(project, "my_seed", expected_actual) + self.assert_expected_grants_match_actual(project, actual_grants, expected_actual) else: # there should be ZERO grants on the seed - self.assert_expected_grants_match_actual(project, "my_seed", expected_config) + self.assert_expected_grants_match_actual(project, actual_grants, expected_config) # now run with ZERO grants -- all grants should be removed # whether explicitly (revoke) or implicitly (recreated without any grants added on) @@ -148,14 +153,16 @@ def test_seed_grants(self, project, get_test_users): if self.seeds_support_partial_refresh(): assert "revoke " in log_output expected = {} - self.assert_expected_grants_match_actual(project, "my_seed", expected) + actual_grants = self.get_grants_on_relation(project, "my_seed") + self.assert_expected_grants_match_actual(project, actual_grants, 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) + actual_grants = self.get_grants_on_relation(project, "my_seed") + self.assert_expected_grants_match_actual(project, actual_grants, expected) # run with ZERO grants -- since all grants were removed previously # nothing should have changed @@ -166,4 +173,5 @@ def test_seed_grants(self, project, get_test_users): assert "grant " not in log_output assert "revoke " not in log_output expected = {} - self.assert_expected_grants_match_actual(project, "my_seed", expected) + actual_grants = self.get_grants_on_relation(project, "my_seed") + self.assert_expected_grants_match_actual(project, actual_grants, expected) diff --git a/tests/functional/adapter/grants/test_snapshot_grants.py b/tests/functional/adapter/grants/test_snapshot_grants.py index c9e3e2947..bc0d3691a 100644 --- a/tests/functional/adapter/grants/test_snapshot_grants.py +++ b/tests/functional/adapter/grants/test_snapshot_grants.py @@ -86,14 +86,16 @@ def test_snapshot_grants(self, project, get_test_users, get_test_groups, get_tes 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) + actual_grants = self.get_grants_on_relation(project, "my_snapshot") + self.assert_expected_grants_match_actual(project, actual_grants, 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) + actual_grants = self.get_grants_on_relation(project, "my_snapshot") + self.assert_expected_grants_match_actual(project, actual_grants, expected) # change the grantee, assert it updates updated_yaml = self.interpolate_name_overrides(user2_snapshot_schema_yml) @@ -101,7 +103,8 @@ def test_snapshot_grants(self, project, get_test_users, get_test_groups, get_tes (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) + actual_grants = self.get_grants_on_relation(project, "my_snapshot") + self.assert_expected_grants_match_actual(project, actual_grants, expected) # change the grants, assert that it updates updated_yaml = self.interpolate_name_overrides(extended_snapshot_schema_yml) @@ -115,7 +118,8 @@ def test_snapshot_grants(self, project, get_test_users, get_test_groups, get_tes "role": [test_roles[0]], } } - self.assert_expected_grants_match_actual(project, "my_snapshot", expected) + actual_grants = self.get_grants_on_relation(project, "my_snapshot") + self.assert_expected_grants_match_actual(project, actual_grants, expected) # change the grants again, assert that it updates updated_yaml = self.interpolate_name_overrides(extended2_snapshot_schema_yml) @@ -129,4 +133,5 @@ def test_snapshot_grants(self, project, get_test_users, get_test_groups, get_tes "role": [test_roles[1]], } } - self.assert_expected_grants_match_actual(project, "my_snapshot", expected) + actual_grants = self.get_grants_on_relation(project, "my_snapshot") + self.assert_expected_grants_match_actual(project, actual_grants, expected) From 9fd05c165c4ff58f27173716b2f2079ed46efedc Mon Sep 17 00:00:00 2001 From: Colin Date: Fri, 3 Nov 2023 14:41:12 -0700 Subject: [PATCH 19/33] change rolename alias to grantee in get_show_grant_sql --- dbt/include/redshift/macros/adapters/apply_grants.sql | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/dbt/include/redshift/macros/adapters/apply_grants.sql b/dbt/include/redshift/macros/adapters/apply_grants.sql index c6dedfa3f..d2a782f37 100644 --- a/dbt/include/redshift/macros/adapters/apply_grants.sql +++ b/dbt/include/redshift/macros/adapters/apply_grants.sql @@ -83,7 +83,7 @@ union all -- check that role has table privilege select 'role' as grantee_type, - r.role_name as rolename, + r.role_name as grantee, p.privilege_type from svv_roles r cross join privileges p From 1f2f9e0b7812bf919d4a8af35fed4de4626f43f8 Mon Sep 17 00:00:00 2001 From: Doug Beatty <44704949+dbeatty10@users.noreply.github.com> Date: Tue, 9 Jul 2024 11:21:54 -0600 Subject: [PATCH 20/33] Update package due to decoupling of dbt-adapters from dbt-core --- tests/functional/adapter/conftest.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/functional/adapter/conftest.py b/tests/functional/adapter/conftest.py index 32c8b5e1e..53a9405b4 100644 --- a/tests/functional/adapter/conftest.py +++ b/tests/functional/adapter/conftest.py @@ -1,7 +1,7 @@ import pytest import os -from dbt.exceptions import DbtDatabaseError +from dbt_common.exceptions import DbtDatabaseError # This is a hack to prevent the fixture from running more than once GRANTS_AND_ROLES_SETUP = False From 53956b44a25f83c9ac090d2ef9e4f32d588d0e34 Mon Sep 17 00:00:00 2001 From: Doug Beatty <44704949+dbeatty10@users.noreply.github.com> Date: Wed, 10 Jul 2024 15:28:34 -0600 Subject: [PATCH 21/33] Update body of changelog entry Co-authored-by: Colin Rogers <111200756+colin-rogers-dbt@users.noreply.github.com> --- .changes/unreleased/Features-20231004-103938.yaml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.changes/unreleased/Features-20231004-103938.yaml b/.changes/unreleased/Features-20231004-103938.yaml index 7c9090171..37982fd3b 100644 --- a/.changes/unreleased/Features-20231004-103938.yaml +++ b/.changes/unreleased/Features-20231004-103938.yaml @@ -1,5 +1,5 @@ kind: Features -body: 'Support groups and roles for grants ' +body: 'Add support for specifying groups and roles in grant statements' time: 2023-10-04T10:39:38.680813-07:00 custom: Author: soksamnanglim From 5901cf2ba8d6ea713b860858ffc6742983185987 Mon Sep 17 00:00:00 2001 From: Doug Beatty Date: Wed, 10 Jul 2024 15:43:26 -0600 Subject: [PATCH 22/33] Update whitespace --- .../redshift/macros/adapters/apply_grants.sql | 16 ++++++++-------- .../adapter/grants/test_model_table_grants.py | 10 +++++----- 2 files changed, 13 insertions(+), 13 deletions(-) diff --git a/dbt/include/redshift/macros/adapters/apply_grants.sql b/dbt/include/redshift/macros/adapters/apply_grants.sql index d2a782f37..2351a66a1 100644 --- a/dbt/include/redshift/macros/adapters/apply_grants.sql +++ b/dbt/include/redshift/macros/adapters/apply_grants.sql @@ -4,11 +4,11 @@ {#-- 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 -%} + {%- if grantee_type == 'user' and grantees -%} {{ " " + (grantees | join(', ')) }} - {%- elif grantee_type=='group' and grantees -%} - {{ " " +("group " + grantees | join(', group ')) }} - {%- elif grantee_type=='role' and grantees -%} + {%- 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 -%} @@ -21,11 +21,11 @@ {#-- 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 -%} + {%- if revokee_type == 'user' and revokees -%} {{ " " + (revokees | join(', ')) }} - {%- elif revokee_type=='group' and revokees -%} - {{ " " +("group " + revokees | join(', group ')) }} - {%- elif revokee_type=='role' and revokees -%} + {%- 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 -%} diff --git a/tests/functional/adapter/grants/test_model_table_grants.py b/tests/functional/adapter/grants/test_model_table_grants.py index 300117615..2e9108722 100644 --- a/tests/functional/adapter/grants/test_model_table_grants.py +++ b/tests/functional/adapter/grants/test_model_table_grants.py @@ -62,7 +62,7 @@ select: user: ["{{ env_var('DBT_TEST_USER_1') }}"] group: ["{{ env_var('DBT_TEST_GROUP_1') }}"] - role: ["{{ env_var('DBT_TEST_ROLE_1') }}"] + role: ["{{ env_var('DBT_TEST_ROLE_1') }}"] """ # table materialization change select @@ -76,7 +76,7 @@ select: user: ["{{ env_var('DBT_TEST_USER_2') }}"] group: ["{{ env_var('DBT_TEST_GROUP_2') }}"] - role: ["{{ env_var('DBT_TEST_ROLE_2') }}"] + role: ["{{ env_var('DBT_TEST_ROLE_2') }}"] """ # table materialization multiple grantees @@ -90,7 +90,7 @@ 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') }}"] + role: ["{{ env_var('DBT_TEST_ROLE_1') }}", "{{ env_var('DBT_TEST_ROLE_2') }}"] """ # table materialization multiple privileges extended_multiple_privileges_table_model_schema_yml = """ @@ -103,11 +103,11 @@ select: user: ["{{ env_var('DBT_TEST_USER_1') }}"] group: ["{{ env_var('DBT_TEST_GROUP_1') }}"] - role: ["{{ env_var('DBT_TEST_ROLE_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') }}"] + role: ["{{ env_var('DBT_TEST_ROLE_2') }}"] """ From f37d91daf168931aa369c2ac91fb6ac0f5fd33b8 Mon Sep 17 00:00:00 2001 From: Doug Beatty Date: Wed, 10 Jul 2024 16:04:02 -0600 Subject: [PATCH 23/33] Try printing the environment variables configured in the CI environment --- .../adapter/grants/test_model_table_grants.py | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/tests/functional/adapter/grants/test_model_table_grants.py b/tests/functional/adapter/grants/test_model_table_grants.py index 2e9108722..bb52cbda0 100644 --- a/tests/functional/adapter/grants/test_model_table_grants.py +++ b/tests/functional/adapter/grants/test_model_table_grants.py @@ -7,6 +7,18 @@ from tests.functional.adapter.grants.base_grants import BaseGrantsRedshift my_model_sql = """ + {{ log("DBT_TEST_USER_1: " ~ env_var('DBT_TEST_USER_1'), True) }} + {{ log("DBT_TEST_USER_2: " ~ env_var('DBT_TEST_USER_2'), True) }} + {{ log("DBT_TEST_USER_3: " ~ env_var('DBT_TEST_USER_3'), True) }} + + {{ log("DBT_TEST_GROUP_1: " ~ env_var('DBT_TEST_GROUP_1'), True) }} + {{ log("DBT_TEST_GROUP_2: " ~ env_var('DBT_TEST_GROUP_2'), True) }} + {{ log("DBT_TEST_GROUP_3: " ~ env_var('DBT_TEST_GROUP_3'), True) }} + + {{ log("DBT_TEST_ROLE_1: " ~ env_var('DBT_TEST_ROLE_1'), True) }} + {{ log("DBT_TEST_ROLE_2: " ~ env_var('DBT_TEST_ROLE_2'), True) }} + {{ log("DBT_TEST_ROLE_3: " ~ env_var('DBT_TEST_ROLE_3'), True) }} + select 1 as fun """ From 220c810a9273bd598db02f3b07e3c7644ed387f0 Mon Sep 17 00:00:00 2001 From: Doug Beatty Date: Wed, 10 Jul 2024 16:38:42 -0600 Subject: [PATCH 24/33] Examine roles in the Redshift database --- .../adapter/grants/test_model_table_grants.py | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/tests/functional/adapter/grants/test_model_table_grants.py b/tests/functional/adapter/grants/test_model_table_grants.py index bb52cbda0..0f8dc90aa 100644 --- a/tests/functional/adapter/grants/test_model_table_grants.py +++ b/tests/functional/adapter/grants/test_model_table_grants.py @@ -19,6 +19,19 @@ {{ log("DBT_TEST_ROLE_2: " ~ env_var('DBT_TEST_ROLE_2'), True) }} {{ log("DBT_TEST_ROLE_3: " ~ env_var('DBT_TEST_ROLE_3'), True) }} + {% set query = 'select * from svv_roles r' %} + {% set results = run_query(query) %} + + {{ log("query: " ~ query, True) }} + + {{ log("Start query results", True) }} + + {% for result in results %} + {{ log(result, True) }} + {% endfor %} + + {{ log("End query results", True) }} + select 1 as fun """ From c56c61da786a66f49b4723fd78803e6b3b938049 Mon Sep 17 00:00:00 2001 From: Doug Beatty Date: Wed, 10 Jul 2024 17:05:46 -0600 Subject: [PATCH 25/33] Remove debugging logic --- .../adapter/grants/test_model_table_grants.py | 25 ------------------- 1 file changed, 25 deletions(-) diff --git a/tests/functional/adapter/grants/test_model_table_grants.py b/tests/functional/adapter/grants/test_model_table_grants.py index 0f8dc90aa..2e9108722 100644 --- a/tests/functional/adapter/grants/test_model_table_grants.py +++ b/tests/functional/adapter/grants/test_model_table_grants.py @@ -7,31 +7,6 @@ from tests.functional.adapter.grants.base_grants import BaseGrantsRedshift my_model_sql = """ - {{ log("DBT_TEST_USER_1: " ~ env_var('DBT_TEST_USER_1'), True) }} - {{ log("DBT_TEST_USER_2: " ~ env_var('DBT_TEST_USER_2'), True) }} - {{ log("DBT_TEST_USER_3: " ~ env_var('DBT_TEST_USER_3'), True) }} - - {{ log("DBT_TEST_GROUP_1: " ~ env_var('DBT_TEST_GROUP_1'), True) }} - {{ log("DBT_TEST_GROUP_2: " ~ env_var('DBT_TEST_GROUP_2'), True) }} - {{ log("DBT_TEST_GROUP_3: " ~ env_var('DBT_TEST_GROUP_3'), True) }} - - {{ log("DBT_TEST_ROLE_1: " ~ env_var('DBT_TEST_ROLE_1'), True) }} - {{ log("DBT_TEST_ROLE_2: " ~ env_var('DBT_TEST_ROLE_2'), True) }} - {{ log("DBT_TEST_ROLE_3: " ~ env_var('DBT_TEST_ROLE_3'), True) }} - - {% set query = 'select * from svv_roles r' %} - {% set results = run_query(query) %} - - {{ log("query: " ~ query, True) }} - - {{ log("Start query results", True) }} - - {% for result in results %} - {{ log(result, True) }} - {% endfor %} - - {{ log("End query results", True) }} - select 1 as fun """ From 00231be43ed3db31fe0a97d572298654f382ba5a Mon Sep 17 00:00:00 2001 From: Doug Beatty Date: Wed, 10 Jul 2024 17:39:19 -0600 Subject: [PATCH 26/33] Debug output for setting up groups and roles for CI --- tests/functional/adapter/conftest.py | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/tests/functional/adapter/conftest.py b/tests/functional/adapter/conftest.py index 53a9405b4..8c024c435 100644 --- a/tests/functional/adapter/conftest.py +++ b/tests/functional/adapter/conftest.py @@ -20,12 +20,15 @@ @pytest.fixture(scope="class", autouse=True) def setup_grants_and_roles(project): + print("Start setup for groups and roles") + global GRANTS_AND_ROLES_SETUP 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: + print("Create groups and roles") with project.adapter.connection_named("__test"): for group in GROUPS.values(): try: @@ -43,6 +46,8 @@ def setup_grants_and_roles(project): GRANTS_AND_ROLES_SETUP = True + print("End setup for groups and roles") + @pytest.fixture def model_ddl(request) -> str: From 30043a70dc60f3cfcd1fbd967fb8ff090f391aa0 Mon Sep 17 00:00:00 2001 From: Doug Beatty Date: Wed, 10 Jul 2024 18:02:19 -0600 Subject: [PATCH 27/33] Force creation of groups and roles --- tests/functional/adapter/conftest.py | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/tests/functional/adapter/conftest.py b/tests/functional/adapter/conftest.py index 8c024c435..f15088148 100644 --- a/tests/functional/adapter/conftest.py +++ b/tests/functional/adapter/conftest.py @@ -27,11 +27,13 @@ def setup_grants_and_roles(project): 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: + # if not GRANTS_AND_ROLES_SETUP: + if True: print("Create groups and roles") with project.adapter.connection_named("__test"): for group in GROUPS.values(): try: + print(f"CREATE GROUP {group}") project.adapter.execute(f"CREATE GROUP {group}") except DbtDatabaseError: # This is expected if the group already exists @@ -39,6 +41,7 @@ def setup_grants_and_roles(project): for role in ROLES.values(): try: + print(f"CREATE ROLE {group}") project.adapter.execute(f"CREATE ROLE {role}") except DbtDatabaseError: # This is expected if the group already exists From 12d339dd702354429dc0c25117533c75b6385cc9 Mon Sep 17 00:00:00 2001 From: Doug Beatty Date: Wed, 10 Jul 2024 18:51:35 -0600 Subject: [PATCH 28/33] Fix typo --- tests/functional/adapter/conftest.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/functional/adapter/conftest.py b/tests/functional/adapter/conftest.py index f15088148..1c6024d23 100644 --- a/tests/functional/adapter/conftest.py +++ b/tests/functional/adapter/conftest.py @@ -41,7 +41,7 @@ def setup_grants_and_roles(project): for role in ROLES.values(): try: - print(f"CREATE ROLE {group}") + print(f"CREATE ROLE {role}") project.adapter.execute(f"CREATE ROLE {role}") except DbtDatabaseError: # This is expected if the group already exists From 3aeff0d58cf264e2060cb18c4178ee324a656d22 Mon Sep 17 00:00:00 2001 From: Doug Beatty Date: Wed, 10 Jul 2024 18:54:51 -0600 Subject: [PATCH 29/33] Debugging database roles --- .../adapter/grants/test_model_table_grants.py | 25 +++++++++++++++++++ 1 file changed, 25 insertions(+) diff --git a/tests/functional/adapter/grants/test_model_table_grants.py b/tests/functional/adapter/grants/test_model_table_grants.py index 2e9108722..0f8dc90aa 100644 --- a/tests/functional/adapter/grants/test_model_table_grants.py +++ b/tests/functional/adapter/grants/test_model_table_grants.py @@ -7,6 +7,31 @@ from tests.functional.adapter.grants.base_grants import BaseGrantsRedshift my_model_sql = """ + {{ log("DBT_TEST_USER_1: " ~ env_var('DBT_TEST_USER_1'), True) }} + {{ log("DBT_TEST_USER_2: " ~ env_var('DBT_TEST_USER_2'), True) }} + {{ log("DBT_TEST_USER_3: " ~ env_var('DBT_TEST_USER_3'), True) }} + + {{ log("DBT_TEST_GROUP_1: " ~ env_var('DBT_TEST_GROUP_1'), True) }} + {{ log("DBT_TEST_GROUP_2: " ~ env_var('DBT_TEST_GROUP_2'), True) }} + {{ log("DBT_TEST_GROUP_3: " ~ env_var('DBT_TEST_GROUP_3'), True) }} + + {{ log("DBT_TEST_ROLE_1: " ~ env_var('DBT_TEST_ROLE_1'), True) }} + {{ log("DBT_TEST_ROLE_2: " ~ env_var('DBT_TEST_ROLE_2'), True) }} + {{ log("DBT_TEST_ROLE_3: " ~ env_var('DBT_TEST_ROLE_3'), True) }} + + {% set query = 'select * from svv_roles r' %} + {% set results = run_query(query) %} + + {{ log("query: " ~ query, True) }} + + {{ log("Start query results", True) }} + + {% for result in results %} + {{ log(result, True) }} + {% endfor %} + + {{ log("End query results", True) }} + select 1 as fun """ From c8e06f5f89ce0b8e223840ebb713c87f427f375a Mon Sep 17 00:00:00 2001 From: Doug Beatty Date: Thu, 11 Jul 2024 09:02:37 -0600 Subject: [PATCH 30/33] Re-run CI tests --- tests/functional/adapter/grants/test_model_table_grants.py | 1 + 1 file changed, 1 insertion(+) diff --git a/tests/functional/adapter/grants/test_model_table_grants.py b/tests/functional/adapter/grants/test_model_table_grants.py index 0f8dc90aa..65497818b 100644 --- a/tests/functional/adapter/grants/test_model_table_grants.py +++ b/tests/functional/adapter/grants/test_model_table_grants.py @@ -20,6 +20,7 @@ {{ log("DBT_TEST_ROLE_3: " ~ env_var('DBT_TEST_ROLE_3'), True) }} {% set query = 'select * from svv_roles r' %} + {% set results = run_query(query) %} {{ log("query: " ~ query, True) }} From eb07cfbf4461a88ce5f298eef55ff5396615b8d7 Mon Sep 17 00:00:00 2001 From: Doug Beatty Date: Thu, 11 Jul 2024 11:07:42 -0600 Subject: [PATCH 31/33] Remove debugging --- tests/functional/adapter/conftest.py | 10 +------ .../adapter/grants/test_model_table_grants.py | 26 ------------------- 2 files changed, 1 insertion(+), 35 deletions(-) diff --git a/tests/functional/adapter/conftest.py b/tests/functional/adapter/conftest.py index 1c6024d23..53a9405b4 100644 --- a/tests/functional/adapter/conftest.py +++ b/tests/functional/adapter/conftest.py @@ -20,20 +20,15 @@ @pytest.fixture(scope="class", autouse=True) def setup_grants_and_roles(project): - print("Start setup for groups and roles") - global GRANTS_AND_ROLES_SETUP 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: - if True: - print("Create groups and roles") + if not GRANTS_AND_ROLES_SETUP: with project.adapter.connection_named("__test"): for group in GROUPS.values(): try: - print(f"CREATE GROUP {group}") project.adapter.execute(f"CREATE GROUP {group}") except DbtDatabaseError: # This is expected if the group already exists @@ -41,7 +36,6 @@ def setup_grants_and_roles(project): for role in ROLES.values(): try: - print(f"CREATE ROLE {role}") project.adapter.execute(f"CREATE ROLE {role}") except DbtDatabaseError: # This is expected if the group already exists @@ -49,8 +43,6 @@ def setup_grants_and_roles(project): GRANTS_AND_ROLES_SETUP = True - print("End setup for groups and roles") - @pytest.fixture def model_ddl(request) -> str: diff --git a/tests/functional/adapter/grants/test_model_table_grants.py b/tests/functional/adapter/grants/test_model_table_grants.py index 65497818b..2e9108722 100644 --- a/tests/functional/adapter/grants/test_model_table_grants.py +++ b/tests/functional/adapter/grants/test_model_table_grants.py @@ -7,32 +7,6 @@ from tests.functional.adapter.grants.base_grants import BaseGrantsRedshift my_model_sql = """ - {{ log("DBT_TEST_USER_1: " ~ env_var('DBT_TEST_USER_1'), True) }} - {{ log("DBT_TEST_USER_2: " ~ env_var('DBT_TEST_USER_2'), True) }} - {{ log("DBT_TEST_USER_3: " ~ env_var('DBT_TEST_USER_3'), True) }} - - {{ log("DBT_TEST_GROUP_1: " ~ env_var('DBT_TEST_GROUP_1'), True) }} - {{ log("DBT_TEST_GROUP_2: " ~ env_var('DBT_TEST_GROUP_2'), True) }} - {{ log("DBT_TEST_GROUP_3: " ~ env_var('DBT_TEST_GROUP_3'), True) }} - - {{ log("DBT_TEST_ROLE_1: " ~ env_var('DBT_TEST_ROLE_1'), True) }} - {{ log("DBT_TEST_ROLE_2: " ~ env_var('DBT_TEST_ROLE_2'), True) }} - {{ log("DBT_TEST_ROLE_3: " ~ env_var('DBT_TEST_ROLE_3'), True) }} - - {% set query = 'select * from svv_roles r' %} - - {% set results = run_query(query) %} - - {{ log("query: " ~ query, True) }} - - {{ log("Start query results", True) }} - - {% for result in results %} - {{ log(result, True) }} - {% endfor %} - - {{ log("End query results", True) }} - select 1 as fun """ From 655cb538664cf65aa91f9ef6fcb9b88e9f006272 Mon Sep 17 00:00:00 2001 From: Doug Beatty Date: Thu, 11 Jul 2024 11:27:29 -0600 Subject: [PATCH 32/33] Add groups and roles for various release workflows in GitHub Actions --- scripts/env-setup.sh | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/scripts/env-setup.sh b/scripts/env-setup.sh index 866d8f749..844958b08 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 From 5247bab7c7e4a72a3d4174272df7e21e0181f870 Mon Sep 17 00:00:00 2001 From: Doug Beatty Date: Thu, 11 Jul 2024 11:28:14 -0600 Subject: [PATCH 33/33] Database groups and roles for testing locally --- test.env.example | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/test.env.example b/test.env.example index 6816b4ec2..17360a6d8 100644 --- a/test.env.example +++ b/test.env.example @@ -24,3 +24,13 @@ REDSHIFT_TEST_IAM_ROLE_PROFILE= DBT_TEST_USER_1=dbt_test_user_1 DBT_TEST_USER_2=dbt_test_user_2 DBT_TEST_USER_3=dbt_test_user_3 + +# Database groups for testing +DBT_TEST_GROUP_1=dbt_test_group_1 +DBT_TEST_GROUP_2=dbt_test_group_2 +DBT_TEST_GROUP_3=dbt_test_group_3 + +# Database roles for testing +DBT_TEST_ROLE_1=dbt_test_role_1 +DBT_TEST_ROLE_2=dbt_test_role_2 +DBT_TEST_ROLE_3=dbt_test_role_3