Pass copy of `dyn.Path` to callback function (#1747)

## Changes

Some call sites hold on to the `dyn.Path` provided to them by the
callback. It must therefore never be mutated after the callback returns,
or these mutations leak out into unknown scope.

This change means it is no longer possible for this failure mode to
happen.

## Tests

Unit test.

CHANGELOG.md

@@ -1,5 +1,34 @@
 # Version changelog
 
+## [Release] Release v0.228.0
+
+CLI:
+ * Do not error if we cannot prompt for a profile in `auth login` ([#1745](https://github.com/databricks/cli/pull/1745)).
+
+Bundles:
+
+As of this release, the CLI will show a prompt if there are configuration changes that lead to DLT pipeline recreation.
+Users can skip the prompt by specifying the `--auto-approve` flag.
+
+ * Pass along  to Terraform process ([#1734](https://github.com/databricks/cli/pull/1734)).
+ * Add prompt when a pipeline recreation happens ([#1672](https://github.com/databricks/cli/pull/1672)).
+ * Use materialized views in the default-sql template ([#1709](https://github.com/databricks/cli/pull/1709)).
+ * Update templates to latest LTS DBR ([#1715](https://github.com/databricks/cli/pull/1715)).
+ * Make lock optional in the JSON schema ([#1738](https://github.com/databricks/cli/pull/1738)).
+ * Do not suppress normalisation diagnostics for resolving variables ([#1740](https://github.com/databricks/cli/pull/1740)).
+ * Include a permissions section in all templates ([#1713](https://github.com/databricks/cli/pull/1713)).
+ * Fixed complex variables are not being correctly merged from include files ([#1746](https://github.com/databricks/cli/pull/1746)).
+ * Fixed variable override in target with full variable syntax ([#1749](https://github.com/databricks/cli/pull/1749)).
+
+Internal:
+ * Consider serverless clusters as compatible for Python wheel tasks ([#1733](https://github.com/databricks/cli/pull/1733)).
+ * PythonMutator: explain missing package error ([#1736](https://github.com/databricks/cli/pull/1736)).
+ * Add `dyn.Time` to box a timestamp with its original string value ([#1732](https://github.com/databricks/cli/pull/1732)).
+ * Fix streaming of stdout, stdin, stderr in cobra test runner ([#1742](https://github.com/databricks/cli/pull/1742)).
+
+Dependency updates:
+ * Bump github.com/Masterminds/semver/v3 from 3.2.1 to 3.3.0 ([#1741](https://github.com/databricks/cli/pull/1741)).
+
 ## [Release] Release v0.227.1
 
 CLI:

bundle/artifacts/expand_globs.go

@@ -33,12 +33,7 @@ func createGlobError(v dyn.Value, p dyn.Path, message string) diag.Diagnostic {
 		Severity:  diag.Error,
 		Summary:   fmt.Sprintf("%s: %s", source, message),
 		Locations: []dyn.Location{v.Location()},
-
-		Paths: []dyn.Path{
-			// Hack to clone the path. This path copy is mutable.
-			// To be addressed in a later PR.
-			p.Append(),
-		},
+		Paths:     []dyn.Path{p},
 	}
 }
 

bundle/config/validate/unique_resource_keys.go

@@ -3,7 +3,6 @@ package validate
 import (
 	"context"
 	"fmt"
-	"slices"
 	"sort"
 
 	"github.com/databricks/cli/bundle"
@@ -66,10 +65,7 @@ func (m *uniqueResourceKeys) Apply(ctx context.Context, b *bundle.Bundle) diag.D
 				}
 			}
 
-			// dyn.Path under the hood is a slice. The code that walks the configuration
-			// tree uses the same underlying slice to track the path as it walks
-			// the tree. So, we need to clone it here.
-			m.paths = append(m.paths, slices.Clone(p))
+			m.paths = append(m.paths, p)
 			m.locations = append(m.locations, v.Locations()...)
 
 			resourceMetadata[k] = m

bundle/libraries/expand_glob_references.go

@@ -16,12 +16,10 @@ type expand struct {
 
 func matchError(p dyn.Path, l []dyn.Location, message string) diag.Diagnostic {
 	return diag.Diagnostic{
-		Severity: diag.Error,
-		Summary:  message,
-		Paths: []dyn.Path{
-			p.Append(),
-		},
+		Severity:  diag.Error,
+		Summary:   message,
 		Locations: l,
+		Paths:     []dyn.Path{p},
 	}
 }
 

bundle/libraries/upload.go

@@ -76,7 +76,7 @@ func collectLocalLibraries(b *bundle.Bundle) (map[string][]configLocation, error
 
 				source = filepath.Join(b.RootPath, source)
 				libs[source] = append(libs[source], configLocation{
-					configPath: p.Append(), // Hack to get the copy of path
+					configPath: p,
 					location:   v.Location(),
 				})
 

libs/dyn/visit.go

@@ -70,7 +70,7 @@ type visitOptions struct {
 
 func visit(v Value, prefix Path, suffix Pattern, opts visitOptions) (Value, error) {
 	if len(suffix) == 0 {
-		return opts.fn(prefix, v)
+		return opts.fn(slices.Clone(prefix), v)
 	}
 
 	// Initialize prefix if it is empty.

libs/dyn/visit_map.go

@@ -21,7 +21,7 @@ func Foreach(fn MapFunc) MapFunc {
 			for _, pair := range m.Pairs() {
 				pk := pair.Key
 				pv := pair.Value
-				nv, err := fn(append(p, Key(pk.MustString())), pv)
+				nv, err := fn(p.Append(Key(pk.MustString())), pv)
 				if err != nil {
 					return InvalidValue, err
 				}
@@ -32,7 +32,7 @@ func Foreach(fn MapFunc) MapFunc {
 			s := slices.Clone(v.MustSequence())
 			for i, value := range s {
 				var err error
-				s[i], err = fn(append(p, Index(i)), value)
+				s[i], err = fn(p.Append(Index(i)), value)
 				if err != nil {
 					return InvalidValue, err
 				}

libs/dyn/visit_test.go

@@ -0,0 +1,36 @@
+package dyn_test
+
+import (
+	"testing"
+
+	"github.com/databricks/cli/libs/dyn"
+	assert "github.com/databricks/cli/libs/dyn/dynassert"
+)
+
+func TestVisitCallbackPathCopy(t *testing.T) {
+	vin := dyn.V(map[string]dyn.Value{
+		"foo": dyn.V(42),
+		"bar": dyn.V(43),
+	})
+
+	var paths []dyn.Path
+
+	// The callback should receive a copy of the path.
+	// If the same underlying value is used, all collected paths will be the same.
+	// This test uses `MapByPattern` to collect all paths in the map.
+	// Visit itself doesn't have public functions and we exclusively use black-box testing for this package.
+	_, _ = dyn.MapByPattern(vin, dyn.NewPattern(dyn.AnyKey()), func(p dyn.Path, v dyn.Value) (dyn.Value, error) {
+		paths = append(paths, p)
+		return v, nil
+	})
+
+	// Verify that the paths retained their original values.
+	var strings []string
+	for _, p := range paths {
+		strings = append(strings, p.String())
+	}
+	assert.ElementsMatch(t, strings, []string{
+		"foo",
+		"bar",
+	})
+}