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 order of events on create or delete #112

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
2 changes: 1 addition & 1 deletion Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -193,7 +193,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 @@ -1575,7 +1575,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 @@ -1588,18 +1587,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 @@ -1608,6 +1609,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 @@ -1623,10 +1626,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 @@ -1980,7 +1985,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 @@ -1998,7 +2003,6 @@ func (r *ConfigurationPolicyReconciler) enforceByCreatingOrDeleting(obj singleOb
}

var completed bool
var reason, msg string
var err error

if obj.shouldExist {
Expand Down Expand Up @@ -2034,7 +2038,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