Skip to content

Commit

Permalink
Consolidate compliance messages
Browse files Browse the repository at this point in the history 
Ref: https://issues.redhat.com/browse/ACM-15773
Signed-off-by: yiraeChristineKim <[email protected]>
  • Loading branch information
@yiraeChristineKim @openshift-merge-bot
yiraeChristineKim authored and openshift-merge-bot[bot] committed Dec 10, 2024
1 parent dcd8d0e commit facb1a8
Show file tree
Hide file tree
Showing 4 changed files with 176 additions and 89 deletions.
32 changes: 30 additions & 2 deletions controllers/configurationpolicy_controller_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -790,8 +790,8 @@ func TestCreateStatus(t *testing.T) {
},
true,
"K8s `must have` object already exists",
"secrets [bo-peep] found as specified in namespace toy-story4; secrets [buzz] found as specified in " +
"namespace toy-story",
"secrets [buzz] found as specified in namespace toy-story; " +
"secrets [bo-peep] found as specified in namespace toy-story4",
},
{
"must have single object created",
Expand Down Expand Up @@ -959,6 +959,34 @@ func TestCreateStatus(t *testing.T) {
"namespaced object of kind ConfigMap has no namespace specified from the policy namespaceSelector " +
"nor the object metadata",
},
{
"unnamed object multiple error",
"configmaps",
map[string]*objectTmplEvalResultWithEvent{
"toy-story1": {
result: objectTmplEvalResult{
objectNames: []string{"rex", "woody"},
},
event: objectTmplEvalEvent{
compliant: false,
reason: reasonWantFoundNoMatch,
},
},
"toy-story2": {
result: objectTmplEvalResult{
objectNames: []string{"buzz", "potato"},
},
event: objectTmplEvalEvent{
compliant: false,
reason: reasonWantFoundNoMatch,
},
},
},
false,
"K8s does not have a `must have` object",
"configmaps [rex, woody] found but not as specified in namespace toy-story1; " +
"configmaps [buzz, potato] found but not as specified in namespace toy-story2",
},
{
"multiple errors",
"configmaps",
Expand Down
207 changes: 135 additions & 72 deletions controllers/configurationpolicy_utils.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -565,7 +565,7 @@ func createStatus(
_, namesStr, _ := strings.Cut(nsName, "/")

if len(nsNameToEvent[nsName].result.objectNames) > 0 {
namesStr = " [" + strings.Join(nsNameToEvent[nsName].result.objectNames, ", ") + "]"
namesStr = strings.Join(nsNameToEvent[nsName].result.objectNames, ", ")
}

if _, ok := objectNameStrsToNsNames[namesStr]; !ok {
Expand All @@ -577,98 +577,161 @@ func createStatus(

slices.Sort(sortedObjectNamesStrs)

// Process the object name strings in order to ensure a deterministic reason and message.
for i, namesStr := range sortedObjectNamesStrs {
if compliancyDetailsMsg != "" {
compliancyDetailsMsg += "; "
}
var msgMap map[string][]string

var generatedReason, generatedMsg string

switch reason {
case reasonWantFoundExists:
generatedReason = "K8s `must have` object already exists"
generatedMsg = fmt.Sprintf("%s%s found as specified", resourceName, namesStr)
case reasonWantFoundCreated:
generatedReason = reasonWantFoundCreated
generatedMsg = fmt.Sprintf("%s%s was created successfully", resourceName, namesStr)
case reasonUpdateSuccess:
generatedReason = reasonUpdateSuccess
generatedMsg = fmt.Sprintf("%s%s was updated successfully", resourceName, namesStr)
case reasonDeleteSuccess:
generatedReason = reasonDeleteSuccess
generatedMsg = fmt.Sprintf("%s%s was deleted successfully", resourceName, namesStr)
case reasonWantFoundDNE:
generatedReason = "K8s does not have a `must have` object"
compliancyDetailsMsg += fmt.Sprintf("%s%s not found", resourceName, namesStr)
case reasonWantFoundNoMatch:
generatedReason = "K8s does not have a `must have` object"
compliancyDetailsMsg += fmt.Sprintf("%s%s found but not as specified", resourceName, namesStr)
case reasonWantNotFoundExists:
generatedReason = "K8s has a `must not have` object"
compliancyDetailsMsg += fmt.Sprintf("%s%s found", resourceName, namesStr)
case reasonWantNotFoundDNE:
generatedReason = "K8s `must not have` object already missing"
compliancyDetailsMsg += fmt.Sprintf("%s%s missing as expected", resourceName, namesStr)
default:
// If it's not one of the above reasons, then skip consolidation. This is likely an error being
// reported.
if i == 0 {
if compliancyDetailsReason != "" {
compliancyDetailsReason += "; "
}

compliancyDetailsReason += reason
}
msgMap, compliancyDetailsReason, compliancyDetailsMsg = generateMessageWithReason(sortedObjectNamesStrs,
objectNameStrsToNsNames, nsNameToEvent,
compliancyDetailsReason, compliancyDetailsMsg, reason)

for j, nsName := range objectNameStrsToNsNames[namesStr] {
if j != 0 {
compliancyDetailsMsg += "; "
}
compliancyDetailsMsg = getCombinedCompliancyDetailsMsg(msgMap, resourceName, compliancyDetailsMsg)
}

compliancyDetailsMsg += nsNameToEvent[nsName].event.message
}
return
}

// Assume the included messages include the namespace.
continue
}
func setCompliancyDetailsMsgEnd(compliancyDetailsMsg string) string {
if compliancyDetailsMsg != "" && !strings.HasSuffix(compliancyDetailsMsg, "; ") {
compliancyDetailsMsg += "; "
}

return compliancyDetailsMsg
}

// Process the object name strings in order to ensure a deterministic reason and message.
func getCombinedCompliancyDetailsMsg(msgMap map[string][]string,
resourceName string, compliancyDetailsMsg string,
) string {
// Sort msgTemplate keys for consistent processing
msgTemplates := make([]string, 0, len(msgMap))
for k := range msgMap {
msgTemplates = append(msgTemplates, k)
}

slices.Sort(msgTemplates)

for j, msgTemplate := range msgTemplates {
names := msgMap[msgTemplate]
slices.Sort(names) // Sort names for consistent output

if j != 0 {
compliancyDetailsMsg += "; "
}

joinedNames := strings.Join(names, ", ")
if joinedNames != "" {
joinedNames = fmt.Sprintf(" [%s]", joinedNames)
}

compliancyDetailsMsg += fmt.Sprintf(msgTemplate, resourceName, joinedNames)
}

// This prevents repeating the same reason for each unique object name list.
return compliancyDetailsMsg
}

// This function returns msgMap, which is a map where the key is a message template
// and the value is a list of resource names that share the same message template.
func generateMessageWithReason(sortedObjectNamesStrs []string,
objectNameStrsToNsNames map[string][]string,
nsNameToEvent map[string]*objectTmplEvalResultWithEvent,
compliancyDetailsReason string,
compliancyDetailsMsg string,
reason string,
) (map[string][]string, string, string) {
msgMap := map[string][]string{}

for i, namesStr := range sortedObjectNamesStrs {
compliancyDetailsMsg = setCompliancyDetailsMsgEnd(compliancyDetailsMsg)

var generatedReason, msgTemplate string

// Generate reason and message template based on the 'reason' value.
switch reason {
case reasonWantFoundExists:
generatedReason = "K8s `must have` object already exists"
msgTemplate = "%s%s found as specified"
case reasonWantFoundCreated:
generatedReason = reasonWantFoundCreated
msgTemplate = "%s%s was created successfully"
case reasonUpdateSuccess:
generatedReason = reasonUpdateSuccess
msgTemplate = "%s%s was updated successfully"
case reasonDeleteSuccess:
generatedReason = reasonDeleteSuccess
msgTemplate = "%s%s was deleted successfully"
case reasonWantFoundDNE:
generatedReason = "K8s does not have a `must have` object"
msgTemplate = "%s%s not found"
case reasonWantFoundNoMatch:
generatedReason = "K8s does not have a `must have` object"
msgTemplate = "%s%s found but not as specified"
case reasonWantNotFoundExists:
generatedReason = "K8s has a `must not have` object"
msgTemplate = "%s%s found"
case reasonWantNotFoundDNE:
generatedReason = "K8s `must not have` object already missing"
msgTemplate = "%s%s missing as expected"
default:
// If it's not one of the above reasons, then skip consolidation. This is likely an error being
// reported.
if i == 0 {
if compliancyDetailsReason != "" {
compliancyDetailsReason += "; "
}

compliancyDetailsReason += generatedReason
compliancyDetailsReason += reason
}

compliancyDetailsMsg += generatedMsg
for j, nsName := range objectNameStrsToNsNames[namesStr] {
if j != 0 {
compliancyDetailsMsg += "; "
}

// If it is namespaced, include the namespaces that were checked. A namespace of "" indicates
// cluster scoped. This length check is not necessary but is added for additional safety in case the logic
// above is changed.
nsList := []string{}
compliancyDetailsMsg += nsNameToEvent[nsName].event.message
}
// Assume the included messages include the namespace.
continue
}

for _, nsName := range objectNameStrsToNsNames[namesStr] {
ns, _, _ := strings.Cut(nsName, "/")
if ns != "" {
nsList = append(nsList, ns)
}
// This prevents repeating the same reason for each unique object name list.
if i == 0 {
if compliancyDetailsReason != "" {
compliancyDetailsReason += "; "
}

if len(nsList) > 0 {
if len(objectNameStrsToNsNames[namesStr]) > 1 {
compliancyDetailsMsg += " in namespaces: "
} else {
compliancyDetailsMsg += " in namespace "
}
compliancyDetailsReason += generatedReason
}

compliancyDetailsMsg += strings.Join(nsList, ", ")
// If it is namespaced, include the namespaces that were checked. A namespace of "" indicates
// cluster scoped. This length check is not necessary but is added for additional safety in case the logic
// above is changed.
nsList := []string{}

for _, nsName := range objectNameStrsToNsNames[namesStr] {
ns, _, _ := strings.Cut(nsName, "/")
if ns != "" {
nsList = append(nsList, ns)
}
}

if len(nsList) > 0 {
if len(objectNameStrsToNsNames[namesStr]) > 1 {
msgTemplate += " in namespaces: "
} else {
msgTemplate += " in namespace "
}

sort.Strings(nsList)
msgTemplate += strings.Join(nsList, ", ")
}

if _, ok := msgMap[msgTemplate]; !ok {
msgMap[msgTemplate] = []string{}
}

msgMap[msgTemplate] = append(msgMap[msgTemplate], namesStr)
}

return
return msgMap, compliancyDetailsReason, compliancyDetailsMsg
}

func objHasFinalizer(obj metav1.Object, finalizer string) bool {
Expand Down
5 changes: 3 additions & 2 deletions test/e2e/case13_templatization_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -601,8 +601,9 @@ var _ = Describe("Test templatization", Ordered, func() {

utils.CheckComplianceStatus(g, managedPlc, "Compliant")
g.Expect(utils.GetStatusMessage(managedPlc)).Should(Equal(fmt.Sprintf(
"configmaps [case13-e2e-objectname-var2] found as specified in namespace %[1]s; "+
"configmaps [case13-e2e-objectname-var3] found as specified in namespace %[1]s", e2eBaseName)))
"configmaps [case13-e2e-objectname-var2, case13-e2e-objectname-var3] "+
"found as specified in namespace %[1]s",
e2eBaseName)))

relatedObjects, _, _ := unstructured.NestedSlice(managedPlc.Object, "status", "relatedObjects")
g.Expect(relatedObjects).To(HaveLen(2))
Expand Down
21 changes: 8 additions & 13 deletions test/e2e/case42_resource_selection_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -81,11 +81,8 @@ var _ = Describe("Test results of resource selection", Ordered, func() {

return utils.GetStatusMessage(managedPlc)
}, defaultTimeoutSeconds, 1).Should(Equal(fmt.Sprintf(
"fakeapis [case42-1-e2e] found as specified in namespace %[1]s; "+
"fakeapis [case42-2-e2e] found as specified in namespace %[1]s; "+
"fakeapis [case42-3-e2e] found as specified in namespace %[1]s; "+
"fakeapis [case42-4-e2e] found as specified in namespace %[1]s; "+
"fakeapis [case42-5-e2e] found as specified in namespace %[1]s", targetNs)))
"fakeapis [case42-1-e2e, case42-2-e2e, case42-3-e2e, case42-4-e2e, case42-5-e2e]"+
" found as specified in namespace %s", targetNs)))
},
Entry("Empty label selector", `{}`),
Entry("Empty matchLabels", `{"matchLabels":{}}`),
Expand Down Expand Up @@ -152,8 +149,7 @@ var _ = Describe("Test behavior of resource selection as resources change", Orde

return utils.GetStatusMessage(managedPlc)
}, defaultTimeoutSeconds, 1).Should(Equal(fmt.Sprintf(
"fakeapis [case42-1-e2e] found but not as specified in namespace %[1]s; "+
"fakeapis [case42-2-e2e] found but not as specified in namespace %[1]s", targetNs)))
"fakeapis [case42-1-e2e, case42-2-e2e] found but not as specified in namespace %s", targetNs)))
})

It("should evaluate when a matching labeled resource is added", func(ctx SpecContext) {
Expand All @@ -166,7 +162,7 @@ var _ = Describe("Test behavior of resource selection as resources change", Orde

return utils.GetStatusMessage(managedPlc)
}, defaultTimeoutSeconds, 1).Should(HaveSuffix(
fmt.Sprintf("; fakeapis [case42-3-e2e] found but not as specified in namespace %s", targetNs)))
fmt.Sprintf("case42-3-e2e] found but not as specified in namespace %s", targetNs)))
})

It("should not change when a non-matching resource is added", func(ctx SpecContext) {
Expand All @@ -190,7 +186,7 @@ var _ = Describe("Test behavior of resource selection as resources change", Orde

return utils.GetStatusMessage(managedPlc)
}, defaultTimeoutSeconds, 1).Should(HaveSuffix(
fmt.Sprintf("; fakeapis [case42-4-e2e] found but not as specified in namespace %s", targetNs)))
fmt.Sprintf("case42-4-e2e] found but not as specified in namespace %s", targetNs)))
})

It("should evaluate when a matching resource label is removed", func() {
Expand All @@ -202,7 +198,8 @@ var _ = Describe("Test behavior of resource selection as resources change", Orde

return utils.GetStatusMessage(managedPlc)
}, defaultTimeoutSeconds, 1).ShouldNot(ContainSubstring(
fmt.Sprintf("fakeapis [case42-3-e2e] found but not as specified in namespace %s", targetNs)))
"case42-3-e2e",
fmt.Sprintf("found but not as specified in namespace %s", targetNs)))
})

It("should become compliant when enforced", func() {
Expand All @@ -220,9 +217,7 @@ var _ = Describe("Test behavior of resource selection as resources change", Orde

return utils.GetStatusMessage(managedPlc)
}, defaultTimeoutSeconds, 1).Should(Equal(fmt.Sprintf(
"fakeapis [case42-1-e2e] found as specified in namespace %[1]s; "+
"fakeapis [case42-2-e2e] found as specified in namespace %[1]s; "+
"fakeapis [case42-4-e2e] found as specified in namespace %[1]s", targetNs)))
"fakeapis [case42-1-e2e, case42-2-e2e, case42-4-e2e] found as specified in namespace %s", targetNs)))
})

It("should ignore the objectSelector when a name is provided", func() {
Expand Down

0 comments on commit facb1a8

Please sign in to comment.