Only suppress diff for run_if when it is ALL_SUCCESS

common/customizable_schema.go

@@ -1,6 +1,9 @@
 package common
 
 import (
+	"fmt"
+	"slices"
+
 	"github.com/hashicorp/go-cty/cty"
 	"github.com/hashicorp/terraform-plugin-sdk/v2/diag"
 	"github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema"
@@ -82,6 +85,36 @@ func (s *CustomizableSchema) SetSuppressDiff() *CustomizableSchema {
 	return s
 }
 
+// SetSuppressDiffWithDefault suppresses the diff if the
+// new value (ie value from HCL config) is not set and
+// the old value (ie value from state / platform) is equal to the default value.
+//
+// Often Databricks HTTP APIs will return values for fields that were not set by
+// the author in their terraform configuration This function allows us to suppress
+// the diff in these cases.
+func (s *CustomizableSchema) SetSuppressDiffWithDefault(dv any) *CustomizableSchema {
+	primitiveTypes := []schema.ValueType{schema.TypeBool, schema.TypeString, schema.TypeInt, schema.TypeFloat}
+	if !slices.Contains(primitiveTypes, s.Schema.Type) {
+		panic(fmt.Errorf("expected primitive type, got: %s", s.Schema.Type))
+	}
+
+	// Get zero value for the schema type
+	zero := fmt.Sprintf("%v", s.Schema.Type.Zero())
+
+	// Get string representation of the default value
+	sv := fmt.Sprintf("%v", dv)
+
+	// Suppress diff if the new value (ie value from HCL config) is not set and
+	// the old value (ie value from state / platform) is equal to the default value.
+	s.Schema.DiffSuppressFunc = func(k, old, new string, d *schema.ResourceData) bool {
+		if new == zero && old == sv {
+			return true
+		}
+		return false
+	}
+	return s
+}
+
 func (s *CustomizableSchema) SetCustomSuppressDiff(suppressor func(k, old, new string, d *schema.ResourceData) bool) *CustomizableSchema {
 	s.Schema.DiffSuppressFunc = suppressor
 	return s

common/customizable_schema_test.go

@@ -43,6 +43,99 @@ func TestCustomizableSchemaSetSuppressDiff(t *testing.T) {
 	assert.Truef(t, testCustomizableSchemaScm["non_optional"].DiffSuppressFunc != nil, "DiffSuppressfunc should be set in field: non_optional")
 }
 
+func TestCustomizableSchemaSetCustomSuppressDiffWithDefault(t *testing.T) {
+	tc := []struct {
+		name      string
+		fieldName string
+		dv        any
+		old       string
+		new       string
+		result    bool
+	}{
+		{
+			name:      "default string",
+			fieldName: "string",
+			dv:        "foobar",
+			old:       "foobar",
+			new:       "",
+			result:    true,
+		},
+		{
+			name:      "default int",
+			fieldName: "integer",
+			dv:        123,
+			old:       "123",
+			new:       "0",
+			result:    true,
+		},
+		{
+			name:      "default bool",
+			fieldName: "bool",
+			dv:        true,
+			old:       "true",
+			new:       "false",
+			result:    true,
+		},
+		{
+			name:      "default float",
+			fieldName: "float",
+			dv:        123.456,
+			old:       "123.456",
+			new:       "0",
+			result:    true,
+		},
+		{
+			name:      "non default string",
+			fieldName: "string",
+			dv:        "foobar",
+			old:       "non-default-val",
+			new:       "",
+			result:    false,
+		},
+		{
+			name:      "non default int",
+			fieldName: "integer",
+			dv:        123,
+			old:       "non-default-val",
+			new:       "0",
+			result:    false,
+		},
+		{
+			name:      "non default bool",
+			fieldName: "bool",
+			dv:        true,
+			old:       "non-default-val",
+			new:       "false",
+			result:    false,
+		},
+		{
+			name:      "non default float",
+			fieldName: "float",
+			dv:        123.456,
+			old:       "non-default-val",
+			new:       "0",
+			result:    false,
+		},
+		{
+			name:      "override in config",
+			fieldName: "string",
+			dv:        "foobar",
+			old:       "foobar",
+			new:       "new-val-in-config",
+			result:    false,
+		},
+	}
+
+	for _, tt := range tc {
+		t.Run(tt.name, func(t *testing.T) {
+			CustomizeSchemaPath(testCustomizableSchemaScm, tt.fieldName).SetSuppressDiffWithDefault(tt.dv)
+			assert.Truef(t, testCustomizableSchemaScm[tt.fieldName].DiffSuppressFunc != nil, "DiffSuppressFunc should be set in field: %s", tt.fieldName)
+
+			assert.Equal(t, tt.result, testCustomizableSchemaScm[tt.fieldName].DiffSuppressFunc("", tt.old, tt.new, nil))
+		})
+	}
+}
+
 func TestCustomizableSchemaSetCustomSuppressDiff(t *testing.T) {
 	CustomizeSchemaPath(testCustomizableSchemaScm, "non_optional").SetCustomSuppressDiff(diffSuppressor("test", CustomizeSchemaPath(testCustomizableSchemaScm, "non_optional").Schema))
 	assert.Truef(t, testCustomizableSchemaScm["non_optional"].DiffSuppressFunc != nil, "DiffSuppressfunc should be set in field: non_optional")

jobs/resource_job.go

@@ -133,7 +133,7 @@ type ForEachNestedTask struct {
 	TaskKey     string                `json:"task_key,omitempty"`
 	Description string                `json:"description,omitempty"`
 	DependsOn   []jobs.TaskDependency `json:"depends_on,omitempty"`
-	RunIf       string                `json:"run_if,omitempty" tf:"suppress_diff"`
+	RunIf       string                `json:"run_if,omitempty"`
 
 	ExistingClusterID string              `json:"existing_cluster_id,omitempty" tf:"group:cluster_type"`
 	NewCluster        *clusters.Cluster   `json:"new_cluster,omitempty" tf:"group:cluster_type"`
@@ -213,7 +213,7 @@ type JobTaskSettings struct {
 	TaskKey     string                `json:"task_key,omitempty"`
 	Description string                `json:"description,omitempty"`
 	DependsOn   []jobs.TaskDependency `json:"depends_on,omitempty"`
-	RunIf       string                `json:"run_if,omitempty" tf:"suppress_diff"`
+	RunIf       string                `json:"run_if,omitempty"`
 
 	ExistingClusterID string              `json:"existing_cluster_id,omitempty" tf:"group:cluster_type"`
 	NewCluster        *clusters.Cluster   `json:"new_cluster,omitempty" tf:"group:cluster_type"`
@@ -775,6 +775,10 @@ var jobSchema = common.StructToSchema(JobSettings{},
 		fixWebhookNotifications(s)
 		fixWebhookNotifications(common.MustSchemaMap(s, "task"))
 
+		// Suppress diff if the platform returns ALL_SUCCESS for run_if in a task
+		common.CustomizeSchemaPath(s, "task", "run_if").SetSuppressDiffWithDefault(jobs.RunIfAllSuccess)
+		common.CustomizeSchemaPath(s, "task", "for_each_task", "task", "run_if").SetSuppressDiffWithDefault(jobs.RunIfAllSuccess)
+
 		return s
 	})
 

jobs/resource_job_test.go

@@ -2111,6 +2111,183 @@ func TestResourceJobUpdate(t *testing.T) {
 	assert.Equal(t, "Featurizer New", d.Get("name"))
 }
 
+func TestResourceJobUpdate_RunIfSuppressesDiffIfAllSuccess(t *testing.T) {
+	_, err := qa.ResourceFixture{
+		Fixtures: []qa.HTTPFixture{
+			{
+				Method:   "POST",
+				Resource: "/api/2.1/jobs/reset",
+				ExpectedRequest: UpdateJobRequest{
+					JobID: 789,
+					NewSettings: &JobSettings{
+						MaxConcurrentRuns: 1,
+						Tasks: []JobTaskSettings{
+							{
+								NotebookTask: &NotebookTask{
+									NotebookPath: "/foo/bar",
+								},
+								// The diff is suppressed here. The API payload
+								// contains the "run_if" value from the terraform
+								// state.
+								RunIf: "ALL_SUCCESS",
+							},
+							{
+								ForEachTask: &ForEachTask{
+									Inputs: "abc",
+									Task: ForEachNestedTask{
+										NotebookTask: &NotebookTask{NotebookPath: "/bar/foo"},
+										// The diff is suppressed here. Value is from
+										// the terraform state.
+										RunIf: "ALL_SUCCESS",
+									},
+								},
+							},
+						},
+						Name: "My job",
+					},
+				},
+			},
+			{
+				Method:   "GET",
+				Resource: "/api/2.1/jobs/get?job_id=789",
+				Response: Job{
+					JobID: 789,
+					Settings: &JobSettings{
+						Name: "My job",
+						Tasks: []JobTaskSettings{
+							{
+								NotebookTask: &NotebookTask{NotebookPath: "/foo/bar"},
+								RunIf:        "ALL_SUCCESS",
+							},
+							{
+								ForEachTask: &ForEachTask{
+									Inputs: "abc",
+									Task: ForEachNestedTask{
+										NotebookTask: &NotebookTask{NotebookPath: "/bar/foo"},
+										RunIf:        "ALL_SUCCESS",
+									},
+								},
+							},
+						},
+					},
+				},
+			},
+		},
+		ID:       "789",
+		Update:   true,
+		Resource: ResourceJob(),
+		InstanceState: map[string]string{
+			"task.0.run_if":                        "ALL_SUCCESS",
+			"task.1.for_each_task.0.task.0.run_if": "ALL_SUCCESS",
+		},
+		HCL: `
+		name = "My job"
+		task {
+			notebook_task {
+				notebook_path = "/foo/bar"
+			}
+		}
+		task {
+			for_each_task {
+				inputs = "abc"
+				task {
+					notebook_task {
+						notebook_path = "/bar/foo"
+					}
+				}
+			}
+		}`,
+	}.Apply(t)
+	assert.NoError(t, err)
+}
+
+func TestResourceJobUpdate_RunIfDoesNotSuppressIfNotAllSuccess(t *testing.T) {
+	_, err := qa.ResourceFixture{
+		Fixtures: []qa.HTTPFixture{
+			{
+				Method:   "POST",
+				Resource: "/api/2.1/jobs/reset",
+				ExpectedRequest: UpdateJobRequest{
+					JobID: 789,
+					NewSettings: &JobSettings{
+						MaxConcurrentRuns: 1,
+						Tasks: []JobTaskSettings{
+							{
+								NotebookTask: &NotebookTask{
+									NotebookPath: "/foo/bar",
+								},
+								// The diff is not suppressed here. Thus the API payload
+								// explicitly does not set run_if here, to unset it in the
+								// job definition.
+								// RunIf is not set, as implied from the HCL config.
+							},
+							{
+								ForEachTask: &ForEachTask{
+									Inputs: "abc",
+									Task: ForEachNestedTask{
+										NotebookTask: &NotebookTask{NotebookPath: "/bar/foo"},
+										// The diff is not suppressed. RunIf is
+										// not set, as implied from the HCL config.
+									},
+								},
+							},
+						},
+						Name: "My job",
+					},
+				},
+			},
+			{
+				Method:   "GET",
+				Resource: "/api/2.1/jobs/get?job_id=789",
+				Response: Job{
+					JobID: 789,
+					Settings: &JobSettings{
+						Name: "My job",
+						Tasks: []JobTaskSettings{
+							{
+								NotebookTask: &NotebookTask{NotebookPath: "/foo/bar"},
+								RunIf:        "AT_LEAST_ONE_FAILED",
+							},
+							{
+								ForEachTask: &ForEachTask{
+									Task: ForEachNestedTask{
+										RunIf: "AT_LEAST_ONE_FAILED",
+									},
+								},
+							},
+						},
+					},
+				},
+			},
+		},
+		ID:     "789",
+		Update: true,
+		InstanceState: map[string]string{
+			"task.0.run_if":                        "AT_LEAST_ONE_FAILED",
+			"task.1.for_each_task.0.task.0.run_if": "AT_LEAST_ONE_FAILED",
+		},
+		Resource: ResourceJob(),
+		HCL: `
+		name = "My job"
+		task {
+			notebook_task {
+				notebook_path = "/foo/bar"
+			}
+		}
+		task {
+			for_each_task {
+				inputs = "abc"
+				task {
+					notebook_task {
+						notebook_path = "/bar/foo"
+					}
+				}
+			}
+		}`,
+	}.Apply(t)
+	assert.NoError(t, err)
+}
+
 func TestResourceJobUpdate_NodeTypeToInstancePool(t *testing.T) {
 	d, err := qa.ResourceFixture{
 		Fixtures: []qa.HTTPFixture{