Fix handling of undefined fields for mustonlyhave

If a field exists in the object but wasn't defined
for a mustonlyhave policy, we need to check the
existing object's fields and set them accordingly,
particularly to set them to null.

Signed-off-by: Dale Haiducek <19750917+dhaiducek@users.noreply.github.com>

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,36 @@ 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
+ }
+
+ // check key for mismatch
+ errorMsg, keyUpdateNeeded, mergedObj, skipped := handleSingleKey(
+ key, desiredObj, existingObjectCopy, compType, zeroValueEqualsNil,
+ )
+ if errorMsg != "" {
+ log.Info(errorMsg)
+
+ return true, errorMsg, true, statusMismatch
+ }
+
+ // If the mergedObj is nil, we'll use the value to erase it from the existing object.
+ if skipped {
+ continue
+ }
+
+ if keyUpdateNeeded {
+ existingObj.Object[key] = mergedObj
+ updateNeeded = true
+ }
+ }
+ }
+
  return
 }
 

test/e2e/case2_role_handling_test.go

@@ -6,6 +6,7 @@ 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"
 )
@@ -22,6 +23,11 @@ var _ = Describe("Test role obj template handling", Ordered, func() {
  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")
@@ -31,9 +37,11 @@ var _ = Describe("Test role obj template handling", Ordered, func() {
  "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() {
@@ -44,7 +52,7 @@ var _ = Describe("Test role obj template handling", Ordered, func() {
  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"))
@@ -106,6 +114,60 @@ var _ = Describe("Test role obj template handling", Ordered, func() {
  }, defaultTimeoutSeconds, 1).Should(Equal("Compliant"))
  })
 
+ 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