From 9d9f63a874437f64cd5620bf8ece00f33aac8137 Mon Sep 17 00:00:00 2001 From: Lennart Kats Date: Wed, 4 Nov 2026 14:25:20 +0000 Subject: [PATCH 1/2] bundle: merge target task overrides with complex variable tasks --- .../variables/issue_2436/databricks.yml | 6 +- .../bundle/variables/issue_2436/output.txt | 121 +++--------------- acceptance/bundle/variables/issue_2436/script | 2 +- .../bundle/variables/issue_2436/test.toml | 2 +- bundle/config/mutator/select_target_test.go | 97 ++++++++++++++ bundle/config/mutator/set_variables.go | 4 + bundle/config/root.go | 89 ++++++++++++- bundle/config/root_test.go | 99 ++++++++++++++ 8 files changed, 310 insertions(+), 110 deletions(-) diff --git a/acceptance/bundle/variables/issue_2436/databricks.yml b/acceptance/bundle/variables/issue_2436/databricks.yml index 45fc94eb53c..4de4dd56e31 100644 --- a/acceptance/bundle/variables/issue_2436/databricks.yml +++ b/acceptance/bundle/variables/issue_2436/databricks.yml @@ -9,14 +9,14 @@ variables: default: - task_key: Task_One spark_python_task: - python_file: ./main_dummy.py + python_file: ./databricks.yml job_cluster_key: Small - task_key: Task_Two depends_on: - task_key: Task_One run_if: "ALL_SUCCESS" spark_python_task: - python_file: ./main_dummy.py + python_file: ./databricks.yml job_cluster_key: Small resources: @@ -30,8 +30,6 @@ targets: dev: mode: development default: true - workspace: - host: https://... resources: jobs: job_one: diff --git a/acceptance/bundle/variables/issue_2436/output.txt b/acceptance/bundle/variables/issue_2436/output.txt index cd4c8e61f62..ef1dfc0fdd7 100644 --- a/acceptance/bundle/variables/issue_2436/output.txt +++ b/acceptance/bundle/variables/issue_2436/output.txt @@ -1,107 +1,26 @@ -Warning: Ignoring Databricks CLI version constraint for development build. Required: >= 0.229.0, current: [CLI_VERSION] -Error: failed to perform target override for target=dev: failed to merge target=dev field=resources: cannot merge string with sequence +>>> [CLI] bundle validate -o json +Warning: Ignoring Databricks CLI version constraint for development build. Required: >= 0.229.0, current: [CLI_VERSION] { - "bundle": { - "databricks_cli_version": "\u003e= 0.229.0", - "name": "testing-bundle" - }, - "resources": { - "jobs": { - "job_one": { - "tasks": "${var.job_tasks}" - }, - "job_two": { - "tasks": "${var.job_tasks}" - } + "job_one": [ + { + "task_key": "Task_One", + "job_cluster_key": "Medium" + }, + { + "task_key": "Task_Two", + "job_cluster_key": "Small" } - }, - "targets": { - "dev": { - "default": true, - "mode": "development", - "resources": { - "jobs": { - "job_one": { - "job_clusters": [ - { - "job_cluster_key": "Small", - "new_cluster": { - "data_security_mode": "USER_ISOLATION", - "node_type_id": "Standard_DS3_v2", - "num_workers": 1, - "runtime_engine": "STANDARD", - "spark_version": "15.4.x-scala2.12" - } - }, - { - "job_cluster_key": "Medium", - "new_cluster": { - "data_security_mode": "USER_ISOLATION", - "node_type_id": "Standard_DS5_v2", - "num_workers": 1, - "runtime_engine": "STANDARD", - "spark_version": "14.3.x-scala2.12" - } - } - ], - "tasks": [ - { - "job_cluster_key": "Medium", - "task_key": "Task_One" - } - ] - }, - "job_two": { - "job_clusters": [ - { - "job_cluster_key": "Small", - "new_cluster": { - "data_security_mode": "USER_ISOLATION", - "node_type_id": "Standard_DS3_v2", - "num_workers": 2, - "runtime_engine": "STANDARD", - "spark_version": "15.4.x-scala2.12" - } - } - ] - } - } - }, - "workspace": { - "host": "https://..." - } + ], + "job_two": [ + { + "task_key": "Task_One", + "job_cluster_key": "Small" + }, + { + "task_key": "Task_Two", + "job_cluster_key": "Small" } - }, - "variables": { - "job_tasks": { - "default": [ - { - "job_cluster_key": "Small", - "spark_python_task": { - "python_file": "./main_dummy.py" - }, - "task_key": "Task_One" - }, - { - "depends_on": [ - { - "task_key": "Task_One" - } - ], - "job_cluster_key": "Small", - "run_if": "ALL_SUCCESS", - "spark_python_task": { - "python_file": "./main_dummy.py" - }, - "task_key": "Task_Two" - } - ], - "description": "Common tasks", - "type": "complex" - } - } + ] } - -Exit code: 1 diff --git a/acceptance/bundle/variables/issue_2436/script b/acceptance/bundle/variables/issue_2436/script index c1b368c6ec5..dc2fc5ccd4b 100644 --- a/acceptance/bundle/variables/issue_2436/script +++ b/acceptance/bundle/variables/issue_2436/script @@ -1 +1 @@ -$CLI bundle validate -o json +trace $CLI bundle validate -o json | jq '{job_one: [.resources.jobs.job_one.tasks[] | {task_key, job_cluster_key}], job_two: [.resources.jobs.job_two.tasks[] | {task_key, job_cluster_key}]}' diff --git a/acceptance/bundle/variables/issue_2436/test.toml b/acceptance/bundle/variables/issue_2436/test.toml index e2a93c2b2a4..18b1a88417e 100644 --- a/acceptance/bundle/variables/issue_2436/test.toml +++ b/acceptance/bundle/variables/issue_2436/test.toml @@ -1 +1 @@ -Badness = "Fails to merge resources because targets are resolved before variables" +Cloud = false diff --git a/bundle/config/mutator/select_target_test.go b/bundle/config/mutator/select_target_test.go index 14cec05d9b7..66e662b3cda 100644 --- a/bundle/config/mutator/select_target_test.go +++ b/bundle/config/mutator/select_target_test.go @@ -1,11 +1,14 @@ package mutator_test import ( + "os" + "path/filepath" "testing" "github.com/databricks/cli/bundle" "github.com/databricks/cli/bundle/config" "github.com/databricks/cli/bundle/config/mutator" + "github.com/databricks/cli/bundle/config/mutator/resourcemutator" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" ) @@ -41,3 +44,97 @@ func TestSelectTargetNotFound(t *testing.T) { diags := bundle.Apply(t.Context(), b, mutator.SelectTarget("doesnt-exist")) require.Error(t, diags.Error(), "no targets defined") } + +func TestSelectTargetMergesComplexVariableTaskOverride(t *testing.T) { + root, diags := config.LoadFromBytes("databricks.yml", []byte(` +bundle: + name: testing-bundle +variables: + job_tasks: + type: complex + default: + - task_key: Task_One + job_cluster_key: Small +resources: + jobs: + job_one: + tasks: ${var.job_tasks} + job_two: + tasks: ${var.job_tasks} +targets: + dev: + resources: + jobs: + job_one: + tasks: + - task_key: Task_One + job_cluster_key: Medium +`)) + require.NoError(t, diags.Error()) + + dir := t.TempDir() + path := filepath.Join(dir, ".databricks", "bundle", "dev", "variable-overrides.json") + require.NoError(t, os.MkdirAll(filepath.Dir(path), 0o755)) + require.NoError(t, os.WriteFile(path, []byte(`{"job_tasks":[{"task_key":"Task_One","job_cluster_key":"Small"},{"task_key":"Task_Two","job_cluster_key":"Small"},{"task_key":"Task_Three","job_cluster_key":"Small"}]}`), 0o600)) + + b := &bundle.Bundle{BundleRootPath: dir, Config: *root} + diags = bundle.ApplySeq(t.Context(), b, + mutator.SelectTarget("dev"), + mutator.SetVariables(), + mutator.ResolveVariableReferencesOnlyResources(), + resourcemutator.MergeJobTasks(), + ) + require.NoError(t, diags.Error()) + + jobOne := b.Config.Resources.Jobs["job_one"].Tasks + require.Len(t, jobOne, 3) + assert.Equal(t, "Task_One", jobOne[0].TaskKey) + assert.Equal(t, "Medium", jobOne[0].JobClusterKey) + assert.ElementsMatch(t, []string{"Task_Two", "Task_Three"}, []string{jobOne[1].TaskKey, jobOne[2].TaskKey}) + + jobTwo := b.Config.Resources.Jobs["job_two"].Tasks + require.Len(t, jobTwo, 3) + assert.Equal(t, "Small", jobTwo[0].JobClusterKey) +} + +func TestSelectTargetRewritesComplexVariableTaskOverridePath(t *testing.T) { + root, diags := config.LoadFromBytes("databricks.yml", []byte(` +bundle: + name: testing-bundle +workspace: + file_path: /Workspace/files +variables: + job_tasks: + type: complex + default: + - task_key: Task_One + notebook_task: + notebook_path: /base +resources: + jobs: + job_one: + tasks: ${var.job_tasks} +targets: + dev: + resources: + jobs: + job_one: + tasks: + - task_key: Task_One + notebook_task: + notebook_path: /Workspace${workspace.file_path}/nb +`)) + require.NoError(t, diags.Error()) + + b := &bundle.Bundle{BundleRootPath: t.TempDir(), Config: *root} + diags = bundle.ApplySeq(t.Context(), b, + mutator.SelectTarget("dev"), + mutator.RewriteWorkspacePrefix(), + mutator.SetVariables(), + mutator.ResolveVariableReferencesOnlyResources(), + resourcemutator.MergeJobTasks(), + ) + require.NoError(t, diags.Error()) + require.Len(t, b.Config.Resources.Jobs["job_one"].Tasks, 1) + assert.Equal(t, "/Workspace/files/nb", b.Config.Resources.Jobs["job_one"].Tasks[0].NotebookTask.NotebookPath) +} diff --git a/bundle/config/mutator/set_variables.go b/bundle/config/mutator/set_variables.go index f8f3b35394c..e50cadb4a67 100644 --- a/bundle/config/mutator/set_variables.go +++ b/bundle/config/mutator/set_variables.go @@ -140,6 +140,10 @@ func (m *setVariables) Apply(ctx context.Context, b *bundle.Bundle) diag.Diagnos return setVariable(ctx, variable, v, name, fileDefault) })) }) + if err != nil { + return diags.Extend(diag.FromErr(err)) + } + err = b.Config.MergeDeferredTaskOverrides() return diags.Extend(diag.FromErr(err)) } diff --git a/bundle/config/root.go b/bundle/config/root.go index a64b742630e..c09da88d2bb 100644 --- a/bundle/config/root.go +++ b/bundle/config/root.go @@ -15,6 +15,7 @@ import ( "github.com/databricks/cli/libs/dyn" "github.com/databricks/cli/libs/dyn/convert" "github.com/databricks/cli/libs/dyn/dynloc" + "github.com/databricks/cli/libs/dyn/dynvar" "github.com/databricks/cli/libs/dyn/merge" "github.com/databricks/cli/libs/dyn/yamlloader" "github.com/databricks/cli/libs/log" @@ -35,8 +36,9 @@ type Script struct { } type Root struct { //nolint:recvcheck // value receivers for read-only accessors, pointer for mutators - value dyn.Value - depth int + value dyn.Value + depth int + pendingTaskOverrides []taskVariableOverride // Contains user defined variables Variables map[string]*variable.Variable `json:"variables,omitempty"` @@ -163,9 +165,11 @@ func (r *Root) updateWithDynamicValue(nv dyn.Value) error { // Hack: restore state; it may be cleared by [ToTyped] if // the configuration equals nil (happens in tests). depth := r.depth + pending := r.pendingTaskOverrides defer func() { r.depth = depth + r.pendingTaskOverrides = pending }() // Convert normalized configuration tree to typed configuration. @@ -302,6 +306,12 @@ func (r *Root) Merge(other *Root) error { }) } +type taskVariableOverride struct { + jobName string + variableName string + reference dyn.Value +} + func mergeField(rv, ov dyn.Value, name string) (dyn.Value, error) { path := dyn.NewPath(dyn.Key(name)) reference, _ := dyn.GetByPath(rv, path) @@ -326,6 +336,70 @@ func mergeField(rv, ov dyn.Value, name string) (dyn.Value, error) { return dyn.SetByPath(rv, path, out) } +func deferTaskVariableOverrides(root, target dyn.Value) (dyn.Value, []taskVariableOverride, error) { + var pending []taskVariableOverride + _, err := dyn.Map(target, "resources.jobs", dyn.Foreach(func(path dyn.Path, job dyn.Value) (dyn.Value, error) { + jobName := path[2].Key() + reference := root.Get("resources").Get("jobs").Get(jobName).Get("tasks") + override := job.Get("tasks") + if reference.Kind() != dyn.KindString || override.Kind() != dyn.KindSequence { + return job, nil + } + + refPath, ok := dynvar.PureReferenceToPath(reference.MustString()) + if !ok || len(refPath) != 2 || refPath[0].Key() != "var" { + return job, nil + } + + variableName := refPath[1].Key() + variableType := root.Get("variables").Get(variableName).Get("type") + if variableType.Kind() != dyn.KindString || variableType.MustString() != string(variable.VariableTypeComplex) { + return job, nil + } + + updatedRoot, err := dyn.SetByPath(root, dyn.NewPath(dyn.Key("resources"), dyn.Key("jobs"), dyn.Key(jobName), dyn.Key("tasks")), dyn.NewValue(nil, reference.Locations())) + if err != nil { + return dyn.InvalidValue, err + } + root = updatedRoot + pending = append(pending, taskVariableOverride{jobName, variableName, reference}) + return job, nil + })) + return root, pending, err +} + +// MergeDeferredTaskOverrides merges target task overrides after complex variables are assigned. +func (r *Root) MergeDeferredTaskOverrides() error { + if len(r.pendingTaskOverrides) == 0 { + return nil + } + + root := r.value + for _, pending := range r.pendingTaskOverrides { + value := root.Get("variables").Get(pending.variableName).Get("value") + if value.Kind() != dyn.KindSequence { + return fmt.Errorf("resources.jobs.%s.tasks at %s: variable %s must resolve to a task sequence", pending.jobName, pending.reference.Location(), pending.variableName) + } + + base := dyn.NewValue(value.Value(), pending.reference.Locations()) + override := root.Get("resources").Get("jobs").Get(pending.jobName).Get("tasks") + tasks, err := merge.Merge(base, override) + if err != nil { + return fmt.Errorf("failed to merge resources.jobs.%s.tasks at %s: %w", pending.jobName, pending.reference.Location(), err) + } + root, err = dyn.SetByPath(root, dyn.NewPath(dyn.Key("resources"), dyn.Key("jobs"), dyn.Key(pending.jobName), dyn.Key("tasks")), tasks) + if err != nil { + return fmt.Errorf("failed to set resources.jobs.%s.tasks: %w", pending.jobName, err) + } + } + + if err := r.updateWithDynamicValue(root); err != nil { + return err + } + r.pendingTaskOverrides = nil + return nil +} + func (r *Root) MergeTargetOverrides(name string) error { root := r.value target, err := dyn.GetByPath(root, dyn.NewPath(dyn.Key("targets"), dyn.Key(name))) @@ -339,6 +413,11 @@ func (r *Root) MergeTargetOverrides(name string) error { return err } + root, pending, err := deferTaskVariableOverrides(root, target) + if err != nil { + return fmt.Errorf("failed to defer task overrides for target=%s: %w", name, err) + } + // Merge fields that can be merged 1:1. for _, f := range []string{ "bundle", @@ -447,7 +526,11 @@ func (r *Root) MergeTargetOverrides(name string) error { } // Convert normalized configuration tree to typed configuration. - return r.updateWithDynamicValue(root) + if err := r.updateWithDynamicValue(root); err != nil { + return err + } + r.pendingTaskOverrides = pending + return nil } var allowedVariableDefinitions = []([]string){ diff --git a/bundle/config/root_test.go b/bundle/config/root_test.go index f735c44405e..e549da23d8c 100644 --- a/bundle/config/root_test.go +++ b/bundle/config/root_test.go @@ -105,6 +105,105 @@ func TestRootMergeTargetOverridesWithMode(t *testing.T) { assert.Equal(t, Development, root.Bundle.Mode) } +func TestRootMergeTargetOverridesWithComplexTaskVariable(t *testing.T) { + root, diags := LoadFromBytes("databricks.yml", []byte(` +bundle: + name: testing-bundle +variables: + job_tasks: + type: complex + default: + - task_key: Task_One + job_cluster_key: Small + - task_key: Task_Two + job_cluster_key: Small +resources: + jobs: + job_one: + tasks: ${var.job_tasks} + job_two: + tasks: ${var.job_tasks} +targets: + dev: + resources: + jobs: + job_one: + tasks: + - task_key: Task_One + job_cluster_key: Medium +`)) + require.NoError(t, diags.Error()) + require.NoError(t, root.MergeTargetOverrides("dev")) + + tasks, err := dyn.Get(root.Value(), "resources.jobs.job_one.tasks") + require.NoError(t, err) + require.Len(t, tasks.MustSequence(), 1) + assert.Equal(t, "Medium", tasks.Index(0).Get("job_cluster_key").MustString()) + + defaultTasks, err := dyn.Get(root.Value(), "variables.job_tasks.default") + require.NoError(t, err) + variableTasks := append([]dyn.Value{}, defaultTasks.MustSequence()...) + variableTasks = append(variableTasks, dyn.V(map[string]dyn.Value{"task_key": dyn.V("Task_Three")})) + require.NoError(t, root.Mutate(func(v dyn.Value) (dyn.Value, error) { + return dyn.SetByPath(v, dyn.MustPathFromString("variables.job_tasks.value"), dyn.V(variableTasks)) + })) + require.NoError(t, root.MergeDeferredTaskOverrides()) + + tasks, err = dyn.Get(root.Value(), "resources.jobs.job_one.tasks") + require.NoError(t, err) + require.Len(t, tasks.MustSequence(), 4) + assert.Equal(t, "Task_One", tasks.Index(0).Get("task_key").MustString()) + assert.Equal(t, "Task_Two", tasks.Index(1).Get("task_key").MustString()) + assert.Equal(t, "Task_Three", tasks.Index(2).Get("task_key").MustString()) + assert.Equal(t, "Medium", tasks.Index(3).Get("job_cluster_key").MustString()) + + sharedTasks, err := dyn.Get(root.Value(), "resources.jobs.job_two.tasks") + require.NoError(t, err) + assert.Equal(t, "${var.job_tasks}", sharedTasks.MustString()) +} + +func TestRootMergeTargetOverridesWithTargetComplexTaskVariable(t *testing.T) { + root, diags := LoadFromBytes("databricks.yml", []byte(` +bundle: + name: testing-bundle +variables: + job_tasks: + type: complex + default: + - task_key: Task_One +resources: + jobs: + job_one: + tasks: ${var.job_tasks} +targets: + dev: + variables: + job_tasks: + - task_key: Task_One + - task_key: Task_Two + resources: + jobs: + job_one: + tasks: + - task_key: Task_One + job_cluster_key: Medium +`)) + require.NoError(t, diags.Error()) + require.NoError(t, root.MergeTargetOverrides("dev")) + + defaultTasks, err := dyn.Get(root.Value(), "variables.job_tasks.default") + require.NoError(t, err) + require.NoError(t, root.Mutate(func(v dyn.Value) (dyn.Value, error) { + return dyn.SetByPath(v, dyn.MustPathFromString("variables.job_tasks.value"), defaultTasks) + })) + require.NoError(t, root.MergeDeferredTaskOverrides()) + + tasks, err := dyn.Get(root.Value(), "resources.jobs.job_one.tasks") + require.NoError(t, err) + require.Len(t, tasks.MustSequence(), 3) + assert.Equal(t, "Task_Two", tasks.Index(1).Get("task_key").MustString()) +} + func TestInitializeComplexVariablesViaFlagIsNotAllowed(t *testing.T) { root := &Root{ Variables: map[string]*variable.Variable{ From 98256b3443ca6ac3c115e0b74ea0eae8ca64a331 Mon Sep 17 00:00:00 2001 From: Lennart Kats Date: Thu, 5 Nov 2026 13:58:29 +0000 Subject: [PATCH 2/2] bundle: add release note for task variable overrides --- .nextchanges/bundles/complex-variable-task-overrides.md | 1 + 1 file changed, 1 insertion(+) create mode 100644 .nextchanges/bundles/complex-variable-task-overrides.md diff --git a/.nextchanges/bundles/complex-variable-task-overrides.md b/.nextchanges/bundles/complex-variable-task-overrides.md new file mode 100644 index 00000000000..2014f9d82e2 --- /dev/null +++ b/.nextchanges/bundles/complex-variable-task-overrides.md @@ -0,0 +1 @@ +* Fix target task overrides for jobs whose tasks come from a complex variable while preserving other tasks. ([#7347](https://github.com/databricks/cli/pull/7347))