diff --git a/controllers/configurationpolicy_controller.go b/controllers/configurationpolicy_controller.go index 9fd9b8de..599ab83b 100644 --- a/controllers/configurationpolicy_controller.go +++ b/controllers/configurationpolicy_controller.go @@ -1041,9 +1041,9 @@ func (r *ConfigurationPolicyReconciler) handleObjectTemplates(plc policyv1.Confi nonCompliantObjects := map[string]map[string]interface{}{} compliantObjects := map[string]map[string]interface{}{} enforce := strings.EqualFold(string(plc.Spec.RemediationAction), string(policyv1.Enforce)) - kind := "" + kind := templateObjs[indx].kind objShouldExist := !strings.EqualFold(string(objectT.ComplianceType), string(policyv1.MustNotHave)) - + mergeMessageEnforce := false // If the object does not have a namespace specified, use the previously retrieved namespaces // from the NamespaceSelector. If no namespaces are found/specified, use the value from the // object so that the objectTemplate is processed: @@ -1057,19 +1057,15 @@ func (r *ConfigurationPolicyReconciler) handleObjectTemplates(plc policyv1.Confi relevantNamespaces = []string{templateObjs[indx].namespace} } + if enforce && len(relevantNamespaces) > 1 { + mergeMessageEnforce = true + } + numCompliant := 0 numNonCompliant := 0 handled := false - // iterate through all namespaces the configurationpolicy is set on for _, ns := range relevantNamespaces { - log.Info( - "Handling the object template for the relevant namespace", - "namespace", ns, - "desiredName", templateObjs[indx].name, - "index", indx, - ) - names, compliant, reason, objKind, related, statusUpdateNeeded := r.handleObjects( objectT, ns, templateObjs[indx], indx, &plc, ) @@ -1079,20 +1075,32 @@ func (r *ConfigurationPolicyReconciler) handleObjectTemplates(plc policyv1.Confi } if objKind != "" { + // object template enforced, objKind is empty kind = objKind } + if mergeMessageEnforce { + names = append([]string{}, templateObjs[indx].name) + } + + // object template enforced, already handled in handleObjects if names == nil { - // object template enforced, already handled in handleObjects handled = true } else { enforce = false + } + + // violations for enforce configurationpolicies are already handled in handleObjects, + // so we only need to generate a violation if the remediationAction is set to inform + // Or multiple namespaces and enforce use this + if mergeMessageEnforce || (!handled && !enforce) { if !compliant { if len(names) == 0 { numNonCompliant++ } else { numNonCompliant += len(names) } + nonCompliantObjects[ns] = map[string]interface{}{ "names": names, "reason": reason, @@ -1110,28 +1118,79 @@ func (r *ConfigurationPolicyReconciler) handleObjectTemplates(plc policyv1.Confi relatedObjects = updateRelatedObjectsStatus(relatedObjects, object) } } - // violations for enforce configurationpolicies are already handled in handleObjects, - // so we only need to generate a violation if the remediationAction is set to inform - if !handled && !enforce { - objData := map[string]interface{}{ - "indx": indx, - "kind": kind, - "desiredName": templateObjs[indx].name, - "namespaced": templateObjs[indx].isNamespaced, - } - statusUpdateNeeded := createInformStatus( + objData := map[string]interface{}{ + "indx": indx, + "kind": kind, + "desiredName": templateObjs[indx].name, + "namespaced": templateObjs[indx].isNamespaced, + } + if !handled && !enforce { + statusUpdateNeeded := createMergedStatus( objShouldExist, numCompliant, numNonCompliant, compliantObjects, nonCompliantObjects, &plc, objData, ) if statusUpdateNeeded { parentStatusUpdateNeeded = true } } + + // When enforce and multiple namespaces, it creates integrated messages, + if mergeMessageEnforce { + parentStatusUpdateNeeded = createMergedStatus( + objShouldExist, numCompliant, numNonCompliant, compliantObjects, nonCompliantObjects, &plc, objData, + ) + + r.Recorder.Event(&plc, eventNormal, fmt.Sprintf(plcFmtStr, plc.GetName()), + convertPolicyStatusToString(&plc)) + } } r.checkRelatedAndUpdate(plc, relatedObjects, oldRelated, parentStatusUpdateNeeded) } +// createMergedStatus updates the status field for a configurationpolicy with remediationAction=inform +// based on how many compliant/noncompliant objects are found when processing the templates in the configurationpolicy +// Or multiple namespaces and enforce use this +func createMergedStatus( + objShouldExist bool, + numCompliant, + numNonCompliant int, + compliantObjects, + nonCompliantObjects map[string]map[string]interface{}, + plc *policyv1.ConfigurationPolicy, + objData map[string]interface{}, +) bool { + //nolint:forcetypeassert + desiredName := objData["desiredName"].(string) + //nolint:forcetypeassert + indx := objData["indx"].(int) + //nolint:forcetypeassert + kind := objData["kind"].(string) + //nolint:forcetypeassert + namespaced := objData["namespaced"].(bool) + + if kind == "" { + return false + } + + var compObjs map[string]map[string]interface{} + var compliant bool + + if numNonCompliant > 0 { + compliant = false + compObjs = nonCompliantObjects + } else if objShouldExist && numCompliant == 0 { + // Special case: No resources found is NonCompliant + compliant = false + compObjs = nonCompliantObjects + } else { + compliant = true + compObjs = compliantObjects + } + + return createStatus(desiredName, kind, compObjs, namespaced, plc, indx, compliant, objShouldExist) +} + // checkRelatedAndUpdate checks the related objects field and triggers an update on the ConfigurationPolicy func (r *ConfigurationPolicyReconciler) checkRelatedAndUpdate( plc policyv1.ConfigurationPolicy, related, oldRelated []policyv1.RelatedObject, sendEvent bool, @@ -1294,48 +1353,6 @@ func addConditionToStatus( return updateNeeded } -// createInformStatus updates the status field for a configurationpolicy with remediationAction=inform -// based on how many compliant/noncompliant objects are found when processing the templates in the configurationpolicy -func createInformStatus( - objShouldExist bool, - numCompliant, - numNonCompliant int, - compliantObjects, - nonCompliantObjects map[string]map[string]interface{}, - plc *policyv1.ConfigurationPolicy, - objData map[string]interface{}, -) bool { - //nolint:forcetypeassert - desiredName := objData["desiredName"].(string) - //nolint:forcetypeassert - indx := objData["indx"].(int) - //nolint:forcetypeassert - kind := objData["kind"].(string) - //nolint:forcetypeassert - namespaced := objData["namespaced"].(bool) - - if kind == "" { - return false - } - - var compObjs map[string]map[string]interface{} - var compliant bool - - if numNonCompliant > 0 { - compliant = false - compObjs = nonCompliantObjects - } else if objShouldExist && numCompliant == 0 { - // Special case: No resources found is NonCompliant - compliant = false - compObjs = nonCompliantObjects - } else { - compliant = true - compObjs = compliantObjects - } - - return createStatus(desiredName, kind, compObjs, namespaced, plc, indx, compliant, objShouldExist) -} - // handleObjects controls the processing of each individual object template within a configurationpolicy func (r *ConfigurationPolicyReconciler) handleObjects( objectT *policyv1.ObjectTemplate, diff --git a/controllers/configurationpolicy_controller_test.go b/controllers/configurationpolicy_controller_test.go index d58f5bf6..8fcb4585 100644 --- a/controllers/configurationpolicy_controller_test.go +++ b/controllers/configurationpolicy_controller_test.go @@ -435,7 +435,7 @@ func TestSortRelatedObjectsAndUpdate(t *testing.T) { assert.True(t, relatedList[0].Object.Metadata.Name == "bar") } -func TestCreateInformStatus(t *testing.T) { +func TestCreateMergedStatus(t *testing.T) { policy := &policyv1.ConfigurationPolicy{ ObjectMeta: metav1.ObjectMeta{ Name: "foo", @@ -473,7 +473,7 @@ func TestCreateInformStatus(t *testing.T) { } // Test 1 NonCompliant resource - createInformStatus(!mustNotHave, numCompliant, numNonCompliant, + createMergedStatus(!mustNotHave, numCompliant, numNonCompliant, compliantObjects, nonCompliantObjects, policy, objData) assert.True(t, policy.Status.CompliancyDetails[0].ComplianceState == policyv1.NonCompliant) @@ -484,7 +484,7 @@ func TestCreateInformStatus(t *testing.T) { numNonCompliant = 2 // Test 2 NonCompliant resources - createInformStatus(!mustNotHave, numCompliant, numNonCompliant, + createMergedStatus(!mustNotHave, numCompliant, numNonCompliant, compliantObjects, nonCompliantObjects, policy, objData) assert.True(t, policy.Status.CompliancyDetails[0].ComplianceState == policyv1.NonCompliant) @@ -493,7 +493,7 @@ func TestCreateInformStatus(t *testing.T) { // Test 0 resources numNonCompliant = 0 - createInformStatus(!mustNotHave, numCompliant, numNonCompliant, + createMergedStatus(!mustNotHave, numCompliant, numNonCompliant, compliantObjects, nonCompliantObjects, policy, objData) assert.True(t, policy.Status.CompliancyDetails[0].ComplianceState == policyv1.NonCompliant) @@ -509,7 +509,7 @@ func TestCreateInformStatus(t *testing.T) { numNonCompliant = 1 // Test 1 compliant and 1 noncompliant resource NOTE: This use case is the new behavior change! - createInformStatus(!mustNotHave, numCompliant, numNonCompliant, + createMergedStatus(!mustNotHave, numCompliant, numNonCompliant, compliantObjects, nonCompliantObjects, policy, objData) assert.True(t, policy.Status.CompliancyDetails[0].ComplianceState == policyv1.NonCompliant) @@ -523,7 +523,7 @@ func TestCreateInformStatus(t *testing.T) { delete(nonCompliantObjects, "test2") // Test 2 compliant resources - createInformStatus(!mustNotHave, numCompliant, numNonCompliant, + createMergedStatus(!mustNotHave, numCompliant, numNonCompliant, compliantObjects, nonCompliantObjects, policy, objData) assert.True(t, policy.Status.CompliancyDetails[0].ComplianceState == policyv1.Compliant) } diff --git a/test/e2e/case5_multi_test.go b/test/e2e/case5_multi_test.go index 7fcaa9e6..0aa7e58f 100644 --- a/test/e2e/case5_multi_test.go +++ b/test/e2e/case5_multi_test.go @@ -87,4 +87,87 @@ var _ = Describe("Test multiple obj template handling", func() { deleteConfigPolicies(policies) }) }) + + Describe("Check messages when it is multiple namesapces and multiple obj-templates", Ordered, func() { + const ( + case5MultiNamespace1 string = "n1" + case5MultiNamespace2 string = "n2" + case5MultiNamespace3 string = "n3" + case5MultiNSConfigPolicyName string = "policy-multi-namespace-enforce" + case5MultiNSInformConfigPolicyName string = "policy-multi-namespace-inform" + case5MultiObjNSConfigPolicyName string = "policy-pod-multi-obj-temp-enforce" + case5InformYaml string = "../resources/case5_multi/case5_multi_namespace_inform.yaml" + case5EnforceYaml string = "../resources/case5_multi/case5_multi_namespace_enforce.yaml" + case5MultiObjTmpYaml string = "../resources/case5_multi/case5_multi_obj_template_enforce.yaml" + ) + BeforeAll(func() { + nss := []string{ + case5MultiNamespace1, + case5MultiNamespace2, + case5MultiNamespace3, + } + + for _, ns := range nss { + utils.Kubectl("create", "ns", ns) + } + }) + It("Should show merged Noncompliant messages when it is multiple namespaces and inform", func() { + expectedMsg := "pods not found: [case5-multi-namespace-inform-pod] " + + "in namespace n1 missing; [case5-multi-namespace-inform-pod] " + + "in namespace n2 missing; [case5-multi-namespace-inform-pod] " + + "in namespace n3 missing" + utils.Kubectl("apply", "-f", case5InformYaml) + utils.DoConfigPolicyMessageTest(clientManagedDynamic, gvrConfigPolicy, testNamespace, + case5MultiNSInformConfigPolicyName, 0, defaultTimeoutSeconds, expectedMsg) + }) + It("Should show merged messages when it is multiple namespaces", func() { + expectedMsg := "Pod [case5-multi-namespace-enforce-pod] in namespace n1 found; " + + "[case5-multi-namespace-enforce-pod] in namespace n2 found; " + + "[case5-multi-namespace-enforce-pod] in namespace n3 found " + + "as specified, therefore this Object template is compliant" + utils.Kubectl("apply", "-f", case5EnforceYaml) + utils.DoConfigPolicyMessageTest(clientManagedDynamic, gvrConfigPolicy, testNamespace, + case5MultiNSConfigPolicyName, 0, defaultTimeoutSeconds, expectedMsg) + }) + It("Should show 3 merged messages when it is multiple namespaces and multiple obj-template", func() { + firstMsg := "Pod [case5-multi-obj-temp-pod-11] in namespace n1 found; " + + "[case5-multi-obj-temp-pod-11] in namespace n2 found; " + + "[case5-multi-obj-temp-pod-11] in namespace n3 found " + + "as specified, therefore this Object template is compliant" + secondMsg := "Pod [case5-multi-obj-temp-pod-22] in namespace n1 found; " + + "[case5-multi-obj-temp-pod-22] in namespace n2 found; " + + "[case5-multi-obj-temp-pod-22] in namespace n3 found " + + "as specified, therefore this Object template is compliant" + thirdMsg := "Pod [case5-multi-obj-temp-pod-33] in namespace n1 found; " + + "[case5-multi-obj-temp-pod-33] in namespace n2 found; " + + "[case5-multi-obj-temp-pod-33] in namespace n3 found " + + "as specified, therefore this Object template is compliant" + utils.Kubectl("apply", "-f", case5MultiObjTmpYaml) + utils.DoConfigPolicyMessageTest(clientManagedDynamic, gvrConfigPolicy, testNamespace, + case5MultiObjNSConfigPolicyName, 0, defaultTimeoutSeconds, firstMsg) + utils.DoConfigPolicyMessageTest(clientManagedDynamic, gvrConfigPolicy, testNamespace, + case5MultiObjNSConfigPolicyName, 1, defaultTimeoutSeconds, secondMsg) + utils.DoConfigPolicyMessageTest(clientManagedDynamic, gvrConfigPolicy, testNamespace, + case5MultiObjNSConfigPolicyName, 2, defaultTimeoutSeconds, thirdMsg) + }) + cleanup := func() { + policies := []string{ + case5MultiNSConfigPolicyName, + case5MultiNSInformConfigPolicyName, + case5MultiObjNSConfigPolicyName, + } + + deleteConfigPolicies(policies) + nss := []string{ + case5MultiNamespace1, + case5MultiNamespace2, + case5MultiNamespace3, + } + + for _, ns := range nss { + utils.Kubectl("delete", "ns", ns, "--ignore-not-found") + } + } + AfterAll(cleanup) + }) }) diff --git a/test/e2e/case8_status_check_test.go b/test/e2e/case8_status_check_test.go index 1e41fd99..50f95d54 100644 --- a/test/e2e/case8_status_check_test.go +++ b/test/e2e/case8_status_check_test.go @@ -25,7 +25,7 @@ const ( ) var _ = Describe("Test pod obj template handling", func() { - Describe("Create a policy on managed cluster in ns:"+testNamespace, func() { + Describe("Create a policy on managed cluster in ns:"+testNamespace, Ordered, func() { It("should create a policy properly on the managed cluster", func() { By("Creating " + case8ConfigPolicyNamePod + " on managed") utils.Kubectl("apply", "-f", case8PolicyYamlPod, "-n", testNamespace) @@ -89,7 +89,7 @@ var _ = Describe("Test pod obj template handling", func() { deleteConfigPolicies(policies) }) }) - Describe("Create a policy with status on managed cluster in ns:"+testNamespace, func() { + Describe("Create a policy with status on managed cluster in ns:"+testNamespace, Ordered, func() { It("should create a policy properly on the managed cluster", func() { By("Creating " + case8ConfigPolicyStatusPod + " on managed") utils.Kubectl("apply", "-f", case8PolicyYamlBadPod, "-n", testNamespace) @@ -140,11 +140,13 @@ var _ = Describe("Test pod obj template handling", func() { return utils.GetComplianceState(managedPlc) }, defaultTimeoutSeconds, 1).Should(Equal("Compliant")) }) - It("Cleans up", func() { + AfterEach(func() { policies := []string{ case8ConfigPolicyStatusPod, } deleteConfigPolicies(policies) + + utils.Kubectl("delete", "pod", "nginx-badpod-e2e-8", "-n", "default", "--ignore-not-found") }) }) }) diff --git a/test/resources/case5_multi/case5_multi_namespace_enforce.yaml b/test/resources/case5_multi/case5_multi_namespace_enforce.yaml new file mode 100644 index 00000000..ccc35be9 --- /dev/null +++ b/test/resources/case5_multi/case5_multi_namespace_enforce.yaml @@ -0,0 +1,24 @@ +apiVersion: policy.open-cluster-management.io/v1 +kind: ConfigurationPolicy +metadata: + name: policy-multi-namespace-enforce + namespace: managed +spec: + remediationAction: enforce + pruneObjectBehavior: DeleteAll + namespaceSelector: + exclude: ["kube-*"] + include: ["n1","n2","n3"] + object-templates: + - complianceType: musthave + objectDefinition: + apiVersion: v1 + kind: Pod + metadata: + name: case5-multi-namespace-enforce-pod + spec: + containers: + - image: nginx:1.7.9 + name: nginx + ports: + - containerPort: 80 diff --git a/test/resources/case5_multi/case5_multi_namespace_inform.yaml b/test/resources/case5_multi/case5_multi_namespace_inform.yaml new file mode 100644 index 00000000..c538a76f --- /dev/null +++ b/test/resources/case5_multi/case5_multi_namespace_inform.yaml @@ -0,0 +1,24 @@ +apiVersion: policy.open-cluster-management.io/v1 +kind: ConfigurationPolicy +metadata: + name: policy-multi-namespace-inform + namespace: managed +spec: + remediationAction: inform + pruneObjectBehavior: DeleteAll + namespaceSelector: + exclude: ["kube-*"] + include: ["n1","n2","n3"] + object-templates: + - complianceType: musthave + objectDefinition: + apiVersion: v1 + kind: Pod + metadata: + name: case5-multi-namespace-inform-pod + spec: + containers: + - image: nginx:1.7.9 + name: nginx + ports: + - containerPort: 80 diff --git a/test/resources/case5_multi/case5_multi_obj_template_enforce.yaml b/test/resources/case5_multi/case5_multi_obj_template_enforce.yaml new file mode 100644 index 00000000..0fa1a9d2 --- /dev/null +++ b/test/resources/case5_multi/case5_multi_obj_template_enforce.yaml @@ -0,0 +1,49 @@ +apiVersion: policy.open-cluster-management.io/v1 +kind: ConfigurationPolicy +metadata: + name: policy-pod-multi-obj-temp-enforce + namespace: managed +spec: + remediationAction: enforce + pruneObjectBehavior: DeleteAll + namespaceSelector: + exclude: ["kube-*"] + include: ["n1","n2","n3"] + object-templates: + - complianceType: musthave + objectDefinition: + apiVersion: v1 + kind: Pod + metadata: + name: case5-multi-obj-temp-pod-11 + spec: + containers: + - image: nginx:1.7.9 + name: nginx + ports: + - containerPort: 80 + - complianceType: musthave + objectDefinition: + apiVersion: v1 + kind: Pod + metadata: + name: case5-multi-obj-temp-pod-22 + spec: + containers: + - image: nginx:1.7.9 + name: nginx + ports: + - containerPort: 80 + - complianceType: musthave + objectDefinition: + apiVersion: v1 + kind: Pod + metadata: + name: case5-multi-obj-temp-pod-33 + spec: + containers: + - image: nginx:1.7.9 + name: nginx + ports: + - containerPort: 80 + diff --git a/test/utils/utils.go b/test/utils/utils.go index 1e4de09d..2663c8da 100644 --- a/test/utils/utils.go +++ b/test/utils/utils.go @@ -292,3 +292,51 @@ func GetMetrics(metricPatterns ...string) []string { return values } + +func getConfigPolicyStatusMessages(clientHubDynamic dynamic.Interface, + gvrConfigPolicy schema.GroupVersionResource, namespace string, policyName string, templateIdx int, +) (string, bool, error) { + empty := "" + policyInterface := clientHubDynamic.Resource(gvrConfigPolicy).Namespace(namespace) + + configPolicy, err := policyInterface.Get(context.TODO(), policyName, metav1.GetOptions{}) + if err != nil { + return empty, false, fmt.Errorf("error in getting policy") + } + + details, found, err := unstructured.NestedSlice(configPolicy.Object, "status", "compliancyDetails") + if !found || err != nil || len(details) <= templateIdx { + return empty, false, fmt.Errorf("error in getting status") + } + + templateDetails, ok := details[templateIdx].(map[string]interface{}) + if !ok { + return empty, false, fmt.Errorf("error in getting detail") + } + + conditions, ok, _ := unstructured.NestedSlice(templateDetails, "conditions") + + if !ok { + return empty, false, fmt.Errorf("error conditions not found") + } + + condition := conditions[0].(map[string]interface{}) + + message, ok, err := unstructured.NestedString(condition, "message") + + return message, ok, err +} + +func DoConfigPolicyMessageTest(clientHubDynamic dynamic.Interface, + gvrConfigPolicy schema.GroupVersionResource, namespace string, policyName string, + templateIdx int, timeout int, expectedMsg string, +) { + Eventually(func(g Gomega) string { + message, _, err := getConfigPolicyStatusMessages(clientHubDynamic, + gvrConfigPolicy, namespace, policyName, templateIdx) + + g.Expect(err).Should(BeNil()) + + return message + }, timeout, 1).Should(Equal(expectedMsg)) +}