Merge pull request #8585 from hashicorp/f-diff-suppression

helper/schema: Add diff suppression callback

builtin/providers/digitalocean/resource_digitalocean_ssh_key.go

@@ -21,26 +21,24 @@ func resourceDigitalOceanSSHKey() *schema.Resource {
 		},
 
 		Schema: map[string]*schema.Schema{
-			"id": &schema.Schema{
+			"id": {
 				Type:     schema.TypeString,
 				Computed: true,
 			},
 
-			"name": &schema.Schema{
+			"name": {
 				Type:     schema.TypeString,
 				Required: true,
 			},
 
-			"public_key": &schema.Schema{
+			"public_key": {
-				Type:     schema.TypeString,
+				Type:             schema.TypeString,
-				Required: true,
+				Required:         true,
-				ForceNew: true,
+				ForceNew:         true,
-				StateFunc: func(val interface{}) string {
+				DiffSuppressFunc: resourceDigitalOceanSSHKeyPublicKeyDiffSuppress,
-					return strings.TrimSpace(val.(string))
-				},
 			},
 
-			"fingerprint": &schema.Schema{
+			"fingerprint": {
 				Type:     schema.TypeString,
 				Computed: true,
 			},
@@ -48,6 +46,10 @@ func resourceDigitalOceanSSHKey() *schema.Resource {
 	}
 }
 
+func resourceDigitalOceanSSHKeyPublicKeyDiffSuppress(k, old, new string, d *schema.ResourceData) bool {
+	return strings.TrimSpace(old) == strings.TrimSpace(new)
+}
+
 func resourceDigitalOceanSSHKeyCreate(d *schema.ResourceData, meta interface{}) error {
 	client := meta.(*godo.Client)
 

helper/schema/schema.go

@@ -52,6 +52,15 @@ type Schema struct {
 	Optional bool
 	Required bool
 
+	// If this is non-nil, the provided function will be used during diff
+	// of this field. If this is nil, a default diff for the type of the
+	// schema will be used.
+	//
+	// This allows comparison based on something other than primitive, list
+	// or map equality - for example SSH public keys may be considered
+	// equivalent regardless of trailing whitespace.
+	DiffSuppressFunc SchemaDiffSuppressFunc
+
 	// If this is non-nil, then this will be a default value that is used
 	// when this item is not set in the configuration/state.
 	//
@@ -153,6 +162,13 @@ type Schema struct {
 	Sensitive bool
 }
 
+// SchemaDiffSuppresFunc is a function which can be used to determine
+// whether a detected diff on a schema element is "valid" or not, and
+// suppress it from the plan if necessary.
+//
+// Return true if the diff should be suppressed, false to retain it.
+type SchemaDiffSuppressFunc func(k, old, new string, d *ResourceData) bool
+
 // SchemaDefaultFunc is a function called to return a default value for
 // a field.
 type SchemaDefaultFunc func() (interface{}, error)
@@ -603,20 +619,32 @@ func (m schemaMap) diff(
 	diff *terraform.InstanceDiff,
 	d *ResourceData,
 	all bool) error {
+
+	unsupressedDiff := new(terraform.InstanceDiff)
+	unsupressedDiff.Attributes = make(map[string]*terraform.ResourceAttrDiff)
+
 	var err error
 	switch schema.Type {
 	case TypeBool, TypeInt, TypeFloat, TypeString:
-		err = m.diffString(k, schema, diff, d, all)
+		err = m.diffString(k, schema, unsupressedDiff, d, all)
 	case TypeList:
-		err = m.diffList(k, schema, diff, d, all)
+		err = m.diffList(k, schema, unsupressedDiff, d, all)
 	case TypeMap:
-		err = m.diffMap(k, schema, diff, d, all)
+		err = m.diffMap(k, schema, unsupressedDiff, d, all)
 	case TypeSet:
-		err = m.diffSet(k, schema, diff, d, all)
+		err = m.diffSet(k, schema, unsupressedDiff, d, all)
 	default:
 		err = fmt.Errorf("%s: unknown type %#v", k, schema.Type)
 	}
 
+	for attrK, attrV := range unsupressedDiff.Attributes {
+		if schema.DiffSuppressFunc != nil && schema.DiffSuppressFunc(attrK, attrV.Old, attrV.New, d) {
+			continue
+		}
+
+		diff.Attributes[attrK] = attrV
+	}
+
 	return err
 }
 

helper/schema/schema_test.go

@@ -2939,6 +2939,92 @@ func TestSchemaMap_InternalValidate(t *testing.T) {
 
 }
 
+func TestSchemaMap_DiffSuppress(t *testing.T) {
+	cases := map[string]struct {
+		Schema          map[string]*Schema
+		State           *terraform.InstanceState
+		Config          map[string]interface{}
+		ConfigVariables map[string]ast.Variable
+		ExpectedDiff    *terraform.InstanceDiff
+		Err             bool
+	}{
+		"#0 - Suppress otherwise valid diff by returning true": {
+			Schema: map[string]*Schema{
+				"availability_zone": {
+					Type:     TypeString,
+					Optional: true,
+					DiffSuppressFunc: func(k, old, new string, d *ResourceData) bool {
+						// Always suppress any diff
+						return true
+					},
+				},
+			},
+
+			State: nil,
+
+			Config: map[string]interface{}{
+				"availability_zone": "foo",
+			},
+
+			ExpectedDiff: nil,
+
+			Err: false,
+		},
+		"#1 - Don't suppress diff by returning false": {
+			Schema: map[string]*Schema{
+				"availability_zone": {
+					Type:     TypeString,
+					Optional: true,
+					DiffSuppressFunc: func(k, old, new string, d *ResourceData) bool {
+						// Always suppress any diff
+						return false
+					},
+				},
+			},
+
+			State: nil,
+
+			Config: map[string]interface{}{
+				"availability_zone": "foo",
+			},
+
+			ExpectedDiff: &terraform.InstanceDiff{
+				Attributes: map[string]*terraform.ResourceAttrDiff{
+					"availability_zone": {
+						Old: "",
+						New: "foo",
+					},
+				},
+			},
+
+			Err: false,
+		},
+	}
+
+	for tn, tc := range cases {
+		c, err := config.NewRawConfig(tc.Config)
+		if err != nil {
+			t.Fatalf("#%q err: %s", tn, err)
+		}
+
+		if len(tc.ConfigVariables) > 0 {
+			if err := c.Interpolate(tc.ConfigVariables); err != nil {
+				t.Fatalf("#%q err: %s", tn, err)
+			}
+		}
+
+		d, err := schemaMap(tc.Schema).Diff(
+			tc.State, terraform.NewResourceConfig(c))
+		if err != nil != tc.Err {
+			t.Fatalf("#%q err: %s", tn, err)
+		}
+
+		if !reflect.DeepEqual(tc.ExpectedDiff, d) {
+			t.Fatalf("#%q:\n\nexpected:\n%#v\n\ngot:\n%#v", tn, tc.ExpectedDiff, d)
+		}
+	}
+}
+
 func TestSchemaMap_Validate(t *testing.T) {
 	cases := map[string]struct {
 		Schema   map[string]*Schema