From e307013fbffeb4ddc371a0d393e73214136c6fbc Mon Sep 17 00:00:00 2001 From: Andrew Nester Date: Mon, 31 Mar 2025 13:51:49 +0200 Subject: [PATCH 1/4] Do not use app config section in test templates and generated app configuration --- bundle/config/generate/app.go | 11 +-- bundle/tests/apps/databricks.yml | 71 ------------------- bundle/tests/apps_test.go | 58 --------------- cmd/bundle/generate/app.go | 13 +--- integration/bundle/apps_test.go | 25 +------ .../bundle/bundles/apps/template/app/app.yml | 8 +++ .../bundles/apps/template/databricks.yml.tmpl | 10 --- .../bundle/testdata/apps/bundle_deploy.txt | 4 -- .../bundle/testdata/apps/bundle_validate.txt | 6 +- 9 files changed, 13 insertions(+), 193 deletions(-) delete mode 100644 bundle/tests/apps/databricks.yml delete mode 100644 bundle/tests/apps_test.go create mode 100644 integration/bundle/bundles/apps/template/app/app.yml diff --git a/bundle/config/generate/app.go b/bundle/config/generate/app.go index 1255d63f89..b4a37d8d6d 100644 --- a/bundle/config/generate/app.go +++ b/bundle/config/generate/app.go @@ -6,12 +6,7 @@ import ( "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 - } - +func ConvertAppToValue(app *apps.App, sourceCodePath string) (dyn.Value, error) { ar, err := convert.FromTyped(app.Resources, dyn.NilValue) if err != nil { return dyn.NilValue, err @@ -25,10 +20,6 @@ func ConvertAppToValue(app *apps.App, sourceCodePath string, appConfig map[strin "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}}) } diff --git a/bundle/tests/apps/databricks.yml b/bundle/tests/apps/databricks.yml deleted file mode 100644 index ad7e930065..0000000000 --- a/bundle/tests/apps/databricks.yml +++ /dev/null @@ -1,71 +0,0 @@ -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 deleted file mode 100644 index df977ae2a3..0000000000 --- a/bundle/tests/apps_test.go +++ /dev/null @@ -1,58 +0,0 @@ -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.ApplySeq(context.Background(), b, - mutator.SetVariables(), - mutator.ResolveVariableReferencesOnlyResources("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, []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) - 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.ApplySeq(context.Background(), b, - mutator.SetVariables(), - mutator.ResolveVariableReferencesOnlyResources("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, []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) - 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/cmd/bundle/generate/app.go b/cmd/bundle/generate/app.go index bcbbda7e87..333e9446f5 100644 --- a/cmd/bundle/generate/app.go +++ b/cmd/bundle/generate/app.go @@ -62,18 +62,13 @@ func NewGenerateAppCommand() *cobra.Command { 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) + v, err := generate.ConvertAppToValue(app, filepath.ToSlash(rel)) if err != nil { return err } @@ -91,12 +86,6 @@ func NewGenerateAppCommand() *cobra.Command { }), } - // 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 diff --git a/integration/bundle/apps_test.go b/integration/bundle/apps_test.go index 12bd2fcbfb..5696a2aee2 100644 --- a/integration/bundle/apps_test.go +++ b/integration/bundle/apps_test.go @@ -87,36 +87,15 @@ func TestDeployBundleWithApp(t *testing.T) { data, err := io.ReadAll(reader) require.NoError(t, err) - job, err := wt.W.Jobs.GetBySettingsName(ctx, "test-job-with-cluster-"+uniqueId) - require.NoError(t, err) - content := string(data) - require.Contains(t, content, fmt.Sprintf(`command: + require.Contains(t, content, `command: - flask - --app - app - run env: - name: JOB_ID - value: "%d"`, job.JobId)) - - // Redeploy bundle with changed config env for app and confirm it's updated in app.yaml - deployBundleWithArgs(t, ctx, root, `--var="env_var_name=ANOTHER_JOB_ID"`, "--force-lock", "--auto-approve") - reader, err = wt.W.Workspace.Download(ctx, pathToAppYml) - require.NoError(t, err) - - data, err = io.ReadAll(reader) - require.NoError(t, err) - - content = string(data) - require.Contains(t, content, fmt.Sprintf(`command: - - flask - - --app - - app - - run -env: - - name: ANOTHER_JOB_ID - value: "%d"`, job.JobId)) + valueFrom: "app-job"`) if testing.Short() { t.Log("Skip the app run in short mode") diff --git a/integration/bundle/bundles/apps/template/app/app.yml b/integration/bundle/bundles/apps/template/app/app.yml new file mode 100644 index 0000000000..7ae2a2fb12 --- /dev/null +++ b/integration/bundle/bundles/apps/template/app/app.yml @@ -0,0 +1,8 @@ +command: + - flask + - --app + - app + - run +env: + - name: JOB_ID + valueFrom: "app-job" diff --git a/integration/bundle/bundles/apps/template/databricks.yml.tmpl b/integration/bundle/bundles/apps/template/databricks.yml.tmpl index e0937be717..15fa5e599d 100644 --- a/integration/bundle/bundles/apps/template/databricks.yml.tmpl +++ b/integration/bundle/bundles/apps/template/databricks.yml.tmpl @@ -14,16 +14,6 @@ resources: name: "{{.app_id}}" description: "App which manages job created by this bundle" source_code_path: ./app - config: - command: - - flask - - --app - - app - - run - env: - - name: ${var.env_var_name} - value: ${resources.jobs.foo.id} - resources: - name: "app-job" description: "A job for app to be able to work with" diff --git a/integration/bundle/testdata/apps/bundle_deploy.txt b/integration/bundle/testdata/apps/bundle_deploy.txt index 55b8dedc6b..437a555965 100644 --- a/integration/bundle/testdata/apps/bundle_deploy.txt +++ b/integration/bundle/testdata/apps/bundle_deploy.txt @@ -2,7 +2,3 @@ Uploading bundle files to /Workspace/Users/[USERNAME]/.bundle/$UNIQUE_PRJ/files. Deploying resources... Updating deployment state... Deployment complete! -Warning: App config section detected - -remove 'config' from app resource 'test_app' section and use app.yml file in the root of this app instead - diff --git a/integration/bundle/testdata/apps/bundle_validate.txt b/integration/bundle/testdata/apps/bundle_validate.txt index cb37fc4bde..567fafd24b 100644 --- a/integration/bundle/testdata/apps/bundle_validate.txt +++ b/integration/bundle/testdata/apps/bundle_validate.txt @@ -1,11 +1,7 @@ -Warning: App config section detected - -remove 'config' from app resource 'test_app' section and use app.yml file in the root of this app instead - Name: basic Target: default Workspace: User: [USERNAME] Path: /Workspace/Users/[USERNAME]/.bundle/$UNIQUE_PRJ -Found 1 warning +Validation OK! From bfb0d8816870ce9a084547c74ae69e0eab29546a Mon Sep 17 00:00:00 2001 From: Andrew Nester Date: Mon, 31 Mar 2025 14:02:32 +0200 Subject: [PATCH 2/4] remove getAppConfig --- cmd/bundle/generate/app.go | 47 -------------------------------------- 1 file changed, 47 deletions(-) diff --git a/cmd/bundle/generate/app.go b/cmd/bundle/generate/app.go index 333e9446f5..427a01878f 100644 --- a/cmd/bundle/generate/app.go +++ b/cmd/bundle/generate/app.go @@ -1,11 +1,7 @@ package generate import ( - "context" - "errors" "fmt" - "io" - "io/fs" "path/filepath" "github.com/databricks/cli/bundle/config/generate" @@ -13,13 +9,9 @@ import ( "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 { @@ -105,42 +97,3 @@ func NewGenerateAppCommand() *cobra.Command { 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, "Reading app configuration from "+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 -} From 2735570c460200c00f03c8f56fb966b754b1d8c9 Mon Sep 17 00:00:00 2001 From: Andrew Nester Date: Mon, 31 Mar 2025 14:56:16 +0200 Subject: [PATCH 3/4] fix windows tests --- integration/bundle/apps_test.go | 2 ++ internal/testutil/helpers.go | 4 ++++ 2 files changed, 6 insertions(+) diff --git a/integration/bundle/apps_test.go b/integration/bundle/apps_test.go index 5696a2aee2..8e3fab68aa 100644 --- a/integration/bundle/apps_test.go +++ b/integration/bundle/apps_test.go @@ -88,6 +88,8 @@ func TestDeployBundleWithApp(t *testing.T) { require.NoError(t, err) content := string(data) + // Replace windows line endings with unix line endings + content = testutil.ReplaceWindowsLineEndings(content) require.Contains(t, content, `command: - flask - --app diff --git a/internal/testutil/helpers.go b/internal/testutil/helpers.go index 44c2c9375a..6c036344b9 100644 --- a/internal/testutil/helpers.go +++ b/internal/testutil/helpers.go @@ -42,3 +42,7 @@ func SkipUntil(t TestingT, date string) { t.Skipf("Skipping test until %s. Time right now: %s", deadline.Format(time.DateOnly), time.Now()) } } + +func ReplaceWindowsLineEndings(s string) string { + return strings.ReplaceAll(s, "\r\n", "\n") +} From 3a13857118840f47ee0a669e2ad0beb31b640186 Mon Sep 17 00:00:00 2001 From: Andrew Nester Date: Mon, 31 Mar 2025 16:28:41 +0200 Subject: [PATCH 4/4] added next changelog --- NEXT_CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/NEXT_CHANGELOG.md b/NEXT_CHANGELOG.md index 1c129d08cf..2e72c8a57f 100644 --- a/NEXT_CHANGELOG.md +++ b/NEXT_CHANGELOG.md @@ -9,5 +9,6 @@ ### CLI ### Bundles +* Do not use app config section in test templates and generated app configuration ([#2599](https://github.com/databricks/cli/pull/2599)) ### API Changes