CT 1993 handle invalid access to private models (#7069)

.changes/unreleased/Features-20230224-182214.yaml

@@ -0,0 +1,6 @@
+kind: Features
+body: Disallow refing private model across groups
+time: 2023-02-24T18:22:14.965136-05:00
+custom:
+  Author: gshank
+  Issue: "6826"

core/dbt/contracts/graph/manifest.py

@@ -802,8 +802,8 @@ def build_group_map(self):
         )
         group_map = {group.name: [] for group in self.groups.values()}
         for node in groupable_nodes:
-            if node.config.group is not None:
-                group_map[node.config.group].append(node.unique_id)
+            if node.group is not None:
+                group_map[node.group].append(node.unique_id)
         self.group_map = group_map
 
     def writable_manifest(self):

core/dbt/contracts/graph/nodes.py

@@ -1063,6 +1063,7 @@ class Metric(GraphNode):
     refs: List[List[str]] = field(default_factory=list)
     metrics: List[List[str]] = field(default_factory=list)
     created_at: float = field(default_factory=lambda: time.time())
+    group: Optional[str] = None
 
     @property
     def depends_on_nodes(self):

core/dbt/exceptions.py

@@ -1114,6 +1114,20 @@ def __init__(self, exc: ValidationError, node):
         super().__init__(msg=self.msg)
 
 
+class DbtReferenceError(ParsingError):
+    def __init__(self, unique_id: str, ref_unique_id: str, group: str):
+        self.unique_id = unique_id
+        self.ref_unique_id = ref_unique_id
+        self.group = group
+        super().__init__(msg=self.get_message())
+
+    def get_message(self) -> str:
+        return (
+            f"Node {self.unique_id} attempted to reference node {self.ref_unique_id}, "
+            f"which is not allowed because the referenced node is private to the {self.group} group."
+        )
+
+
 class InvalidAccessTypeError(ParsingError):
     def __init__(self, unique_id: str, field_value: str):
         self.unique_id = unique_id

core/dbt/parser/manifest.py

@@ -37,7 +37,7 @@
     Note,
 )
 from dbt.logger import DbtProcessState
-from dbt.node_types import NodeType
+from dbt.node_types import NodeType, AccessType
 from dbt.clients.jinja import get_rendered, MacroStack
 from dbt.clients.jinja_static import statically_extract_macro_calls
 from dbt.clients.system import make_directory, write_file
@@ -1076,7 +1076,7 @@ def _check_valid_group_config(manifest: Manifest):
 def _check_valid_group_config_node(
     groupable_node: Union[Metric, ManifestNode], valid_group_names: Set[str]
 ):
-    groupable_node_group = groupable_node.config.group
+    groupable_node_group = groupable_node.group
     if groupable_node_group and groupable_node_group not in valid_group_names:
         raise dbt.exceptions.ParsingError(
             f"Invalid group '{groupable_node_group}', expected one of {sorted(list(valid_group_names))}",
@@ -1196,6 +1196,16 @@ def _process_refs_for_exposure(manifest: Manifest, current_project: str, exposur
             )
 
             continue
+        elif (
+            target_model.resource_type == NodeType.Model
+            and target_model.access == AccessType.Private
+        ):
+            # Exposures do not have a group and so can never reference private models
+            raise dbt.exceptions.DbtReferenceError(
+                unique_id=exposure.unique_id,
+                ref_unique_id=target_model.unique_id,
+                group=dbt.utils.cast_to_str(target_model.group),
+            )
 
         target_model_id = target_model.unique_id
 
@@ -1239,6 +1249,16 @@ def _process_refs_for_metric(manifest: Manifest, current_project: str, metric: M
                 should_warn_if_disabled=False,
             )
             continue
+        elif (
+            target_model.resource_type == NodeType.Model
+            and target_model.access == AccessType.Private
+        ):
+            if not metric.group or metric.group != target_model.group:
+                raise dbt.exceptions.DbtReferenceError(
+                    unique_id=metric.unique_id,
+                    ref_unique_id=target_model.unique_id,
+                    group=dbt.utils.cast_to_str(target_model.group),
+                )
 
         target_model_id = target_model.unique_id
 
@@ -1336,6 +1356,18 @@ def _process_refs_for_node(manifest: Manifest, current_project: str, node: Manif
             )
             continue
 
+        # Handle references to models that are private
+        elif (
+            target_model.resource_type == NodeType.Model
+            and target_model.access == AccessType.Private
+        ):
+            if not node.group or node.group != target_model.group:
+                raise dbt.exceptions.DbtReferenceError(
+                    unique_id=node.unique_id,
+                    ref_unique_id=target_model.unique_id,
+                    group=dbt.utils.cast_to_str(target_model.group),
+                )
+
         target_model_id = target_model.unique_id
 
         node.depends_on.nodes.append(target_model_id)

core/dbt/parser/schemas.py

@@ -432,7 +432,7 @@ def render_test_update(self, node, config, builder, schema_file_id):
         attached_node = self._lookup_attached_node(builder.target)
         if attached_node:
             node.attached_node = attached_node.unique_id
-            node.group, node.config.group = attached_node.config.group, attached_node.config.group
+            node.group, node.group = attached_node.group, attached_node.group
 
     def parse_node(self, block: GenericTestBlock) -> GenericTestNode:
         """In schema parsing, we rewrite most of the part of parse_node that
@@ -1235,6 +1235,7 @@ def parse_metric(self, unparsed: UnparsedMetric):
             tags=unparsed.tags,
             config=config,
             unrendered_config=unrendered_config,
+            group=config.group,
         )
 
         ctx = generate_parse_metrics(
@@ -1274,14 +1275,15 @@ def _generate_metric_config(
         # first apply metric configs
         precedence_configs.update(target.config)
 
-        return generator.calculate_node_config(
+        config = generator.calculate_node_config(
             config_call_dict={},
             fqn=fqn,
             resource_type=NodeType.Metric,
             project_name=package_name,
             base=False,
             patch_config_dict=precedence_configs,
         )
+        return config
 
     def parse(self):
         for data in self.get_key_dicts():

schemas/dbt/manifest/v9.json

@@ -221,7 +221,7 @@
         "generated_at": {
           "type": "string",
           "format": "date-time",
-          "default": "2023-02-24T18:16:27.384938Z"
+          "default": "2023-02-27T20:52:53.570061Z"
         },
         "invocation_id": {
           "oneOf": [
@@ -232,7 +232,7 @@
               "type": "null"
             }
           ],
-          "default": "678dd0f6-ab20-41c8-a3d8-bbc986a26583"
+          "default": "f578f819-6554-4f6f-8af4-635e35249cb2"
         },
         "env": {
           "type": "object",
@@ -449,7 +449,7 @@
         },
         "created_at": {
           "type": "number",
-          "default": 1677262587.389697
+          "default": 1677531173.5741172
         },
         "config_call_dict": {
           "type": "object",
@@ -1048,7 +1048,7 @@
         },
         "created_at": {
           "type": "number",
-          "default": 1677262587.3926132
+          "default": 1677531173.577135
         },
         "config_call_dict": {
           "type": "object",
@@ -1433,7 +1433,7 @@
         },
         "created_at": {
           "type": "number",
-          "default": 1677262587.394294
+          "default": 1677531173.578816
         },
         "config_call_dict": {
           "type": "object",
@@ -1706,7 +1706,7 @@
         },
         "created_at": {
           "type": "number",
-          "default": 1677262587.39604
+          "default": 1677531173.580534
         },
         "config_call_dict": {
           "type": "object",
@@ -1978,7 +1978,7 @@
         },
         "created_at": {
           "type": "number",
-          "default": 1677262587.397707
+          "default": 1677531173.5821729
         },
         "config_call_dict": {
           "type": "object",
@@ -2241,7 +2241,7 @@
         },
         "created_at": {
           "type": "number",
-          "default": 1677262587.399359
+          "default": 1677531173.5838
         },
         "config_call_dict": {
           "type": "object",
@@ -2499,7 +2499,7 @@
         },
         "created_at": {
           "type": "number",
-          "default": 1677262587.4014988
+          "default": 1677531173.5856462
         },
         "config_call_dict": {
           "type": "object",
@@ -2794,7 +2794,7 @@
         },
         "created_at": {
           "type": "number",
-          "default": 1677262587.404913
+          "default": 1677531173.588902
         },
         "config_call_dict": {
           "type": "object",
@@ -3277,7 +3277,7 @@
         },
         "created_at": {
           "type": "number",
-          "default": 1677262587.40784
+          "default": 1677531173.591784
         },
         "config_call_dict": {
           "type": "object",
@@ -3677,7 +3677,7 @@
         },
         "created_at": {
           "type": "number",
-          "default": 1677262587.410422
+          "default": 1677531173.594429
         }
       },
       "additionalProperties": false,
@@ -3792,7 +3792,7 @@
         "generated_at": {
           "type": "string",
           "format": "date-time",
-          "default": "2023-02-24T18:16:27.379048Z"
+          "default": "2023-02-27T20:52:53.564664Z"
         },
         "invocation_id": {
           "oneOf": [
@@ -3803,7 +3803,7 @@
               "type": "null"
             }
           ],
-          "default": "678dd0f6-ab20-41c8-a3d8-bbc986a26583"
+          "default": "f578f819-6554-4f6f-8af4-635e35249cb2"
         },
         "env": {
           "type": "object",
@@ -4156,7 +4156,7 @@
         },
         "created_at": {
           "type": "number",
-          "default": 1677262587.411136
+          "default": 1677531173.595732
         },
         "supported_languages": {
           "oneOf": [
@@ -4399,7 +4399,7 @@
         },
         "created_at": {
           "type": "number",
-          "default": 1677262587.4128408
+          "default": 1677531173.597582
         }
       },
       "additionalProperties": false,
@@ -4622,11 +4622,21 @@
         },
         "created_at": {
           "type": "number",
-          "default": 1677262587.414759
+          "default": 1677531173.599355
+        },
+        "group": {
+          "oneOf": [
+            {
+              "type": "string"
+            },
+            {
+              "type": "null"
+            }
+          ]
         }
       },
       "additionalProperties": false,
-      "description": "Metric(name: str, resource_type: dbt.node_types.NodeType, package_name: str, path: str, original_file_path: str, unique_id: str, fqn: List[str], description: str, label: str, calculation_method: str, expression: str, filters: List[dbt.contracts.graph.unparsed.MetricFilter], time_grains: List[str], dimensions: List[str], timestamp: Optional[str] = None, window: Optional[dbt.contracts.graph.unparsed.MetricTime] = None, model: Optional[str] = None, model_unique_id: Optional[str] = None, meta: Dict[str, Any] = <factory>, tags: List[str] = <factory>, config: dbt.contracts.graph.model_config.MetricConfig = <factory>, unrendered_config: Dict[str, Any] = <factory>, sources: List[List[str]] = <factory>, depends_on: dbt.contracts.graph.nodes.DependsOn = <factory>, refs: List[List[str]] = <factory>, metrics: List[List[str]] = <factory>, created_at: float = <factory>)"
+      "description": "Metric(name: str, resource_type: dbt.node_types.NodeType, package_name: str, path: str, original_file_path: str, unique_id: str, fqn: List[str], description: str, label: str, calculation_method: str, expression: str, filters: List[dbt.contracts.graph.unparsed.MetricFilter], time_grains: List[str], dimensions: List[str], timestamp: Optional[str] = None, window: Optional[dbt.contracts.graph.unparsed.MetricTime] = None, model: Optional[str] = None, model_unique_id: Optional[str] = None, meta: Dict[str, Any] = <factory>, tags: List[str] = <factory>, config: dbt.contracts.graph.model_config.MetricConfig = <factory>, unrendered_config: Dict[str, Any] = <factory>, sources: List[List[str]] = <factory>, depends_on: dbt.contracts.graph.nodes.DependsOn = <factory>, refs: List[List[str]] = <factory>, metrics: List[List[str]] = <factory>, created_at: float = <factory>, group: Optional[str] = None)"
     },
     "MetricFilter": {
       "type": "object",

tests/functional/artifacts/test_artifacts.py

@@ -207,7 +207,8 @@
 models:
   - name: ephemeral_summary
     description: "{{ doc('ephemeral_summary') }}"
-    group: test_group
+    config:
+      group: test_group
     columns: &summary_columns
       - name: first_name
         description: "{{ doc('summary_first_name') }}"