From 74add6a6fc8f1ebd54ab4c2b2e9fb9cac66bf444 Mon Sep 17 00:00:00 2001 From: Will Kutler Date: Fri, 6 Jan 2023 11:09:59 -0500 Subject: [PATCH] Fire event on object update Signed-off-by: Will Kutler --- controllers/configurationpolicy_controller.go | 37 ++++++++----- test/e2e/case27_showupdateinstatus_test.go | 52 +++++++++++++++++++ .../case27-create-pod-policy.yaml | 22 ++++++++ .../case27-update-pod-policy.yaml | 24 +++++++++ 4 files changed, 123 insertions(+), 12 deletions(-) create mode 100644 test/e2e/case27_showupdateinstatus_test.go create mode 100644 test/resources/case27_showupdateinstatus/case27-create-pod-policy.yaml create mode 100644 test/resources/case27_showupdateinstatus/case27-update-pod-policy.yaml diff --git a/controllers/configurationpolicy_controller.go b/controllers/configurationpolicy_controller.go index 5cfd1ee1..40e3c506 100644 --- a/controllers/configurationpolicy_controller.go +++ b/controllers/configurationpolicy_controller.go @@ -1514,7 +1514,7 @@ func (r *ConfigurationPolicyReconciler) handleSingleObj( before := time.Now().UTC() - throwSpecViolation, msg, pErr := r.checkAndUpdateResource(obj, compType, mdCompType, remediation) + throwSpecViolation, msg, pErr, updatedObj := r.checkAndUpdateResource(obj, compType, mdCompType, remediation) duration := time.Now().UTC().Sub(before) seconds := float64(duration) / float64(time.Second) @@ -1538,8 +1538,19 @@ func (r *ConfigurationPolicyReconciler) handleSingleObj( // it is a must have and it does exist, so it is compliant compliant = true if strings.EqualFold(string(remediation), string(policyv1.Enforce)) { - statusUpdateNeeded = createStatus("", obj.gvr.Resource, compliantObject, obj.namespaced, obj.policy, - obj.index, compliant, true) + if updatedObj { + // object updated in checkAndUpdateResource, send event + + log.Info("Updated must have object", "resource", obj.gvr.Resource, "name", obj.name) + reason := "K8s update success" + idStr := identifierStr([]string{obj.name}, obj.namespace, obj.namespaced) + msg := fmt.Sprintf("%v %v was updated successfully", obj.gvr.Resource, idStr) + + statusUpdateNeeded = addConditionToStatus(obj.policy, obj.index, true, reason, msg) + } else { + statusUpdateNeeded = createStatus("", obj.gvr.Resource, compliantObject, obj.namespaced, obj.policy, + obj.index, compliant, true) + } created := false creationInfo = &policyv1.ObjectProperties{ CreatedByPolicy: &created, @@ -2325,7 +2336,7 @@ func (r *ConfigurationPolicyReconciler) checkAndUpdateResource( complianceType string, mdComplianceType string, remediation policyv1.RemediationAction, -) (throwSpecViolation bool, message string, processingErr bool) { +) (throwSpecViolation bool, message string, processingErr bool, updatedObj bool) { log := log.WithValues( "policy", obj.policy.Name, "name", obj.name, "namespace", obj.namespace, "resource", obj.gvr.Resource, ) @@ -2333,7 +2344,7 @@ func (r *ConfigurationPolicyReconciler) checkAndUpdateResource( if obj.object == nil { log.Info("Skipping update: Previous object retrieval from the API server failed") - return false, "", false + return false, "", false, false } var res dynamic.ResourceInterface @@ -2347,6 +2358,8 @@ func (r *ConfigurationPolicyReconciler) checkAndUpdateResource( var updateNeeded bool var statusUpdated bool + updatedObj = false + for key := range obj.unstruct.Object { isStatus := key == "status" @@ -2363,7 +2376,7 @@ func (r *ConfigurationPolicyReconciler) checkAndUpdateResource( if errorMsg != "" { log.Info(errorMsg) - return false, errorMsg, true + return false, errorMsg, true, false } if mergedObj == nil && skipped { @@ -2382,7 +2395,7 @@ func (r *ConfigurationPolicyReconciler) checkAndUpdateResource( if keyUpdateNeeded { if strings.EqualFold(string(remediation), string(policyv1.Inform)) { - return true, "", false + return true, "", false, false } else if isStatus { statusUpdated = true log.Info("Ignoring an update to the object status", "key", key) @@ -2399,26 +2412,26 @@ func (r *ConfigurationPolicyReconciler) checkAndUpdateResource( if err := r.validateObject(obj.object); err != nil { message := fmt.Sprintf("Error validating the object %s, the error is `%v`", obj.name, err) - return false, message, true + return false, message, true, false } _, err = res.Update(context.TODO(), obj.object, metav1.UpdateOptions{}) if k8serrors.IsNotFound(err) { message := fmt.Sprintf("`%v` is not present and must be created", obj.object.GetKind()) - return false, message, true + return false, message, true, false } if err != nil { message := fmt.Sprintf("Error updating the object `%v`, the error is `%v`", obj.name, err) - return false, message, true + return false, message, true, false } - log.Info("Updated the object based on the template definition") + updatedObj = true } - return statusUpdated, "", false + return statusUpdated, "", false, updatedObj } // AppendCondition check and appends conditions to the policy status diff --git a/test/e2e/case27_showupdateinstatus_test.go b/test/e2e/case27_showupdateinstatus_test.go new file mode 100644 index 00000000..11d1f3da --- /dev/null +++ b/test/e2e/case27_showupdateinstatus_test.go @@ -0,0 +1,52 @@ +// Copyright (c) 2020 Red Hat, Inc. +// Copyright Contributors to the Open Cluster Management project + +package e2e + +import ( + . "github.com/onsi/ginkgo/v2" + . "github.com/onsi/gomega" + + "open-cluster-management.io/config-policy-controller/test/utils" +) + +const ( + case27ConfigPolicyName string = "policy-pod-create" + case27CreateYaml string = "../resources/case27_showupdateinstatus/case27-create-pod-policy.yaml" + case27UpdateYaml string = "../resources/case27_showupdateinstatus/case27-update-pod-policy.yaml" +) + +var _ = Describe("Test cluster version obj template handling", func() { + Describe("enforce patch on unnamespaced resource clusterversion "+testNamespace, func() { + It("should be created properly on the managed cluster", func() { + By("Creating " + case27ConfigPolicyName + " on managed") + utils.Kubectl("apply", "-f", case27CreateYaml, "-n", testNamespace) + plc := utils.GetWithTimeout(clientManagedDynamic, gvrConfigPolicy, + case27ConfigPolicyName, testNamespace, true, defaultTimeoutSeconds) + Expect(plc).NotTo(BeNil()) + Eventually(func() interface{} { + managedPlc := utils.GetWithTimeout(clientManagedDynamic, gvrConfigPolicy, + case27ConfigPolicyName, testNamespace, true, defaultTimeoutSeconds) + + return utils.GetStatusMessage(managedPlc) + }, 120, 1).Should(Equal( + "pods [nginx-pod-e2e] in namespace default found as specified, " + + "therefore this Object template is compliant")) + }) + It("should be updated properly on the managed cluster", func() { + By("Creating " + case27ConfigPolicyName + " on managed") + utils.Kubectl("apply", "-f", case27UpdateYaml, "-n", testNamespace) + Eventually(func() interface{} { + managedPlc := utils.GetWithTimeout(clientManagedDynamic, gvrConfigPolicy, + case27ConfigPolicyName, testNamespace, true, defaultTimeoutSeconds) + + return utils.GetStatusMessage(managedPlc) + }, 30, 0.5).Should(Equal( + "pods [nginx-pod-e2e] in namespace default was updated successfully")) + }) + It("Cleans up", func() { + deleteConfigPolicies([]string{case27ConfigPolicyName}) + utils.Kubectl("delete", "pod", "nginx-pod-e2e") + }) + }) +}) diff --git a/test/resources/case27_showupdateinstatus/case27-create-pod-policy.yaml b/test/resources/case27_showupdateinstatus/case27-create-pod-policy.yaml new file mode 100644 index 00000000..f6fb9a8f --- /dev/null +++ b/test/resources/case27_showupdateinstatus/case27-create-pod-policy.yaml @@ -0,0 +1,22 @@ +apiVersion: policy.open-cluster-management.io/v1 +kind: ConfigurationPolicy +metadata: + name: policy-pod-create +spec: + remediationAction: enforce + namespaceSelector: + exclude: ["kube-*"] + include: ["default"] + object-templates: + - complianceType: musthave + objectDefinition: + apiVersion: v1 + kind: Pod + metadata: + name: nginx-pod-e2e + spec: + containers: + - image: nginx:1.7.9 + name: nginx + ports: + - containerPort: 80 diff --git a/test/resources/case27_showupdateinstatus/case27-update-pod-policy.yaml b/test/resources/case27_showupdateinstatus/case27-update-pod-policy.yaml new file mode 100644 index 00000000..886300b3 --- /dev/null +++ b/test/resources/case27_showupdateinstatus/case27-update-pod-policy.yaml @@ -0,0 +1,24 @@ +apiVersion: policy.open-cluster-management.io/v1 +kind: ConfigurationPolicy +metadata: + name: policy-pod-create +spec: + remediationAction: enforce + namespaceSelector: + exclude: ["kube-*"] + include: ["default"] + object-templates: + - complianceType: musthave + objectDefinition: + apiVersion: v1 + kind: Pod + metadata: + name: nginx-pod-e2e + labels: + test: test + spec: + containers: + - image: nginx:1.7.9 + name: nginx + ports: + - containerPort: 80