From 9a5f1552ffd015109746bbd84b7c025d147b613e Mon Sep 17 00:00:00 2001 From: Abhishek Singh Date: Mon, 29 Aug 2022 16:18:31 -0700 Subject: [PATCH 1/3] Support for managing access grants - Implemented macros oracle__get_show_grant_sql() and oracle__call_dcl_statements() - Updated materializations to fetch and update grants - Added new testcases for test grants - Updated Github actions workflow to create new test users DBT_TEST_USER_1, DBT_TEST_USER_2 and DBT_TEST_USER_3 --- .github/workflows/oracle-xe-adapter-tests.yml | 17 +++- dbt/adapters/oracle/__version__.py | 2 +- dbt/include/oracle/macros/apply_grants.sql | 33 +++++++ .../incremental/incremental.sql | 5 +- .../materializations/snapshot/snapshot.sql | 5 +- .../macros/materializations/table/table.sql | 4 + .../macros/materializations/view/view.sql | 4 + setup.cfg | 4 +- setup.py | 4 +- tests/functional/adapter/test_grants.py | 98 +++++++++++++++++++ 10 files changed, 167 insertions(+), 9 deletions(-) create mode 100644 dbt/include/oracle/macros/apply_grants.sql create mode 100644 tests/functional/adapter/test_grants.py diff --git a/.github/workflows/oracle-xe-adapter-tests.yml b/.github/workflows/oracle-xe-adapter-tests.yml index 56ee3f7..a3d0590 100644 --- a/.github/workflows/oracle-xe-adapter-tests.yml +++ b/.github/workflows/oracle-xe-adapter-tests.yml @@ -38,14 +38,17 @@ jobs: chmod +x ${{ github.workspace }}/.github/scripts/create_new_user.sh docker cp ${{ github.workspace }}/.github/scripts/create_new_user.sh oracle_db_xe:/home/oracle/create_new_user.sh - - name: Create dbt test user + - name: Create dbt test users run: | docker exec oracle_db_xe /home/oracle/create_new_user.sh dbt_test ${{ secrets.DBT_ORACLE_PASSWORD }} + docker exec oracle_db_xe /home/oracle/create_new_user.sh dbt_test_user_1 ${{ secrets.DBT_ORACLE_PASSWORD }} + docker exec oracle_db_xe /home/oracle/create_new_user.sh dbt_test_user_2 ${{ secrets.DBT_ORACLE_PASSWORD }} + docker exec oracle_db_xe /home/oracle/create_new_user.sh dbt_test_user_3 ${{ secrets.DBT_ORACLE_PASSWORD }} - name: Install dbt-oracle with core dependencies run: | python -m pip install --upgrade pip - pip install pytest dbt-tests-adapter==1.2.0 + pip install pytest dbt-tests-adapter==1.2.1 pip install -r requirements.txt pip install -e . @@ -68,6 +71,9 @@ jobs: DBT_ORACLE_PROTOCOL: tcp LD_LIBRARY_PATH: /opt/oracle/instantclient_21_6 TNS_ADMIN: /opt/tns_admin + DBT_TEST_USER_1: DBT_TEST_USER_1 + DBT_TEST_USER_2: DBT_TEST_USER_2 + DBT_TEST_USER_3: DBT_TEST_USER_3 - name: Run adapter tests - ORA_PYTHON_DRIVER_TYPE => THICK run: | @@ -84,6 +90,9 @@ jobs: DBT_ORACLE_PROTOCOL: tcp LD_LIBRARY_PATH: /opt/oracle/instantclient_21_6 TNS_ADMIN: /opt/tns_admin + DBT_TEST_USER_1: DBT_TEST_USER_1 + DBT_TEST_USER_2: DBT_TEST_USER_2 + DBT_TEST_USER_3: DBT_TEST_USER_3 - name: Run adapter tests - ORA_PYTHON_DRIVER_TYPE => THIN run: | @@ -100,3 +109,7 @@ jobs: DBT_ORACLE_PROTOCOL: tcp DISABLE_OOB: on TNS_ADMIN: /opt/tns_admin + DBT_TEST_USER_1: DBT_TEST_USER_1 + DBT_TEST_USER_2: DBT_TEST_USER_2 + DBT_TEST_USER_3: DBT_TEST_USER_3 + diff --git a/dbt/adapters/oracle/__version__.py b/dbt/adapters/oracle/__version__.py index a79a1d1..60f94fc 100644 --- a/dbt/adapters/oracle/__version__.py +++ b/dbt/adapters/oracle/__version__.py @@ -14,4 +14,4 @@ See the License for the specific language governing permissions and limitations under the License. """ -version = "1.2.0" +version = "1.2.1" diff --git a/dbt/include/oracle/macros/apply_grants.sql b/dbt/include/oracle/macros/apply_grants.sql new file mode 100644 index 0000000..7302a8d --- /dev/null +++ b/dbt/include/oracle/macros/apply_grants.sql @@ -0,0 +1,33 @@ +{# + Copyright (c) 2022, Oracle and/or its affiliates. + Copyright (c) 2020, Vitor Avancini + + Licensed under the Apache License, Version 2.0 (the "License"); + you may not use this file except in compliance with the License. + You may obtain a copy of the License at + + https://www.apache.org/licenses/LICENSE-2.0 + + Unless required by applicable law or agreed to in writing, software + distributed under the License is distributed on an "AS IS" BASIS, + 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. +#} + +{% macro oracle__get_show_grant_sql(relation) %} + {# SQL that returns the current grants (grantee-privilege pairs) #} + SELECT grantee as "grantee", privilege as "privilege_type" + FROM SYS.ALL_TAB_PRIVS + WHERE UPPER(table_name) = UPPER('{{ relation.identifier }}') + {% if relation.schema %} + AND UPPER(table_schema) = UPPER('{{ relation.schema }}') + {% endif %} +{% endmacro %} + +{% macro oracle__call_dcl_statements(dcl_statement_list) %} + {# Run each grant/revoke statement against the database. This is the culmination of apply_grants() #} + {% for dcl_statement in dcl_statement_list %} + {% do run_query(dcl_statement) %} + {% endfor %} +{% endmacro %} \ No newline at end of file diff --git a/dbt/include/oracle/macros/materializations/incremental/incremental.sql b/dbt/include/oracle/macros/materializations/incremental/incremental.sql index 5a1a39a..897b075 100644 --- a/dbt/include/oracle/macros/materializations/incremental/incremental.sql +++ b/dbt/include/oracle/macros/materializations/incremental/incremental.sql @@ -23,7 +23,7 @@ {% set existing_relation = load_relation(this) %} {% set tmp_relation = make_temp_relation(this) %} {% set on_schema_change = incremental_validate_on_schema_change(config.get('on_schema_change'), default='ignore') %} - + {% set grant_config = config.get('grants') %} {{ run_hooks(pre_hooks, inside_transaction=False) }} @@ -77,6 +77,9 @@ {{ run_hooks(post_hooks, inside_transaction=False) }} + {% set should_revoke = should_revoke(existing_relation.is_table, full_refresh_mode) %} + {% do apply_grants(target_relation, grant_config, should_revoke=should_revoke) %} + {{ return({'relations': [target_relation]}) }} {%- endmaterialization %} diff --git a/dbt/include/oracle/macros/materializations/snapshot/snapshot.sql b/dbt/include/oracle/macros/materializations/snapshot/snapshot.sql index 844cda8..61d661d 100644 --- a/dbt/include/oracle/macros/materializations/snapshot/snapshot.sql +++ b/dbt/include/oracle/macros/materializations/snapshot/snapshot.sql @@ -208,7 +208,7 @@ {%- set strategy_name = config.get('strategy') -%} {%- set unique_key = config.get('unique_key') %} - + {%- set grant_config = config.get('grants') -%} {% set model_database = model.database %} {% if model_database == 'None' or model_database is undefined or model_database is none %} {% set model_database = get_database_name() %} @@ -285,6 +285,9 @@ {{ final_sql }} {% endcall %} + {% set should_revoke = should_revoke(target_relation_exists, full_refresh_mode=False) %} + {% do apply_grants(target_relation, grant_config, should_revoke=should_revoke) %} + {% do persist_docs(target_relation, model) %} {{ run_hooks(post_hooks, inside_transaction=True) }} diff --git a/dbt/include/oracle/macros/materializations/table/table.sql b/dbt/include/oracle/macros/materializations/table/table.sql index de04c53..dcac918 100644 --- a/dbt/include/oracle/macros/materializations/table/table.sql +++ b/dbt/include/oracle/macros/materializations/table/table.sql @@ -16,6 +16,7 @@ #} {% materialization table, adapter='oracle' %} {% set identifier = model['alias'] %} + {% set grant_config = config.get('grants') %} {% set tmp_identifier = model['name'] + '__dbt_tmp' %} {% set backup_identifier = model['name'] + '__dbt_backup' %} {% set old_relation = adapter.get_relation(database=database, schema=schema, identifier=identifier) %} @@ -90,5 +91,8 @@ {{ run_hooks(post_hooks, inside_transaction=False) }} + {% set should_revoke = should_revoke(old_relation, full_refresh_mode=True) %} + {% do apply_grants(target_relation, grant_config, should_revoke=should_revoke) %} + {{ return({'relations': [target_relation]}) }} {% endmaterialization %} diff --git a/dbt/include/oracle/macros/materializations/view/view.sql b/dbt/include/oracle/macros/materializations/view/view.sql index 7edcda8..40b2b9c 100644 --- a/dbt/include/oracle/macros/materializations/view/view.sql +++ b/dbt/include/oracle/macros/materializations/view/view.sql @@ -17,6 +17,7 @@ {%- materialization view, adapter='oracle' -%} {%- set identifier = model['alias'] -%} + {%- set grant_config = config.get('grants') -%} {%- set backup_identifier = model['name'] + '__dbt_backup' -%} {%- set old_relation = adapter.get_relation(database=database, schema=schema, identifier=identifier) -%} @@ -71,6 +72,9 @@ {{ run_hooks(post_hooks, inside_transaction=False) }} + {% set should_revoke = should_revoke(old_relation, full_refresh_mode=True) %} + {% do apply_grants(target_relation, grant_config, should_revoke=should_revoke) %} + {{ return({'relations': [target_relation]}) }} {%- endmaterialization -%} diff --git a/setup.cfg b/setup.cfg index 5f94579..732c466 100644 --- a/setup.cfg +++ b/setup.cfg @@ -32,12 +32,12 @@ zip_safe = False packages = find: include_package_data = True install_requires = - dbt-core==1.2.0 + dbt-core==1.2.1 cx_Oracle==8.3.0 oracledb==1.0.3 test_suite=tests test_requires = - dbt-tests-adapter==1.2.0 + dbt-tests-adapter==1.2.1 pytest scripts = bin/create-pem-from-p12 diff --git a/setup.py b/setup.py index a7a9356..ca7e9a2 100644 --- a/setup.py +++ b/setup.py @@ -32,13 +32,13 @@ requirements = [ - "dbt-core==1.2.0", + "dbt-core==1.2.1", "cx_Oracle==8.3.0", "oracledb==1.0.3" ] test_requirements = [ - "dbt-tests-adapter==1.2.0", + "dbt-tests-adapter==1.2.1", "pytest" ] diff --git a/tests/functional/adapter/test_grants.py b/tests/functional/adapter/test_grants.py new file mode 100644 index 0000000..acb3555 --- /dev/null +++ b/tests/functional/adapter/test_grants.py @@ -0,0 +1,98 @@ +""" +Copyright (c) 2022, Oracle and/or its affiliates. +Copyright (c) 2020, Vitor Avancini + + Licensed under the Apache License, Version 2.0 (the "License"); + you may not use this file except in compliance with the License. + You may obtain a copy of the License at + + https://www.apache.org/licenses/LICENSE-2.0 + + Unless required by applicable law or agreed to in writing, software + distributed under the License is distributed on an "AS IS" BASIS, + 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 + +from dbt.tests.adapter.grants.test_model_grants import BaseModelGrants, model_schema_yml +from dbt.tests.adapter.grants.test_incremental_grants import BaseIncrementalGrants, incremental_model_schema_yml +from dbt.tests.adapter.grants.test_invalid_grants import BaseInvalidGrants +from dbt.tests.adapter.grants.test_seed_grants import BaseSeedGrants +from dbt.tests.adapter.grants.test_snapshot_grants import BaseSnapshotGrants, snapshot_schema_yml + +my_model_sql = """ + select 1 as fun from dual +""" + +my_incremental_model_sql = """ + select 1 as fun from dual +""" + +my_invalid_model_sql = """ + select 1 as fun from dual +""" + +my_snapshot_sql = """ +{% snapshot my_snapshot %} + {{ config( + check_cols='all', unique_key='id', strategy='check', + target_database=database, target_schema=schema + ) }} + select 1 as id, cast('blue' as {{ type_string() }}) as color from dual +{% endsnapshot %} +""".strip() + + +class TestSeedGrantsOracle(BaseSeedGrants): + pass + + +class TestModelGrantsOracle(BaseModelGrants): + + @pytest.fixture(scope="class") + def models(self): + updated_schema = self.interpolate_name_overrides(model_schema_yml) + + return { + "my_model.sql": my_model_sql, + "schema.yml": updated_schema, + } + + +class TestIncrementalGrantsOracle(BaseIncrementalGrants): + + @pytest.fixture(scope="class") + def models(self): + updated_schema = self.interpolate_name_overrides(incremental_model_schema_yml) + return { + "my_incremental_model.sql": my_incremental_model_sql, + "schema.yml": updated_schema, + } + + +class TestInvalidGrantsOracle(BaseInvalidGrants): + + def grantee_does_not_exist_error(self): + return "ORA-01917: user or role" + + def privilege_does_not_exist_error(self): + return "ORA-00990: missing or invalid privilege" + + @pytest.fixture(scope="class") + def models(self): + return { + "my_invalid_model.sql": my_invalid_model_sql, + } + + +class TestSnapshotGrantsOracle(BaseSnapshotGrants): + + @pytest.fixture(scope="class") + def snapshots(self): + return { + "my_snapshot.sql": my_snapshot_sql, + "schema.yml": self.interpolate_name_overrides(snapshot_schema_yml), + } From e50a488688c33670838f0e15dd181321013df2df Mon Sep 17 00:00:00 2001 From: Abhishek Singh Date: Mon, 29 Aug 2022 17:40:48 -0700 Subject: [PATCH 2/3] Added a newline to EOF --- dbt/include/oracle/macros/apply_grants.sql | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/dbt/include/oracle/macros/apply_grants.sql b/dbt/include/oracle/macros/apply_grants.sql index 7302a8d..9ea35d6 100644 --- a/dbt/include/oracle/macros/apply_grants.sql +++ b/dbt/include/oracle/macros/apply_grants.sql @@ -30,4 +30,4 @@ {% for dcl_statement in dcl_statement_list %} {% do run_query(dcl_statement) %} {% endfor %} -{% endmacro %} \ No newline at end of file +{% endmacro %} From 731be290c494be06c3a6aa21c17d3becc6c62b45 Mon Sep 17 00:00:00 2001 From: Abhishek Singh Date: Tue, 30 Aug 2022 11:12:48 -0700 Subject: [PATCH 3/3] Fix basic tests macro for where condition to filter results --- dbt/include/oracle/macros/schema_tests.sql | 4 +- .../adapter/test_generictests_where.py | 82 +++++++++++++++++++ 2 files changed, 84 insertions(+), 2 deletions(-) create mode 100644 tests/functional/adapter/test_generictests_where.py diff --git a/dbt/include/oracle/macros/schema_tests.sql b/dbt/include/oracle/macros/schema_tests.sql index 7c5d15e..a5b2288 100644 --- a/dbt/include/oracle/macros/schema_tests.sql +++ b/dbt/include/oracle/macros/schema_tests.sql @@ -21,7 +21,7 @@ with all_values as ( select distinct {{ column_name }} as value_field - from {{ model.include(False, True, True) }} + from {{ model }} ), @@ -53,7 +53,7 @@ select * from( select * from ( select count(*) as null_count -from {{ model.include(False, True, True) }} +from {{ model }} where {{ column_name }} is null) c where c.null_count != 0 {% endmacro %} diff --git a/tests/functional/adapter/test_generictests_where.py b/tests/functional/adapter/test_generictests_where.py new file mode 100644 index 0000000..d5aa05b --- /dev/null +++ b/tests/functional/adapter/test_generictests_where.py @@ -0,0 +1,82 @@ +""" +Copyright (c) 2022, Oracle and/or its affiliates. +Copyright (c) 2020, Vitor Avancini + + Licensed under the Apache License, Version 2.0 (the "License"); + you may not use this file except in compliance with the License. + You may obtain a copy of the License at + + https://www.apache.org/licenses/LICENSE-2.0 + + Unless required by applicable law or agreed to in writing, software + distributed under the License is distributed on an "AS IS" BASIS, + 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 + +from dbt.tests.adapter.basic.test_generic_tests import BaseGenericTests +from dbt.tests.adapter.basic.files import ( + seeds_base_csv, + base_view_sql, + base_table_sql, + schema_base_yml, +) + +generic_test_seed_yml = """ +version: 2 +models: + - name: base + columns: + - name: id + tests: + - not_null: + config: + where: "name = 'Easton'" +""" + +generic_test_view_yml = """ +version: 2 +models: + - name: view_model + columns: + - name: id + tests: + - not_null: + config: + where: "name = 'Easton'" +""" + +generic_test_table_yml = """ +version: 2 +models: + - name: table_model + columns: + - name: id + tests: + - not_null: + config: + where: "name = 'Easton'" +""" + + +class TestGenericTestsOracle(BaseGenericTests): + + @pytest.fixture(scope="class") + def seeds(self): + return { + "base.csv": seeds_base_csv, + "schema.yml": generic_test_seed_yml, + } + + @pytest.fixture(scope="class") + def models(self): + return { + "view_model.sql": base_view_sql, + "table_model.sql": base_table_sql, + "schema.yml": schema_base_yml, + "schema_view.yml": generic_test_view_yml, + "schema_table.yml": generic_test_table_yml, + }