Fix groupable node partial parsing, raise DbtReferenceError in Runtim…

…eRefResolver (#7438) (#7461) (cherry picked from commit 9874f9e) Co-authored-by: Michelle Ark <[email protected]>
dbt-labs · Apr 25, 2023 · f37cb92 · f37cb92
1 parent 77867d7
commit f37cb92
Show file tree

Hide file tree

Showing 5 changed files with 110 additions and 19 deletions.
diff --git a/.changes/unreleased/Fixes-20230424-161843.yaml b/.changes/unreleased/Fixes-20230424-161843.yaml
@@ -0,0 +1,6 @@
+kind: Fixes
+body: fix groupable node partial parsing, raise DbtReferenceError at runtime for safety
+time: 2023-04-24T16:18:43.130637-04:00
+custom:
+  Author: MichelleArk
+  Issue: "7437"
diff --git a/core/dbt/context/providers.py b/core/dbt/context/providers.py
@@ -38,6 +38,7 @@
     Resource,
     ManifestNode,
     RefArgs,
+    AccessType,
 )
 from dbt.contracts.graph.metrics import MetricReference, ResolvedMetricReference
 from dbt.contracts.graph.unparsed import NodeVersion
@@ -65,11 +66,12 @@
     DbtRuntimeError,
     TargetNotFoundError,
     DbtValidationError,
+    DbtReferenceError,
 )
 from dbt.config import IsFQNResource
 from dbt.node_types import NodeType, ModelLanguage
 
-from dbt.utils import merge, AttrDict, MultiDict, args_to_dict
+from dbt.utils import merge, AttrDict, MultiDict, args_to_dict, cast_to_str
 
 from dbt import selected_resources
 
@@ -494,6 +496,17 @@ def resolve(
                 target_version=target_version,
                 disabled=isinstance(target_model, Disabled),
             )
+        elif (
+            target_model.resource_type == NodeType.Model
+            and target_model.access == AccessType.Private
+        ):
+            if not self.model.group or self.model.group != target_model.group:
+                raise DbtReferenceError(
+                    unique_id=self.model.unique_id,
+                    ref_unique_id=target_model.unique_id,
+                    group=cast_to_str(target_model.group),
+                )
+
         self.validate(target_model, target_name, target_package, target_version)
         return self.create_relation(target_model)
 

diff --git a/core/dbt/parser/partial.py b/core/dbt/parser/partial.py
@@ -851,9 +851,12 @@ def delete_schema_mssa_links(self, schema_file, dict_key, elem):
                     if self.saved_files[file_id]:
                         source_file = self.saved_files[file_id]
                         self.add_to_pp_files(source_file)
+                    # if the node's group has changed - need to reparse all referencing nodes to ensure valid ref access
+                    if node.group != elem.get("group"):
+                        self.schedule_referencing_nodes_for_parsing(node.unique_id)
                     # if the node's latest version has changed - need to reparse all referencing nodes to ensure correct ref resolution
                     if node.is_versioned and node.latest_version != elem.get("latest_version"):
-                        self.schedule_referencing_nodes_for_parsing(elem_unique_id)
+                        self.schedule_referencing_nodes_for_parsing(node.unique_id)
             # remove from patches
             schema_file.node_patches.remove(elem_unique_id)
 

diff --git a/tests/functional/partial_parsing/fixtures.py b/tests/functional/partial_parsing/fixtures.py
@@ -628,6 +628,11 @@
 
 """
 
+orders_downstream_sql = """
+select * from {{ ref('orders') }}
+
+"""
+
 model_a_sql = """
 select 1 as fun
 
@@ -1040,6 +1045,47 @@
     description: "Some order data"
 """
 
+
+groups_schema_yml_two_groups_private_orders_valid_access = """
+
+groups:
+  - name: test_group
+    owner:
+      name: test_group_owner
+  - name: test_group2
+    owner:
+      name: test_group_owner2
+
+models:
+  - name: orders
+    group: test_group
+    access: private
+    description: "Some order data"
+  - name: orders_downstream
+    group: test_group
+    description: "Some order data"
+"""
+
+groups_schema_yml_two_groups_private_orders_invalid_access = """
+
+groups:
+  - name: test_group
+    owner:
+      name: test_group_owner
+  - name: test_group2
+    owner:
+      name: test_group_owner2
+
+models:
+  - name: orders
+    group: test_group2
+    access: private
+    description: "Some order data"
+  - name: orders_downstream
+    group: test_group
+    description: "Some order data"
+"""
+
 groups_schema_yml_one_group_model_in_group2 = """
 
 groups:

diff --git a/tests/functional/partial_parsing/test_partial_parsing.py b/tests/functional/partial_parsing/test_partial_parsing.py
@@ -55,6 +55,7 @@
     gsm_override_sql,
     gsm_override2_sql,
     orders_sql,
+    orders_downstream_sql,
     snapshot_sql,
     snapshot2_sql,
     generic_schema_yml,
@@ -65,6 +66,8 @@
     groups_schema_yml_two_groups,
     groups_schema_yml_two_groups_edited,
     groups_schema_yml_one_group_model_in_group2,
+    groups_schema_yml_two_groups_private_orders_valid_access,
+    groups_schema_yml_two_groups_private_orders_invalid_access,
 )
 
 from dbt.exceptions import CompilationError, ParsingError
@@ -699,56 +702,57 @@ class TestGroups:
     def models(self):
         return {
             "orders.sql": orders_sql,
+            "orders_downstream.sql": orders_downstream_sql,
             "schema.yml": groups_schema_yml_one_group,
         }
 
     def test_pp_groups(self, project):
 
         # initial run
         results = run_dbt()
-        assert len(results) == 1
+        assert len(results) == 2
         manifest = get_manifest(project.project_root)
-        expected_nodes = ["model.test.orders"]
+        expected_nodes = ["model.test.orders", "model.test.orders_downstream"]
         expected_groups = ["group.test.test_group"]
-        assert expected_nodes == list(manifest.nodes.keys())
-        assert expected_groups == list(manifest.groups.keys())
+        assert expected_nodes == sorted(list(manifest.nodes.keys()))
+        assert expected_groups == sorted(list(manifest.groups.keys()))
 
         # add group to schema
         write_file(groups_schema_yml_two_groups, project.project_root, "models", "schema.yml")
         results = run_dbt(["--partial-parse", "run"])
-        assert len(results) == 1
+        assert len(results) == 2
         manifest = get_manifest(project.project_root)
-        expected_nodes = ["model.test.orders"]
+        expected_nodes = ["model.test.orders", "model.test.orders_downstream"]
         expected_groups = ["group.test.test_group", "group.test.test_group2"]
-        assert expected_nodes == list(manifest.nodes.keys())
-        assert expected_groups == list(manifest.groups.keys())
+        assert expected_nodes == sorted(list(manifest.nodes.keys()))
+        assert expected_groups == sorted(list(manifest.groups.keys()))
 
         # edit group in schema
         write_file(
             groups_schema_yml_two_groups_edited, project.project_root, "models", "schema.yml"
         )
         results = run_dbt(["--partial-parse", "run"])
-        assert len(results) == 1
+        assert len(results) == 2
         manifest = get_manifest(project.project_root)
-        expected_nodes = ["model.test.orders"]
+        expected_nodes = ["model.test.orders", "model.test.orders_downstream"]
         expected_groups = ["group.test.test_group", "group.test.test_group2_edited"]
-        assert expected_nodes == list(manifest.nodes.keys())
-        assert expected_groups == list(manifest.groups.keys())
+        assert expected_nodes == sorted(list(manifest.nodes.keys()))
+        assert expected_groups == sorted(list(manifest.groups.keys()))
 
         # delete group in schema
         write_file(groups_schema_yml_one_group, project.project_root, "models", "schema.yml")
         results = run_dbt(["--partial-parse", "run"])
-        assert len(results) == 1
+        assert len(results) == 2
         manifest = get_manifest(project.project_root)
-        expected_nodes = ["model.test.orders"]
+        expected_nodes = ["model.test.orders", "model.test.orders_downstream"]
         expected_groups = ["group.test.test_group"]
-        assert expected_nodes == list(manifest.nodes.keys())
-        assert expected_groups == list(manifest.groups.keys())
+        assert expected_nodes == sorted(list(manifest.nodes.keys()))
+        assert expected_groups == sorted(list(manifest.groups.keys()))
 
         # add back second group
         write_file(groups_schema_yml_two_groups, project.project_root, "models", "schema.yml")
         results = run_dbt(["--partial-parse", "run"])
-        assert len(results) == 1
+        assert len(results) == 2
 
         # remove second group with model still configured to second group
         write_file(
@@ -759,3 +763,22 @@ def test_pp_groups(self, project):
         )
         with pytest.raises(ParsingError):
             results = run_dbt(["--partial-parse", "run"])
+
+        # add back second group, make orders private with valid ref
+        write_file(
+            groups_schema_yml_two_groups_private_orders_valid_access,
+            project.project_root,
+            "models",
+            "schema.yml",
+        )
+        results = run_dbt(["--partial-parse", "run"])
+        assert len(results) == 2
+
+        write_file(
+            groups_schema_yml_two_groups_private_orders_invalid_access,
+            project.project_root,
+            "models",
+            "schema.yml",
+        )
+        with pytest.raises(ParsingError):
+            results = run_dbt(["--partial-parse", "run"])