direct: Reuse graph between plan & apply; sort plan (#3455)

denik · web-flow · commit 52578a54f3b0 · 2025-08-21T14:56:06.000Z
## Changes
- Same graph is reused between plan and apply stages. Practical
consequence is that dependencies that are implied via ${resources...}
references are always respected during deploy even if we can evaluate
the reference right away.
- The order of plan is changed to match terraform. Sorting is not
necessary in tests anymore.
- New ResourceNode type that replaces nodeKey is using Group/Key rather
than Group/Name, it's less ambiguous.

## Why
Matching terraform behavior. Users could be using ${resources...}
references to enforce dependencies.

## Tests
New regression acceptance test.
diff --git a/acceptance/bundle/resource_deps/create_error/script b/acceptance/bundle/resource_deps/create_error/script
@@ -9,9 +9,7 @@ print_sorted_requests() {
 }
 
 echo "*" > .gitignore
-# In direct, plan is sorted by deployment order. In TF it comes back sorted alphabetically.
-# I think deployment order is better, sorting here to remove the mismatch
-trace $CLI bundle plan 2>&1 | sort
+trace $CLI bundle plan 2>&1
 musterr $CLI bundle deploy &> out.deploy.$DATABRICKS_CLI_DEPLOYMENT.txt
 trace print_sorted_requests
 
@@ -21,7 +19,7 @@ ind_id=`read_id.py jobs independent`
 echo "$ind_id:IND_ID" >> ACC_REPLS
 
 title "Plan should still contain foo and bar"
-trace $CLI bundle plan  2>&1 | sort
+trace $CLI bundle plan
 
 rm out.requests.txt
 musterr $CLI bundle deploy &> out.deploy2.$DATABRICKS_CLI_DEPLOYMENT.txt
diff --git a/acceptance/bundle/resource_deps/id_chain/databricks.yml b/acceptance/bundle/resource_deps/id_chain/databricks.yml
@@ -0,0 +1,18 @@
+
+resources:
+  jobs:
+    a:
+      name: aa
+      description: aa_desc
+    b:
+      name: bb
+      description: prefix ${resources.jobs.a.id}
+    c:
+      name: cc
+      description: prefix ${resources.jobs.b.id}
+    d:
+      name: dd
+      description: prefix ${resources.jobs.c.id}
+    e:
+      name: ee
+      description: prefix ${resources.jobs.d.id}
diff --git a/acceptance/bundle/resource_deps/id_chain/out.test.toml b/acceptance/bundle/resource_deps/id_chain/out.test.toml
@@ -0,0 +1,5 @@
+Local = true
+Cloud = false
+
+[EnvMatrix]
+  DATABRICKS_CLI_DEPLOYMENT = ["terraform", "direct-exp"]
diff --git a/acceptance/bundle/resource_deps/id_chain/output.txt b/acceptance/bundle/resource_deps/id_chain/output.txt
@@ -0,0 +1,48 @@
+
+>>> [CLI] bundle plan
+create jobs.a
+create jobs.b
+create jobs.c
+create jobs.d
+create jobs.e
+
+>>> print_requests
+
+>>> [CLI] bundle deploy
+Uploading bundle files to /Workspace/Users/[USERNAME]/.bundle/test-bundle/default/files...
+Deploying resources...
+Updating deployment state...
+Deployment complete!
+
+>>> print_requests_short
+"aa"
+"bb"
+"cc"
+"dd"
+"ee"
+
+>>> update_file.py databricks.yml aa_desc aa_new_desc
+
+>>> update_file.py databricks.yml prefix new_prefix
+
+>>> [CLI] bundle plan
+update jobs.a
+update jobs.b
+update jobs.c
+update jobs.d
+update jobs.e
+
+>>> print_requests
+
+>>> [CLI] bundle deploy
+Uploading bundle files to /Workspace/Users/[USERNAME]/.bundle/test-bundle/default/files...
+Deploying resources...
+Updating deployment state...
+Deployment complete!
+
+>>> print_requests_short
+"aa"
+"bb"
+"cc"
+"dd"
+"ee"
diff --git a/acceptance/bundle/resource_deps/id_chain/script b/acceptance/bundle/resource_deps/id_chain/script
@@ -0,0 +1,29 @@
+print_all_requests() {
+    jq --sort-keys 'select(.path | contains("/jobs"))' < out.requests.txt
+    rm out.requests.txt
+}
+
+print_requests() {
+    jq --sort-keys 'select(.method != "GET" and (.path | contains("/jobs")))' < out.requests.txt
+    rm out.requests.txt
+}
+
+print_requests_short() {
+    jq --sort-keys 'select(.method != "GET" and (.path | contains("/jobs"))) | (.body.name // .body.new_settings.name // empty)' < out.requests.txt
+    rm out.requests.txt
+}
+
+trace $CLI bundle plan
+trace print_requests
+
+trace $CLI bundle deploy
+trace print_requests_short
+
+trace update_file.py databricks.yml aa_desc aa_new_desc
+trace update_file.py databricks.yml prefix new_prefix
+
+trace $CLI bundle plan
+trace print_requests
+
+trace $CLI bundle deploy
+trace print_requests_short
diff --git a/acceptance/bundle/resource_deps/jobs_update/script b/acceptance/bundle/resource_deps/jobs_update/script
@@ -9,9 +9,7 @@ print_sorted_requests() {
 }
 
 echo "*" > .gitignore
-# In direct, plan is sorted by deployment order. In TF it comes back sorted alphabetically.
-# I think deployment order is better, sorting here to remove the mismatch
-trace $CLI bundle plan 2>&1 | sort
+trace $CLI bundle plan
 trace $CLI bundle deploy
 trace print_requests
 
diff --git a/acceptance/bundle/resource_deps/pipelines_recreate/output.txt b/acceptance/bundle/resource_deps/pipelines_recreate/output.txt
@@ -49,8 +49,8 @@ Deployment complete!
 >>> update_file.py databricks.yml mycatalog mynewcatalog
 
 >>> [CLI] bundle plan
-recreate pipelines.foo
 update jobs.bar
+recreate pipelines.foo
 
 >>> [CLI] bundle deploy --auto-approve
 Uploading bundle files to /Workspace/Users/[USERNAME]/.bundle/test-bundle/default/files...
diff --git a/acceptance/bundle/resource_deps/pipelines_recreate/script b/acceptance/bundle/resource_deps/pipelines_recreate/script
@@ -9,9 +9,7 @@ print_sorted_requests() {
 }
 
 echo "*" > .gitignore
-# In direct, plan is sorted by deployment order. In TF it comes back sorted alphabetically.
-# I think deployment order is better, sorting here to remove the mismatch
-trace $CLI bundle plan 2>&1 | sort
+trace $CLI bundle plan
 trace $CLI bundle deploy
 trace print_requests
 
@@ -25,7 +23,7 @@ trace $CLI bundle plan  # empty
 
 title "Update catalog, triggering recreate for pipeline; this means updating downstream deps"
 trace update_file.py databricks.yml mycatalog mynewcatalog
-trace $CLI bundle plan 2>&1 | sort
+trace $CLI bundle plan
 trace $CLI bundle deploy --auto-approve
 trace print_requests
 
diff --git a/bundle/bundle.go b/bundle/bundle.go
@@ -22,6 +22,7 @@ import (
 	"github.com/databricks/cli/bundle/metadata"
 	"github.com/databricks/cli/bundle/terranova/tnstate"
 	"github.com/databricks/cli/libs/auth"
+	"github.com/databricks/cli/libs/dagrun"
 	"github.com/databricks/cli/libs/dyn"
 	"github.com/databricks/cli/libs/fileset"
 	"github.com/databricks/cli/libs/locker"
@@ -127,7 +128,21 @@ type Bundle struct {
 	// Stores the locker responsible for acquiring/releasing a deployment lock.
 	Locker *locker.Locker
 
-	Plan deployplan.Plan
+	// TerraformPlanPath is the path to the plan from the terraform CLI
+	TerraformPlanPath string
+
+	// If true, the plan is empty and applying it will not do anything
+	TerraformPlanIsEmpty bool
+
+	// (direct only) graph of dependencies between resources
+	Graph *dagrun.Graph[deployplan.ResourceNode]
+
+	// (direct only) planned action for each resource
+	PlannedActions map[deployplan.ResourceNode]deployplan.ActionType
+
+	// (direct only) flag that tells if a given node has references to it
+	// TODO: move this inside Graph
+	IsReferenced map[deployplan.ResourceNode]bool
 
 	// if true, we skip approval checks for deploy, destroy resources and delete
 	// files
diff --git a/bundle/deploy/terraform/apply.go b/bundle/deploy/terraform/apply.go
@@ -18,7 +18,7 @@ func (w *apply) Name() string {
 
 func (w *apply) Apply(ctx context.Context, b *bundle.Bundle) diag.Diagnostics {
 	// return early if plan is empty
-	if b.Plan.TerraformIsEmpty {
+	if b.TerraformPlanIsEmpty {
 		log.Debugf(ctx, "No changes in plan. Skipping terraform apply.")
 		return nil
 	}
@@ -28,12 +28,12 @@ func (w *apply) Apply(ctx context.Context, b *bundle.Bundle) diag.Diagnostics {
 		return diag.Errorf("terraform not initialized")
 	}
 
-	if b.Plan.TerraformPlanPath == "" {
+	if b.TerraformPlanPath == "" {
 		return diag.Errorf("no plan found")
 	}
 
 	// Apply terraform according to the computed plan
-	err := tf.Apply(ctx, tfexec.DirOrPlan(b.Plan.TerraformPlanPath))
+	err := tf.Apply(ctx, tfexec.DirOrPlan(b.TerraformPlanPath))
 	if err != nil {
 		diags := permissions.TryExtendTerraformPermissionError(ctx, b, err)
 		if diags != nil {
diff --git a/bundle/deploy/terraform/plan.go b/bundle/deploy/terraform/plan.go
@@ -5,7 +5,6 @@ import (
 	"path/filepath"
 
 	"github.com/databricks/cli/bundle"
-	"github.com/databricks/cli/bundle/deployplan"
 	"github.com/databricks/cli/libs/diag"
 	"github.com/databricks/cli/libs/log"
 	"github.com/hashicorp/terraform-exec/tfexec"
@@ -51,10 +50,8 @@ func (p *plan) Apply(ctx context.Context, b *bundle.Bundle) diag.Diagnostics {
 	}
 
 	// Set plan in main bundle struct for downstream mutators
-	b.Plan = deployplan.Plan{
-		TerraformPlanPath: planPath,
-		TerraformIsEmpty:  !notEmpty,
-	}
+	b.TerraformPlanPath = planPath
+	b.TerraformPlanIsEmpty = !notEmpty
 
 	log.Debugf(ctx, "Planning complete and persisted at %s\n", planPath)
 	return nil
diff --git a/bundle/deploy/terraform/showplanfile.go b/bundle/deploy/terraform/showplanfile.go
@@ -41,8 +41,10 @@ func GetActions(changes []*tfjson.ResourceChange) []deployplan.Action {
 
 		result = append(result, deployplan.Action{
 			ActionType: actionType,
-			Group:      group,
-			Name:       rc.Name,
+			ResourceNode: deployplan.ResourceNode{
+				Group: group,
+				Key:   rc.Name,
+			},
 		})
 	}
 
diff --git a/bundle/deployplan/action.go b/bundle/deployplan/action.go
@@ -5,30 +5,15 @@ import (
 	"strings"
 )
 
-type Plan struct {
-	// TerraformPlanPath is the path to the plan from the terraform CLI
-	TerraformPlanPath string
-
-	// If true, the plan is empty and applying it will not do anything
-	TerraformIsEmpty bool
-
-	// List of actions to apply (direct deployment)
-	Actions []Action
-}
-
 type Action struct {
-	// Resource group in the config, e.g. "jobs", "pipelines" etc
-	Group string
-
-	// Key of the resource the config
-	Name string
+	ResourceNode
 
 	ActionType ActionType
 }
 
 func (a Action) String() string {
 	typ, _ := strings.CutSuffix(a.Group, "s")
-	return fmt.Sprintf("  %s %s %s", a.ActionType, typ, a.Name)
+	return fmt.Sprintf("  %s %s %s", a.ActionType, typ, a.Key)
 }
 
 // Implements cmdio.Event for cmdio.Log
diff --git a/bundle/deployplan/node.go b/bundle/deployplan/node.go
@@ -0,0 +1,11 @@
+package deployplan
+
+type ResourceNode struct {
+	Group string
+	Key   string
+}
+
+// String implements StringerComparable
+func (n ResourceNode) String() string {
+	return n.Group + "." + n.Key
+}
diff --git a/bundle/phases/deploy.go b/bundle/phases/deploy.go
@@ -29,17 +29,18 @@ import (
 )
 
 func getActions(ctx context.Context, b *bundle.Bundle) ([]deployplan.Action, error) {
-	var actions []deployplan.Action
-	var err error
-
 	if b.DirectDeployment {
-		return terranova.CalculateDeployActions(ctx, b)
+		err := terranova.CalculatePlanForDeploy(ctx, b)
+		if err != nil {
+			return nil, err
+		}
+		return terranova.GetDeployActions(ctx, b), nil
 	} else {
 		tf := b.Terraform
 		if tf == nil {
 			return nil, errors.New("terraform not initialized")
 		}
-		actions, err = terraform.ShowPlanFile(ctx, tf, b.Plan.TerraformPlanPath)
+		actions, err := terraform.ShowPlanFile(ctx, tf, b.TerraformPlanPath)
 		return actions, err
 	}
 }
@@ -49,7 +50,6 @@ func approvalForDeploy(ctx context.Context, b *bundle.Bundle) (bool, error) {
 	if err != nil {
 		return false, err
 	}
-	b.Plan.Actions = actions
 
 	types := []deployplan.ActionType{deployplan.ActionTypeRecreate, deployplan.ActionTypeDelete}
 	schemaActions := deployplan.FilterGroup(actions, "schemas", types...)
diff --git a/bundle/phases/deploy_test.go b/bundle/phases/deploy_test.go
@@ -47,13 +47,17 @@ func TestParseTerraformActions(t *testing.T) {
 	assert.Equal(t, []deployplan.Action{
 		{
 			ActionType: deployplan.ActionTypeDelete,
-			Group:      "pipelines",
-			Name:       "delete pipeline",
+			ResourceNode: deployplan.ResourceNode{
+				Group: "pipelines",
+				Key:   "delete pipeline",
+			},
 		},
 		{
 			ActionType: deployplan.ActionTypeRecreate,
-			Group:      "pipelines",
-			Name:       "recreate pipeline",
+			ResourceNode: deployplan.ResourceNode{
+				Group: "pipelines",
+				Key:   "recreate pipeline",
+			},
 		},
 	}, res)
 }
diff --git a/bundle/phases/destroy.go b/bundle/phases/destroy.go
@@ -33,7 +33,11 @@ func assertRootPathExists(ctx context.Context, b *bundle.Bundle) (bool, error) {
 
 func getDeleteActions(ctx context.Context, b *bundle.Bundle) ([]deployplan.Action, error) {
 	if b.DirectDeployment {
-		return terranova.CalculateDestroyActions(ctx, b)
+		err := terranova.CalculatePlanForDestroy(ctx, b)
+		if err != nil {
+			return nil, err
+		}
+		return terranova.GetDeployActions(ctx, b), nil
 	}
 
 	tf := b.Terraform
@@ -42,7 +46,7 @@ func getDeleteActions(ctx context.Context, b *bundle.Bundle) ([]deployplan.Actio
 		return nil, errors.New("terraform not initialized")
 	}
 
-	actions, err := terraform.ShowPlanFile(ctx, tf, b.Plan.TerraformPlanPath)
+	actions, err := terraform.ShowPlanFile(ctx, tf, b.TerraformPlanPath)
 	if err != nil {
 		return nil, err
 	}
@@ -58,8 +62,6 @@ func approvalForDestroy(ctx context.Context, b *bundle.Bundle) (bool, error) {
 		return false, err
 	}
 
-	b.Plan.Actions = deleteActions
-
 	if len(deleteActions) > 0 {
 		cmdio.LogString(ctx, "The following resources will be deleted:")
 		for _, a := range deleteActions {
diff --git a/bundle/terranova/apply.go b/bundle/terranova/apply.go
diff --git a/bundle/terranova/graph.go b/bundle/terranova/graph.go
diff --git a/bundle/terranova/plan.go b/bundle/terranova/plan.go
diff --git a/cmd/bundle/plan.go b/cmd/bundle/plan.go

Original file line number	Diff line number	Diff line change
`@@ -9,9 +9,7 @@ print_sorted_requests() {`
`9`	`9`	`}`
`10`	`10`
`11`	`11`	`echo "*" > .gitignore`
`12`		`-# In direct, plan is sorted by deployment order. In TF it comes back sorted alphabetically.`
`13`		`-# I think deployment order is better, sorting here to remove the mismatch`
`14`		`-trace $CLI bundle plan 2>&1 \| sort`
	`12`	`+trace $CLI bundle plan`
`15`	`13`	`trace $CLI bundle deploy`
`16`	`14`	`trace print_requests`
`17`	`15`