Check all items in lists

controllers/configurationpolicy_controller_test.go

@@ -271,8 +271,8 @@ func TestConvertPolicyStatusToStringLongMsg(t *testing.T) {
 	assert.Greater(t, len(statusMsg), 1024)
 }
 
-func TestMerge(t *testing.T) {
-	oldList := []interface{}{
+func TestMergeArrays(t *testing.T) {
+	twoFullItems := []interface{}{
 		map[string]interface{}{
 			"a": "apple",
 			"b": "boy",
@@ -282,25 +282,50 @@ func TestMerge(t *testing.T) {
 			"d": "dog",
 		},
 	}
-	newList := []interface{}{
+
+	oneFullItem := []interface{}{
 		map[string]interface{}{
 			"a": "apple",
 			"b": "boy",
 		},
 	}
 
-	merged1 := mergeArrays(newList, oldList, "musthave")
-	assert.Equal(t, checkListsMatch(oldList, merged1), true)
+	assert.Equal(t, twoFullItems, mergeArrays(oneFullItem, twoFullItems, "musthave"),
+		"A musthave with a single item should preserve the larger list")
 
-	merged2 := mergeArrays(newList, oldList, "mustonlyhave")
-	assert.Equal(t, checkListsMatch(newList, merged2), true)
+	assert.Equal(t, oneFullItem, mergeArrays(oneFullItem, twoFullItems, "mustonlyhave"),
+		"A mustonlyhave with a single item should only keep the single item")
 
-	newList2 := []interface{}{
+	oneSmallItem := []interface{}{
 		map[string]interface{}{
 			"b": "boy",
 		},
 	}
-	oldList2 := []interface{}{
+
+	assert.Equal(t, twoFullItems, mergeArrays(oneSmallItem, twoFullItems, "musthave"),
+		"A musthave with a single smaller item should preserve the larger list")
+
+	assert.Equal(t, oneSmallItem, mergeArrays(oneSmallItem, twoFullItems, "mustonlyhave"),
+		"A mustonlyhave with a single smaller item should only keep that smaller single item")
+
+	twoSmallItems := []interface{}{
+		map[string]interface{}{
+			"a": "apple",
+		},
+		map[string]interface{}{
+			"c": "candy",
+		},
+	}
+
+	assert.Equal(t, twoFullItems, mergeArrays(twoSmallItems, twoFullItems, "musthave"),
+		"A musthave with a list of items with some fields removed should preserve the larger list")
+
+	assert.Equal(t, twoSmallItems, mergeArrays(twoSmallItems, twoFullItems, "mustonlyhave"),
+		"A mustonlyhave with a list of items with some fields removed should only keep that new list")
+}
+
+func TestCheckListsMatch(t *testing.T) {
+	twoFullItems := []interface{}{
 		map[string]interface{}{
 			"a": "apple",
 			"b": "boy",
@@ -310,31 +335,79 @@ func TestMerge(t *testing.T) {
 			"d": "dog",
 		},
 	}
-	checkList2 := []interface{}{
+
+	assert.True(t, checkListsMatch(twoFullItems, twoFullItems))
+
+	twoFullItemsDifferentOrder := []interface{}{
+		map[string]interface{}{
+			"c": "candy",
+			"d": "dog",
+		},
+		map[string]interface{}{
+			"a": "apple",
+			"b": "boy",
+		},
+	}
+
+	assert.True(t, checkListsMatch(twoFullItems, twoFullItemsDifferentOrder))
+	assert.True(t, checkListsMatch(twoFullItemsDifferentOrder, twoFullItems))
+
+	oneFullItem := []interface{}{
 		map[string]interface{}{
 			"a": "apple",
 			"b": "boy",
 		},
+	}
+
+	assert.False(t, checkListsMatch(twoFullItems, oneFullItem))
+	assert.False(t, checkListsMatch(oneFullItem, twoFullItems))
+
+	oneSmallItem := []interface{}{
+		map[string]interface{}{
+			"b": "boy",
+		},
+	}
+
+	assert.False(t, checkListsMatch(twoFullItems, oneSmallItem))
+	assert.False(t, checkListsMatch(oneSmallItem, twoFullItems))
+
+	twoSmallItems := []interface{}{
+		map[string]interface{}{
+			"a": "apple",
+		},
+		map[string]interface{}{
+			"c": "candy",
+		},
+	}
+
+	assert.False(t, checkListsMatch(twoFullItems, twoSmallItems))
+	assert.False(t, checkListsMatch(twoSmallItems, twoFullItems))
+
+	oneSmallOneBig := []interface{}{
+		map[string]interface{}{
+			"a": "apple",
+		},
 		map[string]interface{}{
 			"c": "candy",
 			"d": "dog",
 		},
 	}
-	merged3 := mergeArrays(newList2, oldList2, "musthave")
 
-	assert.Equal(t, checkListsMatch(checkList2, merged3), true)
+	assert.False(t, checkListsMatch(twoFullItems, oneSmallOneBig))
+	assert.False(t, checkListsMatch(oneSmallOneBig, twoFullItems))
 
-	newList3 := []interface{}{
+	oneBigOneSmall := []interface{}{
 		map[string]interface{}{
 			"a": "apple",
+			"b": "boy",
 		},
 		map[string]interface{}{
 			"c": "candy",
 		},
 	}
-	merged4 := mergeArrays(newList3, oldList2, "musthave")
 
-	assert.Equal(t, checkListsMatch(checkList2, merged4), true)
+	assert.False(t, checkListsMatch(twoFullItems, oneBigOneSmall))
+	assert.False(t, checkListsMatch(oneBigOneSmall, twoFullItems))
 }
 
 func TestAddRelatedObject(t *testing.T) {

controllers/configurationpolicy_utils.go

@@ -149,7 +149,7 @@ func equalObjWithSort(mergedObj interface{}, oldObj interface{}) (areEqual bool)
 	return true
 }
 
-// checFieldsWithSort is a check for maps that uses an arbitrary sort to ensure it is
+// checkFieldsWithSort is a check for maps that uses an arbitrary sort to ensure it is
 // comparing the right values
 func checkFieldsWithSort(mergedObj map[string]interface{}, oldObj map[string]interface{}) (matches bool) {
 	// needed to compare lists, since merge messes up the order
@@ -173,12 +173,6 @@ func checkFieldsWithSort(mergedObj map[string]interface{}, oldObj map[string]int
 			if !checkFieldsWithSort(mVal, oVal) {
 				return false
 			}
-		case []map[string]interface{}:
-			// if field is a list of maps, use checkListFieldsWithSort to check for a match
-			oVal, ok := oldObj[i].([]map[string]interface{})
-			if !ok || !checkListFieldsWithSort(mVal, oVal) {
-				return false
-			}
 		case []interface{}:
 			// if field is a generic list, sort and iterate through them to make sure each value matches
 			oVal, ok := oldObj[i].([]interface{})
@@ -236,94 +230,37 @@ func checkFieldsWithSort(mergedObj map[string]interface{}, oldObj map[string]int
 	return true
 }
 
-// checkListFieldsWithSort is a check for lists of maps that uses an arbitrary sort to ensure it is
-// comparing the right values
-func checkListFieldsWithSort(mergedObj []map[string]interface{}, oldObj []map[string]interface{}) (matches bool) {
-	sort.Slice(oldObj, func(i, j int) bool {
-		return fmt.Sprintf("%v", oldObj[i]) < fmt.Sprintf("%v", oldObj[j])
-	})
-	sort.Slice(mergedObj, func(x, y int) bool {
-		return fmt.Sprintf("%v", mergedObj[x]) < fmt.Sprintf("%v", mergedObj[y])
-	})
-
-	// needed to compare lists, since merge messes up the order
-	for listIdx, mergedItem := range mergedObj {
-		oldItem := oldObj[listIdx]
-
-		for i, mVal := range mergedItem {
-			switch mVal := mVal.(type) {
-			case []interface{}:
-				// if a map in the list contains a nested list, sort and check for equality
-				if oVal, ok := oldItem[i].([]interface{}); ok {
-					return len(mVal) == len(oVal) && checkListsMatch(oVal, mVal)
-				}
-
-				return false
-			case map[string]interface{}:
-				// if a map in the list contains another map, check fields for equality
-				if oVal, ok := oldItem[i].(map[string]interface{}); ok {
-					return len(mVal) == len(oVal) && checkFieldsWithSort(mVal, oVal)
-				}
-
-				return false
-			case string:
-				// extra check to see if value is a byte value
-				mQty, err := apiRes.ParseQuantity(mVal)
-				if err != nil {
-					// An error indicates the value is a regular string, so check equality normally
-					if fmt.Sprint(oldItem[i]) != fmt.Sprint(mVal) {
-						return false
-					}
-				} else {
-					// if the value is a quantity of bytes, convert original
-					oVal, ok := oldItem[i].(string)
-					if !ok {
-						return false
-					}
-
-					oQty, err := apiRes.ParseQuantity(oVal)
-					if err != nil || !oQty.Equal(mQty) {
-						return false
-					}
-				}
-			default:
-				// if the field in the map is not an object, just do a generic check
-				if fmt.Sprint(oldItem[i]) != fmt.Sprint(mVal) {
-					return false
-				}
-			}
-		}
-	}
-
-	return true
-}
-
 // checkListsMatch is a generic list check that uses an arbitrary sort to ensure it is comparing the right values
 func checkListsMatch(oldVal []interface{}, mergedVal []interface{}) (m bool) {
-	oVal := append([]interface{}{}, oldVal...)
-	mVal := append([]interface{}{}, mergedVal...)
+	if (oldVal == nil && mergedVal != nil) || (oldVal != nil && mergedVal == nil) {
+		return false
+	}
 
-	if (oVal == nil && mVal != nil) || (oVal != nil && mVal == nil) {
+	if len(mergedVal) != len(oldVal) {
 		return false
 	}
 
+	// Make copies of the lists, so we can sort them without mutating this function's inputs
+	oVal := append([]interface{}{}, oldVal...)
+	mVal := append([]interface{}{}, mergedVal...)
+
 	sort.Slice(oVal, func(i, j int) bool {
 		return fmt.Sprintf("%v", oVal[i]) < fmt.Sprintf("%v", oVal[j])
 	})
 	sort.Slice(mVal, func(x, y int) bool {
 		return fmt.Sprintf("%v", mVal[x]) < fmt.Sprintf("%v", mVal[y])
 	})
 
-	if len(mVal) != len(oVal) {
-		return false
-	}
-
 	for idx, oNestedVal := range oVal {
 		switch oNestedVal := oNestedVal.(type) {
 		case map[string]interface{}:
 			// if list contains maps, recurse on those maps to check for a match
 			if mVal, ok := mVal[idx].(map[string]interface{}); ok {
-				return len(mVal) == len(oNestedVal) && checkFieldsWithSort(mVal, oNestedVal)
+				if len(mVal) != len(oNestedVal) || !checkFieldsWithSort(mVal, oNestedVal) {
+					return false
+				}
+
+				continue
 			}
 
 			return false