direct: internal: simplify makeResourceGraph function (#3450)

More clear responsibility of makeResourceGraph.

Also doing more validation on planned actions there.

Author: denik

bundle/terranova/apply.go

@@ -14,7 +14,6 @@ import (
 	"github.com/databricks/cli/libs/diag"
 	"github.com/databricks/cli/libs/log"
 	"github.com/databricks/cli/libs/logdiag"
-	"github.com/databricks/cli/libs/utils"
 	"github.com/databricks/databricks-sdk-go"
 )
 
@@ -37,32 +36,32 @@ func (m *terranovaApplyMutator) Apply(ctx context.Context, b *bundle.Bundle) dia
 		return nil
 	}
 
+	g, isReferenced, err := makeResourceGraph(ctx, b)
+	if err != nil {
+		logdiag.LogError(ctx, fmt.Errorf("error while reading config: %w", err))
+	}
+
 	// Maps node key to originally planned action
 	plannedActionsMap := map[nodeKey]deployplan.ActionType{}
 
 	for _, action := range b.Plan.Actions {
+		node := nodeKey{action.Group, action.Name}
 		plannedActionsMap[nodeKey{action.Group, action.Name}] = action.ActionType
-	}
-
-	state := b.ResourceDatabase.ExportState(ctx)
-	g, isReferenced, err := makeResourceGraph(ctx, b, state)
-	if err != nil {
-		logdiag.LogError(ctx, fmt.Errorf("error while reading config: %w", err))
-	}
-
-	// Remained in state are resources that no longer present in the config
-	for _, group := range utils.SortedKeys(state) {
-		groupData := state[group]
-		for _, name := range utils.SortedKeys(groupData) {
-			n := nodeKey{group, name}
-			g.AddNode(n)
-			if plannedActionsMap[n] != deployplan.ActionTypeDelete {
-				logdiag.LogError(ctx, fmt.Errorf("internal error, resources %s.%s is missing from state but action is not delete but %v", group, name, plannedActionsMap[n]))
-				return nil
+		if !g.HasNode(node) {
+			if action.ActionType == deployplan.ActionTypeDelete {
+				// it is expected that this node is not seen by makeResourceGraph
+				g.AddNode(node)
+			} else {
+				// it's internal error today because plan cannot be outdated. In the future when we load serialized plan, this will become user error
+				logdiag.LogError(ctx, fmt.Errorf("cannot %s %s.%s: internal error, plan is outdated", action.ActionType, action.Group, action.Name))
 			}
 		}
 	}
 
+	if logdiag.HasError(ctx) {
+		return nil
+	}
+
 	err = g.DetectCycle()
 	if err != nil {
 		return diag.FromErr(err)

bundle/terranova/graph.go

@@ -8,7 +8,6 @@ import (
 	"strings"
 
 	"github.com/databricks/cli/bundle"
-	"github.com/databricks/cli/bundle/statemgmt/resourcestate"
 	"github.com/databricks/cli/libs/dagrun"
 	"github.com/databricks/cli/libs/dyn"
 	"github.com/databricks/cli/libs/dyn/convert"
@@ -38,8 +37,7 @@ type fieldRef struct {
 
 // makeResourceGraph creates node graph based on ${resources.group.name.id} references.
 // Returns a graph and a map of all references that have references to them
-// Modifies 'state' in place: all the resources found in the config are removed from state
-func makeResourceGraph(ctx context.Context, b *bundle.Bundle, state resourcestate.ExportedResourcesMap) (*dagrun.Graph[nodeKey], map[nodeKey]bool, error) {
+func makeResourceGraph(ctx context.Context, b *bundle.Bundle) (*dagrun.Graph[nodeKey], map[nodeKey]bool, error) {
 	isReferenced := make(map[nodeKey]bool)
 	g := dagrun.NewGraph[nodeKey]()
 
@@ -74,9 +72,6 @@ func makeResourceGraph(ctx context.Context, b *bundle.Bundle, state resourcestat
 	})
 
 	for _, node := range nodes {
-		groupState := state[node.Group]
-		delete(groupState, node.Name)
-
 		g.AddNode(node)
 
 		fieldRefs, err := extractReferences(b.Config.Value(), node)

bundle/terranova/plan.go

@@ -78,9 +78,7 @@ func CalculateDeployActions(ctx context.Context, b *bundle.Bundle) ([]deployplan
 
 	client := b.WorkspaceClient()
 
-	state := b.ResourceDatabase.ExportState(ctx)
-
-	g, isReferenced, err := makeResourceGraph(ctx, b, state)
+	g, isReferenced, err := makeResourceGraph(ctx, b)
 	if err != nil {
 		return nil, fmt.Errorf("reading config: %w", err)
 	}
@@ -90,13 +88,19 @@ func CalculateDeployActions(ctx context.Context, b *bundle.Bundle) ([]deployplan
 		return nil, err
 	}
 
+	state := b.ResourceDatabase.ExportState(ctx)
+
 	// Remained in state are resources that no longer present in the config
 	for _, group := range utils.SortedKeys(state) {
 		groupData := state[group]
-		for _, name := range utils.SortedKeys(groupData) {
+		for _, key := range utils.SortedKeys(groupData) {
+			if g.HasNode(nodeKey{group, key}) {
+				// alive resources are processed by makeResourceGraph
+				continue
+			}
 			actions = append(actions, deployplan.Action{
 				Group:      group,
-				Name:       name,
+				Name:       key,
 				ActionType: deployplan.ActionTypeDelete,
 			})
 		}

libs/dagrun/dagrun.go

@@ -36,6 +36,11 @@ func (g *Graph[N]) AddNode(n N) {
 	}
 }
 
+func (g *Graph[N]) HasNode(n N) bool {
+	_, ok := g.adj[n]
+	return ok
+}
+
 func (g *Graph[N]) AddDirectedEdge(from, to N, label string) {
 	g.AddNode(from)
 	g.AddNode(to)