Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix handling of undefined fields for mustonlyhave #223

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 18 additions & 0 deletions controllers/configurationpolicy_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
}

Expand Down
314 changes: 158 additions & 156 deletions test/e2e/case12_list_compare_test.go

Large diffs are not rendered by default.

137 changes: 102 additions & 35 deletions test/e2e/case2_role_handling_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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())
Expand All @@ -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())
Expand All @@ -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())
Expand All @@ -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())
})
})
})
6 changes: 6 additions & 0 deletions test/e2e/e2e_suite_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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",
Expand Down
Original file line number Diff line number Diff line change
@@ -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
Original file line number Diff line number Diff line change
@@ -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
4 changes: 1 addition & 3 deletions test/utils/utils.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
}

Expand Down