diff --git a/NEXT_CHANGELOG.md b/NEXT_CHANGELOG.md index 0e4d6de08f..9344db5523 100644 --- a/NEXT_CHANGELOG.md +++ b/NEXT_CHANGELOG.md @@ -9,6 +9,7 @@ * Added `--limit` flag to all paginated list commands for client-side result capping ([#4984](https://github.com/databricks/cli/pull/4984)). ### Bundles +* engine/direct: Fix phantom diffs from `depends_on` reordering in job tasks ([#4990](https://github.com/databricks/cli/pull/4990)) ### Dependency updates diff --git a/acceptance/bundle/invariant/configs/job_with_depends_on.yml.tmpl b/acceptance/bundle/invariant/configs/job_with_depends_on.yml.tmpl new file mode 100644 index 0000000000..8a460d1d90 --- /dev/null +++ b/acceptance/bundle/invariant/configs/job_with_depends_on.yml.tmpl @@ -0,0 +1,22 @@ +bundle: + name: test-bundle-$UNIQUE_NAME + +resources: + jobs: + foo: + name: test-job-$UNIQUE_NAME + tasks: + - task_key: main + notebook_task: + notebook_path: /Shared/notebook + - task_key: process + depends_on: + - task_key: main + notebook_task: + notebook_path: /Shared/notebook + - task_key: finalize + depends_on: + - task_key: process + - task_key: main + notebook_task: + notebook_path: /Shared/notebook diff --git a/acceptance/bundle/invariant/continue_293/out.test.toml b/acceptance/bundle/invariant/continue_293/out.test.toml index 7abd75f42e..18de4aecc1 100644 --- a/acceptance/bundle/invariant/continue_293/out.test.toml +++ b/acceptance/bundle/invariant/continue_293/out.test.toml @@ -4,4 +4,4 @@ RequiresUnityCatalog = true [EnvMatrix] DATABRICKS_BUNDLE_ENGINE = ["direct"] - INPUT_CONFIG = ["alert.yml.tmpl", "app.yml.tmpl", "catalog.yml.tmpl", "cluster.yml.tmpl", "dashboard.yml.tmpl", "database_catalog.yml.tmpl", "database_instance.yml.tmpl", "experiment.yml.tmpl", "external_location.yml.tmpl", "job.yml.tmpl", "job_pydabs_10_tasks.yml.tmpl", "job_pydabs_1000_tasks.yml.tmpl", "job_cross_resource_ref.yml.tmpl", "job_permission_ref.yml.tmpl", "job_with_permissions.yml.tmpl", "job_with_task.yml.tmpl", "model.yml.tmpl", "model_with_permissions.yml.tmpl", "model_serving_endpoint.yml.tmpl", "pipeline.yml.tmpl", "pipeline_config_dots.yml.tmpl", "postgres_branch.yml.tmpl", "postgres_endpoint.yml.tmpl", "postgres_project.yml.tmpl", "registered_model.yml.tmpl", "schema.yml.tmpl", "schema_grant_ref.yml.tmpl", "schema_with_grants.yml.tmpl", "secret_scope.yml.tmpl", "secret_scope_default_backend_type.yml.tmpl", "secret_scope_with_permissions.yml.tmpl", "synced_database_table.yml.tmpl", "volume.yml.tmpl"] + INPUT_CONFIG = ["alert.yml.tmpl", "app.yml.tmpl", "catalog.yml.tmpl", "cluster.yml.tmpl", "dashboard.yml.tmpl", "database_catalog.yml.tmpl", "database_instance.yml.tmpl", "experiment.yml.tmpl", "external_location.yml.tmpl", "job.yml.tmpl", "job_pydabs_10_tasks.yml.tmpl", "job_pydabs_1000_tasks.yml.tmpl", "job_cross_resource_ref.yml.tmpl", "job_permission_ref.yml.tmpl", "job_with_depends_on.yml.tmpl", "job_with_permissions.yml.tmpl", "job_with_task.yml.tmpl", "model.yml.tmpl", "model_with_permissions.yml.tmpl", "model_serving_endpoint.yml.tmpl", "pipeline.yml.tmpl", "pipeline_config_dots.yml.tmpl", "postgres_branch.yml.tmpl", "postgres_endpoint.yml.tmpl", "postgres_project.yml.tmpl", "registered_model.yml.tmpl", "schema.yml.tmpl", "schema_grant_ref.yml.tmpl", "schema_with_grants.yml.tmpl", "secret_scope.yml.tmpl", "secret_scope_default_backend_type.yml.tmpl", "secret_scope_with_permissions.yml.tmpl", "synced_database_table.yml.tmpl", "volume.yml.tmpl"] diff --git a/acceptance/bundle/invariant/migrate/out.test.toml b/acceptance/bundle/invariant/migrate/out.test.toml index 7abd75f42e..18de4aecc1 100644 --- a/acceptance/bundle/invariant/migrate/out.test.toml +++ b/acceptance/bundle/invariant/migrate/out.test.toml @@ -4,4 +4,4 @@ RequiresUnityCatalog = true [EnvMatrix] DATABRICKS_BUNDLE_ENGINE = ["direct"] - INPUT_CONFIG = ["alert.yml.tmpl", "app.yml.tmpl", "catalog.yml.tmpl", "cluster.yml.tmpl", "dashboard.yml.tmpl", "database_catalog.yml.tmpl", "database_instance.yml.tmpl", "experiment.yml.tmpl", "external_location.yml.tmpl", "job.yml.tmpl", "job_pydabs_10_tasks.yml.tmpl", "job_pydabs_1000_tasks.yml.tmpl", "job_cross_resource_ref.yml.tmpl", "job_permission_ref.yml.tmpl", "job_with_permissions.yml.tmpl", "job_with_task.yml.tmpl", "model.yml.tmpl", "model_with_permissions.yml.tmpl", "model_serving_endpoint.yml.tmpl", "pipeline.yml.tmpl", "pipeline_config_dots.yml.tmpl", "postgres_branch.yml.tmpl", "postgres_endpoint.yml.tmpl", "postgres_project.yml.tmpl", "registered_model.yml.tmpl", "schema.yml.tmpl", "schema_grant_ref.yml.tmpl", "schema_with_grants.yml.tmpl", "secret_scope.yml.tmpl", "secret_scope_default_backend_type.yml.tmpl", "secret_scope_with_permissions.yml.tmpl", "synced_database_table.yml.tmpl", "volume.yml.tmpl"] + INPUT_CONFIG = ["alert.yml.tmpl", "app.yml.tmpl", "catalog.yml.tmpl", "cluster.yml.tmpl", "dashboard.yml.tmpl", "database_catalog.yml.tmpl", "database_instance.yml.tmpl", "experiment.yml.tmpl", "external_location.yml.tmpl", "job.yml.tmpl", "job_pydabs_10_tasks.yml.tmpl", "job_pydabs_1000_tasks.yml.tmpl", "job_cross_resource_ref.yml.tmpl", "job_permission_ref.yml.tmpl", "job_with_depends_on.yml.tmpl", "job_with_permissions.yml.tmpl", "job_with_task.yml.tmpl", "model.yml.tmpl", "model_with_permissions.yml.tmpl", "model_serving_endpoint.yml.tmpl", "pipeline.yml.tmpl", "pipeline_config_dots.yml.tmpl", "postgres_branch.yml.tmpl", "postgres_endpoint.yml.tmpl", "postgres_project.yml.tmpl", "registered_model.yml.tmpl", "schema.yml.tmpl", "schema_grant_ref.yml.tmpl", "schema_with_grants.yml.tmpl", "secret_scope.yml.tmpl", "secret_scope_default_backend_type.yml.tmpl", "secret_scope_with_permissions.yml.tmpl", "synced_database_table.yml.tmpl", "volume.yml.tmpl"] diff --git a/acceptance/bundle/invariant/migrate/script b/acceptance/bundle/invariant/migrate/script index d02200cb53..78f45faa7d 100644 --- a/acceptance/bundle/invariant/migrate/script +++ b/acceptance/bundle/invariant/migrate/script @@ -34,7 +34,17 @@ cat LOG.deploy | contains.py '!panic:' '!internal error' > /dev/null echo INPUT_CONFIG_OK -trace $CLI bundle deployment migrate &> LOG.migrate +MIGRATE_ARGS="" +# The terraform provider sorts depends_on entries alphabetically by task_key on Read +# (see terraform-provider-databricks PR #3000). Since depends_on uses TypeList +# (order-sensitive), terraform plan reports positional drift when the bundle config +# specifies depends_on in a different order than the provider's sorted state. +# This is a false positive -- the logical dependencies are identical. +if [[ "$INPUT_CONFIG" == "job_with_depends_on.yml.tmpl" ]]; then + MIGRATE_ARGS="--noplancheck" +fi + +trace $CLI bundle deployment migrate $MIGRATE_ARGS &> LOG.migrate cat LOG.migrate | contains.py '!panic:' '!internal error' > /dev/null diff --git a/acceptance/bundle/invariant/no_drift/out.test.toml b/acceptance/bundle/invariant/no_drift/out.test.toml index 7abd75f42e..18de4aecc1 100644 --- a/acceptance/bundle/invariant/no_drift/out.test.toml +++ b/acceptance/bundle/invariant/no_drift/out.test.toml @@ -4,4 +4,4 @@ RequiresUnityCatalog = true [EnvMatrix] DATABRICKS_BUNDLE_ENGINE = ["direct"] - INPUT_CONFIG = ["alert.yml.tmpl", "app.yml.tmpl", "catalog.yml.tmpl", "cluster.yml.tmpl", "dashboard.yml.tmpl", "database_catalog.yml.tmpl", "database_instance.yml.tmpl", "experiment.yml.tmpl", "external_location.yml.tmpl", "job.yml.tmpl", "job_pydabs_10_tasks.yml.tmpl", "job_pydabs_1000_tasks.yml.tmpl", "job_cross_resource_ref.yml.tmpl", "job_permission_ref.yml.tmpl", "job_with_permissions.yml.tmpl", "job_with_task.yml.tmpl", "model.yml.tmpl", "model_with_permissions.yml.tmpl", "model_serving_endpoint.yml.tmpl", "pipeline.yml.tmpl", "pipeline_config_dots.yml.tmpl", "postgres_branch.yml.tmpl", "postgres_endpoint.yml.tmpl", "postgres_project.yml.tmpl", "registered_model.yml.tmpl", "schema.yml.tmpl", "schema_grant_ref.yml.tmpl", "schema_with_grants.yml.tmpl", "secret_scope.yml.tmpl", "secret_scope_default_backend_type.yml.tmpl", "secret_scope_with_permissions.yml.tmpl", "synced_database_table.yml.tmpl", "volume.yml.tmpl"] + INPUT_CONFIG = ["alert.yml.tmpl", "app.yml.tmpl", "catalog.yml.tmpl", "cluster.yml.tmpl", "dashboard.yml.tmpl", "database_catalog.yml.tmpl", "database_instance.yml.tmpl", "experiment.yml.tmpl", "external_location.yml.tmpl", "job.yml.tmpl", "job_pydabs_10_tasks.yml.tmpl", "job_pydabs_1000_tasks.yml.tmpl", "job_cross_resource_ref.yml.tmpl", "job_permission_ref.yml.tmpl", "job_with_depends_on.yml.tmpl", "job_with_permissions.yml.tmpl", "job_with_task.yml.tmpl", "model.yml.tmpl", "model_with_permissions.yml.tmpl", "model_serving_endpoint.yml.tmpl", "pipeline.yml.tmpl", "pipeline_config_dots.yml.tmpl", "postgres_branch.yml.tmpl", "postgres_endpoint.yml.tmpl", "postgres_project.yml.tmpl", "registered_model.yml.tmpl", "schema.yml.tmpl", "schema_grant_ref.yml.tmpl", "schema_with_grants.yml.tmpl", "secret_scope.yml.tmpl", "secret_scope_default_backend_type.yml.tmpl", "secret_scope_with_permissions.yml.tmpl", "synced_database_table.yml.tmpl", "volume.yml.tmpl"] diff --git a/acceptance/bundle/invariant/test.toml b/acceptance/bundle/invariant/test.toml index 85b2defc92..2924a6062c 100644 --- a/acceptance/bundle/invariant/test.toml +++ b/acceptance/bundle/invariant/test.toml @@ -35,6 +35,7 @@ EnvMatrix.INPUT_CONFIG = [ "job_pydabs_1000_tasks.yml.tmpl", "job_cross_resource_ref.yml.tmpl", "job_permission_ref.yml.tmpl", + "job_with_depends_on.yml.tmpl", "job_with_permissions.yml.tmpl", "job_with_task.yml.tmpl", "model.yml.tmpl", diff --git a/bundle/direct/dresources/job.go b/bundle/direct/dresources/job.go index f10813b2fc..9477bf5251 100644 --- a/bundle/direct/dresources/job.go +++ b/bundle/direct/dresources/job.go @@ -71,12 +71,18 @@ func getEnvironmentKey(x jobs.JobEnvironment) (string, string) { return "environment_key", x.EnvironmentKey } +func getDependsOnTaskKey(x jobs.TaskDependency) (string, string) { + return "task_key", x.TaskKey +} + func (*ResourceJob) KeyedSlices() map[string]any { return map[string]any{ - "tasks": getTaskKey, - "parameters": getParameterName, - "job_clusters": getJobClusterKey, - "environments": getEnvironmentKey, + "tasks": getTaskKey, + "parameters": getParameterName, + "job_clusters": getJobClusterKey, + "environments": getEnvironmentKey, + "tasks[*].depends_on": getDependsOnTaskKey, + "tasks[*].for_each_task.task.depends_on": getDependsOnTaskKey, } } diff --git a/libs/structs/structdiff/diff.go b/libs/structs/structdiff/diff.go index 61c909dfd1..a852d347e4 100644 --- a/libs/structs/structdiff/diff.go +++ b/libs/structs/structdiff/diff.go @@ -308,7 +308,7 @@ func (ctx *diffContext) findKeyFunc(path *structpath.PathNode) KeyFunc { } // pathToPattern converts a PathNode to a pattern string for matching. -// Slice indices are converted to [*] wildcard. +// Slice indices and key-value pairs are converted to [*] wildcard. func pathToPattern(path *structpath.PathNode) string { if path == nil { return "" @@ -318,17 +318,10 @@ func pathToPattern(path *structpath.PathNode) string { var result strings.Builder for i, node := range components { - if idx, ok := node.Index(); ok { - // Convert numeric index to wildcard - _ = idx + if _, ok := node.Index(); ok { + result.WriteString("[*]") + } else if _, _, ok := node.KeyValue(); ok { result.WriteString("[*]") - } else if key, value, ok := node.KeyValue(); ok { - // Key-value syntax - result.WriteString("[") - result.WriteString(key) - result.WriteString("=") - result.WriteString(structpath.EncodeMapKey(value)) - result.WriteString("]") } else if key, ok := node.StringKey(); ok { if i != 0 { result.WriteString(".") diff --git a/libs/structs/structdiff/diff_test.go b/libs/structs/structdiff/diff_test.go index 4b57f87c88..474694d5e6 100644 --- a/libs/structs/structdiff/diff_test.go +++ b/libs/structs/structdiff/diff_test.go @@ -558,10 +558,16 @@ func TestGetStructDiffEmbedTagWithKeyFunc(t *testing.T) { } } +type Dep struct { + TaskKey string `json:"task_key,omitempty"` + Outcome string `json:"outcome,omitempty"` +} + type Task struct { TaskKey string `json:"task_key,omitempty"` Description string `json:"description,omitempty"` Timeout int `json:"timeout,omitempty"` + DependsOn []Dep `json:"depends_on,omitempty"` } type Job struct { @@ -573,6 +579,10 @@ func taskKeyFunc(task Task) (string, string) { return "task_key", task.TaskKey } +func depKeyFunc(dep Dep) (string, string) { + return "task_key", dep.TaskKey +} + func TestGetStructDiffSliceKeys(t *testing.T) { sliceKeys := map[string]KeyFunc{ "tasks": taskKeyFunc, @@ -642,6 +652,64 @@ func TestGetStructDiffSliceKeys(t *testing.T) { } } +func TestGetStructDiffNestedDependsOn(t *testing.T) { + sliceKeys := map[string]KeyFunc{ + "tasks": taskKeyFunc, + "tasks[*].depends_on": depKeyFunc, + } + + tests := []struct { + name string + a, b Job + want []ResolvedChange + }{ + { + name: "depends_on reordered no diff", + a: Job{Tasks: []Task{{TaskKey: "c", DependsOn: []Dep{{TaskKey: "a"}, {TaskKey: "b"}}}}}, + b: Job{Tasks: []Task{{TaskKey: "c", DependsOn: []Dep{{TaskKey: "b"}, {TaskKey: "a"}}}}}, + want: nil, + }, + { + name: "depends_on field change", + a: Job{Tasks: []Task{{TaskKey: "c", DependsOn: []Dep{{TaskKey: "a", Outcome: "success"}}}}}, + b: Job{Tasks: []Task{{TaskKey: "c", DependsOn: []Dep{{TaskKey: "a", Outcome: "failed"}}}}}, + want: []ResolvedChange{{Field: "tasks[task_key='c'].depends_on[task_key='a'].outcome", Old: "success", New: "failed"}}, + }, + { + name: "depends_on element added", + a: Job{Tasks: []Task{{TaskKey: "c", DependsOn: []Dep{{TaskKey: "a"}}}}}, + b: Job{Tasks: []Task{{TaskKey: "c", DependsOn: []Dep{{TaskKey: "a"}, {TaskKey: "b"}}}}}, + want: []ResolvedChange{{Field: "tasks[task_key='c'].depends_on[task_key='b']", Old: nil, New: Dep{TaskKey: "b"}}}, + }, + { + name: "depends_on element removed", + a: Job{Tasks: []Task{{TaskKey: "c", DependsOn: []Dep{{TaskKey: "a"}, {TaskKey: "b"}}}}}, + b: Job{Tasks: []Task{{TaskKey: "c", DependsOn: []Dep{{TaskKey: "a"}}}}}, + want: []ResolvedChange{{Field: "tasks[task_key='c'].depends_on[task_key='b']", Old: Dep{TaskKey: "b"}, New: nil}}, + }, + { + name: "tasks and depends_on both reordered no diff", + a: Job{Tasks: []Task{ + {TaskKey: "x", DependsOn: []Dep{{TaskKey: "a"}, {TaskKey: "b"}}}, + {TaskKey: "y", DependsOn: []Dep{{TaskKey: "c"}}}, + }}, + b: Job{Tasks: []Task{ + {TaskKey: "y", DependsOn: []Dep{{TaskKey: "c"}}}, + {TaskKey: "x", DependsOn: []Dep{{TaskKey: "b"}, {TaskKey: "a"}}}, + }}, + want: nil, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got, err := GetStructDiff(tt.a, tt.b, sliceKeys) + assert.NoError(t, err) + assert.Equal(t, tt.want, resolveChanges(got)) + }) + } +} + type Nested struct { Items []Item `json:"items,omitempty"` } diff --git a/libs/testserver/jobs.go b/libs/testserver/jobs.go index 15800341de..cd9432fac1 100644 --- a/libs/testserver/jobs.go +++ b/libs/testserver/jobs.go @@ -106,6 +106,12 @@ func jobFixUps(jobSettings *jobs.JobSettings) { for i := range jobSettings.Tasks { task := &jobSettings.Tasks[i] + // Sort depends_on by task_key to simulate the real API which returns + // dependencies in a different order than submitted. + slices.SortFunc(task.DependsOn, func(a, b jobs.TaskDependency) int { + return cmp.Compare(a.TaskKey, b.TaskKey) + }) + // Set task email notifications to empty struct if not set if task.EmailNotifications == nil { task.EmailNotifications = &jobs.TaskEmailNotifications{}