Skip to content

Commit

Permalink
Support grants config
Browse files Browse the repository at this point in the history
  • Loading branch information
mdesmet committed Oct 13, 2022
1 parent d190a41 commit 9471340
Show file tree
Hide file tree
Showing 9 changed files with 154 additions and 31 deletions.
7 changes: 7 additions & 0 deletions .changes/unreleased/Features-20221005-134613.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
kind: Features
body: Support security grants
time: 2022-10-05T13:46:13.789545+02:00
custom:
Author: mdesmet
Issue: ""
PR: "130"
6 changes: 6 additions & 0 deletions Makefile
Original file line number Diff line number Diff line change
@@ -1,3 +1,9 @@
.EXPORT_ALL_VARIABLES:

DBT_TEST_USER_1=user1
DBT_TEST_USER_2=user2
DBT_TEST_USER_3=user3

start-trino:
docker network create dbt-net || true
./docker/dbt/build.sh
Expand Down
73 changes: 45 additions & 28 deletions README.md

Large diffs are not rendered by default.

45 changes: 45 additions & 0 deletions dbt/include/trino/macros/apply_grants.sql
Original file line number Diff line number Diff line change
@@ -0,0 +1,45 @@
{% macro trino__get_show_grant_sql(relation) -%}
select
grantee,
lower(privilege_type) as privilege_type
from information_schema.table_privileges
where table_catalog = '{{ relation.database }}'
and table_schema = '{{ relation.schema }}'
and table_name = '{{ relation.identifier }}'
{%- endmacro %}

{% macro trino__copy_grants() %}
{#
-- This macro should return true or false depending on the answer to
-- following question:
-- when an object is fully replaced on your database, do grants copy over?
-- e.g. on Postgres this is never true,
-- on Spark this is different for views vs. non-Delta tables vs. Delta tables,
-- on Snowflake it depends on the user-supplied copy_grants configuration.
-- true by default, which means “play it safe”: grants MIGHT have copied over,
-- so dbt will run an extra query to check them + calculate diffs.
#}
{{ return(False) }}
{% endmacro %}

{%- macro trino__get_grant_sql(relation, privilege, grantees) -%}
grant {{ privilege }} on {{ relation }} to {{ adapter.quote(grantees[0]) }}
{%- endmacro %}

{%- macro trino__support_multiple_grantees_per_dcl_statement() -%}
{#
-- This macro should return true or false depending on the answer to
-- following question:
-- does this database support grant {privilege} to user_a, user_b, ...?
-- or do user_a + user_b need their own separate grant statements?
#}
{{ return(False) }}
{%- endmacro -%}

{% macro trino__call_dcl_statements(dcl_statement_list) %}
{% for dcl_statement in dcl_statement_list %}
{% call statement('grant_or_revoke') %}
{{ dcl_statement }}
{% endcall %}
{% endfor %}
{% endmacro %}
7 changes: 7 additions & 0 deletions dbt/include/trino/macros/materializations/incremental.sql
Original file line number Diff line number Diff line change
Expand Up @@ -130,6 +130,9 @@

{{ run_hooks(pre_hooks) }}

-- grab current tables grants config for comparision later on
{% set grant_config = config.get('grants') %}

{% if existing_relation is none %}
{% set build_sql = create_table_as(False, target_relation, sql) %}
{% elif existing_relation.is_view or full_refresh_mode %}
Expand All @@ -153,6 +156,10 @@

{{ run_hooks(post_hooks) }}

{% set should_revoke =
should_revoke(existing_relation.is_table, full_refresh_mode) %}
{% do apply_grants(target_relation, grant_config, should_revoke=should_revoke) %}

{% do persist_docs(target_relation, model) %}

{{ return({'relations': [target_relation]}) }}
Expand Down
10 changes: 8 additions & 2 deletions dbt/include/trino/macros/materializations/table.sql
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,9 @@

{{ run_hooks(pre_hooks) }}

-- grab current tables grants config for comparision later on
{% set grant_config = config.get('grants') %}

{% if on_table_exists == 'rename' %}
{#-- build modeldock #}
{% call statement('main') -%}
Expand Down Expand Up @@ -65,9 +68,12 @@
{%- endcall %}
{% endif %}

{{ run_hooks(post_hooks) }}

{% do persist_docs(target_relation, model) %}

{% set should_revoke = should_revoke(existing_relation, full_refresh_mode=True) %}
{% do apply_grants(target_relation, grant_config, should_revoke=should_revoke) %}

{{ run_hooks(post_hooks) }}

{{ return({'relations': [target_relation]}) }}
{% endmaterialization %}
3 changes: 3 additions & 0 deletions tests/conftest.py
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,9 @@ def get_trino_starburst_target():
"port": 8080,
"user": "admin",
"password": "",
"roles": {
"hive": "ROLE{admin}",
},
"catalog": "memory",
"schema": "default",
}
Expand Down
32 changes: 32 additions & 0 deletions tests/functional/adapter/test_model_grants.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
import pytest
from dbt.context.base import BaseContext # diff_of_two_dicts only
from dbt.tests.adapter.grants.test_model_grants import BaseModelGrants


@pytest.mark.hive
# TODO: setup Galaxy and Starbust tests
# See https://github.com/starburstdata/dbt-trino/issues/147
# and also https://github.com/starburstdata/dbt-trino/issues/146
@pytest.mark.skip_profile("starburst_galaxy")
# To run this test locally add following env vars:
# DBT_TEST_USER_1=user1
# DBT_TEST_USER_2=user2
# DBT_TEST_USER_3=user3
class TestModelGrants(BaseModelGrants):
def assert_expected_grants_match_actual(self, project, relation_name, expected_grants):
actual_grants = self.get_grants_on_relation(project, relation_name)
# Remove the creation user
try:
for privilege in ["delete", "update", "insert", "select"]:
if privilege in actual_grants:
actual_grants[privilege].remove("admin")
if len(actual_grants[privilege]) == 0:
del actual_grants[privilege]
except ValueError:
pass

# need a case-insensitive comparison
# so just a simple "assert expected == actual_grants" won't work
diff_a = BaseContext.diff_of_two_dicts(actual_grants, expected_grants)
diff_b = BaseContext.diff_of_two_dicts(expected_grants, actual_grants)
assert diff_a == diff_b == {}
2 changes: 1 addition & 1 deletion tox.ini
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ deps =
[testenv:integration]
basepython = python3
commands = /bin/bash -c '{envpython} -m pytest tests/functional'
passenv = DBT_INVOCATION_ENV DBT_TEST_TRINO_HOST
passenv = DBT_INVOCATION_ENV DBT_TEST_TRINO_HOST DBT_TEST_USER_1 DBT_TEST_USER_2 DBT_TEST_USER_3
deps =
-r{toxinidir}/dev_requirements.txt
-e.

0 comments on commit 9471340

Please sign in to comment.