From e9d9caf07ae274098b779da139d732b0605a55d2 Mon Sep 17 00:00:00 2001 From: Andrew Nester Date: Thu, 21 Nov 2024 17:21:25 +0100 Subject: [PATCH 01/25] Added support for Databricks Apps in DABs --- bundle/config/mutator/apply_presets.go | 13 + bundle/config/mutator/apply_presets_test.go | 57 ++++ bundle/config/mutator/merge_apps.go | 45 +++ bundle/config/mutator/merge_apps_test.go | 64 +++++ .../mutator/process_target_mode_test.go | 8 + bundle/config/mutator/run_as_test.go | 2 + bundle/config/mutator/translate_paths.go | 1 + bundle/config/mutator/translate_paths_apps.go | 28 ++ .../mutator/translate_paths_apps_test.go | 57 ++++ bundle/config/resources.go | 7 + bundle/config/resources/apps.go | 75 +++++ bundle/deploy/terraform/convert.go | 15 + bundle/deploy/terraform/convert_test.go | 57 ++++ bundle/deploy/terraform/interpolate.go | 2 + bundle/deploy/terraform/interpolate_test.go | 2 + bundle/deploy/terraform/tfdyn/convert_app.go | 55 ++++ .../terraform/tfdyn/convert_app_test.go | 99 +++++++ bundle/permissions/mutator.go | 4 + bundle/permissions/mutator_test.go | 8 + bundle/phases/initialize.go | 2 + bundle/run/app.go | 260 ++++++++++++++++++ bundle/run/app_test.go | 214 ++++++++++++++ bundle/run/runner.go | 12 +- bundle/tests/apps/databricks.yml | 71 +++++ bundle/tests/apps_test.go | 61 ++++ bundle/tests/loader.go | 1 + 26 files changed, 1219 insertions(+), 1 deletion(-) create mode 100644 bundle/config/mutator/merge_apps.go create mode 100644 bundle/config/mutator/merge_apps_test.go create mode 100644 bundle/config/mutator/translate_paths_apps.go create mode 100644 bundle/config/mutator/translate_paths_apps_test.go create mode 100644 bundle/config/resources/apps.go create mode 100644 bundle/deploy/terraform/tfdyn/convert_app.go create mode 100644 bundle/deploy/terraform/tfdyn/convert_app_test.go create mode 100644 bundle/run/app.go create mode 100644 bundle/run/app_test.go create mode 100644 bundle/tests/apps/databricks.yml create mode 100644 bundle/tests/apps_test.go diff --git a/bundle/config/mutator/apply_presets.go b/bundle/config/mutator/apply_presets.go index 3817037565..57b765a8d7 100644 --- a/bundle/config/mutator/apply_presets.go +++ b/bundle/config/mutator/apply_presets.go @@ -222,6 +222,19 @@ func (m *applyPresets) Apply(ctx context.Context, b *bundle.Bundle) diag.Diagnos dashboard.DisplayName = prefix + dashboard.DisplayName } + // Apps: Prefix + for key, app := range r.Apps { + if app == nil || app.App == nil { + diags = diags.Extend(diag.Errorf("app %s is not defined", key)) + continue + } + + app.Name = textutil.NormalizeString(prefix + app.Name) + // Normalize the app name to ensure it is a valid identifier. + // App supports only alphanumeric characters and hyphens. + app.Name = strings.ReplaceAll(app.Name, "_", "-") + } + if config.IsExplicitlyEnabled((b.Config.Presets.SourceLinkedDeployment)) { isDatabricksWorkspace := dbr.RunsOnRuntime(ctx) && strings.HasPrefix(b.SyncRootPath, "/Workspace/") if !isDatabricksWorkspace { diff --git a/bundle/config/mutator/apply_presets_test.go b/bundle/config/mutator/apply_presets_test.go index c26f203832..2af54e74e2 100644 --- a/bundle/config/mutator/apply_presets_test.go +++ b/bundle/config/mutator/apply_presets_test.go @@ -12,6 +12,7 @@ import ( "github.com/databricks/cli/bundle/internal/bundletest" "github.com/databricks/cli/libs/dbr" "github.com/databricks/cli/libs/dyn" + "github.com/databricks/databricks-sdk-go/service/apps" "github.com/databricks/databricks-sdk-go/service/catalog" "github.com/databricks/databricks-sdk-go/service/jobs" "github.com/stretchr/testify/require" @@ -482,3 +483,59 @@ func TestApplyPresetsSourceLinkedDeployment(t *testing.T) { }) } } + +func TestApplyPresetsPrefixForApps(t *testing.T) { + tests := []struct { + name string + prefix string + app *resources.App + want string + }{ + { + name: "add prefix to app", + prefix: "[prefix] ", + app: &resources.App{ + App: &apps.App{ + Name: "app1", + }, + }, + want: "prefix-app1", + }, + { + name: "add empty prefix to app", + prefix: "", + app: &resources.App{ + App: &apps.App{ + Name: "app1", + }, + }, + want: "app1", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + b := &bundle.Bundle{ + Config: config.Root{ + Resources: config.Resources{ + Apps: map[string]*resources.App{ + "app1": tt.app, + }, + }, + Presets: config.Presets{ + NamePrefix: tt.prefix, + }, + }, + } + + ctx := context.Background() + diag := bundle.Apply(ctx, b, mutator.ApplyPresets()) + + if diag.HasError() { + t.Fatalf("unexpected error: %v", diag) + } + + require.Equal(t, tt.want, b.Config.Resources.Apps["app1"].Name) + }) + } +} diff --git a/bundle/config/mutator/merge_apps.go b/bundle/config/mutator/merge_apps.go new file mode 100644 index 0000000000..88c745a834 --- /dev/null +++ b/bundle/config/mutator/merge_apps.go @@ -0,0 +1,45 @@ +package mutator + +import ( + "context" + + "github.com/databricks/cli/bundle" + "github.com/databricks/cli/libs/diag" + "github.com/databricks/cli/libs/dyn" + "github.com/databricks/cli/libs/dyn/merge" +) + +type mergeApps struct{} + +func MergeApps() bundle.Mutator { + return &mergeApps{} +} + +func (m *mergeApps) Name() string { + return "MergeApps" +} + +func (m *mergeApps) resourceName(v dyn.Value) string { + switch v.Kind() { + case dyn.KindInvalid, dyn.KindNil: + return "" + case dyn.KindString: + return v.MustString() + default: + panic("job cluster key must be a string") + } +} + +func (m *mergeApps) Apply(ctx context.Context, b *bundle.Bundle) diag.Diagnostics { + err := b.Config.Mutate(func(v dyn.Value) (dyn.Value, error) { + if v.Kind() == dyn.KindNil { + return v, nil + } + + return dyn.Map(v, "resources.apps", dyn.Foreach(func(_ dyn.Path, app dyn.Value) (dyn.Value, error) { + return dyn.Map(app, "resources", merge.ElementsByKey("name", m.resourceName)) + })) + }) + + return diag.FromErr(err) +} diff --git a/bundle/config/mutator/merge_apps_test.go b/bundle/config/mutator/merge_apps_test.go new file mode 100644 index 0000000000..2cdef830ec --- /dev/null +++ b/bundle/config/mutator/merge_apps_test.go @@ -0,0 +1,64 @@ +package mutator_test + +import ( + "context" + "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/resources" + "github.com/databricks/databricks-sdk-go/service/apps" + "github.com/stretchr/testify/assert" +) + +func TestMergeApps(t *testing.T) { + b := &bundle.Bundle{ + Config: config.Root{ + Resources: config.Resources{ + Apps: map[string]*resources.App{ + "foo": { + App: &apps.App{ + Name: "foo", + Resources: []apps.AppResource{ + { + Name: "job1", + Job: &apps.AppResourceJob{ + Id: "1234", + Permission: "CAN_MANAGE_RUN", + }, + }, + { + Name: "sql1", + SqlWarehouse: &apps.AppResourceSqlWarehouse{ + Id: "5678", + Permission: "CAN_USE", + }, + }, + { + Name: "job1", + Job: &apps.AppResourceJob{ + Id: "1234", + Permission: "CAN_MANAGE", + }, + }, + }, + }, + }, + }, + }, + }, + } + + diags := bundle.Apply(context.Background(), b, mutator.MergeApps()) + assert.NoError(t, diags.Error()) + + j := b.Config.Resources.Apps["foo"] + + assert.Len(t, j.Resources, 2) + assert.Equal(t, "job1", j.Resources[0].Name) + assert.Equal(t, "sql1", j.Resources[1].Name) + + assert.Equal(t, "CAN_MANAGE", string(j.Resources[0].Job.Permission)) + assert.Equal(t, "CAN_USE", string(j.Resources[1].SqlWarehouse.Permission)) +} diff --git a/bundle/config/mutator/process_target_mode_test.go b/bundle/config/mutator/process_target_mode_test.go index 14d524416e..ce75dc2fa1 100644 --- a/bundle/config/mutator/process_target_mode_test.go +++ b/bundle/config/mutator/process_target_mode_test.go @@ -15,6 +15,7 @@ import ( "github.com/databricks/cli/libs/tags" "github.com/databricks/cli/libs/vfs" sdkconfig "github.com/databricks/databricks-sdk-go/config" + "github.com/databricks/databricks-sdk-go/service/apps" "github.com/databricks/databricks-sdk-go/service/catalog" "github.com/databricks/databricks-sdk-go/service/compute" "github.com/databricks/databricks-sdk-go/service/dashboards" @@ -144,6 +145,13 @@ func mockBundle(mode config.Mode) *bundle.Bundle { }, }, }, + Apps: map[string]*resources.App{ + "app1": { + App: &apps.App{ + Name: "app1", + }, + }, + }, }, }, SyncRoot: vfs.MustNew("/Users/lennart.kats@databricks.com"), diff --git a/bundle/config/mutator/run_as_test.go b/bundle/config/mutator/run_as_test.go index dbf4bf8060..afd10e748f 100644 --- a/bundle/config/mutator/run_as_test.go +++ b/bundle/config/mutator/run_as_test.go @@ -32,6 +32,7 @@ func allResourceTypes(t *testing.T) []string { // the dyn library gives us the correct list of all resources supported. Please // also update this check when adding a new resource require.Equal(t, []string{ + "apps", "clusters", "dashboards", "experiments", @@ -143,6 +144,7 @@ func TestRunAsErrorForUnsupportedResources(t *testing.T) { "experiments", "schemas", "volumes", + "apps", } base := config.Root{ diff --git a/bundle/config/mutator/translate_paths.go b/bundle/config/mutator/translate_paths.go index af0f941201..1915cf36e1 100644 --- a/bundle/config/mutator/translate_paths.go +++ b/bundle/config/mutator/translate_paths.go @@ -262,6 +262,7 @@ func (m *translatePaths) Apply(_ context.Context, b *bundle.Bundle) diag.Diagnos t.applyPipelineTranslations, t.applyArtifactTranslations, t.applyDashboardTranslations, + t.applyAppsTranslations, } { v, err = fn(v) if err != nil { diff --git a/bundle/config/mutator/translate_paths_apps.go b/bundle/config/mutator/translate_paths_apps.go new file mode 100644 index 0000000000..0ed7e19280 --- /dev/null +++ b/bundle/config/mutator/translate_paths_apps.go @@ -0,0 +1,28 @@ +package mutator + +import ( + "fmt" + + "github.com/databricks/cli/libs/dyn" +) + +func (t *translateContext) applyAppsTranslations(v dyn.Value) (dyn.Value, error) { + // Convert the `source_code_path` field to a remote absolute path. + // We use this path for app deployment to point to the source code. + pattern := dyn.NewPattern( + dyn.Key("resources"), + dyn.Key("apps"), + dyn.AnyKey(), + dyn.Key("source_code_path"), + ) + + return dyn.MapByPattern(v, pattern, func(p dyn.Path, v dyn.Value) (dyn.Value, error) { + key := p[2].Key() + dir, err := v.Location().Directory() + if err != nil { + return dyn.InvalidValue, fmt.Errorf("unable to determine directory for app %s: %w", key, err) + } + + return t.rewriteRelativeTo(p, v, t.translateDirectoryPath, dir, "") + }) +} diff --git a/bundle/config/mutator/translate_paths_apps_test.go b/bundle/config/mutator/translate_paths_apps_test.go new file mode 100644 index 0000000000..5692934b83 --- /dev/null +++ b/bundle/config/mutator/translate_paths_apps_test.go @@ -0,0 +1,57 @@ +package mutator_test + +import ( + "context" + "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/resources" + "github.com/databricks/cli/bundle/internal/bundletest" + "github.com/databricks/cli/libs/dyn" + "github.com/databricks/cli/libs/vfs" + "github.com/databricks/databricks-sdk-go/service/apps" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func TestTranslatePathsApps_FilePathRelativeSubDirectory(t *testing.T) { + dir := t.TempDir() + touchEmptyFile(t, filepath.Join(dir, "src", "app", "app.py")) + + b := &bundle.Bundle{ + SyncRootPath: dir, + SyncRoot: vfs.MustNew(dir), + Config: config.Root{ + Workspace: config.Workspace{ + FilePath: "/bundle/files", + }, + Resources: config.Resources{ + Apps: map[string]*resources.App{ + "app": { + App: &apps.App{ + Name: "My App", + }, + SourceCodePath: "../src/app", + }, + }, + }, + }, + } + + bundletest.SetLocation(b, "resources.apps", []dyn.Location{{ + File: filepath.Join(dir, "resources/app.yml"), + }}) + + diags := bundle.Apply(context.Background(), b, mutator.TranslatePaths()) + require.NoError(t, diags.Error()) + + // Assert that the file path for the app has been converted to its local absolute path. + assert.Equal( + t, + "/bundle/files/src/app", + b.Config.Resources.Apps["app"].SourceCodePath, + ) +} diff --git a/bundle/config/resources.go b/bundle/config/resources.go index 13cf0d4625..e6821c009b 100644 --- a/bundle/config/resources.go +++ b/bundle/config/resources.go @@ -23,6 +23,7 @@ type Resources struct { Volumes map[string]*resources.Volume `json:"volumes,omitempty"` Clusters map[string]*resources.Cluster `json:"clusters,omitempty"` Dashboards map[string]*resources.Dashboard `json:"dashboards,omitempty"` + Apps map[string]*resources.App `json:"apps,omitempty"` } type ConfigResource interface { @@ -87,6 +88,7 @@ func (r *Resources) AllResources() []ResourceGroup { collectResourceMap(descriptions["clusters"], r.Clusters), collectResourceMap(descriptions["dashboards"], r.Dashboards), collectResourceMap(descriptions["volumes"], r.Volumes), + collectResourceMap(descriptions["apps"], r.Apps), } } @@ -196,6 +198,11 @@ func SupportedResources() map[string]ResourceDescription { PluralName: "volumes", SingularTitle: "Volume", PluralTitle: "Volumes", + "apps": { + SingularName: "app", + PluralName: "apps", + SingularTitle: "App", + PluralTitle: "Apps", }, } } diff --git a/bundle/config/resources/apps.go b/bundle/config/resources/apps.go new file mode 100644 index 0000000000..aad9308185 --- /dev/null +++ b/bundle/config/resources/apps.go @@ -0,0 +1,75 @@ +package resources + +import ( + "context" + "fmt" + "net/url" + + "github.com/databricks/cli/libs/log" + "github.com/databricks/databricks-sdk-go" + "github.com/databricks/databricks-sdk-go/marshal" + "github.com/databricks/databricks-sdk-go/service/apps" +) + +type App struct { + // This represents the id which is the name of the app that can be used + // as a reference in other resources. This value is returned by terraform. + ID string `json:"id,omitempty" bundle:"readonly"` + + // SourceCodePath is a required field used by DABs to point databricks app source code + // on local disk and use it to point to this source code in the 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 already, it will be overridden. + Config map[string]interface{} `json:"config,omitempty"` + + Permissions []Permission `json:"permissions,omitempty"` + ModifiedStatus ModifiedStatus `json:"modified_status,omitempty" bundle:"internal"` + URL string `json:"url,omitempty" bundle:"internal"` + + *apps.App +} + +func (a *App) UnmarshalJSON(b []byte) error { + return marshal.Unmarshal(b, a) +} + +func (a App) MarshalJSON() ([]byte, error) { + return marshal.Marshal(a) +} + +func (a *App) Exists(ctx context.Context, w *databricks.WorkspaceClient, name string) (bool, error) { + _, err := w.Apps.GetByName(ctx, name) + if err != nil { + log.Debugf(ctx, "app %s does not exist", name) + return false, err + } + return true, nil +} + +func (a *App) TerraformResourceName() string { + return "databricks_cluster" +} + +func (a *App) InitializeURL(baseURL url.URL) { + if a.ID == "" { + return + } + baseURL.Path = fmt.Sprintf("apps/%s", a.ID) + a.URL = baseURL.String() +} + +func (a *App) GetName() string { + return a.Name +} + +func (a *App) GetURL() string { + return a.URL +} + +func (a *App) IsNil() bool { + return a.App == nil +} diff --git a/bundle/deploy/terraform/convert.go b/bundle/deploy/terraform/convert.go index b710c690f3..b47642697f 100644 --- a/bundle/deploy/terraform/convert.go +++ b/bundle/deploy/terraform/convert.go @@ -196,6 +196,16 @@ func TerraformToBundle(state *resourcesState, config *config.Root) error { } cur.ID = instance.Attributes.ID config.Resources.Dashboards[resource.Name] = cur + case "databricks_app": + if config.Resources.Apps == nil { + config.Resources.Apps = make(map[string]*resources.App) + } + cur := config.Resources.Apps[resource.Name] + if cur == nil { + cur = &resources.App{ModifiedStatus: resources.ModifiedStatusDeleted} + } + cur.ID = instance.Attributes.ID + config.Resources.Apps[resource.Name] = cur case "databricks_permissions": case "databricks_grants": // Ignore; no need to pull these back into the configuration. @@ -260,6 +270,11 @@ func TerraformToBundle(state *resourcesState, config *config.Root) error { src.ModifiedStatus = resources.ModifiedStatusCreated } } + for _, src := range config.Resources.Apps { + if src.ModifiedStatus == "" && src.ID == "" { + src.ModifiedStatus = resources.ModifiedStatusCreated + } + } return nil } diff --git a/bundle/deploy/terraform/convert_test.go b/bundle/deploy/terraform/convert_test.go index 61f26c0886..440eea233e 100644 --- a/bundle/deploy/terraform/convert_test.go +++ b/bundle/deploy/terraform/convert_test.go @@ -10,6 +10,7 @@ import ( "github.com/databricks/cli/bundle/internal/tf/schema" "github.com/databricks/cli/libs/dyn" "github.com/databricks/cli/libs/dyn/convert" + "github.com/databricks/databricks-sdk-go/service/apps" "github.com/databricks/databricks-sdk-go/service/catalog" "github.com/databricks/databricks-sdk-go/service/compute" "github.com/databricks/databricks-sdk-go/service/dashboards" @@ -694,6 +695,14 @@ func TestTerraformToBundleEmptyLocalResources(t *testing.T) { {Attributes: stateInstanceAttributes{ID: "1"}}, }, }, + { + Type: "databricks_app", + Mode: "managed", + Name: "test_app", + Instances: []stateResourceInstance{ + {Attributes: stateInstanceAttributes{ID: "1"}}, + }, + }, }, } err := TerraformToBundle(&tfState, &config) @@ -732,6 +741,9 @@ func TestTerraformToBundleEmptyLocalResources(t *testing.T) { assert.Equal(t, "1", config.Resources.Dashboards["test_dashboard"].ID) assert.Equal(t, resources.ModifiedStatusDeleted, config.Resources.Dashboards["test_dashboard"].ModifiedStatus) + assert.Equal(t, "1", config.Resources.Apps["test_app"].ID) + assert.Equal(t, resources.ModifiedStatusDeleted, config.Resources.Apps["test_app"].ModifiedStatus) + AssertFullResourceCoverage(t, &config) } @@ -815,6 +827,13 @@ func TestTerraformToBundleEmptyRemoteResources(t *testing.T) { }, }, }, + Apps: map[string]*resources.App{ + "test_app": { + App: &apps.App{ + Name: "test_app", + }, + }, + }, }, } tfState := resourcesState{ @@ -856,6 +875,9 @@ func TestTerraformToBundleEmptyRemoteResources(t *testing.T) { assert.Equal(t, "", config.Resources.Dashboards["test_dashboard"].ID) assert.Equal(t, resources.ModifiedStatusCreated, config.Resources.Dashboards["test_dashboard"].ModifiedStatus) + assert.Equal(t, "", config.Resources.Apps["test_app"].ID) + assert.Equal(t, resources.ModifiedStatusCreated, config.Resources.Apps["test_app"].ModifiedStatus) + AssertFullResourceCoverage(t, &config) } @@ -994,6 +1016,18 @@ func TestTerraformToBundleModifiedResources(t *testing.T) { }, }, }, + Apps: map[string]*resources.App{ + "test_app": { + App: &apps.App{ + Name: "test_app", + }, + }, + "test_app_new": { + App: &apps.App{ + Name: "test_app_new", + }, + }, + }, }, } tfState := resourcesState{ @@ -1174,6 +1208,22 @@ func TestTerraformToBundleModifiedResources(t *testing.T) { {Attributes: stateInstanceAttributes{ID: "2"}}, }, }, + { + Type: "databricks_app", + Mode: "managed", + Name: "test_app", + Instances: []stateResourceInstance{ + {Attributes: stateInstanceAttributes{ID: "1"}}, + }, + }, + { + Type: "databricks_app", + Mode: "managed", + Name: "test_app_old", + Instances: []stateResourceInstance{ + {Attributes: stateInstanceAttributes{ID: "2"}}, + }, + }, }, } err := TerraformToBundle(&tfState, &config) @@ -1256,6 +1306,13 @@ func TestTerraformToBundleModifiedResources(t *testing.T) { assert.Equal(t, "", config.Resources.Dashboards["test_dashboard_new"].ID) assert.Equal(t, resources.ModifiedStatusCreated, config.Resources.Dashboards["test_dashboard_new"].ModifiedStatus) + assert.Equal(t, "1", config.Resources.Apps["test_app"].ID) + assert.Equal(t, "", config.Resources.Apps["test_app"].ModifiedStatus) + assert.Equal(t, "2", config.Resources.Apps["test_app_old"].ID) + assert.Equal(t, resources.ModifiedStatusDeleted, config.Resources.Apps["test_app_old"].ModifiedStatus) + assert.Equal(t, "", config.Resources.Apps["test_app_new"].ID) + assert.Equal(t, resources.ModifiedStatusCreated, config.Resources.Apps["test_app_new"].ModifiedStatus) + AssertFullResourceCoverage(t, &config) } diff --git a/bundle/deploy/terraform/interpolate.go b/bundle/deploy/terraform/interpolate.go index 813e6bbb7a..719e6ad25a 100644 --- a/bundle/deploy/terraform/interpolate.go +++ b/bundle/deploy/terraform/interpolate.go @@ -63,6 +63,8 @@ func (m *interpolateMutator) Apply(ctx context.Context, b *bundle.Bundle) diag.D path = dyn.NewPath(dyn.Key("databricks_cluster")).Append(path[2:]...) case dyn.Key("dashboards"): path = dyn.NewPath(dyn.Key("databricks_dashboard")).Append(path[2:]...) + case dyn.Key("apps"): + path = dyn.NewPath(dyn.Key("databricks_app")).Append(path[2:]...) default: // Trigger "key not found" for unknown resource types. return dyn.GetByPath(root, path) diff --git a/bundle/deploy/terraform/interpolate_test.go b/bundle/deploy/terraform/interpolate_test.go index fc5c4d184c..91a7bd54a7 100644 --- a/bundle/deploy/terraform/interpolate_test.go +++ b/bundle/deploy/terraform/interpolate_test.go @@ -34,6 +34,7 @@ func TestInterpolate(t *testing.T) { "other_volume": "${resources.volumes.other_volume.id}", "other_cluster": "${resources.clusters.other_cluster.id}", "other_dashboard": "${resources.dashboards.other_dashboard.id}", + "other_app": "${resources.apps.other_app.id}", }, Tasks: []jobs.Task{ { @@ -73,6 +74,7 @@ func TestInterpolate(t *testing.T) { assert.Equal(t, "${databricks_volume.other_volume.id}", j.Tags["other_volume"]) assert.Equal(t, "${databricks_cluster.other_cluster.id}", j.Tags["other_cluster"]) assert.Equal(t, "${databricks_dashboard.other_dashboard.id}", j.Tags["other_dashboard"]) + assert.Equal(t, "${databricks_app.other_app.id}", j.Tags["other_app"]) m := b.Config.Resources.Models["my_model"] assert.Equal(t, "my_model", m.Model.Name) diff --git a/bundle/deploy/terraform/tfdyn/convert_app.go b/bundle/deploy/terraform/tfdyn/convert_app.go new file mode 100644 index 0000000000..f4849cab74 --- /dev/null +++ b/bundle/deploy/terraform/tfdyn/convert_app.go @@ -0,0 +1,55 @@ +package tfdyn + +import ( + "context" + "fmt" + + "github.com/databricks/cli/bundle/internal/tf/schema" + "github.com/databricks/cli/libs/dyn" + "github.com/databricks/cli/libs/dyn/convert" + "github.com/databricks/cli/libs/log" + "github.com/databricks/databricks-sdk-go/service/apps" +) + +func convertAppResource(ctx context.Context, vin dyn.Value) (dyn.Value, error) { + // Normalize the output value to the target schema. + vout, diags := convert.Normalize(apps.App{}, vin) + for _, diag := range diags { + log.Debugf(ctx, "app normalization diagnostic: %s", diag.Summary) + } + + return vout, nil +} + +type appConverter struct{} + +func (appConverter) Convert(ctx context.Context, key string, vin dyn.Value, out *schema.Resources) error { + vout, err := convertAppResource(ctx, vin) + if err != nil { + return err + } + + // Modify top-level keys. + vout, err = renameKeys(vout, map[string]string{ + "resources": "resource", + }) + + if err != nil { + return err + } + + // Add the converted resource to the output. + out.App[key] = vout.AsAny() + + // Configure permissions for this resource. + if permissions := convertPermissionsResource(ctx, vin); permissions != nil { + permissions.AppName = fmt.Sprintf("${databricks_app.%s.name}", key) + out.Permissions["app_"+key] = permissions + } + + return nil +} + +func init() { + registerConverter("apps", appConverter{}) +} diff --git a/bundle/deploy/terraform/tfdyn/convert_app_test.go b/bundle/deploy/terraform/tfdyn/convert_app_test.go new file mode 100644 index 0000000000..95b9bdae45 --- /dev/null +++ b/bundle/deploy/terraform/tfdyn/convert_app_test.go @@ -0,0 +1,99 @@ +package tfdyn + +import ( + "context" + "testing" + + "github.com/databricks/cli/bundle/config/resources" + "github.com/databricks/cli/bundle/internal/tf/schema" + "github.com/databricks/cli/libs/dyn" + "github.com/databricks/cli/libs/dyn/convert" + "github.com/databricks/databricks-sdk-go/service/apps" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func TestConvertApp(t *testing.T) { + var src = resources.App{ + SourceCodePath: "./app", + Config: map[string]interface{}{ + "command": []string{"python", "app.py"}, + }, + App: &apps.App{ + Name: "app_id", + Description: "app description", + Resources: []apps.AppResource{ + { + Name: "job1", + Job: &apps.AppResourceJob{ + Id: "1234", + Permission: "CAN_MANAGE_RUN", + }, + }, + { + Name: "sql1", + SqlWarehouse: &apps.AppResourceSqlWarehouse{ + Id: "5678", + Permission: "CAN_USE", + }, + }, + }, + }, + Permissions: []resources.Permission{ + { + Level: "CAN_RUN", + UserName: "jack@gmail.com", + }, + { + Level: "CAN_MANAGE", + ServicePrincipalName: "sp", + }, + }, + } + + vin, err := convert.FromTyped(src, dyn.NilValue) + require.NoError(t, err) + + ctx := context.Background() + out := schema.NewResources() + err = appConverter{}.Convert(ctx, "my_app", vin, out) + require.NoError(t, err) + + app := out.App["my_app"] + assert.Equal(t, map[string]interface{}{ + "description": "app description", + "name": "app_id", + "resource": []interface{}{ + map[string]interface{}{ + "name": "job1", + "job": map[string]interface{}{ + "id": "1234", + "permission": "CAN_MANAGE_RUN", + }, + }, + map[string]interface{}{ + "name": "sql1", + "sql_warehouse": map[string]interface{}{ + "id": "5678", + "permission": "CAN_USE", + }, + }, + }, + }, app) + + // Assert equality on the permissions + assert.Equal(t, &schema.ResourcePermissions{ + AppName: "${databricks_app.my_app.name}", + AccessControl: []schema.ResourcePermissionsAccessControl{ + { + PermissionLevel: "CAN_RUN", + UserName: "jack@gmail.com", + }, + { + PermissionLevel: "CAN_MANAGE", + ServicePrincipalName: "sp", + }, + }, + }, out.Permissions["app_my_app"]) + +} diff --git a/bundle/permissions/mutator.go b/bundle/permissions/mutator.go index cd7cbf40c8..8a0057deee 100644 --- a/bundle/permissions/mutator.go +++ b/bundle/permissions/mutator.go @@ -51,6 +51,10 @@ var ( CAN_MANAGE: "CAN_MANAGE", CAN_VIEW: "CAN_READ", }, + "apps": { + CAN_MANAGE: "CAN_MANAGE", + CAN_VIEW: "CAN_USE", + }, } ) diff --git a/bundle/permissions/mutator_test.go b/bundle/permissions/mutator_test.go index 78703e90f0..1949cca372 100644 --- a/bundle/permissions/mutator_test.go +++ b/bundle/permissions/mutator_test.go @@ -58,6 +58,10 @@ func TestApplyBundlePermissions(t *testing.T) { "dashboard_1": {}, "dashboard_2": {}, }, + Apps: map[string]*resources.App{ + "app_1": {}, + "app_2": {}, + }, }, }, } @@ -114,6 +118,10 @@ func TestApplyBundlePermissions(t *testing.T) { require.Len(t, b.Config.Resources.Dashboards["dashboard_1"].Permissions, 2) require.Contains(t, b.Config.Resources.Dashboards["dashboard_1"].Permissions, resources.Permission{Level: "CAN_MANAGE", UserName: "TestUser"}) require.Contains(t, b.Config.Resources.Dashboards["dashboard_1"].Permissions, resources.Permission{Level: "CAN_READ", GroupName: "TestGroup"}) + + require.Len(t, b.Config.Resources.Apps["app_1"].Permissions, 2) + require.Contains(t, b.Config.Resources.Apps["app_1"].Permissions, resources.Permission{Level: "CAN_MANAGE", UserName: "TestUser"}) + require.Contains(t, b.Config.Resources.Apps["app_1"].Permissions, resources.Permission{Level: "CAN_USE", GroupName: "TestGroup"}) } func TestWarningOnOverlapPermission(t *testing.T) { diff --git a/bundle/phases/initialize.go b/bundle/phases/initialize.go index 6fa0e5fede..e8a6c8504b 100644 --- a/bundle/phases/initialize.go +++ b/bundle/phases/initialize.go @@ -37,6 +37,8 @@ func Initialize() bundle.Mutator { mutator.MergeJobParameters(), mutator.MergeJobTasks(), mutator.MergePipelineClusters(), + mutator.MergeApps(), + mutator.InitializeWorkspaceClient(), mutator.PopulateCurrentUser(), mutator.LoadGitDetails(), diff --git a/bundle/run/app.go b/bundle/run/app.go new file mode 100644 index 0000000000..84ed43087e --- /dev/null +++ b/bundle/run/app.go @@ -0,0 +1,260 @@ +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) { + if msg == "" { + return + } + cmdio.LogString(ctx, fmt.Sprintf("✓ %s", msg)) +} + +type appRunner struct { + key + + bundle *bundle.Bundle + app *resources.App + + filerFactory deploy.FilerFactory +} + +func (a *appRunner) Name() string { + if a.app == nil { + return "" + } + + return a.app.Name +} + +func isAppStopped(app *apps.App) bool { + return app.ComputeStatus == nil || + (app.ComputeStatus.State == apps.ComputeStateStopped || app.ComputeStatus.State == apps.ComputeStateError) +} + +func (a *appRunner) Run(ctx context.Context, opts *Options) (output.RunOutput, error) { + app := a.app + b := a.bundle + if app == nil { + return nil, fmt.Errorf("app is not defined") + } + + logProgress(ctx, fmt.Sprintf("Getting the status of the app %s", app.Name)) + w := b.WorkspaceClient() + + // Check the status of the app first. + createdApp, err := w.Apps.Get(ctx, apps.GetAppRequest{Name: app.Name}) + if err != nil { + return nil, err + } + + if createdApp.AppStatus != nil { + logProgress(ctx, fmt.Sprintf("App is in %s state", createdApp.AppStatus.State)) + } + + if createdApp.ComputeStatus != nil { + logProgress(ctx, fmt.Sprintf("App compute is in %s state", createdApp.ComputeStatus.State)) + } + + // There could be 2 reasons why the app is not running: + // 1. The app is new and was never deployed yet. + // 2. The app was stopped (compute not running). + // We need to start the app only if the compute is not running. + if isAppStopped(createdApp) { + err := a.start(ctx) + if err != nil { + return nil, err + } + } + + // Deploy the app. + err = a.deploy(ctx) + if err != nil { + return nil, err + } + + // TODO: We should return the app URL here. + cmdio.LogString(ctx, "You can access the app at ") + return nil, nil +} + +func (a *appRunner) start(ctx context.Context) error { + app := a.app + b := a.bundle + w := b.WorkspaceClient() + + logProgress(ctx, fmt.Sprintf("Starting the app %s", app.Name)) + wait, err := w.Apps.Start(ctx, apps.StartAppRequest{Name: app.Name}) + if err != nil { + return err + } + + startedApp, err := wait.OnProgress(func(p *apps.App) { + if p.AppStatus == nil { + return + } + logProgress(ctx, "App is starting...") + }).Get() + + if err != nil { + return err + } + + // If the app has a pending deployment, wait for it to complete. + if startedApp.PendingDeployment != nil { + _, err := w.Apps.WaitGetDeploymentAppSucceeded(ctx, + startedApp.Name, startedApp.PendingDeployment.DeploymentId, + 20*time.Minute, nil) + + if err != nil { + return err + } + } + + // If the app has an active deployment, wait for it to complete as well + if startedApp.ActiveDeployment != nil { + _, err := w.Apps.WaitGetDeploymentAppSucceeded(ctx, + startedApp.Name, startedApp.ActiveDeployment.DeploymentId, + 20*time.Minute, nil) + + if err != nil { + return err + } + } + + logProgress(ctx, "App is started!") + return nil +} + +func (a *appRunner) deploy(ctx context.Context) error { + app := a.app + 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{ + Mode: apps.AppDeploymentModeSnapshot, + SourceCodePath: app.SourceCodePath, + }, + }) + + if err != nil { + return err + } + + _, err = wait.OnProgress(func(ad *apps.AppDeployment) { + if ad.Status == nil { + return + } + logProgress(ctx, ad.Status.Message) + }).Get() + + if err != nil { + return err + } + + return nil +} + +func (a *appRunner) Cancel(ctx context.Context) error { + // We should cancel the app by stopping it. + app := a.app + b := a.bundle + if app == nil { + return fmt.Errorf("app is not defined") + } + + w := b.WorkspaceClient() + + logProgress(ctx, fmt.Sprintf("Stopping app %s", app.Name)) + wait, err := w.Apps.Stop(ctx, apps.StopAppRequest{Name: app.Name}) + if err != nil { + return err + } + + _, err = wait.OnProgress(func(p *apps.App) { + if p.AppStatus == nil { + return + } + logProgress(ctx, p.AppStatus.Message) + }).Get() + + logProgress(ctx, "App is stopped!") + return err +} + +func (a *appRunner) Restart(ctx context.Context, opts *Options) (output.RunOutput, error) { + // We should restart the app by just running it again meaning a new app deployment will be done. + return a.Run(ctx, opts) +} + +func (a *appRunner) ParseArgs(args []string, opts *Options) error { + if len(args) == 0 { + return nil + } + + return fmt.Errorf("received %d unexpected positional arguments", len(args)) +} + +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 +} diff --git a/bundle/run/app_test.go b/bundle/run/app_test.go new file mode 100644 index 0000000000..c0da444289 --- /dev/null +++ b/bundle/run/app_test.go @@ -0,0 +1,214 @@ +package run + +import ( + "bytes" + "context" + "os" + "path/filepath" + "testing" + "time" + + "github.com/databricks/cli/bundle" + "github.com/databricks/cli/bundle/config" + "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" + "github.com/databricks/databricks-sdk-go/service/apps" + "github.com/stretchr/testify/mock" + "github.com/stretchr/testify/require" +) + +type testAppRunner struct { + m *mocks.MockWorkspaceClient + b *bundle.Bundle + mockFiler *mockfiler.MockFiler + ctx context.Context +} + +func (ta *testAppRunner) run(t *testing.T) { + r := appRunner{ + 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{}) + require.NoError(t, err) +} + +func setupBundle(t *testing.T) (context.Context, *bundle.Bundle, *mocks.MockWorkspaceClient) { + 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]interface{}{ + "command": []string{"echo", "hello"}, + "env": []map[string]string{ + {"name": "MY_APP", "value": "my value"}, + }, + }, + }, + }, + }, + }, + } + + mwc := mocks.NewMockWorkspaceClient(t) + b.SetWorkpaceClient(mwc.WorkspaceClient) + bundletest.SetLocation(b, "resources.apps.my_app", []dyn.Location{{File: "./databricks.yml"}}) + + ctx := context.Background() + ctx = cmdio.InContext(ctx, cmdio.NewIO(flags.OutputText, &bytes.Buffer{}, &bytes.Buffer{}, &bytes.Buffer{}, "", "...")) + ctx = cmdio.NewContext(ctx, cmdio.NewLogger(flags.ModeAppend)) + + diags := bundle.Apply(ctx, b, bundle.Seq( + mutator.DefineDefaultWorkspacePaths(), + mutator.TranslatePaths(), + )) + require.Empty(t, diags) + + return ctx, b, mwc +} + +func setupTestApp(t *testing.T, initialAppState apps.ApplicationState, initialComputeState apps.ComputeState) *testAppRunner { + ctx, b, mwc := setupBundle(t) + + appApi := mwc.GetMockAppsAPI() + appApi.EXPECT().Get(mock.Anything, apps.GetAppRequest{ + Name: "my_app", + }).Return(&apps.App{ + Name: "my_app", + AppStatus: &apps.ApplicationStatus{ + State: initialAppState, + }, + ComputeStatus: &apps.ComputeStatus{ + State: initialComputeState, + }, + }, nil) + + wait := &apps.WaitGetDeploymentAppSucceeded[apps.AppDeployment]{ + Poll: func(_ time.Duration, _ func(*apps.AppDeployment)) (*apps.AppDeployment, error) { + return nil, nil + }, + } + appApi.EXPECT().Deploy(mock.Anything, apps.CreateAppDeploymentRequest{ + AppName: "my_app", + AppDeployment: &apps.AppDeployment{ + Mode: apps.AppDeploymentModeSnapshot, + SourceCodePath: "/Workspace/Users/foo@bar.com/files/my_app", + }, + }).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, + } +} + +func TestAppRunStartedApp(t *testing.T) { + r := setupTestApp(t, apps.ApplicationStateRunning, apps.ComputeStateActive) + r.run(t) +} + +func TestAppRunStoppedApp(t *testing.T) { + r := setupTestApp(t, apps.ApplicationStateCrashed, apps.ComputeStateStopped) + + appsApi := r.m.GetMockAppsAPI() + appsApi.EXPECT().Start(mock.Anything, apps.StartAppRequest{ + Name: "my_app", + }).Return(&apps.WaitGetAppActive[apps.App]{ + Poll: func(_ time.Duration, _ func(*apps.App)) (*apps.App, error) { + return &apps.App{ + Name: "my_app", + AppStatus: &apps.ApplicationStatus{ + State: apps.ApplicationStateRunning, + }, + ComputeStatus: &apps.ComputeStatus{ + State: apps.ComputeStateActive, + }, + ActiveDeployment: &apps.AppDeployment{ + SourceCodePath: "/foo/bar", + DeploymentId: "123", + Status: &apps.AppDeploymentStatus{ + State: apps.AppDeploymentStateInProgress, + }, + }, + PendingDeployment: &apps.AppDeployment{ + SourceCodePath: "/foo/bar", + DeploymentId: "456", + Status: &apps.AppDeploymentStatus{ + State: apps.AppDeploymentStateInProgress, + }, + }, + }, nil + }, + }, nil) + + appsApi.EXPECT().WaitGetDeploymentAppSucceeded(mock.Anything, "my_app", "123", mock.Anything, mock.Anything).Return(nil, nil) + appsApi.EXPECT().WaitGetDeploymentAppSucceeded(mock.Anything, "my_app", "456", mock.Anything, mock.Anything).Return(nil, nil) + r.run(t) +} + +func TestStopApp(t *testing.T) { + ctx, b, mwc := setupBundle(t) + appsApi := mwc.GetMockAppsAPI() + appsApi.EXPECT().Stop(mock.Anything, apps.StopAppRequest{ + Name: "my_app", + }).Return(&apps.WaitGetAppStopped[apps.App]{ + Poll: func(_ time.Duration, _ func(*apps.App)) (*apps.App, error) { + return &apps.App{ + Name: "my_app", + AppStatus: &apps.ApplicationStatus{ + State: apps.ApplicationStateUnavailable, + }, + }, nil + }, + }, nil) + + r := appRunner{ + 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) + require.NoError(t, err) +} diff --git a/bundle/run/runner.go b/bundle/run/runner.go index 4c907d068d..e4706ca531 100644 --- a/bundle/run/runner.go +++ b/bundle/run/runner.go @@ -8,6 +8,7 @@ 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 @@ -42,7 +43,7 @@ type Runner interface { // IsRunnable returns a filter that only allows runnable resources. func IsRunnable(ref refs.Reference) bool { switch ref.Resource.(type) { - case *resources.Job, *resources.Pipeline: + case *resources.Job, *resources.Pipeline, *resources.App: return true default: return false @@ -56,6 +57,15 @@ func ToRunner(b *bundle.Bundle, ref refs.Reference) (Runner, error) { return &jobRunner{key: key(ref.KeyWithType), bundle: b, job: resource}, nil case *resources.Pipeline: return &pipelineRunner{key: key(ref.KeyWithType), bundle: b, pipeline: resource}, nil + case *resources.App: + return &appRunner{ + 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) } diff --git a/bundle/tests/apps/databricks.yml b/bundle/tests/apps/databricks.yml new file mode 100644 index 0000000000..ad7e930065 --- /dev/null +++ b/bundle/tests/apps/databricks.yml @@ -0,0 +1,71 @@ +bundle: + name: apps + +workspace: + host: https://acme.cloud.databricks.com/ + +variables: + app_config: + type: complex + default: + command: + - "python" + - "app.py" + env: + - name: SOME_ENV_VARIABLE + value: "Some value" + +resources: + apps: + my_app: + name: "my-app" + description: "My App" + source_code_path: ./app + config: ${var.app_config} + + resources: + - name: "my-sql-warehouse" + sql_warehouse: + id: 1234 + permission: "CAN_USE" + - name: "my-job" + job: + id: 5678 + permission: "CAN_MANAGE_RUN" + permissions: + - user_name: "foo@bar.com" + level: "CAN_VIEW" + - service_principal_name: "my_sp" + level: "CAN_MANAGE" + + +targets: + default: + + development: + variables: + app_config: + command: + - "python" + - "dev.py" + env: + - name: SOME_ENV_VARIABLE_2 + value: "Some value 2" + resources: + apps: + my_app: + source_code_path: ./app-dev + resources: + - name: "my-sql-warehouse" + sql_warehouse: + id: 1234 + permission: "CAN_MANAGE" + - name: "my-job" + job: + id: 5678 + permission: "CAN_MANAGE" + - name: "my-secret" + secret: + key: "key" + scope: "scope" + permission: "CAN_USE" diff --git a/bundle/tests/apps_test.go b/bundle/tests/apps_test.go new file mode 100644 index 0000000000..86c8eb08c7 --- /dev/null +++ b/bundle/tests/apps_test.go @@ -0,0 +1,61 @@ +package config_tests + +import ( + "context" + "testing" + + "github.com/databricks/cli/bundle" + "github.com/databricks/cli/bundle/config/mutator" + "github.com/stretchr/testify/assert" +) + +func TestApps(t *testing.T) { + b := load(t, "./apps") + assert.Equal(t, "apps", b.Config.Bundle.Name) + + diags := bundle.Apply(context.Background(), b, + bundle.Seq( + mutator.SetVariables(), + mutator.ResolveVariableReferences("variables"), + )) + assert.Empty(t, diags) + + 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, []interface{}{map[string]interface{}{"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) + assert.Equal(t, "CAN_USE", string(app.Resources[0].SqlWarehouse.Permission)) + assert.Equal(t, "5678", app.Resources[1].Job.Id) + assert.Equal(t, "CAN_MANAGE_RUN", string(app.Resources[1].Job.Permission)) +} + +func TestAppsOverride(t *testing.T) { + b := loadTarget(t, "./apps", "development") + assert.Equal(t, "apps", b.Config.Bundle.Name) + + diags := bundle.Apply(context.Background(), b, + bundle.Seq( + mutator.SetVariables(), + mutator.ResolveVariableReferences("variables"), + )) + assert.Empty(t, diags) + 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, []interface{}{map[string]interface{}{"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) + assert.Equal(t, "CAN_MANAGE", string(app.Resources[0].SqlWarehouse.Permission)) + assert.Equal(t, "5678", app.Resources[1].Job.Id) + assert.Equal(t, "CAN_MANAGE", string(app.Resources[1].Job.Permission)) + assert.Equal(t, "key", app.Resources[2].Secret.Key) + assert.Equal(t, "scope", app.Resources[2].Secret.Scope) + assert.Equal(t, "CAN_USE", string(app.Resources[2].Secret.Permission)) + +} diff --git a/bundle/tests/loader.go b/bundle/tests/loader.go index 5c48d81cb9..8e009db4ec 100644 --- a/bundle/tests/loader.go +++ b/bundle/tests/loader.go @@ -46,6 +46,7 @@ func loadTargetWithDiags(path, env string) (*bundle.Bundle, diag.Diagnostics) { mutator.MergeJobParameters(), mutator.MergeJobTasks(), mutator.MergePipelineClusters(), + mutator.MergeApps(), )) return b, diags } From 395645f3f460d5cabac1413471ae1df8e052e200 Mon Sep 17 00:00:00 2001 From: Andrew Nester Date: Mon, 2 Dec 2024 16:04:46 +0100 Subject: [PATCH 02/25] Added integration test --- bundle/config/resources/apps.go | 2 +- bundle/run/app.go | 80 +++++++++++++------ integration/bundle/apps_test.go | 80 +++++++++++++++++++ .../apps/databricks_template_schema.json | 24 ++++++ .../bundle/bundles/apps/template/app/app.py | 15 ++++ .../bundles/apps/template/databricks.yml.tmpl | 42 ++++++++++ .../bundles/apps/template/hello_world.py | 1 + integration/bundle/helpers_test.go | 11 ++- 8 files changed, 227 insertions(+), 28 deletions(-) create mode 100644 integration/bundle/apps_test.go create mode 100644 integration/bundle/bundles/apps/databricks_template_schema.json create mode 100644 integration/bundle/bundles/apps/template/app/app.py create mode 100644 integration/bundle/bundles/apps/template/databricks.yml.tmpl create mode 100644 integration/bundle/bundles/apps/template/hello_world.py diff --git a/bundle/config/resources/apps.go b/bundle/config/resources/apps.go index aad9308185..e6d3f00e84 100644 --- a/bundle/config/resources/apps.go +++ b/bundle/config/resources/apps.go @@ -51,7 +51,7 @@ func (a *App) Exists(ctx context.Context, w *databricks.WorkspaceClient, name st } func (a *App) TerraformResourceName() string { - return "databricks_cluster" + return "databricks_app" } func (a *App) InitializeURL(baseURL url.URL) { diff --git a/bundle/run/app.go b/bundle/run/app.go index 84ed43087e..ec46de9abd 100644 --- a/bundle/run/app.go +++ b/bundle/run/app.go @@ -16,6 +16,7 @@ import ( "github.com/databricks/cli/libs/filer" "github.com/databricks/databricks-sdk-go/service/apps" "github.com/spf13/cobra" + "golang.org/x/sync/errgroup" "gopkg.in/yaml.v3" ) @@ -90,8 +91,7 @@ func (a *appRunner) Run(ctx context.Context, opts *Options) (output.RunOutput, e return nil, err } - // TODO: We should return the app URL here. - cmdio.LogString(ctx, "You can access the app at ") + cmdio.LogString(ctx, fmt.Sprintf("You can access the app at %s", createdApp.Url)) return nil, nil } @@ -106,7 +106,7 @@ func (a *appRunner) start(ctx context.Context) error { return err } - startedApp, err := wait.OnProgress(func(p *apps.App) { + _, err = wait.OnProgress(func(p *apps.App) { if p.AppStatus == nil { return } @@ -117,28 +117,6 @@ func (a *appRunner) start(ctx context.Context) error { return err } - // If the app has a pending deployment, wait for it to complete. - if startedApp.PendingDeployment != nil { - _, err := w.Apps.WaitGetDeploymentAppSucceeded(ctx, - startedApp.Name, startedApp.PendingDeployment.DeploymentId, - 20*time.Minute, nil) - - if err != nil { - return err - } - } - - // If the app has an active deployment, wait for it to complete as well - if startedApp.ActiveDeployment != nil { - _, err := w.Apps.WaitGetDeploymentAppSucceeded(ctx, - startedApp.Name, startedApp.ActiveDeployment.DeploymentId, - 20*time.Minute, nil) - - if err != nil { - return err - } - } - logProgress(ctx, "App is started!") return nil } @@ -182,8 +160,27 @@ func (a *appRunner) deploy(ctx context.Context) error { }, }) + // If deploy returns an error, then there's an active deployment in progress, wait for it to complete. if err != nil { - return err + logProgress(ctx, err.Error()) + err := a.waitForInProgressDeployments(ctx) + if err != nil { + return err + } + + // Retry the deployment. + wait, err = w.Apps.Deploy(ctx, apps.CreateAppDeploymentRequest{ + AppName: app.Name, + AppDeployment: &apps.AppDeployment{ + Mode: apps.AppDeploymentModeSnapshot, + SourceCodePath: app.SourceCodePath, + }, + }) + + // If it still fails, return the error. + if err != nil { + return err + } } _, err = wait.OnProgress(func(ad *apps.AppDeployment) { @@ -200,6 +197,37 @@ func (a *appRunner) deploy(ctx context.Context) error { return nil } +func (a *appRunner) waitForInProgressDeployments(ctx context.Context) error { + app := a.app + b := a.bundle + w := b.WorkspaceClient() + + allDeployments, err := w.Apps.ListDeploymentsAll(ctx, apps.ListAppDeploymentsRequest{AppName: app.Name}) + if err != nil { + return err + } + + errGrp, ctx := errgroup.WithContext(ctx) + for _, deployment := range allDeployments { + if deployment.Status.State == apps.AppDeploymentStateInProgress { + id := deployment.DeploymentId + + errGrp.Go(func() error { + logProgress(ctx, fmt.Sprintf("Waiting for deployment %s to complete", id)) + d, err := w.Apps.WaitGetDeploymentAppSucceeded(ctx, app.Name, id, 20*time.Minute, nil) + if err != nil { + return err + } + + logProgress(ctx, fmt.Sprintf("Deployment %s reached state %s", id, d.Status.State)) + return nil + }) + } + } + + return errGrp.Wait() +} + func (a *appRunner) Cancel(ctx context.Context) error { // We should cancel the app by stopping it. app := a.app diff --git a/integration/bundle/apps_test.go b/integration/bundle/apps_test.go new file mode 100644 index 0000000000..ab4d3cb79d --- /dev/null +++ b/integration/bundle/apps_test.go @@ -0,0 +1,80 @@ +package bundle_test + +import ( + "fmt" + "testing" + + "github.com/databricks/cli/internal" + "github.com/databricks/cli/internal/acc" + "github.com/databricks/cli/libs/env" + "github.com/databricks/databricks-sdk-go/service/apps" + "github.com/google/uuid" + "github.com/stretchr/testify/require" +) + +func TestAccDeployBundleWithApp(t *testing.T) { + ctx, wt := acc.WorkspaceTest(t) + uniqueId := uuid.New().String() + appId := fmt.Sprintf("app-%s", uuid.New().String()[0:8]) + nodeTypeId := internal.GetNodeTypeId(env.Get(ctx, "CLOUD_ENV")) + instancePoolId := env.Get(ctx, "TEST_INSTANCE_POOL_ID") + + root, err := initTestTemplate(t, ctx, "apps", map[string]any{ + "unique_id": uniqueId, + "app_id": appId, + "node_type_id": nodeTypeId, + "spark_version": defaultSparkVersion, + "instance_pool_id": instancePoolId, + }) + require.NoError(t, err) + + t.Cleanup(func() { + err = destroyBundle(t, ctx, root) + require.NoError(t, err) + + app, err := wt.W.Apps.Get(ctx, apps.GetAppRequest{Name: "test-app"}) + if err != nil { + require.ErrorContains(t, err, "does not exist") + } else { + require.Contains(t, []apps.ApplicationState{apps.ApplicationStateUnavailable}, app.AppStatus.State) + } + }) + + err = deployBundle(t, ctx, root) + require.NoError(t, err) + + // App should exists after bundle deployment + app, err := wt.W.Apps.Get(ctx, apps.GetAppRequest{Name: appId}) + require.NoError(t, err) + require.NotNil(t, app) + + // Try to run the app + _, out, err := runResourceWithStderr(t, ctx, root, "test_app") + require.NoError(t, err) + require.Contains(t, out, app.Url) + + // App should be in the running state + app, err = wt.W.Apps.Get(ctx, apps.GetAppRequest{Name: appId}) + require.NoError(t, err) + require.NotNil(t, app) + require.Equal(t, apps.ApplicationStateRunning, app.AppStatus.State) + + // Stop the app + wait, err := wt.W.Apps.Stop(ctx, apps.StopAppRequest{Name: appId}) + require.NoError(t, err) + app, err = wait.Get() + require.NoError(t, err) + require.NotNil(t, app) + require.Equal(t, apps.ApplicationStateUnavailable, app.AppStatus.State) + + // Try to run the app again + _, out, err = runResourceWithStderr(t, ctx, root, "test_app") + require.NoError(t, err) + require.Contains(t, out, app.Url) + + // App should be in the running state + app, err = wt.W.Apps.Get(ctx, apps.GetAppRequest{Name: appId}) + require.NoError(t, err) + require.NotNil(t, app) + require.Equal(t, apps.ApplicationStateRunning, app.AppStatus.State) +} diff --git a/integration/bundle/bundles/apps/databricks_template_schema.json b/integration/bundle/bundles/apps/databricks_template_schema.json new file mode 100644 index 0000000000..c9faeabf3d --- /dev/null +++ b/integration/bundle/bundles/apps/databricks_template_schema.json @@ -0,0 +1,24 @@ +{ + "properties": { + "unique_id": { + "type": "string", + "description": "Unique ID for job name" + }, + "app_id": { + "type": "string", + "description": "Unique ID for app name" + }, + "spark_version": { + "type": "string", + "description": "Spark version used for job cluster" + }, + "node_type_id": { + "type": "string", + "description": "Node type id for job cluster" + }, + "instance_pool_id": { + "type": "string", + "description": "Instance pool id for job cluster" + } + } +} diff --git a/integration/bundle/bundles/apps/template/app/app.py b/integration/bundle/bundles/apps/template/app/app.py new file mode 100644 index 0000000000..a60c786fe3 --- /dev/null +++ b/integration/bundle/bundles/apps/template/app/app.py @@ -0,0 +1,15 @@ +import os + +from databricks.sdk import WorkspaceClient +from flask import Flask + +app = Flask(__name__) + + +@app.route("/") +def home(): + job_id = os.getenv("JOB_ID") + + w = WorkspaceClient() + job = w.jobs.get(job_id) + return job.settings.name diff --git a/integration/bundle/bundles/apps/template/databricks.yml.tmpl b/integration/bundle/bundles/apps/template/databricks.yml.tmpl new file mode 100644 index 0000000000..9ab21bf6f3 --- /dev/null +++ b/integration/bundle/bundles/apps/template/databricks.yml.tmpl @@ -0,0 +1,42 @@ +bundle: + name: basic + +workspace: + root_path: "~/.bundle/{{.unique_id}}" + +resources: + apps: + test_app: + name: "{{.app_id}}" + description: "App which manages job created by this bundle" + source_code_path: ./app + config: + command: + - flask + - --app + - app + - run + env: + - name: JOB_ID + valueFrom: "app-job" + + resources: + - name: "app-job" + description: "A job for app to be able to work with" + job: + id: ${resources.jobs.foo.id} + permission: "CAN_MANAGE_RUN" + + jobs: + foo: + name: test-job-with-cluster-{{.unique_id}} + tasks: + - task_key: my_notebook_task + new_cluster: + num_workers: 1 + spark_version: "{{.spark_version}}" + node_type_id: "{{.node_type_id}}" + data_security_mode: USER_ISOLATION + instance_pool_id: "{{.instance_pool_id}}" + spark_python_task: + python_file: ./hello_world.py diff --git a/integration/bundle/bundles/apps/template/hello_world.py b/integration/bundle/bundles/apps/template/hello_world.py new file mode 100644 index 0000000000..f301245e24 --- /dev/null +++ b/integration/bundle/bundles/apps/template/hello_world.py @@ -0,0 +1 @@ +print("Hello World!") diff --git a/integration/bundle/helpers_test.go b/integration/bundle/helpers_test.go index e884cd8c68..081e42ee6e 100644 --- a/integration/bundle/helpers_test.go +++ b/integration/bundle/helpers_test.go @@ -119,7 +119,16 @@ func runResource(t testutil.TestingT, ctx context.Context, path, key string) (st return stdout.String(), err } -func runResourceWithParams(t testutil.TestingT, ctx context.Context, path, key string, params ...string) (string, error) { +func runResourceWithStderr(t testutil.TestingT, ctx context.Context, path string, key string) (string, string, error) { + ctx = env.Set(ctx, "BUNDLE_ROOT", path) + ctx = cmdio.NewContext(ctx, cmdio.Default()) + + c := internal.NewCobraTestRunnerWithContext(t, ctx, "bundle", "run", key) + stdout, stderr, err := c.Run() + return stdout.String(), stderr.String(), err +} + +func runResourceWithParams(t testutil.TestingT, ctx context.Context, path string, key string, params ...string) (string, error) { ctx = env.Set(ctx, "BUNDLE_ROOT", path) ctx = cmdio.NewContext(ctx, cmdio.Default()) From f51eefe6bde1c847352c22d1766ccddddba1fd7d Mon Sep 17 00:00:00 2001 From: Andrew Nester Date: Mon, 2 Dec 2024 16:22:00 +0100 Subject: [PATCH 03/25] fix test --- bundle/run/app_test.go | 95 +++++++++++++++++++++++++++++++++++------- 1 file changed, 81 insertions(+), 14 deletions(-) diff --git a/bundle/run/app_test.go b/bundle/run/app_test.go index c0da444289..c00534df3d 100644 --- a/bundle/run/app_test.go +++ b/bundle/run/app_test.go @@ -3,6 +3,7 @@ package run import ( "bytes" "context" + "fmt" "os" "path/filepath" "testing" @@ -161,26 +162,92 @@ func TestAppRunStoppedApp(t *testing.T) { ComputeStatus: &apps.ComputeStatus{ State: apps.ComputeStateActive, }, - ActiveDeployment: &apps.AppDeployment{ - SourceCodePath: "/foo/bar", - DeploymentId: "123", - Status: &apps.AppDeploymentStatus{ - State: apps.AppDeploymentStateInProgress, - }, + }, nil + }, + }, nil) + + r.run(t) +} + +func TestAppRunWithAnActiveDeploymentInProgress(t *testing.T) { + r := setupTestApp(t, apps.ApplicationStateCrashed, apps.ComputeStateStopped) + + appsApi := r.m.GetMockAppsAPI() + appsApi.EXPECT().Start(mock.Anything, apps.StartAppRequest{ + Name: "my_app", + }).Return(&apps.WaitGetAppActive[apps.App]{ + Poll: func(_ time.Duration, _ func(*apps.App)) (*apps.App, error) { + return &apps.App{ + Name: "my_app", + AppStatus: &apps.ApplicationStatus{ + State: apps.ApplicationStateRunning, }, - PendingDeployment: &apps.AppDeployment{ - SourceCodePath: "/foo/bar", - DeploymentId: "456", - Status: &apps.AppDeploymentStatus{ - State: apps.AppDeploymentStateInProgress, - }, + ComputeStatus: &apps.ComputeStatus{ + State: apps.ComputeStateActive, }, }, nil }, }, nil) - appsApi.EXPECT().WaitGetDeploymentAppSucceeded(mock.Anything, "my_app", "123", mock.Anything, mock.Anything).Return(nil, nil) - appsApi.EXPECT().WaitGetDeploymentAppSucceeded(mock.Anything, "my_app", "456", mock.Anything, mock.Anything).Return(nil, nil) + // First unset existing deploy handlers + appsApi.EXPECT().Deploy(mock.Anything, apps.CreateAppDeploymentRequest{ + AppName: "my_app", + AppDeployment: &apps.AppDeployment{ + Mode: apps.AppDeploymentModeSnapshot, + SourceCodePath: "/Workspace/Users/foo@bar.com/files/my_app", + }, + }).Unset() + + // The first deploy call will return an error + appsApi.EXPECT().Deploy(mock.Anything, apps.CreateAppDeploymentRequest{ + AppName: "my_app", + AppDeployment: &apps.AppDeployment{ + Mode: apps.AppDeploymentModeSnapshot, + SourceCodePath: "/Workspace/Users/foo@bar.com/files/my_app", + }, + }).Return(nil, fmt.Errorf("deployment in progress")).Once() + + // We will then list deployments and find all in progress deployments. + appsApi.EXPECT().ListDeploymentsAll(mock.Anything, apps.ListAppDeploymentsRequest{ + AppName: "my_app", + }).Return([]apps.AppDeployment{ + { + DeploymentId: "deployment-1", + Status: &apps.AppDeploymentStatus{ + State: apps.AppDeploymentStateInProgress, + }, + }, + { + DeploymentId: "deployment-2", + Status: &apps.AppDeploymentStatus{ + State: apps.AppDeploymentStateSucceeded, + }, + }, + }, nil) + + // Wait for the in progress deployment to complete + appsApi.EXPECT().WaitGetDeploymentAppSucceeded(mock.Anything, "my_app", "deployment-1", mock.Anything, mock.Anything).Return(&apps.AppDeployment{ + DeploymentId: "deployment-1", + Status: &apps.AppDeploymentStatus{ + State: apps.AppDeploymentStateSucceeded, + }, + }, nil) + + wait := &apps.WaitGetDeploymentAppSucceeded[apps.AppDeployment]{ + Poll: func(_ time.Duration, _ func(*apps.AppDeployment)) (*apps.AppDeployment, error) { + return nil, nil + }, + } + + // Retry the deployment + appsApi.EXPECT().Deploy(mock.Anything, apps.CreateAppDeploymentRequest{ + AppName: "my_app", + AppDeployment: &apps.AppDeployment{ + Mode: apps.AppDeploymentModeSnapshot, + SourceCodePath: "/Workspace/Users/foo@bar.com/files/my_app", + }, + }).Return(wait, nil) + r.run(t) } From 483a239cce29b18fc34cffa13f93b5283f0d5556 Mon Sep 17 00:00:00 2001 From: Andrew Nester Date: Tue, 3 Dec 2024 13:57:34 +0100 Subject: [PATCH 04/25] chnaged to wait for deployment on start --- bundle/run/app.go | 74 +++++++++++++----------------------------- bundle/run/app_test.go | 72 ++++++++-------------------------------- 2 files changed, 35 insertions(+), 111 deletions(-) diff --git a/bundle/run/app.go b/bundle/run/app.go index ec46de9abd..8a4dc34025 100644 --- a/bundle/run/app.go +++ b/bundle/run/app.go @@ -16,7 +16,6 @@ import ( "github.com/databricks/cli/libs/filer" "github.com/databricks/databricks-sdk-go/service/apps" "github.com/spf13/cobra" - "golang.org/x/sync/errgroup" "gopkg.in/yaml.v3" ) @@ -106,7 +105,7 @@ func (a *appRunner) start(ctx context.Context) error { return err } - _, err = wait.OnProgress(func(p *apps.App) { + startedApp, err := wait.OnProgress(func(p *apps.App) { if p.AppStatus == nil { return } @@ -117,6 +116,26 @@ func (a *appRunner) start(ctx context.Context) error { return err } + if startedApp.ActiveDeployment != nil && + startedApp.ActiveDeployment.Status.State == apps.AppDeploymentStateInProgress { + logProgress(ctx, "Waiting for the active deployment to complete...") + _, err = w.Apps.WaitGetDeploymentAppSucceeded(ctx, app.Name, startedApp.ActiveDeployment.DeploymentId, 20*time.Minute, nil) + if err != nil { + return err + } + logProgress(ctx, "Active deployment is completed!") + } + + if startedApp.PendingDeployment != nil && + startedApp.PendingDeployment.Status.State == apps.AppDeploymentStateInProgress { + logProgress(ctx, "Waiting for the pending deployment to complete...") + _, err = w.Apps.WaitGetDeploymentAppSucceeded(ctx, app.Name, startedApp.PendingDeployment.DeploymentId, 20*time.Minute, nil) + if err != nil { + return err + } + logProgress(ctx, "Pending deployment is completed!") + } + logProgress(ctx, "App is started!") return nil } @@ -162,25 +181,7 @@ func (a *appRunner) deploy(ctx context.Context) error { // If deploy returns an error, then there's an active deployment in progress, wait for it to complete. if err != nil { - logProgress(ctx, err.Error()) - err := a.waitForInProgressDeployments(ctx) - if err != nil { - return err - } - - // Retry the deployment. - wait, err = w.Apps.Deploy(ctx, apps.CreateAppDeploymentRequest{ - AppName: app.Name, - AppDeployment: &apps.AppDeployment{ - Mode: apps.AppDeploymentModeSnapshot, - SourceCodePath: app.SourceCodePath, - }, - }) - - // If it still fails, return the error. - if err != nil { - return err - } + return err } _, err = wait.OnProgress(func(ad *apps.AppDeployment) { @@ -197,37 +198,6 @@ func (a *appRunner) deploy(ctx context.Context) error { return nil } -func (a *appRunner) waitForInProgressDeployments(ctx context.Context) error { - app := a.app - b := a.bundle - w := b.WorkspaceClient() - - allDeployments, err := w.Apps.ListDeploymentsAll(ctx, apps.ListAppDeploymentsRequest{AppName: app.Name}) - if err != nil { - return err - } - - errGrp, ctx := errgroup.WithContext(ctx) - for _, deployment := range allDeployments { - if deployment.Status.State == apps.AppDeploymentStateInProgress { - id := deployment.DeploymentId - - errGrp.Go(func() error { - logProgress(ctx, fmt.Sprintf("Waiting for deployment %s to complete", id)) - d, err := w.Apps.WaitGetDeploymentAppSucceeded(ctx, app.Name, id, 20*time.Minute, nil) - if err != nil { - return err - } - - logProgress(ctx, fmt.Sprintf("Deployment %s reached state %s", id, d.Status.State)) - return nil - }) - } - } - - return errGrp.Wait() -} - func (a *appRunner) Cancel(ctx context.Context) error { // We should cancel the app by stopping it. app := a.app diff --git a/bundle/run/app_test.go b/bundle/run/app_test.go index c00534df3d..98f05127b6 100644 --- a/bundle/run/app_test.go +++ b/bundle/run/app_test.go @@ -3,7 +3,6 @@ package run import ( "bytes" "context" - "fmt" "os" "path/filepath" "testing" @@ -185,68 +184,23 @@ func TestAppRunWithAnActiveDeploymentInProgress(t *testing.T) { ComputeStatus: &apps.ComputeStatus{ State: apps.ComputeStateActive, }, + ActiveDeployment: &apps.AppDeployment{ + DeploymentId: "active_deployment_id", + Status: &apps.AppDeploymentStatus{ + State: apps.AppDeploymentStateInProgress, + }, + }, + PendingDeployment: &apps.AppDeployment{ + DeploymentId: "pending_deployment_id", + Status: &apps.AppDeploymentStatus{ + State: apps.AppDeploymentStateCancelled, + }, + }, }, nil }, }, nil) - // First unset existing deploy handlers - appsApi.EXPECT().Deploy(mock.Anything, apps.CreateAppDeploymentRequest{ - AppName: "my_app", - AppDeployment: &apps.AppDeployment{ - Mode: apps.AppDeploymentModeSnapshot, - SourceCodePath: "/Workspace/Users/foo@bar.com/files/my_app", - }, - }).Unset() - - // The first deploy call will return an error - appsApi.EXPECT().Deploy(mock.Anything, apps.CreateAppDeploymentRequest{ - AppName: "my_app", - AppDeployment: &apps.AppDeployment{ - Mode: apps.AppDeploymentModeSnapshot, - SourceCodePath: "/Workspace/Users/foo@bar.com/files/my_app", - }, - }).Return(nil, fmt.Errorf("deployment in progress")).Once() - - // We will then list deployments and find all in progress deployments. - appsApi.EXPECT().ListDeploymentsAll(mock.Anything, apps.ListAppDeploymentsRequest{ - AppName: "my_app", - }).Return([]apps.AppDeployment{ - { - DeploymentId: "deployment-1", - Status: &apps.AppDeploymentStatus{ - State: apps.AppDeploymentStateInProgress, - }, - }, - { - DeploymentId: "deployment-2", - Status: &apps.AppDeploymentStatus{ - State: apps.AppDeploymentStateSucceeded, - }, - }, - }, nil) - - // Wait for the in progress deployment to complete - appsApi.EXPECT().WaitGetDeploymentAppSucceeded(mock.Anything, "my_app", "deployment-1", mock.Anything, mock.Anything).Return(&apps.AppDeployment{ - DeploymentId: "deployment-1", - Status: &apps.AppDeploymentStatus{ - State: apps.AppDeploymentStateSucceeded, - }, - }, nil) - - wait := &apps.WaitGetDeploymentAppSucceeded[apps.AppDeployment]{ - Poll: func(_ time.Duration, _ func(*apps.AppDeployment)) (*apps.AppDeployment, error) { - return nil, nil - }, - } - - // Retry the deployment - appsApi.EXPECT().Deploy(mock.Anything, apps.CreateAppDeploymentRequest{ - AppName: "my_app", - AppDeployment: &apps.AppDeployment{ - Mode: apps.AppDeploymentModeSnapshot, - SourceCodePath: "/Workspace/Users/foo@bar.com/files/my_app", - }, - }).Return(wait, nil) + appsApi.EXPECT().WaitGetDeploymentAppSucceeded(mock.Anything, "my_app", "active_deployment_id", mock.Anything, mock.Anything).Return(nil, nil) r.run(t) } From ddc012c324418dc95952f0a57770d92933658b63 Mon Sep 17 00:00:00 2001 From: Andrew Nester Date: Thu, 5 Dec 2024 15:40:34 +0100 Subject: [PATCH 05/25] addressed feedback --- bundle/apps/validate.go | 41 +++++++++++++ bundle/apps/validate_test.go | 55 ++++++++++++++++++ bundle/config/mutator/apply_presets.go | 13 +---- bundle/config/mutator/apply_presets_test.go | 57 ------------------- bundle/config/mutator/merge_apps.go | 2 +- bundle/config/mutator/merge_apps_test.go | 11 +++- .../mutator/process_target_mode_test.go | 6 ++ bundle/config/mutator/run_as.go | 10 ++++ bundle/config/resources/apps.go | 3 +- bundle/phases/initialize.go | 3 + libs/dyn/merge/elements_by_key.go | 28 ++++++++- 11 files changed, 155 insertions(+), 74 deletions(-) create mode 100644 bundle/apps/validate.go create mode 100644 bundle/apps/validate_test.go diff --git a/bundle/apps/validate.go b/bundle/apps/validate.go new file mode 100644 index 0000000000..5e3082f2d8 --- /dev/null +++ b/bundle/apps/validate.go @@ -0,0 +1,41 @@ +package apps + +import ( + "context" + "fmt" + "os" + "path/filepath" + + "github.com/databricks/cli/bundle" + "github.com/databricks/cli/libs/diag" +) + +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 _, configFile := range possibleConfigFiles { + cf := filepath.Join(b.SyncRootPath, app.SourceCodePath, configFile) + if _, err := os.Stat(cf); err == nil { + diags = append(diags, diag.Diagnostic{ + Severity: diag.Error, + Summary: "app.yml detected", + Detail: fmt.Sprintf("remove %s and use 'config' property for app resource '%s' instead", cf, app.Name), + }) + } + } + } + + return diags +} + +func (v *validate) Name() string { + return "apps.Validate" +} + +func Validate() bundle.Mutator { + return &validate{} +} diff --git a/bundle/apps/validate_test.go b/bundle/apps/validate_test.go new file mode 100644 index 0000000000..d14d23731f --- /dev/null +++ b/bundle/apps/validate_test.go @@ -0,0 +1,55 @@ +package apps + +import ( + "context" + "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/resources" + "github.com/databricks/cli/bundle/internal/bundletest" + "github.com/databricks/cli/internal/testutil" + "github.com/databricks/cli/libs/dyn" + "github.com/databricks/cli/libs/vfs" + "github.com/databricks/databricks-sdk-go/service/apps" + "github.com/stretchr/testify/require" +) + +func TestAppsValidate(t *testing.T) { + tmpDir := t.TempDir() + testutil.Touch(t, tmpDir, "app1", "app.yml") + testutil.Touch(t, tmpDir, "app2", "app.py") + + b := &bundle.Bundle{ + BundleRootPath: tmpDir, + SyncRootPath: tmpDir, + SyncRoot: vfs.MustNew(tmpDir), + Config: config.Root{ + Resources: config.Resources{ + Apps: map[string]*resources.App{ + "app1": { + App: &apps.App{ + Name: "app1", + }, + SourceCodePath: "./app1", + }, + "app2": { + App: &apps.App{ + Name: "app2", + }, + SourceCodePath: "./app2", + }, + }, + }, + }, + } + + bundletest.SetLocation(b, ".", []dyn.Location{{File: filepath.Join(tmpDir, "databricks.yml")}}) + + diags := bundle.Apply(context.Background(), b, bundle.Seq(mutator.TranslatePaths(), Validate())) + require.Len(t, diags, 1) + require.Equal(t, "app.yml detected", diags[0].Summary) + require.Contains(t, diags[0].Detail, "app.yml and use 'config' property for app resource") +} diff --git a/bundle/config/mutator/apply_presets.go b/bundle/config/mutator/apply_presets.go index 57b765a8d7..049fa9c8f9 100644 --- a/bundle/config/mutator/apply_presets.go +++ b/bundle/config/mutator/apply_presets.go @@ -222,18 +222,7 @@ func (m *applyPresets) Apply(ctx context.Context, b *bundle.Bundle) diag.Diagnos dashboard.DisplayName = prefix + dashboard.DisplayName } - // Apps: Prefix - for key, app := range r.Apps { - if app == nil || app.App == nil { - diags = diags.Extend(diag.Errorf("app %s is not defined", key)) - continue - } - - app.Name = textutil.NormalizeString(prefix + app.Name) - // Normalize the app name to ensure it is a valid identifier. - // App supports only alphanumeric characters and hyphens. - app.Name = strings.ReplaceAll(app.Name, "_", "-") - } + // Apps: No presets if config.IsExplicitlyEnabled((b.Config.Presets.SourceLinkedDeployment)) { isDatabricksWorkspace := dbr.RunsOnRuntime(ctx) && strings.HasPrefix(b.SyncRootPath, "/Workspace/") diff --git a/bundle/config/mutator/apply_presets_test.go b/bundle/config/mutator/apply_presets_test.go index 2af54e74e2..c26f203832 100644 --- a/bundle/config/mutator/apply_presets_test.go +++ b/bundle/config/mutator/apply_presets_test.go @@ -12,7 +12,6 @@ import ( "github.com/databricks/cli/bundle/internal/bundletest" "github.com/databricks/cli/libs/dbr" "github.com/databricks/cli/libs/dyn" - "github.com/databricks/databricks-sdk-go/service/apps" "github.com/databricks/databricks-sdk-go/service/catalog" "github.com/databricks/databricks-sdk-go/service/jobs" "github.com/stretchr/testify/require" @@ -483,59 +482,3 @@ func TestApplyPresetsSourceLinkedDeployment(t *testing.T) { }) } } - -func TestApplyPresetsPrefixForApps(t *testing.T) { - tests := []struct { - name string - prefix string - app *resources.App - want string - }{ - { - name: "add prefix to app", - prefix: "[prefix] ", - app: &resources.App{ - App: &apps.App{ - Name: "app1", - }, - }, - want: "prefix-app1", - }, - { - name: "add empty prefix to app", - prefix: "", - app: &resources.App{ - App: &apps.App{ - Name: "app1", - }, - }, - want: "app1", - }, - } - - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - b := &bundle.Bundle{ - Config: config.Root{ - Resources: config.Resources{ - Apps: map[string]*resources.App{ - "app1": tt.app, - }, - }, - Presets: config.Presets{ - NamePrefix: tt.prefix, - }, - }, - } - - ctx := context.Background() - diag := bundle.Apply(ctx, b, mutator.ApplyPresets()) - - if diag.HasError() { - t.Fatalf("unexpected error: %v", diag) - } - - require.Equal(t, tt.want, b.Config.Resources.Apps["app1"].Name) - }) - } -} diff --git a/bundle/config/mutator/merge_apps.go b/bundle/config/mutator/merge_apps.go index 88c745a834..3be9ae6d2f 100644 --- a/bundle/config/mutator/merge_apps.go +++ b/bundle/config/mutator/merge_apps.go @@ -37,7 +37,7 @@ func (m *mergeApps) Apply(ctx context.Context, b *bundle.Bundle) diag.Diagnostic } return dyn.Map(v, "resources.apps", dyn.Foreach(func(_ dyn.Path, app dyn.Value) (dyn.Value, error) { - return dyn.Map(app, "resources", merge.ElementsByKey("name", m.resourceName)) + return dyn.Map(app, "resources", merge.ElementsByKeyWithOverride("name", m.resourceName)) })) }) diff --git a/bundle/config/mutator/merge_apps_test.go b/bundle/config/mutator/merge_apps_test.go index 2cdef830ec..0a161b8452 100644 --- a/bundle/config/mutator/merge_apps_test.go +++ b/bundle/config/mutator/merge_apps_test.go @@ -42,6 +42,13 @@ func TestMergeApps(t *testing.T) { Permission: "CAN_MANAGE", }, }, + { + Name: "sql1", + Job: &apps.AppResourceJob{ + Id: "9876", + Permission: "CAN_MANAGE", + }, + }, }, }, }, @@ -60,5 +67,7 @@ func TestMergeApps(t *testing.T) { assert.Equal(t, "sql1", j.Resources[1].Name) assert.Equal(t, "CAN_MANAGE", string(j.Resources[0].Job.Permission)) - assert.Equal(t, "CAN_USE", string(j.Resources[1].SqlWarehouse.Permission)) + + assert.Nil(t, j.Resources[1].SqlWarehouse) + assert.Equal(t, "CAN_MANAGE", string(j.Resources[1].Job.Permission)) } diff --git a/bundle/config/mutator/process_target_mode_test.go b/bundle/config/mutator/process_target_mode_test.go index ce75dc2fa1..db55e8edc2 100644 --- a/bundle/config/mutator/process_target_mode_test.go +++ b/bundle/config/mutator/process_target_mode_test.go @@ -426,6 +426,12 @@ func TestAllNonUcResourcesAreRenamed(t *testing.T) { for _, key := range field.MapKeys() { resource := field.MapIndex(key) nameField := resource.Elem().FieldByName("Name") + + // Skip apps, as they are not renamed + if resourceType == "Apps" { + continue + } + if !nameField.IsValid() || nameField.Kind() != reflect.String { continue } diff --git a/bundle/config/mutator/run_as.go b/bundle/config/mutator/run_as.go index 7ffd782c2b..3d7391b015 100644 --- a/bundle/config/mutator/run_as.go +++ b/bundle/config/mutator/run_as.go @@ -119,6 +119,16 @@ func validateRunAs(b *bundle.Bundle) diag.Diagnostics { )) } + // Apps do not support run_as in the API. + if len(b.Config.Resources.Apps) > 0 { + diags = diags.Extend(reportRunAsNotSupported( + "apps", + b.Config.GetLocation("resources.apps"), + b.Config.Workspace.CurrentUser.UserName, + identity, + )) + } + return diags } diff --git a/bundle/config/resources/apps.go b/bundle/config/resources/apps.go index e6d3f00e84..7484110965 100644 --- a/bundle/config/resources/apps.go +++ b/bundle/config/resources/apps.go @@ -14,6 +14,7 @@ import ( type App struct { // This represents the id which is the name of the app that can be used // as a reference in other resources. This value is returned by terraform. + // 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 @@ -23,7 +24,7 @@ type App struct { // 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 already, it will be overridden. + // If there’s app.yml defined locally, DABs will raise an error. Config map[string]interface{} `json:"config,omitempty"` Permissions []Permission `json:"permissions,omitempty"` diff --git a/bundle/phases/initialize.go b/bundle/phases/initialize.go index e8a6c8504b..8585912a88 100644 --- a/bundle/phases/initialize.go +++ b/bundle/phases/initialize.go @@ -2,6 +2,7 @@ package phases import ( "github.com/databricks/cli/bundle" + "github.com/databricks/cli/bundle/apps" "github.com/databricks/cli/bundle/config" "github.com/databricks/cli/bundle/config/mutator" pythonmutator "github.com/databricks/cli/bundle/config/mutator/python" @@ -82,6 +83,8 @@ func Initialize() bundle.Mutator { mutator.TranslatePaths(), trampoline.WrapperWarning(), + apps.Validate(), + permissions.ValidateSharedRootPermissions(), permissions.ApplyBundlePermissions(), permissions.FilterCurrentUser(), diff --git a/libs/dyn/merge/elements_by_key.go b/libs/dyn/merge/elements_by_key.go index e6e640d146..e94c5fdc74 100644 --- a/libs/dyn/merge/elements_by_key.go +++ b/libs/dyn/merge/elements_by_key.go @@ -7,7 +7,7 @@ type elementsByKey struct { keyFunc func(dyn.Value) string } -func (e elementsByKey) Map(_ dyn.Path, v dyn.Value) (dyn.Value, error) { +func (e elementsByKey) doMap(_ dyn.Path, v dyn.Value, mergeFunc func(a dyn.Value, b dyn.Value) (dyn.Value, error)) (dyn.Value, error) { // We know the type of this value is a sequence. // For additional defence, return self if it is not. elements, ok := v.AsSequence() @@ -33,7 +33,7 @@ func (e elementsByKey) Map(_ dyn.Path, v dyn.Value) (dyn.Value, error) { } // Merge this instance into the reference. - nv, err := Merge(ref, elements[i]) + nv, err := mergeFunc(ref, elements[i]) if err != nil { return v, err } @@ -55,6 +55,26 @@ func (e elementsByKey) Map(_ dyn.Path, v dyn.Value) (dyn.Value, error) { return dyn.NewValue(out, v.Locations()), nil } +func (e elementsByKey) Map(_ dyn.Path, v dyn.Value) (dyn.Value, error) { + return e.doMap(nil, v, Merge) +} + +func (e elementsByKey) MapWithOverride(p dyn.Path, v dyn.Value) (dyn.Value, error) { + return e.doMap(nil, v, func(a dyn.Value, b dyn.Value) (dyn.Value, error) { + return Override(a, b, OverrideVisitor{ + VisitInsert: func(_ dyn.Path, v dyn.Value) (dyn.Value, error) { + return v, nil + }, + VisitDelete: func(valuePath dyn.Path, left dyn.Value) error { + return nil + }, + VisitUpdate: func(_ dyn.Path, a dyn.Value, b dyn.Value) (dyn.Value, error) { + return b, nil + }, + }) + }) +} + // ElementsByKey returns a [dyn.MapFunc] that operates on a sequence // where each element is a map. It groups elements by a key and merges // elements with the same key. @@ -65,3 +85,7 @@ func (e elementsByKey) Map(_ dyn.Path, v dyn.Value) (dyn.Value, error) { func ElementsByKey(key string, keyFunc func(dyn.Value) string) dyn.MapFunc { return elementsByKey{key, keyFunc}.Map } + +func ElementsByKeyWithOverride(key string, keyFunc func(dyn.Value) string) dyn.MapFunc { + return elementsByKey{key, keyFunc}.MapWithOverride +} From 28d69a5144c3c2a98b95142ab30003ece7be4c02 Mon Sep 17 00:00:00 2001 From: Andrew Nester Date: Thu, 5 Dec 2024 15:47:08 +0100 Subject: [PATCH 06/25] fix fmt --- bundle/config/resources.go | 1 + 1 file changed, 1 insertion(+) diff --git a/bundle/config/resources.go b/bundle/config/resources.go index e6821c009b..bf81b92958 100644 --- a/bundle/config/resources.go +++ b/bundle/config/resources.go @@ -198,6 +198,7 @@ func SupportedResources() map[string]ResourceDescription { PluralName: "volumes", SingularTitle: "Volume", PluralTitle: "Volumes", + }, "apps": { SingularName: "app", PluralName: "apps", From 1a58f1b1e455ee47ae5f3e3d9a9f1c7076ec0e30 Mon Sep 17 00:00:00 2001 From: Andrew Nester Date: Tue, 10 Dec 2024 17:11:41 +0100 Subject: [PATCH 07/25] addressed feedback --- bundle/apps/upload_config.go | 105 ++++++++++++++++++ bundle/apps/upload_config_test.go | 69 ++++++++++++ bundle/apps/validate.go | 12 +- bundle/config/mutator/merge_apps.go | 2 +- bundle/config/resources/apps.go | 6 +- .../terraform/tfdyn/convert_app_test.go | 14 +-- bundle/run/app.go | 56 +--------- bundle/run/app_test.go | 33 ++---- bundle/run/runner.go | 4 - bundle/tests/apps_test.go | 8 +- libs/dyn/merge/elements_by_key_test.go | 39 +++++++ 11 files changed, 247 insertions(+), 101 deletions(-) create mode 100644 bundle/apps/upload_config.go create mode 100644 bundle/apps/upload_config_test.go diff --git a/bundle/apps/upload_config.go b/bundle/apps/upload_config.go new file mode 100644 index 0000000000..4f70976a3e --- /dev/null +++ b/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 +} diff --git a/bundle/apps/upload_config_test.go b/bundle/apps/upload_config_test.go new file mode 100644 index 0000000000..56e3101e9b --- /dev/null +++ b/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()) +} diff --git a/bundle/apps/validate.go b/bundle/apps/validate.go index 5e3082f2d8..d12d013134 100644 --- a/bundle/apps/validate.go +++ b/bundle/apps/validate.go @@ -3,8 +3,7 @@ package apps import ( "context" "fmt" - "os" - "path/filepath" + "path" "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 + possibleConfigFiles := []string{"app.yml", "app.yaml"} + for _, app := range b.Config.Resources.Apps { - possibleConfigFiles := []string{"app.yml", "app.yaml"} for _, configFile := range possibleConfigFiles { - cf := filepath.Join(b.SyncRootPath, app.SourceCodePath, configFile) - if _, err := os.Stat(cf); err == nil { + cf := path.Join(app.SourceCodePath, configFile) + 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), }) } diff --git a/bundle/config/mutator/merge_apps.go b/bundle/config/mutator/merge_apps.go index 3be9ae6d2f..d91e8dd7f9 100644 --- a/bundle/config/mutator/merge_apps.go +++ b/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") } } diff --git a/bundle/config/resources/apps.go b/bundle/config/resources/apps.go index 7484110965..00b7de4dda 100644 --- a/bundle/config/resources/apps.go +++ b/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 - // on local disk and use it to point to this source code in the app deployment + // SourceCodePath is a required field used by DABs to point to Databricks app source code + // 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"` diff --git a/bundle/deploy/terraform/tfdyn/convert_app_test.go b/bundle/deploy/terraform/tfdyn/convert_app_test.go index 95b9bdae45..5589610595 100644 --- a/bundle/deploy/terraform/tfdyn/convert_app_test.go +++ b/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{}{ - map[string]interface{}{ + "resource": []any{ + 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", }, diff --git a/bundle/run/app.go b/bundle/run/app.go index 8a4dc34025..2bbce10cc2 100644 --- a/bundle/run/app.go +++ b/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 -} diff --git a/bundle/run/app_test.go b/bundle/run/app_test.go index 98f05127b6..ddbdc2a309 100644 --- a/bundle/run/app_test.go +++ b/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" @@ -26,10 +24,9 @@ import ( ) type testAppRunner struct { - m *mocks.MockWorkspaceClient - b *bundle.Bundle - mockFiler *mockfiler.MockFiler - ctx context.Context + m *mocks.MockWorkspaceClient + b *bundle.Bundle + ctx context.Context } func (ta *testAppRunner) run(t *testing.T) { @@ -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,20 +117,10 @@ 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, + m: mwc, + b: b, + 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) diff --git a/bundle/run/runner.go b/bundle/run/runner.go index e4706ca531..23c2c0a41f 100644 --- a/bundle/run/runner.go +++ b/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) diff --git a/bundle/tests/apps_test.go b/bundle/tests/apps_test.go index 86c8eb08c7..2b777a327c 100644 --- a/bundle/tests/apps_test.go +++ b/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, []interface{}{map[string]interface{}{"name": "SOME_ENV_VARIABLE", "value": "Some value"}}, app.Config["env"]) + assert.Equal(t, []any{"python", "app.py"}, app.Config["command"]) + 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, []interface{}{map[string]interface{}{"name": "SOME_ENV_VARIABLE_2", "value": "Some value 2"}}, app.Config["env"]) + assert.Equal(t, []any{"python", "dev.py"}, app.Config["command"]) + 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) diff --git a/libs/dyn/merge/elements_by_key_test.go b/libs/dyn/merge/elements_by_key_test.go index ef316cc666..09efece074 100644 --- a/libs/dyn/merge/elements_by_key_test.go +++ b/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, + }, + ) +} From 2c359b4372bd8a3e3bf7f5389f12fec6630276d8 Mon Sep 17 00:00:00 2001 From: Andrew Nester Date: Wed, 11 Dec 2024 13:42:32 +0100 Subject: [PATCH 08/25] interpolate after --- bundle/apps/interpolate_variables.go | 69 +++++++++++++++++++ bundle/apps/interpolate_variables_test.go | 49 +++++++++++++ bundle/phases/deploy.go | 3 + integration/bundle/apps_test.go | 24 +++++++ .../bundles/apps/template/databricks.yml.tmpl | 2 +- 5 files changed, 146 insertions(+), 1 deletion(-) create mode 100644 bundle/apps/interpolate_variables.go create mode 100644 bundle/apps/interpolate_variables_test.go diff --git a/bundle/apps/interpolate_variables.go b/bundle/apps/interpolate_variables.go new file mode 100644 index 0000000000..206609a12f --- /dev/null +++ b/bundle/apps/interpolate_variables.go @@ -0,0 +1,69 @@ +package apps + +import ( + "context" + + "github.com/databricks/cli/bundle" + "github.com/databricks/cli/libs/diag" + "github.com/databricks/cli/libs/dyn" + "github.com/databricks/cli/libs/dyn/dynvar" +) + +type interpolateVariables struct{} + +func (i *interpolateVariables) Apply(ctx context.Context, b *bundle.Bundle) diag.Diagnostics { + pattern := dyn.NewPattern( + dyn.Key("resources"), + dyn.Key("apps"), + dyn.AnyKey(), + dyn.Key("config"), + ) + + err := b.Config.Mutate(func(root dyn.Value) (dyn.Value, error) { + return dyn.MapByPattern(root, pattern, func(p dyn.Path, v dyn.Value) (dyn.Value, error) { + return dynvar.Resolve(v, func(path dyn.Path) (dyn.Value, error) { + switch path[0] { + case dyn.Key("databricks_pipeline"): + path = dyn.NewPath(dyn.Key("resources"), dyn.Key("pipelines")).Append(path[1:]...) + case dyn.Key("databricks_job"): + path = dyn.NewPath(dyn.Key("resources"), dyn.Key("jobs")).Append(path[1:]...) + case dyn.Key("databricks_mlflow_model"): + path = dyn.NewPath(dyn.Key("resources"), dyn.Key("models")).Append(path[1:]...) + case dyn.Key("databricks_mlflow_experiment"): + path = dyn.NewPath(dyn.Key("resources"), dyn.Key("experiments")).Append(path[1:]...) + case dyn.Key("databricks_model_serving"): + path = dyn.NewPath(dyn.Key("resources"), dyn.Key("model_serving_endpoints")).Append(path[1:]...) + case dyn.Key("databricks_registered_model"): + path = dyn.NewPath(dyn.Key("resources"), dyn.Key("registered_models")).Append(path[1:]...) + case dyn.Key("databricks_quality_monitor"): + path = dyn.NewPath(dyn.Key("resources"), dyn.Key("quality_monitors")).Append(path[1:]...) + case dyn.Key("databricks_schema"): + path = dyn.NewPath(dyn.Key("resources"), dyn.Key("schemas")).Append(path[1:]...) + case dyn.Key("databricks_volume"): + path = dyn.NewPath(dyn.Key("resources"), dyn.Key("volumes")).Append(path[1:]...) + case dyn.Key("databricks_cluster"): + path = dyn.NewPath(dyn.Key("resources"), dyn.Key("clusters")).Append(path[1:]...) + case dyn.Key("databricks_dashboard"): + path = dyn.NewPath(dyn.Key("resources"), dyn.Key("dashboards")).Append(path[1:]...) + case dyn.Key("databricks_app"): + path = dyn.NewPath(dyn.Key("resources"), dyn.Key("apps")).Append(path[1:]...) + default: + // Trigger "key not found" for unknown resource types. + return dyn.GetByPath(root, path) + } + + return dyn.GetByPath(root, path) + }) + }) + }) + + return diag.FromErr(err) +} + +func (i *interpolateVariables) Name() string { + return "apps.InterpolateVariables" +} + +func InterpolateVariables() bundle.Mutator { + return &interpolateVariables{} +} diff --git a/bundle/apps/interpolate_variables_test.go b/bundle/apps/interpolate_variables_test.go new file mode 100644 index 0000000000..4fec8c6da3 --- /dev/null +++ b/bundle/apps/interpolate_variables_test.go @@ -0,0 +1,49 @@ +package apps + +import ( + "context" + "testing" + + "github.com/databricks/cli/bundle" + "github.com/databricks/cli/bundle/config" + "github.com/databricks/cli/bundle/config/resources" + "github.com/databricks/databricks-sdk-go/service/apps" + "github.com/stretchr/testify/require" +) + +func TestAppInterpolateVariables(t *testing.T) { + b := &bundle.Bundle{ + Config: config.Root{ + Resources: config.Resources{ + Apps: map[string]*resources.App{ + "my_app_1": { + App: &apps.App{ + Name: "my_app_1", + }, + Config: map[string]any{ + "command": []string{"echo", "hello"}, + "env": []map[string]string{ + {"name": "JOB_ID", "value": "${resources.jobs.my_job.id}"}, + }, + }, + }, + "my_app_2": { + App: &apps.App{ + Name: "my_app_2", + }, + }, + }, + Jobs: map[string]*resources.Job{ + "my_job": { + ID: "123", + }, + }, + }, + }, + } + + diags := bundle.Apply(context.Background(), b, InterpolateVariables()) + require.Empty(t, diags) + require.Equal(t, []any([]any{map[string]any{"name": "JOB_ID", "value": "123"}}), b.Config.Resources.Apps["my_app_1"].Config["env"]) + require.Nil(t, b.Config.Resources.Apps["my_app_2"].Config) +} diff --git a/bundle/phases/deploy.go b/bundle/phases/deploy.go index 2dc9623bd0..17c09bcffe 100644 --- a/bundle/phases/deploy.go +++ b/bundle/phases/deploy.go @@ -5,6 +5,7 @@ import ( "fmt" "github.com/databricks/cli/bundle" + "github.com/databricks/cli/bundle/apps" "github.com/databricks/cli/bundle/artifacts" "github.com/databricks/cli/bundle/config" "github.com/databricks/cli/bundle/config/mutator" @@ -135,6 +136,8 @@ func Deploy(outputHandler sync.OutputHandler) bundle.Mutator { bundle.Seq( terraform.StatePush(), terraform.Load(), + apps.InterpolateVariables(), + apps.UploadConfig(), metadata.Compute(), metadata.Upload(), bundle.LogString("Deployment complete!"), diff --git a/integration/bundle/apps_test.go b/integration/bundle/apps_test.go index ab4d3cb79d..97061e178d 100644 --- a/integration/bundle/apps_test.go +++ b/integration/bundle/apps_test.go @@ -2,6 +2,7 @@ package bundle_test import ( "fmt" + "io" "testing" "github.com/databricks/cli/internal" @@ -48,6 +49,29 @@ func TestAccDeployBundleWithApp(t *testing.T) { require.NoError(t, err) require.NotNil(t, app) + // Check app config + currentUser, err := wt.W.CurrentUser.Me(ctx) + require.NoError(t, err) + + pathToAppYml := fmt.Sprintf("/Workspace/Users/%s/.bundle/%s/files/app/app.yml", currentUser.UserName, uniqueId) + reader, err := wt.W.Workspace.Download(ctx, pathToAppYml) + require.NoError(t, err) + + data, err := io.ReadAll(reader) + require.NoError(t, err) + + job, err := wt.W.Jobs.GetBySettingsName(ctx, fmt.Sprintf("test-job-with-cluster-%s", uniqueId)) + + content := string(data) + require.Contains(t, content, fmt.Sprintf(`command: + - flask + - --app + - app + - run +env: + - name: JOB_ID + value: "%d"`, job.JobId)) + // Try to run the app _, out, err := runResourceWithStderr(t, ctx, root, "test_app") require.NoError(t, err) diff --git a/integration/bundle/bundles/apps/template/databricks.yml.tmpl b/integration/bundle/bundles/apps/template/databricks.yml.tmpl index 9ab21bf6f3..4d862a06fe 100644 --- a/integration/bundle/bundles/apps/template/databricks.yml.tmpl +++ b/integration/bundle/bundles/apps/template/databricks.yml.tmpl @@ -18,7 +18,7 @@ resources: - run env: - name: JOB_ID - valueFrom: "app-job" + value: ${resources.jobs.foo.id} resources: - name: "app-job" From a71289a96938710c3c5d056559756aa9da531388 Mon Sep 17 00:00:00 2001 From: Andrew Nester Date: Fri, 13 Dec 2024 15:38:08 +0100 Subject: [PATCH 09/25] use TF provider 1.61.0 --- bundle/deploy/terraform/tfdyn/convert_app.go | 5 ----- bundle/deploy/terraform/tfdyn/convert_app_test.go | 2 +- 2 files changed, 1 insertion(+), 6 deletions(-) diff --git a/bundle/deploy/terraform/tfdyn/convert_app.go b/bundle/deploy/terraform/tfdyn/convert_app.go index f4849cab74..a07d81726b 100644 --- a/bundle/deploy/terraform/tfdyn/convert_app.go +++ b/bundle/deploy/terraform/tfdyn/convert_app.go @@ -29,11 +29,6 @@ func (appConverter) Convert(ctx context.Context, key string, vin dyn.Value, out return err } - // Modify top-level keys. - vout, err = renameKeys(vout, map[string]string{ - "resources": "resource", - }) - if err != nil { return err } diff --git a/bundle/deploy/terraform/tfdyn/convert_app_test.go b/bundle/deploy/terraform/tfdyn/convert_app_test.go index 5589610595..5f309d1c1e 100644 --- a/bundle/deploy/terraform/tfdyn/convert_app_test.go +++ b/bundle/deploy/terraform/tfdyn/convert_app_test.go @@ -63,7 +63,7 @@ func TestConvertApp(t *testing.T) { assert.Equal(t, map[string]any{ "description": "app description", "name": "app_id", - "resource": []any{ + "resources": []any{ map[string]any{ "name": "job1", "job": map[string]any{ From ddeeefef21862e15da6e3f105cc87cf7e79bd360 Mon Sep 17 00:00:00 2001 From: Andrew Nester Date: Mon, 16 Dec 2024 13:15:27 +0100 Subject: [PATCH 10/25] fixes after rebase --- bundle/config/mutator/process_target_mode_test.go | 1 + integration/bundle/apps_test.go | 4 ++-- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/bundle/config/mutator/process_target_mode_test.go b/bundle/config/mutator/process_target_mode_test.go index db55e8edc2..02da5f258a 100644 --- a/bundle/config/mutator/process_target_mode_test.go +++ b/bundle/config/mutator/process_target_mode_test.go @@ -426,6 +426,7 @@ func TestAllNonUcResourcesAreRenamed(t *testing.T) { for _, key := range field.MapKeys() { resource := field.MapIndex(key) nameField := resource.Elem().FieldByName("Name") + resourceType := resources.Type().Field(i).Name // Skip apps, as they are not renamed if resourceType == "Apps" { diff --git a/integration/bundle/apps_test.go b/integration/bundle/apps_test.go index 97061e178d..92e4d521fd 100644 --- a/integration/bundle/apps_test.go +++ b/integration/bundle/apps_test.go @@ -5,8 +5,8 @@ import ( "io" "testing" - "github.com/databricks/cli/internal" "github.com/databricks/cli/internal/acc" + "github.com/databricks/cli/internal/testutil" "github.com/databricks/cli/libs/env" "github.com/databricks/databricks-sdk-go/service/apps" "github.com/google/uuid" @@ -17,7 +17,7 @@ func TestAccDeployBundleWithApp(t *testing.T) { ctx, wt := acc.WorkspaceTest(t) uniqueId := uuid.New().String() appId := fmt.Sprintf("app-%s", uuid.New().String()[0:8]) - nodeTypeId := internal.GetNodeTypeId(env.Get(ctx, "CLOUD_ENV")) + nodeTypeId := testutil.GetCloud(t).NodeTypeID() instancePoolId := env.Get(ctx, "TEST_INSTANCE_POOL_ID") root, err := initTestTemplate(t, ctx, "apps", map[string]any{ From 1ff7bd2b2e20e9f69923ccd8c5666144e0143095 Mon Sep 17 00:00:00 2001 From: Andrew Nester Date: Mon, 16 Dec 2024 13:36:44 +0100 Subject: [PATCH 11/25] fixes --- bundle/run/app_test.go | 2 +- integration/bundle/helpers_test.go | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/bundle/run/app_test.go b/bundle/run/app_test.go index ddbdc2a309..cb324382ca 100644 --- a/bundle/run/app_test.go +++ b/bundle/run/app_test.go @@ -42,7 +42,7 @@ func (ta *testAppRunner) run(t *testing.T) { func setupBundle(t *testing.T) (context.Context, *bundle.Bundle, *mocks.MockWorkspaceClient) { root := t.TempDir() - err := os.MkdirAll(filepath.Join(root, "my_app"), 0700) + err := os.MkdirAll(filepath.Join(root, "my_app"), 0o700) require.NoError(t, err) b := &bundle.Bundle{ diff --git a/integration/bundle/helpers_test.go b/integration/bundle/helpers_test.go index 081e42ee6e..5f931b61b0 100644 --- a/integration/bundle/helpers_test.go +++ b/integration/bundle/helpers_test.go @@ -123,7 +123,7 @@ func runResourceWithStderr(t testutil.TestingT, ctx context.Context, path string ctx = env.Set(ctx, "BUNDLE_ROOT", path) ctx = cmdio.NewContext(ctx, cmdio.Default()) - c := internal.NewCobraTestRunnerWithContext(t, ctx, "bundle", "run", key) + c := testcli.NewRunnerWithContext(t, ctx, "bundle", "run", key) stdout, stderr, err := c.Run() return stdout.String(), stderr.String(), err } From 53393d57b1a3ae8c3ba19451261eea3751ade30f Mon Sep 17 00:00:00 2001 From: Andrew Nester Date: Mon, 16 Dec 2024 13:38:22 +0100 Subject: [PATCH 12/25] test fixes --- bundle/run/app_test.go | 2 +- integration/bundle/helpers_test.go | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/bundle/run/app_test.go b/bundle/run/app_test.go index cb324382ca..44ff698e59 100644 --- a/bundle/run/app_test.go +++ b/bundle/run/app_test.go @@ -76,7 +76,7 @@ func setupBundle(t *testing.T) (context.Context, *bundle.Bundle, *mocks.MockWork bundletest.SetLocation(b, "resources.apps.my_app", []dyn.Location{{File: "./databricks.yml"}}) ctx := context.Background() - ctx = cmdio.InContext(ctx, cmdio.NewIO(flags.OutputText, &bytes.Buffer{}, &bytes.Buffer{}, &bytes.Buffer{}, "", "...")) + ctx = cmdio.InContext(ctx, cmdio.NewIO(ctx, flags.OutputText, &bytes.Buffer{}, &bytes.Buffer{}, &bytes.Buffer{}, "", "...")) ctx = cmdio.NewContext(ctx, cmdio.NewLogger(flags.ModeAppend)) diags := bundle.Apply(ctx, b, bundle.Seq( diff --git a/integration/bundle/helpers_test.go b/integration/bundle/helpers_test.go index 5f931b61b0..eb5095e984 100644 --- a/integration/bundle/helpers_test.go +++ b/integration/bundle/helpers_test.go @@ -123,7 +123,7 @@ func runResourceWithStderr(t testutil.TestingT, ctx context.Context, path string ctx = env.Set(ctx, "BUNDLE_ROOT", path) ctx = cmdio.NewContext(ctx, cmdio.Default()) - c := testcli.NewRunnerWithContext(t, ctx, "bundle", "run", key) + c := testcli.NewRunner(t, ctx, "bundle", "run", key) stdout, stderr, err := c.Run() return stdout.String(), stderr.String(), err } From e7a377cd1c3f95b9487f94a6badcdb6667babcc6 Mon Sep 17 00:00:00 2001 From: Andrew Nester Date: Mon, 16 Dec 2024 13:42:12 +0100 Subject: [PATCH 13/25] fixed lint --- bundle/apps/upload_config_test.go | 2 +- bundle/apps/validate.go | 3 +-- bundle/deploy/terraform/tfdyn/convert_app.go | 4 ---- bundle/deploy/terraform/tfdyn/convert_app_test.go | 3 +-- bundle/run/app.go | 3 --- bundle/tests/apps_test.go | 1 - integration/bundle/helpers_test.go | 4 ++-- libs/dyn/merge/elements_by_key.go | 6 +++--- 8 files changed, 8 insertions(+), 18 deletions(-) diff --git a/bundle/apps/upload_config_test.go b/bundle/apps/upload_config_test.go index 56e3101e9b..2d9b1c70d1 100644 --- a/bundle/apps/upload_config_test.go +++ b/bundle/apps/upload_config_test.go @@ -20,7 +20,7 @@ import ( func TestAppUploadConfig(t *testing.T) { root := t.TempDir() - err := os.MkdirAll(filepath.Join(root, "my_app"), 0700) + err := os.MkdirAll(filepath.Join(root, "my_app"), 0o700) require.NoError(t, err) b := &bundle.Bundle{ diff --git a/bundle/apps/validate.go b/bundle/apps/validate.go index d12d013134..dfb3fffada 100644 --- a/bundle/apps/validate.go +++ b/bundle/apps/validate.go @@ -9,8 +9,7 @@ import ( "github.com/databricks/cli/libs/diag" ) -type validate struct { -} +type validate struct{} func (v *validate) Apply(ctx context.Context, b *bundle.Bundle) diag.Diagnostics { var diags diag.Diagnostics diff --git a/bundle/deploy/terraform/tfdyn/convert_app.go b/bundle/deploy/terraform/tfdyn/convert_app.go index a07d81726b..235b3a56fc 100644 --- a/bundle/deploy/terraform/tfdyn/convert_app.go +++ b/bundle/deploy/terraform/tfdyn/convert_app.go @@ -29,10 +29,6 @@ func (appConverter) Convert(ctx context.Context, key string, vin dyn.Value, out return err } - if err != nil { - return err - } - // Add the converted resource to the output. out.App[key] = vout.AsAny() diff --git a/bundle/deploy/terraform/tfdyn/convert_app_test.go b/bundle/deploy/terraform/tfdyn/convert_app_test.go index 5f309d1c1e..5dfebdc0ef 100644 --- a/bundle/deploy/terraform/tfdyn/convert_app_test.go +++ b/bundle/deploy/terraform/tfdyn/convert_app_test.go @@ -14,7 +14,7 @@ import ( ) func TestConvertApp(t *testing.T) { - var src = resources.App{ + src := resources.App{ SourceCodePath: "./app", Config: map[string]any{ "command": []string{"python", "app.py"}, @@ -95,5 +95,4 @@ func TestConvertApp(t *testing.T) { }, }, }, out.Permissions["app_my_app"]) - } diff --git a/bundle/run/app.go b/bundle/run/app.go index 2bbce10cc2..3bcdc22185 100644 --- a/bundle/run/app.go +++ b/bundle/run/app.go @@ -102,7 +102,6 @@ func (a *appRunner) start(ctx context.Context) error { } logProgress(ctx, "App is starting...") }).Get() - if err != nil { return err } @@ -149,7 +148,6 @@ func (a *appRunner) deploy(ctx context.Context) error { SourceCodePath: app.SourceCodePath, }, }) - // If deploy returns an error, then there's an active deployment in progress, wait for it to complete. if err != nil { return err @@ -161,7 +159,6 @@ func (a *appRunner) deploy(ctx context.Context) error { } logProgress(ctx, ad.Status.Message) }).Get() - if err != nil { return err } diff --git a/bundle/tests/apps_test.go b/bundle/tests/apps_test.go index 2b777a327c..7fee60d149 100644 --- a/bundle/tests/apps_test.go +++ b/bundle/tests/apps_test.go @@ -57,5 +57,4 @@ func TestAppsOverride(t *testing.T) { assert.Equal(t, "key", app.Resources[2].Secret.Key) assert.Equal(t, "scope", app.Resources[2].Secret.Scope) assert.Equal(t, "CAN_USE", string(app.Resources[2].Secret.Permission)) - } diff --git a/integration/bundle/helpers_test.go b/integration/bundle/helpers_test.go index eb5095e984..c62c79307a 100644 --- a/integration/bundle/helpers_test.go +++ b/integration/bundle/helpers_test.go @@ -119,7 +119,7 @@ func runResource(t testutil.TestingT, ctx context.Context, path, key string) (st return stdout.String(), err } -func runResourceWithStderr(t testutil.TestingT, ctx context.Context, path string, key string) (string, string, error) { +func runResourceWithStderr(t testutil.TestingT, ctx context.Context, path, key string) (string, string, error) { ctx = env.Set(ctx, "BUNDLE_ROOT", path) ctx = cmdio.NewContext(ctx, cmdio.Default()) @@ -128,7 +128,7 @@ func runResourceWithStderr(t testutil.TestingT, ctx context.Context, path string return stdout.String(), stderr.String(), err } -func runResourceWithParams(t testutil.TestingT, ctx context.Context, path string, key string, params ...string) (string, error) { +func runResourceWithParams(t testutil.TestingT, ctx context.Context, path, key string, params ...string) (string, error) { ctx = env.Set(ctx, "BUNDLE_ROOT", path) ctx = cmdio.NewContext(ctx, cmdio.Default()) diff --git a/libs/dyn/merge/elements_by_key.go b/libs/dyn/merge/elements_by_key.go index e94c5fdc74..df393003ac 100644 --- a/libs/dyn/merge/elements_by_key.go +++ b/libs/dyn/merge/elements_by_key.go @@ -7,7 +7,7 @@ type elementsByKey struct { keyFunc func(dyn.Value) string } -func (e elementsByKey) doMap(_ dyn.Path, v dyn.Value, mergeFunc func(a dyn.Value, b dyn.Value) (dyn.Value, error)) (dyn.Value, error) { +func (e elementsByKey) doMap(_ dyn.Path, v dyn.Value, mergeFunc func(a, b dyn.Value) (dyn.Value, error)) (dyn.Value, error) { // We know the type of this value is a sequence. // For additional defence, return self if it is not. elements, ok := v.AsSequence() @@ -60,7 +60,7 @@ func (e elementsByKey) Map(_ dyn.Path, v dyn.Value) (dyn.Value, error) { } func (e elementsByKey) MapWithOverride(p dyn.Path, v dyn.Value) (dyn.Value, error) { - return e.doMap(nil, v, func(a dyn.Value, b dyn.Value) (dyn.Value, error) { + return e.doMap(nil, v, func(a, b dyn.Value) (dyn.Value, error) { return Override(a, b, OverrideVisitor{ VisitInsert: func(_ dyn.Path, v dyn.Value) (dyn.Value, error) { return v, nil @@ -68,7 +68,7 @@ func (e elementsByKey) MapWithOverride(p dyn.Path, v dyn.Value) (dyn.Value, erro VisitDelete: func(valuePath dyn.Path, left dyn.Value) error { return nil }, - VisitUpdate: func(_ dyn.Path, a dyn.Value, b dyn.Value) (dyn.Value, error) { + VisitUpdate: func(_ dyn.Path, a, b dyn.Value) (dyn.Value, error) { return b, nil }, }) From 6655eb6a00f435b2e6c218530c569c37ac9559c8 Mon Sep 17 00:00:00 2001 From: Andrew Nester Date: Mon, 16 Dec 2024 15:29:04 +0100 Subject: [PATCH 14/25] fixed tests after rebase --- integration/bundle/apps_test.go | 11 ++++------- 1 file changed, 4 insertions(+), 7 deletions(-) diff --git a/integration/bundle/apps_test.go b/integration/bundle/apps_test.go index 92e4d521fd..aac9a9d055 100644 --- a/integration/bundle/apps_test.go +++ b/integration/bundle/apps_test.go @@ -20,19 +20,16 @@ func TestAccDeployBundleWithApp(t *testing.T) { nodeTypeId := testutil.GetCloud(t).NodeTypeID() instancePoolId := env.Get(ctx, "TEST_INSTANCE_POOL_ID") - root, err := initTestTemplate(t, ctx, "apps", map[string]any{ + root := initTestTemplate(t, ctx, "apps", map[string]any{ "unique_id": uniqueId, "app_id": appId, "node_type_id": nodeTypeId, "spark_version": defaultSparkVersion, "instance_pool_id": instancePoolId, }) - require.NoError(t, err) t.Cleanup(func() { - err = destroyBundle(t, ctx, root) - require.NoError(t, err) - + destroyBundle(t, ctx, root) app, err := wt.W.Apps.Get(ctx, apps.GetAppRequest{Name: "test-app"}) if err != nil { require.ErrorContains(t, err, "does not exist") @@ -41,8 +38,7 @@ func TestAccDeployBundleWithApp(t *testing.T) { } }) - err = deployBundle(t, ctx, root) - require.NoError(t, err) + deployBundle(t, ctx, root) // App should exists after bundle deployment app, err := wt.W.Apps.Get(ctx, apps.GetAppRequest{Name: appId}) @@ -61,6 +57,7 @@ func TestAccDeployBundleWithApp(t *testing.T) { require.NoError(t, err) job, err := wt.W.Jobs.GetBySettingsName(ctx, fmt.Sprintf("test-job-with-cluster-%s", uniqueId)) + require.NoError(t, err) content := string(data) require.Contains(t, content, fmt.Sprintf(`command: From 1ceec5ea3b68b2d1371d8a3e5edec23a4f9d2c9f Mon Sep 17 00:00:00 2001 From: Andrew Nester Date: Tue, 17 Dec 2024 12:00:00 +0100 Subject: [PATCH 15/25] fixed acc test --- integration/bundle/apps_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/integration/bundle/apps_test.go b/integration/bundle/apps_test.go index aac9a9d055..e45f74f34e 100644 --- a/integration/bundle/apps_test.go +++ b/integration/bundle/apps_test.go @@ -5,7 +5,7 @@ import ( "io" "testing" - "github.com/databricks/cli/internal/acc" + "github.com/databricks/cli/integration/internal/acc" "github.com/databricks/cli/internal/testutil" "github.com/databricks/cli/libs/env" "github.com/databricks/databricks-sdk-go/service/apps" From e8cae54be989437dd75c86cc7f9d6822fc59023e Mon Sep 17 00:00:00 2001 From: Andrew Nester Date: Tue, 17 Dec 2024 14:44:14 +0100 Subject: [PATCH 16/25] removed test prefix --- integration/bundle/apps_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/integration/bundle/apps_test.go b/integration/bundle/apps_test.go index e45f74f34e..0707403dc9 100644 --- a/integration/bundle/apps_test.go +++ b/integration/bundle/apps_test.go @@ -13,7 +13,7 @@ import ( "github.com/stretchr/testify/require" ) -func TestAccDeployBundleWithApp(t *testing.T) { +func TestDeployBundleWithApp(t *testing.T) { ctx, wt := acc.WorkspaceTest(t) uniqueId := uuid.New().String() appId := fmt.Sprintf("app-%s", uuid.New().String()[0:8]) From ab85a7f992903066d908e284e3adb27ac47f4769 Mon Sep 17 00:00:00 2001 From: Andrew Nester Date: Tue, 17 Dec 2024 19:56:06 +0100 Subject: [PATCH 17/25] removed app id --- bundle/config/resources/apps.go | 9 ++------- bundle/deploy/terraform/convert.go | 7 ++++--- bundle/deploy/terraform/convert_test.go | 22 +++++++++++----------- bundle/deploy/terraform/util.go | 1 + bundle/deploy/terraform/util_test.go | 2 +- 5 files changed, 19 insertions(+), 22 deletions(-) diff --git a/bundle/config/resources/apps.go b/bundle/config/resources/apps.go index 00b7de4dda..ee3c40ad0e 100644 --- a/bundle/config/resources/apps.go +++ b/bundle/config/resources/apps.go @@ -12,11 +12,6 @@ import ( ) type App struct { - // This represents the id which is the name of the app that can be used - // as a reference in other resources. This value is returned by terraform. - // 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 to Databricks app source code // on local disk and to the corresponding workspace path during app deployment. SourceCodePath string `json:"source_code_path"` @@ -56,10 +51,10 @@ func (a *App) TerraformResourceName() string { } func (a *App) InitializeURL(baseURL url.URL) { - if a.ID == "" { + if a.Name == "" { return } - baseURL.Path = fmt.Sprintf("apps/%s", a.ID) + baseURL.Path = fmt.Sprintf("apps/%s", a.Name) a.URL = baseURL.String() } diff --git a/bundle/deploy/terraform/convert.go b/bundle/deploy/terraform/convert.go index b47642697f..cdae42b3ec 100644 --- a/bundle/deploy/terraform/convert.go +++ b/bundle/deploy/terraform/convert.go @@ -9,6 +9,7 @@ import ( "github.com/databricks/cli/bundle/deploy/terraform/tfdyn" "github.com/databricks/cli/bundle/internal/tf/schema" "github.com/databricks/cli/libs/dyn" + "github.com/databricks/databricks-sdk-go/service/apps" tfjson "github.com/hashicorp/terraform-json" ) @@ -202,9 +203,9 @@ func TerraformToBundle(state *resourcesState, config *config.Root) error { } cur := config.Resources.Apps[resource.Name] if cur == nil { - cur = &resources.App{ModifiedStatus: resources.ModifiedStatusDeleted} + cur = &resources.App{ModifiedStatus: resources.ModifiedStatusDeleted, App: &apps.App{}} } - cur.ID = instance.Attributes.ID + cur.Name = instance.Attributes.Name config.Resources.Apps[resource.Name] = cur case "databricks_permissions": case "databricks_grants": @@ -271,7 +272,7 @@ func TerraformToBundle(state *resourcesState, config *config.Root) error { } } for _, src := range config.Resources.Apps { - if src.ModifiedStatus == "" && src.ID == "" { + if src.ModifiedStatus == "" && src.Name == "" { src.ModifiedStatus = resources.ModifiedStatusCreated } } diff --git a/bundle/deploy/terraform/convert_test.go b/bundle/deploy/terraform/convert_test.go index 440eea233e..3da364f472 100644 --- a/bundle/deploy/terraform/convert_test.go +++ b/bundle/deploy/terraform/convert_test.go @@ -700,7 +700,7 @@ func TestTerraformToBundleEmptyLocalResources(t *testing.T) { Mode: "managed", Name: "test_app", Instances: []stateResourceInstance{ - {Attributes: stateInstanceAttributes{ID: "1"}}, + {Attributes: stateInstanceAttributes{Name: "app1"}}, }, }, }, @@ -741,7 +741,7 @@ func TestTerraformToBundleEmptyLocalResources(t *testing.T) { assert.Equal(t, "1", config.Resources.Dashboards["test_dashboard"].ID) assert.Equal(t, resources.ModifiedStatusDeleted, config.Resources.Dashboards["test_dashboard"].ModifiedStatus) - assert.Equal(t, "1", config.Resources.Apps["test_app"].ID) + assert.Equal(t, "app1", config.Resources.Apps["test_app"].Name) assert.Equal(t, resources.ModifiedStatusDeleted, config.Resources.Apps["test_app"].ModifiedStatus) AssertFullResourceCoverage(t, &config) @@ -830,7 +830,7 @@ func TestTerraformToBundleEmptyRemoteResources(t *testing.T) { Apps: map[string]*resources.App{ "test_app": { App: &apps.App{ - Name: "test_app", + Description: "test_app", }, }, }, @@ -875,7 +875,7 @@ func TestTerraformToBundleEmptyRemoteResources(t *testing.T) { assert.Equal(t, "", config.Resources.Dashboards["test_dashboard"].ID) assert.Equal(t, resources.ModifiedStatusCreated, config.Resources.Dashboards["test_dashboard"].ModifiedStatus) - assert.Equal(t, "", config.Resources.Apps["test_app"].ID) + assert.Equal(t, "", config.Resources.Apps["test_app"].Name) assert.Equal(t, resources.ModifiedStatusCreated, config.Resources.Apps["test_app"].ModifiedStatus) AssertFullResourceCoverage(t, &config) @@ -1019,12 +1019,12 @@ func TestTerraformToBundleModifiedResources(t *testing.T) { Apps: map[string]*resources.App{ "test_app": { App: &apps.App{ - Name: "test_app", + Description: "test_app", }, }, "test_app_new": { App: &apps.App{ - Name: "test_app_new", + Description: "test_app_new", }, }, }, @@ -1213,7 +1213,7 @@ func TestTerraformToBundleModifiedResources(t *testing.T) { Mode: "managed", Name: "test_app", Instances: []stateResourceInstance{ - {Attributes: stateInstanceAttributes{ID: "1"}}, + {Attributes: stateInstanceAttributes{Name: "app1"}}, }, }, { @@ -1221,7 +1221,7 @@ func TestTerraformToBundleModifiedResources(t *testing.T) { Mode: "managed", Name: "test_app_old", Instances: []stateResourceInstance{ - {Attributes: stateInstanceAttributes{ID: "2"}}, + {Attributes: stateInstanceAttributes{Name: "app2"}}, }, }, }, @@ -1306,11 +1306,11 @@ func TestTerraformToBundleModifiedResources(t *testing.T) { assert.Equal(t, "", config.Resources.Dashboards["test_dashboard_new"].ID) assert.Equal(t, resources.ModifiedStatusCreated, config.Resources.Dashboards["test_dashboard_new"].ModifiedStatus) - assert.Equal(t, "1", config.Resources.Apps["test_app"].ID) + assert.Equal(t, "app1", config.Resources.Apps["test_app"].Name) assert.Equal(t, "", config.Resources.Apps["test_app"].ModifiedStatus) - assert.Equal(t, "2", config.Resources.Apps["test_app_old"].ID) + assert.Equal(t, "app2", config.Resources.Apps["test_app_old"].Name) assert.Equal(t, resources.ModifiedStatusDeleted, config.Resources.Apps["test_app_old"].ModifiedStatus) - assert.Equal(t, "", config.Resources.Apps["test_app_new"].ID) + assert.Equal(t, "", config.Resources.Apps["test_app_new"].Name) assert.Equal(t, resources.ModifiedStatusCreated, config.Resources.Apps["test_app_new"].ModifiedStatus) AssertFullResourceCoverage(t, &config) diff --git a/bundle/deploy/terraform/util.go b/bundle/deploy/terraform/util.go index 4da015c23f..e2564ac22e 100644 --- a/bundle/deploy/terraform/util.go +++ b/bundle/deploy/terraform/util.go @@ -34,6 +34,7 @@ type stateResourceInstance struct { type stateInstanceAttributes struct { ID string `json:"id"` + Name string `json:"name,omitempty"` ETag string `json:"etag,omitempty"` } diff --git a/bundle/deploy/terraform/util_test.go b/bundle/deploy/terraform/util_test.go index 74b3292594..5d13103923 100644 --- a/bundle/deploy/terraform/util_test.go +++ b/bundle/deploy/terraform/util_test.go @@ -97,7 +97,7 @@ func TestParseResourcesStateWithExistingStateFile(t *testing.T) { Type: "databricks_pipeline", Name: "test_pipeline", Instances: []stateResourceInstance{ - {Attributes: stateInstanceAttributes{ID: "123"}}, + {Attributes: stateInstanceAttributes{ID: "123", Name: "test_pipeline"}}, }, }, }, From f6de4c1e9e862ba3e394279e16f3469b16d0fe78 Mon Sep 17 00:00:00 2001 From: Andrew Nester Date: Wed, 18 Dec 2024 11:49:22 +0100 Subject: [PATCH 18/25] feedback --- bundle/apps/interpolate_variables.go | 46 +++++++++++----------------- bundle/apps/upload_config.go | 12 ++++++-- integration/bundle/apps_test.go | 6 ++-- integration/bundle/helpers_test.go | 6 ++-- 4 files changed, 33 insertions(+), 37 deletions(-) diff --git a/bundle/apps/interpolate_variables.go b/bundle/apps/interpolate_variables.go index 206609a12f..f4860127b0 100644 --- a/bundle/apps/interpolate_variables.go +++ b/bundle/apps/interpolate_variables.go @@ -19,37 +19,27 @@ func (i *interpolateVariables) Apply(ctx context.Context, b *bundle.Bundle) diag dyn.Key("config"), ) + tfToConfigMap := map[string]string{ + "databricks_pipeline": "pipelines", + "databricks_job": "jobs", + "databricks_mlflow_model": "models", + "databricks_mlflow_experiment": "experiments", + "databricks_model_serving": "model_serving_endpoints", + "databricks_registered_model": "registered_models", + "databricks_quality_monitor": "quality_monitors", + "databricks_schema": "schemas", + "databricks_volume": "volumes", + "databricks_cluster": "clusters", + "databricks_dashboard": "dashboards", + "databricks_app": "apps", + } + err := b.Config.Mutate(func(root dyn.Value) (dyn.Value, error) { return dyn.MapByPattern(root, pattern, func(p dyn.Path, v dyn.Value) (dyn.Value, error) { return dynvar.Resolve(v, func(path dyn.Path) (dyn.Value, error) { - switch path[0] { - case dyn.Key("databricks_pipeline"): - path = dyn.NewPath(dyn.Key("resources"), dyn.Key("pipelines")).Append(path[1:]...) - case dyn.Key("databricks_job"): - path = dyn.NewPath(dyn.Key("resources"), dyn.Key("jobs")).Append(path[1:]...) - case dyn.Key("databricks_mlflow_model"): - path = dyn.NewPath(dyn.Key("resources"), dyn.Key("models")).Append(path[1:]...) - case dyn.Key("databricks_mlflow_experiment"): - path = dyn.NewPath(dyn.Key("resources"), dyn.Key("experiments")).Append(path[1:]...) - case dyn.Key("databricks_model_serving"): - path = dyn.NewPath(dyn.Key("resources"), dyn.Key("model_serving_endpoints")).Append(path[1:]...) - case dyn.Key("databricks_registered_model"): - path = dyn.NewPath(dyn.Key("resources"), dyn.Key("registered_models")).Append(path[1:]...) - case dyn.Key("databricks_quality_monitor"): - path = dyn.NewPath(dyn.Key("resources"), dyn.Key("quality_monitors")).Append(path[1:]...) - case dyn.Key("databricks_schema"): - path = dyn.NewPath(dyn.Key("resources"), dyn.Key("schemas")).Append(path[1:]...) - case dyn.Key("databricks_volume"): - path = dyn.NewPath(dyn.Key("resources"), dyn.Key("volumes")).Append(path[1:]...) - case dyn.Key("databricks_cluster"): - path = dyn.NewPath(dyn.Key("resources"), dyn.Key("clusters")).Append(path[1:]...) - case dyn.Key("databricks_dashboard"): - path = dyn.NewPath(dyn.Key("resources"), dyn.Key("dashboards")).Append(path[1:]...) - case dyn.Key("databricks_app"): - path = dyn.NewPath(dyn.Key("resources"), dyn.Key("apps")).Append(path[1:]...) - default: - // Trigger "key not found" for unknown resource types. - return dyn.GetByPath(root, path) + key, ok := tfToConfigMap[path[0].Key()] + if ok { + path = dyn.NewPath(dyn.Key("resources"), dyn.Key(key)).Append(path[1:]...) } return dyn.GetByPath(root, path) diff --git a/bundle/apps/upload_config.go b/bundle/apps/upload_config.go index 4f70976a3e..4eb90f905f 100644 --- a/bundle/apps/upload_config.go +++ b/bundle/apps/upload_config.go @@ -25,6 +25,8 @@ func (u *uploadConfig) Apply(ctx context.Context, b *bundle.Bundle) diag.Diagnos var diags diag.Diagnostics errGroup, ctx := errgroup.WithContext(ctx) + diagsPerApp := make(map[string]diag.Diagnostic) + 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 @@ -57,12 +59,12 @@ func (u *uploadConfig) Apply(ctx context.Context, b *bundle.Bundle) diag.Diagnos errGroup.Go(func() error { err = f.Write(ctx, path.Join(appPath, "app.yml"), buf, filer.OverwriteIfExists) if err != nil { - diags = append(diags, diag.Diagnostic{ + diagsPerApp[key] = 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 }) @@ -70,7 +72,11 @@ func (u *uploadConfig) Apply(ctx context.Context, b *bundle.Bundle) diag.Diagnos } if err := errGroup.Wait(); err != nil { - return diag.FromErr(err) + return diags.Extend(diag.FromErr(err)) + } + + for _, diag := range diagsPerApp { + diags = append(diags, diag) } return diags diff --git a/integration/bundle/apps_test.go b/integration/bundle/apps_test.go index 0707403dc9..7fc565e913 100644 --- a/integration/bundle/apps_test.go +++ b/integration/bundle/apps_test.go @@ -70,8 +70,7 @@ env: value: "%d"`, job.JobId)) // Try to run the app - _, out, err := runResourceWithStderr(t, ctx, root, "test_app") - require.NoError(t, err) + _, out := runResourceWithStderr(t, ctx, root, "test_app") require.Contains(t, out, app.Url) // App should be in the running state @@ -89,8 +88,7 @@ env: require.Equal(t, apps.ApplicationStateUnavailable, app.AppStatus.State) // Try to run the app again - _, out, err = runResourceWithStderr(t, ctx, root, "test_app") - require.NoError(t, err) + _, out = runResourceWithStderr(t, ctx, root, "test_app") require.Contains(t, out, app.Url) // App should be in the running state diff --git a/integration/bundle/helpers_test.go b/integration/bundle/helpers_test.go index c62c79307a..a537ca3517 100644 --- a/integration/bundle/helpers_test.go +++ b/integration/bundle/helpers_test.go @@ -119,13 +119,15 @@ func runResource(t testutil.TestingT, ctx context.Context, path, key string) (st return stdout.String(), err } -func runResourceWithStderr(t testutil.TestingT, ctx context.Context, path, key string) (string, string, error) { +func runResourceWithStderr(t testutil.TestingT, ctx context.Context, path, key string) (string, string) { ctx = env.Set(ctx, "BUNDLE_ROOT", path) ctx = cmdio.NewContext(ctx, cmdio.Default()) c := testcli.NewRunner(t, ctx, "bundle", "run", key) stdout, stderr, err := c.Run() - return stdout.String(), stderr.String(), err + require.NoError(t, err) + + return stdout.String(), stderr.String() } func runResourceWithParams(t testutil.TestingT, ctx context.Context, path, key string, params ...string) (string, error) { From e9e0566adae1aa5240745d5cbbe8511429b93e3f Mon Sep 17 00:00:00 2001 From: Andrew Nester Date: Wed, 18 Dec 2024 12:34:12 +0100 Subject: [PATCH 19/25] added annotations --- bundle/internal/schema/annotations.yml | 161 +++++++++++++++++++++++++ 1 file changed, 161 insertions(+) diff --git a/bundle/internal/schema/annotations.yml b/bundle/internal/schema/annotations.yml index e52189daa1..c3503ad737 100644 --- a/bundle/internal/schema/annotations.yml +++ b/bundle/internal/schema/annotations.yml @@ -1,3 +1,161 @@ +github.com/databricks/databricks-sdk-go/service/apps.AppResourceJob: + "id": + "description": |- + PLACEHOLDER + "permission": + "description": |- + PLACEHOLDER +github.com/databricks/databricks-sdk-go/service/apps.ComputeStatus: + "message": + "description": |- + PLACEHOLDER + "state": + "description": |- + PLACEHOLDER +github.com/databricks/databricks-sdk-go/service/apps.AppDeploymentArtifacts: + "source_code_path": + "description": |- + PLACEHOLDER +github.com/databricks/databricks-sdk-go/service/apps.AppResourceSqlWarehouse: + "id": + "description": |- + PLACEHOLDER + "permission": + "description": |- + PLACEHOLDER +github.com/databricks/cli/bundle/config/resources.App: + "create_time": + "description": |- + PLACEHOLDER + "permissions": + "description": |- + PLACEHOLDER + "resources": + "description": |- + PLACEHOLDER + "url": + "description": |- + PLACEHOLDER + "active_deployment": + "description": |- + PLACEHOLDER + "description": + "description": |- + PLACEHOLDER + "default_source_code_path": + "description": |- + PLACEHOLDER + "service_principal_client_id": + "description": |- + PLACEHOLDER + "service_principal_name": + "description": |- + PLACEHOLDER + "config": + "description": |- + PLACEHOLDER + "source_code_path": + "description": |- + PLACEHOLDER + "service_principal_id": + "description": |- + PLACEHOLDER + "name": + "description": |- + PLACEHOLDER + "compute_status": + "description": |- + PLACEHOLDER + "creator": + "description": |- + PLACEHOLDER + "app_status": + "description": |- + PLACEHOLDER + "pending_deployment": + "description": |- + PLACEHOLDER + "update_time": + "description": |- + PLACEHOLDER + "updater": + "description": |- + PLACEHOLDER +github.com/databricks/databricks-sdk-go/service/apps.AppResource: + "name": + "description": |- + PLACEHOLDER + "secret": + "description": |- + PLACEHOLDER + "serving_endpoint": + "description": |- + PLACEHOLDER + "sql_warehouse": + "description": |- + PLACEHOLDER + "description": + "description": |- + PLACEHOLDER + "job": + "description": |- + PLACEHOLDER +github.com/databricks/databricks-sdk-go/service/apps.AppDeployment: + "create_time": + "description": |- + PLACEHOLDER + "creator": + "description": |- + PLACEHOLDER + "deployment_artifacts": + "description": |- + PLACEHOLDER + "deployment_id": + "description": |- + PLACEHOLDER + "mode": + "description": |- + PLACEHOLDER + "source_code_path": + "description": |- + PLACEHOLDER + "status": + "description": |- + PLACEHOLDER + "update_time": + "description": |- + PLACEHOLDER +github.com/databricks/databricks-sdk-go/service/apps.ApplicationStatus: + "state": + "description": |- + PLACEHOLDER + "message": + "description": |- + PLACEHOLDER +github.com/databricks/databricks-sdk-go/service/apps.AppDeploymentStatus: + "message": + "description": |- + PLACEHOLDER + "state": + "description": |- + PLACEHOLDER +github.com/databricks/databricks-sdk-go/service/apps.AppResourceSecret: + "key": + "description": |- + PLACEHOLDER + "permission": + "description": |- + PLACEHOLDER + "scope": + "description": |- + PLACEHOLDER +github.com/databricks/databricks-sdk-go/service/apps.AppResourceServingEndpoint: + "permission": + "description": |- + PLACEHOLDER + "name": + "description": |- + PLACEHOLDER github.com/databricks/cli/bundle/config.Artifact: "build": "description": |- @@ -126,6 +284,9 @@ github.com/databricks/cli/bundle/config.PyDABs: "description": |- The Python virtual environment path github.com/databricks/cli/bundle/config.Resources: + "apps": + "description": |- + PLACEHOLDER "clusters": "description": |- The cluster definitions for the bundle. From e622ab51f0f0069324f62db79e77d0b3c95c5737 Mon Sep 17 00:00:00 2001 From: Andrew Nester Date: Wed, 18 Dec 2024 13:16:23 +0100 Subject: [PATCH 20/25] Added support for bundle generate and bind for Apps (#1946) ## Changes Added support for bundle generate and bind for Apps ## Tests - [ ] Add E2E test --- bundle/config/generate/app.go | 37 ++++++++ bundle/config/resources.go | 7 ++ cmd/bundle/generate.go | 1 + cmd/bundle/generate/app.go | 166 ++++++++++++++++++++++++++++++++++ cmd/bundle/generate/utils.go | 32 +++++++ 5 files changed, 243 insertions(+) create mode 100644 bundle/config/generate/app.go create mode 100644 cmd/bundle/generate/app.go diff --git a/bundle/config/generate/app.go b/bundle/config/generate/app.go new file mode 100644 index 0000000000..1255d63f89 --- /dev/null +++ b/bundle/config/generate/app.go @@ -0,0 +1,37 @@ +package generate + +import ( + "github.com/databricks/cli/libs/dyn" + "github.com/databricks/cli/libs/dyn/convert" + "github.com/databricks/databricks-sdk-go/service/apps" +) + +func ConvertAppToValue(app *apps.App, sourceCodePath string, appConfig map[string]any) (dyn.Value, error) { + ac, err := convert.FromTyped(appConfig, dyn.NilValue) + if err != nil { + return dyn.NilValue, err + } + + ar, err := convert.FromTyped(app.Resources, dyn.NilValue) + if err != nil { + return dyn.NilValue, err + } + + // The majority of fields of the app struct are read-only. + // We copy the relevant fields manually. + dv := map[string]dyn.Value{ + "name": dyn.NewValue(app.Name, []dyn.Location{{Line: 1}}), + "description": dyn.NewValue(app.Description, []dyn.Location{{Line: 2}}), + "source_code_path": dyn.NewValue(sourceCodePath, []dyn.Location{{Line: 3}}), + } + + if ac.Kind() != dyn.KindNil { + dv["config"] = ac.WithLocations([]dyn.Location{{Line: 4}}) + } + + if ar.Kind() != dyn.KindNil { + dv["resources"] = ar.WithLocations([]dyn.Location{{Line: 5}}) + } + + return dyn.V(dv), nil +} diff --git a/bundle/config/resources.go b/bundle/config/resources.go index bf81b92958..b06a44b4dd 100644 --- a/bundle/config/resources.go +++ b/bundle/config/resources.go @@ -99,12 +99,19 @@ func (r *Resources) FindResourceByConfigKey(key string) (ConfigResource, error) found = append(found, r.Jobs[k]) } } + for k := range r.Pipelines { if k == key { found = append(found, r.Pipelines[k]) } } + for k := range r.Apps { + if k == key { + found = append(found, r.Apps[k]) + } + } + if len(found) == 0 { return nil, fmt.Errorf("no such resource: %s", key) } diff --git a/cmd/bundle/generate.go b/cmd/bundle/generate.go index 7dea19ff9d..d09c6feb43 100644 --- a/cmd/bundle/generate.go +++ b/cmd/bundle/generate.go @@ -17,6 +17,7 @@ func newGenerateCommand() *cobra.Command { cmd.AddCommand(generate.NewGenerateJobCommand()) cmd.AddCommand(generate.NewGeneratePipelineCommand()) cmd.AddCommand(generate.NewGenerateDashboardCommand()) + cmd.AddCommand(generate.NewGenerateAppCommand()) cmd.PersistentFlags().StringVar(&key, "key", "", `resource key to use for the generated configuration`) return cmd } diff --git a/cmd/bundle/generate/app.go b/cmd/bundle/generate/app.go new file mode 100644 index 0000000000..c948cf074c --- /dev/null +++ b/cmd/bundle/generate/app.go @@ -0,0 +1,166 @@ +package generate + +import ( + "context" + "errors" + "fmt" + "io" + "io/fs" + "path/filepath" + + "github.com/databricks/cli/bundle/config/generate" + "github.com/databricks/cli/cmd/root" + "github.com/databricks/cli/libs/cmdio" + "github.com/databricks/cli/libs/dyn" + "github.com/databricks/cli/libs/dyn/yamlsaver" + "github.com/databricks/cli/libs/filer" + "github.com/databricks/cli/libs/textutil" + "github.com/databricks/databricks-sdk-go" + "github.com/databricks/databricks-sdk-go/service/apps" + "github.com/spf13/cobra" + + "gopkg.in/yaml.v3" +) + +func NewGenerateAppCommand() *cobra.Command { + var configDir string + var sourceDir string + var appName string + var force bool + + cmd := &cobra.Command{ + Use: "app", + Short: "Generate bundle configuration for a Databricks app", + } + + cmd.Flags().StringVar(&appName, "existing-app-name", "", `App name to generate config for`) + cmd.MarkFlagRequired("existing-app-name") + + cmd.Flags().StringVarP(&configDir, "config-dir", "d", filepath.Join("resources"), `Directory path where the output bundle config will be stored`) + cmd.Flags().StringVarP(&sourceDir, "source-dir", "s", filepath.Join("src", "app"), `Directory path where the app files will be stored`) + cmd.Flags().BoolVarP(&force, "force", "f", false, `Force overwrite existing files in the output directory`) + + cmd.RunE = func(cmd *cobra.Command, args []string) error { + ctx := cmd.Context() + b, diags := root.MustConfigureBundle(cmd) + if err := diags.Error(); err != nil { + return diags.Error() + } + + w := b.WorkspaceClient() + cmdio.LogString(ctx, fmt.Sprintf("Loading app '%s' configuration", appName)) + app, err := w.Apps.Get(ctx, apps.GetAppRequest{Name: appName}) + if err != nil { + return err + } + + // Making sure the config directory and source directory are absolute paths. + if !filepath.IsAbs(configDir) { + configDir = filepath.Join(b.BundleRootPath, configDir) + } + + if !filepath.IsAbs(sourceDir) { + sourceDir = filepath.Join(b.BundleRootPath, sourceDir) + } + + downloader := newDownloader(w, sourceDir, configDir) + + sourceCodePath := app.DefaultSourceCodePath + err = downloader.markDirectoryForDownload(ctx, &sourceCodePath) + if err != nil { + return err + } + + appConfig, err := getAppConfig(ctx, app, w) + if err != nil { + return fmt.Errorf("failed to get app config: %w", err) + } + + // Making sure the source code path is relative to the config directory. + rel, err := filepath.Rel(configDir, sourceDir) + if err != nil { + return err + } + + v, err := generate.ConvertAppToValue(app, filepath.ToSlash(rel), appConfig) + if err != nil { + return err + } + + appKey := cmd.Flag("key").Value.String() + if appKey == "" { + appKey = textutil.NormalizeString(app.Name) + } + + result := map[string]dyn.Value{ + "resources": dyn.V(map[string]dyn.Value{ + "apps": dyn.V(map[string]dyn.Value{ + appKey: v, + }), + }), + } + + // If there are app.yaml or app.yml files in the source code path, they will be downloaded but we don't want to include them in the bundle. + // We include this configuration inline, so we need to remove these files. + for _, configFile := range []string{"app.yml", "app.yaml"} { + delete(downloader.files, filepath.Join(sourceDir, configFile)) + } + + err = downloader.FlushToDisk(ctx, force) + if err != nil { + return err + } + + filename := filepath.Join(configDir, fmt.Sprintf("%s.app.yml", appKey)) + + saver := yamlsaver.NewSaver() + err = saver.SaveAsYAML(result, filename, force) + if err != nil { + return err + } + + cmdio.LogString(ctx, fmt.Sprintf("App configuration successfully saved to %s", filename)) + return nil + } + + return cmd +} + +func getAppConfig(ctx context.Context, app *apps.App, w *databricks.WorkspaceClient) (map[string]any, error) { + sourceCodePath := app.DefaultSourceCodePath + + f, err := filer.NewWorkspaceFilesClient(w, sourceCodePath) + if err != nil { + return nil, err + } + + // The app config is stored in app.yml or app.yaml file in the source code path. + configFileNames := []string{"app.yml", "app.yaml"} + for _, configFile := range configFileNames { + r, err := f.Read(ctx, configFile) + if err != nil { + if errors.Is(err, fs.ErrNotExist) { + continue + } + return nil, err + } + defer r.Close() + + cmdio.LogString(ctx, fmt.Sprintf("Reading app configuration from %s", configFile)) + content, err := io.ReadAll(r) + if err != nil { + return nil, err + } + + var appConfig map[string]any + err = yaml.Unmarshal(content, &appConfig) + if err != nil { + cmdio.LogString(ctx, fmt.Sprintf("Failed to parse app configuration:\n%s\nerr: %v", string(content), err)) + return nil, nil + } + + return appConfig, nil + } + + return nil, nil +} diff --git a/cmd/bundle/generate/utils.go b/cmd/bundle/generate/utils.go index 8e3764e352..65bf8ad2c7 100644 --- a/cmd/bundle/generate/utils.go +++ b/cmd/bundle/generate/utils.go @@ -13,6 +13,7 @@ import ( "github.com/databricks/databricks-sdk-go" "github.com/databricks/databricks-sdk-go/service/jobs" "github.com/databricks/databricks-sdk-go/service/pipelines" + "github.com/databricks/databricks-sdk-go/service/workspace" "golang.org/x/sync/errgroup" ) @@ -63,6 +64,37 @@ func (n *downloader) markFileForDownload(ctx context.Context, filePath *string) return nil } +func (n *downloader) markDirectoryForDownload(ctx context.Context, dirPath *string) error { + _, err := n.w.Workspace.GetStatusByPath(ctx, *dirPath) + if err != nil { + return err + } + + objects, err := n.w.Workspace.RecursiveList(ctx, *dirPath) + if err != nil { + return err + } + + for _, obj := range objects { + if obj.ObjectType == workspace.ObjectTypeDirectory { + continue + } + + err := n.markFileForDownload(ctx, &obj.Path) + if err != nil { + return err + } + } + + rel, err := filepath.Rel(n.configDir, n.sourceDir) + if err != nil { + return err + } + + *dirPath = rel + return nil +} + func (n *downloader) markNotebookForDownload(ctx context.Context, notebookPath *string) error { info, err := n.w.Workspace.GetStatusByPath(ctx, *notebookPath) if err != nil { From db516b6e7262412d535addd7b28c377958ae9889 Mon Sep 17 00:00:00 2001 From: Andrew Nester Date: Wed, 18 Dec 2024 14:34:01 +0100 Subject: [PATCH 21/25] fixed test for gcp --- integration/bundle/apps_test.go | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/integration/bundle/apps_test.go b/integration/bundle/apps_test.go index 7fc565e913..aa0d1a4678 100644 --- a/integration/bundle/apps_test.go +++ b/integration/bundle/apps_test.go @@ -15,6 +15,11 @@ import ( func TestDeployBundleWithApp(t *testing.T) { ctx, wt := acc.WorkspaceTest(t) + + if testutil.GetCloud(t) == testutil.GCP { + t.Skip("Skipping test for GCP cloud because /api/2.0/apps is temporarily unavailable there.") + } + uniqueId := uuid.New().String() appId := fmt.Sprintf("app-%s", uuid.New().String()[0:8]) nodeTypeId := testutil.GetCloud(t).NodeTypeID() From 2f8df81159e2e20de06ae49d5aa290324100fd2c Mon Sep 17 00:00:00 2001 From: Andrew Nester Date: Thu, 19 Dec 2024 14:00:06 +0100 Subject: [PATCH 22/25] Added same source validation and made thread safe --- bundle/apps/upload_config.go | 14 ++++++-------- bundle/apps/validate.go | 13 ++++++++++++- bundle/apps/validate_test.go | 36 ++++++++++++++++++++++++++++++++++++ 3 files changed, 54 insertions(+), 9 deletions(-) diff --git a/bundle/apps/upload_config.go b/bundle/apps/upload_config.go index 4eb90f905f..b37f06351a 100644 --- a/bundle/apps/upload_config.go +++ b/bundle/apps/upload_config.go @@ -6,6 +6,7 @@ import ( "fmt" "path" "strings" + "sync" "github.com/databricks/cli/bundle" "github.com/databricks/cli/bundle/config/resources" @@ -25,8 +26,7 @@ func (u *uploadConfig) Apply(ctx context.Context, b *bundle.Bundle) diag.Diagnos var diags diag.Diagnostics errGroup, ctx := errgroup.WithContext(ctx) - diagsPerApp := make(map[string]diag.Diagnostic) - + mu := &sync.Mutex{} 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 @@ -59,12 +59,14 @@ func (u *uploadConfig) Apply(ctx context.Context, b *bundle.Bundle) diag.Diagnos errGroup.Go(func() error { err = f.Write(ctx, path.Join(appPath, "app.yml"), buf, filer.OverwriteIfExists) if err != nil { - diagsPerApp[key] = diag.Diagnostic{ + mu.Lock() + 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)), - } + }) + mu.Unlock() } return nil }) @@ -75,10 +77,6 @@ func (u *uploadConfig) Apply(ctx context.Context, b *bundle.Bundle) diag.Diagnos return diags.Extend(diag.FromErr(err)) } - for _, diag := range diagsPerApp { - diags = append(diags, diag) - } - return diags } diff --git a/bundle/apps/validate.go b/bundle/apps/validate.go index dfb3fffada..2579db73e3 100644 --- a/bundle/apps/validate.go +++ b/bundle/apps/validate.go @@ -14,8 +14,19 @@ type validate struct{} func (v *validate) Apply(ctx context.Context, b *bundle.Bundle) diag.Diagnostics { var diags diag.Diagnostics possibleConfigFiles := []string{"app.yml", "app.yaml"} + usedSourceCodePaths := make(map[string]string) + + for key, app := range b.Config.Resources.Apps { + if _, ok := usedSourceCodePaths[app.SourceCodePath]; ok { + diags = append(diags, diag.Diagnostic{ + Severity: diag.Error, + Summary: "Duplicate app source code path", + Detail: fmt.Sprintf("app resource '%s' has the same source code path as app resource '%s'", key, usedSourceCodePaths[app.SourceCodePath]), + Locations: b.Config.GetLocations(fmt.Sprintf("resources.apps.%s.source_code_path", key)), + }) + } + usedSourceCodePaths[app.SourceCodePath] = key - for _, app := range b.Config.Resources.Apps { for _, configFile := range possibleConfigFiles { cf := path.Join(app.SourceCodePath, configFile) if _, err := b.SyncRoot.Stat(cf); err == nil { diff --git a/bundle/apps/validate_test.go b/bundle/apps/validate_test.go index d14d23731f..6ae0fd8060 100644 --- a/bundle/apps/validate_test.go +++ b/bundle/apps/validate_test.go @@ -53,3 +53,39 @@ func TestAppsValidate(t *testing.T) { require.Equal(t, "app.yml detected", diags[0].Summary) require.Contains(t, diags[0].Detail, "app.yml and use 'config' property for app resource") } + +func TestAppsValidateSameSourcePath(t *testing.T) { + tmpDir := t.TempDir() + testutil.Touch(t, tmpDir, "app1", "app.py") + + b := &bundle.Bundle{ + BundleRootPath: tmpDir, + SyncRootPath: tmpDir, + SyncRoot: vfs.MustNew(tmpDir), + Config: config.Root{ + Resources: config.Resources{ + Apps: map[string]*resources.App{ + "app1": { + App: &apps.App{ + Name: "app1", + }, + SourceCodePath: "./app1", + }, + "app2": { + App: &apps.App{ + Name: "app2", + }, + SourceCodePath: "./app1", + }, + }, + }, + }, + } + + bundletest.SetLocation(b, ".", []dyn.Location{{File: filepath.Join(tmpDir, "databricks.yml")}}) + + diags := bundle.Apply(context.Background(), b, bundle.Seq(mutator.TranslatePaths(), Validate())) + require.Len(t, diags, 1) + require.Equal(t, "Duplicate app source code path", diags[0].Summary) + require.Contains(t, diags[0].Detail, "has the same source code path as app resource") +} From 86630b61dd994cc62d3aca72c2370e6669c9d201 Mon Sep 17 00:00:00 2001 From: Andrew Nester Date: Mon, 23 Dec 2024 16:12:00 +0100 Subject: [PATCH 23/25] feedback fixes --- bundle/apps/interpolate_variables_test.go | 2 +- bundle/apps/upload_config.go | 14 +- bundle/apps/validate.go | 12 +- bundle/apps/validate_test.go | 34 ++ bundle/config/resources/apps.go | 2 +- bundle/deploy/terraform/convert.go | 6 +- bundle/deploy/terraform/convert_test.go | 16 +- bundle/deploy/terraform/util.go | 2 +- bundle/internal/schema/annotations.yml | 316 ++++++++--------- bundle/schema/jsonschema.json | 394 ++++++++++++++++++++++ integration/bundle/apps_test.go | 6 + 11 files changed, 620 insertions(+), 184 deletions(-) diff --git a/bundle/apps/interpolate_variables_test.go b/bundle/apps/interpolate_variables_test.go index 4fec8c6da3..a2909006f6 100644 --- a/bundle/apps/interpolate_variables_test.go +++ b/bundle/apps/interpolate_variables_test.go @@ -23,7 +23,7 @@ func TestAppInterpolateVariables(t *testing.T) { Config: map[string]any{ "command": []string{"echo", "hello"}, "env": []map[string]string{ - {"name": "JOB_ID", "value": "${resources.jobs.my_job.id}"}, + {"name": "JOB_ID", "value": "${databricks_job.my_job.id}"}, }, }, }, diff --git a/bundle/apps/upload_config.go b/bundle/apps/upload_config.go index b37f06351a..14893a1699 100644 --- a/bundle/apps/upload_config.go +++ b/bundle/apps/upload_config.go @@ -26,23 +26,12 @@ func (u *uploadConfig) Apply(ctx context.Context, b *bundle.Bundle) diag.Diagnos var diags diag.Diagnostics errGroup, ctx := errgroup.WithContext(ctx) - mu := &sync.Mutex{} + mu := sync.Mutex{} 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) @@ -50,7 +39,6 @@ func (u *uploadConfig) Apply(ctx context.Context, b *bundle.Bundle) diag.Diagnos 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) diff --git a/bundle/apps/validate.go b/bundle/apps/validate.go index 2579db73e3..da776aa053 100644 --- a/bundle/apps/validate.go +++ b/bundle/apps/validate.go @@ -4,6 +4,7 @@ import ( "context" "fmt" "path" + "strings" "github.com/databricks/cli/bundle" "github.com/databricks/cli/libs/diag" @@ -21,7 +22,7 @@ func (v *validate) Apply(ctx context.Context, b *bundle.Bundle) diag.Diagnostics diags = append(diags, diag.Diagnostic{ Severity: diag.Error, Summary: "Duplicate app source code path", - Detail: fmt.Sprintf("app resource '%s' has the same source code path as app resource '%s'", key, usedSourceCodePaths[app.SourceCodePath]), + Detail: fmt.Sprintf("app resource '%s' has the same source code path as app resource '%s', this will lead to the app configuration being overriden by each other", key, usedSourceCodePaths[app.SourceCodePath]), Locations: b.Config.GetLocations(fmt.Sprintf("resources.apps.%s.source_code_path", key)), }) } @@ -37,6 +38,15 @@ func (v *validate) Apply(ctx context.Context, b *bundle.Bundle) diag.Diagnostics }) } } + + 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)), + }) + } } return diags diff --git a/bundle/apps/validate_test.go b/bundle/apps/validate_test.go index 6ae0fd8060..3d860f3c47 100644 --- a/bundle/apps/validate_test.go +++ b/bundle/apps/validate_test.go @@ -11,6 +11,7 @@ import ( "github.com/databricks/cli/bundle/config/resources" "github.com/databricks/cli/bundle/internal/bundletest" "github.com/databricks/cli/internal/testutil" + "github.com/databricks/cli/libs/diag" "github.com/databricks/cli/libs/dyn" "github.com/databricks/cli/libs/vfs" "github.com/databricks/databricks-sdk-go/service/apps" @@ -89,3 +90,36 @@ func TestAppsValidateSameSourcePath(t *testing.T) { require.Equal(t, "Duplicate app source code path", diags[0].Summary) require.Contains(t, diags[0].Detail, "has the same source code path as app resource") } + +func TestAppsValidateIncorrectSourceCodePath(t *testing.T) { + tmpDir := t.TempDir() + + b := &bundle.Bundle{ + BundleRootPath: tmpDir, + SyncRootPath: tmpDir, + SyncRoot: vfs.MustNew(tmpDir), + Config: config.Root{ + Workspace: config.Workspace{ + FilePath: "/Workspace/Users/foo@bar.com/files", + }, + Resources: config.Resources{ + Apps: map[string]*resources.App{ + "app1": { + App: &apps.App{ + Name: "app1", + }, + SourceCodePath: "/Workspace/Random/app1", + }, + }, + }, + }, + } + + bundletest.SetLocation(b, ".", []dyn.Location{{File: filepath.Join(tmpDir, "databricks.yml")}}) + + diags := bundle.Apply(context.Background(), b, bundle.Seq(Validate())) + require.Len(t, diags, 1) + require.Equal(t, diag.Error, diags[0].Severity) + require.Equal(t, "App source code invalid", diags[0].Summary) + require.Contains(t, diags[0].Detail, "App source code path /Workspace/Random/app1 is not within file path /Workspace/Users/foo@bar.com/files") +} diff --git a/bundle/config/resources/apps.go b/bundle/config/resources/apps.go index ee3c40ad0e..723d5641f3 100644 --- a/bundle/config/resources/apps.go +++ b/bundle/config/resources/apps.go @@ -51,7 +51,7 @@ func (a *App) TerraformResourceName() string { } func (a *App) InitializeURL(baseURL url.URL) { - if a.Name == "" { + if a.ModifiedStatus == "" || a.ModifiedStatus == ModifiedStatusCreated { return } baseURL.Path = fmt.Sprintf("apps/%s", a.Name) diff --git a/bundle/deploy/terraform/convert.go b/bundle/deploy/terraform/convert.go index cdae42b3ec..d549b97973 100644 --- a/bundle/deploy/terraform/convert.go +++ b/bundle/deploy/terraform/convert.go @@ -204,6 +204,10 @@ func TerraformToBundle(state *resourcesState, config *config.Root) error { cur := config.Resources.Apps[resource.Name] if cur == nil { cur = &resources.App{ModifiedStatus: resources.ModifiedStatusDeleted, App: &apps.App{}} + } else { + // If the app exists in terraform and bundle, we always set modified status to updated + // because we don't really know if the app source code was updated or not. + cur.ModifiedStatus = resources.ModifiedStatusUpdated } cur.Name = instance.Attributes.Name config.Resources.Apps[resource.Name] = cur @@ -272,7 +276,7 @@ func TerraformToBundle(state *resourcesState, config *config.Root) error { } } for _, src := range config.Resources.Apps { - if src.ModifiedStatus == "" && src.Name == "" { + if src.ModifiedStatus == "" { src.ModifiedStatus = resources.ModifiedStatusCreated } } diff --git a/bundle/deploy/terraform/convert_test.go b/bundle/deploy/terraform/convert_test.go index 3da364f472..18e7693560 100644 --- a/bundle/deploy/terraform/convert_test.go +++ b/bundle/deploy/terraform/convert_test.go @@ -1019,12 +1019,12 @@ func TestTerraformToBundleModifiedResources(t *testing.T) { Apps: map[string]*resources.App{ "test_app": { App: &apps.App{ - Description: "test_app", + Name: "test_app", }, }, "test_app_new": { App: &apps.App{ - Description: "test_app_new", + Name: "test_app_new", }, }, }, @@ -1213,7 +1213,7 @@ func TestTerraformToBundleModifiedResources(t *testing.T) { Mode: "managed", Name: "test_app", Instances: []stateResourceInstance{ - {Attributes: stateInstanceAttributes{Name: "app1"}}, + {Attributes: stateInstanceAttributes{Name: "test_app"}}, }, }, { @@ -1221,7 +1221,7 @@ func TestTerraformToBundleModifiedResources(t *testing.T) { Mode: "managed", Name: "test_app_old", Instances: []stateResourceInstance{ - {Attributes: stateInstanceAttributes{Name: "app2"}}, + {Attributes: stateInstanceAttributes{Name: "test_app_old"}}, }, }, }, @@ -1306,11 +1306,11 @@ func TestTerraformToBundleModifiedResources(t *testing.T) { assert.Equal(t, "", config.Resources.Dashboards["test_dashboard_new"].ID) assert.Equal(t, resources.ModifiedStatusCreated, config.Resources.Dashboards["test_dashboard_new"].ModifiedStatus) - assert.Equal(t, "app1", config.Resources.Apps["test_app"].Name) - assert.Equal(t, "", config.Resources.Apps["test_app"].ModifiedStatus) - assert.Equal(t, "app2", config.Resources.Apps["test_app_old"].Name) + assert.Equal(t, "test_app", config.Resources.Apps["test_app"].Name) + assert.Equal(t, resources.ModifiedStatusUpdated, config.Resources.Apps["test_app"].ModifiedStatus) + assert.Equal(t, "test_app_old", config.Resources.Apps["test_app_old"].Name) assert.Equal(t, resources.ModifiedStatusDeleted, config.Resources.Apps["test_app_old"].ModifiedStatus) - assert.Equal(t, "", config.Resources.Apps["test_app_new"].Name) + assert.Equal(t, "test_app_new", config.Resources.Apps["test_app_new"].Name) assert.Equal(t, resources.ModifiedStatusCreated, config.Resources.Apps["test_app_new"].ModifiedStatus) AssertFullResourceCoverage(t, &config) diff --git a/bundle/deploy/terraform/util.go b/bundle/deploy/terraform/util.go index e2564ac22e..953554916f 100644 --- a/bundle/deploy/terraform/util.go +++ b/bundle/deploy/terraform/util.go @@ -34,7 +34,7 @@ type stateResourceInstance struct { type stateInstanceAttributes struct { ID string `json:"id"` - Name string `json:"name,omitempty"` + Name string `json:"name,omitempty"` // Some resources such as Apps do not have an ID, so we use the name instead. ETag string `json:"etag,omitempty"` } diff --git a/bundle/internal/schema/annotations.yml b/bundle/internal/schema/annotations.yml index b99fdc6140..e48ed065eb 100644 --- a/bundle/internal/schema/annotations.yml +++ b/bundle/internal/schema/annotations.yml @@ -1,161 +1,3 @@ -github.com/databricks/databricks-sdk-go/service/apps.AppResourceJob: - "id": - "description": |- - PLACEHOLDER - "permission": - "description": |- - PLACEHOLDER -github.com/databricks/databricks-sdk-go/service/apps.ComputeStatus: - "message": - "description": |- - PLACEHOLDER - "state": - "description": |- - PLACEHOLDER -github.com/databricks/databricks-sdk-go/service/apps.AppDeploymentArtifacts: - "source_code_path": - "description": |- - PLACEHOLDER -github.com/databricks/databricks-sdk-go/service/apps.AppResourceSqlWarehouse: - "id": - "description": |- - PLACEHOLDER - "permission": - "description": |- - PLACEHOLDER -github.com/databricks/cli/bundle/config/resources.App: - "create_time": - "description": |- - PLACEHOLDER - "permissions": - "description": |- - PLACEHOLDER - "resources": - "description": |- - PLACEHOLDER - "url": - "description": |- - PLACEHOLDER - "active_deployment": - "description": |- - PLACEHOLDER - "description": - "description": |- - PLACEHOLDER - "default_source_code_path": - "description": |- - PLACEHOLDER - "service_principal_client_id": - "description": |- - PLACEHOLDER - "service_principal_name": - "description": |- - PLACEHOLDER - "config": - "description": |- - PLACEHOLDER - "source_code_path": - "description": |- - PLACEHOLDER - "service_principal_id": - "description": |- - PLACEHOLDER - "name": - "description": |- - PLACEHOLDER - "compute_status": - "description": |- - PLACEHOLDER - "creator": - "description": |- - PLACEHOLDER - "app_status": - "description": |- - PLACEHOLDER - "pending_deployment": - "description": |- - PLACEHOLDER - "update_time": - "description": |- - PLACEHOLDER - "updater": - "description": |- - PLACEHOLDER -github.com/databricks/databricks-sdk-go/service/apps.AppResource: - "name": - "description": |- - PLACEHOLDER - "secret": - "description": |- - PLACEHOLDER - "serving_endpoint": - "description": |- - PLACEHOLDER - "sql_warehouse": - "description": |- - PLACEHOLDER - "description": - "description": |- - PLACEHOLDER - "job": - "description": |- - PLACEHOLDER -github.com/databricks/databricks-sdk-go/service/apps.AppDeployment: - "create_time": - "description": |- - PLACEHOLDER - "creator": - "description": |- - PLACEHOLDER - "deployment_artifacts": - "description": |- - PLACEHOLDER - "deployment_id": - "description": |- - PLACEHOLDER - "mode": - "description": |- - PLACEHOLDER - "source_code_path": - "description": |- - PLACEHOLDER - "status": - "description": |- - PLACEHOLDER - "update_time": - "description": |- - PLACEHOLDER -github.com/databricks/databricks-sdk-go/service/apps.ApplicationStatus: - "state": - "description": |- - PLACEHOLDER - "message": - "description": |- - PLACEHOLDER -github.com/databricks/databricks-sdk-go/service/apps.AppDeploymentStatus: - "message": - "description": |- - PLACEHOLDER - "state": - "description": |- - PLACEHOLDER -github.com/databricks/databricks-sdk-go/service/apps.AppResourceSecret: - "key": - "description": |- - PLACEHOLDER - "permission": - "description": |- - PLACEHOLDER - "scope": - "description": |- - PLACEHOLDER -github.com/databricks/databricks-sdk-go/service/apps.AppResourceServingEndpoint: - "permission": - "description": |- - PLACEHOLDER - "name": - "description": |- - PLACEHOLDER github.com/databricks/cli/bundle/config.Artifact: "build": "description": |- @@ -511,6 +353,64 @@ github.com/databricks/cli/bundle/config.Workspace: "state_path": "description": |- The workspace state path +github.com/databricks/cli/bundle/config/resources.App: + "active_deployment": + "description": |- + PLACEHOLDER + "app_status": + "description": |- + PLACEHOLDER + "compute_status": + "description": |- + PLACEHOLDER + "config": + "description": |- + PLACEHOLDER + "create_time": + "description": |- + PLACEHOLDER + "creator": + "description": |- + PLACEHOLDER + "default_source_code_path": + "description": |- + PLACEHOLDER + "description": + "description": |- + PLACEHOLDER + "name": + "description": |- + PLACEHOLDER + "pending_deployment": + "description": |- + PLACEHOLDER + "permissions": + "description": |- + PLACEHOLDER + "resources": + "description": |- + PLACEHOLDER + "service_principal_client_id": + "description": |- + PLACEHOLDER + "service_principal_id": + "description": |- + PLACEHOLDER + "service_principal_name": + "description": |- + PLACEHOLDER + "source_code_path": + "description": |- + PLACEHOLDER + "update_time": + "description": |- + PLACEHOLDER + "updater": + "description": |- + PLACEHOLDER + "url": + "description": |- + PLACEHOLDER github.com/databricks/cli/bundle/config/resources.Grant: "principal": "description": |- @@ -599,3 +499,103 @@ github.com/databricks/cli/bundle/config/variable.Variable: "type": "description": |- The type of the variable. +github.com/databricks/databricks-sdk-go/service/apps.AppDeployment: + "create_time": + "description": |- + PLACEHOLDER + "creator": + "description": |- + PLACEHOLDER + "deployment_artifacts": + "description": |- + PLACEHOLDER + "deployment_id": + "description": |- + PLACEHOLDER + "mode": + "description": |- + PLACEHOLDER + "source_code_path": + "description": |- + PLACEHOLDER + "status": + "description": |- + PLACEHOLDER + "update_time": + "description": |- + PLACEHOLDER +github.com/databricks/databricks-sdk-go/service/apps.AppDeploymentArtifacts: + "source_code_path": + "description": |- + PLACEHOLDER +github.com/databricks/databricks-sdk-go/service/apps.AppDeploymentStatus: + "message": + "description": |- + PLACEHOLDER + "state": + "description": |- + PLACEHOLDER +github.com/databricks/databricks-sdk-go/service/apps.AppResource: + "description": + "description": |- + PLACEHOLDER + "job": + "description": |- + PLACEHOLDER + "name": + "description": |- + PLACEHOLDER + "secret": + "description": |- + PLACEHOLDER + "serving_endpoint": + "description": |- + PLACEHOLDER + "sql_warehouse": + "description": |- + PLACEHOLDER +github.com/databricks/databricks-sdk-go/service/apps.AppResourceJob: + "id": + "description": |- + PLACEHOLDER + "permission": + "description": |- + PLACEHOLDER +github.com/databricks/databricks-sdk-go/service/apps.AppResourceSecret: + "key": + "description": |- + PLACEHOLDER + "permission": + "description": |- + PLACEHOLDER + "scope": + "description": |- + PLACEHOLDER +github.com/databricks/databricks-sdk-go/service/apps.AppResourceServingEndpoint: + "name": + "description": |- + PLACEHOLDER + "permission": + "description": |- + PLACEHOLDER +github.com/databricks/databricks-sdk-go/service/apps.AppResourceSqlWarehouse: + "id": + "description": |- + PLACEHOLDER + "permission": + "description": |- + PLACEHOLDER +github.com/databricks/databricks-sdk-go/service/apps.ApplicationStatus: + "message": + "description": |- + PLACEHOLDER + "state": + "description": |- + PLACEHOLDER +github.com/databricks/databricks-sdk-go/service/apps.ComputeStatus: + "message": + "description": |- + PLACEHOLDER + "state": + "description": |- + PLACEHOLDER diff --git a/bundle/schema/jsonschema.json b/bundle/schema/jsonschema.json index 9a352ebb2a..b31f54b7af 100644 --- a/bundle/schema/jsonschema.json +++ b/bundle/schema/jsonschema.json @@ -59,6 +59,81 @@ "cli": { "bundle": { "config": { + "resources.App": { + "oneOf": [ + { + "type": "object", + "properties": { + "active_deployment": { + "$ref": "#/$defs/github.com/databricks/databricks-sdk-go/service/apps.AppDeployment" + }, + "app_status": { + "$ref": "#/$defs/github.com/databricks/databricks-sdk-go/service/apps.ApplicationStatus" + }, + "compute_status": { + "$ref": "#/$defs/github.com/databricks/databricks-sdk-go/service/apps.ComputeStatus" + }, + "config": { + "$ref": "#/$defs/map/interface" + }, + "create_time": { + "$ref": "#/$defs/string" + }, + "creator": { + "$ref": "#/$defs/string" + }, + "default_source_code_path": { + "$ref": "#/$defs/string" + }, + "description": { + "$ref": "#/$defs/string" + }, + "name": { + "$ref": "#/$defs/string" + }, + "pending_deployment": { + "$ref": "#/$defs/github.com/databricks/databricks-sdk-go/service/apps.AppDeployment" + }, + "permissions": { + "$ref": "#/$defs/slice/github.com/databricks/cli/bundle/config/resources.Permission" + }, + "resources": { + "$ref": "#/$defs/slice/github.com/databricks/databricks-sdk-go/service/apps.AppResource" + }, + "service_principal_client_id": { + "$ref": "#/$defs/string" + }, + "service_principal_id": { + "$ref": "#/$defs/int64" + }, + "service_principal_name": { + "$ref": "#/$defs/string" + }, + "source_code_path": { + "$ref": "#/$defs/string" + }, + "update_time": { + "$ref": "#/$defs/string" + }, + "updater": { + "$ref": "#/$defs/string" + }, + "url": { + "$ref": "#/$defs/string" + } + }, + "additionalProperties": false, + "required": [ + "source_code_path", + "name" + ] + }, + { + "type": "string", + "pattern": "\\$\\{(var(\\.[a-zA-Z]+([-_]?[a-zA-Z0-9]+)*(\\[[0-9]+\\])*)+)\\}" + } + ] + }, "resources.Cluster": { "oneOf": [ { @@ -1239,6 +1314,9 @@ { "type": "object", "properties": { + "apps": { + "$ref": "#/$defs/map/github.com/databricks/cli/bundle/config/resources.App" + }, "clusters": { "description": "The cluster definitions for the bundle.", "$ref": "#/$defs/map/github.com/databricks/cli/bundle/config/resources.Cluster", @@ -1494,6 +1572,280 @@ }, "databricks-sdk-go": { "service": { + "apps.AppDeployment": { + "oneOf": [ + { + "type": "object", + "properties": { + "create_time": { + "$ref": "#/$defs/string" + }, + "creator": { + "$ref": "#/$defs/string" + }, + "deployment_artifacts": { + "$ref": "#/$defs/github.com/databricks/databricks-sdk-go/service/apps.AppDeploymentArtifacts" + }, + "deployment_id": { + "$ref": "#/$defs/string" + }, + "mode": { + "$ref": "#/$defs/github.com/databricks/databricks-sdk-go/service/apps.AppDeploymentMode" + }, + "source_code_path": { + "$ref": "#/$defs/string" + }, + "status": { + "$ref": "#/$defs/github.com/databricks/databricks-sdk-go/service/apps.AppDeploymentStatus" + }, + "update_time": { + "$ref": "#/$defs/string" + } + }, + "additionalProperties": false + }, + { + "type": "string", + "pattern": "\\$\\{(var(\\.[a-zA-Z]+([-_]?[a-zA-Z0-9]+)*(\\[[0-9]+\\])*)+)\\}" + } + ] + }, + "apps.AppDeploymentArtifacts": { + "oneOf": [ + { + "type": "object", + "properties": { + "source_code_path": { + "$ref": "#/$defs/string" + } + }, + "additionalProperties": false + }, + { + "type": "string", + "pattern": "\\$\\{(var(\\.[a-zA-Z]+([-_]?[a-zA-Z0-9]+)*(\\[[0-9]+\\])*)+)\\}" + } + ] + }, + "apps.AppDeploymentMode": { + "type": "string" + }, + "apps.AppDeploymentState": { + "type": "string" + }, + "apps.AppDeploymentStatus": { + "oneOf": [ + { + "type": "object", + "properties": { + "message": { + "$ref": "#/$defs/string" + }, + "state": { + "$ref": "#/$defs/github.com/databricks/databricks-sdk-go/service/apps.AppDeploymentState" + } + }, + "additionalProperties": false + }, + { + "type": "string", + "pattern": "\\$\\{(var(\\.[a-zA-Z]+([-_]?[a-zA-Z0-9]+)*(\\[[0-9]+\\])*)+)\\}" + } + ] + }, + "apps.AppResource": { + "oneOf": [ + { + "type": "object", + "properties": { + "description": { + "$ref": "#/$defs/string" + }, + "job": { + "$ref": "#/$defs/github.com/databricks/databricks-sdk-go/service/apps.AppResourceJob" + }, + "name": { + "$ref": "#/$defs/string" + }, + "secret": { + "$ref": "#/$defs/github.com/databricks/databricks-sdk-go/service/apps.AppResourceSecret" + }, + "serving_endpoint": { + "$ref": "#/$defs/github.com/databricks/databricks-sdk-go/service/apps.AppResourceServingEndpoint" + }, + "sql_warehouse": { + "$ref": "#/$defs/github.com/databricks/databricks-sdk-go/service/apps.AppResourceSqlWarehouse" + } + }, + "additionalProperties": false, + "required": [ + "name" + ] + }, + { + "type": "string", + "pattern": "\\$\\{(var(\\.[a-zA-Z]+([-_]?[a-zA-Z0-9]+)*(\\[[0-9]+\\])*)+)\\}" + } + ] + }, + "apps.AppResourceJob": { + "oneOf": [ + { + "type": "object", + "properties": { + "id": { + "$ref": "#/$defs/string" + }, + "permission": { + "$ref": "#/$defs/github.com/databricks/databricks-sdk-go/service/apps.AppResourceJobJobPermission" + } + }, + "additionalProperties": false, + "required": [ + "id", + "permission" + ] + }, + { + "type": "string", + "pattern": "\\$\\{(var(\\.[a-zA-Z]+([-_]?[a-zA-Z0-9]+)*(\\[[0-9]+\\])*)+)\\}" + } + ] + }, + "apps.AppResourceJobJobPermission": { + "type": "string" + }, + "apps.AppResourceSecret": { + "oneOf": [ + { + "type": "object", + "properties": { + "key": { + "$ref": "#/$defs/string" + }, + "permission": { + "$ref": "#/$defs/github.com/databricks/databricks-sdk-go/service/apps.AppResourceSecretSecretPermission" + }, + "scope": { + "$ref": "#/$defs/string" + } + }, + "additionalProperties": false, + "required": [ + "key", + "permission", + "scope" + ] + }, + { + "type": "string", + "pattern": "\\$\\{(var(\\.[a-zA-Z]+([-_]?[a-zA-Z0-9]+)*(\\[[0-9]+\\])*)+)\\}" + } + ] + }, + "apps.AppResourceSecretSecretPermission": { + "type": "string" + }, + "apps.AppResourceServingEndpoint": { + "oneOf": [ + { + "type": "object", + "properties": { + "name": { + "$ref": "#/$defs/string" + }, + "permission": { + "$ref": "#/$defs/github.com/databricks/databricks-sdk-go/service/apps.AppResourceServingEndpointServingEndpointPermission" + } + }, + "additionalProperties": false, + "required": [ + "name", + "permission" + ] + }, + { + "type": "string", + "pattern": "\\$\\{(var(\\.[a-zA-Z]+([-_]?[a-zA-Z0-9]+)*(\\[[0-9]+\\])*)+)\\}" + } + ] + }, + "apps.AppResourceServingEndpointServingEndpointPermission": { + "type": "string" + }, + "apps.AppResourceSqlWarehouse": { + "oneOf": [ + { + "type": "object", + "properties": { + "id": { + "$ref": "#/$defs/string" + }, + "permission": { + "$ref": "#/$defs/github.com/databricks/databricks-sdk-go/service/apps.AppResourceSqlWarehouseSqlWarehousePermission" + } + }, + "additionalProperties": false, + "required": [ + "id", + "permission" + ] + }, + { + "type": "string", + "pattern": "\\$\\{(var(\\.[a-zA-Z]+([-_]?[a-zA-Z0-9]+)*(\\[[0-9]+\\])*)+)\\}" + } + ] + }, + "apps.AppResourceSqlWarehouseSqlWarehousePermission": { + "type": "string" + }, + "apps.ApplicationState": { + "type": "string" + }, + "apps.ApplicationStatus": { + "oneOf": [ + { + "type": "object", + "properties": { + "message": { + "$ref": "#/$defs/string" + }, + "state": { + "$ref": "#/$defs/github.com/databricks/databricks-sdk-go/service/apps.ApplicationState" + } + }, + "additionalProperties": false + }, + { + "type": "string", + "pattern": "\\$\\{(var(\\.[a-zA-Z]+([-_]?[a-zA-Z0-9]+)*(\\[[0-9]+\\])*)+)\\}" + } + ] + }, + "apps.ComputeState": { + "type": "string" + }, + "apps.ComputeStatus": { + "oneOf": [ + { + "type": "object", + "properties": { + "message": { + "$ref": "#/$defs/string" + }, + "state": { + "$ref": "#/$defs/github.com/databricks/databricks-sdk-go/service/apps.ComputeState" + } + }, + "additionalProperties": false + }, + { + "type": "string", + "pattern": "\\$\\{(var(\\.[a-zA-Z]+([-_]?[a-zA-Z0-9]+)*(\\[[0-9]+\\])*)+)\\}" + } + ] + }, "catalog.MonitorCronSchedule": { "oneOf": [ { @@ -5684,6 +6036,20 @@ "cli": { "bundle": { "config": { + "resources.App": { + "oneOf": [ + { + "type": "object", + "additionalProperties": { + "$ref": "#/$defs/github.com/databricks/cli/bundle/config/resources.App" + } + }, + { + "type": "string", + "pattern": "\\$\\{(var(\\.[a-zA-Z]+([-_]?[a-zA-Z0-9]+)*(\\[[0-9]+\\])*)+)\\}" + } + ] + }, "resources.Cluster": { "oneOf": [ { @@ -5913,6 +6279,20 @@ } } }, + "interface": { + "oneOf": [ + { + "type": "object", + "additionalProperties": { + "$ref": "#/$defs/interface" + } + }, + { + "type": "string", + "pattern": "\\$\\{(var(\\.[a-zA-Z]+([-_]?[a-zA-Z0-9]+)*(\\[[0-9]+\\])*)+)\\}" + } + ] + }, "string": { "oneOf": [ { @@ -5981,6 +6361,20 @@ }, "databricks-sdk-go": { "service": { + "apps.AppResource": { + "oneOf": [ + { + "type": "array", + "items": { + "$ref": "#/$defs/github.com/databricks/databricks-sdk-go/service/apps.AppResource" + } + }, + { + "type": "string", + "pattern": "\\$\\{(var(\\.[a-zA-Z]+([-_]?[a-zA-Z0-9]+)*(\\[[0-9]+\\])*)+)\\}" + } + ] + }, "catalog.MonitorMetric": { "oneOf": [ { diff --git a/integration/bundle/apps_test.go b/integration/bundle/apps_test.go index aa0d1a4678..6873842191 100644 --- a/integration/bundle/apps_test.go +++ b/integration/bundle/apps_test.go @@ -16,6 +16,12 @@ import ( func TestDeployBundleWithApp(t *testing.T) { ctx, wt := acc.WorkspaceTest(t) + // TODO: should only skip app run when app can be created with no_compute option. + if testing.Short() { + t.Log("Skip the app creation and run in short mode") + return + } + if testutil.GetCloud(t) == testutil.GCP { t.Skip("Skipping test for GCP cloud because /api/2.0/apps is temporarily unavailable there.") } From 4b4cc42e5d97ba0178c618d976f03d2fd55706af Mon Sep 17 00:00:00 2001 From: Andrew Nester Date: Mon, 30 Dec 2024 12:44:09 +0100 Subject: [PATCH 24/25] fixs --- bundle/apps/interpolate_variables.go | 17 +--- bundle/apps/upload_config.go | 2 +- bundle/apps/upload_config_test.go | 8 +- bundle/apps/validate.go | 12 +-- bundle/apps/validate_test.go | 40 ++------ bundle/config/mutator/run_as_test.go | 132 ++++++++++++++++++--------- bundle/config/resources.go | 110 ++++++++++++---------- bundle/deploy/terraform/util.go | 8 +- integration/bundle/apps_test.go | 3 + 9 files changed, 182 insertions(+), 150 deletions(-) diff --git a/bundle/apps/interpolate_variables.go b/bundle/apps/interpolate_variables.go index f4860127b0..f88e7e9db7 100644 --- a/bundle/apps/interpolate_variables.go +++ b/bundle/apps/interpolate_variables.go @@ -4,6 +4,7 @@ import ( "context" "github.com/databricks/cli/bundle" + "github.com/databricks/cli/bundle/config" "github.com/databricks/cli/libs/diag" "github.com/databricks/cli/libs/dyn" "github.com/databricks/cli/libs/dyn/dynvar" @@ -19,19 +20,9 @@ func (i *interpolateVariables) Apply(ctx context.Context, b *bundle.Bundle) diag dyn.Key("config"), ) - tfToConfigMap := map[string]string{ - "databricks_pipeline": "pipelines", - "databricks_job": "jobs", - "databricks_mlflow_model": "models", - "databricks_mlflow_experiment": "experiments", - "databricks_model_serving": "model_serving_endpoints", - "databricks_registered_model": "registered_models", - "databricks_quality_monitor": "quality_monitors", - "databricks_schema": "schemas", - "databricks_volume": "volumes", - "databricks_cluster": "clusters", - "databricks_dashboard": "dashboards", - "databricks_app": "apps", + tfToConfigMap := map[string]string{} + for k, r := range config.SupportedResources() { + tfToConfigMap[r.TerraformResourceName] = k } err := b.Config.Mutate(func(root dyn.Value) (dyn.Value, error) { diff --git a/bundle/apps/upload_config.go b/bundle/apps/upload_config.go index 14893a1699..0e3db269e8 100644 --- a/bundle/apps/upload_config.go +++ b/bundle/apps/upload_config.go @@ -45,7 +45,7 @@ func (u *uploadConfig) Apply(ctx context.Context, b *bundle.Bundle) diag.Diagnos } errGroup.Go(func() error { - err = f.Write(ctx, path.Join(appPath, "app.yml"), buf, filer.OverwriteIfExists) + err := f.Write(ctx, path.Join(appPath, "app.yml"), buf, filer.OverwriteIfExists) if err != nil { mu.Lock() diags = append(diags, diag.Diagnostic{ diff --git a/bundle/apps/upload_config_test.go b/bundle/apps/upload_config_test.go index 2d9b1c70d1..a1a6b3afb1 100644 --- a/bundle/apps/upload_config_test.go +++ b/bundle/apps/upload_config_test.go @@ -9,8 +9,11 @@ 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/resources" + "github.com/databricks/cli/bundle/internal/bundletest" mockfiler "github.com/databricks/cli/internal/mocks/libs/filer" + "github.com/databricks/cli/libs/dyn" "github.com/databricks/cli/libs/filer" "github.com/databricks/cli/libs/vfs" "github.com/databricks/databricks-sdk-go/service/apps" @@ -25,6 +28,7 @@ func TestAppUploadConfig(t *testing.T) { b := &bundle.Bundle{ BundleRootPath: root, + SyncRootPath: root, SyncRoot: vfs.MustNew(root), Config: config.Root{ Workspace: config.Workspace{ @@ -64,6 +68,8 @@ env: }, } - diags := bundle.Apply(context.Background(), b, &u) + bundletest.SetLocation(b, ".", []dyn.Location{{File: filepath.Join(root, "databricks.yml")}}) + + diags := bundle.Apply(context.Background(), b, bundle.Seq(mutator.TranslatePaths(), &u)) require.NoError(t, diags.Error()) } diff --git a/bundle/apps/validate.go b/bundle/apps/validate.go index da776aa053..d3221a948a 100644 --- a/bundle/apps/validate.go +++ b/bundle/apps/validate.go @@ -29,7 +29,8 @@ func (v *validate) Apply(ctx context.Context, b *bundle.Bundle) diag.Diagnostics usedSourceCodePaths[app.SourceCodePath] = key for _, configFile := range possibleConfigFiles { - cf := path.Join(app.SourceCodePath, configFile) + appPath := strings.TrimPrefix(app.SourceCodePath, b.Config.Workspace.FilePath) + cf := path.Join(appPath, configFile) if _, err := b.SyncRoot.Stat(cf); err == nil { diags = append(diags, diag.Diagnostic{ Severity: diag.Error, @@ -38,15 +39,6 @@ func (v *validate) Apply(ctx context.Context, b *bundle.Bundle) diag.Diagnostics }) } } - - 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)), - }) - } } return diags diff --git a/bundle/apps/validate_test.go b/bundle/apps/validate_test.go index 3d860f3c47..6c3a881910 100644 --- a/bundle/apps/validate_test.go +++ b/bundle/apps/validate_test.go @@ -11,7 +11,6 @@ import ( "github.com/databricks/cli/bundle/config/resources" "github.com/databricks/cli/bundle/internal/bundletest" "github.com/databricks/cli/internal/testutil" - "github.com/databricks/cli/libs/diag" "github.com/databricks/cli/libs/dyn" "github.com/databricks/cli/libs/vfs" "github.com/databricks/databricks-sdk-go/service/apps" @@ -28,6 +27,9 @@ func TestAppsValidate(t *testing.T) { SyncRootPath: tmpDir, SyncRoot: vfs.MustNew(tmpDir), Config: config.Root{ + Workspace: config.Workspace{ + FilePath: "/foo/bar/", + }, Resources: config.Resources{ Apps: map[string]*resources.App{ "app1": { @@ -64,6 +66,9 @@ func TestAppsValidateSameSourcePath(t *testing.T) { SyncRootPath: tmpDir, SyncRoot: vfs.MustNew(tmpDir), Config: config.Root{ + Workspace: config.Workspace{ + FilePath: "/foo/bar/", + }, Resources: config.Resources{ Apps: map[string]*resources.App{ "app1": { @@ -90,36 +95,3 @@ func TestAppsValidateSameSourcePath(t *testing.T) { require.Equal(t, "Duplicate app source code path", diags[0].Summary) require.Contains(t, diags[0].Detail, "has the same source code path as app resource") } - -func TestAppsValidateIncorrectSourceCodePath(t *testing.T) { - tmpDir := t.TempDir() - - b := &bundle.Bundle{ - BundleRootPath: tmpDir, - SyncRootPath: tmpDir, - SyncRoot: vfs.MustNew(tmpDir), - Config: config.Root{ - Workspace: config.Workspace{ - FilePath: "/Workspace/Users/foo@bar.com/files", - }, - Resources: config.Resources{ - Apps: map[string]*resources.App{ - "app1": { - App: &apps.App{ - Name: "app1", - }, - SourceCodePath: "/Workspace/Random/app1", - }, - }, - }, - }, - } - - bundletest.SetLocation(b, ".", []dyn.Location{{File: filepath.Join(tmpDir, "databricks.yml")}}) - - diags := bundle.Apply(context.Background(), b, bundle.Seq(Validate())) - require.Len(t, diags, 1) - require.Equal(t, diag.Error, diags[0].Severity) - require.Equal(t, "App source code invalid", diags[0].Summary) - require.Contains(t, diags[0].Detail, "App source code path /Workspace/Random/app1 is not within file path /Workspace/Users/foo@bar.com/files") -} diff --git a/bundle/config/mutator/run_as_test.go b/bundle/config/mutator/run_as_test.go index afd10e748f..650b65d61c 100644 --- a/bundle/config/mutator/run_as_test.go +++ b/bundle/config/mutator/run_as_test.go @@ -105,48 +105,47 @@ func TestRunAsWorksForAllowedResources(t *testing.T) { } } -func TestRunAsErrorForUnsupportedResources(t *testing.T) { - // Bundle "run_as" has two modes of operation, each with a different set of - // resources that are supported. - // Cases: - // 1. When the bundle "run_as" identity is same as the current deployment - // identity. In this case all resources are supported. - // 2. When the bundle "run_as" identity is different from the current - // deployment identity. In this case only a subset of resources are - // supported. This subset of resources are defined in the allow list below. - // - // To be a part of the allow list, the resource must satisfy one of the following - // two conditions: - // 1. The resource supports setting a run_as identity to a different user - // from the owner/creator of the resource. For example, jobs. - // 2. Run as semantics do not apply to the resource. We do not plan to add - // platform side support for `run_as` for these resources. For example, - // experiments or registered models. - // - // Any resource that is not on the allow list cannot be used when the bundle - // run_as is different from the current deployment user. "bundle validate" must - // return an error if such a resource has been defined, and the run_as identity - // is different from the current deployment identity. - // - // Action Item: If you are adding a new resource to DABs, please check in with - // the relevant owning team whether the resource should be on the allow list or (implicitly) on - // the deny list. Any resources that could have run_as semantics in the future - // should be on the deny list. - // For example: Teams for pipelines, model serving endpoints or Lakeview dashboards - // are planning to add platform side support for `run_as` for these resources at - // some point in the future. These resources are (implicitly) on the deny list, since - // they are not on the allow list below. - allowList := []string{ - "clusters", - "jobs", - "models", - "registered_models", - "experiments", - "schemas", - "volumes", - "apps", - } +// Bundle "run_as" has two modes of operation, each with a different set of +// resources that are supported. +// Cases: +// 1. When the bundle "run_as" identity is same as the current deployment +// identity. In this case all resources are supported. +// 2. When the bundle "run_as" identity is different from the current +// deployment identity. In this case only a subset of resources are +// supported. This subset of resources are defined in the allow list below. +// +// To be a part of the allow list, the resource must satisfy one of the following +// two conditions: +// 1. The resource supports setting a run_as identity to a different user +// from the owner/creator of the resource. For example, jobs. +// 2. Run as semantics do not apply to the resource. We do not plan to add +// platform side support for `run_as` for these resources. For example, +// experiments or registered models. +// +// Any resource that is not on the allow list cannot be used when the bundle +// run_as is different from the current deployment user. "bundle validate" must +// return an error if such a resource has been defined, and the run_as identity +// is different from the current deployment identity. +// +// Action Item: If you are adding a new resource to DABs, please check in with +// the relevant owning team whether the resource should be on the allow list or (implicitly) on +// the deny list. Any resources that could have run_as semantics in the future +// should be on the deny list. +// For example: Teams for pipelines, model serving endpoints or Lakeview dashboards +// are planning to add platform side support for `run_as` for these resources at +// some point in the future. These resources are (implicitly) on the deny list, since +// they are not on the allow list below. +var allowList = []string{ + "clusters", + "jobs", + "models", + "registered_models", + "experiments", + "schemas", + "volumes", +} +func TestRunAsErrorForUnsupportedResources(t *testing.T) { base := config.Root{ Workspace: config.Workspace{ CurrentUser: &config.User{ @@ -199,3 +198,54 @@ func TestRunAsErrorForUnsupportedResources(t *testing.T) { "See https://docs.databricks.com/dev-tools/bundles/run-as.html to learn more about the run_as property.", rt) } } + +func TestRunAsNoErrorForSupportedResources(t *testing.T) { + base := config.Root{ + Workspace: config.Workspace{ + CurrentUser: &config.User{ + User: &iam.User{ + UserName: "alice", + }, + }, + }, + RunAs: &jobs.JobRunAs{ + UserName: "bob", + }, + } + + v, err := convert.FromTyped(base, dyn.NilValue) + require.NoError(t, err) + + // Define top level resources key in the bundle configuration. + // This is not part of the typed configuration, so we need to add it manually. + v, err = dyn.Set(v, "resources", dyn.V(map[string]dyn.Value{})) + require.NoError(t, err) + + for _, rt := range allResourceTypes(t) { + // Skip unsupported resources + if !slices.Contains(allowList, rt) { + continue + } + + // Add an instance of the resource type that is not on the allow list to + // the bundle configuration. + nv, err := dyn.SetByPath(v, dyn.NewPath(dyn.Key("resources"), dyn.Key(rt)), dyn.V(map[string]dyn.Value{ + "foo": dyn.V(map[string]dyn.Value{ + "name": dyn.V("bar"), + }), + })) + require.NoError(t, err) + + // Get back typed configuration from the newly created invalid bundle configuration. + r := &config.Root{} + err = convert.ToTyped(r, nv) + require.NoError(t, err) + + // Assert this configuration passes validation. + b := &bundle.Bundle{ + Config: *r, + } + diags := bundle.Apply(context.Background(), b, SetRunAs()) + require.NoError(t, diags.Error()) + } +} diff --git a/bundle/config/resources.go b/bundle/config/resources.go index b06a44b4dd..1f523fed30 100644 --- a/bundle/config/resources.go +++ b/bundle/config/resources.go @@ -135,82 +135,96 @@ type ResourceDescription struct { // Singular and plural title when used in summaries / terminal UI. SingularTitle string PluralTitle string + + TerraformResourceName string } // The keys of the map corresponds to the resource key in the bundle configuration. func SupportedResources() map[string]ResourceDescription { return map[string]ResourceDescription{ "jobs": { - SingularName: "job", - PluralName: "jobs", - SingularTitle: "Job", - PluralTitle: "Jobs", + SingularName: "job", + PluralName: "jobs", + SingularTitle: "Job", + PluralTitle: "Jobs", + TerraformResourceName: "databricks_job", }, "pipelines": { - SingularName: "pipeline", - PluralName: "pipelines", - SingularTitle: "Pipeline", - PluralTitle: "Pipelines", + SingularName: "pipeline", + PluralName: "pipelines", + SingularTitle: "Pipeline", + PluralTitle: "Pipelines", + TerraformResourceName: "databricks_pipeline", }, "models": { - SingularName: "model", - PluralName: "models", - SingularTitle: "Model", - PluralTitle: "Models", + SingularName: "model", + PluralName: "models", + SingularTitle: "Model", + PluralTitle: "Models", + TerraformResourceName: "databricks_mlflow_model", }, "experiments": { - SingularName: "experiment", - PluralName: "experiments", - SingularTitle: "Experiment", - PluralTitle: "Experiments", + SingularName: "experiment", + PluralName: "experiments", + SingularTitle: "Experiment", + PluralTitle: "Experiments", + TerraformResourceName: "databricks_mlflow_experiment", }, "model_serving_endpoints": { - SingularName: "model_serving_endpoint", - PluralName: "model_serving_endpoints", - SingularTitle: "Model Serving Endpoint", - PluralTitle: "Model Serving Endpoints", + SingularName: "model_serving_endpoint", + PluralName: "model_serving_endpoints", + SingularTitle: "Model Serving Endpoint", + PluralTitle: "Model Serving Endpoints", + TerraformResourceName: "databricks_model_serving_endpoint", }, "registered_models": { - SingularName: "registered_model", - PluralName: "registered_models", - SingularTitle: "Registered Model", - PluralTitle: "Registered Models", + SingularName: "registered_model", + PluralName: "registered_models", + SingularTitle: "Registered Model", + PluralTitle: "Registered Models", + TerraformResourceName: "databricks_registered_model", }, "quality_monitors": { - SingularName: "quality_monitor", - PluralName: "quality_monitors", - SingularTitle: "Quality Monitor", - PluralTitle: "Quality Monitors", + SingularName: "quality_monitor", + PluralName: "quality_monitors", + SingularTitle: "Quality Monitor", + PluralTitle: "Quality Monitors", + TerraformResourceName: "databricks_quality_monitor", }, "schemas": { - SingularName: "schema", - PluralName: "schemas", - SingularTitle: "Schema", - PluralTitle: "Schemas", + SingularName: "schema", + PluralName: "schemas", + SingularTitle: "Schema", + PluralTitle: "Schemas", + TerraformResourceName: "databricks_schema", }, "clusters": { - SingularName: "cluster", - PluralName: "clusters", - SingularTitle: "Cluster", - PluralTitle: "Clusters", + SingularName: "cluster", + PluralName: "clusters", + SingularTitle: "Cluster", + PluralTitle: "Clusters", + TerraformResourceName: "databricks_cluster", }, "dashboards": { - SingularName: "dashboard", - PluralName: "dashboards", - SingularTitle: "Dashboard", - PluralTitle: "Dashboards", + SingularName: "dashboard", + PluralName: "dashboards", + SingularTitle: "Dashboard", + PluralTitle: "Dashboards", + TerraformResourceName: "databricks_dashboard", }, "volumes": { - SingularName: "volume", - PluralName: "volumes", - SingularTitle: "Volume", - PluralTitle: "Volumes", + SingularName: "volume", + PluralName: "volumes", + SingularTitle: "Volume", + PluralTitle: "Volumes", + TerraformResourceName: "databricks_volume", }, "apps": { - SingularName: "app", - PluralName: "apps", - SingularTitle: "App", - PluralTitle: "Apps", + SingularName: "app", + PluralName: "apps", + SingularTitle: "App", + PluralTitle: "Apps", + TerraformResourceName: "databricks_app", }, } } diff --git a/bundle/deploy/terraform/util.go b/bundle/deploy/terraform/util.go index 953554916f..90dfe37b2a 100644 --- a/bundle/deploy/terraform/util.go +++ b/bundle/deploy/terraform/util.go @@ -33,8 +33,12 @@ type stateResourceInstance struct { } type stateInstanceAttributes struct { - ID string `json:"id"` - Name string `json:"name,omitempty"` // Some resources such as Apps do not have an ID, so we use the name instead. + ID string `json:"id"` + + // Some resources such as Apps do not have an ID, so we use the name instead. + // We need this for cases when such resource is removed from bundle config but + // exists in the workspace still so we can correctly display its summary. + Name string `json:"name,omitempty"` ETag string `json:"etag,omitempty"` } diff --git a/integration/bundle/apps_test.go b/integration/bundle/apps_test.go index 6873842191..6d77db492a 100644 --- a/integration/bundle/apps_test.go +++ b/integration/bundle/apps_test.go @@ -107,4 +107,7 @@ env: require.NoError(t, err) require.NotNil(t, app) require.Equal(t, apps.ApplicationStateRunning, app.AppStatus.State) + + // Redeploy it again just to check that it can be redeployed + deployBundle(t, ctx, root) } From 7693f0add0ab88bd49c109047cedc75cc16edd42 Mon Sep 17 00:00:00 2001 From: Andrew Nester Date: Tue, 31 Dec 2024 15:16:47 +0100 Subject: [PATCH 25/25] always set description field --- bundle/deploy/terraform/tfdyn/convert_app.go | 8 +++ .../terraform/tfdyn/convert_app_test.go | 58 +++++++++++++++++++ 2 files changed, 66 insertions(+) diff --git a/bundle/deploy/terraform/tfdyn/convert_app.go b/bundle/deploy/terraform/tfdyn/convert_app.go index 235b3a56fc..802ebc4c82 100644 --- a/bundle/deploy/terraform/tfdyn/convert_app.go +++ b/bundle/deploy/terraform/tfdyn/convert_app.go @@ -12,6 +12,14 @@ import ( ) func convertAppResource(ctx context.Context, vin dyn.Value) (dyn.Value, error) { + // Check if the description is not set and if it's not, set it to an empty string. + if _, err := dyn.Get(vin, "description"); err != nil { + vin, err = dyn.Set(vin, "description", dyn.V("")) + if err != nil { + return vin, err + } + } + // Normalize the output value to the target schema. vout, diags := convert.Normalize(apps.App{}, vin) for _, diag := range diags { diff --git a/bundle/deploy/terraform/tfdyn/convert_app_test.go b/bundle/deploy/terraform/tfdyn/convert_app_test.go index 5dfebdc0ef..be8152cc62 100644 --- a/bundle/deploy/terraform/tfdyn/convert_app_test.go +++ b/bundle/deploy/terraform/tfdyn/convert_app_test.go @@ -96,3 +96,61 @@ func TestConvertApp(t *testing.T) { }, }, out.Permissions["app_my_app"]) } + +func TestConvertAppWithNoDescription(t *testing.T) { + src := resources.App{ + SourceCodePath: "./app", + Config: map[string]any{ + "command": []string{"python", "app.py"}, + }, + App: &apps.App{ + Name: "app_id", + Resources: []apps.AppResource{ + { + Name: "job1", + Job: &apps.AppResourceJob{ + Id: "1234", + Permission: "CAN_MANAGE_RUN", + }, + }, + { + Name: "sql1", + SqlWarehouse: &apps.AppResourceSqlWarehouse{ + Id: "5678", + Permission: "CAN_USE", + }, + }, + }, + }, + } + + vin, err := convert.FromTyped(src, dyn.NilValue) + require.NoError(t, err) + + ctx := context.Background() + out := schema.NewResources() + err = appConverter{}.Convert(ctx, "my_app", vin, out) + require.NoError(t, err) + + app := out.App["my_app"] + assert.Equal(t, map[string]any{ + "name": "app_id", + "description": "", // Due to Apps API always returning a description field, we set it in the output as well to avoid permanent TF drift + "resources": []any{ + map[string]any{ + "name": "job1", + "job": map[string]any{ + "id": "1234", + "permission": "CAN_MANAGE_RUN", + }, + }, + map[string]any{ + "name": "sql1", + "sql_warehouse": map[string]any{ + "id": "5678", + "permission": "CAN_USE", + }, + }, + }, + }, app) +}