From a8e8e5967cce3aa65bc54b269f7898b0c30f9b1d Mon Sep 17 00:00:00 2001 From: mprahl Date: Tue, 21 Nov 2023 18:20:47 -0500 Subject: [PATCH] Fix a bug related to unnamed objects In the case where an object-template included an unnamed object and only one object was returned, it went into the typical `handleSingleObj` flow but marked `exists` as true without setting `existingObj` in the input to `handleSingleObj`. There was an assumption that `existingObj` would always be set if `exists` was set. Commit ffc115c made this panic because `GetUID` was being called on a nil `existingObj`. Prior to this, it seems it would just erroneously say it was compliant without checking it. Relates: https://issues.redhat.com/browse/ACM-8731 Signed-off-by: mprahl --- controllers/configurationpolicy_controller.go | 11 +++++++++++ .../case10_kind_field/case10_kind_check.yaml | 7 +++++++ 2 files changed, 18 insertions(+) diff --git a/controllers/configurationpolicy_controller.go b/controllers/configurationpolicy_controller.go index 49a4c7d2..dcc51a73 100644 --- a/controllers/configurationpolicy_controller.go +++ b/controllers/configurationpolicy_controller.go @@ -1525,6 +1525,13 @@ func (r *ConfigurationPolicyReconciler) handleObjects( if len(objNames) == 0 { exists = false + } else if len(objNames) == 1 { + // If the object couldn't be retrieved, this will be handled later on. + existingObj, _ = getObject( + objDetails.isNamespaced, namespace, objNames[0], mapping.Resource, r.TargetK8sDynamicClient, + ) + + exists = existingObj != nil } } @@ -2746,6 +2753,10 @@ func (r *ConfigurationPolicyReconciler) setEvaluatedObject( func (r *ConfigurationPolicyReconciler) alreadyEvaluated( policy *policyv1.ConfigurationPolicy, currentObject *unstructured.Unstructured, ) (evaluated bool, compliant bool) { + if policy == nil || currentObject == nil { + return false, false + } + loadedPolicyMap, loaded := r.processedPolicyCache.Load(policy.GetUID()) if !loaded { return false, false diff --git a/test/resources/case10_kind_field/case10_kind_check.yaml b/test/resources/case10_kind_field/case10_kind_check.yaml index eabf442c..43804783 100644 --- a/test/resources/case10_kind_field/case10_kind_check.yaml +++ b/test/resources/case10_kind_field/case10_kind_check.yaml @@ -6,6 +6,13 @@ spec: remediationAction: inform # will be overridden by remediationAction in parent policy severity: low object-templates: + # The first object template is ensure no regression for https://issues.redhat.com/browse/ACM-8731 + - complianceType: musthave + objectDefinition: + apiVersion: v1 + kind: Namespace + metadata: + name: default - complianceType: musthave objectDefinition: apiVersion: v1