Skip to content

Commit e8ff321

Browse files
jonhopper-dataengineersCortex Code
andauthored
Optimize grant macros for performance, case-insensitive roles, and empty-list fix (#34)
* Optimize grant macros for performance and add case-insensitive role matching Grant macros now check existing state before issuing SQL statements, skipping schemas/objects where grants are already in place. This reduces runtime from 20-30 minutes to under 2-3 minutes on large databases where grants are already applied. Also makes all role comparisons case-insensitive via normalization to uppercase. Adds integration tests for grant helpers and idempotency. .... Generated with [Cortex Code](https://docs.snowflake.com/en/user-guide/cortex-code/cortex-code) Co-Authored-By: Cortex Code <noreply@snowflake.com> * Address PR review: fix object_type filtering and use Snowflake grant idempotency - _grants_get_schema_object_privs: removed unused _role variable, added internal grantees uppercase normalization, added optional object_type parameter to filter by specific object type - grant_schema_read_specific: removed incorrect per-privilege skip logic that aggregated across object types. GRANT ON ALL is idempotent in Snowflake so always issue it. Only skip schema USAGE (single check) and future grants (which error on duplicates) - grant_schema_object_privileges: now passes object_type to both the helper query and the revoke query to avoid cross-object-type leakage .... Generated with [Cortex Code](https://docs.snowflake.com/en/user-guide/cortex-code/cortex-code) Co-Authored-By: Cortex Code <noreply@snowflake.com> * Fix SQL syntax error when grant_roles is empty list Guard NOT IN () clauses with length checks to prevent invalid SQL when grant_roles/role_list is an empty list. Snowflake rejects NOT IN () as a syntax error. .... Generated with [Cortex Code](https://docs.snowflake.com/en/user-guide/cortex-code/cortex-code) Co-Authored-By: Cortex Code <noreply@snowflake.com> * Address PR #34 review: normalize schema grants, filter by object_type and granted_to - _grants_get_schema_grants: now uppercases grantee_name so output matches normalized grant_roles for consistent comparisons - grant_procedure_usage: added granted_to = 'ROLE' filter to prevent picking up APPLICATION/SHARE grantees and generating invalid revokes - grant_schema_object_privileges: uppercase permission_list in revoke query to match information_schema which stores privileges uppercase - grant_schema_operate_specific: filter by object_type in ('PIPE','TASK') so OPERATE grants on other object types don't affect reconciliation - grant_schema_monitor_specific: same object_type filter for consistency .... Generated with [Cortex Code](https://docs.snowflake.com/en/user-guide/cortex-code/cortex-code) Co-Authored-By: Cortex Code <noreply@snowflake.com> --------- Co-authored-by: Cortex Code <noreply@snowflake.com>
1 parent 2dc80fe commit e8ff321

20 files changed

Lines changed: 792 additions & 423 deletions

CHANGELOG.md

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,19 @@
11
# Data Engineers Snowflake DataOps Utils Project Changelog
22
This file contains the changelog for the Data Engineers Snowflake DataOps Utils project, detailing updates, fixes, and enhancements made to the project over time.
33

4+
## v1.0.7 - 2026-06-25 - Grant Performance Optimization
5+
6+
### Changed
7+
- Modified `grant_object` macro to be grant-only: it no longer revokes privileges from roles not in the supplied list or revokes non-desired privileges. The macro now only ensures the specified privileges are granted to the specified roles.
8+
- **Performance**: `grant_schema_read_specific` now checks existing privileges via `information_schema.object_privileges` and `SHOW FUTURE GRANTS` before issuing statements. Schemas where all grants are already in place are skipped entirely.
9+
- **Performance**: `grant_schema_monitor_specific` and `grant_schema_operate_specific` now skip roles that already have the required privilege and only issue `GRANT USAGE ON SCHEMA` when missing.
10+
- **Performance**: `grant_schema_procedure_usage_specific` replaced O(n×m) per-procedure `SHOW GRANTS` calls with a single `information_schema.object_privileges` query per schema.
11+
- **Performance**: `grant_schema_object_privileges` replaced per-object `SHOW GRANTS` calls with a single bulk `information_schema.object_privileges` query per schema.
12+
- **Performance**: `grant_internal_share_read` and `grant_external_share_read` now check existing share grants via `DESC SHARE` upfront and skip schemas/objects that are already granted.
13+
- **Performance**: `grant_share_read_specific_schema` now checks existing share state before issuing grants.
14+
- Added helper macros `_grants_get_schema_object_privs`, `_grants_get_schema_grants`, and `_grants_get_future_grants` to `_helpers.sql` for bulk privilege state checking.
15+
- Bumped version from 1.0.6 to 1.0.7
16+
417
## v1.0.6 - 2026-06-23 - Database Clone Grant Ownership
518

619
### Added

README.md

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@
22

33
A macro-only [dbt](https://github.com/dbt-labs/dbt) package for Snowflake DataOps. Provides utilities for object lifecycle management, RBAC grant orchestration, dimensional modelling helpers, tagging, shares, and more.
44

5-
- **Version**: 1.0.6
5+
- **Version**: 1.0.7
66
- **dbt**: `>=1.3.0, <3.0.0`
77
- **Dependencies**: None (zero external package dependencies)
88
- **dbt Fusion**: Compatible
@@ -108,7 +108,7 @@ The following `vars` can be set in your `dbt_project.yml` or via `--vars` on the
108108
| `grant_schema_procedure_usage(exclude_schemas, grant_roles)` | Grant USAGE on all procedures across all schemas |
109109
| `grant_schema_procedure_usage_specific(schemas, grant_roles, revoke_current_grants, dry_run)` | Grant USAGE on all procedures in specific schemas |
110110
| `grant_schema_object_privileges(object_type, schema_name, permissions, roles)` | Bulk grant privileges on all objects of a type within a schema |
111-
| `grant_object(object_type, objects, grant_types, grant_roles)` | Reconcile privilege sets on specific objects for roles |
111+
| `grant_object(object_type, objects, grant_types, grant_roles)` | Grant privileges on specific objects to roles (grant-only, no revokes) |
112112
| `grant_object_application(object_type, objects, grant_types, grant_applications)` | Reconcile privilege sets on specific objects for applications |
113113
| `grant_usage_to_application(object_type, prefix, grant_applications)` | Grant USAGE on objects matching a prefix to applications |
114114
| `grant_operate_to_application(prefix, grant_applications)` | Grant OPERATE on tasks matching a prefix to applications |
@@ -231,7 +231,7 @@ vars:
231231
| `grant_schema_operate*` | OPERATE on tasks/pipes + schema usage |
232232
| `grant_schema_procedure_usage*` | USAGE on all procedures + schema usage |
233233
| `grant_share_read*` | Secure view exposure to outbound shares |
234-
| `grant_object` | Per-object privilege reconciliation for roles |
234+
| `grant_object` | Per-object privilege granting for roles (no revokes) |
235235
| `grant_object_application` | Per-object privilege reconciliation for applications |
236236
| `grant_privileges` | Environment-aware bundle orchestrator |
237237

dbt_project.yml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
name: 'dbt_dataengineers_utils'
2-
version: '1.0.6'
2+
version: '1.0.7'
33
config-version: 2
44

55
require-dbt-version: [">=1.9.4", "<3.0.0"]
Lines changed: 113 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,113 @@
1+
{#
2+
Test helper macros for grants management.
3+
Run via: dbt run-operation test_grants_helpers
4+
Returns nothing on success; raises an error on any assertion failure.
5+
#}
6+
{% macro test_grants_helpers() %}
7+
{% set failures = [] %}
8+
9+
{# ── Test _grants_normalize_roles ── #}
10+
11+
{# Test 1: lowercase roles are uppercased #}
12+
{% set result = dbt_dataengineers_utils._grants_normalize_roles(['analyst', 'developer']) %}
13+
{% if result != ['ANALYST', 'DEVELOPER'] %}
14+
{% do failures.append("Test 1 FAILED: lowercase normalization expected ['ANALYST', 'DEVELOPER'], got " ~ result) %}
15+
{% endif %}
16+
17+
{# Test 2: mixed-case roles are uppercased #}
18+
{% set result = dbt_dataengineers_utils._grants_normalize_roles(['Analyst', 'OPS_Support', 'readers_prod']) %}
19+
{% if result != ['ANALYST', 'OPS_SUPPORT', 'READERS_PROD'] %}
20+
{% do failures.append("Test 2 FAILED: mixed-case normalization expected ['ANALYST', 'OPS_SUPPORT', 'READERS_PROD'], got " ~ result) %}
21+
{% endif %}
22+
23+
{# Test 3: already-uppercase roles stay unchanged #}
24+
{% set result = dbt_dataengineers_utils._grants_normalize_roles(['ADMIN', 'DATAOPS_ADMIN']) %}
25+
{% if result != ['ADMIN', 'DATAOPS_ADMIN'] %}
26+
{% do failures.append("Test 3 FAILED: uppercase passthrough expected ['ADMIN', 'DATAOPS_ADMIN'], got " ~ result) %}
27+
{% endif %}
28+
29+
{# Test 4: empty list returns empty list #}
30+
{% set result = dbt_dataengineers_utils._grants_normalize_roles([]) %}
31+
{% if result != [] %}
32+
{% do failures.append("Test 4 FAILED: empty list expected [], got " ~ result) %}
33+
{% endif %}
34+
35+
{# Test 5: single role #}
36+
{% set result = dbt_dataengineers_utils._grants_normalize_roles(['my_Role']) %}
37+
{% if result != ['MY_ROLE'] %}
38+
{% do failures.append("Test 5 FAILED: single role expected ['MY_ROLE'], got " ~ result) %}
39+
{% endif %}
40+
41+
{# ── Test _grants_format_list ── #}
42+
43+
{# Test 6: formats a list of values into quoted comma-separated string #}
44+
{% set result = dbt_dataengineers_utils._grants_format_list(['SCHEMA_A', 'SCHEMA_B']) %}
45+
{% if result != "'SCHEMA_A', 'SCHEMA_B'" %}
46+
{% do failures.append("Test 6 FAILED: format_list expected \"'SCHEMA_A', 'SCHEMA_B'\", got " ~ result) %}
47+
{% endif %}
48+
49+
{# Test 7: empty list returns empty string #}
50+
{% set result = dbt_dataengineers_utils._grants_format_list([]) %}
51+
{% if result != '' %}
52+
{% do failures.append("Test 7 FAILED: format_list empty expected '', got " ~ result) %}
53+
{% endif %}
54+
55+
{# Test 8: single item list #}
56+
{% set result = dbt_dataengineers_utils._grants_format_list(['PUBLIC']) %}
57+
{% if result != "'PUBLIC'" %}
58+
{% do failures.append("Test 8 FAILED: format_list single expected \"'PUBLIC'\", got " ~ result) %}
59+
{% endif %}
60+
61+
{# Test 9: escapes single quotes within values #}
62+
{% set result = dbt_dataengineers_utils._grants_format_list(["it's"]) %}
63+
{% if result != "'it''s'" %}
64+
{% do failures.append("Test 9 FAILED: format_list escaping expected \"'it''s'\", got " ~ result) %}
65+
{% endif %}
66+
67+
{# ── Test _grants_append_unique ── #}
68+
69+
{# Test 10: appends unique value #}
70+
{% set test_list = ['A', 'B'] %}
71+
{% do dbt_dataengineers_utils._grants_append_unique(test_list, 'C') %}
72+
{% if test_list != ['A', 'B', 'C'] %}
73+
{% do failures.append("Test 10 FAILED: append_unique expected ['A', 'B', 'C'], got " ~ test_list) %}
74+
{% endif %}
75+
76+
{# Test 11: does not append duplicate #}
77+
{% set test_list = ['A', 'B', 'C'] %}
78+
{% do dbt_dataengineers_utils._grants_append_unique(test_list, 'B') %}
79+
{% if test_list != ['A', 'B', 'C'] %}
80+
{% do failures.append("Test 11 FAILED: append_unique duplicate expected ['A', 'B', 'C'], got " ~ test_list) %}
81+
{% endif %}
82+
83+
{# ── Test case-insensitive role comparisons ── #}
84+
85+
{# Test 12: normalized role list membership check #}
86+
{% set roles = dbt_dataengineers_utils._grants_normalize_roles(['analyst', 'Developer']) %}
87+
{% if 'ANALYST' not in roles %}
88+
{% do failures.append("Test 12 FAILED: 'ANALYST' should be in normalized roles") %}
89+
{% endif %}
90+
{% if 'DEVELOPER' not in roles %}
91+
{% do failures.append("Test 12b FAILED: 'DEVELOPER' should be in normalized roles") %}
92+
{% endif %}
93+
{% if 'analyst' in roles %}
94+
{% do failures.append("Test 12c FAILED: 'analyst' (lowercase) should NOT be in normalized roles") %}
95+
{% endif %}
96+
97+
{# Test 13: grantee_name from Snowflake (uppercase) matches normalized roles #}
98+
{% set roles = dbt_dataengineers_utils._grants_normalize_roles(['ops_support']) %}
99+
{% set snowflake_grantee = 'OPS_SUPPORT' %}
100+
{% if snowflake_grantee not in roles %}
101+
{% do failures.append("Test 13 FAILED: Snowflake grantee 'OPS_SUPPORT' should match normalized ['ops_support']") %}
102+
{% endif %}
103+
104+
{# ── Report results ── #}
105+
{% if failures | length > 0 %}
106+
{% for f in failures %}
107+
{% do log(f, info=True) %}
108+
{% endfor %}
109+
{{ exceptions.raise_compiler_error("test_grants_helpers: " ~ (failures | length) ~ " test(s) failed. See log above.") }}
110+
{% else %}
111+
{% do log("test_grants_helpers: all 13 tests passed", info=True) %}
112+
{% endif %}
113+
{% endmacro %}
Lines changed: 127 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,127 @@
1+
{#
2+
Integration test: verifies grant macros are idempotent (skip when no changes needed).
3+
Run via: dbt run-operation test_grants_idempotency
4+
Requires a Snowflake connection. Uses grants_dry_run=true to avoid mutations.
5+
Returns nothing on success; raises an error on any assertion failure.
6+
7+
This test:
8+
1. Runs grant macros once to establish state
9+
2. Captures the log output / statement counts
10+
3. Verifies that the macros correctly identify "no changes required" scenarios
11+
#}
12+
{% macro test_grants_idempotency() %}
13+
{% if flags.WHICH not in ['run', 'run-operation'] %}
14+
{% do log('test_grants_idempotency: skipped (context)', info=True) %}
15+
{% do return(none) %}
16+
{% endif %}
17+
{% if not execute %}
18+
{% do log('test_grants_idempotency: skipped (compile phase)', info=True) %}
19+
{% do return(none) %}
20+
{% endif %}
21+
22+
{% set failures = [] %}
23+
{% set test_schema = 'PUBLIC' %}
24+
25+
{# ── Test 1: _grants_get_schema_grants returns a list ── #}
26+
{% set roles_with_usage = dbt_dataengineers_utils._grants_get_schema_grants(test_schema, 'USAGE', 'ROLE') %}
27+
{% if roles_with_usage is not iterable %}
28+
{% do failures.append("Test 1 FAILED: _grants_get_schema_grants should return a list, got " ~ roles_with_usage) %}
29+
{% else %}
30+
{% do log("Test 1 PASSED: _grants_get_schema_grants returned " ~ (roles_with_usage | length) ~ " roles with USAGE on " ~ test_schema, info=True) %}
31+
{% endif %}
32+
33+
{# ── Test 2: _grants_get_schema_object_privs returns a dict ── #}
34+
{% set privs = dbt_dataengineers_utils._grants_get_schema_object_privs(test_schema, ['SELECT'], []) %}
35+
{% if privs is not mapping %}
36+
{% do failures.append("Test 2 FAILED: _grants_get_schema_object_privs should return a dict, got " ~ privs) %}
37+
{% else %}
38+
{% do log("Test 2 PASSED: _grants_get_schema_object_privs returned dict with " ~ (privs.keys() | list | length) ~ " roles", info=True) %}
39+
{% endif %}
40+
41+
{# ── Test 3: _grants_get_future_grants returns a dict ── #}
42+
{% set future = dbt_dataengineers_utils._grants_get_future_grants(test_schema) %}
43+
{% if future is not mapping %}
44+
{% do failures.append("Test 3 FAILED: _grants_get_future_grants should return a dict, got " ~ future) %}
45+
{% else %}
46+
{% do log("Test 3 PASSED: _grants_get_future_grants returned dict with " ~ (future.keys() | list | length) ~ " roles", info=True) %}
47+
{% endif %}
48+
49+
{# ── Test 4: _grants_collect_schemas returns non-empty list ── #}
50+
{% set schemas = dbt_dataengineers_utils._grants_collect_schemas(['INFORMATION_SCHEMA'], is_exclude_list=true) %}
51+
{% if schemas | length == 0 %}
52+
{% do failures.append("Test 4 FAILED: _grants_collect_schemas returned no schemas (database may be empty)") %}
53+
{% else %}
54+
{% do log("Test 4 PASSED: _grants_collect_schemas found " ~ (schemas | length) ~ " schemas", info=True) %}
55+
{% endif %}
56+
57+
{# ── Test 5: grant_schema_monitor_specific with existing role skips ── #}
58+
{# Find a role that already has MONITOR grants in the test schema #}
59+
{% set monitor_query %}
60+
select distinct grantee
61+
from information_schema.object_privileges
62+
where privilege_type = 'MONITOR' and object_schema = '{{ test_schema }}'
63+
limit 1
64+
{% endset %}
65+
{% set monitor_results = run_query(monitor_query) %}
66+
{% if monitor_results and monitor_results | length > 0 %}
67+
{% set existing_role = monitor_results[0][0] %}
68+
{% do log("Test 5: Found existing MONITOR role: " ~ existing_role ~ ", running monitor_specific with dry_run", info=True) %}
69+
{# Running with the role that already has MONITOR should produce minimal/no new statements #}
70+
{{ dbt_dataengineers_utils.grant_schema_monitor_specific([test_schema], [existing_role], false, true) }}
71+
{% do log("Test 5 PASSED: grant_schema_monitor_specific completed without error for existing role", info=True) %}
72+
{% else %}
73+
{% do log("Test 5 SKIPPED: no existing MONITOR grants found in " ~ test_schema, info=True) %}
74+
{% endif %}
75+
76+
{# ── Test 6: grant_schema_read_specific runs without error (dry-run equivalent) ── #}
77+
{# Use a role that likely already has SELECT on PUBLIC #}
78+
{% set select_query %}
79+
select distinct grantee
80+
from information_schema.object_privileges
81+
where privilege_type = 'SELECT' and object_schema = '{{ test_schema }}'
82+
limit 1
83+
{% endset %}
84+
{% set select_results = run_query(select_query) %}
85+
{% if select_results and select_results | length > 0 %}
86+
{% set existing_role = select_results[0][0] %}
87+
{% do log("Test 6: Running grant_schema_read_specific for role " ~ existing_role ~ " which already has SELECT", info=True) %}
88+
{# revoke_current_grants=false prevents any mutations; just tests the skip-logic path #}
89+
{{ dbt_dataengineers_utils.grant_schema_read_specific([test_schema], [existing_role], false, false) }}
90+
{% do log("Test 6 PASSED: grant_schema_read_specific completed without error", info=True) %}
91+
{% else %}
92+
{% do log("Test 6 SKIPPED: no existing SELECT grants found in " ~ test_schema, info=True) %}
93+
{% endif %}
94+
95+
{# ── Test 7: Case-insensitive role matching works against live data ── #}
96+
{% if select_results and select_results | length > 0 %}
97+
{% set existing_role = select_results[0][0] %}
98+
{# Pass lowercase version of the role - should still match #}
99+
{% set lower_role = existing_role | lower %}
100+
{% set normalized = dbt_dataengineers_utils._grants_normalize_roles([lower_role]) %}
101+
{% if normalized[0] != existing_role | upper %}
102+
{% do failures.append("Test 7 FAILED: normalize_roles('" ~ lower_role ~ "') should be '" ~ (existing_role | upper) ~ "', got " ~ normalized[0]) %}
103+
{% else %}
104+
{% do log("Test 7 PASSED: normalize_roles('" ~ lower_role ~ "') == '" ~ normalized[0] ~ "'", info=True) %}
105+
{% endif %}
106+
{% else %}
107+
{% do log("Test 7 SKIPPED: no roles to test", info=True) %}
108+
{% endif %}
109+
110+
{# ── Test 8: grant_schema_procedure_usage_specific runs without error ── #}
111+
{{ dbt_dataengineers_utils.grant_schema_procedure_usage_specific([test_schema], ['SYSADMIN'], false, true) }}
112+
{% do log("Test 8 PASSED: grant_schema_procedure_usage_specific completed without error (dry_run=true)", info=True) %}
113+
114+
{# ── Test 9: grant_schema_operate_specific runs without error ── #}
115+
{{ dbt_dataengineers_utils.grant_schema_operate_specific([test_schema], ['SYSADMIN'], false, true) }}
116+
{% do log("Test 9 PASSED: grant_schema_operate_specific completed without error (dry_run=true)", info=True) %}
117+
118+
{# ── Report results ── #}
119+
{% if failures | length > 0 %}
120+
{% for f in failures %}
121+
{% do log(f, info=True) %}
122+
{% endfor %}
123+
{{ exceptions.raise_compiler_error("test_grants_idempotency: " ~ (failures | length) ~ " test(s) failed. See log above.") }}
124+
{% else %}
125+
{% do log("test_grants_idempotency: all tests passed", info=True) %}
126+
{% endif %}
127+
{% endmacro %}

0 commit comments

Comments
 (0)