Skip whole-block references above the key when setting defaults

acceptance/bundle/validate/references/job_cluster_whole_reference/databricks.yml

@@ -0,0 +1,34 @@
+bundle:
+  name: test-bundle
+
+resources:
+  jobs:
+    base:
+      job_clusters:
+        - job_cluster_key: main
+          new_cluster:
+            spark_version: 15.4.x-scala2.12
+            node_type_id: i3.xlarge
+            num_workers: 1
+            workload_type:
+              clients:
+                jobs: false
+    viaref:
+      job_clusters:
+        - job_cluster_key: main
+          new_cluster: ${resources.jobs.base.job_clusters[0].new_cluster}
+  clusters:
+    base_cluster:
+      cluster_name: base_cluster
+      spark_version: 15.4.x-scala2.12
+      node_type_id: i3.xlarge
+      num_workers: 1
+      workload_type:
+        clients:
+          jobs: false
+    viaref_cluster:
+      cluster_name: viaref_cluster
+      spark_version: 15.4.x-scala2.12
+      node_type_id: i3.xlarge
+      num_workers: 1
+      workload_type: ${resources.clusters.base_cluster.workload_type}

acceptance/bundle/validate/references/job_cluster_whole_reference/out.test.toml

@@ -0,0 +1,2 @@
+Cloud = false
+EnvMatrix.DMS = ["", "true"]

acceptance/bundle/validate/references/job_cluster_whole_reference/output.txt

@@ -0,0 +1,25 @@
+
+>>> [CLI] bundle validate
+Name: test-bundle
+Target: default
+Workspace:
+  User: [USERNAME]
+  Path: /Workspace/Users/[USERNAME]/.bundle/test-bundle/default
+
+Validation OK!
+
+>>> [CLI] bundle plan -o json
+{
+  "clients": {
+    "jobs": false,
+    "notebooks": true
+  }
+}
+
+>>> [CLI] bundle plan -o json
+{
+  "clients": {
+    "jobs": false,
+    "notebooks": true
+  }
+}

acceptance/bundle/validate/references/job_cluster_whole_reference/script

@@ -0,0 +1,6 @@
+# The new_cluster of viaref is a reference to the whole new_cluster of another job.
+# The defaults (e.g. workload_type.clients.notebooks) are not written into the reference; the referenced new_cluster gets them.
+trace $CLI bundle validate
+trace $CLI bundle plan -o json | jq '.plan."resources.jobs.viaref".new_state.value.job_clusters[0].new_cluster.workload_type'
+# The same holds for a cluster whose workload_type is a reference to the whole workload_type of another cluster.
+trace $CLI bundle plan -o json | jq '.plan."resources.clusters.viaref_cluster".new_state.value.workload_type'

bundle/set_default.go

@@ -2,6 +2,7 @@ package bundle
 
 import (
 	"context"
+	"errors"
 	"fmt"
 
 	"github.com/databricks/cli/libs/diag"
@@ -30,20 +31,7 @@ func (m *setDefault) Name() string {
 
 func (m *setDefault) Apply(ctx context.Context, b *Bundle) diag.Diagnostics {
 	err := b.Config.Mutate(func(v dyn.Value) (dyn.Value, error) {
-		return dyn.MapByPattern(v, m.pattern, func(p dyn.Path, v dyn.Value) (dyn.Value, error) {
-			// A default cannot be written into a pure reference to a whole block
-			// (e.g. "${resources.jobs.base.schedule}"), so leave it to the referenced value.
-			if s, ok := v.AsString(); ok && dynvar.IsPureVariableReference(s) {
-				return v, nil
-			}
-			_, err := dyn.GetByPath(v, m.key)
-			switch {
-			case dyn.IsNoSuchKeyError(err):
-				return dyn.SetByPath(v, m.key, dyn.V(m.value))
-			default:
-				return v, err
-			}
-		})
+		return m.apply(v, dyn.EmptyPath, m.pattern)
 	})
 	if err != nil {
 		return diag.FromErr(err)
@@ -52,6 +40,56 @@ func (m *setDefault) Apply(ctx context.Context, b *Bundle) diag.Diagnostics {
 	return nil
 }
 
+// setDefaultPathError prefixes an error with the full path of the value it was
+// raised on, because apply visits sub-values and dyn errors only carry the path
+// relative to the value they were raised on.
+type setDefaultPathError struct {
+	path dyn.Path
+	err  error
+}
+
+func (e setDefaultPathError) Error() string {
+	return fmt.Sprintf("%s: %s", e.path, e.err)
+}
+
+// Unwrap lets wildcard visits and callers still classify the underlying dyn error.
+func (e setDefaultPathError) Unwrap() error {
+	return e.err
+}
+
+// apply walks the pattern one component at a time so that a pure reference
+// anywhere along the way is skipped, not only where the pattern ends.
+func (m *setDefault) apply(v dyn.Value, prefix dyn.Path, pattern dyn.Pattern) (dyn.Value, error) {
+	// A default cannot be written into a pure reference to a whole block
+	// (e.g. "${resources.jobs.base.schedule}"), so leave it to the referenced value.
+	if s, ok := v.AsString(); ok && dynvar.IsPureVariableReference(s) {
+		return v, nil
+	}
+
+	var nv dyn.Value
+	var err error
+	if len(pattern) == 0 {
+		_, err = dyn.GetByPath(v, m.key)
+		if dyn.IsNoSuchKeyError(err) {
+			return dyn.SetByPath(v, m.key, dyn.V(m.value))
+		}
+		nv = v
+	} else {
+		nv, err = dyn.MapByPattern(v, pattern[:1], func(p dyn.Path, v dyn.Value) (dyn.Value, error) {
+			return m.apply(v, prefix.Append(p...), pattern[1:])
+		})
+	}
+	if err == nil || len(prefix) == 0 {
+		return nv, err
+	}
+	// Errors from deeper levels already carry their full path.
+	if _, ok := errors.AsType[setDefaultPathError](err); ok {
+		return nv, err
+	}
+	// The error is relative to v, so prefix it with the path of v itself.
+	return nv, setDefaultPathError{path: prefix, err: err}
+}
+
 func SetDefault(ctx context.Context, b *Bundle, pattern string, value any) {
 	pat, err := dyn.NewPatternFromString(pattern)
 	if err != nil {

bundle/set_default_test.go

@@ -5,6 +5,7 @@ import (
 
 	"github.com/databricks/cli/bundle/config"
 	"github.com/databricks/cli/libs/dyn"
+	"github.com/databricks/cli/libs/dyn/dynvar"
 	"github.com/databricks/cli/libs/logdiag"
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
@@ -37,3 +38,118 @@ resources:
 	require.NoError(t, err)
 	assert.Equal(t, "${resources.jobs.base.schedule}", viaref.MustString())
 }
+
+func TestSetDefaultSkipsPureReferenceAboveKey(t *testing.T) {
+	tests := []struct {
+		name    string
+		config  string
+		pattern string
+		set     string
+		ref     string
+	}{
+		{
+			name: "job_cluster_new_cluster",
+			config: `
+resources:
+  jobs:
+    base:
+      job_clusters:
+        - job_cluster_key: main
+          new_cluster:
+            workload_type:
+              clients:
+                jobs: false
+    viaref:
+      job_clusters:
+        - job_cluster_key: main
+          new_cluster: ${resources.jobs.base.job_clusters[0].new_cluster}
+`,
+			pattern: "resources.jobs.*.job_clusters[*].new_cluster.workload_type.clients.notebooks",
+			set:     "resources.jobs.base.job_clusters[0].new_cluster.workload_type.clients.notebooks",
+			ref:     "resources.jobs.viaref.job_clusters[0].new_cluster",
+		},
+		{
+			name: "cluster_workload_type",
+			config: `
+resources:
+  clusters:
+    base:
+      workload_type:
+        clients:
+          jobs: false
+    viaref:
+      workload_type: ${resources.clusters.base.workload_type}
+`,
+			pattern: "resources.clusters.*.workload_type.clients.notebooks",
+			set:     "resources.clusters.base.workload_type.clients.notebooks",
+			ref:     "resources.clusters.viaref.workload_type",
+		},
+	}
+
+	for _, tc := range tests {
+		t.Run(tc.name, func(t *testing.T) {
+			root, diags := config.LoadFromBytes("databricks.yml", []byte(tc.config))
+			require.Empty(t, diags)
+			b := &Bundle{Config: *root}
+
+			ctx := logdiag.InitContext(t.Context())
+			logdiag.SetCollect(ctx, true)
+			SetDefault(ctx, b, tc.pattern, true)
+			require.Empty(t, logdiag.FlushCollected(ctx))
+
+			v := b.Config.Value()
+			set, err := dyn.GetByPath(v, dyn.MustPathFromString(tc.set))
+			require.NoError(t, err)
+			assert.True(t, set.MustBool())
+
+			ref, err := dyn.GetByPath(v, dyn.MustPathFromString(tc.ref))
+			require.NoError(t, err)
+			assert.True(t, dynvar.IsPureVariableReference(ref.MustString()))
+		})
+	}
+}
+
+func TestSetDefaultErrorHasFullPath(t *testing.T) {
+	tests := []struct {
+		name          string
+		workloadType  dyn.Value
+		expectedError string
+	}{
+		{
+			name:          "mismatch_inside_pattern",
+			workloadType:  dyn.V("not a map"),
+			expectedError: `resources.jobs.job.job_clusters[0].new_cluster.workload_type: expected a map to index "clients", found string`,
+		},
+		{
+			name:          "mismatch_at_pattern_end",
+			workloadType:  dyn.V(map[string]dyn.Value{"clients": dyn.V("x")}),
+			expectedError: `resources.jobs.job.job_clusters[0].new_cluster.workload_type.clients: expected a map to index "notebooks", found string`,
+		},
+	}
+
+	for _, tc := range tests {
+		t.Run(tc.name, func(t *testing.T) {
+			// Built directly because loading the config would drop a wrong-kind workload_type.
+			v := dyn.V(map[string]dyn.Value{
+				"resources": dyn.V(map[string]dyn.Value{
+					"jobs": dyn.V(map[string]dyn.Value{
+						"job": dyn.V(map[string]dyn.Value{
+							"job_clusters": dyn.V([]dyn.Value{
+								dyn.V(map[string]dyn.Value{
+									"new_cluster": dyn.V(map[string]dyn.Value{
+										"workload_type": tc.workloadType,
+									}),
+								}),
+							}),
+						}),
+					}),
+				}),
+			})
+
+			pattern := dyn.MustPatternFromString("resources.jobs.*.job_clusters[*].new_cluster.workload_type.clients")
+			m := SetDefaultMutator(pattern, "notebooks", true).(*setDefault)
+			_, err := m.apply(v, dyn.EmptyPath, m.pattern)
+			assert.EqualError(t, err, tc.expectedError)
+		})
+	}
+}