bundle: read and edit the typed config in five mutators

acceptance/bundle/validate/secret_scope_invalid_permission_level/output.txt

@@ -2,9 +2,11 @@
 >>> [CLI] bundle validate
 Warning: invalid value "12" for enum field. Valid values are [READ WRITE MANAGE]
   at resources.secret_scopes.bad_levels.permissions[0].level
+  in databricks.yml:10:18
 
 Warning: invalid value "fuzz_badlevel" for enum field. Valid values are [READ WRITE MANAGE]
   at resources.secret_scopes.bad_levels.permissions[1].level
+  in databricks.yml:12:18
 
 Name: test-bundle
 Target: default
@@ -17,9 +19,11 @@ Found 2 warnings
 >>> [CLI] bundle validate --strict
 Warning: invalid value "12" for enum field. Valid values are [READ WRITE MANAGE]
   at resources.secret_scopes.bad_levels.permissions[0].level
+  in databricks.yml:10:18
 
 Warning: invalid value "fuzz_badlevel" for enum field. Valid values are [READ WRITE MANAGE]
   at resources.secret_scopes.bad_levels.permissions[1].level
+  in databricks.yml:12:18
 
 Name: test-bundle
 Target: default

bundle/config/loader/process_root_includes.go

@@ -10,7 +10,6 @@ import (
 	"github.com/databricks/cli/bundle"
 	"github.com/databricks/cli/bundle/config"
 	"github.com/databricks/cli/libs/diag"
-	"github.com/databricks/cli/libs/dyn"
 )
 
 type processRootIncludes struct{}
@@ -42,7 +41,8 @@ func (m *processRootIncludes) Apply(ctx context.Context, b *bundle.Bundle) diag.
 
 	// Preserve the raw include patterns before they are replaced below with the
 	// expanded list of loaded files, so IsFileIncluded can re-match against them.
-	b.SetIncludePatterns(b.Config.Include)
+	patterns := b.Config.Include
+	b.SetIncludePatterns(patterns)
 
 	// Map with files we've already seen to avoid loading them twice.
 	seen := map[string]bool{}
@@ -131,27 +131,6 @@ func (m *processRootIncludes) Apply(ctx context.Context, b *bundle.Bundle) diag.
 		}
 	}
 
-	// Swap out the original includes list with the expanded globs. This goes through
-	// Mutate so the dynamic tree is updated too: the includes below are applied without
-	// their own mutator scope, so nothing converts the typed field back into the dynamic
-	// tree afterwards, and the next ToTyped would otherwise restore the raw patterns.
-	err := b.Config.Mutate(func(root dyn.Value) (dyn.Value, error) {
-		// Include is omitempty in the typed configuration, so an empty list must stay
-		// absent from the dynamic tree rather than be written as [].
-		if len(files) == 0 {
-			return dyn.DropKeys(root, []string{"include"})
-		}
-
-		includeValues := make([]dyn.Value, 0, len(files))
-		for _, file := range files {
-			includeValues = append(includeValues, dyn.V(file))
-		}
-		return dyn.Set(root, "include", dyn.NewValue(includeValues, root.Get("include").Locations()))
-	})
-	if err != nil {
-		return diag.FromErr(err)
-	}
-
 	// Track number of bundle YAML (or JSON) files in the configuration. The +1 is there
 	// to account for the root databricks.yaml file.
 	b.Metrics.ConfigurationFileCount = int64(len(files)) + 1
@@ -161,5 +140,12 @@ func (m *processRootIncludes) Apply(ctx context.Context, b *bundle.Bundle) diag.
 	// whole accumulated configuration per file, making load quadratic in the number of
 	// included files (~20 minutes for 6000 files).
 	bundle.ApplySeqInScopeContext(ctx, b, out...)
+
+	// Swap out the original include patterns with the expanded globs. This is done after
+	// the includes are merged: [config.Root.Merge] reloads the typed configuration from
+	// the dynamic tree, which still holds the patterns, followed by the include sections
+	// of included files (merging concatenates sequences). Those are kept as they are.
+	// Include is omitempty, so an empty list is absent from the configuration.
+	b.Config.Include = append(files, b.Config.Include[len(patterns):]...)
 	return nil
 }

bundle/config/loader/process_root_includes_test.go

@@ -1,6 +1,7 @@
 package loader_test
 
 import (
+	"path/filepath"
 	"runtime"
 	"testing"
 
@@ -100,8 +101,8 @@ func TestProcessRootIncludesRemoveDups(t *testing.T) {
 }
 
 // The expanded include list must be visible in both the typed and the dynamic
-// configuration: the per-file includes are applied without their own mutator scope, so
-// nothing converts the typed field back into the dynamic tree afterwards.
+// configuration: merging the per-file includes reloads the typed configuration from the
+// dynamic tree, so the expanded list must not be overwritten by the raw patterns.
 func TestProcessRootIncludesUpdatesDynamicValue(t *testing.T) {
 	b := &bundle.Bundle{
 		BundleRootPath: t.TempDir(),
@@ -142,6 +143,28 @@ func TestProcessRootIncludesEmptyOmitsDynamicValue(t *testing.T) {
 	assert.Equal(t, dyn.KindInvalid, b.Config.Value().Get("include").Kind())
 }
 
+// Include sections of included files are not applied, but they are merged into the
+// include list after the expanded globs.
+func TestProcessRootIncludesKeepsIncludesOfIncludedFiles(t *testing.T) {
+	b := &bundle.Bundle{
+		BundleRootPath: t.TempDir(),
+		Config: config.Root{
+			Include: []string{
+				"*.yml",
+			},
+		},
+	}
+
+	testutil.Touch(t, b.BundleRootPath, "databricks.yml")
+	testutil.WriteFile(t, filepath.Join(b.BundleRootPath, "a.yml"), "include:\n  - c.yml\n")
+	testutil.Touch(t, b.BundleRootPath, "b.yml")
+
+	diags := bundle.Apply(t.Context(), b, loader.ProcessRootIncludes())
+	require.NoError(t, diags.Error())
+	assert.Equal(t, []string{"a.yml", "b.yml", "c.yml"}, b.Config.Include)
+	assert.Equal(t, []any{"a.yml", "b.yml", "c.yml"}, b.Config.Value().Get("include").AsAny())
+}
+
 func TestProcessRootIncludesNotExists(t *testing.T) {
 	b := &bundle.Bundle{
 		BundleRootPath: t.TempDir(),

bundle/config/mutator/prepend_workspace_prefix.go

@@ -2,12 +2,10 @@ package mutator
 
 import (
 	"context"
-	"fmt"
 	"strings"
 
 	"github.com/databricks/cli/bundle"
 	"github.com/databricks/cli/libs/diag"
-	"github.com/databricks/cli/libs/dyn"
 )
 
 type prependWorkspacePrefix struct{}
@@ -27,46 +25,31 @@ var skipPrefixes = []string{
 }
 
 func (m *prependWorkspacePrefix) Apply(ctx context.Context, b *bundle.Bundle) diag.Diagnostics {
-	patterns := []dyn.Pattern{
-		dyn.NewPattern(dyn.Key("workspace"), dyn.Key("root_path")),
-		dyn.NewPattern(dyn.Key("workspace"), dyn.Key("file_path")),
-		dyn.NewPattern(dyn.Key("workspace"), dyn.Key("artifact_path")),
-		dyn.NewPattern(dyn.Key("workspace"), dyn.Key("state_path")),
-		dyn.NewPattern(dyn.Key("workspace"), dyn.Key("resource_path")),
+	w := &b.Config.Workspace
+	for _, path := range []*string{
+		&w.RootPath,
+		&w.FilePath,
+		&w.ArtifactPath,
+		&w.StatePath,
+		&w.ResourcePath,
+	} {
+		*path = prependWorkspacePrefixTo(*path)
 	}
 
-	err := b.Config.Mutate(func(v dyn.Value) (dyn.Value, error) {
-		var err error
-		for _, pattern := range patterns {
-			v, err = dyn.MapByPattern(v, pattern, func(p dyn.Path, pv dyn.Value) (dyn.Value, error) {
-				path, ok := pv.AsString()
-				if !ok {
-					return dyn.InvalidValue, fmt.Errorf("expected string, got %s", pv.Kind())
-				}
-
-				// Skip prefixing if the path does not start with /, it might be variable reference or smth else.
-				if !strings.HasPrefix(path, "/") {
-					return pv, nil
-				}
+	return nil
+}
 
-				for _, prefix := range skipPrefixes {
-					if strings.HasPrefix(path, prefix) {
-						return pv, nil
-					}
-				}
+func prependWorkspacePrefixTo(path string) string {
+	// Skip prefixing if the path does not start with /, it might be variable reference or smth else.
+	if !strings.HasPrefix(path, "/") {
+		return path
+	}
 
-				// Use pv's locations, not the root v's, so diagnostics point at the original config line.
-				return dyn.NewValue("/Workspace"+path, pv.Locations()), nil
-			})
-			if err != nil {
-				return dyn.InvalidValue, err
-			}
+	for _, prefix := range skipPrefixes {
+		if strings.HasPrefix(path, prefix) {
+			return path
 		}
-		return v, nil
-	})
-	if err != nil {
-		return diag.FromErr(err)
 	}
 
-	return nil
+	return "/Workspace" + path
 }

bundle/config/mutator/resourcemutator/apply_bundle_permissions.go

@@ -3,6 +3,8 @@ package resourcemutator
 import (
 	"context"
 	"fmt"
+	"maps"
+	"reflect"
 	"slices"
 	"strings"
 
@@ -11,8 +13,6 @@ import (
 	"github.com/databricks/cli/bundle"
 	"github.com/databricks/cli/bundle/config/resources"
 	"github.com/databricks/cli/libs/diag"
-	"github.com/databricks/cli/libs/dyn"
-	"github.com/databricks/cli/libs/dyn/convert"
 	"github.com/databricks/cli/libs/logdiag"
 	"github.com/databricks/databricks-sdk-go/service/iam"
 )
@@ -108,63 +108,65 @@ func (m *bundlePermissions) Apply(ctx context.Context, b *bundle.Bundle) diag.Di
 		return diag.FromErr(err)
 	}
 
-	patterns := make(map[string]dyn.Pattern, 0)
-	for key := range levelsMap {
-		patterns[key] = dyn.NewPattern(
-			dyn.Key("resources"),
-			dyn.Key(key),
-			dyn.AnyKey(),
-		)
-	}
-
-	err = b.Config.Mutate(func(v dyn.Value) (dyn.Value, error) {
-		for key, pattern := range patterns {
-			v, err = dyn.MapByPattern(v, pattern, func(p dyn.Path, v dyn.Value) (dyn.Value, error) {
-				var permissions []resources.Permission
-				pv, err := dyn.Get(v, "permissions")
-				// If the permissions field is not found, we set to an empty array
-				if err != nil {
-					pv = dyn.V([]dyn.Value{})
-				}
-
-				err = convert.ToTyped(&permissions, pv)
-				if err != nil {
-					return dyn.InvalidValue, fmt.Errorf("failed to convert permissions: %w", err)
-				}
-
-				permissions = append(permissions, convertPermissions(
-					ctx,
-					b.Config.Permissions,
-					permissions,
-					key,
-					levelsMap[key],
-				)...)
-
-				if len(permissions) == 0 {
-					permissions = nil
-				}
-
-				pv, err = convert.FromTyped(permissions, dyn.NilValue)
-				if err != nil {
-					return dyn.InvalidValue, fmt.Errorf("failed to convert permissions: %w", err)
-				}
-
-				return dyn.Set(v, "permissions", pv)
-			})
-			if err != nil {
-				return dyn.InvalidValue, err
-			}
+	for _, group := range b.Config.Resources.AllResources() {
+		key := group.Description.PluralName
+		lm, ok := levelsMap[key]
+		if !ok {
+			continue
 		}
 
-		return v, nil
-	})
-	if err != nil {
-		return diag.FromErr(err)
+		for _, name := range slices.Sorted(maps.Keys(group.Resources)) {
+			field := reflect.ValueOf(group.Resources[name]).Elem().FieldByName("Permissions")
+			permissions := readPermissions(field)
+			permissions = append(permissions, convertPermissions(
+				ctx,
+				b.Config.Permissions,
+				permissions,
+				key,
+				lm,
+			)...)
+			writePermissions(field, permissions)
+		}
 	}
 
 	return nil
 }
 
+// readPermissions converts the permissions field of a resource to []resources.Permission.
+// The element type of the field differs between resources, but all have the same fields.
+func readPermissions(field reflect.Value) []resources.Permission {
+	var permissions []resources.Permission
+	for i := range field.Len() {
+		p := field.Index(i)
+		permissions = append(permissions, resources.Permission{
+			Level:                iam.PermissionLevel(p.FieldByName("Level").String()),
+			UserName:             p.FieldByName("UserName").String(),
+			ServicePrincipalName: p.FieldByName("ServicePrincipalName").String(),
+			GroupName:            p.FieldByName("GroupName").String(),
+		})
+	}
+	return permissions
+}
+
+// writePermissions sets the permissions field of a resource to permissions.
+// An empty list is stored as nil, so that the field is absent from the configuration.
+func writePermissions(field reflect.Value, permissions []resources.Permission) {
+	if len(permissions) == 0 {
+		field.SetZero()
+		return
+	}
+
+	out := reflect.MakeSlice(field.Type(), len(permissions), len(permissions))
+	for i, p := range permissions {
+		e := out.Index(i)
+		e.FieldByName("Level").SetString(string(p.Level))
+		e.FieldByName("UserName").SetString(p.UserName)
+		e.FieldByName("ServicePrincipalName").SetString(p.ServicePrincipalName)
+		e.FieldByName("GroupName").SetString(p.GroupName)
+	}
+	field.Set(out)
+}
+
 func validatePermissions(b *bundle.Bundle) error {
 	for _, p := range b.Config.Permissions {
 		if !slices.Contains(allowedLevels, string(p.Level)) {

bundle/config/mutator/resourcemutator/apply_bundle_permissions_test.go

@@ -11,6 +11,7 @@ import (
 	"github.com/databricks/cli/bundle/config"
 	"github.com/databricks/cli/bundle/config/resources"
 	"github.com/databricks/cli/libs/diag"
+	"github.com/databricks/cli/libs/dyn"
 	"github.com/databricks/databricks-sdk-go/service/jobs"
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
@@ -237,3 +238,32 @@ func TestAllResourcesExplicitlyDefinedForPermissionsSupport(t *testing.T) {
 		}
 	}
 }
+
+func TestApplyBundlePermissionsEmpty(t *testing.T) {
+	b := &bundle.Bundle{
+		Config: config.Root{
+			Resources: config.Resources{
+				Jobs: map[string]*resources.Job{
+					"empty": {
+						JobSettings: jobs.JobSettings{Name: "empty"},
+						Permissions: []resources.JobPermission{},
+					},
+					"unset": {
+						JobSettings: jobs.JobSettings{Name: "unset"},
+					},
+				},
+			},
+		},
+	}
+
+	diags := bundle.Apply(t.Context(), b, ApplyBundlePermissions())
+	require.NoError(t, diags.Error())
+
+	// An empty list of permissions is dropped, an unset one stays unset.
+	for _, name := range []string{"empty", "unset"} {
+		require.Nil(t, b.Config.Resources.Jobs[name].Permissions)
+		v, err := dyn.GetByPath(b.Config.Value(), dyn.MustPathFromString("resources.jobs."+name))
+		require.NoError(t, err)
+		assert.False(t, v.Get("permissions").IsValid(), name)
+	}
+}