Fix the conditions history in compliancyDetails

The status.compliancyDetails[].conditions array was always limited to
just a single condition due to a bug. This sets the limit to 10 instead.

Relates:
https://issues.redhat.com/browse/ACM-2708

Signed-off-by: mprahl <mprahl@users.noreply.github.com>

controllers/configurationpolicy_controller.go

@@ -48,8 +48,9 @@ import (
 )
 
 const (
- ControllerName string = "configuration-policy-controller"
- CRDName string = "configurationpolicies.policy.open-cluster-management.io"
+ ControllerName string = "configuration-policy-controller"
+ CRDName string = "configurationpolicies.policy.open-cluster-management.io"
+ complianceStatusConditionLimit = 10
 )
 
 var log = ctrl.Log.WithName(ControllerName)
@@ -1155,7 +1156,7 @@ func addConditionToStatus(
 
  // do not add condition unless it does not already appear in the status
  if !checkMessageSimilarity(plc.Status.CompliancyDetails[index].Conditions, cond) {
- conditions := AppendCondition(plc.Status.CompliancyDetails[index].Conditions, cond, "", false)
+ conditions := AppendCondition(plc.Status.CompliancyDetails[index].Conditions, cond)
  plc.Status.CompliancyDetails[index].Conditions = conditions
  updateNeeded = true
  }
@@ -1715,8 +1716,7 @@ func (r *ConfigurationPolicyReconciler) getMapping(
  policy.Status.CompliancyDetails[index].ComplianceState = policyv1.NonCompliant
 
  if !checkMessageSimilarity(policy.Status.CompliancyDetails[index].Conditions, cond) {
- conditions := AppendCondition(policy.Status.CompliancyDetails[index].Conditions,
- cond, gvk.GroupKind().Kind, false)
+ conditions := AppendCondition(policy.Status.CompliancyDetails[index].Conditions, cond)
  policy.Status.CompliancyDetails[index].Conditions = conditions
  updateNeeded = true
  }
@@ -2423,26 +2423,19 @@ func (r *ConfigurationPolicyReconciler) checkAndUpdateResource(
 
 // AppendCondition check and appends conditions to the policy status
 func AppendCondition(
- conditions []policyv1.Condition, newCond *policyv1.Condition, resourceType string, resolved ...bool,
+ conditions []policyv1.Condition, newCond *policyv1.Condition,
 ) (conditionsRes []policyv1.Condition) {
- defer recoverFlow()
-
- lastIndex := len(conditions)
- if lastIndex > 0 {
- oldCond := conditions[lastIndex-1]
- if IsSimilarToLastCondition(oldCond, *newCond) {
- conditions[lastIndex-1] = *newCond
-
- return conditions
- }
- } else {
- // first condition => trigger event
- conditions = append(conditions, *newCond)
+ if len(conditions) != 0 && IsSimilarToLastCondition(conditions[len(conditions)-1], *newCond) {
+ conditions[len(conditions)-1] = *newCond
 
  return conditions
  }
 
- conditions[lastIndex-1] = *newCond
+ conditions = append(conditions, *newCond)
+
+ if len(conditions) > complianceStatusConditionLimit {
+ conditions = conditions[1:]
+ }
 
  return conditions
 }
@@ -2632,10 +2625,3 @@ func convertPolicyStatusToString(plc *policyv1.ConfigurationPolicy) (results str
 
  return result
 }
-
-func recoverFlow() {
- if r := recover(); r != nil {
- // V(-2) is the error level
- log.V(-2).Info("ALERT!!!! -> recovered from ", "recover", r)
- }
-}

controllers/configurationpolicy_controller_test.go

@@ -701,3 +701,69 @@ func TestShouldEvaluatePolicy(t *testing.T) {
  )
  }
 }
+
+func TestAppendCondition(t *testing.T) {
+ conditions := []policyv1.Condition{}
+ newCond1 := &policyv1.Condition{
+ Type: "violation",
+ LastTransitionTime: metav1.Time{Time: metav1.Now().Add(5 * time.Minute)},
+ Reason: "K8s `must have` object already exists",
+ Message: "some message",
+ }
+
+ conditions = AppendCondition(conditions, newCond1)
+ if len(conditions) != 1 {
+ t.Fatalf("Expected a single condition to be appended but the length was %d", len(conditions))
+ }
+
+ lastTransition := metav1.Time{Time: metav1.Now().Add(5 * time.Minute)}
+ newCond2 := &policyv1.Condition{
+ Type: "violation",
+ LastTransitionTime: lastTransition,
+ Reason: "K8s `must have` object already exists",
+ Message: "some message",
+ }
+ conditions = AppendCondition(conditions, newCond2)
+
+ if len(conditions) != 1 {
+ t.Fatalf("Expected the single condition to be updated and not appended but the length was %d", len(conditions))
+ }
+
+ if conditions[0].LastTransitionTime != lastTransition {
+ t.Fatal("The single condition was not updated with the last transition time")
+ }
+
+ newCond3 := &policyv1.Condition{
+ Type: "violation",
+ LastTransitionTime: lastTransition,
+ Reason: "K8s creation success",
+ Message: "some other message",
+ }
+ conditions = AppendCondition(conditions, newCond3)
+
+ if len(conditions) != 2 {
+ t.Fatalf("Expected the new condition to be appended but the length was %d", len(conditions))
+ }
+
+ if conditions[0].Reason != newCond2.Reason || conditions[1].Reason != newCond3.Reason {
+ t.Fatalf("Expected the new condition to be appended the order is incorrect")
+ }
+
+ for i := 0; i < complianceStatusConditionLimit+2; i++ {
+ newCond := &policyv1.Condition{
+ Type: "violation",
+ LastTransitionTime: lastTransition,
+ Reason: "K8s creation success",
+ Message: fmt.Sprintf("some other message%d", i),
+ }
+ conditions = AppendCondition(conditions, newCond)
+ }
+
+ if len(conditions) != complianceStatusConditionLimit {
+ t.Fatalf(
+ "Expected the conditions array to be limited to %d but the length was %d",
+ complianceStatusConditionLimit,
+ len(conditions),
+ )
+ }
+}

test/e2e/case1_pod_handling_test.go

@@ -6,6 +6,7 @@ package e2e
 import (
  . "github.com/onsi/ginkgo/v2"
  . "github.com/onsi/gomega"
+ "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
 
  "open-cluster-management.io/config-policy-controller/test/utils"
 )
@@ -47,12 +48,26 @@ var _ = Describe("Test pod obj template handling", func() {
  plc := utils.GetWithTimeout(clientManagedDynamic, gvrConfigPolicy,
  case1ConfigPolicyNameEnforce, testNamespace, true, defaultTimeoutSeconds)
  Expect(plc).NotTo(BeNil())
+
+ var managedPlc *unstructured.Unstructured
  Eventually(func() interface{} {
- managedPlc := utils.GetWithTimeout(clientManagedDynamic, gvrConfigPolicy,
+ managedPlc = utils.GetWithTimeout(clientManagedDynamic, gvrConfigPolicy,
  case1ConfigPolicyNameEnforce, testNamespace, true, defaultTimeoutSeconds)
 
  return utils.GetComplianceState(managedPlc)
  }, defaultTimeoutSeconds, 1).Should(Equal("Compliant"))
+
+ By("verifying status.compliancyDetails[].conditions")
+ compliancyDetails, _, _ := unstructured.NestedSlice(managedPlc.Object, "status", "compliancyDetails")
+ Expect(compliancyDetails).To(HaveLen(1))
+
+ conditions, ok := compliancyDetails[0].(map[string]interface{})["conditions"].([]interface{})
+ Expect(ok).To(BeTrue())
+ Expect(conditions).To(HaveLen(2))
+ Expect(conditions[0].(map[string]interface{})["reason"]).To(Equal("K8s creation success"))
+ Expect(conditions[1].(map[string]interface{})["reason"]).To(Equal("K8s `must have` object already exists"))
+
+ By("verifying that " + case1ConfigPolicyNameInform + "is compliant")
  Eventually(func() interface{} {
  informPlc := utils.GetWithTimeout(clientManagedDynamic, gvrConfigPolicy,
  case1ConfigPolicyNameInform, testNamespace, true, defaultTimeoutSeconds)