Test running on Postgres + Snowflake

.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"

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,26 @@ class BaseAdapter(metaclass=AdapterMeta):
             "`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.
+
+        If relevant -- filter down to grants made BY the current user role,
+        and filter OUT any grants TO the current user/role (e.g. OWNERSHIP).
+
+        :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
+        """
+        raise NotImplementedException(
+            "`standardize_grants_dict` is not implemented for this adapter!"
+        )
+
     ###
     # Provided methods about relations
     ###
@@ -1072,6 +1093,11 @@ class BaseAdapter(metaclass=AdapterMeta):
 
         return Compiler(self.config)
 
+    # used in apply_grants -- True is a safe default
+    @available
+    def do_i_carry_over_grants_when_an_object_is_replaced(self) -> bool:
+        return True
+
     # Methods used in adapter tests
     def update_column_sql(
         self,

core/dbt/context/base.py

@@ -657,6 +657,30 @@ class BaseContext(metaclass=ContextMeta):
             print(msg)
         return ""
 
+    @contextmember
+    @staticmethod
+    def diff_of_two_dicts(dict_a, dict_b):
+        """
+        Given two dictionaries:
+            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
+        """
+
+        dict_diff = {}
+        dict_a = {k.lower(): v for k, v in dict_a.items()}
+        dict_b = {k.lower(): v for k, v in dict_b.items()}
+        for k in dict_a:
+            if k in dict_b:
+                a_lowered = map(lambda x: x.lower(), dict_a[k])
+                b_lowered = map(lambda x: x.lower(), dict_b[k])
+                diff = list(set(a_lowered) - set(b_lowered))
+                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)

core/dbt/include/global_project/macros/adapters/apply_grants.sql

@@ -0,0 +1,95 @@
+{% macro are_grants_copied_over_when_replaced() %}
+    {{ return(adapter.dispatch('are_grants_copied_over_when_replaced', 'dbt')()) }}
+{% endmacro %}
+
+{% macro default__are_grants_copied_over_when_replaced() %}
+    {{ return(True) }}
+{% endmacro %}
+
+{% macro do_we_need_to_show_and_revoke_grants(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(are_grants_copied_over_when_replaced()) }}
+    {% else %}
+        {#-- The table is being merged/upserted/inserted -- grants will be carried over --#}
+        {{ return(True) }}
+    {% endif %}
+
+{% endmacro %}
+
+{% 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, grant_config) %}
+    {{ return(adapter.dispatch('get_grant_sql', 'dbt')(relation, grant_config)) }}
+{% endmacro %}
+
+{%- macro default__get_grant_sql(relation, grant_config) -%}
+    {%- for privilege in grant_config.keys() -%}
+        {%- set grantees = grant_config[privilege] -%}
+        {%- if grantees -%}
+            {%- for grantee in grantees -%}
+                grant {{ privilege }} on {{ relation }} to {{ grantee }};
+            {%- endfor -%}
+        {%- endif -%}
+    {%- endfor -%}
+{%- endmacro %}
+
+{% macro get_revoke_sql(relation, grant_config) %}
+    {{ return(adapter.dispatch("get_revoke_sql", "dbt")(relation, grant_config)) }}
+{% endmacro %}
+
+{% macro default__get_revoke_sql(relation, grant_config) %}
+    {%- for privilege in grant_config.keys() -%}
+        {%- set grantees = [] -%}
+        {%- set all_grantees = grant_config[privilege] -%}
+        {%- for grantee in all_grantees -%}
+            {%- if grantee != target.user -%}
+                {% do grantees.append(grantee) %}
+            {%- endif -%}
+        {% endfor -%}
+        {%- if grantees -%}
+                {%- for grantee in grantees -%}
+                    revoke {{ privilege }} on {{ relation }} from {{ grantee }};
+                {% endfor -%}
+        {%- endif -%}
+    {%- endfor -%}
+{%- endmacro -%}
+
+{% macro apply_grants(relation, grant_config, should_revoke) %}
+    {% if grant_config %}
+        {{ return(adapter.dispatch("apply_grants", "dbt")(relation, grant_config, should_revoke)) }}
+    {% endif %}
+{% endmacro %}
+
+{% macro default__apply_grants(relation, grant_config, should_revoke=True) %}
+    {% if grant_config %}
+            {% if should_revoke %}
+                {% set current_grants_table = run_query(get_show_grant_sql(relation)) %}
+                {% set current_grants_dict = adapter.standardize_grants_dict(current_grants_table) %}
+                {% set needs_granting = diff_of_two_dicts(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('All grants are in place, no revocation or granting needed.')}}
+                {% endif %}
+            {% else %}
+                {% set needs_revoking = {} %}
+                {% set needs_granting = grant_config %}
+            {% endif %}
+            {% if needs_granting or needs_revoking %}
+                {% call statement('grants') %}
+                    {{ get_revoke_sql(relation, needs_revoking) }}
+                    {{ get_grant_sql(relation, needs_granting) }}
+                {% endcall %}
+            {% endif %}
+    {% endif %}
+{% endmacro %}

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 = do_we_need_to_show_and_revoke_grants(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() %}

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 = do_we_need_to_show_and_revoke_grants(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

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,7 @@
     {{ get_create_view_as_sql(target_relation, sql) }}
   {%- endcall %}
 
+  {% do apply_grants(target_relation, grant_config, should_revoke=True) %}
   {{ run_hooks(post_hooks) }}
 
   {{ return({'relations': [target_relation]}) }}

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 = do_we_need_to_show_and_revoke_grants(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) }}

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 = do_we_need_to_show_and_revoke_grants(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 %}

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 = do_we_need_to_show_and_revoke_grants(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 %}

core/dbt/parser/manifest.py

@@ -634,7 +634,9 @@ class ManifestLoader:
         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
 

plugins/postgres/dbt/adapters/postgres/impl.py

@@ -10,6 +10,7 @@ from dbt.adapters.postgres import PostgresRelation
 from dbt.dataclass_schema import dbtClassMixin, ValidationError
 import dbt.exceptions
 import dbt.utils
+import agate
 
 
 # note that this isn't an adapter macro, so just a single underscore
@@ -85,6 +86,18 @@ class PostgresAdapter(SQLAdapter):
     def parse_index(self, raw_index: Any) -> Optional[PostgresIndexConfig]:
         return PostgresIndexConfig.parse(raw_index)
 
+    @available
+    def standardize_grants_dict(self, grants_table: agate.Table) -> dict:
+        grants_dict = {}
+        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
+
     def _link_cached_database_relations(self, schemas: Set[str]):
         """
         :param schemas: The set of schemas that should have links added.

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 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__are_grants_copied_over_when_replaced() %}
+    {{ return(False) }}
+{% endmacro %}

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;'

test/unit/test_context.py

@@ -199,6 +199,7 @@ REQUIRED_BASE_KEYS = frozenset(
         "modules",
         "flags",
         "print",
+        "diff_of_two_dicts"
     }
 )
 

tests/adapter/dbt/tests/adapter/grants/test_models.py

@@ -0,0 +1,108 @@
+import pytest
+import os
+from dbt.tests.util import (
+    run_dbt,
+    get_manifest,
+    read_file,
+    relation_from_name,
+    rm_file,
+    write_file,
+    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"]
+
+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') }}"]
+"""
+
+
+class TestModelGrants:
+    @pytest.fixture(scope="class")
+    def models(self):
+        return {"my_model.sql": my_model_sql, "schema.yml": model_schema_yml}
+
+    @pytest.fixture(scope="class", autouse=True)
+    def get_test_users(self, project):
+        test_users = []
+        missing = []
+        for env_var in TEST_USER_ENV_VARS:
+            user_name = os.getenv(env_var)
+            if not user_name:
+                missing.append(env_var)
+            else:
+                test_users.append(user_name)
+        if missing:
+            pytest.skip(f"Test requires env vars with test users. Missing {', '.join(missing)}.")
+        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 test_basic(self, project, get_test_users, logs_dir):
+        # Tests a project with a single model, view materialization
+        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]
+        expected = {"select": [get_test_users[0]]}
+        assert model.config.grants == expected
+        assert model.config.materialized == "view"
+
+        # validate grant statements in logs
+        log_contents = read_file(logs_dir, "dbt.log")
+        my_model_relation = relation_from_name(project.adapter, "my_model")
+        grant_log_line = f"grant select on {my_model_relation} to {get_test_users[0]};"
+        assert grant_log_line in log_contents
+
+        # validate actual grants in database
+        actual_grants = self.get_grants_on_relation(project, "my_model")
+        # actual_grants: {'SELECT': ['dbt_test_user_1']}
+        # need a case-insensitive comparison
+        # so just a simple "assert expected == actual_grants" won't work
+        diff = BaseContext.diff_of_two_dicts(expected, actual_grants)
+        assert diff == {}
+
+        # Switch to a different user, still view materialization
+        rm_file(logs_dir, "dbt.log")
+        write_file(user2_model_schema_yml, project.project_root, "models", "schema.yml")
+        results = run_dbt(["run"])
+        assert len(results) == 1
+        log_contents = read_file(logs_dir, "dbt.log")
+        print(log_contents)
+        grant_log_line = f"grant select on {my_model_relation} to {get_test_users[1]};"
+        assert grant_log_line in log_contents
+        # Note: We are not revoking grants here, so there is no revoke in the log
+
+        expected = {"select": [get_test_users[1]]}
+        actual_grants = self.get_grants_on_relation(project, "my_model")
+        diff = BaseContext.diff_of_two_dicts(expected, actual_grants)
+        assert diff == {}

tests/functional/configs/test_grant_configs.py

@@ -57,11 +57,10 @@ class TestGrantConfigs:
         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 @@ class TestGrantConfigs:
 
         # 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 @@ class TestGrantConfigs:
 
         # 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 @@ class TestGrantConfigs:
 
         # 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 @@ class TestGrantConfigs:
 
         # 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 @@ class TestGrantConfigs:
 
         # 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 @@ class TestGrantConfigs:
             "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 @@ class TestGrantConfigs:
 
         # 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