From 0e9c5461917c8e44cf998745f4f3342b93e05f22 Mon Sep 17 00:00:00 2001 From: Bernardo Meireles <67381633+bcmeireles@users.noreply.github.com> Date: Mon, 27 Jan 2025 15:08:47 +0000 Subject: [PATCH] Support for roles in grants (#259) ### Summary Added support for roles in grants ### Description Grants can now be set for both users and for roles. A prefix was added to handle this, with `user:` and `role:` being the valid prefixes. For example, `user:dbt_test_user_1` and `role:dbt_test_role_1`. If no prefix is provided, defaults to user for backwards compatibility. ### Test Results Tests were ran against a local Dremio Enterprise instance and are skipped since the current pipeline uses Dremio OSS, and grants are an Enterprise feature ### Changelog - Added support for roles in grants ### Related Issue #177 --- .github/expected_failures.txt | 6 - .github/scripts/create_env_file.sh | 3 + CHANGELOG.md | 5 + dbt/adapters/dremio/impl.py | 6 +- .../dremio/macros/adapters/apply_grants.sql | 32 ++- .../adapter/grants/test_incremental_grants.py | 8 +- .../adapter/grants/test_invalid_grants.py | 3 + .../adapter/grants/test_model_grants.py | 201 +++++++++++++++++- .../adapter/grants/test_seed_grants.py | 4 + .../adapter/grants/test_snapshot_grants.py | 4 +- 10 files changed, 251 insertions(+), 21 deletions(-) diff --git a/.github/expected_failures.txt b/.github/expected_failures.txt index c92e6daf..9627fc12 100644 --- a/.github/expected_failures.txt +++ b/.github/expected_failures.txt @@ -3,12 +3,6 @@ tests/functional/adapter/dbt_clone/test_dbt_clone.py::TestCloneNotPossibleDremio tests/functional/adapter/dremio_specific/test_drop_temp_table.py::TestDropTempTableDremio::test_drop_temp_table tests/functional/adapter/dremio_specific/test_schema_parsing.py::TestSchemaParsingDremio::test_schema_with_dots tests/functional/adapter/dremio_specific/test_verify_ssl.py::TestVerifyCertificateDremio::test_insecure_request_warning_not_exist -tests/functional/adapter/grants/test_incremental_grants.py::TestIncrementalGrantsDremio::test_incremental_grants -tests/functional/adapter/grants/test_invalid_grants.py::TestInvalidGrantsDremio::test_invalid_grants -tests/functional/adapter/grants/test_model_grants.py::TestViewGrantsDremio::test_view_table_grants -tests/functional/adapter/grants/test_model_grants.py::TestTableGrantsDremio::test_view_table_grants -tests/functional/adapter/grants/test_seed_grants.py::TestSeedGrantsDremio::test_seed_grants -tests/functional/adapter/grants/test_snapshot_grants.py::TestSnapshotGrantsDremio::test_snapshot_grants tests/functional/adapter/relation/test_get_relation_last_modified.py::TestGetLastRelationModified::test_get_last_relation_modified tests/functional/adapter/unit_testing/test_unit_testing.py::TestDremioUnitTestingTypes::test_unit_test_data_type tests/functional/adapter/unit_testing/test_unit_testing.py::TestDremioUnitTestCaseInsensitivity::test_case_insensitivity diff --git a/.github/scripts/create_env_file.sh b/.github/scripts/create_env_file.sh index 75e09707..ddd58f32 100644 --- a/.github/scripts/create_env_file.sh +++ b/.github/scripts/create_env_file.sh @@ -16,6 +16,9 @@ DREMIO_DATABASE=dbt_test 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_ROLE_1=dbt_test_role_1 +DBT_TEST_ROLE_2=dbt_test_role_2 +DREMIO_EDITION=community EOF echo ".env file created successfully." \ No newline at end of file diff --git a/CHANGELOG.md b/CHANGELOG.md index 5b29cb2f..6f6a4efb 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -3,6 +3,11 @@ ## Changes - When naming reflections, if a `name` config is not set, the `alias` config parameter will be used instead. If also undefined, it will refer to the model name instead of using `Unnamed Reflection` +- Grants can now be set for both users and for roles. A prefix was added to handle this, with `user:` and `role:` being the valid prefixes. For example, `user:dbt_test_user_1` and `role:dbt_test_role_1`. If no prefix is provided, defaults to user for backwards compatibility. + +## Features + +- [#259](https://github.com/dremio/dbt-dremio/pull/259) Added support for roles in grants # dbt-dremio v1.8.1 diff --git a/dbt/adapters/dremio/impl.py b/dbt/adapters/dremio/impl.py index 2f24acdb..3210ea18 100644 --- a/dbt/adapters/dremio/impl.py +++ b/dbt/adapters/dremio/impl.py @@ -149,10 +149,12 @@ def standardize_grants_dict(self, grants_table: agate.Table) -> dict: # Just needed to change these two values to match Dremio cols grantee = row["grantee_id"] privilege = row["privilege"] + grantee_type = row["grantee_type"] + if privilege in grants_dict.keys(): - grants_dict[privilege].append(grantee) + grants_dict[privilege].append(f"{grantee_type}:{grantee}") else: - grants_dict.update({privilege: [grantee]}) + grants_dict.update({privilege: [f"{grantee_type}:{grantee}"]}) return grants_dict # This is for use in the test suite diff --git a/dbt/include/dremio/macros/adapters/apply_grants.sql b/dbt/include/dremio/macros/adapters/apply_grants.sql index 1345f986..409cdfa1 100644 --- a/dbt/include/dremio/macros/adapters/apply_grants.sql +++ b/dbt/include/dremio/macros/adapters/apply_grants.sql @@ -16,6 +16,24 @@ limitations under the License.*/ {{ return(False) }} {%- endmacro -%} +{%- macro dremio__split_grantee(grantee) -%} + {%- set splitted = grantee.split(':') -%} + + {%- if splitted | length < 2 -%} + {{ log("Deprecation warning: grants to users will soon require the user: prefix", info=True) }} + {{ return(("user", grantee)) }} + {%- else -%} + {%- set prefix = splitted[0] -%} + {%- set remainder = splitted[1:] | join(':') -%} + + {%- if prefix not in ['user', 'role'] -%} + {% do exceptions.CompilationError("Invalid prefix. Use either user or role") %} + {%- endif -%} + + {{ return((prefix, remainder)) }} + {%- endif -%} +{%- endmacro -%} + {% macro dremio__get_show_grant_sql(relation) %} {%- if relation.type == 'table' -%} {%- set relation_without_double_quotes = target.datalake ~ '.' ~ target.root_path ~ '.' ~ relation.identifier-%} @@ -30,17 +48,23 @@ limitations under the License.*/ {%- else -%} {% do exceptions.CompilationError("Invalid profile configuration: please only specify one of cloud_host or software_host in profiles.yml") %} {%- endif %} - SELECT privilege, grantee_id + SELECT privilege, grantee_type, grantee_id FROM {{privileges_table}} WHERE object_id='{{ relation_without_double_quotes }}' {% endmacro %} {%- macro dremio__get_grant_sql(relation, privilege, grantees) -%} - grant {{ privilege }} on {{relation.type}} {{ relation }} to user {{adapter.quote(grantees[0])}} + {%- set type, name = dremio__split_grantee(grantees[0]) %} + + grant {{ privilege }} on {{ relation.type }} {{ relation }} + to {{ type }} {{ adapter.quote(name) }} {%- endmacro -%} -{%- macro default__get_revoke_sql(relation, privilege, grantees) -%} - revoke {{ privilege }} on {{ relation.type }} {{ relation }} from user {{adapter.quote(grantees[0])}} +{%- macro dremio__get_revoke_sql(relation, privilege, grantees) -%} + {%- set type, name = dremio__split_grantee(grantees[0]) %} + + revoke {{ privilege }} on {{ relation.type }} {{ relation }} + from {{ type }} {{ adapter.quote(name) }} {%- endmacro -%} {% macro dremio__call_dcl_statements(dcl_statement_list) %} diff --git a/tests/functional/adapter/grants/test_incremental_grants.py b/tests/functional/adapter/grants/test_incremental_grants.py index ff5624d4..7767a1d2 100644 --- a/tests/functional/adapter/grants/test_incremental_grants.py +++ b/tests/functional/adapter/grants/test_incremental_grants.py @@ -12,6 +12,8 @@ # See the License for the specific language governing permissions and # limitations under the License. +import pytest, os + from dbt.tests.util import ( run_dbt_and_capture, get_manifest, @@ -26,7 +28,9 @@ user2_incremental_model_schema_yml, ) +DREMIO_EDITION = os.getenv("DREMIO_EDITION") +@pytest.mark.skipif(DREMIO_EDITION == "community", reason="Dremio only supports grants in EE/DC editions.") class TestIncrementalGrantsDremio(BaseGrantsDremio, BaseIncrementalGrants): # Define this here to use our modified version of relation_from_name def get_grants_on_relation(self, project, relation_name): @@ -53,7 +57,7 @@ def test_incremental_grants(self, project, get_test_users): model_id = "model.test.my_incremental_model" model = manifest.nodes[model_id] assert model.config.materialized == "incremental" - expected = {select_privilege_name: [test_users[0]]} + expected = {select_privilege_name: ["user:" + test_users[0]]} self.assert_expected_grants_match_actual( project, "my_incremental_model", expected ) @@ -80,7 +84,7 @@ def test_incremental_grants(self, project, get_test_users): manifest = get_manifest(project.project_root) model = manifest.nodes[model_id] assert model.config.materialized == "incremental" - expected = {select_privilege_name: [test_users[1]]} + expected = {select_privilege_name: ["user:" + test_users[1]]} self.assert_expected_grants_match_actual( project, "my_incremental_model", expected ) diff --git a/tests/functional/adapter/grants/test_invalid_grants.py b/tests/functional/adapter/grants/test_invalid_grants.py index 56689199..c4ac745a 100644 --- a/tests/functional/adapter/grants/test_invalid_grants.py +++ b/tests/functional/adapter/grants/test_invalid_grants.py @@ -12,13 +12,16 @@ # See the License for the specific language governing permissions and # limitations under the License. +import pytest, os from dbt.tests.adapter.grants.test_invalid_grants import BaseInvalidGrants from tests.functional.adapter.grants.base_grants import BaseGrantsDremio from tests.utils.util import relation_from_name from dbt.tests.util import get_connection +DREMIO_EDITION = os.getenv("DREMIO_EDITION") +@pytest.mark.skipif(DREMIO_EDITION == "community", reason="Dremio only supports grants in EE/DC editions.") class TestInvalidGrantsDremio(BaseGrantsDremio, BaseInvalidGrants): def grantee_does_not_exist_error(self): return "Grant on catalog entity failed. User invalid_user does not exist." diff --git a/tests/functional/adapter/grants/test_model_grants.py b/tests/functional/adapter/grants/test_model_grants.py index c9b99b33..47f8a32e 100644 --- a/tests/functional/adapter/grants/test_model_grants.py +++ b/tests/functional/adapter/grants/test_model_grants.py @@ -12,6 +12,8 @@ # See the License for the specific language governing permissions and # limitations under the License. +import pytest, os + from dbt.tests.adapter.grants.test_model_grants import ( BaseModelGrants, user2_model_schema_yml, @@ -29,13 +31,112 @@ from tests.functional.adapter.grants.base_grants import BaseGrantsDremio from tests.utils.util import relation_from_name +DREMIO_EDITION = os.getenv("DREMIO_EDITION") + +role_model_schema_yml = """ +version: 2 +models: + - name: my_model + config: + grants: + select: ["role:{{ env_var('DBT_TEST_ROLE_1') }}"] +""" + +role2_model_schema_yml = """ +version: 2 +models: + - name: my_model + config: + grants: + select: ["role:{{ env_var('DBT_TEST_ROLE_2') }}"] +""" + +table_role_model_schema_yml = """ +version: 2 +models: + - name: my_model + config: + materialized: table + grants: + select: ["role:{{ env_var('DBT_TEST_ROLE_1') }}"] +""" + +role2_table_model_schema_yml = """ +version: 2 +models: + - name: my_model + config: + materialized: table + grants: + select: ["role:{{ env_var('DBT_TEST_ROLE_2') }}"] +""" + +multiple_roles_table_model_schema_yml = """ +version: 2 +models: + - name: my_model + config: + materialized: table + grants: + select: ["role:{{ env_var('DBT_TEST_ROLE_1') }}", "role:{{ env_var('DBT_TEST_ROLE_2') }}"] +""" + +multiple_privileges_role_table_model_schema_yml = """ +version: 2 +models: + - name: my_model + config: + materialized: table + grants: + select: ["role:{{ env_var('DBT_TEST_ROLE_1') }}"] + insert: ["role:{{ env_var('DBT_TEST_ROLE_2') }}"] +""" + +users_and_roles_model_schema_yml = """ +version: 2 +models: + - name: my_model + config: + grants: + select: ["role:{{ env_var('DBT_TEST_ROLE_1') }}", "user:{{ env_var('DBT_TEST_USER_1') }}", "user:{{ env_var('DBT_TEST_USER_2') }}"] + insert: ["role:{{ env_var('DBT_TEST_ROLE_2') }}", "user:{{ env_var('DBT_TEST_USER_3') }}"] +""" +users_and_roles_table_model_schema_yml = """ +version: 2 +models: + - name: my_model + config: + materialized: table + grants: + select: ["role:{{ env_var('DBT_TEST_ROLE_1') }}", "user:{{ env_var('DBT_TEST_USER_1') }}", "user:{{ env_var('DBT_TEST_USER_2') }}"] + insert: ["role:{{ env_var('DBT_TEST_ROLE_2') }}", "user:{{ env_var('DBT_TEST_USER_3') }}"] +""" + +special_character_role_model_schema_yml = """ +version: 2 +models: + - name: my_model + config: + grants: + select: ["role:test:role"] +""" + +@pytest.mark.skipif(DREMIO_EDITION == "community", reason="Dremio only supports grants in EE/DC editions.") class TestViewGrantsDremio(BaseGrantsDremio, BaseModelGrants): + def get_test_roles(self): + test_roles = [] + for env_var in ["DBT_TEST_ROLE_1", "DBT_TEST_ROLE_2"]: + role_name = os.getenv(env_var) + if role_name: + test_roles.append(role_name) + return test_roles # Overridden to only include view materialization def test_view_table_grants(self, project, get_test_users): # we want the test to fail, not silently skip test_users = get_test_users + test_roles = self.get_test_roles() select_privilege_name = self.privilege_grantee_name_overrides()["select"] assert len(test_users) == 3 @@ -48,6 +149,7 @@ def test_view_table_grants(self, project, get_test_users): expected = {select_privilege_name: [test_users[0]]} assert model.config.grants == expected assert model.config.materialized == "view" + expected = {select_privilege_name: ["user:" + test_users[0]]} self.assert_expected_grants_match_actual(project, "my_model", expected) # View materialization, change select grant user @@ -55,10 +157,46 @@ def test_view_table_grants(self, project, get_test_users): write_file(updated_yaml, project.project_root, "models", "schema.yml") (results, log_output) = run_dbt_and_capture(["--debug", "run"]) assert len(results) == 1 - expected = {select_privilege_name: [get_test_users[1]]} + expected = {select_privilege_name: ["user:" + get_test_users[1]]} + self.assert_expected_grants_match_actual(project, "my_model", expected) + + # roles + updated_yaml = self.interpolate_name_overrides(role_model_schema_yml) + write_file(updated_yaml, project.project_root, "models", "schema.yml") + (results, log_output) = run_dbt_and_capture(["--debug", "run"]) + assert len(results) == 1 + expected = {select_privilege_name: ["role:" + test_roles[0]]} + self.assert_expected_grants_match_actual(project, "my_model", expected) + + updated_yaml = self.interpolate_name_overrides(role2_model_schema_yml) + write_file(updated_yaml, project.project_root, "models", "schema.yml") + (results, log_output) = run_dbt_and_capture(["--debug", "run"]) + assert len(results) == 1 + expected = {select_privilege_name: ["role:" + test_roles[1]]} + self.assert_expected_grants_match_actual(project, "my_model", expected) + + updated_yaml = self.interpolate_name_overrides(users_and_roles_model_schema_yml) + write_file(updated_yaml, project.project_root, "models", "schema.yml") + (results, log_output) = run_dbt_and_capture(["--debug", "run"]) + assert len(results) == 1 + insert_privilege_name = self.privilege_grantee_name_overrides()["insert"] + expected = { + select_privilege_name: ["role:" + test_roles[0], "user:" + get_test_users[0], + "user:" + get_test_users[1]], + insert_privilege_name: ["role:" + test_roles[1], "user:" + get_test_users[2]], + } + self.assert_expected_grants_match_actual(project, "my_model", expected) + + # special character (:) in role name + updated_yaml = self.interpolate_name_overrides(special_character_role_model_schema_yml) + write_file(updated_yaml, project.project_root, "models", "schema.yml") + (results, log_output) = run_dbt_and_capture(["--debug", "run"]) + assert len(results) == 1 + expected = {select_privilege_name: ["role:test:role"]} self.assert_expected_grants_match_actual(project, "my_model", expected) +@pytest.mark.skipif(DREMIO_EDITION == "community", reason="Dremio only supports grants in EE/DC editions.") class TestTableGrantsDremio(BaseGrantsDremio, BaseModelGrants): # Need to override this to make sure it uses our modified version of relation_from_name # This isn't needed for views, as dbt-core's version defaults to database/schema path @@ -72,9 +210,18 @@ def get_grants_on_relation(self, project, relation_name): actual_grants = adapter.standardize_grants_dict(grant_table) return actual_grants + def get_test_roles(self): + test_roles = [] + for env_var in ["DBT_TEST_ROLE_1", "DBT_TEST_ROLE_2"]: + role_name = os.getenv(env_var) + if role_name: + test_roles.append(role_name) + return test_roles + # Overridden to only include table materializations def test_view_table_grants(self, project, get_test_users): test_users = get_test_users + test_roles = self.get_test_roles() select_privilege_name = self.privilege_grantee_name_overrides()["select"] insert_privilege_name = self.privilege_grantee_name_overrides()["insert"] assert len(test_users) == 3 @@ -87,7 +234,7 @@ def test_view_table_grants(self, project, get_test_users): model_id = "model.test.my_model" model = manifest.nodes[model_id] assert model.config.materialized == "table" - expected = {select_privilege_name: [test_users[0]]} + expected = {select_privilege_name: ["user:" + test_users[0]]} self.assert_expected_grants_match_actual(project, "my_model", expected) # Table materialization, change select grant user @@ -98,7 +245,7 @@ def test_view_table_grants(self, project, get_test_users): manifest = get_manifest(project.project_root) model = manifest.nodes[model_id] assert model.config.materialized == "table" - expected = {select_privilege_name: [test_users[1]]} + expected = {select_privilege_name: ["user:" + test_users[1]]} self.assert_expected_grants_match_actual(project, "my_model", expected) # Table materialization, multiple grantees @@ -111,7 +258,7 @@ def test_view_table_grants(self, project, get_test_users): manifest = get_manifest(project.project_root) model = manifest.nodes[model_id] assert model.config.materialized == "table" - expected = {select_privilege_name: [test_users[0], test_users[1]]} + expected = {select_privilege_name: ["user:" + test_users[0], "user:" + test_users[1]]} self.assert_expected_grants_match_actual(project, "my_model", expected) # Table materialization, multiple privileges @@ -125,7 +272,49 @@ def test_view_table_grants(self, project, get_test_users): model = manifest.nodes[model_id] assert model.config.materialized == "table" expected = { - select_privilege_name: [test_users[0]], - insert_privilege_name: [test_users[1]], + select_privilege_name: ["user:" + test_users[0]], + insert_privilege_name: ["user:" + test_users[1]], + } + self.assert_expected_grants_match_actual(project, "my_model", expected) + + # roles + updated_yaml = self.interpolate_name_overrides(table_role_model_schema_yml) + write_file(updated_yaml, project.project_root, "models", "schema.yml") + (results, log_output) = run_dbt_and_capture(["--debug", "run"]) + assert len(results) == 1 + expected = {select_privilege_name: ["role:" + test_roles[0]]} + self.assert_expected_grants_match_actual(project, "my_model", expected) + + updated_yaml = self.interpolate_name_overrides(role2_table_model_schema_yml) + write_file(updated_yaml, project.project_root, "models", "schema.yml") + (results, log_output) = run_dbt_and_capture(["--debug", "run"]) + assert len(results) == 1 + expected = {select_privilege_name: ["role:" + test_roles[1]]} + self.assert_expected_grants_match_actual(project, "my_model", expected) + + updated_yaml = self.interpolate_name_overrides(multiple_roles_table_model_schema_yml) + write_file(updated_yaml, project.project_root, "models", "schema.yml") + (results, log_output) = run_dbt_and_capture(["--debug", "run"]) + assert len(results) == 1 + expected = {select_privilege_name: ["role:" + test_roles[0], "role:" + test_roles[1]]} + self.assert_expected_grants_match_actual(project, "my_model", expected) + + updated_yaml = self.interpolate_name_overrides(multiple_privileges_role_table_model_schema_yml) + write_file(updated_yaml, project.project_root, "models", "schema.yml") + (results, log_output) = run_dbt_and_capture(["--debug", "run"]) + assert len(results) == 1 + expected = { + select_privilege_name: ["role:" + test_roles[0]], + insert_privilege_name: ["role:" + test_roles[1]], + } + self.assert_expected_grants_match_actual(project, "my_model", expected) + + updated_yaml = self.interpolate_name_overrides(users_and_roles_table_model_schema_yml) + write_file(updated_yaml, project.project_root, "models", "schema.yml") + (results, log_output) = run_dbt_and_capture(["--debug", "run"]) + assert len(results) == 1 + expected = { + select_privilege_name: ["role:" + test_roles[0], "user:" + get_test_users[0], "user:" + get_test_users[1]], + insert_privilege_name: ["role:" + test_roles[1], "user:" + get_test_users[2]], } self.assert_expected_grants_match_actual(project, "my_model", expected) diff --git a/tests/functional/adapter/grants/test_seed_grants.py b/tests/functional/adapter/grants/test_seed_grants.py index cb41028a..df4c7bf2 100644 --- a/tests/functional/adapter/grants/test_seed_grants.py +++ b/tests/functional/adapter/grants/test_seed_grants.py @@ -12,12 +12,16 @@ # See the License for the specific language governing permissions and # limitations under the License. +import pytest, os + from dbt.tests.adapter.grants.test_seed_grants import BaseSeedGrants from tests.functional.adapter.grants.base_grants import BaseGrantsDremio from tests.utils.util import relation_from_name from dbt.tests.util import get_connection +DREMIO_EDITION = os.getenv("DREMIO_EDITION") +@pytest.mark.skipif(DREMIO_EDITION == "community", reason="Dremio only supports grants in EE/DC editions.") class TestSeedGrantsDremio(BaseGrantsDremio, BaseSeedGrants): # Grants are reapplied if dbt run is ran twice without changes diff --git a/tests/functional/adapter/grants/test_snapshot_grants.py b/tests/functional/adapter/grants/test_snapshot_grants.py index ce60d919..38cdfa00 100644 --- a/tests/functional/adapter/grants/test_snapshot_grants.py +++ b/tests/functional/adapter/grants/test_snapshot_grants.py @@ -11,7 +11,7 @@ # WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. # See the License for the specific language governing permissions and # limitations under the License. -import pytest +import pytest, os from dbt.tests.adapter.grants.test_snapshot_grants import ( BaseSnapshotGrants, snapshot_schema_yml, @@ -36,7 +36,9 @@ {% endsnapshot %} """.strip() +DREMIO_EDITION = os.getenv("DREMIO_EDITION") +@pytest.mark.skipif(DREMIO_EDITION == "community", reason="Dremio only supports grants in EE/DC editions.") class TestSnapshotGrantsDremio(BaseGrantsDremio, BaseSnapshotGrants): # Override this to use our modified snapshot model @pytest.fixture(scope="class")