Fix handling of undefined fields for mustonlyhave

controllers/configurationpolicy_controller.go

@@ -2750,7 +2750,11 @@ func handleKeys(
 	mdCompType string,
 	zeroValueEqualsNil bool,
 ) (throwSpecViolation bool, message string, updateNeeded bool, statusMismatch bool) {
+	handledKeys := map[string]bool{}
+
+	// Iterate over keys of the desired object to compare with the existing object on the cluster
 	for key := range desiredObj.Object {
+		handledKeys[key] = true
 		isStatus := key == "status"
 
 		// use metadatacompliancetype to evaluate metadata if it is set
@@ -2798,6 +2802,20 @@ func handleKeys(
 		}
 	}
 
+	// If the complianceType is "mustonlyhave", then compare the existing object's keys,
+	// skipping over: previously compared keys, metadata, and status.
+	if compType == "mustonlyhave" {
+		for key := range existingObj.Object {
+			if handledKeys[key] || key == "status" || key == "metadata" {
+				continue
+			}
+
+			delete(existingObj.Object, key)
+
+			updateNeeded = true
+		}
+	}
+
 	return
 }
 

test/e2e/case2_role_handling_test.go

@@ -6,60 +6,83 @@ package e2e
 import (
 	. "github.com/onsi/ginkgo/v2"
 	. "github.com/onsi/gomega"
+	"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
 
 	"open-cluster-management.io/config-policy-controller/test/utils"
 )
 
-const (
-	case2ConfigPolicyNameInform  string = "policy-role-create-inform"
-	case2ConfigPolicyNameEnforce string = "policy-role-create"
-	case2roleName                string = "pod-reader-e2e"
-	case2PolicyYamlInform        string = "../resources/case2_role_handling/case2_role_create_inform.yaml"
-	case2PolicyYamlEnforce       string = "../resources/case2_role_handling/case2_role_create_enforce.yaml"
-	case2PolicyCheckMNHYaml      string = "../resources/case2_role_handling/case2_role_check-mnh.yaml"
-	case2PolicyCheckMOHYaml      string = "../resources/case2_role_handling/case2_role_check-moh.yaml"
-	case2PolicyCheckCompliant    string = "../resources/case2_role_handling/case2_role_check-c.yaml"
-)
-
 var _ = Describe("Test role obj template handling", Ordered, func() {
 	Describe("Create a policy on managed cluster in ns:"+testNamespace, Ordered, func() {
+		const (
+			resourcePrefix                 string = "../resources/case2_role_handling/"
+			configPolicyNameInform         string = "policy-role-create-inform"
+			configPolicyNameEnforce        string = "policy-role-create"
+			roleName                       string = "pod-reader-e2e"
+			policyYamlInform               string = resourcePrefix + "case2_role_create_inform.yaml"
+			policyYamlEnforce              string = resourcePrefix + "case2_role_create_enforce.yaml"
+			policyCheckMNHYaml             string = resourcePrefix + "case2_role_check-mnh.yaml"
+			policyCheckMOHYaml             string = resourcePrefix + "case2_role_check-moh.yaml"
+			policyCheckCompliant           string = resourcePrefix + "case2_role_check-c.yaml"
+			configPolicyNameBindingEnforce string = "policy-rolebinding-create"
+			bindingName                    string = "pod-reader-e2e-binding"
+			policyYamlBindingEnforce       string = resourcePrefix + "case2_rolebinding_create_enforce.yaml"
+			policyYamlBindingPatch         string = resourcePrefix + "case2_rolebinding_create_patch.yaml"
+		)
+
+		AfterAll(func() {
+			By("clean up case2")
+			policies := []string{
+				configPolicyNameInform,
+				configPolicyNameEnforce,
+				"policy-role-check-comp",
+				"policy-role-check-mnh",
+				"policy-role-check-moh",
+				configPolicyNameBindingEnforce,
+			}
+			deleteConfigPolicies(policies)
+			utils.Kubectl("delete", "role", roleName, "-n", "default", "--ignore-not-found")
+			utils.Kubectl("delete", "rolebinding", bindingName, "-n", "default", "--ignore-not-found")
+		})
+
 		It("should be created properly on the managed cluster", func() {
-			By("Creating " + case2PolicyYamlInform + " on managed")
-			utils.Kubectl("apply", "-f", case2PolicyYamlInform, "-n", testNamespace)
+			By("Creating " + policyYamlInform + " on managed")
+			utils.Kubectl("apply", "-f", policyYamlInform, "-n", testNamespace)
 			plc := utils.GetWithTimeout(clientManagedDynamic, gvrConfigPolicy,
-				case2ConfigPolicyNameInform, testNamespace, true, defaultTimeoutSeconds)
+				configPolicyNameInform, testNamespace, true, defaultTimeoutSeconds)
 			Expect(plc).NotTo(BeNil())
 			Eventually(func() interface{} {
 				managedPlc := utils.GetWithTimeout(clientManagedDynamic, gvrConfigPolicy,
-					case2ConfigPolicyNameInform, testNamespace, true, defaultTimeoutSeconds)
+					configPolicyNameInform, testNamespace, true, defaultTimeoutSeconds)
 
 				return utils.GetComplianceState(managedPlc)
 			}, defaultTimeoutSeconds, 1).Should(Equal("NonCompliant"))
 		})
+
 		It("should create role on managed cluster", func() {
-			By("creating " + case2PolicyYamlEnforce + " on hub with spec.remediationAction = enforce")
-			utils.Kubectl("apply", "-f", case2PolicyYamlEnforce, "-n", testNamespace)
+			By("creating " + policyYamlEnforce + " on hub with spec.remediationAction = enforce")
+			utils.Kubectl("apply", "-f", policyYamlEnforce, "-n", testNamespace)
 			plc := utils.GetWithTimeout(clientManagedDynamic, gvrConfigPolicy,
-				case2ConfigPolicyNameEnforce, testNamespace, true, defaultTimeoutSeconds)
+				configPolicyNameEnforce, testNamespace, true, defaultTimeoutSeconds)
 			Expect(plc).NotTo(BeNil())
 			Eventually(func() interface{} {
 				managedPlc := utils.GetWithTimeout(clientManagedDynamic, gvrConfigPolicy,
-					case2ConfigPolicyNameEnforce, testNamespace, true, defaultTimeoutSeconds)
+					configPolicyNameEnforce, testNamespace, true, defaultTimeoutSeconds)
 
 				return utils.GetComplianceState(managedPlc)
 			}, defaultTimeoutSeconds, 1).Should(Equal("Compliant"))
 			Eventually(func() interface{} {
 				informPlc := utils.GetWithTimeout(clientManagedDynamic, gvrConfigPolicy,
-					case2ConfigPolicyNameInform, testNamespace, true, defaultTimeoutSeconds)
+					configPolicyNameInform, testNamespace, true, defaultTimeoutSeconds)
 
 				return utils.GetComplianceState(informPlc)
 			}, defaultTimeoutSeconds, 1).Should(Equal("Compliant"))
-			role := utils.GetWithTimeout(clientManagedDynamic, gvrRole, case2roleName,
+			role := utils.GetWithTimeout(clientManagedDynamic, gvrRole, roleName,
 				"default", true, defaultTimeoutSeconds)
 			Expect(role).NotTo(BeNil())
 		})
+
 		It("should create statuses properly", func() {
-			utils.Kubectl("apply", "-f", case2PolicyCheckMNHYaml, "-n", testNamespace)
+			utils.Kubectl("apply", "-f", policyCheckMNHYaml, "-n", testNamespace)
 			plc := utils.GetWithTimeout(clientManagedDynamic, gvrConfigPolicy,
 				"policy-role-check-mnh", testNamespace, true, defaultTimeoutSeconds)
 			Expect(plc).NotTo(BeNil())
@@ -69,7 +92,7 @@ var _ = Describe("Test role obj template handling", Ordered, func() {
 
 				return utils.GetComplianceState(managedPlc)
 			}, defaultTimeoutSeconds, 1).Should(Equal("NonCompliant"))
-			utils.Kubectl("apply", "-f", case2PolicyCheckMOHYaml, "-n", testNamespace)
+			utils.Kubectl("apply", "-f", policyCheckMOHYaml, "-n", testNamespace)
 			plc = utils.GetWithTimeout(clientManagedDynamic, gvrConfigPolicy,
 				"policy-role-check-moh", testNamespace, true, defaultTimeoutSeconds)
 			Expect(plc).NotTo(BeNil())
@@ -79,7 +102,7 @@ var _ = Describe("Test role obj template handling", Ordered, func() {
 
 				return utils.GetComplianceState(managedPlc)
 			}, defaultTimeoutSeconds, 1).Should(Equal("NonCompliant"))
-			utils.Kubectl("apply", "-f", case2PolicyCheckCompliant, "-n", testNamespace)
+			utils.Kubectl("apply", "-f", policyCheckCompliant, "-n", testNamespace)
 			plc = utils.GetWithTimeout(clientManagedDynamic, gvrConfigPolicy,
 				"policy-role-check-comp", testNamespace, true, defaultTimeoutSeconds)
 			Expect(plc).NotTo(BeNil())
@@ -90,17 +113,61 @@ var _ = Describe("Test role obj template handling", Ordered, func() {
 				return utils.GetComplianceState(managedPlc)
 			}, defaultTimeoutSeconds, 1).Should(Equal("Compliant"))
 		})
-		AfterAll(func() {
-			By("clean up case2")
-			policies := []string{
-				case2ConfigPolicyNameInform,
-				case2ConfigPolicyNameEnforce,
-				"policy-role-check-comp",
-				"policy-role-check-mnh",
-				"policy-role-check-moh",
-			}
-			deleteConfigPolicies(policies)
-			utils.Kubectl("delete", "role", "pod-reader-e2e", "-n", "default", "--ignore-not-found")
+
+		It("should create rolebinding on managed cluster", func() {
+			By("creating " + policyYamlBindingEnforce + " on hub with spec.remediationAction = enforce")
+			utils.Kubectl("apply", "-f", policyYamlBindingEnforce, "-n", testNamespace)
+			plc := utils.GetWithTimeout(clientManagedDynamic, gvrConfigPolicy,
+				configPolicyNameBindingEnforce, testNamespace, true, defaultTimeoutSeconds)
+			Expect(plc).NotTo(BeNil())
+			Eventually(func() interface{} {
+				managedPlc := utils.GetWithTimeout(clientManagedDynamic, gvrConfigPolicy,
+					configPolicyNameBindingEnforce, testNamespace, true, defaultTimeoutSeconds)
+
+				return utils.GetComplianceState(managedPlc)
+			}, defaultTimeoutSeconds, 1).Should(Equal("Compliant"))
+
+			Eventually(func() interface{} {
+				informPlc := utils.GetWithTimeout(clientManagedDynamic, gvrConfigPolicy,
+					configPolicyNameBindingEnforce, testNamespace, true, defaultTimeoutSeconds)
+
+				return utils.GetComplianceState(informPlc)
+			}, defaultTimeoutSeconds, 1).Should(Equal("Compliant"))
+			binding := utils.GetWithTimeout(clientManagedDynamic, gvrRoleBinding, bindingName,
+				"default", true, defaultTimeoutSeconds)
+			Expect(binding).NotTo(BeNil())
+			subjects, _, err := unstructured.NestedSlice(binding.Object, "subjects")
+			Expect(err).ToNot(HaveOccurred())
+			Expect(subjects).To(HaveLen(1))
+		})
+
+		It("should patch the rolebinding on managed cluster to an empty subjects", func() {
+			By("creating " + policyYamlBindingPatch + " on hub with spec.remediationAction = inform")
+			utils.Kubectl("apply", "-f", policyYamlBindingPatch, "-n", testNamespace)
+			plc := utils.GetWithTimeout(clientManagedDynamic, gvrConfigPolicy,
+				configPolicyNameBindingEnforce, testNamespace, true, defaultTimeoutSeconds)
+			Expect(plc).NotTo(BeNil())
+			Eventually(func() interface{} {
+				managedPlc := utils.GetWithTimeout(clientManagedDynamic, gvrConfigPolicy,
+					configPolicyNameBindingEnforce, testNamespace, true, defaultTimeoutSeconds)
+
+				return utils.GetComplianceState(managedPlc)
+			}, defaultTimeoutSeconds, 1).Should(Equal("NonCompliant"))
+			By("patching policy spec.remediationAction = enforce")
+			utils.Kubectl("patch", "configurationpolicy", configPolicyNameBindingEnforce, `--type=json`,
+				`-p=[{"op":"replace","path":"/spec/remediationAction","value":"enforce"}]`, "-n", testNamespace)
+			Eventually(func() interface{} {
+				informPlc := utils.GetWithTimeout(clientManagedDynamic, gvrConfigPolicy,
+					configPolicyNameBindingEnforce, testNamespace, true, defaultTimeoutSeconds)
+
+				return utils.GetComplianceState(informPlc)
+			}, defaultTimeoutSeconds, 1).Should(Equal("Compliant"))
+			binding := utils.GetWithTimeout(clientManagedDynamic, gvrRoleBinding, bindingName,
+				"default", true, defaultTimeoutSeconds)
+			Expect(binding).NotTo(BeNil())
+			subjects, _, err := unstructured.NestedSlice(binding.Object, "subjects")
+			Expect(err).ToNot(HaveOccurred())
+			Expect(subjects).To(BeNil())
 		})
 	})
 })

test/e2e/e2e_suite_test.go

@@ -40,6 +40,7 @@ var (
 	gvrCRD                      schema.GroupVersionResource
 	gvrPod                      schema.GroupVersionResource
 	gvrRole                     schema.GroupVersionResource
+	gvrRoleBinding              schema.GroupVersionResource
 	gvrNS                       schema.GroupVersionResource
 	gvrSCC                      schema.GroupVersionResource
 	gvrSecret                   schema.GroupVersionResource
@@ -75,6 +76,11 @@ var _ = BeforeSuite(func() {
 	gvrNS = schema.GroupVersionResource{Group: "", Version: "v1", Resource: "namespaces"}
 	gvrConfigMap = schema.GroupVersionResource{Group: "", Version: "v1", Resource: "configmaps"}
 	gvrRole = schema.GroupVersionResource{Group: "rbac.authorization.k8s.io", Version: "v1", Resource: "roles"}
+	gvrRoleBinding = schema.GroupVersionResource{
+		Group:    "rbac.authorization.k8s.io",
+		Version:  "v1",
+		Resource: "rolebindings",
+	}
 	gvrConfigPolicy = schema.GroupVersionResource{
 		Group:    "policy.open-cluster-management.io",
 		Version:  "v1",

test/resources/case2_role_handling/case2_rolebinding_create_enforce.yaml

@@ -0,0 +1,23 @@
+apiVersion: policy.open-cluster-management.io/v1
+kind: ConfigurationPolicy
+metadata:
+  name: policy-rolebinding-create
+spec:
+  remediationAction: enforce
+  namespaceSelector:
+    include: ["default"]
+  object-templates:
+    - complianceType: mustonlyhave
+      objectDefinition:
+        apiVersion: rbac.authorization.k8s.io/v1
+        kind: RoleBinding
+        metadata:
+          name: pod-reader-e2e-binding
+        roleRef:
+          apiGroup: rbac.authorization.k8s.io
+          kind: Role
+          name: pod-reader-e2e
+        subjects:
+          - apiGroup: rbac.authorization.k8s.io
+            kind: Group
+            name: system:authenticated:oauth

test/resources/case2_role_handling/case2_rolebinding_create_patch.yaml

@@ -0,0 +1,19 @@
+apiVersion: policy.open-cluster-management.io/v1
+kind: ConfigurationPolicy
+metadata:
+  name: policy-rolebinding-create
+spec:
+  remediationAction: inform
+  namespaceSelector:
+    include: ["default"]
+  object-templates:
+    - complianceType: mustonlyhave
+      objectDefinition:
+        apiVersion: rbac.authorization.k8s.io/v1
+        kind: RoleBinding
+        metadata:
+          name: pod-reader-e2e-binding
+        roleRef:
+          apiGroup: rbac.authorization.k8s.io
+          kind: Role
+          name: pod-reader-e2e

test/utils/utils.go

@@ -155,9 +155,7 @@ func Kubectl(args ...string) {
 	output, err := cmd.CombinedOutput()
 	if err != nil {
 		// in case of failure, print command output (including error)
-		//nolint:forbidigo
-		fmt.Printf("%s\n", output)
-		Fail(fmt.Sprintf("Error: %v", err))
+		Fail(fmt.Sprintf("Error running 'kubectl %s'\n: %s: %v", strings.Join(cmd.Args, " "), output, err), 1)
 	}
 }