Skip to content

Commit 0e9d4ce

Browse files
trodgetrodge
authored
Add support for diffing field sets (GoogleCloudPlatform#12330)
1 parent 75ef317 commit 0e9d4ce

File tree

4 files changed

+280
-40
lines changed

4 files changed

+280
-40
lines changed

‎tools/diff-processor/diff/diff.go

+148-39
Original file line numberDiff line numberDiff line change
@@ -1,12 +1,13 @@
11
package diff
22

33
import (
4+
"maps"
45
"reflect"
6+
"slices"
7+
"strings"
58

69
"github.com/google/go-cmp/cmp"
7-
"github.com/google/go-cmp/cmp/cmpopts"
810
"github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema"
9-
"golang.org/x/exp/maps"
1011
)
1112

1213
// SchemaDiff is a nested map with resource names as top-level keys.
@@ -15,8 +16,35 @@ type SchemaDiff map[string]ResourceDiff
1516
type ResourceDiff struct {
1617
ResourceConfig ResourceConfigDiff
1718
Fields map[string]FieldDiff
19+
FieldSets ResourceFieldSetsDiff
1820
}
1921

22+
type ResourceFieldSetsDiff struct {
23+
Old ResourceFieldSets
24+
New ResourceFieldSets
25+
}
26+
27+
type ResourceFieldSetsDiffWithKeys struct {
28+
Old ResourceFieldSetsWithKeys
29+
New ResourceFieldSetsWithKeys
30+
}
31+
32+
type ResourceFieldSets struct {
33+
ConflictsWith []FieldSet
34+
ExactlyOneOf []FieldSet
35+
AtLeastOneOf []FieldSet
36+
RequiredWith []FieldSet
37+
}
38+
39+
type ResourceFieldSetsWithKeys struct {
40+
ConflictsWith map[string]FieldSet
41+
ExactlyOneOf map[string]FieldSet
42+
AtLeastOneOf map[string]FieldSet
43+
RequiredWith map[string]FieldSet
44+
}
45+
46+
type FieldSet map[string]struct{}
47+
2048
type ResourceConfigDiff struct {
2149
Old *schema.Resource
2250
New *schema.Resource
@@ -29,7 +57,7 @@ type FieldDiff struct {
2957

3058
func ComputeSchemaDiff(oldResourceMap, newResourceMap map[string]*schema.Resource) SchemaDiff {
3159
schemaDiff := make(SchemaDiff)
32-
for resource, _ := range union(maps.Keys(oldResourceMap), maps.Keys(newResourceMap)) {
60+
for resource := range union(oldResourceMap, newResourceMap) {
3361
// Compute diff between old and new resources and fields.
3462
// TODO: add support for computing diff between resource configs, not just whether the
3563
// resource was added/removed. b/300114839
@@ -47,34 +75,23 @@ func ComputeSchemaDiff(oldResourceMap, newResourceMap map[string]*schema.Resourc
4775
}
4876

4977
resourceDiff.Fields = make(map[string]FieldDiff)
50-
for key, _ := range union(maps.Keys(flattenedOldSchema), maps.Keys(flattenedNewSchema)) {
78+
fieldSetsDiffWithKeys := ResourceFieldSetsDiffWithKeys{}
79+
for key := range union(flattenedOldSchema, flattenedNewSchema) {
5180
oldField := flattenedOldSchema[key]
5281
newField := flattenedNewSchema[key]
53-
if fieldChanged(oldField, newField) {
54-
resourceDiff.Fields[key] = FieldDiff{
55-
Old: oldField,
56-
New: newField,
57-
}
82+
if fieldDiff, fieldSetsDiff, changed := diffFields(oldField, newField, key); changed {
83+
resourceDiff.Fields[key] = fieldDiff
84+
fieldSetsDiffWithKeys = mergeFieldSetsDiff(fieldSetsDiffWithKeys, fieldSetsDiff)
5885
}
5986
}
87+
resourceDiff.FieldSets = removeFieldSetsDiffKeys(fieldSetsDiffWithKeys)
6088
if len(resourceDiff.Fields) > 0 || !cmp.Equal(resourceDiff.ResourceConfig.Old, resourceDiff.ResourceConfig.New) {
6189
schemaDiff[resource] = resourceDiff
6290
}
6391
}
6492
return schemaDiff
6593
}
6694

67-
func union(keys1, keys2 []string) map[string]struct{} {
68-
allKeys := make(map[string]struct{})
69-
for _, key := range keys1 {
70-
allKeys[key] = struct{}{}
71-
}
72-
for _, key := range keys2 {
73-
allKeys[key] = struct{}{}
74-
}
75-
return allKeys
76-
}
77-
7895
func flattenSchema(parentKey string, schemaObj map[string]*schema.Schema) map[string]*schema.Schema {
7996
flattened := make(map[string]*schema.Schema)
8097

@@ -96,16 +113,48 @@ func flattenSchema(parentKey string, schemaObj map[string]*schema.Schema) map[st
96113
return flattened
97114
}
98115

99-
func fieldChanged(oldField, newField *schema.Schema) bool {
116+
func diffFields(oldField, newField *schema.Schema, fieldName string) (FieldDiff, ResourceFieldSetsDiff, bool) {
100117
// If either field is nil, it is changed; if both are nil (which should never happen) it's not
101118
if oldField == nil && newField == nil {
102-
return false
119+
return FieldDiff{}, ResourceFieldSetsDiff{}, false
120+
}
121+
122+
oldFieldSets := fieldSets(oldField, fieldName)
123+
newFieldSets := fieldSets(newField, fieldName)
124+
125+
fieldDiff := FieldDiff{
126+
Old: oldField,
127+
New: newField,
128+
}
129+
fieldSetsDiff := ResourceFieldSetsDiff{
130+
Old: oldFieldSets,
131+
New: newFieldSets,
103132
}
104133
if oldField == nil || newField == nil {
105-
return true
134+
return fieldDiff, fieldSetsDiff, true
106135
}
107136
// Check if any basic Schema struct fields have changed.
108137
// https://github.com/hashicorp/terraform-plugin-sdk/blob/v2.24.0/helper/schema/schema.go#L44
138+
if basicSchemaChanged(oldField, newField) {
139+
return fieldDiff, fieldSetsDiff, true
140+
}
141+
142+
if !cmp.Equal(oldFieldSets, newFieldSets) {
143+
return fieldDiff, fieldSetsDiff, true
144+
}
145+
146+
if elemChanged(oldField, newField) {
147+
return fieldDiff, fieldSetsDiff, true
148+
}
149+
150+
if funcsChanged(oldField, newField) {
151+
return fieldDiff, fieldSetsDiff, true
152+
}
153+
154+
return FieldDiff{}, ResourceFieldSetsDiff{}, false
155+
}
156+
157+
func basicSchemaChanged(oldField, newField *schema.Schema) bool {
109158
if oldField.Type != newField.Type {
110159
return true
111160
}
@@ -148,26 +197,35 @@ func fieldChanged(oldField, newField *schema.Schema) bool {
148197
if oldField.Sensitive != newField.Sensitive {
149198
return true
150199
}
200+
return false
201+
}
151202

152-
// Compare slices
153-
less := func(a, b string) bool { return a < b }
154-
155-
if (len(oldField.ConflictsWith) > 0 || len(newField.ConflictsWith) > 0) && !cmp.Equal(oldField.ConflictsWith, newField.ConflictsWith, cmpopts.SortSlices(less)) {
156-
return true
203+
func fieldSets(field *schema.Schema, fieldName string) ResourceFieldSets {
204+
if field == nil {
205+
return ResourceFieldSets{}
157206
}
158-
159-
if (len(oldField.ExactlyOneOf) > 0 || len(newField.ExactlyOneOf) > 0) && !cmp.Equal(oldField.ExactlyOneOf, newField.ExactlyOneOf, cmpopts.SortSlices(less)) {
160-
return true
207+
var conflictsWith, exactlyOneOf, atLeastOneOf, requiredWith []FieldSet
208+
if len(field.ConflictsWith) > 0 {
209+
conflictsWith = []FieldSet{sliceToSetRemoveZeroPadding(append(field.ConflictsWith, fieldName))}
161210
}
162-
163-
if (len(oldField.AtLeastOneOf) > 0 || len(newField.AtLeastOneOf) > 0) && !cmp.Equal(oldField.AtLeastOneOf, newField.AtLeastOneOf, cmpopts.SortSlices(less)) {
164-
return true
211+
if len(field.ExactlyOneOf) > 0 {
212+
exactlyOneOf = []FieldSet{sliceToSetRemoveZeroPadding(append(field.ExactlyOneOf, fieldName))}
165213
}
166-
167-
if (len(oldField.RequiredWith) > 0 || len(newField.RequiredWith) > 0) && !cmp.Equal(oldField.RequiredWith, newField.RequiredWith, cmpopts.SortSlices(less)) {
168-
return true
214+
if len(field.AtLeastOneOf) > 0 {
215+
atLeastOneOf = []FieldSet{sliceToSetRemoveZeroPadding(append(field.AtLeastOneOf, fieldName))}
216+
}
217+
if len(field.RequiredWith) > 0 {
218+
requiredWith = []FieldSet{sliceToSetRemoveZeroPadding(append(field.RequiredWith, fieldName))}
169219
}
220+
return ResourceFieldSets{
221+
ConflictsWith: conflictsWith,
222+
ExactlyOneOf: exactlyOneOf,
223+
AtLeastOneOf: atLeastOneOf,
224+
RequiredWith: requiredWith,
225+
}
226+
}
170227

228+
func elemChanged(oldField, newField *schema.Schema) bool {
171229
// Check if Elem changed (unless old and new both represent nested fields)
172230
if (oldField.Elem == nil && newField.Elem != nil) || (oldField.Elem != nil && newField.Elem == nil) {
173231
return true
@@ -183,12 +241,15 @@ func fieldChanged(oldField, newField *schema.Schema) bool {
183241
return true
184242
}
185243
if !oldIsResource && !newIsResource {
186-
if fieldChanged(oldField.Elem.(*schema.Schema), newField.Elem.(*schema.Schema)) {
244+
if _, _, changed := diffFields(oldField.Elem.(*schema.Schema), newField.Elem.(*schema.Schema), ""); changed {
187245
return true
188246
}
189247
}
190248
}
249+
return false
250+
}
191251

252+
func funcsChanged(oldField, newField *schema.Schema) bool {
192253
// Check if any Schema struct fields that are functions have changed
193254
if funcChanged(oldField.DiffSuppressFunc, newField.DiffSuppressFunc) {
194255
return true
@@ -208,7 +269,6 @@ func fieldChanged(oldField, newField *schema.Schema) bool {
208269
if funcChanged(oldField.ValidateDiagFunc, newField.ValidateDiagFunc) {
209270
return true
210271
}
211-
212272
return false
213273
}
214274

@@ -225,3 +285,52 @@ func funcChanged(oldFunc, newFunc interface{}) bool {
225285
// b/300157205
226286
return false
227287
}
288+
289+
func mergeFieldSetsDiff(allFields ResourceFieldSetsDiffWithKeys, currentField ResourceFieldSetsDiff) ResourceFieldSetsDiffWithKeys {
290+
allFields.Old = mergeResourceFieldSets(allFields.Old, currentField.Old)
291+
allFields.New = mergeResourceFieldSets(allFields.New, currentField.New)
292+
return allFields
293+
}
294+
295+
func mergeResourceFieldSets(allFields ResourceFieldSetsWithKeys, currentField ResourceFieldSets) ResourceFieldSetsWithKeys {
296+
allFields.ConflictsWith = mergeFieldSets(allFields.ConflictsWith, currentField.ConflictsWith)
297+
allFields.ExactlyOneOf = mergeFieldSets(allFields.ExactlyOneOf, currentField.ExactlyOneOf)
298+
allFields.AtLeastOneOf = mergeFieldSets(allFields.AtLeastOneOf, currentField.AtLeastOneOf)
299+
allFields.RequiredWith = mergeFieldSets(allFields.RequiredWith, currentField.RequiredWith)
300+
return allFields
301+
}
302+
303+
func mergeFieldSets(allFields map[string]FieldSet, currentField []FieldSet) map[string]FieldSet {
304+
if allFields == nil {
305+
allFields = make(map[string]FieldSet)
306+
}
307+
for _, fieldSet := range currentField {
308+
allFields[setKey(fieldSet)] = fieldSet
309+
}
310+
return allFields
311+
}
312+
313+
func setKey(set FieldSet) string {
314+
slice := setToSortedSlice(set)
315+
return strings.Join(slice, ",")
316+
}
317+
318+
func removeFieldSetsDiffKeys(fieldSets ResourceFieldSetsDiffWithKeys) ResourceFieldSetsDiff {
319+
return ResourceFieldSetsDiff{
320+
Old: removeFieldSetsKey(fieldSets.Old),
321+
New: removeFieldSetsKey(fieldSets.New),
322+
}
323+
}
324+
325+
func removeFieldSetsKey(fieldSets ResourceFieldSetsWithKeys) ResourceFieldSets {
326+
return ResourceFieldSets{
327+
ConflictsWith: removeFieldSetKey(fieldSets.ConflictsWith),
328+
ExactlyOneOf: removeFieldSetKey(fieldSets.ExactlyOneOf),
329+
AtLeastOneOf: removeFieldSetKey(fieldSets.AtLeastOneOf),
330+
RequiredWith: removeFieldSetKey(fieldSets.RequiredWith),
331+
}
332+
}
333+
334+
func removeFieldSetKey(fieldSets map[string]FieldSet) []FieldSet {
335+
return slices.Collect(maps.Values(fieldSets))
336+
}

‎tools/diff-processor/diff/diff_test.go

+44-1
Original file line numberDiff line numberDiff line change
@@ -984,7 +984,7 @@ func TestFieldChanged(t *testing.T) {
984984
tc := tc
985985
t.Run(tn, func(t *testing.T) {
986986
t.Parallel()
987-
changed := fieldChanged(tc.oldField, tc.newField)
987+
_, _, changed := diffFields(tc.oldField, tc.newField, "")
988988
if changed != tc.expectChanged {
989989
if diff := cmp.Diff(tc.oldField, tc.newField); diff != "" {
990990
t.Errorf("want %t; got %t.\nField diff (-old, +new):\n%s",
@@ -1074,9 +1074,15 @@ func TestComputeSchemaDiff(t *testing.T) {
10741074
Schema: map[string]*schema.Schema{
10751075
"field_one": {
10761076
Type: schema.TypeString,
1077+
ConflictsWith: []string{
1078+
"field_two",
1079+
},
10771080
},
10781081
"field_two": {
10791082
Type: schema.TypeList,
1083+
ConflictsWith: []string{
1084+
"field_one",
1085+
},
10801086
Elem: &schema.Resource{
10811087
Schema: map[string]*schema.Schema{
10821088
"field_three": {
@@ -1110,9 +1116,15 @@ func TestComputeSchemaDiff(t *testing.T) {
11101116
Schema: map[string]*schema.Schema{
11111117
"field_one": {
11121118
Type: schema.TypeString,
1119+
ConflictsWith: []string{
1120+
"field_two",
1121+
},
11131122
},
11141123
"field_two": {
11151124
Type: schema.TypeList,
1125+
ConflictsWith: []string{
1126+
"field_one",
1127+
},
11161128
Elem: &schema.Resource{
11171129
Schema: map[string]*schema.Schema{
11181130
"field_three": {
@@ -1134,9 +1146,15 @@ func TestComputeSchemaDiff(t *testing.T) {
11341146
Schema: map[string]*schema.Schema{
11351147
"field_three": {
11361148
Type: schema.TypeString,
1149+
ConflictsWith: []string{
1150+
"field_two.0.field_four",
1151+
},
11371152
},
11381153
"field_four": {
11391154
Type: schema.TypeInt,
1155+
ConflictsWith: []string{
1156+
"field_two.0.field_three",
1157+
},
11401158
},
11411159
},
11421160
},
@@ -1151,10 +1169,35 @@ func TestComputeSchemaDiff(t *testing.T) {
11511169
New: &schema.Resource{},
11521170
},
11531171
Fields: map[string]FieldDiff{
1172+
"field_two.field_three": FieldDiff{
1173+
Old: &schema.Schema{
1174+
Type: schema.TypeString,
1175+
},
1176+
New: &schema.Schema{
1177+
Type: schema.TypeString,
1178+
ConflictsWith: []string{
1179+
"field_two.0.field_four",
1180+
},
1181+
},
1182+
},
11541183
"field_two.field_four": FieldDiff{
11551184
Old: nil,
11561185
New: &schema.Schema{
11571186
Type: schema.TypeInt,
1187+
ConflictsWith: []string{
1188+
"field_two.0.field_three",
1189+
},
1190+
},
1191+
},
1192+
},
1193+
FieldSets: ResourceFieldSetsDiff{
1194+
Old: ResourceFieldSets{},
1195+
New: ResourceFieldSets{
1196+
ConflictsWith: []FieldSet{
1197+
{
1198+
"field_two.field_three": {},
1199+
"field_two.field_four": {},
1200+
},
11581201
},
11591202
},
11601203
},

0 commit comments

Comments
 (0)