addressed feedback

bundle/apps/upload_config.go

@@ -0,0 +1,105 @@
+package apps
+
+import (
+	"bytes"
+	"context"
+	"fmt"
+	"path"
+	"strings"
+
+	"github.com/databricks/cli/bundle"
+	"github.com/databricks/cli/bundle/config/resources"
+	"github.com/databricks/cli/bundle/deploy"
+	"github.com/databricks/cli/libs/diag"
+	"github.com/databricks/cli/libs/filer"
+	"golang.org/x/sync/errgroup"
+
+	"gopkg.in/yaml.v3"
+)
+
+type uploadConfig struct {
+	filerFactory deploy.FilerFactory
+}
+
+func (u *uploadConfig) Apply(ctx context.Context, b *bundle.Bundle) diag.Diagnostics {
+	var diags diag.Diagnostics
+	errGroup, ctx := errgroup.WithContext(ctx)
+
+	for key, app := range b.Config.Resources.Apps {
+		// If the app has a config, we need to deploy it first.
+		// It means we need to write app.yml file with the content of the config field
+		// to the remote source code path of the app.
+		if app.Config != nil {
+			if !strings.HasPrefix(app.SourceCodePath, b.Config.Workspace.FilePath) {
+				diags = append(diags, diag.Diagnostic{
+					Severity:  diag.Error,
+					Summary:   "App source code invalid",
+					Detail:    fmt.Sprintf("App source code path %s is not within file path %s", app.SourceCodePath, b.Config.Workspace.FilePath),
+					Locations: b.Config.GetLocations(fmt.Sprintf("resources.apps.%s.source_code_path", key)),
+				})
+
+				continue
+			}
+
+			appPath := strings.TrimPrefix(app.SourceCodePath, b.Config.Workspace.FilePath)
+
+			buf, err := configToYaml(app)
+			if err != nil {
+				return diag.FromErr(err)
+			}
+
+			// When the app is started, create a new app deployment and wait for it to complete.
+			f, err := u.filerFactory(b)
+			if err != nil {
+				return diag.FromErr(err)
+			}
+
+			errGroup.Go(func() error {
+				err = f.Write(ctx, path.Join(appPath, "app.yml"), buf, filer.OverwriteIfExists)
+				if err != nil {
+					diags = append(diags, diag.Diagnostic{
+						Severity:  diag.Error,
+						Summary:   "Failed to save config",
+						Detail:    fmt.Sprintf("Failed to write %s file: %s", path.Join(app.SourceCodePath, "app.yml"), err),
+						Locations: b.Config.GetLocations(fmt.Sprintf("resources.apps.%s", key)),
+					})
+				}
+				return nil
+			})
+		}
+	}
+
+	if err := errGroup.Wait(); err != nil {
+		return diag.FromErr(err)
+	}
+
+	return diags
+}
+
+// Name implements bundle.Mutator.
+func (u *uploadConfig) Name() string {
+	return "apps:UploadConfig"
+}
+
+func UploadConfig() bundle.Mutator {
+	return &uploadConfig{
+		filerFactory: func(b *bundle.Bundle) (filer.Filer, error) {
+			return filer.NewWorkspaceFilesClient(b.WorkspaceClient(), b.Config.Workspace.FilePath)
+		},
+	}
+}
+
+func configToYaml(app *resources.App) (*bytes.Buffer, error) {
+	buf := bytes.NewBuffer(nil)
+	enc := yaml.NewEncoder(buf)
+	enc.SetIndent(2)
+
+	err := enc.Encode(app.Config)
+	defer enc.Close()
+
+	if err != nil {
+		return nil, fmt.Errorf("failed to encode app config to yaml: %w", err)
+	}
+
+	return buf, nil
+}

bundle/apps/upload_config_test.go

@@ -0,0 +1,69 @@
+package apps
+
+import (
+	"bytes"
+	"context"
+	"os"
+	"path/filepath"
+	"testing"
+
+	"github.com/databricks/cli/bundle"
+	"github.com/databricks/cli/bundle/config"
+	"github.com/databricks/cli/bundle/config/resources"
+	mockfiler "github.com/databricks/cli/internal/mocks/libs/filer"
+	"github.com/databricks/cli/libs/filer"
+	"github.com/databricks/cli/libs/vfs"
+	"github.com/databricks/databricks-sdk-go/service/apps"
+	"github.com/stretchr/testify/mock"
+	"github.com/stretchr/testify/require"
+)
+
+func TestAppUploadConfig(t *testing.T) {
+	root := t.TempDir()
+	err := os.MkdirAll(filepath.Join(root, "my_app"), 0700)
+	require.NoError(t, err)
+
+	b := &bundle.Bundle{
+		BundleRootPath: root,
+		SyncRoot:       vfs.MustNew(root),
+		Config: config.Root{
+			Workspace: config.Workspace{
+				RootPath: "/Workspace/Users/foo@bar.com/",
+			},
+			Resources: config.Resources{
+				Apps: map[string]*resources.App{
+					"my_app": {
+						App: &apps.App{
+							Name: "my_app",
+						},
+						SourceCodePath: "./my_app",
+						Config: map[string]any{
+							"command": []string{"echo", "hello"},
+							"env": []map[string]string{
+								{"name": "MY_APP", "value": "my value"},
+							},
+						},
+					},
+				},
+			},
+		},
+	}
+
+	mockFiler := mockfiler.NewMockFiler(t)
+	mockFiler.EXPECT().Write(mock.Anything, "my_app/app.yml", bytes.NewBufferString(`command:
+  - echo
+  - hello
+env:
+  - name: MY_APP
+    value: my value
+`), filer.OverwriteIfExists).Return(nil)
+
+	u := uploadConfig{
+		filerFactory: func(b *bundle.Bundle) (filer.Filer, error) {
+			return mockFiler, nil
+		},
+	}
+
+	diags := bundle.Apply(context.Background(), b, &u)
+	require.NoError(t, diags.Error())
+}

bundle/apps/validate.go

@@ -3,8 +3,7 @@ package apps
 import (
 	"context"
 	"fmt"
-	"os"
+	"path"
-	"path/filepath"
 
 	"github.com/databricks/cli/bundle"
 	"github.com/databricks/cli/libs/diag"
@@ -15,14 +14,15 @@ type validate struct {
 
 func (v *validate) Apply(ctx context.Context, b *bundle.Bundle) diag.Diagnostics {
 	var diags diag.Diagnostics
-	for _, app := range b.Config.Resources.Apps {
 	possibleConfigFiles := []string{"app.yml", "app.yaml"}
+
+	for _, app := range b.Config.Resources.Apps {
 		for _, configFile := range possibleConfigFiles {
-			cf := filepath.Join(b.SyncRootPath, app.SourceCodePath, configFile)
+			cf := path.Join(app.SourceCodePath, configFile)
-			if _, err := os.Stat(cf); err == nil {
+			if _, err := b.SyncRoot.Stat(cf); err == nil {
 				diags = append(diags, diag.Diagnostic{
 					Severity: diag.Error,
-					Summary:  "app.yml detected",
+					Summary:  fmt.Sprintf("%s detected", configFile),
 					Detail:   fmt.Sprintf("remove %s and use 'config' property for app resource '%s' instead", cf, app.Name),
 				})
 			}

bundle/config/mutator/merge_apps.go

@@ -26,7 +26,7 @@ func (m *mergeApps) resourceName(v dyn.Value) string {
 	case dyn.KindString:
 		return v.MustString()
 	default:
-		panic("job cluster key must be a string")
+		panic("app name must be a string")
 	}
 }
 

bundle/config/resources/apps.go

@@ -17,15 +17,15 @@ type App struct {
 	// This equals to app name and added for symmetry with other resources.
 	ID string `json:"id,omitempty" bundle:"readonly"`
 
-	// SourceCodePath is a required field used by DABs to point databricks app source code
+	// SourceCodePath is a required field used by DABs to point to Databricks app source code
-	// on local disk and use it to point to this source code in the app deployment
+	// on local disk and to the corresponding workspace path during app deployment.
 	SourceCodePath string `json:"source_code_path"`
 
 	// Config is an optional field which allows configuring the app following Databricks app configuration format like in app.yml.
 	// When this field is set, DABs read the configuration set in this field and write
 	// it to app.yml in the root of the source code folder in Databricks workspace.
 	// If there’s app.yml defined locally, DABs will raise an error.
-	Config map[string]interface{} `json:"config,omitempty"`
+	Config map[string]any `json:"config,omitempty"`
 
 	Permissions    []Permission   `json:"permissions,omitempty"`
 	ModifiedStatus ModifiedStatus `json:"modified_status,omitempty" bundle:"internal"`

bundle/deploy/terraform/tfdyn/convert_app_test.go

@@ -16,7 +16,7 @@ import (
 func TestConvertApp(t *testing.T) {
 	var src = resources.App{
 		SourceCodePath: "./app",
-		Config: map[string]interface{}{
+		Config: map[string]any{
 			"command": []string{"python", "app.py"},
 		},
 		App: &apps.App{
@@ -60,20 +60,20 @@ func TestConvertApp(t *testing.T) {
 	require.NoError(t, err)
 
 	app := out.App["my_app"]
-	assert.Equal(t, map[string]interface{}{
+	assert.Equal(t, map[string]any{
 		"description": "app description",
 		"name":        "app_id",
-		"resource": []interface{}{
+		"resource": []any{
-			map[string]interface{}{
+			map[string]any{
 				"name": "job1",
-				"job": map[string]interface{}{
+				"job": map[string]any{
 					"id":         "1234",
 					"permission": "CAN_MANAGE_RUN",
 				},
 			},
-			map[string]interface{}{
+			map[string]any{
 				"name": "sql1",
-				"sql_warehouse": map[string]interface{}{
+				"sql_warehouse": map[string]any{
 					"id":         "5678",
 					"permission": "CAN_USE",
 				},

bundle/run/app.go

@@ -1,23 +1,16 @@
 package run
 
 import (
-	"bytes"
 	"context"
 	"fmt"
-	"path"
-	"path/filepath"
 	"time"
 
 	"github.com/databricks/cli/bundle"
 	"github.com/databricks/cli/bundle/config/resources"
-	"github.com/databricks/cli/bundle/deploy"
 	"github.com/databricks/cli/bundle/run/output"
 	"github.com/databricks/cli/libs/cmdio"
-	"github.com/databricks/cli/libs/filer"
 	"github.com/databricks/databricks-sdk-go/service/apps"
 	"github.com/spf13/cobra"
-
-	"gopkg.in/yaml.v3"
 )
 
 func logProgress(ctx context.Context, msg string) {
@@ -32,8 +25,6 @@ type appRunner struct {
 
 	bundle *bundle.Bundle
 	app    *resources.App
-
-	filerFactory deploy.FilerFactory
 }
 
 func (a *appRunner) Name() string {
@@ -116,6 +107,11 @@ func (a *appRunner) start(ctx context.Context) error {
 		return err
 	}
 
+	// After the app is started (meaning the compute is running), the API will return the app object with the
+	// active and pending deployments fields (if any). If there are active or pending deployments,
+	// we need to wait for them to complete before we can do the new deployment.
+	// Otherwise, the new deployment will fail.
+	// Thus, we first wait for the active deployment to complete.
 	if startedApp.ActiveDeployment != nil &&
 		startedApp.ActiveDeployment.Status.State == apps.AppDeploymentStateInProgress {
 		logProgress(ctx, "Waiting for the active deployment to complete...")
@@ -126,6 +122,7 @@ func (a *appRunner) start(ctx context.Context) error {
 		logProgress(ctx, "Active deployment is completed!")
 	}
 
+	// Then, we wait for the pending deployment to complete.
 	if startedApp.PendingDeployment != nil &&
 		startedApp.PendingDeployment.Status.State == apps.AppDeploymentStateInProgress {
 		logProgress(ctx, "Waiting for the pending deployment to complete...")
@@ -145,32 +142,6 @@ func (a *appRunner) deploy(ctx context.Context) error {
 	b := a.bundle
 	w := b.WorkspaceClient()
 
-	// If the app has a config, we need to deploy it first.
-	// It means we need to write app.yml file with the content of the config field
-	// to the remote source code path of the app.
-	if app.Config != nil {
-		appPath, err := filepath.Rel(b.Config.Workspace.FilePath, app.SourceCodePath)
-		if err != nil {
-			return fmt.Errorf("failed to get relative path of app source code path: %w", err)
-		}
-
-		buf, err := configToYaml(app)
-		if err != nil {
-			return err
-		}
-
-		// When the app is started, create a new app deployment and wait for it to complete.
-		f, err := a.filerFactory(b)
-		if err != nil {
-			return err
-		}
-
-		err = f.Write(ctx, path.Join(appPath, "app.yml"), buf, filer.OverwriteIfExists)
-		if err != nil {
-			return fmt.Errorf("failed to write %s file: %w", path.Join(app.SourceCodePath, "app.yml"), err)
-		}
-	}
-
 	wait, err := w.Apps.Deploy(ctx, apps.CreateAppDeploymentRequest{
 		AppName: app.Name,
 		AppDeployment: &apps.AppDeployment{
@@ -241,18 +212,3 @@ func (a *appRunner) ParseArgs(args []string, opts *Options) error {
 func (a *appRunner) CompleteArgs(args []string, toComplete string) ([]string, cobra.ShellCompDirective) {
 	return nil, cobra.ShellCompDirectiveNoFileComp
 }
-
-func configToYaml(app *resources.App) (*bytes.Buffer, error) {
-	buf := bytes.NewBuffer(nil)
-	enc := yaml.NewEncoder(buf)
-	enc.SetIndent(2)
-
-	err := enc.Encode(app.Config)
-	defer enc.Close()
-
-	if err != nil {
-		return nil, fmt.Errorf("failed to encode app config to yaml: %w", err)
-	}
-
-	return buf, nil
-}

bundle/run/app_test.go

@@ -13,10 +13,8 @@ import (
 	"github.com/databricks/cli/bundle/config/mutator"
 	"github.com/databricks/cli/bundle/config/resources"
 	"github.com/databricks/cli/bundle/internal/bundletest"
-	mockfiler "github.com/databricks/cli/internal/mocks/libs/filer"
 	"github.com/databricks/cli/libs/cmdio"
 	"github.com/databricks/cli/libs/dyn"
-	"github.com/databricks/cli/libs/filer"
 	"github.com/databricks/cli/libs/flags"
 	"github.com/databricks/cli/libs/vfs"
 	"github.com/databricks/databricks-sdk-go/experimental/mocks"
@@ -28,7 +26,6 @@ import (
 type testAppRunner struct {
 	m   *mocks.MockWorkspaceClient
 	b   *bundle.Bundle
-	mockFiler *mockfiler.MockFiler
 	ctx context.Context
 }
 
@@ -37,9 +34,6 @@ func (ta *testAppRunner) run(t *testing.T) {
 		key:    "my_app",
 		bundle: ta.b,
 		app:    ta.b.Config.Resources.Apps["my_app"],
-		filerFactory: func(b *bundle.Bundle) (filer.Filer, error) {
-			return ta.mockFiler, nil
-		},
 	}
 
 	_, err := r.Run(ta.ctx, &Options{})
@@ -65,7 +59,7 @@ func setupBundle(t *testing.T) (context.Context, *bundle.Bundle, *mocks.MockWork
 							Name: "my_app",
 						},
 						SourceCodePath: "./my_app",
-						Config: map[string]interface{}{
+						Config: map[string]any{
 							"command": []string{"echo", "hello"},
 							"env": []map[string]string{
 								{"name": "MY_APP", "value": "my value"},
@@ -123,19 +117,9 @@ func setupTestApp(t *testing.T, initialAppState apps.ApplicationState, initialCo
 		},
 	}).Return(wait, nil)
 
-	mockFiler := mockfiler.NewMockFiler(t)
-	mockFiler.EXPECT().Write(mock.Anything, "my_app/app.yml", bytes.NewBufferString(`command:
-  - echo
-  - hello
-env:
-  - name: MY_APP
-    value: my value
-`), filer.OverwriteIfExists).Return(nil)
-
 	return &testAppRunner{
 		m:   mwc,
 		b:   b,
-		mockFiler: mockFiler,
 		ctx: ctx,
 	}
 }
@@ -225,9 +209,6 @@ func TestStopApp(t *testing.T) {
 		key:    "my_app",
 		bundle: b,
 		app:    b.Config.Resources.Apps["my_app"],
-		filerFactory: func(b *bundle.Bundle) (filer.Filer, error) {
-			return nil, nil
-		},
 	}
 
 	err := r.Cancel(ctx)

bundle/run/runner.go

@@ -8,7 +8,6 @@ import (
 	"github.com/databricks/cli/bundle/config/resources"
 	refs "github.com/databricks/cli/bundle/resources"
 	"github.com/databricks/cli/bundle/run/output"
-	"github.com/databricks/cli/libs/filer"
 )
 
 type key string
@@ -62,9 +61,6 @@ func ToRunner(b *bundle.Bundle, ref refs.Reference) (Runner, error) {
 			key:    key(ref.KeyWithType),
 			bundle: b,
 			app:    resource,
-			filerFactory: func(b *bundle.Bundle) (filer.Filer, error) {
-				return filer.NewWorkspaceFilesClient(b.WorkspaceClient(), b.Config.Workspace.FilePath)
-			},
 		}, nil
 	default:
 		return nil, fmt.Errorf("unsupported resource type: %T", resource)

bundle/tests/apps_test.go

@@ -23,8 +23,8 @@ func TestApps(t *testing.T) {
 	app := b.Config.Resources.Apps["my_app"]
 	assert.Equal(t, "my-app", app.Name)
 	assert.Equal(t, "My App", app.Description)
-	assert.Equal(t, []interface{}{"python", "app.py"}, app.Config["command"])
+	assert.Equal(t, []any{"python", "app.py"}, app.Config["command"])
-	assert.Equal(t, []interface{}{map[string]interface{}{"name": "SOME_ENV_VARIABLE", "value": "Some value"}}, app.Config["env"])
+	assert.Equal(t, []any{map[string]any{"name": "SOME_ENV_VARIABLE", "value": "Some value"}}, app.Config["env"])
 
 	assert.Len(t, app.Resources, 2)
 	assert.Equal(t, "1234", app.Resources[0].SqlWarehouse.Id)
@@ -46,8 +46,8 @@ func TestAppsOverride(t *testing.T) {
 	app := b.Config.Resources.Apps["my_app"]
 	assert.Equal(t, "my-app", app.Name)
 	assert.Equal(t, "My App", app.Description)
-	assert.Equal(t, []interface{}{"python", "dev.py"}, app.Config["command"])
+	assert.Equal(t, []any{"python", "dev.py"}, app.Config["command"])
-	assert.Equal(t, []interface{}{map[string]interface{}{"name": "SOME_ENV_VARIABLE_2", "value": "Some value 2"}}, app.Config["env"])
+	assert.Equal(t, []any{map[string]any{"name": "SOME_ENV_VARIABLE_2", "value": "Some value 2"}}, app.Config["env"])
 
 	assert.Len(t, app.Resources, 3)
 	assert.Equal(t, "1234", app.Resources[0].SqlWarehouse.Id)

libs/dyn/merge/elements_by_key_test.go

@@ -50,3 +50,42 @@ func TestElementByKey(t *testing.T) {
 		},
 	)
 }
+
+func TestElementByKeyWithOverride(t *testing.T) {
+	vin := dyn.V([]dyn.Value{
+		dyn.V(map[string]dyn.Value{
+			"key":   dyn.V("foo"),
+			"value": dyn.V(42),
+		}),
+		dyn.V(map[string]dyn.Value{
+			"key":   dyn.V("bar"),
+			"value": dyn.V(43),
+		}),
+		dyn.V(map[string]dyn.Value{
+			"key":        dyn.V("foo"),
+			"othervalue": dyn.V(44),
+		}),
+	})
+
+	keyFunc := func(v dyn.Value) string {
+		return strings.ToLower(v.MustString())
+	}
+
+	vout, err := dyn.MapByPath(vin, dyn.EmptyPath, ElementsByKeyWithOverride("key", keyFunc))
+	require.NoError(t, err)
+	assert.Len(t, vout.MustSequence(), 2)
+	assert.Equal(t,
+		vout.Index(0).AsAny(),
+		map[string]any{
+			"key":        "foo",
+			"othervalue": 44,
+		},
+	)
+	assert.Equal(t,
+		vout.Index(1).AsAny(),
+		map[string]any{
+			"key":   "bar",
+			"value": 43,
+		},
+	)
+}