Fix status history toggling

Signed-off-by: Yi Rae Kim <yikim@redhat.com>

controllers/configurationpolicy_controller_test.go

@@ -12,6 +12,7 @@ import (
 
 	"github.com/stretchr/testify/assert"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+	"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
 	"k8s.io/apimachinery/pkg/runtime"
 	"k8s.io/apimachinery/pkg/types"
 	"k8s.io/client-go/kubernetes/scheme"
@@ -754,3 +755,99 @@ func TestShouldEvaluatePolicy(t *testing.T) {
 		)
 	}
 }
+
+func TestShouldHandleSingleKeyFalse(t *testing.T) {
+	t.Parallel()
+
+	var unstruct unstructured.Unstructured
+	var unstructObj unstructured.Unstructured
+
+	var update, skip bool
+
+	type ExpectResult struct {
+		key    string
+		expect bool
+	}
+
+	type TestSingleKey struct {
+		input        map[string]interface{}
+		fromAPI      map[string]interface{}
+		expectResult ExpectResult
+	}
+
+	tests := []TestSingleKey{
+		{
+			input: map[string]interface{}{
+				"hostIPC":   false,
+				"container": "test",
+			},
+			fromAPI: map[string]interface{}{
+				"container": "test",
+			},
+			expectResult: ExpectResult{
+				"hostIPC",
+				false,
+			},
+		},
+		{
+			input: map[string]interface{}{
+				"container": map[string]interface{}{
+					"image":   "nginx1.7.9",
+					"name":    "nginx",
+					"hostIPC": false,
+				},
+			},
+			fromAPI: map[string]interface{}{
+				"container": map[string]interface{}{
+					"image": "nginx1.7.9",
+					"name":  "nginx",
+				},
+			},
+			expectResult: ExpectResult{
+				"container",
+				false,
+			},
+		},
+		{
+			input: map[string]interface{}{
+				"hostIPC":   true,
+				"container": "test",
+			},
+			fromAPI: map[string]interface{}{
+				"container": "test",
+			},
+			expectResult: ExpectResult{
+				"hostIPC",
+				true,
+			},
+		},
+		{
+			input: map[string]interface{}{
+				"container": map[string]interface{}{
+					"image":   "nginx1.7.9",
+					"name":    "nginx",
+					"hostIPC": true,
+				},
+			},
+			fromAPI: map[string]interface{}{
+				"container": map[string]interface{}{
+					"image": "nginx1.7.9",
+					"name":  "nginx",
+				},
+			},
+			expectResult: ExpectResult{
+				"container",
+				true,
+			},
+		},
+	}
+
+	for _, test := range tests {
+		unstruct.Object = test.input
+		unstructObj.Object = test.fromAPI
+		key := test.expectResult.key
+		_, update, _, skip = handleSingleKey(key, unstruct, &unstructObj, "musthave")
+		assert.Equal(t, update, test.expectResult.expect)
+		assert.False(t, skip)
+	}
+}

controllers/configurationpolicy_utils.go

@@ -119,6 +119,16 @@ func equalObjWithSort(mergedObj interface{}, oldObj interface{}) (areEqual bool)
 			return false
 		}
 	default:
+		// NOTE: when type is string, int, bool
+		var oVal interface{}
+
+		if oldObj == nil && mergedObj != nil {
+			ref := reflect.ValueOf(mergedObj)
+			oVal = reflect.Zero(ref.Type()).Interface()
+
+			return fmt.Sprint(oVal) == fmt.Sprint(mergedObj)
+		}
+
 		if !reflect.DeepEqual(fmt.Sprint(mergedObj), fmt.Sprint(oldObj)) {
 			return false
 		}
@@ -175,8 +185,12 @@ func checkFieldsWithSort(mergedObj map[string]interface{}, oldObj map[string]int
 			// extra check to see if value is a byte value
 			mQty, err := apiRes.ParseQuantity(mVal)
 			if err != nil {
+				oVal := oldObj[i]
+				if oVal == nil {
+					oVal = ""
+				}
 				// An error indicates the value is a regular string, so check equality normally
-				if fmt.Sprint(oldObj[i]) != fmt.Sprint(mVal) {
+				if fmt.Sprint(oVal) != fmt.Sprint(mVal) {
 					return false
 				}
 			} else {
@@ -194,6 +208,12 @@ func checkFieldsWithSort(mergedObj map[string]interface{}, oldObj map[string]int
 		default:
 			// if field is not an object, just do a basic compare to check for a match
 			oVal := oldObj[i]
+			// When oVal value omitted because of omitempty
+			if oVal == nil && mVal != nil {
+				ref := reflect.ValueOf(mVal)
+				oVal = reflect.Zero(ref.Type()).Interface()
+			}
+
 			if fmt.Sprint(oVal) != fmt.Sprint(mVal) {
 				return false
 			}

controllers/configurationpolicy_utils_test.go

@@ -118,3 +118,34 @@ func TestCheckFieldsWithSort(t *testing.T) {
 
 	assert.True(t, checkFieldsWithSort(mergedObj, oldObj))
 }
+
+func TestEqualObjWithSort(t *testing.T) {
+	t.Parallel()
+
+	oldObj := map[string]interface{}{
+		"nonResourceURLs": []string{"/version", "/healthz"},
+		"verbs":           []string{"get"},
+	}
+	mergedObj := map[string]interface{}{
+		"nonResourceURLs": []string{"/version", "/healthz"},
+		"verbs":           []string{"get"},
+		"apiGroups":       []interface{}{},
+		"resources":       []interface{}{},
+	}
+
+	assert.True(t, equalObjWithSort(mergedObj, oldObj))
+	assert.False(t, equalObjWithSort(mergedObj, nil))
+
+	oldObj = map[string]interface{}{
+		"nonResourceURLs": []string{"/version", "/healthz"},
+		"verbs":           []string{"get"},
+	}
+	mergedObj = map[string]interface{}{
+		"nonResourceURLs": []string{"/version", "/healthz"},
+		"verbs":           []string{"post"},
+		"apiGroups":       []interface{}{},
+		"resources":       []interface{}{},
+	}
+
+	assert.False(t, equalObjWithSort(mergedObj, oldObj))
+}

test/e2e/case12_list_compare_test.go

@@ -356,7 +356,17 @@ var _ = Describe("Test list handling for musthave", func() {
 			deleteConfigPolicies(policies)
 		})
 	})
-	Describe("Create a statefulset object with a byte quantity field on managed cluster in ns:"+testNamespace, func() {
+	Describe("Create a statefulset object with a byte quantity field "+
+		"on managed cluster in ns:"+testNamespace, Ordered, func() {
+		cleanup := func() {
+			// Delete the policies and ignore any errors (in case it was deleted previously)
+			policies := []string{
+				case12ByteCreate,
+				case12ByteInform,
+			}
+
+			deleteConfigPolicies(policies)
+		}
 		It("should only add the list item with the rounded byte value once", func() {
 			By("Creating " + case12ByteCreate + " and " + case12ByteInform + " on managed")
 			utils.Kubectl("apply", "-f", case12ByteCreateYaml, "-n", testNamespace)
@@ -389,14 +399,6 @@ var _ = Describe("Test list handling for musthave", func() {
 				return utils.GetComplianceState(managedPlc)
 			}, defaultTimeoutSeconds, 1).Should(Equal("Compliant"))
 		})
-
-		It("Cleans up", func() {
-			policies := []string{
-				case12ByteCreate,
-				case12ByteInform,
-			}
-
-			deleteConfigPolicies(policies)
-		})
+		AfterAll(cleanup)
 	})
 })

test/e2e/case20_delete_objects_test.go

@@ -527,7 +527,8 @@ var _ = Describe("Test objects that should be deleted are actually being deleted
 				return nil
 			}, defaultTimeoutSeconds, 1).Should(BeNil())
 		})
-		It("deletes the pod after the policy is deleted", func() {
+		AfterAll(func() {
+			By("deletes the pod after the policy is deleted")
 			deleteConfigPolicies([]string{case20ConfigPolicyNameCreate})
 			Eventually(func() interface{} {
 				pod := utils.GetWithTimeout(clientManagedDynamic, gvrPod,

test/e2e/case25_related_object_metric_test.go

@@ -56,7 +56,7 @@ var _ = Describe("Test related object metrics", Ordered, func() {
 		// Delete the policies and ignore any errors (in case it was deleted previously)
 		cmd := exec.Command("kubectl", "delete",
 			"-f", policyYaml,
-			"-n", testNamespace)
+			"-n", testNamespace, "--ignore-not-found")
 		_, _ = cmd.CombinedOutput()
 		opt := metav1.ListOptions{}
 		utils.ListWithTimeout(
@@ -66,13 +66,10 @@ var _ = Describe("Test related object metrics", Ordered, func() {
 	}
 
 	It("should clean up", cleanup)
-
 	It("should have no common related object metrics after clean up", func() {
 		By("Checking metric endpoint for related object gauges")
 		Eventually(func() interface{} {
 			return utils.GetMetrics("common_related_objects")
 		}, defaultTimeoutSeconds, 1).Should(Equal([]string{}))
 	})
-
-	AfterAll(cleanup)
 })

test/e2e/case26_user_error_metric_test.go

@@ -15,6 +15,21 @@ var _ = Describe("Test related object metrics", Ordered, func() {
 		policy1Name = "case26-test-policy-1"
 		policyYaml  = "../resources/case26_user_error_metric/case26-missing-crd.yaml"
 	)
+	cleanup := func() {
+		// Delete the policies and ignore any errors (in case it was deleted previously)
+		cmd := exec.Command("kubectl", "delete",
+			"-f", policyYaml,
+			"-n", testNamespace, "--ignore-not-found")
+		_, _ = cmd.CombinedOutput()
+		opt := metav1.ListOptions{}
+		utils.ListWithTimeout(
+			clientManagedDynamic, gvrConfigPolicy, opt, 0, false, defaultTimeoutSeconds)
+		By("Checking metric endpoint for related object gauges")
+		Eventually(func() interface{} {
+			return utils.GetMetrics("policy_user_errors")
+		}, defaultTimeoutSeconds, 1).Should(Equal([]string{}))
+	}
+
 	It("should create policy", func() {
 		By("Creating " + policyYaml)
 		utils.Kubectl("apply",
@@ -36,25 +51,5 @@ var _ = Describe("Test related object metrics", Ordered, func() {
 		}, defaultTimeoutSeconds, 1).Should(Equal([]string{"policies", "counter", "1"}))
 	})
 
-	cleanup := func() {
-		// Delete the policies and ignore any errors (in case it was deleted previously)
-		cmd := exec.Command("kubectl", "delete",
-			"-f", policyYaml,
-			"-n", testNamespace)
-		_, _ = cmd.CombinedOutput()
-		opt := metav1.ListOptions{}
-		utils.ListWithTimeout(
-			clientManagedDynamic, gvrConfigPolicy, opt, 0, false, defaultTimeoutSeconds)
-	}
-
-	It("should clean up", cleanup)
-
-	It("should have no common related object metrics after clean up", func() {
-		By("Checking metric endpoint for related object gauges")
-		Eventually(func() interface{} {
-			return utils.GetMetrics("policy_user_errors")
-		}, defaultTimeoutSeconds, 1).Should(Equal([]string{}))
-	})
-
 	AfterAll(cleanup)
 })

test/e2e/case28_evauluation_metric_test.go

@@ -102,7 +102,6 @@ var _ = Describe("Test config policy evaluation metrics", Ordered, func() {
 		opt := metav1.ListOptions{}
 		utils.ListWithTimeout(
 			clientManagedDynamic, gvrConfigPolicy, opt, 0, false, defaultTimeoutSeconds)
-
 		Eventually(func() interface{} {
 			return utils.GetMetrics(
 				"config_policy_evaluation_total", fmt.Sprintf(`name=\"%s\"`, policyName))