Load bundle configuration from mutator (#1318)

## Changes

Prior to this change, the bundle configuration entry point was loaded
from the function `bundle.Load`. Other configuration files were only
loaded once the caller applied the first set of mutators. This
separation was unnecessary and not ideal in light of gathering
diagnostics while loading _any_ configuration file, not just the ones
from the includes.

This change:
* Updates `bundle.Load` to only verify that the specified path is a
valid bundle root.
* Moves mutators that perform loading to `bundle/config/loader`.
* Adds a "load" phase that takes the place of applying
`DefaultMutators`.

Follow ups:
* Rename `bundle.Load` -> `bundle.Find` (because it no longer performs
loading)

This change depends on #1316 and #1317.

## Tests

Tests pass.

bundle/bundle.go

@@ -74,12 +74,7 @@ func Load(ctx context.Context, path string) (*Bundle, error) {
 	if err != nil {
 		return nil, err
 	}
-	log.Debugf(ctx, "Loading bundle configuration from: %s", configFile)
-	root, err := config.Load(configFile)
-	if err != nil {
-		return nil, err
-	}
-	b.Config = *root
+	log.Debugf(ctx, "Found bundle root at %s (file %s)", b.RootPath, configFile)
 	return b, nil
 }
 

bundle/bundle_test.go

@@ -20,8 +20,8 @@ func TestLoadNotExists(t *testing.T) {
 
 func TestLoadExists(t *testing.T) {
 	b, err := Load(context.Background(), "./tests/basic")
-	require.Nil(t, err)
-	assert.Equal(t, "basic", b.Config.Bundle.Name)
+	assert.NoError(t, err)
+	assert.NotNil(t, b)
 }
 
 func TestBundleCacheDir(t *testing.T) {

bundle/config/loader/entry_point.go

@@ -0,0 +1,34 @@
+package loader
+
+import (
+	"context"
+
+	"github.com/databricks/cli/bundle"
+	"github.com/databricks/cli/bundle/config"
+	"github.com/databricks/cli/libs/diag"
+)
+
+type entryPoint struct{}
+
+// EntryPoint loads the entry point configuration.
+func EntryPoint() bundle.Mutator {
+	return &entryPoint{}
+}
+
+func (m *entryPoint) Name() string {
+	return "EntryPoint"
+}
+
+func (m *entryPoint) Apply(_ context.Context, b *bundle.Bundle) diag.Diagnostics {
+	path, err := config.FileNames.FindInPath(b.RootPath)
+	if err != nil {
+		return diag.FromErr(err)
+	}
+	this, err := config.Load(path)
+	if err != nil {
+		return diag.FromErr(err)
+	}
+	// TODO: Return actual warnings.
+	err = b.Config.Merge(this)
+	return diag.FromErr(err)
+}

bundle/config/loader/entry_point_test.go

@@ -0,0 +1,26 @@
+package loader_test
+
+import (
+	"context"
+	"testing"
+
+	"github.com/databricks/cli/bundle"
+	"github.com/databricks/cli/bundle/config/loader"
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
+)
+
+func TestEntryPointNoRootPath(t *testing.T) {
+	b := &bundle.Bundle{}
+	diags := bundle.Apply(context.Background(), b, loader.EntryPoint())
+	require.Error(t, diags.Error())
+}
+
+func TestEntryPoint(t *testing.T) {
+	b := &bundle.Bundle{
+		RootPath: "testdata",
+	}
+	diags := bundle.Apply(context.Background(), b, loader.EntryPoint())
+	require.NoError(t, diags.Error())
+	assert.Equal(t, "loader_test", b.Config.Bundle.Name)
+}

bundle/config/loader/process_include.go

@@ -1,4 +1,4 @@
-package mutator
+package loader
 
 import (
 	"context"

bundle/config/loader/process_include_test.go

@@ -1,22 +1,20 @@
-package mutator_test
+package loader_test
 
 import (
 	"context"
-	"fmt"
-	"os"
 	"path/filepath"
 	"testing"
 
 	"github.com/databricks/cli/bundle"
 	"github.com/databricks/cli/bundle/config"
-	"github.com/databricks/cli/bundle/config/mutator"
+	"github.com/databricks/cli/bundle/config/loader"
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
 )
 
 func TestProcessInclude(t *testing.T) {
 	b := &bundle.Bundle{
-		RootPath: t.TempDir(),
+		RootPath: "testdata",
 		Config: config.Root{
 			Workspace: config.Workspace{
 				Host: "foo",
@@ -24,15 +22,14 @@ func TestProcessInclude(t *testing.T) {
 		},
 	}
 
-	relPath := "./file.yml"
-	fullPath := filepath.Join(b.RootPath, relPath)
-	f, err := os.Create(fullPath)
-	require.NoError(t, err)
-	fmt.Fprint(f, "workspace:\n  host: bar\n")
-	f.Close()
+	m := loader.ProcessInclude(filepath.Join(b.RootPath, "host.yml"), "host.yml")
+	assert.Equal(t, "ProcessInclude(host.yml)", m.Name())
 
+	// Assert the host value prior to applying the mutator
 	assert.Equal(t, "foo", b.Config.Workspace.Host)
-	diags := bundle.Apply(context.Background(), b, mutator.ProcessInclude(fullPath, relPath))
+
+	// Apply the mutator and assert that the host value has been updated
+	diags := bundle.Apply(context.Background(), b, m)
 	require.NoError(t, diags.Error())
 	assert.Equal(t, "bar", b.Config.Workspace.Host)
 }

bundle/config/loader/process_root_includes.go

@@ -1,4 +1,4 @@
-package mutator
+package loader
 
 import (
 	"context"

bundle/config/loader/process_root_includes_test.go

@@ -1,4 +1,4 @@
-package mutator_test
+package loader_test
 
 import (
 	"context"
@@ -7,7 +7,7 @@ import (
 
 	"github.com/databricks/cli/bundle"
 	"github.com/databricks/cli/bundle/config"
-	"github.com/databricks/cli/bundle/config/mutator"
+	"github.com/databricks/cli/bundle/config/loader"
 	"github.com/databricks/cli/internal/testutil"
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
@@ -17,7 +17,7 @@ func TestProcessRootIncludesEmpty(t *testing.T) {
 	b := &bundle.Bundle{
 		RootPath: ".",
 	}
-	diags := bundle.Apply(context.Background(), b, mutator.ProcessRootIncludes())
+	diags := bundle.Apply(context.Background(), b, loader.ProcessRootIncludes())
 	require.NoError(t, diags.Error())
 }
 
@@ -37,7 +37,7 @@ func TestProcessRootIncludesAbs(t *testing.T) {
 			},
 		},
 	}
-	diags := bundle.Apply(context.Background(), b, mutator.ProcessRootIncludes())
+	diags := bundle.Apply(context.Background(), b, loader.ProcessRootIncludes())
 	require.True(t, diags.HasError())
 	assert.ErrorContains(t, diags.Error(), "must be relative paths")
 }
@@ -56,7 +56,7 @@ func TestProcessRootIncludesSingleGlob(t *testing.T) {
 	testutil.Touch(t, b.RootPath, "a.yml")
 	testutil.Touch(t, b.RootPath, "b.yml")
 
-	diags := bundle.Apply(context.Background(), b, mutator.ProcessRootIncludes())
+	diags := bundle.Apply(context.Background(), b, loader.ProcessRootIncludes())
 	require.NoError(t, diags.Error())
 	assert.Equal(t, []string{"a.yml", "b.yml"}, b.Config.Include)
 }
@@ -75,7 +75,7 @@ func TestProcessRootIncludesMultiGlob(t *testing.T) {
 	testutil.Touch(t, b.RootPath, "a1.yml")
 	testutil.Touch(t, b.RootPath, "b1.yml")
 
-	diags := bundle.Apply(context.Background(), b, mutator.ProcessRootIncludes())
+	diags := bundle.Apply(context.Background(), b, loader.ProcessRootIncludes())
 	require.NoError(t, diags.Error())
 	assert.Equal(t, []string{"a1.yml", "b1.yml"}, b.Config.Include)
 }
@@ -93,7 +93,7 @@ func TestProcessRootIncludesRemoveDups(t *testing.T) {
 
 	testutil.Touch(t, b.RootPath, "a.yml")
 
-	diags := bundle.Apply(context.Background(), b, mutator.ProcessRootIncludes())
+	diags := bundle.Apply(context.Background(), b, loader.ProcessRootIncludes())
 	require.NoError(t, diags.Error())
 	assert.Equal(t, []string{"a.yml"}, b.Config.Include)
 }
@@ -107,7 +107,7 @@ func TestProcessRootIncludesNotExists(t *testing.T) {
 			},
 		},
 	}
-	diags := bundle.Apply(context.Background(), b, mutator.ProcessRootIncludes())
+	diags := bundle.Apply(context.Background(), b, loader.ProcessRootIncludes())
 	require.True(t, diags.HasError())
 	assert.ErrorContains(t, diags.Error(), "notexist.yml defined in 'include' section does not match any files")
 }

bundle/config/loader/testdata/databricks.yml

@@ -0,0 +1,2 @@
+bundle:
+  name: loader_test

bundle/config/loader/testdata/host.yml

@@ -0,0 +1,2 @@
+workspace:
+  host: bar

bundle/config/mutator/mutator.go

@@ -3,13 +3,17 @@ package mutator
 import (
 	"github.com/databricks/cli/bundle"
 	"github.com/databricks/cli/bundle/config"
+	"github.com/databricks/cli/bundle/config/loader"
 	"github.com/databricks/cli/bundle/scripts"
 )
 
 func DefaultMutators() []bundle.Mutator {
 	return []bundle.Mutator{
+		loader.EntryPoint(),
+		loader.ProcessRootIncludes(),
+
+		// Execute preinit script after loading all configuration files.
 		scripts.Execute(config.ScriptPreInit),
-		ProcessRootIncludes(),
 		EnvironmentsToTargets(),
 		InitializeVariables(),
 		DefineDefaultTarget(),

bundle/phases/load.go

@@ -0,0 +1,29 @@
+package phases
+
+import (
+	"github.com/databricks/cli/bundle"
+	"github.com/databricks/cli/bundle/config/mutator"
+)
+
+// The load phase loads configuration from disk and performs
+// lightweight preprocessing (anything that can be done without network I/O).
+func Load() bundle.Mutator {
+	return newPhase(
+		"load",
+		mutator.DefaultMutators(),
+	)
+}
+
+func LoadDefaultTarget() bundle.Mutator {
+	return newPhase(
+		"load",
+		append(mutator.DefaultMutators(), mutator.SelectDefaultTarget()),
+	)
+}
+
+func LoadNamedTarget(target string) bundle.Mutator {
+	return newPhase(
+		"load",
+		append(mutator.DefaultMutators(), mutator.SelectTarget(target)),
+	)
+}

bundle/tests/conflicting_resource_ids_test.go

@@ -7,23 +7,25 @@ import (
 	"testing"
 
 	"github.com/databricks/cli/bundle"
-	"github.com/databricks/cli/bundle/config/mutator"
+	"github.com/databricks/cli/bundle/phases"
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
 )
 
 func TestConflictingResourceIdsNoSubconfig(t *testing.T) {
 	ctx := context.Background()
-	_, err := bundle.Load(ctx, "./conflicting_resource_ids/no_subconfigurations")
+	b, err := bundle.Load(ctx, "./conflicting_resource_ids/no_subconfigurations")
+	require.NoError(t, err)
+	diags := bundle.Apply(ctx, b, phases.Load())
 	bundleConfigPath := filepath.FromSlash("conflicting_resource_ids/no_subconfigurations/databricks.yml")
-	assert.ErrorContains(t, err, fmt.Sprintf("multiple resources named foo (job at %s, pipeline at %s)", bundleConfigPath, bundleConfigPath))
+	assert.ErrorContains(t, diags.Error(), fmt.Sprintf("multiple resources named foo (job at %s, pipeline at %s)", bundleConfigPath, bundleConfigPath))
 }
 
 func TestConflictingResourceIdsOneSubconfig(t *testing.T) {
 	ctx := context.Background()
 	b, err := bundle.Load(ctx, "./conflicting_resource_ids/one_subconfiguration")
 	require.NoError(t, err)
-	diags := bundle.Apply(ctx, b, bundle.Seq(mutator.DefaultMutators()...))
+	diags := bundle.Apply(ctx, b, phases.Load())
 	bundleConfigPath := filepath.FromSlash("conflicting_resource_ids/one_subconfiguration/databricks.yml")
 	resourcesConfigPath := filepath.FromSlash("conflicting_resource_ids/one_subconfiguration/resources.yml")
 	assert.ErrorContains(t, diags.Error(), fmt.Sprintf("multiple resources named foo (job at %s, pipeline at %s)", bundleConfigPath, resourcesConfigPath))
@@ -33,7 +35,7 @@ func TestConflictingResourceIdsTwoSubconfigs(t *testing.T) {
 	ctx := context.Background()
 	b, err := bundle.Load(ctx, "./conflicting_resource_ids/two_subconfigurations")
 	require.NoError(t, err)
-	diags := bundle.Apply(ctx, b, bundle.Seq(mutator.DefaultMutators()...))
+	diags := bundle.Apply(ctx, b, phases.Load())
 	resources1ConfigPath := filepath.FromSlash("conflicting_resource_ids/two_subconfigurations/resources1.yml")
 	resources2ConfigPath := filepath.FromSlash("conflicting_resource_ids/two_subconfigurations/resources2.yml")
 	assert.ErrorContains(t, diags.Error(), fmt.Sprintf("multiple resources named foo (job at %s, pipeline at %s)", resources1ConfigPath, resources2ConfigPath))

bundle/tests/include_test.go

@@ -7,7 +7,7 @@ import (
 	"testing"
 
 	"github.com/databricks/cli/bundle"
-	"github.com/databricks/cli/bundle/config/mutator"
+	"github.com/databricks/cli/bundle/phases"
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
 	"golang.org/x/exp/maps"
@@ -17,7 +17,7 @@ func TestIncludeInvalid(t *testing.T) {
 	ctx := context.Background()
 	b, err := bundle.Load(ctx, "./include_invalid")
 	require.NoError(t, err)
-	diags := bundle.Apply(ctx, b, bundle.Seq(mutator.DefaultMutators()...))
+	diags := bundle.Apply(ctx, b, phases.Load())
 	require.Error(t, diags.Error())
 	assert.ErrorContains(t, diags.Error(), "notexists.yml defined in 'include' section does not match any files")
 }

bundle/tests/loader.go

@@ -6,6 +6,7 @@ import (
 
 	"github.com/databricks/cli/bundle"
 	"github.com/databricks/cli/bundle/config/mutator"
+	"github.com/databricks/cli/bundle/phases"
 	"github.com/stretchr/testify/require"
 )
 
@@ -13,7 +14,7 @@ func load(t *testing.T, path string) *bundle.Bundle {
 	ctx := context.Background()
 	b, err := bundle.Load(ctx, path)
 	require.NoError(t, err)
-	diags := bundle.Apply(ctx, b, bundle.Seq(mutator.DefaultMutators()...))
+	diags := bundle.Apply(ctx, b, phases.Load())
 	require.NoError(t, diags.Error())
 	return b
 }
@@ -22,9 +23,8 @@ func loadTarget(t *testing.T, path, env string) *bundle.Bundle {
 	ctx := context.Background()
 	b, err := bundle.Load(ctx, path)
 	require.NoError(t, err)
-	diags := bundle.Apply(ctx, b, bundle.Seq(mutator.DefaultMutatorsForTarget(env)...))
-	require.NoError(t, diags.Error())
-	diags = bundle.Apply(ctx, b, bundle.Seq(
+	diags := bundle.Apply(ctx, b, bundle.Seq(
+		phases.LoadNamedTarget(env),
 		mutator.RewriteSyncPaths(),
 		mutator.MergeJobClusters(),
 		mutator.MergeJobTasks(),

bundle/tests/python_wheel_test.go

@@ -16,8 +16,7 @@ func TestPythonWheelBuild(t *testing.T) {
 	b, err := bundle.Load(ctx, "./python_wheel/python_wheel")
 	require.NoError(t, err)
 
-	m := phases.Build()
-	diags := bundle.Apply(ctx, b, m)
+	diags := bundle.Apply(ctx, b, bundle.Seq(phases.Load(), phases.Build()))
 	require.NoError(t, diags.Error())
 
 	matches, err := filepath.Glob("./python_wheel/python_wheel/my_test_code/dist/my_test_code-*.whl")
@@ -34,8 +33,7 @@ func TestPythonWheelBuildAutoDetect(t *testing.T) {
 	b, err := bundle.Load(ctx, "./python_wheel/python_wheel_no_artifact")
 	require.NoError(t, err)
 
-	m := phases.Build()
-	diags := bundle.Apply(ctx, b, m)
+	diags := bundle.Apply(ctx, b, bundle.Seq(phases.Load(), phases.Build()))
 	require.NoError(t, diags.Error())
 
 	matches, err := filepath.Glob("./python_wheel/python_wheel_no_artifact/dist/my_test_code-*.whl")
@@ -52,8 +50,7 @@ func TestPythonWheelWithDBFSLib(t *testing.T) {
 	b, err := bundle.Load(ctx, "./python_wheel/python_wheel_dbfs_lib")
 	require.NoError(t, err)
 
-	m := phases.Build()
-	diags := bundle.Apply(ctx, b, m)
+	diags := bundle.Apply(ctx, b, bundle.Seq(phases.Load(), phases.Build()))
 	require.NoError(t, diags.Error())
 
 	match := libraries.MatchWithArtifacts()
@@ -66,8 +63,7 @@ func TestPythonWheelBuildNoBuildJustUpload(t *testing.T) {
 	b, err := bundle.Load(ctx, "./python_wheel/python_wheel_no_artifact_no_setup")
 	require.NoError(t, err)
 
-	m := phases.Build()
-	diags := bundle.Apply(ctx, b, m)
+	diags := bundle.Apply(ctx, b, bundle.Seq(phases.Load(), phases.Build()))
 	require.NoError(t, diags.Error())
 
 	match := libraries.MatchWithArtifacts()

cmd/root/bundle_test.go

@@ -40,8 +40,12 @@ func emptyCommand(t *testing.T) *cobra.Command {
 func setup(t *testing.T, cmd *cobra.Command, host string) *bundle.Bundle {
 	setupDatabricksCfg(t)
 
+	rootPath := t.TempDir()
+	testutil.Touch(t, rootPath, "databricks.yml")
+
 	err := configureBundle(cmd, []string{"validate"}, func(_ context.Context) (*bundle.Bundle, error) {
 		return &bundle.Bundle{
+			RootPath: rootPath,
 			Config: config.Root{
 				Bundle: config.Bundle{
 					Name: "test",

libs/template/renderer_test.go

@@ -14,7 +14,6 @@ import (
 
 	"github.com/databricks/cli/bundle"
 	bundleConfig "github.com/databricks/cli/bundle/config"
-	"github.com/databricks/cli/bundle/config/mutator"
 	"github.com/databricks/cli/bundle/phases"
 	"github.com/databricks/cli/cmd/root"
 	"github.com/databricks/cli/libs/diag"
@@ -66,23 +65,25 @@ func assertBuiltinTemplateValid(t *testing.T, template string, settings map[stri
 	require.NoError(t, err)
 	err = renderer.persistToDisk()
 	require.NoError(t, err)
+
 	b, err := bundle.Load(ctx, filepath.Join(tempDir, "template", "my_project"))
 	require.NoError(t, err)
+	diags := bundle.Apply(ctx, b, phases.LoadNamedTarget(target))
+	require.NoError(t, diags.Error())
 
 	// Apply initialize / validation mutators
 	bundle.ApplyFunc(ctx, b, func(ctx context.Context, b *bundle.Bundle) diag.Diagnostics {
 		b.Config.Workspace.CurrentUser = &bundleConfig.User{User: cachedUser}
+		b.Config.Bundle.Terraform = &bundleConfig.Terraform{
+			ExecPath: "sh",
+		}
 		return nil
 	})
 
 	b.Tagging = tags.ForCloud(w.Config)
 	b.WorkspaceClient()
-	b.Config.Bundle.Terraform = &bundleConfig.Terraform{
-		ExecPath: "sh",
-	}
-	diags := bundle.Apply(ctx, b, bundle.Seq(
-		bundle.Seq(mutator.DefaultMutators()...),
-		mutator.SelectTarget(target),
+
+	diags = bundle.Apply(ctx, b, bundle.Seq(
 		phases.Initialize(),
 	))
 	require.NoError(t, diags.Error())