From 06c8e4ae5f5b6e09da893a1001407df609524372 Mon Sep 17 00:00:00 2001 From: Rada Kamysheva Date: Fri, 24 Jul 2026 09:58:21 +0000 Subject: [PATCH 1/3] Fix never-converging job diff on reordered webhook notifications The Jobs API treats webhook_notifications.on_* lists as unordered sets and may return them in a different order than submitted. The direct engine compared these lists positionally (structdiff with no key func), so a stable server-side reordering produced a phantom diff that "bundle plan"/"bundle deploy" could never converge past. Add the job-, task-, and for_each-task-level webhook_notifications.on_* paths to ResourceJob.KeyedSlices(), keyed by id, so the diff is order-insensitive. This matches the existing pattern for depends_on. --- bundle/direct/dresources/job.go | 32 +++++++++++- bundle/direct/dresources/job_test.go | 73 ++++++++++++++++++++++++++++ 2 files changed, 104 insertions(+), 1 deletion(-) diff --git a/bundle/direct/dresources/job.go b/bundle/direct/dresources/job.go index cc8ab72b43..d97e0cb02d 100644 --- a/bundle/direct/dresources/job.go +++ b/bundle/direct/dresources/job.go @@ -75,8 +75,32 @@ func getDependsOnTaskKey(x jobs.TaskDependency) (string, string) { return "task_key", x.TaskKey } +func getWebhookKey(x jobs.Webhook) (string, string) { + return "id", x.Id +} + +// webhookNotificationEvents are the on_* fields of jobs.WebhookNotifications, +// each a []jobs.Webhook. The Jobs API treats these lists as unordered sets and +// may return them in a different order than submitted, so they must be diffed +// by id rather than by index to avoid a never-converging phantom diff. +var webhookNotificationEvents = []string{ + "on_start", + "on_success", + "on_failure", + "on_duration_warning_threshold_exceeded", + "on_streaming_backlog_exceeded", +} + +// webhookNotificationParents are the paths that hold a jobs.WebhookNotifications: +// at the job level, on each task, and on the nested for_each task. +var webhookNotificationParents = []string{ + "webhook_notifications", + "tasks[*].webhook_notifications", + "tasks[*].for_each_task.task.webhook_notifications", +} + func (*ResourceJob) KeyedSlices() map[string]any { - return map[string]any{ + result := map[string]any{ "tasks": getTaskKey, "parameters": getParameterName, "job_clusters": getJobClusterKey, @@ -84,6 +108,12 @@ func (*ResourceJob) KeyedSlices() map[string]any { "tasks[*].depends_on": getDependsOnTaskKey, "tasks[*].for_each_task.task.depends_on": getDependsOnTaskKey, } + for _, parent := range webhookNotificationParents { + for _, event := range webhookNotificationEvents { + result[parent+"."+event] = getWebhookKey + } + } + return result } func (r *ResourceJob) DoRead(ctx context.Context, id string) (*JobRemote, error) { diff --git a/bundle/direct/dresources/job_test.go b/bundle/direct/dresources/job_test.go index 8b8c85c8a2..f997c2de36 100644 --- a/bundle/direct/dresources/job_test.go +++ b/bundle/direct/dresources/job_test.go @@ -4,7 +4,10 @@ import ( "reflect" "testing" + "github.com/databricks/cli/libs/structs/structdiff" "github.com/databricks/databricks-sdk-go/service/jobs" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" ) // TestJobRemote verifies that all fields from jobs.Job (except Settings and pagination/internal fields) @@ -17,3 +20,73 @@ func TestJobRemote(t *testing.T) { "NextPageToken": true, // Pagination field, not relevant for single job read }) } + +func webhooks(ids ...string) []jobs.Webhook { + result := make([]jobs.Webhook, len(ids)) + for i, id := range ids { + result[i] = jobs.Webhook{Id: id} + } + return result +} + +// TestJobWebhookNotificationsOrderInsensitive verifies that webhook notification +// lists are diffed by id, so a permutation returned by the Jobs API does not +// produce a phantom diff. Without KeyedSlices this list is compared positionally +// and never converges. +func TestJobWebhookNotificationsOrderInsensitive(t *testing.T) { + keys := (&ResourceJob{}).KeyedSlices() + + config := jobs.JobSettings{ + WebhookNotifications: &jobs.WebhookNotifications{ + OnSuccess: webhooks("a", "b", "c"), + }, + Tasks: []jobs.Task{ + { + TaskKey: "t1", + WebhookNotifications: &jobs.WebhookNotifications{ + OnFailure: webhooks("a", "b"), + }, + ForEachTask: &jobs.ForEachTask{ + Task: jobs.Task{ + TaskKey: "inner", + WebhookNotifications: &jobs.WebhookNotifications{ + OnStart: webhooks("x", "y"), + }, + }, + }, + }, + }, + } + + remote := jobs.JobSettings{ + WebhookNotifications: &jobs.WebhookNotifications{ + OnSuccess: webhooks("a", "c", "b"), + }, + Tasks: []jobs.Task{ + { + TaskKey: "t1", + WebhookNotifications: &jobs.WebhookNotifications{ + OnFailure: webhooks("b", "a"), + }, + ForEachTask: &jobs.ForEachTask{ + Task: jobs.Task{ + TaskKey: "inner", + WebhookNotifications: &jobs.WebhookNotifications{ + OnStart: webhooks("y", "x"), + }, + }, + }, + }, + }, + } + + changes, err := structdiff.GetStructDiff(config, remote, keys) + require.NoError(t, err) + assert.Empty(t, changes) + + // A genuinely different destination set is still detected. + remote.WebhookNotifications.OnSuccess = webhooks("a", "b", "d") + changes, err = structdiff.GetStructDiff(config, remote, keys) + require.NoError(t, err) + assert.NotEmpty(t, changes) +} From ba82b564a23f7d7d42317a313b02289f53c20c6b Mon Sep 17 00:00:00 2001 From: Rada Kamysheva Date: Fri, 24 Jul 2026 10:02:24 +0000 Subject: [PATCH 2/3] Shorten comments --- bundle/direct/dresources/job.go | 9 +++------ bundle/direct/dresources/job_test.go | 6 ++---- 2 files changed, 5 insertions(+), 10 deletions(-) diff --git a/bundle/direct/dresources/job.go b/bundle/direct/dresources/job.go index d97e0cb02d..60eab5cf93 100644 --- a/bundle/direct/dresources/job.go +++ b/bundle/direct/dresources/job.go @@ -79,10 +79,8 @@ func getWebhookKey(x jobs.Webhook) (string, string) { return "id", x.Id } -// webhookNotificationEvents are the on_* fields of jobs.WebhookNotifications, -// each a []jobs.Webhook. The Jobs API treats these lists as unordered sets and -// may return them in a different order than submitted, so they must be diffed -// by id rather than by index to avoid a never-converging phantom diff. +// The Jobs API returns webhook_notifications.on_* in an arbitrary order, so +// diff them by id to avoid a phantom diff that never converges. var webhookNotificationEvents = []string{ "on_start", "on_success", @@ -91,8 +89,7 @@ var webhookNotificationEvents = []string{ "on_streaming_backlog_exceeded", } -// webhookNotificationParents are the paths that hold a jobs.WebhookNotifications: -// at the job level, on each task, and on the nested for_each task. +// webhook_notifications appears at the job, task, and for_each task levels. var webhookNotificationParents = []string{ "webhook_notifications", "tasks[*].webhook_notifications", diff --git a/bundle/direct/dresources/job_test.go b/bundle/direct/dresources/job_test.go index f997c2de36..ca19620023 100644 --- a/bundle/direct/dresources/job_test.go +++ b/bundle/direct/dresources/job_test.go @@ -29,10 +29,8 @@ func webhooks(ids ...string) []jobs.Webhook { return result } -// TestJobWebhookNotificationsOrderInsensitive verifies that webhook notification -// lists are diffed by id, so a permutation returned by the Jobs API does not -// produce a phantom diff. Without KeyedSlices this list is compared positionally -// and never converges. +// TestJobWebhookNotificationsOrderInsensitive verifies that a webhook list +// reordered by the Jobs API produces no diff, but a changed set still does. func TestJobWebhookNotificationsOrderInsensitive(t *testing.T) { keys := (&ResourceJob{}).KeyedSlices() From 0078d4b3a6738e411102a7a8baf789dd8ff0f09b Mon Sep 17 00:00:00 2001 From: Rada Kamysheva Date: Fri, 24 Jul 2026 10:25:10 +0000 Subject: [PATCH 3/3] Add acceptance test for reordered webhook notifications Deploys a job with webhook_notifications.on_* destinations, then uses edit_resource.py to reorder the remote lists (simulating the Jobs API returning them in a different order) and asserts bundle plan reports 0 to change. Without the KeyedSlices fix this plan shows 1 to change. --- .../webhook-reorder-remote/databricks.yml | 20 +++ .../out.plan.direct.json | 27 ++++ .../jobs/webhook-reorder-remote/out.test.toml | 3 + .../jobs/webhook-reorder-remote/output.txt | 123 ++++++++++++++++++ .../jobs/webhook-reorder-remote/script | 19 +++ .../jobs/webhook-reorder-remote/test.toml | 2 + 6 files changed, 194 insertions(+) create mode 100644 acceptance/bundle/resources/jobs/webhook-reorder-remote/databricks.yml create mode 100644 acceptance/bundle/resources/jobs/webhook-reorder-remote/out.plan.direct.json create mode 100644 acceptance/bundle/resources/jobs/webhook-reorder-remote/out.test.toml create mode 100644 acceptance/bundle/resources/jobs/webhook-reorder-remote/output.txt create mode 100644 acceptance/bundle/resources/jobs/webhook-reorder-remote/script create mode 100644 acceptance/bundle/resources/jobs/webhook-reorder-remote/test.toml diff --git a/acceptance/bundle/resources/jobs/webhook-reorder-remote/databricks.yml b/acceptance/bundle/resources/jobs/webhook-reorder-remote/databricks.yml new file mode 100644 index 0000000000..32480227c4 --- /dev/null +++ b/acceptance/bundle/resources/jobs/webhook-reorder-remote/databricks.yml @@ -0,0 +1,20 @@ +bundle: + name: test-bundle + +resources: + jobs: + my_job: + name: webhook reorder + webhook_notifications: + on_success: + - id: alpha + - id: beta + - id: gamma + tasks: + - task_key: main + run_job_task: + job_id: 123 + webhook_notifications: + on_start: + - id: delta + - id: epsilon diff --git a/acceptance/bundle/resources/jobs/webhook-reorder-remote/out.plan.direct.json b/acceptance/bundle/resources/jobs/webhook-reorder-remote/out.plan.direct.json new file mode 100644 index 0000000000..d4a8e9cdc2 --- /dev/null +++ b/acceptance/bundle/resources/jobs/webhook-reorder-remote/out.plan.direct.json @@ -0,0 +1,27 @@ +{ + "email_notifications": { + "action": "skip", + "reason": "empty", + "remote": {} + }, + "tasks[task_key='main'].email_notifications": { + "action": "skip", + "reason": "empty", + "remote": {} + }, + "tasks[task_key='main'].run_if": { + "action": "skip", + "reason": "backend_default", + "remote": "ALL_SUCCESS" + }, + "tasks[task_key='main'].timeout_seconds": { + "action": "skip", + "reason": "empty", + "remote": 0 + }, + "timeout_seconds": { + "action": "skip", + "reason": "empty", + "remote": 0 + } +} diff --git a/acceptance/bundle/resources/jobs/webhook-reorder-remote/out.test.toml b/acceptance/bundle/resources/jobs/webhook-reorder-remote/out.test.toml new file mode 100644 index 0000000000..e90b6d5d1b --- /dev/null +++ b/acceptance/bundle/resources/jobs/webhook-reorder-remote/out.test.toml @@ -0,0 +1,3 @@ +Local = true +Cloud = false +EnvMatrix.DATABRICKS_BUNDLE_ENGINE = ["direct"] diff --git a/acceptance/bundle/resources/jobs/webhook-reorder-remote/output.txt b/acceptance/bundle/resources/jobs/webhook-reorder-remote/output.txt new file mode 100644 index 0000000000..e8c0f8a022 --- /dev/null +++ b/acceptance/bundle/resources/jobs/webhook-reorder-remote/output.txt @@ -0,0 +1,123 @@ + +>>> [CLI] bundle plan +create jobs.my_job + +Plan: 1 to add, 0 to change, 0 to delete, 0 unchanged +Uploading bundle files to /Workspace/Users/[USERNAME]/.bundle/test-bundle/default/files... +Deploying resources... +Updating deployment state... +Deployment complete! + +>>> [CLI] bundle plan +Plan: 0 to add, 0 to change, 0 to delete, 1 unchanged + +=== Remote returns webhook destinations in a different order + +>>> [CLI] bundle plan +Plan: 0 to add, 0 to change, 0 to delete, 1 unchanged + +>>> print_requests.py //jobs +{ + "method": "POST", + "path": "/api/2.2/jobs/create", + "body": { + "deployment": { + "kind": "BUNDLE", + "metadata_file_path": "/Workspace/Users/[USERNAME]/.bundle/test-bundle/default/state/metadata.json" + }, + "edit_mode": "UI_LOCKED", + "format": "MULTI_TASK", + "max_concurrent_runs": 1, + "name": "webhook reorder", + "queue": { + "enabled": true + }, + "tasks": [ + { + "run_job_task": { + "job_id": 123 + }, + "task_key": "main", + "webhook_notifications": { + "on_start": [ + { + "id": "delta" + }, + { + "id": "epsilon" + } + ] + } + } + ], + "webhook_notifications": { + "on_success": [ + { + "id": "alpha" + }, + { + "id": "beta" + }, + { + "id": "gamma" + } + ] + } + } +} +{ + "method": "POST", + "path": "/api/2.2/jobs/reset", + "body": { + "job_id": [MY_JOB_ID], + "new_settings": { + "deployment": { + "kind": "BUNDLE", + "metadata_file_path": "/Workspace/Users/[USERNAME]/.bundle/test-bundle/default/state/metadata.json" + }, + "edit_mode": "UI_LOCKED", + "email_notifications": {}, + "format": "MULTI_TASK", + "max_concurrent_runs": 1, + "name": "webhook reorder", + "queue": { + "enabled": true + }, + "tasks": [ + { + "email_notifications": {}, + "run_if": "ALL_SUCCESS", + "run_job_task": { + "job_id": 123 + }, + "task_key": "main", + "timeout_seconds": 0, + "webhook_notifications": { + "on_start": [ + { + "id": "epsilon" + }, + { + "id": "delta" + } + ] + } + } + ], + "timeout_seconds": 0, + "webhook_notifications": { + "on_success": [ + { + "id": "alpha" + }, + { + "id": "gamma" + }, + { + "id": "beta" + } + ] + } + } + } +} diff --git a/acceptance/bundle/resources/jobs/webhook-reorder-remote/script b/acceptance/bundle/resources/jobs/webhook-reorder-remote/script new file mode 100644 index 0000000000..ebbcb3409b --- /dev/null +++ b/acceptance/bundle/resources/jobs/webhook-reorder-remote/script @@ -0,0 +1,19 @@ +trace $CLI bundle plan +$CLI bundle deploy +job_id="$(read_id.py my_job)" + +trace $CLI bundle plan | contains.py "0 to add, 0 to change, 0 to delete" + +title "Remote returns webhook destinations in a different order\n" +edit_resource.py jobs "$job_id" < out.plan.$DATABRICKS_BUNDLE_ENGINE.json + +trace print_requests.py //jobs diff --git a/acceptance/bundle/resources/jobs/webhook-reorder-remote/test.toml b/acceptance/bundle/resources/jobs/webhook-reorder-remote/test.toml new file mode 100644 index 0000000000..dfe6b08373 --- /dev/null +++ b/acceptance/bundle/resources/jobs/webhook-reorder-remote/test.toml @@ -0,0 +1,2 @@ +# The phantom-diff bug and the KeyedSlices fix are specific to the direct engine. +EnvMatrix.DATABRICKS_BUNDLE_ENGINE = ['direct']