From d7ff6899ee2a8c508051e414f00f49ab01988074 Mon Sep 17 00:00:00 2001
From: Prashant Varanasi <prashant@prashantv.com>
Date: Fri, 26 Aug 2022 12:07:44 -0700
Subject: [PATCH] Fix for TypeSet applying with unexpected empty element

Fixes hashicorp/terraform-plugin-sdk#895

When calculating a diff, elements in sets are tracked as a delete of all
old attributes with `NewRemoved = true`, and a create of all new
attributes. When `DiffSuppressFunc` is used for an attribute, the
`ResourceAttrDiff` is replaced with `New = Old` (no-op):
https://github.com/hashicorp/terraform-plugin-sdk/blob/fa8d313665945816f6eb6c79182e4abdc1540504/helper/schema/schema.go#L1144

However, this also drops all other metadata about the attr diff, such as
`NewRemoved`. This ends up affecting the `InstanceDiff.applyBlockDiff`
which expects to drop removed items:
https://github.com/hashicorp/terraform-plugin-sdk/blob/e14d3b611f2e257d2a0862e5ea0f90ea96fd5bf8/terraform/diff.go#L207

All attributes of the removed set element get removed here, except those
with `DiffSuppressFunc` since the `NewRemoved` attribute was dropped.

Instead of dropping the metadata about the attr diff, clone the original
attr diff, and just set `New = Old`.

The added test fails without this fix:
```
=== RUN   TestSchemaMap_Diff/30-Set_with_DiffSuppressFunc
    schema_test.go:3188: expected:
        *terraform.InstanceDiff{ [...]"rule.80.duration":*terraform.ResourceAttrDiff{Old:"", New:"", NewComputed:false, NewRemoved:true, [...]

        got:
        *terraform.InstanceDiff{ [...]"rule.80.duration":*terraform.ResourceAttrDiff{Old:"", New:"", NewComputed:false, [...] NewRemoved:false, [...]
```

Previously, `NewRemoved` was set to false for the sustain field even though it belonged to an element being removed.
---
 helper/schema/schema.go      |  8 ++--
 helper/schema/schema_test.go | 74 ++++++++++++++++++++++++++++++++++++
 2 files changed, 78 insertions(+), 4 deletions(-)

diff --git a/helper/schema/schema.go b/helper/schema/schema.go
index 22fe196b775..290170386cc 100644
--- a/helper/schema/schema.go
+++ b/helper/schema/schema.go
@@ -1149,10 +1149,10 @@ func (m schemaMap) diff(
 				}
 
 				logging.HelperSchemaDebug(ctx, "Ignoring change due to DiffSuppressFunc", map[string]interface{}{logging.KeyAttributePath: attrK})
-				attrV = &terraform.ResourceAttrDiff{
-					Old: attrV.Old,
-					New: attrV.Old,
-				}
+				// If the diff is suppressed, we want Old = New, but retain the other properties.
+				updatedAttr := *attrV
+				updatedAttr.New = attrV.Old
+				attrV = &updatedAttr
 			}
 		}
 		diff.Attributes[attrK] = attrV
diff --git a/helper/schema/schema_test.go b/helper/schema/schema_test.go
index 0dec6bb27eb..0f8efeab585 100644
--- a/helper/schema/schema_test.go
+++ b/helper/schema/schema_test.go
@@ -1155,6 +1155,80 @@ func TestSchemaMap_Diff(t *testing.T) {
 			Err: false,
 		},
 
+		{
+			Name: "Set with DiffSuppressFunc",
+			Schema: map[string]*Schema{
+				"rule": {
+					Type:     TypeSet,
+					Required: true,
+					Elem: &Resource{
+						Schema: map[string]*Schema{
+							"port": {
+								Type:     TypeInt,
+								Required: true,
+							},
+							"duration": {
+								Type:     TypeString,
+								Optional: true,
+								DiffSuppressFunc: func(k, oldValue, newValue string, d *ResourceData) bool {
+									// Adding a DiffSuppressFunc to an element in a set changes behaviour.
+									// The actual suppress func doesn't matter.
+									return oldValue == newValue
+								},
+							},
+						},
+					},
+					Set: func(v interface{}) int {
+						m := v.(map[string]interface{})
+						port := m["port"].(int)
+						return port
+					},
+				},
+			},
+
+			State: &terraform.InstanceState{
+				Attributes: map[string]string{
+					"rule.#":           "1",
+					"rule.80.port":     "80",
+					"rule.80.duration": "",
+				},
+			},
+
+			Config: map[string]interface{}{
+				"rule": []interface{}{
+					map[string]interface{}{
+						"port":     90,
+						"duration": "30s",
+					},
+				},
+			},
+
+			Diff: &terraform.InstanceDiff{
+				Attributes: map[string]*terraform.ResourceAttrDiff{
+					"rule.80.port": {
+						Old:        "80",
+						New:        "0",
+						NewRemoved: true,
+					},
+					"rule.80.duration": {
+						Old:        "",
+						New:        "",
+						NewRemoved: true,
+					},
+					"rule.90.port": {
+						Old: "",
+						New: "90",
+					},
+					"rule.90.duration": {
+						Old: "",
+						New: "30s",
+					},
+				},
+			},
+
+			Err: false,
+		},
+
 		{
 			Name: "List of structure decode",
 			Schema: map[string]*Schema{