Fire event on object update

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"