diff --git a/.changes/unreleased/Features-20220711-111514.yaml b/.changes/unreleased/Features-20220711-111514.yaml new file mode 100644 index 00000000000..020274a47cb --- /dev/null +++ b/.changes/unreleased/Features-20220711-111514.yaml @@ -0,0 +1,8 @@ +kind: Features +body: Allow users to define grants as a reasonable default in the dbt_project.yml + or within each model sql or yml file combined. +time: 2022-07-11T11:15:14.695386-05:00 +custom: + Author: McKnight-42 + Issue: "5263" + PR: "5369" diff --git a/.changes/unreleased/Under the Hood-20220706-215001.yaml b/.changes/unreleased/Under the Hood-20220706-215001.yaml new file mode 100644 index 00000000000..a18d87f19a8 --- /dev/null +++ b/.changes/unreleased/Under the Hood-20220706-215001.yaml @@ -0,0 +1,7 @@ +kind: Under the Hood +body: Add tests for SQL grants +time: 2022-07-06T21:50:01.498562-04:00 +custom: + Author: gshank + Issue: "5437" + PR: "5447" diff --git a/.github/workflows/main.yml b/.github/workflows/main.yml index d2ede8e3d48..216055031bd 100644 --- a/.github/workflows/main.yml +++ b/.github/workflows/main.yml @@ -125,6 +125,9 @@ jobs: TOXENV: integration PYTEST_ADDOPTS: "-v --color=yes -n4 --csv integration_results.csv" DBT_INVOCATION_ENV: github-actions + DBT_TEST_USER_1: dbt_test_user_1 + DBT_TEST_USER_2: dbt_test_user_2 + DBT_TEST_USER_3: dbt_test_user_3 steps: - name: Check out the repository diff --git a/.github/workflows/structured-logging-schema-check.yml b/.github/workflows/structured-logging-schema-check.yml index 0eb359fbebe..67392b973c7 100644 --- a/.github/workflows/structured-logging-schema-check.yml +++ b/.github/workflows/structured-logging-schema-check.yml @@ -30,6 +30,11 @@ jobs: LOG_DIR: "/home/runner/work/dbt-core/dbt-core/logs" # tells integration tests to output into json format DBT_LOG_FORMAT: "json" + # Additional test users + DBT_TEST_USER_1: dbt_test_user_1 + DBT_TEST_USER_2: dbt_test_user_2 + DBT_TEST_USER_3: dbt_test_user_3 + steps: - name: checkout dev uses: actions/checkout@v2 diff --git a/core/dbt/adapters/base/impl.py b/core/dbt/adapters/base/impl.py index 18d3af5ebff..a77a14717a6 100644 --- a/core/dbt/adapters/base/impl.py +++ b/core/dbt/adapters/base/impl.py @@ -159,6 +159,7 @@ class BaseAdapter(metaclass=AdapterMeta): - convert_datetime_type - convert_date_type - convert_time_type + - standardize_grants_dict Macros: - get_catalog @@ -538,6 +539,33 @@ def list_relations_without_caching(self, schema_relation: BaseRelation) -> List[ "`list_relations_without_caching` is not implemented for this " "adapter!" ) + ### + # Methods about grants + ### + @available + def standardize_grants_dict(self, grants_table: agate.Table) -> dict: + """Translate the result of `show grants` (or equivalent) to match the + grants which a user would configure in their project. + + Ideally, the SQL to show grants should also be filtering: + filter OUT any grants TO the current user/role (e.g. OWNERSHIP). + If that's not possible in SQL, it can be done in this method instead. + + :param grants_table: An agate table containing the query result of + the SQL returned by get_show_grant_sql + :return: A standardized dictionary matching the `grants` config + :rtype: dict + """ + grants_dict: Dict[str, List[str]] = {} + for row in grants_table: + grantee = row["grantee"] + privilege = row["privilege_type"] + if privilege in grants_dict.keys(): + grants_dict[privilege].append(grantee) + else: + grants_dict.update({privilege: [grantee]}) + return grants_dict + ### # Provided methods about relations ### diff --git a/core/dbt/context/base.py b/core/dbt/context/base.py index c2bf912d55c..4a71388f81d 100644 --- a/core/dbt/context/base.py +++ b/core/dbt/context/base.py @@ -1,6 +1,6 @@ import json import os -from typing import Any, Dict, NoReturn, Optional, Mapping, Iterable, Set +from typing import Any, Dict, NoReturn, Optional, Mapping, Iterable, Set, List from dbt import flags from dbt import tracking @@ -657,6 +657,35 @@ def print(msg: str) -> str: print(msg) return "" + @contextmember + @staticmethod + def diff_of_two_dicts( + dict_a: Dict[str, List[str]], dict_b: Dict[str, List[str]] + ) -> Dict[str, List[str]]: + """ + Given two dictionaries of type Dict[str, List[str]]: + dict_a = {'key_x': ['value_1', 'VALUE_2'], 'KEY_Y': ['value_3']} + dict_b = {'key_x': ['value_1'], 'key_z': ['value_4']} + 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 = {} + dict_b_lowered = {k.casefold(): [x.casefold() for x in v] for k, v in dict_b.items()} + for k in dict_a: + if k.casefold() in dict_b_lowered.keys(): + diff = [] + for v in dict_a[k]: + if v.casefold() not in dict_b_lowered[k.casefold()]: + diff.append(v) + if diff: + dict_diff.update({k: diff}) + else: + dict_diff.update({k: dict_a[k]}) + return dict_diff + def generate_base_context(cli_vars: Dict[str, Any]) -> Dict[str, Any]: ctx = BaseContext(cli_vars) diff --git a/core/dbt/include/global_project/macros/adapters/apply_grants.sql b/core/dbt/include/global_project/macros/adapters/apply_grants.sql new file mode 100644 index 00000000000..10906e7ffa7 --- /dev/null +++ b/core/dbt/include/global_project/macros/adapters/apply_grants.sql @@ -0,0 +1,167 @@ +{# ------- BOOLEAN MACROS --------- #} + +{# + -- COPY GRANTS + -- When a relational object (view or table) is replaced in this database, + -- do previous grants carry over to the new object? This may depend on: + -- whether we use alter-rename-swap versus CREATE OR REPLACE + -- user-supplied configuration (e.g. copy_grants on Snowflake) + -- By default, play it safe, assume TRUE: that grants ARE copied over. + -- This means dbt will first "show" current grants and then calculate diffs. + -- It may require an additional query than is strictly necessary, + -- but better safe than sorry. +#} + +{% macro copy_grants() %} + {{ return(adapter.dispatch('copy_grants', 'dbt')()) }} +{% endmacro %} + +{% macro default__copy_grants() %} + {{ return(True) }} +{% endmacro %} + + +{# + -- SUPPORT MULTIPLE GRANTEES PER DCL STATEMENT + -- Does this database support 'grant {privilege} to {grantee_1}, {grantee_2}, ...' + -- Or must these be separate statements: + -- `grant {privilege} to {grantee_1}`; + -- `grant {privilege} to {grantee_2}`; + -- By default, pick the former, because it's what we prefer when available. +#} + +{% macro support_multiple_grantees_per_dcl_statement() %} + {{ return(adapter.dispatch('support_multiple_grantees_per_dcl_statement', 'dbt')()) }} +{% endmacro %} + +{%- macro default__support_multiple_grantees_per_dcl_statement() -%} + {{ return(True) }} +{%- endmacro -%} + + +{% macro should_revoke(existing_relation, full_refresh_mode=True) %} + + {% if not existing_relation %} + {#-- The table doesn't already exist, so no grants to copy over --#} + {{ return(False) }} + {% elif full_refresh_mode %} + {#-- The object is being REPLACED -- whether grants are copied over depends on the value of user config --#} + {{ return(copy_grants()) }} + {% else %} + {#-- The table is being merged/upserted/inserted -- grants will be carried over --#} + {{ return(True) }} + {% endif %} + +{% endmacro %} + +{# ------- DCL STATEMENT TEMPLATES --------- #} + +{% macro get_show_grant_sql(relation) %} + {{ return(adapter.dispatch("get_show_grant_sql", "dbt")(relation)) }} +{% endmacro %} + +{% macro default__get_show_grant_sql(relation) %} + show grants on {{ relation }} +{% endmacro %} + + +{% macro get_grant_sql(relation, privilege, grantees) %} + {{ return(adapter.dispatch('get_grant_sql', 'dbt')(relation, privilege, grantees)) }} +{% endmacro %} + +{%- macro default__get_grant_sql(relation, privilege, grantees) -%} + grant {{ privilege }} on {{ relation }} to {{ grantees | join(', ') }} +{%- endmacro -%} + + +{% macro get_revoke_sql(relation, privilege, grantees) %} + {{ return(adapter.dispatch('get_revoke_sql', 'dbt')(relation, privilege, grantees)) }} +{% endmacro %} + +{%- macro default__get_revoke_sql(relation, privilege, grantees) -%} + revoke {{ privilege }} on {{ relation }} from {{ grantees | join(', ') }} +{%- endmacro -%} + + +{# ------- RUNTIME APPLICATION --------- #} + +{% macro get_dcl_statement_list(relation, grant_config, get_dcl_macro) %} + {{ return(adapter.dispatch('get_dcl_statement_list', 'dbt')(relation, grant_config, get_dcl_macro)) }} +{% endmacro %} + +{%- macro default__get_dcl_statement_list(relation, grant_config, get_dcl_macro) -%} + {# + -- Unpack grant_config into specific privileges and the set of users who need them granted/revoked. + -- Depending on whether this database supports multiple grantees per statement, pass in the list of + -- all grantees per privilege, or (if not) template one statement per privilege-grantee pair. + -- `get_dcl_macro` will be either `get_grant_sql` or `get_revoke_sql` + #} + {%- set dcl_statements = [] -%} + {%- for privilege, grantees in grant_config.items() %} + {%- if support_multiple_grantees_per_dcl_statement() and grantees -%} + {%- set dcl = get_dcl_macro(relation, privilege, grantees) -%} + {%- do dcl_statements.append(dcl) -%} + {%- else -%} + {%- for grantee in grantees -%} + {% set dcl = get_dcl_macro(relation, privilege, [grantee]) %} + {%- do dcl_statements.append(dcl) -%} + {% endfor -%} + {%- endif -%} + {%- endfor -%} + {{ return(dcl_statements) }} +{%- endmacro %} + + +{% macro call_dcl_statements(dcl_statement_list) %} + {{ return(adapter.dispatch("call_dcl_statements", "dbt")(dcl_statement_list)) }} +{% endmacro %} + +{% macro default__call_dcl_statements(dcl_statement_list) %} + {# + -- By default, supply all grant + revoke statements in a single semicolon-separated block, + -- so that they're all processed together. + + -- Some databases do not support this. Those adapters will need to override this macro + -- to run each statement individually. + #} + {% call statement('grants') %} + {% for dcl_statement in dcl_statement_list %} + {{ dcl_statement }}; + {% endfor %} + {% endcall %} +{% endmacro %} + + +{% macro apply_grants(relation, grant_config, should_revoke) %} + {{ return(adapter.dispatch("apply_grants", "dbt")(relation, grant_config, should_revoke)) }} +{% endmacro %} + +{% macro default__apply_grants(relation, grant_config, should_revoke=True) %} + {#-- If grant_config is {} or None, this is a no-op --#} + {% if grant_config %} + {% if should_revoke %} + {#-- We think previous grants may have carried over --#} + {#-- Show current grants and calculate 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 = diff_of_two_dicts(grant_config, current_grants_dict) %} + {% set needs_revoking = diff_of_two_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/core/dbt/include/global_project/macros/materializations/models/incremental/incremental.sql b/core/dbt/include/global_project/macros/materializations/models/incremental/incremental.sql index 1cb316b1ab0..52c141c27a2 100644 --- a/core/dbt/include/global_project/macros/materializations/models/incremental/incremental.sql +++ b/core/dbt/include/global_project/macros/materializations/models/incremental/incremental.sql @@ -20,6 +20,8 @@ -- BEGIN, in a separate transaction {%- set preexisting_intermediate_relation = load_cached_relation(intermediate_relation)-%} {%- set preexisting_backup_relation = load_cached_relation(backup_relation) -%} + -- grab current tables grants config for comparision later on + {% set grant_config = config.get('grants') %} {{ drop_relation_if_exists(preexisting_intermediate_relation) }} {{ drop_relation_if_exists(preexisting_backup_relation) }} @@ -59,6 +61,9 @@ {% do to_drop.append(backup_relation) %} {% endif %} + {% set should_revoke = should_revoke(existing_relation, full_refresh_mode) %} + {% do apply_grants(target_relation, grant_config, should_revoke=should_revoke) %} + {% do persist_docs(target_relation, model) %} {% if existing_relation is none or existing_relation.is_view or should_full_refresh() %} diff --git a/core/dbt/include/global_project/macros/materializations/models/table/table.sql b/core/dbt/include/global_project/macros/materializations/models/table/table.sql index 2009183ec41..d142310addd 100644 --- a/core/dbt/include/global_project/macros/materializations/models/table/table.sql +++ b/core/dbt/include/global_project/macros/materializations/models/table/table.sql @@ -14,6 +14,8 @@ {%- set backup_relation = make_backup_relation(target_relation, backup_relation_type) -%} -- as above, the backup_relation should not already exist {%- set preexisting_backup_relation = load_cached_relation(backup_relation) -%} + -- grab current tables grants config for comparision later on + {% set grant_config = config.get('grants') %} -- drop the temp relations if they exist already in the database {{ drop_relation_if_exists(preexisting_intermediate_relation) }} @@ -40,6 +42,9 @@ {{ run_hooks(post_hooks, inside_transaction=True) }} + {% set should_revoke = should_revoke(existing_relation, full_refresh_mode=True) %} + {% do apply_grants(target_relation, grant_config, should_revoke=should_revoke) %} + {% do persist_docs(target_relation, model) %} -- `COMMIT` happens here diff --git a/core/dbt/include/global_project/macros/materializations/models/view/create_or_replace_view.sql b/core/dbt/include/global_project/macros/materializations/models/view/create_or_replace_view.sql index 967086b7424..649daf2a0b6 100644 --- a/core/dbt/include/global_project/macros/materializations/models/view/create_or_replace_view.sql +++ b/core/dbt/include/global_project/macros/materializations/models/view/create_or_replace_view.sql @@ -13,12 +13,12 @@ {%- set identifier = model['alias'] -%} {%- set old_relation = adapter.get_relation(database=database, schema=schema, identifier=identifier) -%} - {%- set exists_as_view = (old_relation is not none and old_relation.is_view) -%} {%- set target_relation = api.Relation.create( identifier=identifier, schema=schema, database=database, type='view') -%} + {% set grant_config = config.get('grants') %} {{ run_hooks(pre_hooks) }} @@ -34,6 +34,9 @@ {{ get_create_view_as_sql(target_relation, sql) }} {%- endcall %} + {% set should_revoke = should_revoke(exists_as_view, full_refresh_mode=True) %} + {% do apply_grants(target_relation, grant_config, should_revoke=True) %} + {{ run_hooks(post_hooks) }} {{ return({'relations': [target_relation]}) }} diff --git a/core/dbt/include/global_project/macros/materializations/models/view/view.sql b/core/dbt/include/global_project/macros/materializations/models/view/view.sql index 776cf11dfa0..a135da12f31 100644 --- a/core/dbt/include/global_project/macros/materializations/models/view/view.sql +++ b/core/dbt/include/global_project/macros/materializations/models/view/view.sql @@ -25,6 +25,8 @@ {%- set backup_relation = make_backup_relation(target_relation, backup_relation_type) -%} -- as above, the backup_relation should not already exist {%- set preexisting_backup_relation = load_cached_relation(backup_relation) -%} + -- grab current tables grants config for comparision later on + {% set grant_config = config.get('grants') %} {{ run_hooks(pre_hooks, inside_transaction=False) }} @@ -47,6 +49,9 @@ {% endif %} {{ adapter.rename_relation(intermediate_relation, target_relation) }} + {% set should_revoke = should_revoke(existing_relation, full_refresh_mode=True) %} + {% do apply_grants(target_relation, grant_config, should_revoke=should_revoke) %} + {% do persist_docs(target_relation, model) %} {{ run_hooks(post_hooks, inside_transaction=True) }} diff --git a/core/dbt/include/global_project/macros/materializations/seeds/seed.sql b/core/dbt/include/global_project/macros/materializations/seeds/seed.sql index 7461d33eab0..d43b7f3ccf1 100644 --- a/core/dbt/include/global_project/macros/materializations/seeds/seed.sql +++ b/core/dbt/include/global_project/macros/materializations/seeds/seed.sql @@ -8,7 +8,10 @@ {%- set exists_as_table = (old_relation is not none and old_relation.is_table) -%} {%- set exists_as_view = (old_relation is not none and old_relation.is_view) -%} + {%- set grant_config = config.get('grants') -%} {%- set agate_table = load_agate_table() -%} + -- grab current tables grants config for comparision later on + {%- do store_result('agate_table', response='OK', agate_table=agate_table) -%} {{ run_hooks(pre_hooks, inside_transaction=False) }} @@ -35,6 +38,10 @@ {% endcall %} {% set target_relation = this.incorporate(type='table') %} + + {% set should_revoke = should_revoke(old_relation, full_refresh_mode) %} + {% do apply_grants(target_relation, grant_config, should_revoke=should_revoke) %} + {% do persist_docs(target_relation, model) %} {% if full_refresh_mode or not exists_as_table %} diff --git a/core/dbt/include/global_project/macros/materializations/snapshots/snapshot.sql b/core/dbt/include/global_project/macros/materializations/snapshots/snapshot.sql index c1da517c343..ccd0ef422ad 100644 --- a/core/dbt/include/global_project/macros/materializations/snapshots/snapshot.sql +++ b/core/dbt/include/global_project/macros/materializations/snapshots/snapshot.sql @@ -5,6 +5,8 @@ {%- set strategy_name = config.get('strategy') -%} {%- set unique_key = config.get('unique_key') %} + -- grab current tables grants config for comparision later on + {%- set grant_config = config.get('grants') -%} {% set target_relation_exists, target_relation = get_or_create_relation( database=model.database, @@ -73,6 +75,9 @@ {{ final_sql }} {% endcall %} + {% set should_revoke = should_revoke(target_relation_exists, full_refresh_mode=False) %} + {% do apply_grants(target_relation, grant_config, should_revoke=should_revoke) %} + {% do persist_docs(target_relation, model) %} {% if not target_relation_exists %} diff --git a/core/dbt/parser/manifest.py b/core/dbt/parser/manifest.py index aec1a5dff6e..2e6cef4415a 100644 --- a/core/dbt/parser/manifest.py +++ b/core/dbt/parser/manifest.py @@ -636,7 +636,9 @@ def read_manifest_for_partial_parse(self) -> Optional[Manifest]: if not flags.PARTIAL_PARSE: fire_event(PartialParsingNotEnabled()) return None - path = os.path.join(self.root_project.target_path, PARTIAL_PARSE_FILE_NAME) + path = os.path.join( + self.root_project.project_root, self.root_project.target_path, PARTIAL_PARSE_FILE_NAME + ) reparse_reason = None diff --git a/core/dbt/tests/util.py b/core/dbt/tests/util.py index 7ecf36fca7d..311a2249f7c 100644 --- a/core/dbt/tests/util.py +++ b/core/dbt/tests/util.py @@ -73,7 +73,10 @@ def run_dbt(args: List[str] = None, expect_pass=True): return res -# Use this if you need to capture the command logs in a test +# Use this if you need to capture the command logs in a test. +# If you want the logs that are normally written to a file, you must +# start with the "--debug" flag. The structured schema log CI test +# will turn the logs into json, so you have to be prepared for that. def run_dbt_and_capture(args: List[str] = None, expect_pass=True): try: stringbuf = capture_stdout_logs() @@ -83,6 +86,11 @@ def run_dbt_and_capture(args: List[str] = None, expect_pass=True): finally: stop_capture_stdout_logs() + # Json logs will have lots of escape characters which will + # make checks for strings in the logs fail, so remove those. + if '{"code":' in stdout: + stdout = stdout.replace("\\", "") + return res, stdout diff --git a/plugins/postgres/dbt/include/postgres/macros/adapters.sql b/plugins/postgres/dbt/include/postgres/macros/adapters.sql index b7348472854..11e79078e1d 100644 --- a/plugins/postgres/dbt/include/postgres/macros/adapters.sql +++ b/plugins/postgres/dbt/include/postgres/macros/adapters.sql @@ -202,3 +202,16 @@ comment on column {{ relation }}.{{ adapter.quote(column_name) if column_dict[column_name]['quote'] else column_name }} is {{ escaped_comment }}; {% endfor %} {% endmacro %} + +{%- macro postgres__get_show_grant_sql(relation) -%} + select grantee, privilege_type + from {{ relation.information_schema('role_table_grants') }} + where grantor = current_role + and grantee != current_role + and table_schema = '{{ relation.schema }}' + and table_name = '{{ relation.identifier }}' +{%- endmacro -%} + +{% macro postgres__copy_grants() %} + {{ return(False) }} +{% endmacro %} diff --git a/test/setup_db.sh b/test/setup_db.sh index 1b22a004eb6..de59bf0fac6 100755 --- a/test/setup_db.sh +++ b/test/setup_db.sh @@ -46,6 +46,9 @@ psql -c "GRANT CREATE, CONNECT ON DATABASE dbt TO root WITH GRANT OPTION;" psql -c "CREATE ROLE noaccess WITH PASSWORD 'password' NOSUPERUSER;" psql -c "ALTER ROLE noaccess WITH LOGIN;" psql -c "GRANT CONNECT ON DATABASE dbt TO noaccess;" +psql -c "CREATE ROLE dbt_test_user_1;" +psql -c "CREATE ROLE dbt_test_user_2;" +psql -c "CREATE ROLE dbt_test_user_3;" psql -c 'CREATE DATABASE "dbtMixedCase";' psql -c 'GRANT CREATE, CONNECT ON DATABASE "dbtMixedCase" TO root WITH GRANT OPTION;' diff --git a/test/unit/test_context.py b/test/unit/test_context.py index 7d087d67a33..91cea11b3f6 100644 --- a/test/unit/test_context.py +++ b/test/unit/test_context.py @@ -199,6 +199,7 @@ def assert_has_keys(required_keys: Set[str], maybe_keys: Set[str], ctx: Dict[str "modules", "flags", "print", + "diff_of_two_dicts" } ) diff --git a/tests/adapter/dbt/tests/adapter/grants/base_grants.py b/tests/adapter/dbt/tests/adapter/grants/base_grants.py new file mode 100644 index 00000000000..82f5b9fe664 --- /dev/null +++ b/tests/adapter/dbt/tests/adapter/grants/base_grants.py @@ -0,0 +1,58 @@ +import pytest +import os +from dbt.tests.util import ( + relation_from_name, + get_connection, +) +from dbt.context.base import BaseContext # diff_of_two_dicts only + +TEST_USER_ENV_VARS = ["DBT_TEST_USER_1", "DBT_TEST_USER_2", "DBT_TEST_USER_3"] + + +def replace_all(text, dic): + for i, j in dic.items(): + text = text.replace(i, j) + return text + + +class BaseGrants: + 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()) + + @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 + + def assert_expected_grants_match_actual(self, project, relation_name, expected_grants): + actual_grants = self.get_grants_on_relation(project, relation_name) + # need a case-insensitive comparison + # so just a simple "assert expected == actual_grants" won't work + diff_a = BaseContext.diff_of_two_dicts(actual_grants, expected_grants) + diff_b = BaseContext.diff_of_two_dicts(expected_grants, actual_grants) + assert diff_a == diff_b == {} diff --git a/tests/adapter/dbt/tests/adapter/grants/test_incremental_grants.py b/tests/adapter/dbt/tests/adapter/grants/test_incremental_grants.py new file mode 100644 index 00000000000..2f28eac02ab --- /dev/null +++ b/tests/adapter/dbt/tests/adapter/grants/test_incremental_grants.py @@ -0,0 +1,102 @@ +import pytest +from dbt.tests.util import ( + run_dbt, + run_dbt_and_capture, + get_manifest, + write_file, + relation_from_name, + get_connection, +) +from dbt.tests.adapter.grants.base_grants import BaseGrants + +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') }}"] +""" + + +class BaseIncrementalGrants(BaseGrants): + @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): + # we want the test to fail, not silently skip + test_users = get_test_users + select_privilege_name = self.privilege_grantee_name_overrides()["select"] + assert len(test_users) == 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: [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: [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) + + +class TestIncrementalGrants(BaseIncrementalGrants): + pass diff --git a/tests/adapter/dbt/tests/adapter/grants/test_invalid_grants.py b/tests/adapter/dbt/tests/adapter/grants/test_invalid_grants.py new file mode 100644 index 00000000000..b16cedaac84 --- /dev/null +++ b/tests/adapter/dbt/tests/adapter/grants/test_invalid_grants.py @@ -0,0 +1,68 @@ +import pytest +from dbt.tests.util import ( + run_dbt_and_capture, + write_file, +) +from dbt.tests.adapter.grants.base_grants import BaseGrants + +my_invalid_model_sql = """ + select 1 as fun +""" + +invalid_user_table_model_schema_yml = """ +version: 2 +models: + - name: my_invalid_model + config: + materialized: table + grants: + select: ['invalid_user'] +""" + +invalid_privilege_table_model_schema_yml = """ +version: 2 +models: + - name: my_invalid_model + config: + materialized: table + grants: + fake_privilege: ["{{ env_var('DBT_TEST_USER_2') }}"] +""" + + +class BaseInvalidGrants(BaseGrants): + # The purpose of this test is to understand the user experience when providing + # an invalid 'grants' configuration. dbt will *not* try to intercept or interpret + # the database's own error at runtime -- it will just return those error messages. + # Hopefully they're helpful! + + @pytest.fixture(scope="class") + def models(self): + return { + "my_invalid_model.sql": my_invalid_model_sql, + } + + # Adapters will need to reimplement these methods with the specific + # language of their database + def grantee_does_not_exist_error(self): + return "does not exist" + + def privilege_does_not_exist_error(self): + return "unrecognized privilege" + + def test_invalid_grants(self, project, get_test_users, logs_dir): + # failure when grant to a user/role that doesn't exist + yaml_file = self.interpolate_name_overrides(invalid_user_table_model_schema_yml) + write_file(yaml_file, project.project_root, "models", "schema.yml") + (results, log_output) = run_dbt_and_capture(["--debug", "run"], expect_pass=False) + assert self.grantee_does_not_exist_error() in log_output + + # failure when grant to a privilege that doesn't exist + yaml_file = self.interpolate_name_overrides(invalid_privilege_table_model_schema_yml) + write_file(yaml_file, project.project_root, "models", "schema.yml") + (results, log_output) = run_dbt_and_capture(["--debug", "run"], expect_pass=False) + assert self.privilege_does_not_exist_error() in log_output + + +class TestInvalidGrants(BaseInvalidGrants): + pass diff --git a/tests/adapter/dbt/tests/adapter/grants/test_model_grants.py b/tests/adapter/dbt/tests/adapter/grants/test_model_grants.py new file mode 100644 index 00000000000..db2fe379f5b --- /dev/null +++ b/tests/adapter/dbt/tests/adapter/grants/test_model_grants.py @@ -0,0 +1,156 @@ +import pytest +from dbt.tests.util import ( + run_dbt_and_capture, + get_manifest, + write_file, +) +from dbt.tests.adapter.grants.base_grants import BaseGrants + +my_model_sql = """ + select 1 as fun +""" + +model_schema_yml = """ +version: 2 +models: + - name: my_model + config: + grants: + select: ["{{ env_var('DBT_TEST_USER_1') }}"] +""" + +user2_model_schema_yml = """ +version: 2 +models: + - name: my_model + config: + grants: + select: ["{{ env_var('DBT_TEST_USER_2') }}"] +""" + +table_model_schema_yml = """ +version: 2 +models: + - name: my_model + config: + materialized: table + grants: + select: ["{{ env_var('DBT_TEST_USER_1') }}"] +""" + +user2_table_model_schema_yml = """ +version: 2 +models: + - name: my_model + config: + materialized: table + grants: + select: ["{{ env_var('DBT_TEST_USER_2') }}"] +""" + +multiple_users_table_model_schema_yml = """ +version: 2 +models: + - name: my_model + config: + materialized: table + grants: + select: ["{{ env_var('DBT_TEST_USER_1') }}", "{{ env_var('DBT_TEST_USER_2') }}"] +""" + +multiple_privileges_table_model_schema_yml = """ +version: 2 +models: + - name: my_model + config: + materialized: table + grants: + select: ["{{ env_var('DBT_TEST_USER_1') }}"] + insert: ["{{ env_var('DBT_TEST_USER_2') }}"] +""" + + +class BaseModelGrants(BaseGrants): + @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): + # we want the test to fail, not silently skip + test_users = get_test_users + select_privilege_name = self.privilege_grantee_name_overrides()["select"] + insert_privilege_name = self.privilege_grantee_name_overrides()["insert"] + assert len(test_users) == 3 + + # View materialization, single select grant + (results, log_output) = run_dbt_and_capture(["--debug", "run"]) + assert len(results) == 1 + manifest = get_manifest(project.project_root) + model_id = "model.test.my_model" + model = manifest.nodes[model_id] + expected = {select_privilege_name: [test_users[0]]} + assert model.config.grants == expected + assert model.config.materialized == "view" + self.assert_expected_grants_match_actual(project, "my_model", expected) + + # View materialization, change select grant user + updated_yaml = self.interpolate_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: [get_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: [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: [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: [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] + assert model.config.materialized == "table" + expected = {select_privilege_name: [test_users[0]], insert_privilege_name: [test_users[1]]} + self.assert_expected_grants_match_actual(project, "my_model", expected) + + +class TestModelGrants(BaseModelGrants): + pass diff --git a/tests/adapter/dbt/tests/adapter/grants/test_seed_grants.py b/tests/adapter/dbt/tests/adapter/grants/test_seed_grants.py new file mode 100644 index 00000000000..aff20c65cad --- /dev/null +++ b/tests/adapter/dbt/tests/adapter/grants/test_seed_grants.py @@ -0,0 +1,143 @@ +import pytest +from dbt.tests.util import ( + run_dbt, + run_dbt_and_capture, + get_manifest, + write_file, +) +from dbt.tests.adapter.grants.base_grants import BaseGrants + +seeds__my_seed_csv = """ +id,name,some_date +1,Easton,1981-05-20T06:46:51 +2,Lillian,1978-09-03T18:10:33 +""".lstrip() + +schema_base_yml = """ +version: 2 +seeds: + - name: my_seed + config: + grants: + select: ["{{ env_var('DBT_TEST_USER_1') }}"] +""" + +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: [] +""" + + +class BaseSeedGrants(BaseGrants): + 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): + test_users = get_test_users + 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] + expected = {select_privilege_name: [test_users[0]]} + assert seed.config.grants == expected + assert "grant " in log_output + 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: [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: [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) + + +class TestSeedGrants(BaseSeedGrants): + pass diff --git a/tests/adapter/dbt/tests/adapter/grants/test_snapshot_grants.py b/tests/adapter/dbt/tests/adapter/grants/test_snapshot_grants.py new file mode 100644 index 00000000000..6bf69b3bb94 --- /dev/null +++ b/tests/adapter/dbt/tests/adapter/grants/test_snapshot_grants.py @@ -0,0 +1,78 @@ +import pytest +from dbt.tests.util import ( + run_dbt, + run_dbt_and_capture, + get_manifest, + write_file, +) +from dbt.tests.adapter.grants.base_grants import BaseGrants + +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') }}"] +""" + + +class BaseSnapshotGrants(BaseGrants): + @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): + test_users = get_test_users + 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] + expected = {select_privilege_name: [test_users[0]]} + assert snapshot.config.grants == expected + 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: [test_users[1]]} + self.assert_expected_grants_match_actual(project, "my_snapshot", expected) + + +class TestSnapshotGrants(BaseSnapshotGrants): + pass diff --git a/tests/functional/configs/test_grant_configs.py b/tests/functional/configs/test_grant_configs.py index 6d4df29ed78..64d3f48d4ca 100644 --- a/tests/functional/configs/test_grant_configs.py +++ b/tests/functional/configs/test_grant_configs.py @@ -57,11 +57,10 @@ def project_config_update(self): return dbt_project_yml def test_model_grant_config(self, project, logs_dir): - # This test uses "my_select" instead of "select", so that when - # actual granting of permissions happens, it won't break this - # test. - results = run_dbt(["run"]) - assert len(results) == 1 + # This test uses "my_select" instead of "select", so we need + # use "parse" instead of "run" because we will get compilation + # errors for the grants. + run_dbt(["parse"]) manifest = get_manifest(project.project_root) model_id = "model.test.my_model" @@ -77,7 +76,7 @@ def test_model_grant_config(self, project, logs_dir): # add model grant with clobber write_file(my_model_clobber_sql, project.project_root, "models", "my_model.sql") - results = run_dbt(["run"]) + run_dbt(["parse"]) manifest = get_manifest(project.project_root) model_config = manifest.nodes[model_id].config @@ -86,7 +85,7 @@ def test_model_grant_config(self, project, logs_dir): # change model to extend grants write_file(my_model_extend_sql, project.project_root, "models", "my_model.sql") - results = run_dbt(["run"]) + run_dbt(["parse"]) manifest = get_manifest(project.project_root) model_config = manifest.nodes[model_id].config @@ -95,8 +94,7 @@ def test_model_grant_config(self, project, logs_dir): # add schema file with extend write_file(append_schema_yml, project.project_root, "models", "schema.yml") - results = run_dbt(["run"]) - assert len(results) == 1 + run_dbt(["parse"]) manifest = get_manifest(project.project_root) model_config = manifest.nodes[model_id].config @@ -106,8 +104,7 @@ def test_model_grant_config(self, project, logs_dir): # change model file to have string instead of list write_file(my_model_extend_string_sql, project.project_root, "models", "my_model.sql") - results = run_dbt(["run"]) - assert len(results) == 1 + run_dbt(["parse"]) manifest = get_manifest(project.project_root) model_config = manifest.nodes[model_id].config @@ -117,8 +114,7 @@ def test_model_grant_config(self, project, logs_dir): # change model file to have string instead of list write_file(my_model_extend_twice_sql, project.project_root, "models", "my_model.sql") - results = run_dbt(["run"]) - assert len(results) == 1 + run_dbt(["parse"]) manifest = get_manifest(project.project_root) model_config = manifest.nodes[model_id].config @@ -135,8 +131,7 @@ def test_model_grant_config(self, project, logs_dir): "log-path": logs_dir, } write_config_file(config, project.project_root, "dbt_project.yml") - results = run_dbt(["run"]) - assert len(results) == 1 + run_dbt(["parse"]) manifest = get_manifest(project.project_root) model_config = manifest.nodes[model_id].config @@ -146,8 +141,7 @@ def test_model_grant_config(self, project, logs_dir): # Remove my_model config, leaving only schema file write_file(my_model_base_sql, project.project_root, "models", "my_model.sql") - results = run_dbt(["run"]) - assert len(results) == 1 + run_dbt(["parse"]) manifest = get_manifest(project.project_root) model_config = manifest.nodes[model_id].config