Skip to content

Commit 9f5fae0

Browse files
prashantvaschepis
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 9d476d4 commit 9f5fae0

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
@@ -1149,10 +1149,10 @@ func (m schemaMap) diff(
11491149
}
11501150

11511151
logging.HelperSchemaDebug(ctx, "Ignoring change due to DiffSuppressFunc", map[string]interface{}{logging.KeyAttributePath: attrK})
1152-
attrV = &terraform.ResourceAttrDiff{
1153-
Old: attrV.Old,
1154-
New: attrV.Old,
1155-
}
1152+
// If the diff is suppressed, we want Old = New, but retain the other properties.
1153+
updatedAttr := *attrV
1154+
updatedAttr.New = attrV.Old
1155+
attrV = &updatedAttr
11561156
}
11571157
}
11581158
diff.Attributes[attrK] = attrV

helper/schema/schema_test.go

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

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

0 commit comments

Comments
 (0)