direct: Set default reason="custom" for OverrideChangeDesc (#4383)

denik · web-flow · commit 5c11b271c608 · 2026-01-29T09:16:25.000+01:00
## Changes
When OverrideChangeDesc function changes the action but not the reason,
set the reason to "custom".

## Why
It's easy to accidentally forget to update the reason in
OverrideChangeDesc func (this is what happens for dashboards today).

## Tests
Existing tests that record the plan.
diff --git a/acceptance/bundle/migrate/dashboards/out.plan_after_migrate.json b/acceptance/bundle/migrate/dashboards/out.plan_after_migrate.json
@@ -23,11 +23,13 @@
       "changes": {
         "etag": {
           "action": "skip",
+          "reason": "custom",
           "old": "[NUMID]",
           "remote": "[NUMID]"
         },
         "serialized_dashboard": {
           "action": "skip",
+          "reason": "custom",
           "old": "{\"pages\":[{\"name\":\"02724bf2\",\"displayName\":\"Dashboard test bundle-deploy-dashboard\"}]}\n",
           "new": "{\"pages\":[{\"name\":\"02724bf2\",\"displayName\":\"Dashboard test bundle-deploy-dashboard\"}]}\n",
           "remote": "{\"pages\":[{\"displayName\":\"Dashboard test bundle-deploy-dashboard\",\"name\":\"02724bf2\",\"pageType\":\"PAGE_TYPE_CANVAS\"}]}\n"
diff --git a/acceptance/bundle/migrate/runas/out.plan.json b/acceptance/bundle/migrate/runas/out.plan.json
@@ -37,7 +37,7 @@
       "changes": {
         "run_as": {
           "action": "skip",
-          "reason": "override",
+          "reason": "custom",
           "old": {
             "service_principal_name": "[UUID]"
           },
diff --git a/acceptance/bundle/resources/clusters/deploy/update-and-resize-autoscale/out.plan_.direct.json b/acceptance/bundle/resources/clusters/deploy/update-and-resize-autoscale/out.plan_.direct.json
@@ -144,12 +144,14 @@
       "changes": {
         "autoscale.max_workers": {
           "action": "resize",
+          "reason": "custom",
           "old": 5,
           "new": 6,
           "remote": 5
         },
         "autoscale.min_workers": {
           "action": "resize",
+          "reason": "custom",
           "old": 3,
           "new": 4,
           "remote": 3
@@ -190,6 +192,7 @@
       "changes": {
         "autoscale": {
           "action": "resize",
+          "reason": "custom",
           "old": {
             "max_workers": 6,
             "min_workers": 4
@@ -201,6 +204,7 @@
         },
         "num_workers": {
           "action": "resize",
+          "reason": "custom",
           "new": 3
         }
       }
diff --git a/acceptance/bundle/resources/clusters/deploy/update-and-resize/out.plan_.direct.json b/acceptance/bundle/resources/clusters/deploy/update-and-resize/out.plan_.direct.json
@@ -96,6 +96,7 @@
       "changes": {
         "num_workers": {
           "action": "resize",
+          "reason": "custom",
           "old": 3,
           "new": 4,
           "remote": 3
@@ -139,6 +140,7 @@
       "changes": {
         "num_workers": {
           "action": "resize",
+          "reason": "custom",
           "old": 4,
           "new": 5,
           "remote": 4
diff --git a/acceptance/bundle/resources/dashboards/dataset-catalog-schema/out.plan.direct.json b/acceptance/bundle/resources/dashboards/dataset-catalog-schema/out.plan.direct.json
@@ -23,16 +23,19 @@
       "changes": {
         "dataset_catalog": {
           "action": "skip",
+          "reason": "custom",
           "old": "main",
           "new": "main"
         },
         "dataset_schema": {
           "action": "skip",
+          "reason": "custom",
           "old": "default",
           "new": "default"
         },
         "etag": {
           "action": "skip",
+          "reason": "custom",
           "old": [ETAG],
           "remote": [ETAG]
         },
diff --git a/acceptance/bundle/resources/dashboards/delete-trashed-out-of-band/out.plan.direct.json b/acceptance/bundle/resources/dashboards/delete-trashed-out-of-band/out.plan.direct.json
@@ -19,7 +19,7 @@
       "changes": {
         "etag": {
           "action": "update",
-          "reason": "remote_already_set",
+          "reason": "custom",
           "old": [ETAG]
         }
       }
diff --git a/acceptance/bundle/resources/dashboards/detect-change/out.plan.direct.json b/acceptance/bundle/resources/dashboards/detect-change/out.plan.direct.json
@@ -38,6 +38,7 @@
         },
         "serialized_dashboard": {
           "action": "skip",
+          "reason": "custom",
           "old": "{\n  \"pages\": [\n    {\n      \"displayName\": \"New Page\",\n      \"layout\": [\n        {\n          \"position\": {\n            \"height\": 2,\n            \"width\": 6,\n            \"x\": 0,\n            \"y\": 0\n          },\n          \"widget\": {\n            \"name\": \"82eb9107\",\n            \"textbox_spec\": \"# I'm a title\"\n          }\n        },\n        {\n          \"position\": {\n            \"height\": 2,\n            \"width\": 6,\n            \"x\": 0,\n            \"y\": 2\n          },\n          \"widget\": {\n            \"name\": \"ffa6de4f\",\n            \"textbox_spec\": \"Text\"\n          }\n        }\n      ],\n      \"name\": \"fdd21a3c\"\n    }\n  ]\n}\n",
           "new": "{\n  \"pages\": [\n    {\n      \"displayName\": \"New Page\",\n      \"layout\": [\n        {\n          \"position\": {\n            \"height\": 2,\n            \"width\": 6,\n            \"x\": 0,\n            \"y\": 0\n          },\n          \"widget\": {\n            \"name\": \"82eb9107\",\n            \"textbox_spec\": \"# I'm a title\"\n          }\n        },\n        {\n          \"position\": {\n            \"height\": 2,\n            \"width\": 6,\n            \"x\": 0,\n            \"y\": 2\n          },\n          \"widget\": {\n            \"name\": \"ffa6de4f\",\n            \"textbox_spec\": \"Text\"\n          }\n        }\n      ],\n      \"name\": \"fdd21a3c\"\n    }\n  ]\n}\n",
           "remote": "{}\n"
diff --git a/acceptance/bundle/resources/dashboards/simple/out.plan.direct.json b/acceptance/bundle/resources/dashboards/simple/out.plan.direct.json
@@ -23,11 +23,13 @@
       "changes": {
         "etag": {
           "action": "skip",
+          "reason": "custom",
           "old": [ETAG],
           "remote": [ETAG]
         },
         "serialized_dashboard": {
           "action": "skip",
+          "reason": "custom",
           "old": "{ }\n",
           "new": "{ }\n",
           "remote": "{}\n"
diff --git a/acceptance/bundle/resources/dashboards/unpublish-out-of-band/out.plan.direct.json b/acceptance/bundle/resources/dashboards/unpublish-out-of-band/out.plan.direct.json
@@ -33,6 +33,7 @@
       "changes": {
         "etag": {
           "action": "skip",
+          "reason": "custom",
           "old": [ETAG],
           "remote": [ETAG]
         },
@@ -44,6 +45,7 @@
         },
         "serialized_dashboard": {
           "action": "skip",
+          "reason": "custom",
           "old": "{\"pages\":[{\"displayName\":\"Test Dashboard\",\"name\":\"test-page\"}]}",
           "new": "{\"pages\":[{\"displayName\":\"Test Dashboard\",\"name\":\"test-page\"}]}",
           "remote": "{\"pages\":[{\"displayName\":\"Test Dashboard\",\"name\":\"test-page\",\"pageType\":\"PAGE_TYPE_CANVAS\"}]}"
diff --git a/bundle/deployplan/plan.go b/bundle/deployplan/plan.go
@@ -106,6 +106,7 @@ const (
 	ReasonEmptySlice        = "empty_slice"
 	ReasonEmptyMap          = "empty_map"
 	ReasonEmptyStruct       = "empty_struct"
+	ReasonCustom            = "custom"
 )
 
 // HasChange checks if there are any changes for fields with the given prefix.
diff --git a/bundle/direct/bundle_plan.go b/bundle/direct/bundle_plan.go
@@ -400,9 +400,19 @@ func addPerFieldActions(ctx context.Context, adapter *dresources.Adapter, change
 			ch.Action = deployplan.Update
 		}
 
-		err = adapter.OverrideChangeDesc(ctx, path, ch, remoteState)
-		if err != nil {
-			return fmt.Errorf("internal error: failed to classify change: %w", err)
+		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
+			}
 		}
 	}
 
diff --git a/bundle/direct/dresources/adapter.go b/bundle/direct/dresources/adapter.go
@@ -500,11 +500,13 @@ func (a *Adapter) WaitAfterUpdate(ctx context.Context, newState any) (any, error
 	return remoteState, nil
 }
 
+// HasOverrideChangeDesc returns true if OverrideChangeDesc is defined for this resource impl
+func (a *Adapter) HasOverrideChangeDesc() bool {
+	return a.overrideChangeDesc != nil
+}
+
 // OverrideChangeDesc allows custom logic to override change classification.
 func (a *Adapter) OverrideChangeDesc(ctx context.Context, path *structpath.PathNode, change *ChangeDesc, remoteState any) error {
-	if a.overrideChangeDesc == nil {
-		return nil
-	}
 	_, err := a.overrideChangeDesc.Call(ctx, path, change, remoteState)
 	return err
 }
diff --git a/bundle/direct/dresources/all_test.go b/bundle/direct/dresources/all_test.go
@@ -623,8 +623,10 @@ func testCRUD(t *testing.T, group string, adapter *Adapter, client *databricks.W
 	p, err := structpath.Parse("name")
 	require.NoError(t, err)
 
-	err = adapter.OverrideChangeDesc(ctx, p, &deployplan.ChangeDesc{}, nil)
-	require.NoError(t, err)
+	if adapter.HasOverrideChangeDesc() {
+		err = adapter.OverrideChangeDesc(ctx, p, &deployplan.ChangeDesc{}, nil)
+		require.NoError(t, err)
+	}
 
 	deleteIsNoop := strings.HasSuffix(group, "permissions") || strings.HasSuffix(group, "grants")
 
diff --git a/bundle/direct/dresources/cluster.go b/bundle/direct/dresources/cluster.go
@@ -122,6 +122,10 @@ func (r *ResourceCluster) DoDelete(ctx context.Context, id string) error {
 }
 
 func (r *ResourceCluster) OverrideChangeDesc(ctx context.Context, p *structpath.PathNode, change *ChangeDesc, remoteState *compute.ClusterDetails) error {
+	if change.Action == deployplan.Skip {
+		return nil
+	}
+
 	path := p.Prefix(1).String()
 	switch path {
 	case "data_security_mode":
diff --git a/bundle/direct/dresources/pipeline.go b/bundle/direct/dresources/pipeline.go
@@ -126,7 +126,6 @@ func (*ResourcePipeline) OverrideChangeDesc(ctx context.Context, path *structpat
 	if path.String() == "run_as" {
 		if structdiff.IsEqual(ch.Old, ch.New) {
 			ch.Action = deployplan.Skip
-			ch.Reason = "override"
 		}
 	}
 

Original file line number	Diff line number	Diff line change
`@@ -19,7 +19,7 @@`
`19`	`19`	`"changes": {`
`20`	`20`	`"etag": {`
`21`	`21`	`"action": "update",`
`22`		`- "reason": "remote_already_set",`
	`22`	`+ "reason": "custom",`
`23`	`23`	`"old": [ETAG]`
`24`	`24`	`}`
`25`	`25`	`}`
Original file line number	Diff line number	Diff line change
`@@ -106,6 +106,7 @@ const (`
`106`	`106`	`ReasonEmptySlice = "empty_slice"`
`107`	`107`	`ReasonEmptyMap = "empty_map"`
`108`	`108`	`ReasonEmptyStruct = "empty_struct"`
	`109`	`+ ReasonCustom = "custom"`
`109`	`110`	`)`
`110`	`111`
`111`	`112`	`// HasChange checks if there are any changes for fields with the given prefix.`