Add Grant SQL Macros (#5369)

* init push or ct-660 work

* changes to default versions of get_show_grant_sql and get_grant_sql

* completing init default versions of all macros being called for look over and collaboration

* minor update to should_revoke

* post pairing push up (does have log statements to make sure we remove)

* minor spacing changes

* minor changes, and removal of logs so people can have clean grab of code

* minor changes to how get_revoke_sql works

* init attempt at applying apply_grants to all materialzations

* name change from recipents -> grantee

* minor changes

* working on making a context to handle the diff gathering between grant_config and curreent_grants to see what needs to be revoked, I know if we assign a role, and a model becomes dependent on it we can't drop the role now still not seeing the diff appear in log

* removing logs from most materializations to better track diff of grants generation logs

* starting to build out postgres get_show_grant_sql getting empty query errors hopefully will clear up as we add the other postgres versions of macros and isn't a psycopg2 issue as indicated by searching

* 6/27 eod update looking into diff_grants variable not getting passed into get_revoke_sql

* changes to loop cases

* changes after pairing meeting

* adding apply_grants to create_or_replace_view.sql

* models are building but testing out small issues around revoke statement never building

* postgrest must fixes from jeremy's feedback

* postgres minor change to standarize_grants_dict

* updating after pairing with dough and jeremey incorporating the new version of should revoke logic.

* adding  ref of diff_of_two_dicts to base keys ref

* change of method type for standardize_grants_dict

* minor update trying to fix unit test

* changes based on morning feedback

* change log message in default_apply_grants macro

* CT-808 grant adapter tests (#5447)

* Create initial test for grants

Co-authored-by: Jeremy Cohen <[email protected]>

* rename grant[privilege] -> grant_config[privilege]

* postgres macro rename to copy_grants

* CT-808 more grant adapter tests (#5452)

* Add tests for invalid user and invalid privilege

* Add more grant tests

* Macro touch ups

* Many more tests

* Allow easily replacing privilege names

* Keep adding tests

* Refactor macros to return lists, fix test

* Code checks

* Keep tweaking tests

* Revert cool grantees join bc Snowflake isnt happy

* Use Postgres/BQ as standard for standardize_grants_dict

* Code checks

* add missing replace

* small replace tweak,  add additional dict diffs

* All tests passing on BQ

* Add type cast to test_snapshot_grants

* Refactor for DRYer apply_grants macros

Co-authored-by: Jeremy Cohen <[email protected]>
Co-authored-by: Emily Rockman <[email protected]>

* update to main, create changelog, whitespace fixes

Co-authored-by: Gerda Shank <[email protected]>
Co-authored-by: Jeremy Cohen <[email protected]>
Co-authored-by: Emily Rockman <[email protected]>

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

.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

@@ -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
     ###

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)

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 %}

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

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

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]}) }}

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) }}

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 %}