Check live dependents of deletes reached as dependencies in --select

.nextchanges/bundles/select-delete-dependency.md

@@ -0,0 +1 @@
+* Fix `bundle deploy --select` deleting a resource that the selected resource still references when that resource was removed from the config; it now reports an error, as it does when selecting the deleted resource directly. ([#7334](https://github.com/databricks/cli/pull/7334))

acceptance/bundle/select/deletion_dependents/output.txt

@@ -19,6 +19,16 @@ Error: cannot delete jobs.producer: jobs.consumer still references it; remove th
 
 Files: 1 uploaded, 0 deleted
 
+>>> [CLI] bundle plan --select jobs.consumer
+Error: cannot delete jobs.producer (not selected, but required by the selection): jobs.consumer still references it; remove the reference or also remove jobs.consumer from the config
+
+
+>>> [CLI] bundle deploy --select jobs.consumer
+Uploading bundle files to /Workspace/Users/[USERNAME]/.bundle/select-deldep-[UNIQUE_NAME]/default/files...
+Error: cannot delete jobs.producer (not selected, but required by the selection): jobs.consumer still references it; remove the reference or also remove jobs.consumer from the config
+
+Files: 0 uploaded, 0 deleted
+
 >>> print_requests.py --sort //jobs/delete --keep
 
 >>> [CLI] bundle plan

acceptance/bundle/select/deletion_dependents/script

@@ -23,7 +23,12 @@ musterr trace $CLI bundle plan --select jobs.producer
 # bundle deploy --select also errors before sending any delete request.
 musterr trace $CLI bundle deploy --select jobs.producer
 
-# No delete request was sent by either command.
+# Selecting only consumer pulls in producer as a dependency. That delete would
+# leave consumer broken just the same, so it must error too.
+musterr trace $CLI bundle plan --select jobs.consumer
+musterr trace $CLI bundle deploy --select jobs.consumer
+
+# No delete request was sent by any of these commands.
 trace print_requests.py --sort //jobs/delete --keep
 
 # A full plan still works and shows producer as a pending delete.

bundle/deployplan/plan.go

@@ -340,8 +340,9 @@ func ResolveSelectors(resourceKeys iter.Seq[string], selectors []string) ([]stri
 // ResolveSelectors) plus the others that must change with them. The closure follows
 // edges by action, like the apply graph (makeGraph): a deployed resource pulls in its
 // dependencies (forward), a deleted one its dependents (reverse) so they are removed
-// together. A delete selection errors if a live resource (not itself being deleted)
-// still references the resource to be deleted; removing it would break the live resource.
+// together. It errors if any delete in the closure, whether selected or pulled in by
+// either edge direction, is still referenced by a live resource (not itself being
+// deleted); removing it would break the live resource.
 func (p *Plan) FilterToSelected(_ context.Context, selected []string) error {
 	before := len(p.Plan)
 
@@ -350,15 +351,9 @@ func (p *Plan) FilterToSelected(_ context.Context, selected []string) error {
 		return err
 	}
 
-	// Only build reverse edges when at least one selected resource is being deleted,
-	// since non-delete selections only follow forward DependsOn edges.
+	// Reverse edges are built lazily on the first delete visited by the BFS. That
+	// delete may be selected or reached as a dependency of a selected resource.
 	var dependents map[string][]string
-	for _, key := range resolved {
-		if e, ok := p.Plan[key]; ok && e.Action == Delete {
-			dependents = p.reverseEdges()
-			break
-		}
-	}
 
 	queue := make([]string, 0, len(selected))
 	reachable := make(map[string]struct{}, len(selected))
@@ -374,13 +369,22 @@ func (p *Plan) FilterToSelected(_ context.Context, selected []string) error {
 	}
 
 	// BFS: deleted nodes pull in their dependents (also being deleted); deployed
-	// nodes pull in their dependencies. Collect errors for live dependents of a
-	// delete selection before returning, so the user sees all conflicts at once.
-	var errs []string
+	// nodes pull in their dependencies. Collect conflicts for live dependents of every
+	// delete visited before returning, so the user sees all conflicts at once.
+	type conflict struct{ deleted, live string }
+	var conflicts []conflict
+	// Unselected deletes reached through a deployed node's forward edge. Errors for
+	// them are marked as such, since the user never named them. Deletes reached
+	// through the reverse (cascade) edge keep the plain message: there the error is
+	// about the cascade, not about the selection requiring the delete.
+	requiredBySelection := map[string]bool{}
 	for len(queue) > 0 {
 		key := queue[0]
 		queue = queue[1:]
 		if p.Plan[key].Action == Delete {
+			if dependents == nil {
+				dependents = p.reverseEdges()
+			}
 			for _, dependent := range dependents[key] {
 				// Sub-nodes (grants/permissions, 4-part key) are not user-facing dependents
 				// and never trigger the live-dependent error. If they are themselves being
@@ -396,18 +400,31 @@ func (p *Plan) FilterToSelected(_ context.Context, selected []string) error {
 					p.enqueueReachable(reachable, &queue, dependent)
 				} else {
 					// Dependent is live (still in config): deleting key would leave
-					// it broken. Report a user-facing error with both names.
-					deleted := strings.TrimPrefix(key, "resources.")
-					live := strings.TrimPrefix(dependent, "resources.")
-					errs = append(errs, fmt.Sprintf("cannot delete %s: %s still references it; remove the reference or also remove %s from the config", deleted, live, live))
+					// it broken.
+					conflicts = append(conflicts, conflict{deleted: key, live: dependent})
 				}
 			}
 		} else {
 			for _, dep := range p.Plan[key].DependsOn {
+				if entry, ok := p.Plan[dep.Node]; ok && entry.Action == Delete && !slices.Contains(resolved, dep.Node) {
+					requiredBySelection[dep.Node] = true
+				}
 				p.enqueueReachable(reachable, &queue, dep.Node)
 			}
 		}
 	}
+
+	// Format after the BFS: a delete can be reached through both edge directions,
+	// and the forward route may be found only after the delete was visited.
+	errs := make([]string, 0, len(conflicts))
+	for _, c := range conflicts {
+		deleted := strings.TrimPrefix(c.deleted, "resources.")
+		live := strings.TrimPrefix(c.live, "resources.")
+		if requiredBySelection[c.deleted] {
+			deleted += " (not selected, but required by the selection)"
+		}
+		errs = append(errs, fmt.Sprintf("cannot delete %s: %s still references it; remove the reference or also remove %s from the config", deleted, live, live))
+	}
 	if len(errs) > 0 {
 		slices.Sort(errs)
 		return errors.New(strings.Join(errs, "\n"))

bundle/deployplan/plan_filter_test.go

@@ -269,6 +269,41 @@ func TestFilterToSelected_DeletionErrorsOnLiveDependent(t *testing.T) {
 	assert.ErrorContains(t, err, "jobs.live still references it")
 }
 
+// A delete reached through a forward dependency edge is subject to the same
+// live-dependent check as a selected delete.
+func TestFilterToSelected_DependencyDeletionErrorsOnLiveDependent(t *testing.T) {
+	p := deployplan.NewPlanDirect()
+	p.Plan["resources.jobs.gone"] = &deployplan.PlanEntry{Action: deployplan.Delete}
+	p.Plan["resources.jobs.live"] = &deployplan.PlanEntry{
+		Action:    deployplan.Update,
+		DependsOn: []deployplan.DependsOnEntry{{Node: "resources.jobs.gone"}},
+	}
+
+	err := p.FilterToSelected(t.Context(), []string{"jobs.live"})
+	require.Error(t, err)
+	assert.ErrorContains(t, err, "cannot delete jobs.gone (not selected, but required by the selection)")
+	assert.ErrorContains(t, err, "jobs.live still references it")
+}
+
+// A delete reached through the cascade (reverse) edge of a selected delete keeps the
+// plain error: it is not required by the selection, it is removed along with it.
+func TestFilterToSelected_CascadeDeletionErrorsOnLiveDependent(t *testing.T) {
+	p := deployplan.NewPlanDirect()
+	p.Plan["resources.jobs.a"] = &deployplan.PlanEntry{Action: deployplan.Delete}
+	p.Plan["resources.jobs.b"] = &deployplan.PlanEntry{
+		Action:    deployplan.Delete,
+		DependsOn: []deployplan.DependsOnEntry{{Node: "resources.jobs.a"}},
+	}
+	p.Plan["resources.jobs.c"] = &deployplan.PlanEntry{
+		Action:    deployplan.Update,
+		DependsOn: []deployplan.DependsOnEntry{{Node: "resources.jobs.b"}},
+	}
+
+	err := p.FilterToSelected(t.Context(), []string{"jobs.a"})
+	require.Error(t, err)
+	assert.EqualError(t, err, "cannot delete jobs.b: jobs.c still references it; remove the reference or also remove jobs.c from the config")
+}
+
 // Selecting a resource for deletion leaves its own dependencies alone: they are not
 // pulled into the plan, so they stay deployed untouched.
 func TestFilterToSelected_DeletionLeavesDependencies(t *testing.T) {