Skip to content

Commit

Permalink
Fix compliance when created resource has a status
Browse files Browse the repository at this point in the history
Previously, the template would always be marked as compliant after the
resource it defines was created. But if the object definition included
a `status`, then the object might not actually match what was desired.
Now, in this case, the template will be marked as non-compliant.

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

Signed-off-by: Justin Kulikauskas <jkulikau@redhat.com>
  • Loading branch information
JustinKuli authored and openshift-merge-robot committed Sep 7, 2023
1 parent 4f3c09a commit 04a91c0
Show file tree
Hide file tree
Showing 6 changed files with 198 additions and 1 deletion.
13 changes: 12 additions & 1 deletion controllers/configurationpolicy_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -1703,7 +1703,18 @@ func (r *ConfigurationPolicyReconciler) handleSingleObj(
var uid string
completed, reason, msg, uid, err := r.enforceByCreatingOrDeleting(obj)

result.events = append(result.events, objectTmplEvalEvent{completed, reason, msg})
hasStatus := false
if tmplObj, err := unmarshalFromJSON(objectT.ObjectDefinition.Raw); err == nil {
_, hasStatus = tmplObj.Object["status"]
}

if completed && hasStatus {
msg += ", the status of the object will be verified in the next evaluation"
reason += ", status unchecked"
result.events = append(result.events, objectTmplEvalEvent{false, reason, msg})
} else {
result.events = append(result.events, objectTmplEvalEvent{completed, reason, msg})
}

if err != nil {
// violation created for handling error
Expand Down
78 changes: 78 additions & 0 deletions test/e2e/case34_enforce_w_status_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,78 @@
// 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 compliance events of enforced policies that define a status", Ordered, func() {
const (
rsrcPath = "../resources/case34_enforce_w_status/"
policyYAML = rsrcPath + "policy.yaml"
policyName = "case34-parent"
cfgPlcYAML = rsrcPath + "config-policy.yaml"
updatedCfgPlc = rsrcPath + "config-policy-updated.yaml"
cfgPlcName = "case34-cfgpol"
nestedPlcYAML = rsrcPath + "nested-cfgpol-updated.yaml"
nestedPlcName = "case34-cfgpol-nested"
)

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;", 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())

By("Updating the policy")
utils.Kubectl("apply", "-f", updatedCfgPlc, "-n", testNamespace)

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

By("Updating the nested policy to increment its generation")
utils.Kubectl("apply", "-f", nestedPlcYAML, "-n", testNamespace)
nestedPlc := utils.GetWithTimeout(clientManagedDynamic, gvrConfigPolicy,
nestedPlcName, testNamespace, true, defaultTimeoutSeconds)
Expect(nestedPlc.GetGeneration()).To(BeNumerically("==", 3))

By("Checking there is now a Compliant event on the policy")
Eventually(func() interface{} {
return utils.GetMatchingEvents(clientManaged, testNamespace,
policyName, cfgPlcName, "^Compliant;", defaultTimeoutSeconds)
}, defaultTimeoutSeconds, 5).ShouldNot(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", "configurationpolicy", nestedPlcName, "-n", "managed", "--ignore-not-found")
nestedPlc := utils.GetWithTimeout(clientManagedDynamic, gvrConfigPolicy,
nestedPlcName, "managed", false, defaultTimeoutSeconds,
)
Expect(nestedPlc).To(BeNil())
utils.Kubectl("delete", "event", "--field-selector=involvedObject.name="+policyName, "-n", "managed")
utils.Kubectl("delete", "event", "--field-selector=involvedObject.name="+cfgPlcName, "-n", "managed")
utils.Kubectl("delete", "event", "--field-selector=involvedObject.name="+nestedPlcName, "-n", "managed")
})
})
27 changes: 27 additions & 0 deletions test/resources/case34_enforce_w_status/config-policy-updated.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
apiVersion: policy.open-cluster-management.io/v1
kind: ConfigurationPolicy
metadata:
name: case34-cfgpol
spec:
remediationAction: enforce
severity: low
object-templates:
- complianceType: musthave
objectDefinition:
apiVersion: policy.open-cluster-management.io/v1
kind: ConfigurationPolicy
metadata:
name: case34-cfgpol-nested
namespace: managed
spec:
remediationAction: inform
severity: low
object-templates:
- complianceType: musthave
objectDefinition:
apiVersion: v1
kind: Namespace
metadata:
name: default
status:
lastEvaluatedGeneration: 3
33 changes: 33 additions & 0 deletions test/resources/case34_enforce_w_status/config-policy.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,33 @@
apiVersion: policy.open-cluster-management.io/v1
kind: ConfigurationPolicy
metadata:
name: case34-cfgpol
ownerReferences:
- apiVersion: policy.open-cluster-management.io/v1
blockOwnerDeletion: false
controller: true
kind: Policy
name: case34-parent
uid: 08bae967-4262-498a-84e9-d1f0e321b41e # to be replaced!
spec:
remediationAction: enforce
severity: low
object-templates:
- complianceType: musthave
objectDefinition:
apiVersion: policy.open-cluster-management.io/v1
kind: ConfigurationPolicy
metadata:
name: case34-cfgpol-nested
namespace: managed
spec:
remediationAction: inform
object-templates:
- complianceType: musthave
objectDefinition:
apiVersion: v1
kind: Namespace
metadata:
name: default
status:
lastEvaluatedGeneration: 3
14 changes: 14 additions & 0 deletions test/resources/case34_enforce_w_status/nested-cfgpol-updated.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
apiVersion: policy.open-cluster-management.io/v1
kind: ConfigurationPolicy
metadata:
name: case34-cfgpol-nested
spec:
remediationAction: inform
severity: high # updated field!
object-templates:
- complianceType: musthave
objectDefinition:
apiVersion: v1
kind: Namespace
metadata:
name: default
34 changes: 34 additions & 0 deletions test/resources/case34_enforce_w_status/policy.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,34 @@
apiVersion: policy.open-cluster-management.io/v1
kind: Policy
metadata:
name: case34-parent
spec:
disabled: false
remediationAction: enforce
policy-templates:
- objectDefinition:
apiVersion: policy.open-cluster-management.io/v1
kind: ConfigurationPolicy
metadata:
name: case34-cfgpol
spec:
remediationAction: enforce
object-templates:
- complianceType: musthave
objectDefinition:
apiVersion: policy.open-cluster-management.io/v1
kind: ConfigurationPolicy
metadata:
name: case34-cfgpol-nested
namespace: managed
spec:
remediationAction: inform
object-templates:
- complianceType: musthave
objectDefinition:
apiVersion: v1
kind: Namespace
metadata:
name: default
status:
lastEvaluatedGeneration: 3

0 comments on commit 04a91c0

Please sign in to comment.