🤖 Sync from open-cluster-management-io/config-policy-controller: #91

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
  }
@@ -1757,8 +1758,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
  }
@@ -2470,26 +2470,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
 }
@@ -2679,10 +2672,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,35 @@ var _ = Describe("Test pod obj template handling", func() {
  plc := utils.GetWithTimeout(clientManagedDynamic, gvrConfigPolicy,
  case1ConfigPolicyNameEnforce, testNamespace, true, defaultTimeoutSeconds)
  Expect(plc).NotTo(BeNil())
+
  Eventually(func() interface{} {
  managedPlc := utils.GetWithTimeout(clientManagedDynamic, gvrConfigPolicy,
  case1ConfigPolicyNameEnforce, testNamespace, true, defaultTimeoutSeconds)
 
  return utils.GetComplianceState(managedPlc)
  }, defaultTimeoutSeconds, 1).Should(Equal("Compliant"))
+
+ By("verifying status.compliancyDetails[].conditions")
+ Eventually(func(g Gomega) {
+ managedPlc := utils.GetWithTimeout(clientManagedDynamic, gvrConfigPolicy,
+ case1ConfigPolicyNameEnforce, testNamespace, true, defaultTimeoutSeconds)
+
+ compliancyDetails, _, _ := unstructured.NestedSlice(managedPlc.Object, "status", "compliancyDetails")
+ g.Expect(compliancyDetails).To(HaveLen(1))
+
+ conditions, ok := compliancyDetails[0].(map[string]interface{})["conditions"].([]interface{})
+ g.Expect(ok).To(BeTrue())
+ g.Expect(conditions).To(HaveLen(3))
+ g.Expect(conditions[0].(map[string]interface{})["reason"]).To(
+ Equal("K8s does not have a `must have` object"),
+ )
+ g.Expect(conditions[1].(map[string]interface{})["reason"]).To(Equal("K8s creation success"))
+ g.Expect(conditions[2].(map[string]interface{})["reason"]).To(
+ Equal("K8s `must have` object already exists"),
+ )
+ }, defaultTimeoutSeconds, 1).Should(Succeed())
+
+ By("verifying that " + case1ConfigPolicyNameInform + " is compliant")
  Eventually(func() interface{} {
  informPlc := utils.GetWithTimeout(clientManagedDynamic, gvrConfigPolicy,
  case1ConfigPolicyNameInform, testNamespace, true, defaultTimeoutSeconds)

test/e2e/case20_delete_objects_test.go

@@ -575,10 +575,12 @@ var _ = Describe("Test objects are not deleted when the CRD is removed", Ordered
  utils.Kubectl("delete", "-f", case20ConfigPolicyCRDPath)
 
  By("Checking that the ConfigurationPolicy is gone")
- namespace := clientManagedDynamic.Resource(gvrConfigPolicy).Namespace(testNamespace)
- _, err := namespace.Get(context.TODO(), case20ConfigPolicyNameMHPDA, metav1.GetOptions{})
- Expect(err).NotTo(BeNil())
- Expect(err.Error()).To(ContainSubstring("the server could not find the requested resource"))
+ Eventually(func(g Gomega) {
+ namespace := clientManagedDynamic.Resource(gvrConfigPolicy).Namespace(testNamespace)
+ _, err := namespace.Get(context.TODO(), case20ConfigPolicyNameMHPDA, metav1.GetOptions{})
+ g.Expect(err).NotTo(BeNil())
+ g.Expect(err.Error()).To(ContainSubstring("the server could not find the requested resource"))
+ }, defaultTimeoutSeconds, 1).Should(Succeed())
 
  By("Recreating the CRD")
  utils.Kubectl("apply", "-f", case20ConfigPolicyCRDPath)

test/utils/utils.go

@@ -207,8 +207,9 @@ func GetComplianceState(managedPlc *unstructured.Unstructured) (result interface
 func GetStatusMessage(managedPlc *unstructured.Unstructured) (result interface{}) {
  if managedPlc.Object["status"] != nil {
  detail := managedPlc.Object["status"].(map[string]interface{})["compliancyDetails"].([]interface{})[0]
+ conditions := detail.(map[string]interface{})["conditions"].([]interface{})
 
- return detail.(map[string]interface{})["conditions"].([]interface{})[0].(map[string]interface{})["message"]
+ return conditions[len(conditions)-1].(map[string]interface{})["message"]
  }
 
  return nil