From 4da6c18364fd78b23e7dd2fd9f09cda5ec5446bb Mon Sep 17 00:00:00 2001 From: Yi Rae Kim Date: Tue, 2 May 2023 09:05:17 -0400 Subject: [PATCH] Bug: Policy compliance status is truncated Description of problem: The status message for non compliant policy is truncated with "..." when the policy has multiple objectTemplates or a namespace. This leaves the user unable to determine why the policy is non compliant. Expect result: message should include all information Signed-off-by: Yi Rae Kim --- controllers/configurationpolicy_controller.go | 7 -- .../configurationpolicy_controller_test.go | 3 +- test/e2e/case31_policy_history_test.go | 65 +++++++++++++++++++ .../long-message-config-policy.yaml | 47 ++++++++++++++ .../long-message-policy.yaml | 50 ++++++++++++++ 5 files changed, 163 insertions(+), 9 deletions(-) create mode 100644 test/resources/case31_policy_history/long-message-config-policy.yaml create mode 100644 test/resources/case31_policy_history/long-message-policy.yaml diff --git a/controllers/configurationpolicy_controller.go b/controllers/configurationpolicy_controller.go index 16271d7f..cb4d509b 100644 --- a/controllers/configurationpolicy_controller.go +++ b/controllers/configurationpolicy_controller.go @@ -2842,7 +2842,6 @@ func (r *ConfigurationPolicyReconciler) sendComplianceEvent(instance *policyv1.C LastTimestamp: metav1.NewTime(now), Count: 1, Type: "Normal", - EventTime: metav1.NewMicroTime(now), Action: "ComplianceStateUpdate", Related: &corev1.ObjectReference{ Kind: instance.Kind, @@ -2884,12 +2883,6 @@ func convertPolicyStatusToString(plc *policyv1.ConfigurationPolicy) (results str } } - runeResult := []rune(result) - - if len(runeResult) > 1024 { - result = string(append(runeResult[:1021], '.', '.', '.')) - } - return result } diff --git a/controllers/configurationpolicy_controller_test.go b/controllers/configurationpolicy_controller_test.go index ca78a7a4..9af39633 100644 --- a/controllers/configurationpolicy_controller_test.go +++ b/controllers/configurationpolicy_controller_test.go @@ -268,8 +268,7 @@ func TestConvertPolicyStatusToStringLongMsg(t *testing.T) { } statusMsg := convertPolicyStatusToString(&samplePolicy) - assert.Contains(t, statusMsg, "...") - assert.Len(t, []rune(statusMsg), 1024) + assert.Greater(t, len(statusMsg), 1024) } func TestMerge(t *testing.T) { diff --git a/test/e2e/case31_policy_history_test.go b/test/e2e/case31_policy_history_test.go index e83a19f5..8cb1be52 100644 --- a/test/e2e/case31_policy_history_test.go +++ b/test/e2e/case31_policy_history_test.go @@ -5,6 +5,7 @@ package e2e import ( "context" + "strconv" . "github.com/onsi/ginkgo/v2" . "github.com/onsi/gomega" @@ -152,4 +153,68 @@ var _ = Describe("Test policy history message when KubeAPI return "+ ExpectWithOffset(1, configlPlc).To(BeNil()) }) }) + Describe("policy message should not be truncated", func() { + const ( + case31LMPolicy = "../resources/case31_policy_history/long-message-policy.yaml" + case31LMConfigPolicy = "../resources/case31_policy_history/long-message-config-policy.yaml" + case31LMPolicyName = "long-message-policy" + case31LMConfigPolicyName = "long-message-config-policy" + namespacePrefix = "innovafertanimvsmvtatasdicereformascorporinnovafertanimvsmvt" + ) + It("Test policy message length is over 1024 ", func() { + By("Create namespaces") + for i := range [15]int{} { + utils.Kubectl("create", "ns", namespacePrefix+strconv.Itoa(i+1)) + } + utils.Kubectl("apply", "-f", case31LMPolicy, "-n", "managed") + By("bind policy and configurationpolicy") + parent := utils.GetWithTimeout(clientManagedDynamic, gvrPolicy, + case31LMPolicyName, testNamespace, true, defaultTimeoutSeconds) + Expect(parent).NotTo(BeNil()) + + plcDef := utils.ParseYaml(case31LMConfigPolicy) + ownerRefs := plcDef.GetOwnerReferences() + ownerRefs[0].UID = parent.GetUID() + plcDef.SetOwnerReferences(ownerRefs) + _, err := clientManagedDynamic.Resource(gvrConfigPolicy).Namespace(testNamespace). + Create(context.TODO(), plcDef, metav1.CreateOptions{}) + Expect(err).To(BeNil()) + + By("check configurationpolicy exist") + Eventually(func() interface{} { + plc := utils.GetWithTimeout(clientManagedDynamic, gvrConfigPolicy, + case31LMConfigPolicyName, testNamespace, true, defaultTimeoutSeconds) + compliant := utils.GetComplianceState(plc) + + return compliant + }, 30, 5).Should(Equal("NonCompliant")) + + By("check message longer than 1024") + Eventually(func() int { + event := utils.GetMatchingEvents(clientManaged, testNamespace, + case31LMPolicyName, case31LMConfigPolicyName, "NonCompliant", defaultTimeoutSeconds) + + Expect(len(event)).ShouldNot(BeZero()) + message := event[len(event)-1].Message + + return len(message) + }, 30, 5).Should(BeNumerically(">", 1024)) + }) + AfterAll(func() { + utils.Kubectl("delete", "policy", case31LMPolicyName, "-n", + "managed", "--ignore-not-found") + configlPlc := utils.GetWithTimeout(clientManagedDynamic, gvrConfigPolicy, + case31LMPolicyName, "managed", false, defaultTimeoutSeconds, + ) + Expect(configlPlc).To(BeNil()) + utils.Kubectl("delete", "event", + "--field-selector=involvedObject.name="+case31LMPolicyName, "-n", "managed") + utils.Kubectl("delete", "event", + "--field-selector=involvedObject.name="+case31LMConfigPolicy, "-n", "managed") + for i := range [15]int{} { + utils.Kubectl("delete", "ns", namespacePrefix+strconv.Itoa(i+1), + "--ignore-not-found", "--force", "--grace-period=0") + } + }) + }) }) diff --git a/test/resources/case31_policy_history/long-message-config-policy.yaml b/test/resources/case31_policy_history/long-message-config-policy.yaml new file mode 100644 index 00000000..3b15c19a --- /dev/null +++ b/test/resources/case31_policy_history/long-message-config-policy.yaml @@ -0,0 +1,47 @@ +apiVersion: policy.open-cluster-management.io/v1 +kind: ConfigurationPolicy +metadata: + name: long-message-config-policy + ownerReferences: + - apiVersion: policy.open-cluster-management.io/v1 + blockOwnerDeletion: false + controller: true + kind: Policy + name: long-message-policy + uid: 08bae967-4262-498a-84e9-d1f0e321b41e +spec: + pruneObjectBehavior: DeleteAll + remediationAction: inform + namespaceSelector: + exclude: + - kube-* + include: + - innovafertanimvsmvtatasdicereformascorporinnovafertanimvsmvt1 + - innovafertanimvsmvtatasdicereformascorporinnovafertanimvsmvt2 + - innovafertanimvsmvtatasdicereformascorporinnovafertanimvsmvt3 + - innovafertanimvsmvtatasdicereformascorporinnovafertanimvsmvt4 + - innovafertanimvsmvtatasdicereformascorporinnovafertanimvsmvt5 + - innovafertanimvsmvtatasdicereformascorporinnovafertanimvsmvt6 + - innovafertanimvsmvtatasdicereformascorporinnovafertanimvsmvt7 + - innovafertanimvsmvtatasdicereformascorporinnovafertanimvsmvt8 + - innovafertanimvsmvtatasdicereformascorporinnovafertanimvsmvt9 + - innovafertanimvsmvtatasdicereformascorporinnovafertanimvsmvt10 + - innovafertanimvsmvtatasdicereformascorporinnovafertanimvsmvt11 + - innovafertanimvsmvtatasdicereformascorporinnovafertanimvsmvt12 + - innovafertanimvsmvtatasdicereformascorporinnovafertanimvsmvt13 + - innovafertanimvsmvtatasdicereformascorporinnovafertanimvsmvt14 + - innovafertanimvsmvtatasdicereformascorporinnovafertanimvsmvt15 + object-templates: + - complianceType: musthave + objectDefinition: + apiVersion: v1 + kind: Pod + metadata: + name: case5-multi-namespace-inform-pod + spec: + containers: + - image: nginx:1.7.9 + imagePullPolicy: Never + name: nginx + ports: + - containerPort: 80 \ No newline at end of file diff --git a/test/resources/case31_policy_history/long-message-policy.yaml b/test/resources/case31_policy_history/long-message-policy.yaml new file mode 100644 index 00000000..c3ee2a74 --- /dev/null +++ b/test/resources/case31_policy_history/long-message-policy.yaml @@ -0,0 +1,50 @@ +apiVersion: policy.open-cluster-management.io/v1 +kind: Policy +metadata: + resourceVersion: "306" + name: long-message-policy +spec: + remediationAction: inform + disabled: false + policy-templates: + - objectDefinition: + apiVersion: policy.open-cluster-management.io/v1 + kind: ConfigurationPolicy + metadata: + name: long-message-config-policy + spec: + namespaceselector: + exclude: + - kube-* + include: + - innovafertanimvsmvtatasdicereformascorporinnovafertanimvsmvt1 + - innovafertanimvsmvtatasdicereformascorporinnovafertanimvsmvt2 + - innovafertanimvsmvtatasdicereformascorporinnovafertanimvsmvt3 + - innovafertanimvsmvtatasdicereformascorporinnovafertanimvsmvt4 + - innovafertanimvsmvtatasdicereformascorporinnovafertanimvsmvt5 + - innovafertanimvsmvtatasdicereformascorporinnovafertanimvsmvt6 + - innovafertanimvsmvtatasdicereformascorporinnovafertanimvsmvt7 + - innovafertanimvsmvtatasdicereformascorporinnovafertanimvsmvt8 + - innovafertanimvsmvtatasdicereformascorporinnovafertanimvsmvt9 + - innovafertanimvsmvtatasdicereformascorporinnovafertanimvsmvt10 + - innovafertanimvsmvtatasdicereformascorporinnovafertanimvsmvt11 + - innovafertanimvsmvtatasdicereformascorporinnovafertanimvsmvt12 + - innovafertanimvsmvtatasdicereformascorporinnovafertanimvsmvt13 + - innovafertanimvsmvtatasdicereformascorporinnovafertanimvsmvt14 + - innovafertanimvsmvtatasdicereformascorporinnovafertanimvsmvt15 + object-templates: + - complianceType: musthave + objectDefinition: + apiVersion: v1 + kind: Pod + metadata: + name: case5-multi-namespace-inform-pod + spec: + containers: + - image: nginx:1.7.9 + imagePullPolicy: Never + name: nginx + ports: + - containerPort: 80 + remediationAction: inform + severity: low