diff --git a/controllers/configurationpolicy_controller.go b/controllers/configurationpolicy_controller.go index 7458e482..13f461db 100644 --- a/controllers/configurationpolicy_controller.go +++ b/controllers/configurationpolicy_controller.go @@ -1140,11 +1140,11 @@ func (r *ConfigurationPolicyReconciler) handleObjectTemplates(plc policyv1.Confi // violations for enforce configurationpolicies are already handled in handleObjects, // so we only need to generate a violation if the remediationAction is set to inform if !handled && !enforce { - objData := map[string]interface{}{ - "indx": indx, - "kind": kind, - "desiredName": templateObjs[indx].name, - "namespaced": templateObjs[indx].isNamespaced, + objData := templateIdentifier{ + index: indx, + kind: kind, + desiredName: templateObjs[indx].name, + namespaced: templateObjs[indx].isNamespaced, } statusUpdateNeeded := createInformStatus( @@ -1335,6 +1335,13 @@ func addConditionToStatus( return updateNeeded } +type templateIdentifier struct { + desiredName string + index int + kind string + namespaced bool +} + // createInformStatus updates the status field for a configurationpolicy with remediationAction=inform // based on how many compliant/noncompliant objects are found when processing the templates in the configurationpolicy func createInformStatus( @@ -1344,18 +1351,9 @@ func createInformStatus( compliantObjects, nonCompliantObjects map[string]map[string]interface{}, plc *policyv1.ConfigurationPolicy, - objData map[string]interface{}, + objData templateIdentifier, ) bool { - //nolint:forcetypeassert - desiredName := objData["desiredName"].(string) - //nolint:forcetypeassert - indx := objData["indx"].(int) - //nolint:forcetypeassert - kind := objData["kind"].(string) - //nolint:forcetypeassert - namespaced := objData["namespaced"].(bool) - - if kind == "" { + if objData.kind == "" { return false } @@ -1374,7 +1372,7 @@ func createInformStatus( compObjs = compliantObjects } - return createStatus(desiredName, kind, compObjs, namespaced, plc, indx, compliant, objShouldExist) + return createStatus(objData, compObjs, plc, compliant, objShouldExist) } // handleObjects controls the processing of each individual object template within a configurationpolicy @@ -1623,6 +1621,12 @@ func (r *ConfigurationPolicyReconciler) handleSingleObj( }, } + objData := templateIdentifier{ + kind: obj.gvr.Resource, + namespaced: obj.namespaced, + index: obj.index, + } + if !exists && obj.shouldExist { // it is a musthave and it does not exist, so it must be created if strings.EqualFold(string(remediation), string(policyv1.Enforce)) { @@ -1638,8 +1642,7 @@ func (r *ConfigurationPolicyReconciler) handleSingleObj( // 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) + _ = createStatus(objData, compliantObject, obj.policy, 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", @@ -1685,8 +1688,7 @@ func (r *ConfigurationPolicyReconciler) handleSingleObj( if strings.EqualFold(string(remediation), string(policyv1.Enforce)) { log.V(2).Info("Entering `does not exist` and `must not have`") - statusUpdateNeeded = createStatus("", obj.gvr.Resource, compliantObject, obj.namespaced, obj.policy, - obj.index, compliant, false) + statusUpdateNeeded = createStatus(objData, compliantObject, obj.policy, compliant, false) } } @@ -1707,8 +1709,7 @@ func (r *ConfigurationPolicyReconciler) handleSingleObj( if triedUpdate && !strings.Contains(msg, "Error validating the object") { // object has a mismatch and needs an update to be enforced, throw violation for mismatch - _ = createStatus("", obj.gvr.Resource, compliantObject, obj.namespaced, obj.policy, obj.index, - false, true) + _ = createStatus(objData, compliantObject, obj.policy, false, true) obj.policy.Status.ComplianceState = policyv1.NonCompliant statusStr := convertPolicyStatusToString(obj.policy) objLog.Info("Sending an update policy status event", "policy", obj.policy.Name, "status", statusStr) @@ -1748,8 +1749,7 @@ func (r *ConfigurationPolicyReconciler) handleSingleObj( statusUpdateNeeded = addConditionToStatus(obj.policy, obj.index, true, reason, msg) } else { - statusUpdateNeeded = createStatus("", obj.gvr.Resource, compliantObject, obj.namespaced, obj.policy, - obj.index, compliant, true) + statusUpdateNeeded = createStatus(objData, compliantObject, obj.policy, compliant, true) } created := false creationInfo = &policyv1.ObjectProperties{ @@ -1978,7 +1978,7 @@ func buildNameList( } if match { - kindNameList = append(kindNameList, uObj.Object["metadata"].(map[string]interface{})["name"].(string)) + kindNameList = append(kindNameList, uObj.GetName()) } } @@ -2264,6 +2264,11 @@ func mergeSpecsHelper(templateVal, existingVal interface{}, ctype string) interf return templateVal.(string) } +type countedVal struct { + value interface{} + count int +} + // mergeArrays is a helper function that takes a list from the existing object and merges in all the data that is // different in the template. This way, comparing the merged object to the one that exists on the cluster will tell // you whether the existing object is compliant with the template @@ -2280,18 +2285,15 @@ func mergeArrays(newArr []interface{}, old []interface{}, ctype string) (result } // create a set with a key for each unique item in the list - oldItemSet := map[string]map[string]interface{}{} + oldItemSet := make(map[string]*countedVal) for _, val2 := range old { key := fmt.Sprint(val2) if entry, ok := oldItemSet[key]; ok { - oldItemSet[key]["count"] = entry["count"].(int) + 1 + entry.count++ } else { - oldItemSet[key] = map[string]interface{}{ - "count": 1, - "value": val2, - } + oldItemSet[key] = &countedVal{value: val2, count: 1} } } @@ -2306,10 +2308,8 @@ func mergeArrays(newArr []interface{}, old []interface{}, ctype string) (result seen[key] = true } - data := oldItemSet[key] count := 0 - reqCount := data["count"] - val2 := data["value"] + val2 := oldItemSet[key].value // for each list item in the existing array, iterate through the template array and try to find a match for newArrIdx, val1 := range newArrCopy { if idxWritten[newArrIdx] { @@ -2353,14 +2353,14 @@ func mergeArrays(newArr []interface{}, old []interface{}, ctype string) (result // If the result of merging val1 (template) into val2 (existing value) matched val2 for the required count, // move on to the next existing value. - if count == reqCount { + if count == oldItemSet[key].count { break } } // if an item in the existing object cannot be found in the template, we add it to the template array // to produce the merged array - if count < reqCount.(int) { - for i := 0; i < (reqCount.(int) - count); i++ { + if count < oldItemSet[key].count { + for i := 0; i < (oldItemSet[key].count - count); i++ { newArr = append(newArr, val2) } } @@ -2423,7 +2423,7 @@ func handleSingleKey( updateNeeded := false - if isDenylisted(key) { + if key == "apiVersion" || key == "kind" { log.V(2).Info("Ignoring the key since it is deny listed", "key", key) return "", false, nil, true @@ -2594,10 +2594,15 @@ func (r *ConfigurationPolicyReconciler) checkAndUpdateResource( // only look at labels and annotations for metadata - configurationPolicies do not update other metadata fields if key == "metadata" { - mergedAnnotations := mergedObj.(map[string]interface{})["annotations"] - mergedLabels := mergedObj.(map[string]interface{})["labels"] - obj.object.UnstructuredContent()["metadata"].(map[string]interface{})["annotations"] = mergedAnnotations - obj.object.UnstructuredContent()["metadata"].(map[string]interface{})["labels"] = mergedLabels + // if it's not the right type, the map will be empty + mdMap, _ := mergedObj.(map[string]interface{}) + + // if either isn't found, they'll just be empty + mergedAnnotations, _, _ := unstructured.NestedStringMap(mdMap, "annotations") + mergedLabels, _, _ := unstructured.NestedStringMap(mdMap, "labels") + + obj.object.SetAnnotations(mergedAnnotations) + obj.object.SetLabels(mergedLabels) } else { obj.object.UnstructuredContent()[key] = mergedObj } diff --git a/controllers/configurationpolicy_controller_test.go b/controllers/configurationpolicy_controller_test.go index 5469aaed..85793a0a 100644 --- a/controllers/configurationpolicy_controller_test.go +++ b/controllers/configurationpolicy_controller_test.go @@ -458,11 +458,11 @@ func TestCreateInformStatus(t *testing.T) { }, } objNamespaced := true - objData := map[string]interface{}{ - "indx": 0, - "kind": "Secret", - "desiredName": "myobject", - "namespaced": objNamespaced, + objData := templateIdentifier{ + index: 0, + kind: "Secret", + desiredName: "myobject", + namespaced: objNamespaced, } mustNotHave := false numCompliant := 0 diff --git a/controllers/configurationpolicy_utils.go b/controllers/configurationpolicy_utils.go index 3756fd83..04d0e04d 100644 --- a/controllers/configurationpolicy_utils.go +++ b/controllers/configurationpolicy_utils.go @@ -68,17 +68,13 @@ func addRelatedObjects( // unmarshalFromJSON unmarshals raw JSON data into an object func unmarshalFromJSON(rawData []byte) (unstructured.Unstructured, error) { var unstruct unstructured.Unstructured - var blob interface{} - if jsonErr := json.Unmarshal(rawData, &blob); jsonErr != nil { + if jsonErr := json.Unmarshal(rawData, &unstruct.Object); jsonErr != nil { log.Error(jsonErr, "Could not unmarshal data from JSON") return unstruct, jsonErr } - unstruct.Object = make(map[string]interface{}) - unstruct.Object = blob.(map[string]interface{}) - return unstruct, nil } @@ -113,13 +109,17 @@ func updateRelatedObjectsStatus( func equalObjWithSort(mergedObj interface{}, oldObj interface{}) (areEqual bool) { switch mergedObj := mergedObj.(type) { case map[string]interface{}: - if oldObj == nil || !checkFieldsWithSort(mergedObj, oldObj.(map[string]interface{})) { - return false + if oldObjMap, ok := oldObj.(map[string]interface{}); ok { + return checkFieldsWithSort(mergedObj, oldObjMap) } + // this includes the case where oldObj is nil + return false case []interface{}: - if oldObj == nil || !checkListsMatch(mergedObj, oldObj.([]interface{})) { - return false + if oldObjList, ok := oldObj.([]interface{}); ok { + return checkListsMatch(mergedObj, oldObjList) } + + return false default: // NOTE: when type is string, int, bool var oVal interface{} @@ -243,15 +243,18 @@ func checkListFieldsWithSort(mergedObj []map[string]interface{}, oldObj []map[st switch mVal := mVal.(type) { case []interface{}: // if a map in the list contains a nested list, sort and check for equality - oVal, ok := oldItem[i].([]interface{}) - if !ok || len(mVal) != len(oVal) || !checkListsMatch(oVal, mVal) { - return false + if oVal, ok := oldItem[i].([]interface{}); ok { + return len(mVal) == len(oVal) && checkListsMatch(oVal, mVal) } + + return false case map[string]interface{}: // if a map in the list contains another map, check fields for equality - if !checkFieldsWithSort(mVal, oldItem[i].(map[string]interface{})) { - return false + if oVal, ok := oldItem[i].(map[string]interface{}); ok { + return len(mVal) == len(oVal) && checkFieldsWithSort(mVal, oVal) } + + return false case string: // extra check to see if value is a byte value mQty, err := apiRes.ParseQuantity(mVal) @@ -308,9 +311,11 @@ func checkListsMatch(oldVal []interface{}, mergedVal []interface{}) (m bool) { switch oNestedVal := oNestedVal.(type) { case map[string]interface{}: // if list contains maps, recurse on those maps to check for a match - if !checkFieldsWithSort(mVal[idx].(map[string]interface{}), oNestedVal) { - return false + if mVal, ok := mVal[idx].(map[string]interface{}); ok { + return len(mVal) == len(oNestedVal) && checkFieldsWithSort(mVal, oNestedVal) } + + return false default: // otherwise, just do a generic check if fmt.Sprint(oNestedVal) != fmt.Sprint(mVal[idx]) { @@ -322,33 +327,26 @@ func checkListsMatch(oldVal []interface{}, mergedVal []interface{}) (m bool) { return true } -func isDenylisted(key string) (result bool) { - denylist := []string{"apiVersion", "kind"} - for _, val := range denylist { - if key == val { - return true - } - } - - return false -} +func filterUnwantedAnnotations(input map[string]interface{}) map[string]interface{} { + out := make(map[string]interface{}) -func isAutogenerated(key string) (result bool) { - denylist := []string{"kubectl.kubernetes.io/last-applied-configuration"} - for _, val := range denylist { - if key == val { - return true + for key, val := range input { + // This could use a denylist if we need to filter more annotations in the future. + if key != "kubectl.kubernetes.io/last-applied-configuration" { + out[key] = val } } - return false + return out } // formatTemplate returns the value of the input key in a manner that the controller can use for comparisons. func formatTemplate(unstruct unstructured.Unstructured, key string) (obj interface{}) { if key == "metadata" { - //nolint:forcetypeassert - metadata := unstruct.Object[key].(map[string]interface{}) + metadata, ok := unstruct.Object[key].(map[string]interface{}) + if !ok { + return metadata // it will just be empty + } return formatMetadata(metadata) } @@ -366,23 +364,13 @@ func formatMetadata(metadata map[string]interface{}) (formatted map[string]inter md["labels"] = labels } - if annos, ok := metadata["annotations"].(map[string]interface{}); ok { - // When a non-map is provided, skip iterating and set the value directly - noAutogenerated := map[string]interface{}{} - - for key, val := range annos { - if isAutogenerated(key) { - log.V(2).Info("Ignoring the annotation during the comparison due to it being deny listed", "key", key) - } else { - noAutogenerated[key] = val - } - } - - if len(noAutogenerated) > 0 { - md["annotations"] = noAutogenerated + if annosTemp, ok := metadata["annotations"]; ok { + if annos, ok := annosTemp.(map[string]interface{}); ok { + md["annotations"] = filterUnwantedAnnotations(annos) + } else { + // When a non-map is provided, set the value directly + md["annotations"] = annosTemp } - } else if annos, ok := metadata["annotations"]; ok { - md["annotations"] = annos } return md @@ -403,29 +391,17 @@ func fmtMetadataForCompare( } if annosTemp, ok := metadataTemp["annotations"]; ok { - noAutogenerated := map[string]interface{}{} - - for key, anno := range annosTemp.(map[string]interface{}) { - if !isAutogenerated(key) { - noAutogenerated[key] = anno - } - } - - if len(noAutogenerated) > 0 { - mdTemp["annotations"] = noAutogenerated + if annos, ok := annosTemp.(map[string]interface{}); ok { + mdTemp["annotations"] = filterUnwantedAnnotations(annos) + } else { + mdTemp["annotations"] = annosTemp } if annosExisting, ok := metadataExisting["annotations"]; ok { - noAutogeneratedExisting := map[string]interface{}{} - - for key, anno := range annosExisting.(map[string]interface{}) { - if !isAutogenerated(key) { - noAutogeneratedExisting[key] = anno - } - } - - if len(noAutogenerated) > 0 { - mdExisting["annotations"] = noAutogeneratedExisting + if annos, ok := annosExisting.(map[string]interface{}); ok { + mdExisting["annotations"] = filterUnwantedAnnotations(annos) + } else { + mdExisting["annotations"] = annosExisting } } } @@ -467,12 +443,9 @@ func identifierStr(names []string, namespace string, namespaced bool) (nameStr s } func createStatus( - desiredName string, - kind string, + tmplID templateIdentifier, complianceObjects map[string]map[string]interface{}, - namespaced bool, plc *policyv1.ConfigurationPolicy, - indx int, compliant bool, objShouldExist bool, ) (update bool) { @@ -487,19 +460,19 @@ func createStatus( sort.Strings(sortedNamespaces) // Noncompliant with no resources -- return violation immediately - if objShouldExist && !compliant && desiredName == "" { - message := fmt.Sprintf("No instances of `%v` found as specified", kind) - if namespaced && len(sortedNamespaces) > 0 { + if objShouldExist && !compliant && tmplID.desiredName == "" { + message := fmt.Sprintf("No instances of `%v` found as specified", tmplID.kind) + if tmplID.namespaced && len(sortedNamespaces) > 0 { message += fmt.Sprintf(" in namespaces: %v", strings.Join(sortedNamespaces, ", ")) } - return addConditionToStatus(plc, indx, false, "K8s does not have a `must have` object", message) + return addConditionToStatus(plc, tmplID.index, false, "K8s does not have a `must have` object", message) } for _, ns := range sortedNamespaces { // if the assertion fails, `names` will effectively be an empty list, which is fine. names, _ := complianceObjects[ns]["names"].([]string) - idStr := identifierStr(names, ns, namespaced) + idStr := identifierStr(names, ns, tmplID.namespaced) if objShouldExist { if compliant { @@ -521,22 +494,23 @@ func createStatus( if objShouldExist { if compliant { reason = "K8s `must have` object already exists" - msg = fmt.Sprintf("%v %v as specified, therefore this Object template is compliant", kind, niceNames) + msg = fmt.Sprintf("%v %v as specified, therefore this Object template is compliant", tmplID.kind, niceNames) } else { reason = "K8s does not have a `must have` object" - msg = fmt.Sprintf("%v not found: %v", kind, niceNames) + msg = fmt.Sprintf("%v not found: %v", tmplID.kind, niceNames) } } else { if compliant { reason = "K8s `must not have` object already missing" - msg = fmt.Sprintf("%v %v missing as expected, therefore this Object template is compliant", kind, niceNames) + msg = fmt.Sprintf( + "%v %v missing as expected, therefore this Object template is compliant", tmplID.kind, niceNames) } else { reason = "K8s has a `must not have` object" - msg = fmt.Sprintf("%v found: %v", kind, niceNames) + msg = fmt.Sprintf("%v found: %v", tmplID.kind, niceNames) } } - return addConditionToStatus(plc, indx, compliant, reason, msg) + return addConditionToStatus(plc, tmplID.index, compliant, reason, msg) } func sortAndJoinKeys(m map[string]bool, sep string) string {