helper/schema: Add diff suppression callback

This commit adds a new callback, DiffSuppressFunc, to  the schema.Schema
structure. If set for a given schema, a callback to the user-supplied
function will be made for each attribute for which the default
type-based diff mechanism produces an attribute diff. Returning `true`
from the callback will suppress the diff (i.e. pretend there was no
diff), and returning false will retain it as part of the plan.

There are a number of motivating examples for this - one of which is
included as an example:

1. On SSH public keys, trailing whitespace does not matter in many
   cases - and in some cases it is added by provider APIs. For
   digitalocean_ssh_key resources we previously had a StateFunc that
   trimmed the whitespace - we now have a DiffSuppressFunc which
   verifies whether the trimmed strings are equivalent.

2. IAM policy equivalence for AWS. A good proportion of AWS issues
   relate to IAM policies which have been "normalized" (used loosely)
   by the IAM API endpoints. This can make the JSON strings differ
   from those generated by iam_policy_document resources or template
   files, even though the semantics are the same (for example,
   reordering of `bucket-prefix/` and `bucket-prefix/*` in an S3
   bucket policy. DiffSupressFunc can be used to test for semantic
   equivalence rather than pure text equivalence, but without having to
   deal with the complexity associated with a full "provider-land" diff
   implementation without helper/schema.

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