From fcc3cc7895f1a353a5eb51c0f6f5af5ff1c7134d Mon Sep 17 00:00:00 2001 From: Ben Knight Date: Wed, 19 Aug 2026 05:17:40 +0000 Subject: [PATCH] test(denies): give each xdist worker its own deny principal The denies suite created and dropped one database-wide user, `dbt_deny_reader`, from a class-scoped fixture shared by all eight test classes. A database principal is database-scoped: unlike `project.test_schema` it is not isolated per test. Under `pytest -n auto` (how `make functional` runs) the classes are spread across worker processes sharing one TestDB, so the first class to finish dropped the user out from under every class still running, taking its sys.database_permissions rows with it. Nothing failed loudly at that point, because apply_denies is deliberately warn-and-skip on an absent principal: the rebuild succeeded having applied no DENY, and the assertions saw an empty set -- `assert ('SELECT', 'dbt_deny_reader') in set()`. Six of the eight tests failed this way, non-deterministically and only under xdist. Derive the principal name from PYTEST_XDIST_WORKER instead. Classes on one worker run sequentially, so per-worker names are enough to keep setup and teardown from overlapping. The model templates carry a `__DENY_PRINCIPAL__` placeholder swapped at import; str.format and f-strings are unusable on them because they are Jinja and `{{ config() }}` would be eaten as an escaped brace. Test-only: no adapter behaviour changes. Co-Authored-By: Claude Opus 5 (1M context) --- tests/functional/adapter/mssql/test_denies.py | 55 +++++++++++++++---- 1 file changed, 44 insertions(+), 11 deletions(-) diff --git a/tests/functional/adapter/mssql/test_denies.py b/tests/functional/adapter/mssql/test_denies.py index 10c80665..b4cb4cea 100644 --- a/tests/functional/adapter/mssql/test_denies.py +++ b/tests/functional/adapter/mssql/test_denies.py @@ -10,12 +10,35 @@ principal that holds a schema-level GRANT. """ +import os + import pytest from dbt.tests.util import get_connection, run_dbt, run_dbt_and_capture # A login-less database user used as the deny target across the suite. -DENY_PRINCIPAL = "dbt_deny_reader" +# +# The name is per-xdist-worker, and that is load-bearing rather than cosmetic. A +# database principal is database-scoped: unlike `project.test_schema`, it is NOT +# isolated per test. The suite runs under `pytest -n auto` (see the Makefile), +# which spreads these classes across worker processes sharing one TestDB, so a +# single shared name lets the first class to finish drop the user out from under +# every class still running — taking its sys.database_permissions rows with it. +# apply_denies is deliberately warn-and-skip on a missing principal, so the +# rebuild does not fail; the DENY just silently never lands and the assertions +# see an empty set. Classes on the same worker run sequentially, so a per-worker +# name is enough to make setup and teardown non-overlapping. +DENY_PRINCIPAL = "dbt_deny_reader_{}".format(os.environ.get("PYTEST_XDIST_WORKER", "main")) + +# Placeholder the model templates below carry in their `denies` config, swapped +# for DENY_PRINCIPAL at import. str.format/f-strings are unusable on these: they +# are Jinja, and `{{ config(...) }}` would be eaten as an escaped brace. +PRINCIPAL_PLACEHOLDER = "__DENY_PRINCIPAL__" + + +def with_principal(sql): + """Substitute the per-worker principal name into a model template.""" + return sql.replace(PRINCIPAL_PLACEHOLDER, DENY_PRINCIPAL) # --------------------------------------------------------------------------- @@ -65,10 +88,12 @@ def deny_principal(project): # table materialization # --------------------------------------------------------------------------- -table_deny_sql = """ -{{ config(materialized="table", denies={"select": ["dbt_deny_reader"]}) }} +table_deny_sql = with_principal( + """ +{{ config(materialized="table", denies={"select": ["__DENY_PRINCIPAL__"]}) }} select 1 as id, cast('secret' as varchar(50)) as ssn """ +) class TestTableDenies: @@ -115,10 +140,12 @@ def test_deny_is_enforced_against_a_schema_grant(self, project): # every run, so an object-level DENY is lost most often here. # --------------------------------------------------------------------------- -view_deny_sql = """ -{{ config(materialized="view", denies={"select": ["dbt_deny_reader"]}) }} +view_deny_sql = with_principal( + """ +{{ config(materialized="view", denies={"select": ["__DENY_PRINCIPAL__"]}) }} select 1 as id, cast('secret' as varchar(50)) as ssn """ +) class TestViewDenies: @@ -140,11 +167,13 @@ def test_deny_survives_ordinary_view_rebuild(self, project): # incremental materialization — append and full-refresh paths # --------------------------------------------------------------------------- -incremental_deny_sql = """ -{{ config(materialized="incremental", denies={"select": ["dbt_deny_reader"]}) }} +incremental_deny_sql = with_principal( + """ +{{ config(materialized="incremental", denies={"select": ["__DENY_PRINCIPAL__"]}) }} select 1 as id {% if is_incremental() %}where 1 = 0{% endif %} """ +) class TestIncrementalDenies: @@ -178,17 +207,19 @@ def test_deny_survives_append_and_full_refresh(self, project): select 1 as id, cast('Smith' as varchar(50)) as surname """ -denied_snapshot_sql = """ +denied_snapshot_sql = with_principal( + """ {% snapshot denied_snapshot %} {{ config( unique_key='id', strategy='check', check_cols=['surname'], - denies={'select': ['dbt_deny_reader']} + denies={'select': ['__DENY_PRINCIPAL__']} ) }} select * from {{ ref('snap_source') }} {% endsnapshot %} """ +) class TestSnapshotDenies: @@ -264,13 +295,15 @@ def test_absent_principal_warns_and_run_succeeds(self, project): # unsupported privilege: warn and skip rather than taking down the run # --------------------------------------------------------------------------- -unsupported_privilege_sql = """ +unsupported_privilege_sql = with_principal( + """ {{ config( materialized="table", - denies={"execute": ["dbt_deny_reader"], "select": ["dbt_deny_reader"]} + denies={"execute": ["__DENY_PRINCIPAL__"], "select": ["__DENY_PRINCIPAL__"]} ) }} select 1 as id """ +) class TestUnsupportedPrivilege: