Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support Privilege Grants to Roles and Groups in Materialization #626

Open
wants to merge 51 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
51 commits
Select commit Hold shift + click to select a range
a1ec09c
Support Privilege Grants to Roles and Groups in Materialization
soksamnanglim Oct 4, 2023
a0ca37d
Support Privilege Grants to Roles and Groups in Materialization
soksamnanglim Oct 4, 2023
ca046c9
rectified code with pre-commit
soksamnanglim Oct 4, 2023
2bb40b8
Update CI workflow to include new env vars
soksamnanglim Oct 4, 2023
f464d44
Add fixture to create grants/roles for test purposes
colin-rogers-dbt Oct 9, 2023
7b89a16
Merge branch 'main' into extend_grant_privilege
colin-rogers-dbt Oct 11, 2023
72a2410
Fixed `ImportError` related to misnaming BaseModelGrantsRedshift
soksamnanglim Oct 12, 2023
0f6442e
Remove debugging comment
soksamnanglim Oct 12, 2023
bfc08f6
Change away from inheritance of BaseGrants
soksamnanglim Oct 12, 2023
53cc135
Merge branch 'main' into extend_grant_privilege
colin-rogers-dbt Oct 13, 2023
b546431
clean up duplicate tests
colin-rogers-dbt Oct 13, 2023
87c491d
Merge branch 'dbt-labs:main' into extend_grant_privilege
soksamnanglim Oct 24, 2023
574193c
Merge branch 'main' into extend_grant_privilege
soksamnanglim Oct 24, 2023
0aa8fb1
Split model grants tests into view and table tests
soksamnanglim Oct 25, 2023
c0235ba
Renamed model view and model table test suites
soksamnanglim Oct 25, 2023
bdc6177
added groups and roles to env-setup
soksamnanglim Oct 25, 2023
ead3a2e
fix pre-commit EOF space
soksamnanglim Oct 25, 2023
0d51ab7
Merge branch 'main' into extend_grant_privilege
soksamnanglim Oct 26, 2023
f691812
adding CI_flags override to makefile
soksamnanglim Oct 27, 2023
de7da53
Merge branch 'dbt-labs:main' into extendGrantPrivilege_test
soksamnanglim Oct 27, 2023
3cee6e2
set env vars in conftest.py
colin-rogers-dbt Oct 31, 2023
2d65c09
merge main
colin-rogers-dbt Oct 31, 2023
8ec3461
Merge remote-tracking branch 'soksamnanglim/extendGrantPrivilege_test…
colin-rogers-dbt Oct 31, 2023
016e62c
Merge branch 'main' into extend_grant_privilege
soksamnanglim Oct 31, 2023
fad2ef8
delete breakpoint and add comment to CONTRIBUTING.md
soksamnanglim Oct 31, 2023
343565f
Merge branch 'extendGrantPrivilege' into HEAD
colin-rogers-dbt Oct 31, 2023
aeadcac
fix get_show_grant_sql macro
soksamnanglim Nov 1, 2023
1da43db
Merge branch 'dbt-labs:main' into extend_grant_privilege
soksamnanglim Nov 1, 2023
cee0637
Refactor assert_expected_grants_match_actual for debugging
soksamnanglim Nov 1, 2023
e3bb550
Merge branch 'main' into extend_grant_privilege
colin-rogers-dbt Nov 3, 2023
9fd05c1
change rolename alias to grantee in get_show_grant_sql
colin-rogers-dbt Nov 3, 2023
767749b
Merge branch 'main' into extend_grant_privilege
colin-rogers-dbt Nov 8, 2023
a738720
Merge branch 'main' into extend_grant_privilege
colin-rogers-dbt Nov 9, 2023
082c590
Merge branch 'main' into extend_grant_privilege
soksamnanglim Nov 30, 2023
66782c4
Merge branch 'main' into extend_grant_privilege
soksamnanglim Dec 5, 2023
96fc734
Merge branch 'main' into extend_grant_privilege
colin-rogers-dbt May 6, 2024
4a28c99
Merge branch 'main' into extend_grant_privilege
dbeatty10 Jul 9, 2024
1f2f9e0
Update package due to decoupling of dbt-adapters from dbt-core
dbeatty10 Jul 9, 2024
53956b4
Update body of changelog entry
dbeatty10 Jul 10, 2024
5901cf2
Update whitespace
dbeatty10 Jul 10, 2024
f37d91d
Try printing the environment variables configured in the CI environment
dbeatty10 Jul 10, 2024
220c810
Examine roles in the Redshift database
dbeatty10 Jul 10, 2024
c56c61d
Remove debugging logic
dbeatty10 Jul 10, 2024
00231be
Debug output for setting up groups and roles for CI
dbeatty10 Jul 10, 2024
30043a7
Force creation of groups and roles
dbeatty10 Jul 11, 2024
12d339d
Fix typo
dbeatty10 Jul 11, 2024
3aeff0d
Debugging database roles
dbeatty10 Jul 11, 2024
c8e06f5
Re-run CI tests
dbeatty10 Jul 11, 2024
eb07cfb
Remove debugging
dbeatty10 Jul 11, 2024
655cb53
Add groups and roles for various release workflows in GitHub Actions
dbeatty10 Jul 11, 2024
5247bab
Database groups and roles for testing locally
dbeatty10 Jul 11, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions .changes/unreleased/Features-20231004-103938.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
kind: Features
body: 'Support groups and roles for grants '
dbeatty10 marked this conversation as resolved.
Show resolved Hide resolved
time: 2023-10-04T10:39:38.680813-07:00
custom:
Author: soksamnanglim
Issue: "415"
93 changes: 90 additions & 3 deletions dbt/adapters/redshift/impl.py
Original file line number Diff line number Diff line change
Expand Up @@ -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"


Expand Down Expand Up @@ -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):
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

"""
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):
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Instead of diff_of_two_dicts.

"""
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
106 changes: 105 additions & 1 deletion dbt/include/redshift/macros/adapters/apply_grants.sql
Original file line number Diff line number Diff line change
@@ -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 --#}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 --#}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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
Expand All @@ -16,6 +53,7 @@ with privileges as (
)

select
'user' as grantee_type,
u.usename as grantee,
p.privilege_type
from pg_user u
Expand All @@ -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
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

From pairing with @dbeatty10 - in order to query svv_table_info, this requires:

  • that dbt's user has role <somerole>
  • grant access system table to role <somerole>;

References:

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 --#}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Overriding default__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 %}
6 changes: 6 additions & 0 deletions test.env.example
Original file line number Diff line number Diff line change
Expand Up @@ -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
47 changes: 47 additions & 0 deletions tests/functional/adapter/grants/base_grants.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,47 @@
from dbt.tests.adapter.grants.base_grants import BaseGrants
import pytest
import os
from dbt.tests.util import (
relation_from_name,
get_connection,
)

TEST_USER_ENV_VARS = ["DBT_TEST_USER_1", "DBT_TEST_USER_2", "DBT_TEST_USER_3"]
TEST_GROUP_ENV_VARS = ["DBT_TEST_GROUP_1", "DBT_TEST_GROUP_2", "DBT_TEST_GROUP_3"]
TEST_ROLE_ENV_VARS = ["DBT_TEST_ROLE_1", "DBT_TEST_ROLE_2", "DBT_TEST_ROLE_3"]


def replace_all(text, dic):
for i, j in dic.items():
text = text.replace(i, j)
return text


def get_test_permissions(permission_env_vars):
test_permissions = []
for env_var in permission_env_vars:
permission_name = os.getenv(env_var)
if permission_name:
test_permissions.append(permission_name)
return test_permissions


class BaseGrantsRedshift(BaseGrants):

@pytest.fixture(scope="class", autouse=True)
def get_test_groups(self, project):
return get_test_permissions(TEST_GROUP_ENV_VARS)

@pytest.fixture(scope="class", autouse=True)
def get_test_roles(self, project):
return get_test_permissions(TEST_ROLE_ENV_VARS)

# This is an override of the BaseGrants class
def assert_expected_grants_match_actual(self, project, relation_name, expected_grants):
actual_grants = self.get_grants_on_relation(project, relation_name)
adapter = project.adapter
# need a case-insensitive comparison
# so just a simple "assert expected == actual_grants" won't work
diff_a = adapter.diff_of_two_nested_dicts(actual_grants, expected_grants)
diff_b = adapter.diff_of_two_nested_dicts(expected_grants, actual_grants)
assert diff_a == diff_b == {}
Loading
Loading