🤖 Sync from open-cluster-management-io/config-policy-controller: #90

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"