From 9f22a7cd8860841325b404ef9c93aec6dfd85077 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 | 75 +++++++++++++++---- test/e2e/case27_showupdateinstatus_test.go | 51 +++++++++++++ .../case27-create-cfgmap-policy.yaml | 18 +++++ .../case27-update-cfgmap-policy.yaml | 18 +++++ 4 files changed, 148 insertions(+), 14 deletions(-) create mode 100644 test/e2e/case27_showupdateinstatus_test.go create mode 100644 test/resources/case27_showupdateinstatus/case27-create-cfgmap-policy.yaml create mode 100644 test/resources/case27_showupdateinstatus/case27-update-cfgmap-policy.yaml diff --git a/controllers/configurationpolicy_controller.go b/controllers/configurationpolicy_controller.go index 5cfd1ee1..4bb3e12c 100644 --- a/controllers/configurationpolicy_controller.go +++ b/controllers/configurationpolicy_controller.go @@ -1459,6 +1459,17 @@ func (r *ConfigurationPolicyReconciler) handleSingleObj( if !exists && obj.shouldExist { // it is a musthave and it does not exist, so it must be created if strings.EqualFold(string(remediation), string(policyv1.Enforce)) { + // object is missing, so send noncompliant event + _ = createStatus("", obj.gvr.Resource, compliantObject, obj.namespaced, obj.policy, + obj.index, false, true) + obj.policy.Status.ComplianceState = policyv1.NonCompliant + statusStr := convertPolicyStatusToString(obj.policy) + objLog.Info("Sending an update policy status event", "policy", obj.policy.Name, "status", statusStr) + r.Recorder.Event(obj.policy, eventWarning, fmt.Sprintf(eventFmtStr, obj.policy.GetName(), obj.name), + statusStr) + // update parent policy status + r.addForUpdate(obj.policy, true) + var uid string statusUpdateNeeded, uid, err = r.enforceByCreatingOrDeleting(obj) @@ -1471,6 +1482,7 @@ func (r *ConfigurationPolicyReconciler) handleSingleObj( CreatedByPolicy: &created, UID: uid, } + compliant = true } } else { // inform compliant = false @@ -1514,7 +1526,21 @@ func (r *ConfigurationPolicyReconciler) handleSingleObj( before := time.Now().UTC() - throwSpecViolation, msg, pErr := r.checkAndUpdateResource(obj, compType, mdCompType, remediation) + throwSpecViolation, msg, pErr, triedUpdate, updatedObj := r.checkAndUpdateResource(obj, compType, mdCompType, + remediation) + + if triedUpdate { + // object has a mismatch and needs an update to be enforced, throw violation for mismatch + _ = createStatus("", obj.gvr.Resource, compliantObject, obj.namespaced, obj.policy, obj.index, + false, true) + obj.policy.Status.ComplianceState = policyv1.NonCompliant + statusStr := convertPolicyStatusToString(obj.policy) + objLog.Info("Sending an update policy status event", "policy", obj.policy.Name, "status", statusStr) + r.Recorder.Event(obj.policy, eventWarning, fmt.Sprintf(eventFmtStr, obj.policy.GetName(), obj.name), + statusStr) + // update parent policy status + r.addForUpdate(obj.policy, true) + } duration := time.Now().UTC().Sub(before) seconds := float64(duration) / float64(time.Second) @@ -1538,8 +1564,17 @@ 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 + 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, @@ -1559,7 +1594,14 @@ func (r *ConfigurationPolicyReconciler) handleSingleObj( compliant = false } + if compliant { + obj.policy.Status.ComplianceState = policyv1.Compliant + } else { + obj.policy.Status.ComplianceState = policyv1.NonCompliant + } + statusStr := convertPolicyStatusToString(obj.policy) + log.V(1).Info("Sending an update policy status event", "policy", obj.policy.Name, "status", statusStr) r.Recorder.Event(obj.policy, eventType, fmt.Sprintf(eventFmtStr, obj.policy.GetName(), obj.name), statusStr) @@ -1845,6 +1887,8 @@ func (r *ConfigurationPolicyReconciler) enforceByCreatingOrDeleting(obj singleOb if !uidIsString || err != nil { log.Error(err, "Tried to set UID in status but the field is not a string") } + + completed = true } } else { log.Info("Enforcing the policy by deleting the object") @@ -2319,13 +2363,15 @@ func (r *ConfigurationPolicyReconciler) validateObject(object *unstructured.Unst } // checkAndUpdateResource checks each individual key of a resource and passes it to handleKeys to see if it -// matches the template and update it if the remediationAction is enforce +// matches the template and update it if the remediationAction is enforce. UpdateNeeded indicates whether the +// function tried to update the child object and updateSucceeded indicates whether the update was applied +// successfully. func (r *ConfigurationPolicyReconciler) checkAndUpdateResource( obj singleObject, complianceType string, mdComplianceType string, remediation policyv1.RemediationAction, -) (throwSpecViolation bool, message string, processingErr bool) { +) (throwSpecViolation bool, message string, processingErr bool, updateNeeded bool, updateSucceeded bool) { log := log.WithValues( "policy", obj.policy.Name, "name", obj.name, "namespace", obj.namespace, "resource", obj.gvr.Resource, ) @@ -2333,7 +2379,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, false } var res dynamic.ResourceInterface @@ -2344,9 +2390,10 @@ func (r *ConfigurationPolicyReconciler) checkAndUpdateResource( } var err error - var updateNeeded bool var statusUpdated bool + updateSucceeded = false + for key := range obj.unstruct.Object { isStatus := key == "status" @@ -2363,7 +2410,7 @@ func (r *ConfigurationPolicyReconciler) checkAndUpdateResource( if errorMsg != "" { log.Info(errorMsg) - return false, errorMsg, true + return false, errorMsg, true, false, false } if mergedObj == nil && skipped { @@ -2382,7 +2429,7 @@ func (r *ConfigurationPolicyReconciler) checkAndUpdateResource( if keyUpdateNeeded { if strings.EqualFold(string(remediation), string(policyv1.Inform)) { - return true, "", false + return true, "", false, false, false } else if isStatus { statusUpdated = true log.Info("Ignoring an update to the object status", "key", key) @@ -2399,26 +2446,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, updateNeeded, 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, updateNeeded, 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, updateNeeded, false } - log.Info("Updated the object based on the template definition") + updateSucceeded = true } - return statusUpdated, "", false + return statusUpdated, "", false, updateNeeded, updateSucceeded } // 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..4d94802e --- /dev/null +++ b/test/e2e/case27_showupdateinstatus_test.go @@ -0,0 +1,51 @@ +// 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-cfgmap-create" + case27CreateYaml string = "../resources/case27_showupdateinstatus/case27-create-cfgmap-policy.yaml" + case27UpdateYaml string = "../resources/case27_showupdateinstatus/case27-update-cfgmap-policy.yaml" +) + +var _ = Describe("Verify status update after updating object", Ordered, func() { + It("configmap 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( + "configmaps [case27-map] in namespace default found as specified, " + + "therefore this Object template is compliant")) + }) + It("configmap and status should be updated properly on the managed cluster", func() { + By("Updating " + 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( + "configmaps [case27-map] in namespace default was updated successfully")) + }) + + AfterAll(func() { + deleteConfigPolicies([]string{case27ConfigPolicyName}) + utils.Kubectl("delete", "configmap", "case27-map") + }) +}) diff --git a/test/resources/case27_showupdateinstatus/case27-create-cfgmap-policy.yaml b/test/resources/case27_showupdateinstatus/case27-create-cfgmap-policy.yaml new file mode 100644 index 00000000..0868ce52 --- /dev/null +++ b/test/resources/case27_showupdateinstatus/case27-create-cfgmap-policy.yaml @@ -0,0 +1,18 @@ +apiVersion: policy.open-cluster-management.io/v1 +kind: ConfigurationPolicy +metadata: + name: policy-cfgmap-create +spec: + remediationAction: enforce + namespaceSelector: + exclude: ["kube-*"] + include: ["default"] + object-templates: + - complianceType: musthave + objectDefinition: + apiVersion: v1 + kind: ConfigMap + metadata: + name: case27-map + data: + fieldToUpdate: "1" diff --git a/test/resources/case27_showupdateinstatus/case27-update-cfgmap-policy.yaml b/test/resources/case27_showupdateinstatus/case27-update-cfgmap-policy.yaml new file mode 100644 index 00000000..6ed25c06 --- /dev/null +++ b/test/resources/case27_showupdateinstatus/case27-update-cfgmap-policy.yaml @@ -0,0 +1,18 @@ +apiVersion: policy.open-cluster-management.io/v1 +kind: ConfigurationPolicy +metadata: + name: policy-cfgmap-create +spec: + remediationAction: enforce + namespaceSelector: + exclude: ["kube-*"] + include: ["default"] + object-templates: + - complianceType: musthave + objectDefinition: + apiVersion: v1 + kind: ConfigMap + metadata: + name: case27-map + data: + fieldToUpdate: "2"