diff --git a/NEXT_CHANGELOG.md b/NEXT_CHANGELOG.md index 95acdbc46d..3537d370ac 100644 --- a/NEXT_CHANGELOG.md +++ b/NEXT_CHANGELOG.md @@ -9,6 +9,7 @@ ### CLI ### Bundles +* Do not use app config section in test templates and generated app configuration ([#2599](https://github.com/databricks/cli/pull/2599)) * Do not exit early when checking incompatible tasks for specified DBR ([#2692](https://github.com/databricks/cli/pull/2692)) * Removed include/exclude flags support from bundle sync command ([#2718](https://github.com/databricks/cli/pull/2718)) 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..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 { @@ -62,18 +54,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 +78,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 @@ -116,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 -} diff --git a/integration/bundle/apps_test.go b/integration/bundle/apps_test.go index 12bd2fcbfb..8e3fab68aa 100644 --- a/integration/bundle/apps_test.go +++ b/integration/bundle/apps_test.go @@ -87,36 +87,17 @@ 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: + // Replace windows line endings with unix line endings + content = testutil.ReplaceWindowsLineEndings(content) + 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! 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") +}