Skip to content

Commit 325bdda

Browse files
prashantvmarcushill
authored andcommitted
Fix for TypeSet applying with unexpected empty element (#2)
hashicorp/terraform-plugin-sdk#895 When calculating a diff, sets are added as delete of all old attributes and a create of all new attributes. When DiffSuppressFunc is used, the delete attribute drops the `NewRemoved` field, which results in sending a diff that ends up creating a new empty set element. Without the fix, the test fails: ``` === 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.
1 parent d636e58 commit 325bdda

File tree

2 files changed

+78
-4
lines changed

2 files changed

+78
-4
lines changed

helper/schema/schema.go

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1320,10 +1320,10 @@ func (m schemaMap) diff(
13201320
}
13211321

13221322
logging.HelperSchemaDebug(ctx, "Ignoring change due to DiffSuppressFunc", map[string]interface{}{logging.KeyAttributePath: attrK})
1323-
attrV = &terraform.ResourceAttrDiff{
1324-
Old: attrV.Old,
1325-
New: attrV.Old,
1326-
}
1323+
// If the diff is suppressed, we want Old = New, but retain the other properties.
1324+
updatedAttr := *attrV
1325+
updatedAttr.New = attrV.Old
1326+
attrV = &updatedAttr
13271327
}
13281328
}
13291329
diff.Attributes[attrK] = attrV

helper/schema/schema_test.go

Lines changed: 74 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1159,6 +1159,80 @@ func TestSchemaMap_Diff(t *testing.T) {
11591159
Err: false,
11601160
},
11611161

1162+
{
1163+
Name: "Set with DiffSuppressFunc",
1164+
Schema: map[string]*Schema{
1165+
"rule": {
1166+
Type: TypeSet,
1167+
Required: true,
1168+
Elem: &Resource{
1169+
Schema: map[string]*Schema{
1170+
"port": {
1171+
Type: TypeInt,
1172+
Required: true,
1173+
},
1174+
"duration": {
1175+
Type: TypeString,
1176+
Optional: true,
1177+
DiffSuppressFunc: func(k, oldValue, newValue string, d *ResourceData) bool {
1178+
// Adding a DiffSuppressFunc to an element in a set changes behaviour.
1179+
// The actual suppress func doesn't matter.
1180+
return oldValue == newValue
1181+
},
1182+
},
1183+
},
1184+
},
1185+
Set: func(v interface{}) int {
1186+
m := v.(map[string]interface{})
1187+
port := m["port"].(int)
1188+
return port
1189+
},
1190+
},
1191+
},
1192+
1193+
State: &terraform.InstanceState{
1194+
Attributes: map[string]string{
1195+
"rule.#": "1",
1196+
"rule.80.port": "80",
1197+
"rule.80.duration": "",
1198+
},
1199+
},
1200+
1201+
Config: map[string]interface{}{
1202+
"rule": []interface{}{
1203+
map[string]interface{}{
1204+
"port": 90,
1205+
"duration": "30s",
1206+
},
1207+
},
1208+
},
1209+
1210+
Diff: &terraform.InstanceDiff{
1211+
Attributes: map[string]*terraform.ResourceAttrDiff{
1212+
"rule.80.port": {
1213+
Old: "80",
1214+
New: "0",
1215+
NewRemoved: true,
1216+
},
1217+
"rule.80.duration": {
1218+
Old: "",
1219+
New: "",
1220+
NewRemoved: true,
1221+
},
1222+
"rule.90.port": {
1223+
Old: "",
1224+
New: "90",
1225+
},
1226+
"rule.90.duration": {
1227+
Old: "",
1228+
New: "30s",
1229+
},
1230+
},
1231+
},
1232+
1233+
Err: false,
1234+
},
1235+
11621236
{
11631237
Name: "List of structure decode",
11641238
Schema: map[string]*Schema{

0 commit comments

Comments
 (0)