Skip to content

Commit

Permalink
Fix a bug related to unnamed objects
Browse files Browse the repository at this point in the history
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 <mprahl@users.noreply.github.com>
  • Loading branch information
mprahl authored and openshift-merge-bot[bot] committed Nov 21, 2023
1 parent 02eee99 commit a901475
Show file tree
Hide file tree
Showing 2 changed files with 18 additions and 0 deletions.
11 changes: 11 additions & 0 deletions controllers/configurationpolicy_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
}

Expand Down Expand Up @@ -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
Expand Down
7 changes: 7 additions & 0 deletions test/resources/case10_kind_field/case10_kind_check.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down

0 comments on commit a901475

Please sign in to comment.