Fire event on object update

Fires an event when an object is updated by the config policy controller, rather than just when one is created or deleted. Also changes events sent by successful enforce actions to compliant and adds a noncompliant "object not found" event before the enforce event.

refs:

https://issues.redhat.com/browse/ACM-2021?filter=-1

Signed-off-by: Will Kutler <wkutler@redhat.com>

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,21 +2363,23 @@ 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,
  )
 
  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

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")
+ })
+})

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"

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"