Skip to content

Commit

Permalink
Fix order of events on create or delete
Browse files Browse the repository at this point in the history
Signed-off-by: Will Kutler <wkutler@redhat.com>
  • Loading branch information
willkutler authored and openshift-merge-robot committed Mar 27, 2023
1 parent 6e9bc97 commit da14bdd
Show file tree
Hide file tree
Showing 3 changed files with 18 additions and 9 deletions.
2 changes: 1 addition & 1 deletion Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -103,7 +103,7 @@ create-ns:
# Run against the current locally configured Kubernetes cluster
.PHONY: run
run:
WATCH_NAMESPACE=$(WATCH_NAMESPACE) go run ./main.go --leader-elect=false
WATCH_NAMESPACE=$(WATCH_NAMESPACE) go run ./main.go controller --leader-elect=false

############################################################
# clean section
Expand Down
18 changes: 11 additions & 7 deletions controllers/configurationpolicy_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -1574,7 +1574,6 @@ func (r *ConfigurationPolicyReconciler) handleSingleObj(
) {
objLog := log.WithValues("object", obj.name, "policy", obj.policy.Name, "index", obj.index)

var err error
var compliant bool

compliantObject := map[string]map[string]interface{}{
Expand All @@ -1587,18 +1586,20 @@ func (r *ConfigurationPolicyReconciler) handleSingleObj(
// it is a musthave and it does not exist, so it must be created
if strings.EqualFold(string(remediation), string(policyv1.Enforce)) {
var uid string
statusUpdateNeeded, uid, err = r.enforceByCreatingOrDeleting(obj)
completed, reason, msg, uid, err := r.enforceByCreatingOrDeleting(obj)

if err != nil {
// violation created for handling error
objLog.Error(err, "Could not handle missing musthave object")

statusUpdateNeeded = addConditionToStatus(obj.policy, obj.index, completed, reason, msg)
} else {
// object is missing and will be created, so send noncompliant "does not exist" event first
// (this check has already happened, but we send the event here to avoid the status flipping on an
// error)
_ = createStatus("", obj.gvr.Resource, compliantObject, obj.namespaced, obj.policy,
obj.index, false, true)
obj.policy.Status.ComplianceState = policyv1.NonCompliant

statusStr := convertPolicyStatusToString(obj.policy)
objLog.Info("Sending a noncompliant status event (object missing)", "policy", obj.policy.Name, "status",
statusStr)
Expand All @@ -1607,6 +1608,8 @@ func (r *ConfigurationPolicyReconciler) handleSingleObj(
// update parent policy status
r.addForUpdate(obj.policy, true)

statusUpdateNeeded = addConditionToStatus(obj.policy, obj.index, completed, reason, msg)

created := true
creationInfo = &policyv1.ObjectProperties{
CreatedByPolicy: &created,
Expand All @@ -1622,10 +1625,12 @@ func (r *ConfigurationPolicyReconciler) handleSingleObj(
if exists && !obj.shouldExist {
// it is a mustnothave but it exist, so it must be deleted
if strings.EqualFold(string(remediation), string(policyv1.Enforce)) {
statusUpdateNeeded, _, err = r.enforceByCreatingOrDeleting(obj)
completed, reason, msg, _, err := r.enforceByCreatingOrDeleting(obj)
if err != nil {
objLog.Error(err, "Could not handle existing mustnothave object")
}

statusUpdateNeeded = addConditionToStatus(obj.policy, obj.index, completed, reason, msg)
} else { // inform
compliant = false
}
Expand Down Expand Up @@ -1979,7 +1984,7 @@ func getNamesOfKind(
// mustnothave object does exist. Eg, it does not handle the case where a targeted update would need
// to be made to an object.
func (r *ConfigurationPolicyReconciler) enforceByCreatingOrDeleting(obj singleObject) (
result bool, uid string, erro error,
result bool, reason string, msg string, uid string, erro error,
) {
log := log.WithValues(
"object", obj.name,
Expand All @@ -1997,7 +2002,6 @@ func (r *ConfigurationPolicyReconciler) enforceByCreatingOrDeleting(obj singleOb
}

var completed bool
var reason, msg string
var err error

if obj.shouldExist {
Expand Down Expand Up @@ -2033,7 +2037,7 @@ func (r *ConfigurationPolicyReconciler) enforceByCreatingOrDeleting(obj singleOb
}
}

return addConditionToStatus(obj.policy, obj.index, completed, reason, msg), uid, err
return completed, reason, msg, uid, err
}

// checkMessageSimilarity decides whether to append a new condition to a configurationPolicy status
Expand Down
7 changes: 6 additions & 1 deletion test/e2e/case15_event_format_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -166,8 +166,13 @@ var _ = Describe("Testing compliance event formatting", func() {
compPlcEvents := utils.GetMatchingEvents(clientManaged, testNamespace,
case15BecomesCompliantName, "", "Policy status is: Compliant", defaultTimeoutSeconds)
Expect(compPlcEvents).NotTo(BeEmpty())
compParentEventsPreCreation := utils.GetMatchingEvents(clientManaged, testNamespace,
case15BecomesCompliantParentName, "policy: "+testNamespace+"/"+case15BecomesCompliantName,
"^NonCompliant;.*No instances of.*found as specified", defaultTimeoutSeconds)
Expect(compParentEventsPreCreation).NotTo(BeEmpty())
compParentEvents := utils.GetMatchingEvents(clientManaged, testNamespace, case15BecomesCompliantParentName,
"policy: "+testNamespace+"/"+case15BecomesCompliantName, "^Compliant;", defaultTimeoutSeconds)
"policy: "+testNamespace+"/"+case15BecomesCompliantName,
"^Compliant;.*and was created successfully$", defaultTimeoutSeconds)
Expect(compParentEvents).NotTo(BeEmpty())
})
It("Records events for a policy that becomes noncompliant", func() {
Expand Down

0 comments on commit da14bdd

Please sign in to comment.