Test

.changes/unreleased/Features-20231004-103938.yaml

@@ -0,0 +1,6 @@
+kind: Features
+body: 'Support groups and roles for grants '
+time: 2023-10-04T10:39:38.680813-07:00
+custom:
+  Author: soksamnanglim
+  Issue: "415"

.github/workflows/integration.yml

@@ -185,6 +185,12 @@ jobs:
           DBT_TEST_USER_1: dbt_test_user_1
           DBT_TEST_USER_2: dbt_test_user_2
           DBT_TEST_USER_3: dbt_test_user_3
+          DBT_TEST_GROUP_1: dbt_test_group_1
+          DBT_TEST_GROUP_2: dbt_test_group_2
+          DBT_TEST_GROUP_3: dbt_test_group_3
+          DBT_TEST_ROLE_1: dbt_test_role_1
+          DBT_TEST_ROLE_2: dbt_test_role_2
+          DBT_TEST_ROLE_3: dbt_test_role_3
         run: tox -- --ddtrace
 
       - uses: actions/upload-artifact@v3

Makefile

@@ -1,5 +1,19 @@
 .DEFAULT_GOAL:=help
 
+CI_FLAGS =\
+	DBT_TEST_USER_1=$(if $(DBT_TEST_USER_1),$(DBT_TEST_USER_1),dbt_test_user_1)\
+	DBT_TEST_USER_2=$(if $(DBT_TEST_USER_2),$(DBT_TEST_USER_2),dbt_test_user_2)\
+	DBT_TEST_USER_3=$(if $(DBT_TEST_USER_3),$(DBT_TEST_USER_3),dbt_test_user_3)\
+	DBT_TEST_GROUP_1=$(if $(DBT_TEST_GROUP_1),$(DBT_TEST_GROUP_1),dbt_test_group_1)\
+	DBT_TEST_GROUP_2=$(if $(DBT_TEST_GROUP_2),$(DBT_TEST_GROUP_2),dbt_test_group_2)\
+	DBT_TEST_GROUP_3=$(if $(DBT_TEST_GROUP_3),$(DBT_TEST_GROUP_3),dbt_test_group_3)\
+	DBT_TEST_ROLE_1=$(if $(DBT_TEST_ROLE_1),$(DBT_TEST_ROLE_1),dbt_test_role_1)\
+	DBT_TEST_ROLE_2=$(if $(DBT_TEST_ROLE_2),$(DBT_TEST_ROLE_2),dbt_test_role_2)\
+	DBT_TEST_ROLE_3=$(if $(DBT_TEST_ROLE_3),$(DBT_TEST_ROLE_3),dbt_test_role_3)\
+	RUSTFLAGS=$(if $(RUSTFLAGS),$(RUSTFLAGS),"-D warnings")\
+	LOG_DIR=$(if $(LOG_DIR),$(LOG_DIR),./logs)\
+	DBT_LOG_FORMAT=$(if $(DBT_LOG_FORMAT),$(DBT_LOG_FORMAT),json)
+
 .PHONY: dev
 dev: ## Installs adapter in develop mode along with development dependencies
 	@\

dbt/adapters/redshift/impl.py

@@ -9,15 +9,12 @@
 from dbt.contracts.graph.nodes import ConstraintType
 from dbt.events import AdapterLogger
 
-
 import dbt.exceptions
 
 from dbt.adapters.redshift import RedshiftConnectionManager, RedshiftRelation
 
-
 logger = AdapterLogger("Redshift")
 
-
 GET_RELATIONS_MACRO_NAME = "redshift__get_relations"
 
 
@@ -168,3 +165,93 @@ def generate_python_submission_response(self, submission_result: Any) -> Adapter
     def debug_query(self):
         """Override for DebugTask method"""
         self.execute("select 1 as id")
+
+    # grant-related methods
+    @available
+    def standardize_grants_dict(self, grants_table):
+        """
+        Override for standardize_grants_dict
+        """
+        grants_dict = {}  # Dict[str, Dict[str, List[str]]]
+        for row in grants_table:
+            grantee_type = row["grantee_type"]
+            grantee = row["grantee"]
+            privilege = row["privilege_type"]
+            if privilege not in grants_dict:
+                grants_dict[privilege] = {}
+
+            if grantee_type not in grants_dict[privilege]:
+                grants_dict[privilege][grantee_type] = []
+
+            grants_dict[privilege][grantee_type].append(grantee)
+
+        return grants_dict
+
+    @available
+    def diff_of_two_nested_dicts(self, dict_a, dict_b):
+        """
+        Given two dictionaries of type Dict[str, Dict[str, List[str]]]:
+            dict_a = {'key_x': {'key_a': 'VALUE_1'}, 'KEY_Y': {'key_b': value_2'}}
+            dict_b = {'key_x': {'key_a': 'VALUE_1'}, 'KEY_Y': {'key_b': value_2'}}
+        Return the same dictionary representation of dict_a MINUS dict_b,
+        performing a case-insensitive comparison between the strings in each.
+        All keys returned will be in the original case of dict_a.
+            returns {'key_x': ['VALUE_2'], 'KEY_Y': ['value_3']}
+        """
+
+        dict_diff = {}
+
+        for k, v_a in dict_a.items():
+            if k.casefold() in dict_b:
+                v_b = dict_b[k.casefold()]
+
+                for sub_key, values_a in v_a.items():
+                    if sub_key in v_b:
+                        values_b = v_b[sub_key]
+                        diff_values = [v for v in values_a if v.casefold() not in values_b]
+                        if diff_values:
+                            if k in dict_diff:
+                                dict_diff[k][sub_key] = diff_values
+                            else:
+                                dict_diff[k] = {sub_key: diff_values}
+                    else:
+                        if k in dict_diff:
+                            if values_a:
+                                dict_diff[k][sub_key] = values_a
+                        else:
+                            if values_a:
+                                dict_diff[k] = {sub_key: values_a}
+            else:
+                dict_diff[k] = v_a
+
+        return dict_diff
+
+    @available
+    def process_grant_dicts(self, unknown_dict):
+        """
+        Given a dictionary where the type can either be of type:
+        - Dict[str, List[str]]
+        - Dict[str, List[Dict[str, List[str]]
+        Return a processed dictionary of the type Dict[str, Dict[str, List[str]]
+        """
+        first_value = next(iter(unknown_dict.values()))
+        if first_value:
+            is_dict = isinstance(first_value[0], dict)
+        else:
+            is_dict = False
+
+        temp = {}
+        if not is_dict:
+            for privilege, grantees in unknown_dict.items():
+                if grantees:
+                    temp[privilege] = {"user": grantees}
+        else:
+            for privilege, grantee_map in unknown_dict.items():
+                grantees_map_temp = {}
+                for grantee_type, grantees in grantee_map[0].items():
+                    if grantees:
+                        grantees_map_temp[grantee_type] = grantees
+                if grantees_map_temp:
+                    temp[privilege] = grantees_map_temp
+
+        return temp

dbt/include/redshift/macros/adapters/apply_grants.sql

@@ -1,5 +1,42 @@
-{% macro redshift__get_show_grant_sql(relation) %}
+{# ------- DCL STATEMENT TEMPLATES ------- #}
+
+{%- macro redshift__get_grant_sql(relation, privilege, grantee_dict) -%}
+    {#-- generates a multiple-grantees grant privilege statement --#}
+    grant {{privilege}} on {{relation}} to
+    {%- for grantee_type, grantees in grantee_dict.items() -%}
+    {%- if grantee_type=='user' and grantees -%}
+        {{ " " + (grantees | join(', ')) }}
+    {%- elif grantee_type=='group' and grantees -%}
+        {{ " " +("group " + grantees | join(', group ')) }}
+    {%- elif grantee_type=='role' and grantees -%}
+        {{ " " + ("role " + grantees | join(', role ')) }}
+    {%- endif -%}
+    {%- if not loop.last -%}
+        ,
+    {%- endif -%}
+    {%- endfor -%}
+{%- endmacro -%}
+
+{%- macro redshift__get_revoke_sql(relation, privilege, revokee_dict) -%}
+    {#-- generates a multiple-grantees revoke privilege statement --#}
+    revoke {{privilege}} on {{relation}} from
+    {%- for revokee_type, revokees in revokee_dict.items() -%}
+    {%- if revokee_type=='user' and revokees -%}
+        {{ " " + (revokees | join(', ')) }}
+    {%- elif revokee_type=='group' and revokees -%}
+        {{ " " +("group " + revokees | join(', group ')) }}
+    {%- elif revokee_type=='role' and revokees -%}
+        {{ " " + ("role " + revokees | join(', role ')) }}
+    {%- endif -%}
+    {%- if not loop.last -%}
+        ,
+    {%- endif -%}
+    {%- endfor -%}
+{%- endmacro -%}
 
+
+{% macro redshift__get_show_grant_sql(relation) %}
+{#-- shows the privilege grants on a table for users, groups, and roles --#}
 with privileges as (
 
     -- valid options per https://docs.aws.amazon.com/redshift/latest/dg/r_HAS_TABLE_PRIVILEGE.html
@@ -16,6 +53,7 @@ with privileges as (
 )
 
 select
+    'user' as grantee_type,
     u.usename as grantee,
     p.privilege_type
 from pg_user u
@@ -24,4 +62,70 @@ where has_table_privilege(u.usename, '{{ relation }}', privilege_type)
     and u.usename != current_user
     and not u.usesuper
 
+union all
+-- check that group has table privilege
+select
+    'group' as grantee_type,
+    g.groname as grantee,
+    p.privilege_type
+from pg_group g
+cross join privileges p
+where exists(
+    select *
+    from information_schema.table_privileges tp
+    where tp.grantee=g.groname
+    and tp.table_name=replace(split_part('{{ relation }}', '.', 3), '"', '')
+    and LOWER(tp.privilege_type)=p.privilege_type
+)
+
+union all
+-- check that role has table privilege
+select
+    'role' as grantee_type,
+    r.role_name as rolename,
+    p.privilege_type
+from svv_roles r
+cross join privileges p
+where exists(
+    select *
+    from svv_relation_privileges rp
+    where rp.identity_name=r.role_name
+    and rp.relation_name=replace(split_part('{{ relation }}', '.', 3), '"', '')
+    and LOWER(rp.privilege_type)=p.privilege_type
+)
+
+{% endmacro %}
+
+{% macro redshift__apply_grants(relation, grant_config, should_revoke=True) %}
+    {#-- Override for apply grants --#}
+    {#-- If grant_config is {} or None, this is a no-op --#}
+    {% if grant_config %}
+        {#-- change grant_config to Dict[str, Dict[str, List[str]] format --#}
+        {% set grant_config = adapter.process_grant_dicts(grant_config) %}
+
+        {% if should_revoke %}
+            {#-- We think that there is a chance that grants are carried over. --#}
+            {#-- Show the current grants for users, roles, and groups and calculate the diffs. --#}
+            {% set current_grants_table = run_query(get_show_grant_sql(relation)) %}
+            {% set current_grants_dict = adapter.standardize_grants_dict(current_grants_table) %}
+            {% set needs_granting = adapter.diff_of_two_nested_dicts(grant_config, current_grants_dict) %}
+            {% set needs_revoking = adapter.diff_of_two_nested_dicts(current_grants_dict, grant_config) %}
+            {% if not (needs_granting or needs_revoking) %}
+                {{ log('On ' ~ relation ~': All grants are in place, no revocation or granting needed.')}}
+            {% endif %}
+        {% else %}
+            {#-- We don't think there's any chance of previous grants having carried over. --#}
+            {#-- Jump straight to granting what the user has configured. --#}
+            {% set needs_revoking = {} %}
+            {% set needs_granting = grant_config %}
+        {% endif %}
+        {% if needs_granting or needs_revoking %}
+            {% set revoke_statement_list = get_dcl_statement_list(relation, needs_revoking, get_revoke_sql) %}
+            {% set grant_statement_list = get_dcl_statement_list(relation, needs_granting, get_grant_sql) %}
+            {% set dcl_statement_list = revoke_statement_list + grant_statement_list %}
+            {% if dcl_statement_list %}
+                {{ call_dcl_statements(dcl_statement_list) }}
+            {% endif %}
+        {% endif %}
+    {% endif %}
 {% endmacro %}

scripts/env-setup.sh

@@ -8,3 +8,9 @@ echo "INTEGRATION_TESTS_SECRETS_PREFIX=REDSHIFT_TEST" >> $GITHUB_ENV
 echo "DBT_TEST_USER_1=dbt_test_user_1" >> $GITHUB_ENV
 echo "DBT_TEST_USER_2=dbt_test_user_2" >> $GITHUB_ENV
 echo "DBT_TEST_USER_3=dbt_test_user_3" >> $GITHUB_ENV
+echo "DBT_TEST_GROUP_1=dbt_test_group_1" >> $GITHUB_ENV
+echo "DBT_TEST_GROUP_2=dbt_test_group_2" >> $GITHUB_ENV
+echo "DBT_TEST_GROUP_3=dbt_test_group_3" >> $GITHUB_ENV
+echo "DBT_TEST_ROLE_1=dbt_test_role_1" >> $GITHUB_ENV
+echo "DBT_TEST_ROLE_2=dbt_test_role_2" >> $GITHUB_ENV
+echo "DBT_TEST_ROLE_3=dbt_test_role_3" >> $GITHUB_ENV

test.env.example

@@ -16,3 +16,9 @@ REDSHIFT_TEST_DBNAME=
 DBT_TEST_USER_1=dbt_test_user_1
 DBT_TEST_USER_2=dbt_test_user_2
 DBT_TEST_USER_3=dbt_test_user_3
+DBT_TEST_GROUP_1=dbt_test_group_1
+DBT_TEST_GROUP_2=dbt_test_group_2
+DBT_TEST_GROUP_3=dbt_test_group_3
+DBT_TEST_ROLE_1=dbt_test_role_1
+DBT_TEST_ROLE_2=dbt_test_role_2
+DBT_TEST_ROLE_3=dbt_test_role_3

tests/functional/adapter/conftest.py

@@ -1,4 +1,36 @@
 import pytest
+import os
+
+from dbt.exceptions import DbtDatabaseError
+
+# This is a hack to prevent the fixture from running more than once
+GRANTS_AND_ROLES_SETUP = False
+
+
+@pytest.fixture(scope="class", autouse=True)
+def setup_grants_and_roles(project):
+    global GRANTS_AND_ROLES_SETUP
+    groups = [
+        os.environ[env_var] for env_var in os.environ if env_var.startswith("DBT_TEST_GROUP_")
+    ]
+    roles = [os.environ[env_var] for env_var in os.environ if env_var.startswith("DBT_TEST_ROLE_")]
+    if not GRANTS_AND_ROLES_SETUP:
+        with project.adapter.connection_named("__test"):
+            for group in groups:
+                try:
+                    project.adapter.execute(f"CREATE GROUP {group}")
+                except DbtDatabaseError:
+                    # This is expected if the group already exists
+                    pass
+
+            for role in roles:
+                try:
+                    project.adapter.execute(f"CREATE ROLE {role}")
+                except DbtDatabaseError:
+                    # This is expected if the group already exists
+                    pass
+
+            GRANTS_AND_ROLES_SETUP = True
 
 
 @pytest.fixture