Skip to content

Commit

Permalink
Add an error when apiVersion is missing
Browse files Browse the repository at this point in the history
The apiVersion field is generally required by kubernetes objects... it's
surprising that this situation wasn't already caught by an error when
decoding or getting a mapping.

Refs:
 - https://issues.redhat.com/browse/ACM-7127

Signed-off-by: Justin Kulikauskas <jkulikau@redhat.com>
  • Loading branch information
JustinKuli committed Sep 20, 2023
1 parent de86a78 commit 3e02a50
Show file tree
Hide file tree
Showing 5 changed files with 127 additions and 0 deletions.
14 changes: 14 additions & 0 deletions controllers/configurationpolicy_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -1849,6 +1849,20 @@ func (r *ConfigurationPolicyReconciler) getMapping(
return nil, result
}

if gvk.Group == "" && gvk.Version == "" {
err := fmt.Errorf("object template at index [%v] in policy `%v` missing apiVersion", index, policy.Name)

log.Error(err, "Can not get mapping for object")

result = &objectTmplEvalResult{
events: []objectTmplEvalEvent{
{compliant: false, reason: "K8s object definition error", message: err.Error()},
},
}

return nil, result
}

// initializes a mapping between Kind and APIVersion to a resource name
r.lock.RLock()
mapper := restmapper.NewDiscoveryRESTMapper(r.apiGroups)
Expand Down
18 changes: 18 additions & 0 deletions test/e2e/case34_enforce_w_status_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,24 @@ var _ = Describe("Test compliance events of enforced policies that define a stat
})

AfterAll(func() {
if CurrentSpecReport().Failed() {
By("Test failed, getting relevant compliance events for debugging")
events := utils.GetMatchingEvents(clientManaged, testNamespace,
policyName, cfgPlcName, ".*", defaultTimeoutSeconds)

for _, ev := range events {
GinkgoWriter.Println("---")
GinkgoWriter.Println("Name:", ev.Name)
GinkgoWriter.Println("Reason:", ev.Reason)
GinkgoWriter.Println("Message:", ev.Message)
GinkgoWriter.Println("FirstTimestamp:", ev.FirstTimestamp)
GinkgoWriter.Println("LastTimestamp:", ev.LastTimestamp)
GinkgoWriter.Println("Count:", ev.Count)
GinkgoWriter.Println("Type:", ev.Type)
GinkgoWriter.Println("---")
}
}

utils.Kubectl("delete", "policy", policyName, "-n", "managed", "--ignore-not-found")
configPlc := utils.GetWithTimeout(clientManagedDynamic, gvrConfigPolicy,
cfgPlcName, "managed", false, defaultTimeoutSeconds,
Expand Down
48 changes: 48 additions & 0 deletions test/e2e/case35_no_apiversion_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,48 @@
// Copyright (c) 2023 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"
)

var _ = Describe("Test a policy with an objectDefinition that is missing apiVersion", Ordered, func() {
const (
rsrcPath = "../resources/case35_no_apiversion/"
policyYAML = rsrcPath + "policy.yaml"
policyName = "case34-parent"
cfgPlcYAML = rsrcPath + "config-policy.yaml"
cfgPlcName = "case34-cfgpol"
)

It("Should have the expected events", func() {
By("Setting up the policy")
createConfigPolicyWithParent(policyYAML, policyName, cfgPlcYAML)

By("Checking there is a NonCompliant event on the policy")
Eventually(func() interface{} {
return utils.GetMatchingEvents(clientManaged, testNamespace,
policyName, cfgPlcName, "^NonCompliant;.*missing apiVersion", defaultTimeoutSeconds)
}, defaultTimeoutSeconds, 5).ShouldNot(BeEmpty())

By("Checking there are no Compliant events on the policy")
Consistently(func() interface{} {
return utils.GetMatchingEvents(clientManaged, testNamespace,
policyName, cfgPlcName, "^Compliant;", defaultTimeoutSeconds)
}, defaultTimeoutSeconds, 5).Should(BeEmpty())
})

AfterAll(func() {
utils.Kubectl("delete", "policy", policyName, "-n", "managed", "--ignore-not-found")
configPlc := utils.GetWithTimeout(clientManagedDynamic, gvrConfigPolicy,
cfgPlcName, "managed", false, defaultTimeoutSeconds,
)
Expect(configPlc).To(BeNil())
utils.Kubectl("delete", "event", "--field-selector=involvedObject.name="+policyName, "-n", "managed")
utils.Kubectl("delete", "event", "--field-selector=involvedObject.name="+cfgPlcName, "-n", "managed")
})
})
23 changes: 23 additions & 0 deletions test/resources/case35_no_apiversion/config-policy.yaml
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: case35-cfgpol
ownerReferences:
- apiVersion: policy.open-cluster-management.io/v1
blockOwnerDeletion: false
controller: true
kind: Policy
name: case35-parent
uid: 08bae967-4262-498a-84e9-d1f0e321b41e # to be replaced!
spec:
remediationAction: enforce
severity: low
object-templates:
- complianceType: musthave
objectDefinition:
kind: ConfigMap
metadata:
name: case35-cfgmap
namespace: default
data:
foo: bar
24 changes: 24 additions & 0 deletions test/resources/case35_no_apiversion/policy.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
apiVersion: policy.open-cluster-management.io/v1
kind: Policy
metadata:
name: case35-parent
spec:
disabled: false
remediationAction: enforce
policy-templates:
- objectDefinition:
apiVersion: policy.open-cluster-management.io/v1
kind: ConfigurationPolicy
metadata:
name: case35-cfgpol
spec:
remediationAction: enforce
object-templates:
- complianceType: musthave
objectDefinition:
kind: ConfigMap
metadata:
name: case35-cfgmap
namespace: default
data:
foo: bar

0 comments on commit 3e02a50

Please sign in to comment.