From d3728a03b372d4d0a4fa086f4b8fa188f4c2fc0f Mon Sep 17 00:00:00 2001 From: Ilya Kuznetsov Date: Fri, 6 Mar 2026 17:28:22 +0100 Subject: [PATCH 1/5] Use resources.yml for server-side defaults --- .../config_edits/output.txt | 6 +- .../job_multiple_tasks/output.txt | 16 +- .../multiple_files/output.txt | 12 +- bundle/configsync/defaults.go | 181 ++---------------- bundle/configsync/diff.go | 64 +++++-- bundle/configsync/diff_test.go | 90 --------- bundle/direct/bundle_plan.go | 93 +++++---- bundle/direct/dresources/resources.yml | 16 ++ 8 files changed, 142 insertions(+), 336 deletions(-) delete mode 100644 bundle/configsync/diff_test.go diff --git a/acceptance/bundle/config-remote-sync/config_edits/output.txt b/acceptance/bundle/config-remote-sync/config_edits/output.txt index ff88716b3c..2a59c6f563 100644 --- a/acceptance/bundle/config-remote-sync/config_edits/output.txt +++ b/acceptance/bundle/config-remote-sync/config_edits/output.txt @@ -26,6 +26,7 @@ Resource: resources.jobs.my_job email_notifications.on_failure[0]: replace max_concurrent_runs: replace tags['env']: remove + timeout_seconds: remove @@ -41,15 +42,16 @@ Resource: resources.jobs.my_job + - remote-failure@example.com parameters: - name: catalog -@@ -35,7 +35,6 @@ +@@ -35,8 +35,6 @@ unit: DAYS tags: - env: config-production team: data-team - max_concurrent_runs: 3 +- timeout_seconds: 3600 + max_concurrent_runs: 5 - timeout_seconds: 3600 environments: + - environment_key: default >>> [CLI] bundle destroy --auto-approve The following resources will be deleted: diff --git a/acceptance/bundle/config-remote-sync/job_multiple_tasks/output.txt b/acceptance/bundle/config-remote-sync/job_multiple_tasks/output.txt index 0d8b9275a9..5e36f714ce 100644 --- a/acceptance/bundle/config-remote-sync/job_multiple_tasks/output.txt +++ b/acceptance/bundle/config-remote-sync/job_multiple_tasks/output.txt @@ -24,12 +24,11 @@ Resource: resources.jobs.no_tasks_job >>> diff.py databricks.yml.backup databricks.yml --- databricks.yml.backup +++ databricks.yml -@@ -13,13 +13,11 @@ +@@ -13,13 +13,10 @@ node_type_id: [NODE_TYPE_ID] num_workers: 1 - - task_key: d_task + - new_cluster: -+ node_type_id: [NODE_TYPE_ID] + num_workers: 1 + spark_version: 13.3.x-snapshot-scala2.12 notebook_task: @@ -44,7 +43,7 @@ Resource: resources.jobs.no_tasks_job + task_key: e_task - task_key: c_task notebook_task: -@@ -28,7 +26,8 @@ +@@ -28,7 +25,8 @@ spark_version: 13.3.x-snapshot-scala2.12 node_type_id: [NODE_TYPE_ID] - num_workers: 2 @@ -55,14 +54,13 @@ Resource: resources.jobs.no_tasks_job + timeout_seconds: 3600 - task_key: a_task notebook_task: -@@ -41,5 +40,13 @@ +@@ -41,5 +39,12 @@ - task_key: c_task - no_tasks_job: {} + no_tasks_job: + tasks: + - new_cluster: -+ node_type_id: [NODE_TYPE_ID] + num_workers: 1 + spark_version: 13.3.x-snapshot-scala2.12 + notebook_task: @@ -94,12 +92,11 @@ Resource: resources.jobs.rename_task_job >>> diff.py databricks.yml.backup2 databricks.yml --- databricks.yml.backup2 +++ databricks.yml -@@ -52,14 +52,14 @@ +@@ -50,14 +50,13 @@ rename_task_job: tasks: - - task_key: b_task + - new_cluster: -+ node_type_id: [NODE_TYPE_ID] + num_workers: 1 + spark_version: 13.3.x-snapshot-scala2.12 notebook_task: @@ -115,14 +112,14 @@ Resource: resources.jobs.rename_task_job + - task_key: b_task_renamed notebook_task: notebook_path: /Users/{{workspace_user_name}}/d_task -@@ -70,5 +70,5 @@ +@@ -68,5 +67,5 @@ - task_key: c_task depends_on: - - task_key: b_task + - task_key: b_task_renamed notebook_task: notebook_path: /Users/{{workspace_user_name}}/c_task -@@ -79,7 +79,14 @@ +@@ -77,7 +76,13 @@ - task_key: a_task notebook_task: - notebook_path: /Users/{{workspace_user_name}}/a_task @@ -132,7 +129,6 @@ Resource: resources.jobs.rename_task_job node_type_id: [NODE_TYPE_ID] num_workers: 1 + - new_cluster: -+ node_type_id: [NODE_TYPE_ID] + num_workers: 1 + spark_version: 13.3.x-snapshot-scala2.12 + notebook_task: diff --git a/acceptance/bundle/config-remote-sync/multiple_files/output.txt b/acceptance/bundle/config-remote-sync/multiple_files/output.txt index e616ca008c..511f0f7f2e 100644 --- a/acceptance/bundle/config-remote-sync/multiple_files/output.txt +++ b/acceptance/bundle/config-remote-sync/multiple_files/output.txt @@ -31,14 +31,13 @@ Resource: resources.jobs.job_two >>> diff.py resources/job1.yml.backup resources/job1.yml --- resources/job1.yml.backup +++ resources/job1.yml -@@ -4,13 +4,13 @@ +@@ -4,13 +4,12 @@ max_concurrent_runs: 1 tasks: - - task_key: c_task + - depends_on: + - task_key: b_task + new_cluster: -+ node_type_id: [NODE_TYPE_ID] + num_workers: 1 + spark_version: 13.3.x-snapshot-scala2.12 notebook_task: @@ -52,13 +51,12 @@ Resource: resources.jobs.job_two + task_key: c_task_renamed - task_key: a_task notebook_task: -@@ -21,3 +21,10 @@ +@@ -21,3 +20,9 @@ num_workers: 1 depends_on: - - task_key: c_task + - task_key: c_task_renamed + - new_cluster: -+ node_type_id: [NODE_TYPE_ID] + num_workers: 1 + spark_version: 13.3.x-snapshot-scala2.12 + notebook_task: @@ -70,7 +68,7 @@ Resource: resources.jobs.job_two >>> diff.py resources/job2.yml.backup resources/job2.yml --- resources/job2.yml.backup +++ resources/job2.yml -@@ -2,13 +2,13 @@ +@@ -2,13 +2,12 @@ jobs: job_two: - max_concurrent_runs: 2 @@ -78,7 +76,6 @@ Resource: resources.jobs.job_two tasks: - - task_key: run_pipeline + - new_cluster: -+ node_type_id: [NODE_TYPE_ID] + num_workers: 1 + spark_version: 13.3.x-snapshot-scala2.12 notebook_task: @@ -90,12 +87,11 @@ Resource: resources.jobs.job_two + task_key: run_pipeline_renamed - task_key: etl_pipeline notebook_task: -@@ -18,5 +18,9 @@ +@@ -18,5 +17,8 @@ node_type_id: [NODE_TYPE_ID] num_workers: 2 - - task_key: extra_task + - new_cluster: -+ node_type_id: [NODE_TYPE_ID] + num_workers: 1 + spark_version: 13.3.x-snapshot-scala2.12 notebook_task: diff --git a/bundle/configsync/defaults.go b/bundle/configsync/defaults.go index 9b0157bf03..f5e5fa9da2 100644 --- a/bundle/configsync/defaults.go +++ b/bundle/configsync/defaults.go @@ -1,177 +1,26 @@ package configsync -import ( - "reflect" - "strings" -) +import "github.com/databricks/cli/libs/structs/structpath" -type ( - skipAlways struct{} - skipIfZeroOrNil struct{} - skipIfEmptyOrDefault struct { - defaults map[string]any - } -) - -var ( - alwaysSkip = skipAlways{} - zeroOrNil = skipIfZeroOrNil{} - emptyEmailNotifications = skipIfEmptyOrDefault{defaults: map[string]any{"no_alert_for_skipped_runs": false}} -) - -// serverSideDefaults contains all hardcoded server-side defaults. -// This is a temporary solution until the bundle plan issue is resolved. -// Fields mapped to alwaysSkip are always considered defaults regardless of value. -// Other fields are compared using reflect.DeepEqual. -var serverSideDefaults = map[string]any{ - // Job-level fields - "resources.jobs.*.timeout_seconds": zeroOrNil, - "resources.jobs.*.email_notifications": emptyEmailNotifications, - "resources.jobs.*.webhook_notifications": map[string]any{}, - "resources.jobs.*.edit_mode": alwaysSkip, // set by CLI - "resources.jobs.*.performance_target": "PERFORMANCE_OPTIMIZED", - - // Task-level fields - "resources.jobs.*.tasks[*].run_if": "ALL_SUCCESS", - "resources.jobs.*.tasks[*].disabled": false, - "resources.jobs.*.tasks[*].timeout_seconds": zeroOrNil, - "resources.jobs.*.tasks[*].notebook_task.source": "WORKSPACE", - "resources.jobs.*.tasks[*].email_notifications": emptyEmailNotifications, - "resources.jobs.*.tasks[*].webhook_notifications": map[string]any{}, - "resources.jobs.*.tasks[*].pipeline_task.full_refresh": false, - - "resources.jobs.*.tasks[*].for_each_task.task.run_if": "ALL_SUCCESS", - "resources.jobs.*.tasks[*].for_each_task.task.disabled": false, - "resources.jobs.*.tasks[*].for_each_task.task.timeout_seconds": zeroOrNil, - "resources.jobs.*.tasks[*].for_each_task.task.notebook_task.source": "WORKSPACE", - "resources.jobs.*.tasks[*].for_each_task.task.email_notifications": emptyEmailNotifications, - "resources.jobs.*.tasks[*].for_each_task.task.webhook_notifications": map[string]any{}, - - // Cluster fields (tasks) - "resources.jobs.*.tasks[*].new_cluster.aws_attributes": alwaysSkip, - "resources.jobs.*.tasks[*].new_cluster.azure_attributes": alwaysSkip, - "resources.jobs.*.tasks[*].new_cluster.gcp_attributes": alwaysSkip, - "resources.jobs.*.tasks[*].new_cluster.data_security_mode": "SINGLE_USER", // TODO this field is computed on some workspaces in integration tests, check why and if we can skip it - "resources.jobs.*.tasks[*].new_cluster.enable_elastic_disk": alwaysSkip, // deprecated field - "resources.jobs.*.tasks[*].new_cluster.single_user_name": alwaysSkip, - - // Cluster fields (job_clusters) - "resources.jobs.*.job_clusters[*].new_cluster.aws_attributes": alwaysSkip, - "resources.jobs.*.job_clusters[*].new_cluster.azure_attributes": alwaysSkip, - "resources.jobs.*.job_clusters[*].new_cluster.gcp_attributes": alwaysSkip, - "resources.jobs.*.job_clusters[*].new_cluster.data_security_mode": "SINGLE_USER", // TODO this field is computed on some workspaces in integration tests, check why and if we can skip it - "resources.jobs.*.job_clusters[*].new_cluster.enable_elastic_disk": alwaysSkip, // deprecated field - "resources.jobs.*.job_clusters[*].new_cluster.single_user_name": alwaysSkip, - - // Standalone cluster fields - "resources.clusters.*.aws_attributes": alwaysSkip, - "resources.clusters.*.azure_attributes": alwaysSkip, - "resources.clusters.*.gcp_attributes": alwaysSkip, - "resources.clusters.*.data_security_mode": "SINGLE_USER", - "resources.clusters.*.driver_node_type_id": alwaysSkip, - "resources.clusters.*.enable_elastic_disk": alwaysSkip, - "resources.clusters.*.single_user_name": alwaysSkip, - - // Experiment fields - "resources.experiments.*.artifact_location": alwaysSkip, - - // Registered model fields - "resources.registered_models.*.full_name": alwaysSkip, - "resources.registered_models.*.metastore_id": alwaysSkip, - "resources.registered_models.*.owner": alwaysSkip, - "resources.registered_models.*.storage_location": alwaysSkip, - - // Volume fields - "resources.volumes.*.storage_location": alwaysSkip, - - // SQL warehouse fields - "resources.sql_warehouses.*.creator_name": alwaysSkip, - "resources.sql_warehouses.*.min_num_clusters": int64(1), - "resources.sql_warehouses.*.warehouse_type": "CLASSIC", - - // Terraform defaults - "resources.jobs.*.run_as": alwaysSkip, - - // Pipeline fields - "resources.pipelines.*.storage": alwaysSkip, - "resources.pipelines.*.continuous": false, -} - -func shouldSkipField(path string, value any) bool { - for pattern, expected := range serverSideDefaults { - if matchPattern(pattern, path) { - if _, ok := expected.(skipAlways); ok { - return true - } - if _, ok := expected.(skipIfZeroOrNil); ok { - return value == nil || value == int64(0) - } - if marker, ok := expected.(skipIfEmptyOrDefault); ok { - m, ok := value.(map[string]any) - if !ok { - return false - } - if len(m) == 0 { - return true - } - return reflect.DeepEqual(m, marker.defaults) - } - return reflect.DeepEqual(value, expected) - } - } - return false -} - -func matchPattern(pattern, path string) bool { - patternParts := strings.Split(pattern, ".") - pathParts := strings.Split(path, ".") - return matchParts(patternParts, pathParts) -} - -func matchParts(patternParts, pathParts []string) bool { - if len(patternParts) == 0 && len(pathParts) == 0 { - return true - } - if len(patternParts) == 0 || len(pathParts) == 0 { - return false - } - - patternPart := patternParts[0] - pathPart := pathParts[0] - - if patternPart == "*" { - return matchParts(patternParts[1:], pathParts[1:]) - } - - if strings.Contains(patternPart, "[*]") { - prefix := strings.Split(patternPart, "[*]")[0] - - if strings.HasPrefix(pathPart, prefix) && strings.Contains(pathPart, "[") { - return matchParts(patternParts[1:], pathParts[1:]) - } - return false - } - - if patternPart == pathPart { - return matchParts(patternParts[1:], pathParts[1:]) - } - - return false +type resetRule struct { + field *structpath.PatternNode + value any } -// resetValues contains all values that should be used to reset CLI-defaulted fields. -// If CLI-defaulted field is changed on remote and should be disabled (e.g. queueing disabled -> remote field is nil) -// we can't define it in the config as "null" because CLI default will be applied again. -var resetValues = map[string]any{ - "resources.jobs.*.queue": map[string]any{ - "enabled": false, +// resetValues defines values that should replace CLI-defaulted fields. +// If a CLI-defaulted field is changed on remote and should be disabled +// (e.g. queueing disabled → remote field is nil), we can't define it +// in the config as "null" because the CLI default will be applied again. +var resetValues = map[string][]resetRule{ + "jobs": { + {field: structpath.MustParsePattern("queue"), value: map[string]any{"enabled": false}}, }, } -func resetValueIfNeeded(path string, value any) any { - for pattern, expected := range resetValues { - if matchPattern(pattern, path) { - return expected +func resetValueIfNeeded(resourceType string, path *structpath.PathNode, value any) any { + for _, rule := range resetValues[resourceType] { + if path.HasPatternPrefix(rule.field) { + return rule.value } } return value diff --git a/bundle/configsync/diff.go b/bundle/configsync/diff.go index 0d8bddc2e9..fa0cd186a4 100644 --- a/bundle/configsync/diff.go +++ b/bundle/configsync/diff.go @@ -8,15 +8,18 @@ import ( "io/fs" "os" "path/filepath" + "strings" "github.com/databricks/cli/bundle" "github.com/databricks/cli/bundle/config/engine" "github.com/databricks/cli/bundle/deploy" "github.com/databricks/cli/bundle/deployplan" "github.com/databricks/cli/bundle/direct" + "github.com/databricks/cli/bundle/direct/dresources" "github.com/databricks/cli/libs/dyn" "github.com/databricks/cli/libs/dyn/convert" "github.com/databricks/cli/libs/log" + "github.com/databricks/cli/libs/structs/structpath" ) type OperationType string @@ -47,7 +50,17 @@ func normalizeValue(v any) (any, error) { return dynValue.AsAny(), nil } -func filterEntityDefaults(basePath string, value any) any { +// shouldClassifySkip checks if a value should be skipped using ClassifyChange +// with a synthetic ChangeDesc (Old=nil, New=nil, Remote=value). +func shouldClassifySkip(ctx context.Context, adapter *dresources.Adapter, path *structpath.PathNode, value, remoteState any) bool { + ch := &deployplan.ChangeDesc{Old: nil, New: nil, Remote: value} + if err := direct.ClassifyChange(ctx, adapter, path, ch, remoteState); err != nil { + return false + } + return ch.Action == deployplan.Skip +} + +func filterEntityDefaults(ctx context.Context, adapter *dresources.Adapter, basePath *structpath.PathNode, value, remoteState any) any { if value == nil { return nil } @@ -55,8 +68,8 @@ func filterEntityDefaults(basePath string, value any) any { if arr, ok := value.([]any); ok { result := make([]any, 0, len(arr)) for i, elem := range arr { - elementPath := fmt.Sprintf("%s[%d]", basePath, i) - result = append(result, filterEntityDefaults(elementPath, elem)) + elemPath := structpath.NewIndex(basePath, i) + result = append(result, filterEntityDefaults(ctx, adapter, elemPath, elem, remoteState)) } return result } @@ -68,37 +81,43 @@ func filterEntityDefaults(basePath string, value any) any { result := make(map[string]any) for key, val := range m { - fieldPath := basePath + "." + key - - if shouldSkipField(fieldPath, val) { + fieldPath := structpath.NewDotString(basePath, key) + if shouldClassifySkip(ctx, adapter, fieldPath, val, remoteState) { continue } - if nestedMap, ok := val.(map[string]any); ok { - result[key] = filterEntityDefaults(fieldPath, nestedMap) + filtered := filterEntityDefaults(ctx, adapter, fieldPath, nestedMap, remoteState) + if filtered != nil { + result[key] = filtered + } } else { result[key] = val } } + if len(result) == 0 { + return nil + } + return result } -func convertChangeDesc(path string, cd *deployplan.ChangeDesc) (*ConfigChangeDesc, error) { - hasConfigValue := cd.Old != nil || cd.New != nil +func convertChangeDesc(ctx context.Context, adapter *dresources.Adapter, resourceType, fieldPath string, cd *deployplan.ChangeDesc, remoteState any) (*ConfigChangeDesc, error) { + pathNode, err := structpath.ParsePath(fieldPath) + if err != nil { + return nil, fmt.Errorf("failed to parse path %q: %w", fieldPath, err) + } + + hasConfigValue := cd.New != nil normalizedValue, err := normalizeValue(cd.Remote) if err != nil { return nil, fmt.Errorf("failed to normalize remote value: %w", err) } - if shouldSkipField(path, normalizedValue) { - return &ConfigChangeDesc{ - Operation: OperationSkip, - }, nil - } + // Recursive nested entity filtering using ClassifyChange. + normalizedValue = filterEntityDefaults(ctx, adapter, pathNode, normalizedValue, remoteState) - normalizedValue = filterEntityDefaults(path, normalizedValue) - normalizedValue = resetValueIfNeeded(path, normalizedValue) + normalizedValue = resetValueIfNeeded(resourceType, pathNode, normalizedValue) var op OperationType if normalizedValue == nil && hasConfigValue { @@ -143,13 +162,22 @@ func DetectChanges(ctx context.Context, b *bundle.Bundle, engine engine.EngineTy for resourceKey, entry := range plan.Plan { resourceChanges := make(ResourceChanges) + // resourceKey is "resources.jobs.foo" → parts[1] is "jobs" + parts := strings.Split(resourceKey, ".") + resourceType := parts[1] + + adapter, ok := deployBundle.Adapters[resourceType] + if !ok { + return nil, fmt.Errorf("no adapter for resource type %q", resourceType) + } + if entry.Changes != nil { for path, changeDesc := range entry.Changes { if changeDesc.Action == deployplan.Skip { continue } - change, err := convertChangeDesc(resourceKey+"."+path, changeDesc) + change, err := convertChangeDesc(ctx, adapter, resourceType, path, changeDesc, entry.RemoteState) if err != nil { return nil, fmt.Errorf("failed to compute config change for path %s: %w", path, err) } diff --git a/bundle/configsync/diff_test.go b/bundle/configsync/diff_test.go deleted file mode 100644 index 8ed7618b5b..0000000000 --- a/bundle/configsync/diff_test.go +++ /dev/null @@ -1,90 +0,0 @@ -package configsync - -import ( - "testing" - - "github.com/stretchr/testify/assert" -) - -func TestMatchPattern(t *testing.T) { - tests := []struct { - name string - pattern string - path string - want bool - }{ - { - name: "exact match", - pattern: "timeout_seconds", - path: "timeout_seconds", - want: true, - }, - { - name: "exact match no match", - pattern: "timeout_seconds", - path: "other_field", - want: false, - }, - { - name: "array wildcard match", - pattern: "tasks[*].run_if", - path: "tasks[task_key='my_task'].run_if", - want: true, - }, - { - name: "array wildcard no match", - pattern: "tasks[*].run_if", - path: "tasks[task_key='my_task'].disabled", - want: false, - }, - { - name: "nested array wildcard match", - pattern: "tasks[*].new_cluster.azure_attributes.availability", - path: "tasks[task_key='task1'].new_cluster.azure_attributes.availability", - want: true, - }, - { - name: "job_clusters array wildcard match", - pattern: "job_clusters[*].new_cluster.aws_attributes.availability", - path: "job_clusters[job_cluster_key='cluster1'].new_cluster.aws_attributes.availability", - want: true, - }, - { - name: "wildcard segment match", - pattern: "*.timeout_seconds", - path: "timeout_seconds", - want: false, - }, - { - name: "different array prefix no match", - pattern: "tasks[*].run_if", - path: "jobs[task_key='my_task'].run_if", - want: false, - }, - { - name: "nested path match", - pattern: "tasks[*].notebook_task.source", - path: "tasks[task_key='notebook'].notebook_task.source", - want: true, - }, - { - name: "path shorter than pattern", - pattern: "tasks[*].new_cluster.azure_attributes", - path: "tasks[task_key='task1'].new_cluster", - want: false, - }, - { - name: "path longer than pattern", - pattern: "tasks[*].new_cluster", - path: "tasks[task_key='task1'].new_cluster.azure_attributes", - want: false, - }, - } - - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - got := matchPattern(tt.pattern, tt.path) - assert.Equal(t, tt.want, got, "matchPattern(%q, %q)", tt.pattern, tt.path) - }) - } -} diff --git a/bundle/direct/bundle_plan.go b/bundle/direct/bundle_plan.go index 5c518adcc9..3fa87d6676 100644 --- a/bundle/direct/bundle_plan.go +++ b/bundle/direct/bundle_plan.go @@ -359,10 +359,58 @@ func prepareChanges(ctx context.Context, adapter *dresources.Adapter, localDiff, return m, nil } -func addPerFieldActions(ctx context.Context, adapter *dresources.Adapter, changes deployplan.Changes, remoteState any) error { +// ClassifyChange applies the full per-field classification to a single change +// using resources.yml rules. It handles skip checks, recreate/updateID, +// and adapter-specific overrides. +func ClassifyChange(ctx context.Context, adapter *dresources.Adapter, path *structpath.PathNode, ch *deployplan.ChangeDesc, remoteState any) error { cfg := adapter.ResourceConfig() generatedCfg := adapter.GeneratedResourceConfig() + if structdiff.IsEqual(ch.Remote, ch.New) { + ch.Action = deployplan.Skip + ch.Reason = deployplan.ReasonRemoteAlreadySet + } else if allEmpty(ch.Old, ch.New, ch.Remote) { + ch.Action = deployplan.Skip + ch.Reason = deployplan.ReasonEmpty + } else if reason, ok := shouldSkip(cfg, path, ch); ok { + ch.Action = deployplan.Skip + ch.Reason = reason + } else if reason, ok := shouldSkip(generatedCfg, path, ch); ok { + ch.Action = deployplan.Skip + ch.Reason = reason + } else if reason, ok := shouldSkipBackendDefault(cfg, path, ch); ok { + ch.Action = deployplan.Skip + ch.Reason = reason + } else if reason, ok := shouldSkipBackendDefault(generatedCfg, path, ch); ok { + ch.Action = deployplan.Skip + ch.Reason = reason + } else if action, reason := shouldUpdateOrRecreate(cfg, path); action != deployplan.Undefined { + ch.Action = action + ch.Reason = reason + } else if action, reason := shouldUpdateOrRecreate(generatedCfg, path); action != deployplan.Undefined { + ch.Action = action + ch.Reason = reason + } else { + ch.Action = deployplan.Update + } + + if adapter.HasOverrideChangeDesc() { + savedAction := ch.Action + savedReason := ch.Reason + + if err := adapter.OverrideChangeDesc(ctx, path, ch, remoteState); err != nil { + return fmt.Errorf("internal error: failed to classify change: %w", err) + } + + if savedAction != ch.Action && savedReason == ch.Reason { + ch.Reason = deployplan.ReasonCustom + } + } + + return nil +} + +func addPerFieldActions(ctx context.Context, adapter *dresources.Adapter, changes deployplan.Changes, remoteState any) error { var toDrop []string for pathString, ch := range changes { @@ -371,47 +419,8 @@ func addPerFieldActions(ctx context.Context, adapter *dresources.Adapter, change return err } - if structdiff.IsEqual(ch.Remote, ch.New) { - ch.Action = deployplan.Skip - ch.Reason = deployplan.ReasonRemoteAlreadySet - } else if allEmpty(ch.Old, ch.New, ch.Remote) { - ch.Action = deployplan.Skip - ch.Reason = deployplan.ReasonEmpty - } else if reason, ok := shouldSkip(cfg, path, ch); ok { - ch.Action = deployplan.Skip - ch.Reason = reason - } else if reason, ok := shouldSkip(generatedCfg, path, ch); ok { - ch.Action = deployplan.Skip - ch.Reason = reason - } else if reason, ok := shouldSkipBackendDefault(cfg, path, ch); ok { - ch.Action = deployplan.Skip - ch.Reason = reason - } else if reason, ok := shouldSkipBackendDefault(generatedCfg, path, ch); ok { - ch.Action = deployplan.Skip - ch.Reason = reason - } else if action, reason := shouldUpdateOrRecreate(cfg, path); action != deployplan.Undefined { - ch.Action = action - ch.Reason = reason - } else if action, reason := shouldUpdateOrRecreate(generatedCfg, path); action != deployplan.Undefined { - ch.Action = action - ch.Reason = reason - } else { - ch.Action = deployplan.Update - } - - if adapter.HasOverrideChangeDesc() { - savedAction := ch.Action - savedReason := ch.Reason - - err = adapter.OverrideChangeDesc(ctx, path, ch, remoteState) - if err != nil { - return fmt.Errorf("internal error: failed to classify change: %w", err) - } - - if savedAction != ch.Action && savedReason == ch.Reason { - // ch.Action was changed but not Reason field; set it to "custom" - ch.Reason = deployplan.ReasonCustom - } + if err := ClassifyChange(ctx, adapter, path, ch, remoteState); err != nil { + return err } if ch.Reason == deployplan.ReasonDrop { diff --git a/bundle/direct/dresources/resources.yml b/bundle/direct/dresources/resources.yml index a6c70ef44e..b442de7e5f 100644 --- a/bundle/direct/dresources/resources.yml +++ b/bundle/direct/dresources/resources.yml @@ -17,6 +17,10 @@ resources: jobs: ignore_remote_changes: + # edit_mode is set by CLI, not user-configurable + - field: edit_mode + reason: managed + # Same as clusters.{aws,azure,gcp}_attributes — see clusters/resource_cluster.go#L361-L363 # s.SchemaPath("aws_attributes").SetSuppressDiff() # s.SchemaPath("azure_attributes").SetSuppressDiff() @@ -35,6 +39,14 @@ resources: reason: managed backend_defaults: + # Backend returns PERFORMANCE_OPTIMIZED for serverless jobs when not set + - field: performance_target + values: ["PERFORMANCE_OPTIMIZED"] + + # Backend sets single_user_name on clusters when not specified + # - field: tasks[*].new_cluster.single_user_name + # - field: job_clusters[*].new_cluster.single_user_name + # Same as clusters.enable_elastic_disk — see clusters/resource_cluster.go#L331 # s.SchemaPath("enable_elastic_disk").SetComputed() - field: tasks[*].new_cluster.enable_elastic_disk @@ -464,6 +476,10 @@ resources: - field: min_num_clusters reason: managed + # creator_name is output-only, not settable by user. When set produces "Error: Value for unconfigurable attribute" error + - field: creator_name + reason: output_only + backend_defaults: # https://github.com/databricks/terraform-provider-databricks/blob/4eba541abe1a9f50993ea7b9dd83874207e224a1/sql/resource_sql_endpoint.go#L69 # m["enable_serverless_compute"].Computed = true From 36e72ff4d0ee66eb81da34a880cdad1a376cd398 Mon Sep 17 00:00:00 2001 From: Ilya Kuznetsov Date: Fri, 6 Mar 2026 17:33:23 +0100 Subject: [PATCH 2/5] Remove node_type_id backend default --- .../job_multiple_tasks/output.txt | 16 ++++++++++------ .../config-remote-sync/multiple_files/output.txt | 12 ++++++++---- bundle/direct/dresources/resources.yml | 13 ------------- 3 files changed, 18 insertions(+), 23 deletions(-) diff --git a/acceptance/bundle/config-remote-sync/job_multiple_tasks/output.txt b/acceptance/bundle/config-remote-sync/job_multiple_tasks/output.txt index 5e36f714ce..0d8b9275a9 100644 --- a/acceptance/bundle/config-remote-sync/job_multiple_tasks/output.txt +++ b/acceptance/bundle/config-remote-sync/job_multiple_tasks/output.txt @@ -24,11 +24,12 @@ Resource: resources.jobs.no_tasks_job >>> diff.py databricks.yml.backup databricks.yml --- databricks.yml.backup +++ databricks.yml -@@ -13,13 +13,10 @@ +@@ -13,13 +13,11 @@ node_type_id: [NODE_TYPE_ID] num_workers: 1 - - task_key: d_task + - new_cluster: ++ node_type_id: [NODE_TYPE_ID] + num_workers: 1 + spark_version: 13.3.x-snapshot-scala2.12 notebook_task: @@ -43,7 +44,7 @@ Resource: resources.jobs.no_tasks_job + task_key: e_task - task_key: c_task notebook_task: -@@ -28,7 +25,8 @@ +@@ -28,7 +26,8 @@ spark_version: 13.3.x-snapshot-scala2.12 node_type_id: [NODE_TYPE_ID] - num_workers: 2 @@ -54,13 +55,14 @@ Resource: resources.jobs.no_tasks_job + timeout_seconds: 3600 - task_key: a_task notebook_task: -@@ -41,5 +39,12 @@ +@@ -41,5 +40,13 @@ - task_key: c_task - no_tasks_job: {} + no_tasks_job: + tasks: + - new_cluster: ++ node_type_id: [NODE_TYPE_ID] + num_workers: 1 + spark_version: 13.3.x-snapshot-scala2.12 + notebook_task: @@ -92,11 +94,12 @@ Resource: resources.jobs.rename_task_job >>> diff.py databricks.yml.backup2 databricks.yml --- databricks.yml.backup2 +++ databricks.yml -@@ -50,14 +50,13 @@ +@@ -52,14 +52,14 @@ rename_task_job: tasks: - - task_key: b_task + - new_cluster: ++ node_type_id: [NODE_TYPE_ID] + num_workers: 1 + spark_version: 13.3.x-snapshot-scala2.12 notebook_task: @@ -112,14 +115,14 @@ Resource: resources.jobs.rename_task_job + - task_key: b_task_renamed notebook_task: notebook_path: /Users/{{workspace_user_name}}/d_task -@@ -68,5 +67,5 @@ +@@ -70,5 +70,5 @@ - task_key: c_task depends_on: - - task_key: b_task + - task_key: b_task_renamed notebook_task: notebook_path: /Users/{{workspace_user_name}}/c_task -@@ -77,7 +76,13 @@ +@@ -79,7 +79,14 @@ - task_key: a_task notebook_task: - notebook_path: /Users/{{workspace_user_name}}/a_task @@ -129,6 +132,7 @@ Resource: resources.jobs.rename_task_job node_type_id: [NODE_TYPE_ID] num_workers: 1 + - new_cluster: ++ node_type_id: [NODE_TYPE_ID] + num_workers: 1 + spark_version: 13.3.x-snapshot-scala2.12 + notebook_task: diff --git a/acceptance/bundle/config-remote-sync/multiple_files/output.txt b/acceptance/bundle/config-remote-sync/multiple_files/output.txt index 511f0f7f2e..e616ca008c 100644 --- a/acceptance/bundle/config-remote-sync/multiple_files/output.txt +++ b/acceptance/bundle/config-remote-sync/multiple_files/output.txt @@ -31,13 +31,14 @@ Resource: resources.jobs.job_two >>> diff.py resources/job1.yml.backup resources/job1.yml --- resources/job1.yml.backup +++ resources/job1.yml -@@ -4,13 +4,12 @@ +@@ -4,13 +4,13 @@ max_concurrent_runs: 1 tasks: - - task_key: c_task + - depends_on: + - task_key: b_task + new_cluster: ++ node_type_id: [NODE_TYPE_ID] + num_workers: 1 + spark_version: 13.3.x-snapshot-scala2.12 notebook_task: @@ -51,12 +52,13 @@ Resource: resources.jobs.job_two + task_key: c_task_renamed - task_key: a_task notebook_task: -@@ -21,3 +20,9 @@ +@@ -21,3 +21,10 @@ num_workers: 1 depends_on: - - task_key: c_task + - task_key: c_task_renamed + - new_cluster: ++ node_type_id: [NODE_TYPE_ID] + num_workers: 1 + spark_version: 13.3.x-snapshot-scala2.12 + notebook_task: @@ -68,7 +70,7 @@ Resource: resources.jobs.job_two >>> diff.py resources/job2.yml.backup resources/job2.yml --- resources/job2.yml.backup +++ resources/job2.yml -@@ -2,13 +2,12 @@ +@@ -2,13 +2,13 @@ jobs: job_two: - max_concurrent_runs: 2 @@ -76,6 +78,7 @@ Resource: resources.jobs.job_two tasks: - - task_key: run_pipeline + - new_cluster: ++ node_type_id: [NODE_TYPE_ID] + num_workers: 1 + spark_version: 13.3.x-snapshot-scala2.12 notebook_task: @@ -87,11 +90,12 @@ Resource: resources.jobs.job_two + task_key: run_pipeline_renamed - task_key: etl_pipeline notebook_task: -@@ -18,5 +17,8 @@ +@@ -18,5 +18,9 @@ node_type_id: [NODE_TYPE_ID] num_workers: 2 - - task_key: extra_task + - new_cluster: ++ node_type_id: [NODE_TYPE_ID] + num_workers: 1 + spark_version: 13.3.x-snapshot-scala2.12 notebook_task: diff --git a/bundle/direct/dresources/resources.yml b/bundle/direct/dresources/resources.yml index b442de7e5f..fbb4e9ebbc 100644 --- a/bundle/direct/dresources/resources.yml +++ b/bundle/direct/dresources/resources.yml @@ -57,11 +57,6 @@ resources: - field: tasks[*].new_cluster.enable_local_disk_encryption - field: job_clusters[*].new_cluster.enable_local_disk_encryption - # Same as clusters.node_type_id — see clusters/resource_cluster.go#L333 - # s.SchemaPath("node_type_id").SetComputed() - - field: tasks[*].new_cluster.node_type_id - - field: job_clusters[*].new_cluster.node_type_id - # Same as clusters.driver_node_type_id — see clusters/resource_cluster.go#L334 # s.SchemaPath("driver_node_type_id").SetComputed() - field: tasks[*].new_cluster.driver_node_type_id @@ -162,10 +157,6 @@ resources: # Backend generates storage path like dbfs:/pipelines/ when not set by user. - field: storage - # https://github.com/databricks/terraform-provider-databricks/blob/4eba541abe1a9f50993ea7b9dd83874207e224a1/pipelines/resource_pipeline.go#L218 - # s.SchemaPath("cluster", "node_type_id").SetComputed() - - field: clusters[*].node_type_id - # https://github.com/databricks/terraform-provider-databricks/blob/4eba541abe1a9f50993ea7b9dd83874207e224a1/pipelines/resource_pipeline.go#L219 # s.SchemaPath("cluster", "driver_node_type_id").SetComputed() - field: clusters[*].driver_node_type_id @@ -433,10 +424,6 @@ resources: # s.SchemaPath("enable_local_disk_encryption").SetComputed() - field: enable_local_disk_encryption - # https://github.com/databricks/terraform-provider-databricks/blob/4eba541abe1a9f50993ea7b9dd83874207e224a1/clusters/resource_cluster.go#L333 - # s.SchemaPath("node_type_id").SetComputed() - - field: node_type_id - # https://github.com/databricks/terraform-provider-databricks/blob/4eba541abe1a9f50993ea7b9dd83874207e224a1/clusters/resource_cluster.go#L334 # s.SchemaPath("driver_node_type_id").SetComputed() - field: driver_node_type_id From b317414c218dfb10f66ccf5b86fb658606d5f6d2 Mon Sep 17 00:00:00 2001 From: Ilya Kuznetsov Date: Sat, 7 Mar 2026 12:01:03 +0100 Subject: [PATCH 3/5] Run /simplify --- bundle/configsync/diff.go | 6 ++---- bundle/direct/dresources/resources.yml | 4 ++-- 2 files changed, 4 insertions(+), 6 deletions(-) diff --git a/bundle/configsync/diff.go b/bundle/configsync/diff.go index fa0cd186a4..93a437a575 100644 --- a/bundle/configsync/diff.go +++ b/bundle/configsync/diff.go @@ -8,9 +8,9 @@ import ( "io/fs" "os" "path/filepath" - "strings" "github.com/databricks/cli/bundle" + "github.com/databricks/cli/bundle/config" "github.com/databricks/cli/bundle/config/engine" "github.com/databricks/cli/bundle/deploy" "github.com/databricks/cli/bundle/deployplan" @@ -162,9 +162,7 @@ func DetectChanges(ctx context.Context, b *bundle.Bundle, engine engine.EngineTy for resourceKey, entry := range plan.Plan { resourceChanges := make(ResourceChanges) - // resourceKey is "resources.jobs.foo" → parts[1] is "jobs" - parts := strings.Split(resourceKey, ".") - resourceType := parts[1] + resourceType := config.GetResourceTypeFromKey(resourceKey) adapter, ok := deployBundle.Adapters[resourceType] if !ok { diff --git a/bundle/direct/dresources/resources.yml b/bundle/direct/dresources/resources.yml index fbb4e9ebbc..7de59c2bf9 100644 --- a/bundle/direct/dresources/resources.yml +++ b/bundle/direct/dresources/resources.yml @@ -44,8 +44,8 @@ resources: values: ["PERFORMANCE_OPTIMIZED"] # Backend sets single_user_name on clusters when not specified - # - field: tasks[*].new_cluster.single_user_name - # - field: job_clusters[*].new_cluster.single_user_name + - field: tasks[*].new_cluster.single_user_name + - field: job_clusters[*].new_cluster.single_user_name # Same as clusters.enable_elastic_disk — see clusters/resource_cluster.go#L331 # s.SchemaPath("enable_elastic_disk").SetComputed() From bb2e6a3906cf422277a0927596f3dd6e924be8a8 Mon Sep 17 00:00:00 2001 From: Ilya Kuznetsov Date: Sat, 7 Mar 2026 13:39:27 +0100 Subject: [PATCH 4/5] Restore comment --- bundle/direct/bundle_plan.go | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/bundle/direct/bundle_plan.go b/bundle/direct/bundle_plan.go index 3fa87d6676..85e61d33ce 100644 --- a/bundle/direct/bundle_plan.go +++ b/bundle/direct/bundle_plan.go @@ -398,11 +398,13 @@ func ClassifyChange(ctx context.Context, adapter *dresources.Adapter, path *stru savedAction := ch.Action savedReason := ch.Reason - if err := adapter.OverrideChangeDesc(ctx, path, ch, remoteState); err != nil { + err := adapter.OverrideChangeDesc(ctx, path, ch, remoteState) + if err != nil { return fmt.Errorf("internal error: failed to classify change: %w", err) } if savedAction != ch.Action && savedReason == ch.Reason { + // ch.Action was changed but not Reason field; set it to "custom" ch.Reason = deployplan.ReasonCustom } } From ff8e770bb3ba06d68a333a95aacd86704bcd788b Mon Sep 17 00:00:00 2001 From: Ilya Kuznetsov Date: Mon, 9 Mar 2026 00:17:48 +0100 Subject: [PATCH 5/5] Remove single_user_name --- bundle/direct/dresources/resources.yml | 4 ---- 1 file changed, 4 deletions(-) diff --git a/bundle/direct/dresources/resources.yml b/bundle/direct/dresources/resources.yml index 7de59c2bf9..64f23ae9fd 100644 --- a/bundle/direct/dresources/resources.yml +++ b/bundle/direct/dresources/resources.yml @@ -43,10 +43,6 @@ resources: - field: performance_target values: ["PERFORMANCE_OPTIMIZED"] - # Backend sets single_user_name on clusters when not specified - - field: tasks[*].new_cluster.single_user_name - - field: job_clusters[*].new_cluster.single_user_name - # Same as clusters.enable_elastic_disk — see clusters/resource_cluster.go#L331 # s.SchemaPath("enable_elastic_disk").SetComputed() - field: tasks[*].new_cluster.enable_elastic_disk