CT-808 grant adapter tests

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

.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

.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

core/dbt/adapters/base/impl.py

@@ -540,9 +540,9 @@ def list_relations_without_caching(self, schema_relation: BaseRelation) -> List[
         )
 
     ###
-    # Abstract methods about grants
+    # Methods about grants
     ###
-    @abc.abstractmethod
+    @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.

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

@@ -26,7 +26,7 @@
 {% endmacro %}
 
 {% macro default__get_show_grant_sql(relation) %}
-    show grants on {{ relation }} {{ relation }}
+    show grants on {{ relation }}
 {% endmacro %}
 
 {% macro get_grant_sql(relation, grant_config) %}
@@ -38,7 +38,7 @@
         {%- set grantees = grant_config[privilege] -%}
         {%- if grantees -%}
             {%- for grantee in grantees -%}
-                grant {{ privilege }} on {{ relation }} {{ relation }} to {{ grantee}};
+                grant {{ privilege }} on {{ relation }} to {{ grantee }};
             {%- endfor -%}
         {%- endif -%}
     {%- endfor -%}
@@ -53,7 +53,7 @@
         {%- set grantees = grant[privilege] -%}
         {%- if grantees -%}
                 {%- for grantee in grantees -%}
-                    revoke {{ privilege }} on {{ relation }} {{ relation }} from {{ grantee }};
+                    revoke {{ privilege }} on {{ relation }} from {{ grantee }};
                 {% endfor -%}
         {%- endif -%}
     {%- endfor -%}

core/dbt/include/global_project/macros/materializations/models/incremental/incremental.sql

@@ -61,7 +61,7 @@
       {% do to_drop.append(backup_relation) %}
   {% endif %}
 
-  {% set should_revoke = should_reovke(existing_relation, full_refresh_mode) %}
+  {% 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) %}

core/dbt/parser/manifest.py

@@ -634,7 +634,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
 

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
 
 

plugins/postgres/dbt/include/postgres/macros/adapters.sql

@@ -212,6 +212,6 @@
         and table_name = '{{ relation.identifier }}'
 {%- endmacro -%}
 
-{% macro postgres__coppy_grants() %}
+{% 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;'

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

@@ -0,0 +1,149 @@
+import pytest
+import os
+from dbt.tests.util import (
+    run_dbt_and_capture,
+    get_manifest,
+    relation_from_name,
+    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') }}"]
+"""
+
+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') }}"]
+"""
+
+
+def format_grant_log_line(relation, user_name):
+    return f"grant select on {relation} to {user_name};"
+
+
+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 = []
+        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 test_model_grants(self, project, get_test_users, logs_dir):
+        # we want the test to fail, not silently skip
+        test_users = get_test_users
+        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": [test_users[0]]}
+        assert model.config.grants == expected
+        assert model.config.materialized == "view"
+
+        my_model_relation = relation_from_name(project.adapter, "my_model")
+        grant_log_line = format_grant_log_line(my_model_relation, test_users[0])
+        assert grant_log_line in log_output
+
+        # 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 == {}
+
+        # View materialization, change select grant user
+        write_file(user2_model_schema_yml, project.project_root, "models", "schema.yml")
+        (results, log_output) = run_dbt_and_capture(["--debug", "run"])
+        assert len(results) == 1
+        grant_log_line = format_grant_log_line(my_model_relation, test_users[1])
+        assert grant_log_line in log_output
+
+        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 == {}
+
+        # Table materialization, single select grant
+        write_file(table_model_schema_yml, project.project_root, "models", "schema.yml")
+        (results, log_output) = run_dbt_and_capture(["--debug", "run"])
+        assert len(results) == 1
+        grant_log_line = format_grant_log_line(my_model_relation, test_users[0])
+        assert grant_log_line in log_output
+        manifest = get_manifest(project.project_root)
+        model = manifest.nodes[model_id]
+        model.config.materialized == "table"
+        actual_grants = self.get_grants_on_relation(project, "my_model")
+        diff = BaseContext.diff_of_two_dicts({"select": [test_users[0]]}, actual_grants)
+        assert diff == {}
+
+        # Table materialization, change select grant user
+        write_file(user2_table_model_schema_yml, project.project_root, "models", "schema.yml")
+        (results, log_output) = run_dbt_and_capture(["--debug", "run"])
+        assert len(results) == 1
+        grant_log_line = format_grant_log_line(my_model_relation, test_users[1])
+        assert grant_log_line in log_output
+        manifest = get_manifest(project.project_root)
+        model = manifest.nodes[model_id]
+        model.config.materialized == "table"
+        actual_grants = self.get_grants_on_relation(project, "my_model")
+        diff = BaseContext.diff_of_two_dicts({"select": [test_users[1]]}, actual_grants)
+        assert diff == {}

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