structaccess: accept dyn.Path directly (#3503)

denik · web-flow · commit 451dfea026f6 · 2025-08-28T09:59:35.000+02:00
## Changes Make structaccess.Get accept dyn.Path rather than string. Follow up to #3466 ## Why I plan to check more than one struct (config and remote state) and it's good to reuse parsed path between those. ## Tests Existing tests. There is also a new test added but it's not a regression test for this PR (the bad syntax never reaches NewPathFromString), just on related topic.
diff --git a/.wsignore b/.wsignore
@@ -28,6 +28,9 @@ acceptance/bundle/templates-machinery/helpers-error/output.txt
 acceptance/bundle/apps/config_section/output.txt
 integration/bundle/testdata/apps/bundle_deploy.txt
 
+# Extra whitespace in test output:
+acceptance/bundle/resource_deps/bad_syntax/out.plan.terraform.txt
+
 # Extra whitespace in command help:
 acceptance/cmd/workspace/apps/output.txt
 
diff --git a/acceptance/bundle/resource_deps/bad_syntax/databricks.yml b/acceptance/bundle/resource_deps/bad_syntax/databricks.yml
@@ -0,0 +1,10 @@
+resources:
+  volumes:
+    bar:
+      catalog_name: mycatalog
+      schema_name: myschema
+      name: barname
+    foo:
+      catalog_name: ${resources.volumes.bar.bad..syntax}
+      schema_name: ${resources.volumes.bar.schema_name}
+      name: myname
diff --git a/acceptance/bundle/resource_deps/bad_syntax/out.deploy.direct-exp.txt b/acceptance/bundle/resource_deps/bad_syntax/out.deploy.direct-exp.txt
@@ -0,0 +1,6 @@
+Uploading bundle files to /Workspace/Users/[USERNAME]/.bundle/test-bundle/default/files...
+Deploying resources...
+Updating deployment state...
+Deployment complete!
+
+Unexpected success
diff --git a/acceptance/bundle/resource_deps/bad_syntax/out.plan.direct-exp.txt b/acceptance/bundle/resource_deps/bad_syntax/out.plan.direct-exp.txt
@@ -0,0 +1,2 @@
+create volumes.bar
+create volumes.foo
diff --git a/acceptance/bundle/resource_deps/bad_syntax/out.plan.terraform.txt b/acceptance/bundle/resource_deps/bad_syntax/out.plan.terraform.txt
@@ -0,0 +1,10 @@
+Error: exit status 1
+
+Error: Invalid attribute name
+
+  on bundle.tf.json line 22, in resource.databricks_volume.foo:
+  22:         "catalog_name": "${resources.volumes.bar.bad..syntax}",
+
+An attribute name is required after a dot.
+
+
diff --git a/acceptance/bundle/resource_deps/bad_syntax/out.test.toml b/acceptance/bundle/resource_deps/bad_syntax/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/bad_syntax/output.txt b/acceptance/bundle/resource_deps/bad_syntax/output.txt
@@ -0,0 +1,20 @@
+
+>>> [CLI] bundle validate -o json
+{
+  "volumes": {
+    "bar": {
+      "catalog_name": "mycatalog",
+      "name": "barname",
+      "schema_name": "myschema",
+      "volume_type": "MANAGED"
+    },
+    "foo": {
+      "catalog_name": "${resources.volumes.bar.bad..syntax}",
+      "name": "myname",
+      "schema_name": "${resources.volumes.bar.schema_name}",
+      "volume_type": "MANAGED"
+    }
+  }
+}
+
+Exit code: 1
diff --git a/acceptance/bundle/resource_deps/bad_syntax/script b/acceptance/bundle/resource_deps/bad_syntax/script
@@ -0,0 +1,3 @@
+trace $CLI bundle validate -o json | jq .resources
+$CLI bundle plan &> out.plan.$DATABRICKS_CLI_DEPLOYMENT.txt
+musterr $CLI bundle deploy &> out.deploy.$DATABRICKS_CLI_DEPLOYMENT.txt
diff --git a/acceptance/bundle/resource_deps/bad_syntax/test.toml b/acceptance/bundle/resource_deps/bad_syntax/test.toml
@@ -0,0 +1,2 @@
+Badness = "In validate we dont detect issue; in direct we dont detect syntax issue, skipping the reference completely"
+RecordRequests = false
diff --git a/bundle/terranova/plan.go b/bundle/terranova/plan.go
@@ -11,6 +11,7 @@ import (
 	"github.com/databricks/cli/bundle/deployplan"
 	"github.com/databricks/cli/bundle/terranova/tnstate"
 	"github.com/databricks/cli/libs/dagrun"
+	"github.com/databricks/cli/libs/dyn"
 	"github.com/databricks/cli/libs/dyn/dynvar"
 	"github.com/databricks/cli/libs/logdiag"
 	"github.com/databricks/cli/libs/structaccess"
@@ -171,7 +172,12 @@ func CalculatePlanForDeploy(ctx context.Context, b *bundle.Bundle) error {
 				}
 				continue
 			}
-			value, err := structaccess.Get(config, fieldPath)
+			dynPath, err := dyn.NewPathFromString(fieldPath)
+			if err != nil {
+				logdiag.LogError(ctx, fmt.Errorf("cannot parse path %s: %w", fieldPath, err))
+				return false
+			}
+			value, err := structaccess.Get(config, dynPath)
 			if err != nil {
 				logdiag.LogError(ctx, fmt.Errorf("cannot resolve %s: %w", reference, err))
 				return false
diff --git a/libs/structaccess/bundle_test.go b/libs/structaccess/bundle_test.go
@@ -44,27 +44,27 @@ func TestGet_ConfigRoot_JobTagsAccess(t *testing.T) {
 	}
 
 	// Access a value inside the tags map
-	v, err := Get(root, "resources.jobs.my_job.tags.env")
+	v, err := GetByString(root, "resources.jobs.my_job.tags.env")
 	require.NoError(t, err)
 	require.Equal(t, "dev", v)
 
 	// Leading dot is allowed
-	v, err = Get(root, ".resources.jobs.my_job.tags.team")
+	v, err = GetByString(root, ".resources.jobs.my_job.tags.team")
 	require.NoError(t, err)
 	require.Equal(t, "platform", v)
 
 	// Access into first task
-	v, err = Get(root, "resources.jobs.my_job.tasks[0].task_key")
+	v, err = GetByString(root, "resources.jobs.my_job.tasks[0].task_key")
 	require.NoError(t, err)
 	require.Equal(t, "t1", v)
 
 	// Test index access
-	v, err = Get(root, "resources.jobs.my_job.tasks[0].notebook_task.notebook_path")
+	v, err = GetByString(root, "resources.jobs.my_job.tasks[0].notebook_task.notebook_path")
 	require.NoError(t, err)
 	require.Equal(t, "/Workspace/Users/user@example.com/nb", v)
 
 	// Test ambiguous field access
-	v, err = Get(root, "resources.apps.my_app.url")
+	v, err = GetByString(root, "resources.apps.my_app.url")
 	require.NoError(t, err)
 	require.Equal(t, "app_inner_url", v)
 }
diff --git a/libs/structaccess/get.go b/libs/structaccess/get.go
@@ -9,7 +9,8 @@ import (
 	"github.com/databricks/cli/libs/structdiff/structtag"
 )
 
-// Get returns the value at the given path inside v.
+// GetByString returns the value at the given path inside v.
+// This is a convenience function that parses the path string and calls Get.
 //
 // Path grammar (compatible with dyn path):
 //   - Struct field names and map keys separated by '.' (e.g., connection.id)
@@ -23,19 +24,28 @@ import (
 //   - For maps: a key indexes map[string]T (or string alias key types).
 //   - For slices/arrays: an index [N] selects the N-th element.
 //   - Wildcards ("*" or "[*]") are not supported and return an error.
-func Get(v any, path string) (any, error) {
+func GetByString(v any, path string) (any, error) {
 	if path == "" {
 		return v, nil
 	}
 
-	p, err := dyn.NewPathFromString(path)
+	dynPath, err := dyn.NewPathFromString(path)
 	if err != nil {
 		return nil, err
 	}
 
+	return Get(v, dynPath)
+}
+
+// Get returns the value at the given path inside v.
+func Get(v any, path dyn.Path) (any, error) {
+	if len(path) == 0 {
+		return v, nil
+	}
+
 	cur := reflect.ValueOf(v)
 	prefix := ""
-	for _, c := range p {
+	for _, c := range path {
 		var ok bool
 		cur, ok = deref(cur)
 		if !ok {
diff --git a/libs/structaccess/get_test.go b/libs/structaccess/get_test.go
@@ -219,7 +219,7 @@ func runCommonTests(t *testing.T, obj any) {
 
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {
-			got, err := Get(obj, tt.path)
+			got, err := GetByString(obj, tt.path)
 			if tt.errFmt != "" {
 				require.EqualError(t, err, tt.errFmt)
 				return
@@ -316,7 +316,7 @@ func runOmitEmptyTests(t *testing.T, obj any, wantNil bool) {
 
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {
-			got, err := Get(obj, tt.path)
+			got, err := GetByString(obj, tt.path)
 			require.NoError(t, err)
 			require.Equal(t, tt.want, got)
 		})
@@ -330,7 +330,7 @@ func TestGet_Embedded_NilPointerAnonymousNotDescended(t *testing.T) {
 	type host struct {
 		*embedded
 	}
-	_, err := Get(host{}, "hidden")
+	_, err := GetByString(host{}, "hidden")
 	typeName := reflect.TypeOf(host{}).String()
 	require.EqualError(t, err, "hidden: field \"hidden\" not found in "+typeName)
 }
@@ -343,14 +343,14 @@ func TestGet_Embedded_ValueAnonymousResolved(t *testing.T) {
 		embedded
 	}
 	in := host{embedded: embedded{Hidden: "x"}}
-	got, err := Get(in, "hidden")
+	got, err := GetByString(in, "hidden")
 	require.NoError(t, err)
 	require.Equal(t, "x", got)
 }
 
 func TestGet_InterfaceRoot_Unwraps(t *testing.T) {
 	v := any(makeOuterNoFSF())
-	got, err := Get(v, "items[0].id")
+	got, err := GetByString(v, "items[0].id")
 	require.NoError(t, err)
 	require.Equal(t, "i0", got)
 }
@@ -363,15 +363,15 @@ func TestGet_BundleTag_SkipsDirect(t *testing.T) {
 	}
 
 	// Direct readonly/internal fields should be invisible
-	_, err := Get(S{A: "x", B: "y", C: "z"}, "a")
+	_, err := GetByString(S{A: "x", B: "y", C: "z"}, "a")
 	typeName := reflect.TypeOf(S{}).String()
 	require.EqualError(t, err, "a: field \"a\" not found in "+typeName)
 
-	_, err = Get(S{}, "b")
+	_, err = GetByString(S{}, "b")
 	require.EqualError(t, err, "b: field \"b\" not found in "+typeName)
 
 	// Visible field works
-	v, err := Get(S{C: "z"}, "c")
+	v, err := GetByString(S{C: "z"}, "c")
 	require.NoError(t, err)
 	require.Equal(t, "z", v)
 }
@@ -384,7 +384,7 @@ func TestGet_BundleTag_SkipsPromoted(t *testing.T) {
 		embedded
 	}
 	// Promoted readonly field should be invisible
-	_, err := Get(host{embedded: embedded{Hidden: "x"}}, "hidden")
+	_, err := GetByString(host{embedded: embedded{Hidden: "x"}}, "hidden")
 	typeName := reflect.TypeOf(host{}).String()
 	require.EqualError(t, err, "hidden: field \"hidden\" not found in "+typeName)
 }

Original file line number	Diff line number	Diff line change
`@@ -0,0 +1,2 @@`
	`1`	`+create volumes.bar`
	`2`	`+create volumes.foo`
Original file line number	Diff line number	Diff line change
`@@ -0,0 +1,3 @@`
	`1`	`+trace $CLI bundle validate -o json \| jq .resources`
	`2`	`+$CLI bundle plan &> out.plan.$DATABRICKS_CLI_DEPLOYMENT.txt`
	`3`	`+musterr $CLI bundle deploy &> out.deploy.$DATABRICKS_CLI_DEPLOYMENT.txt`
Original file line number	Diff line number	Diff line change
`@@ -0,0 +1,2 @@`
	`1`	`+Badness = "In validate we dont detect issue; in direct we dont detect syntax issue, skipping the reference completely"`
	`2`	`+RecordRequests = false`