Skip to content

Commit

Permalink
Fix constantly updating objects with a null struct
Browse files Browse the repository at this point in the history
It was tricky to find an example of this. Most optional structs I found
would get populated with some default values, meaning they wouldn't be
omitted by the kube api.

Signed-off-by: Justin Kulikauskas <jkulikau@redhat.com>
  • Loading branch information
JustinKuli authored and openshift-merge-robot committed May 1, 2023
1 parent fd913d8 commit dff741d
Show file tree
Hide file tree
Showing 4 changed files with 120 additions and 6 deletions.
18 changes: 12 additions & 6 deletions controllers/configurationpolicy_utils.go
Original file line number Diff line number Diff line change
Expand Up @@ -124,15 +124,21 @@ func equalObjWithSort(mergedObj interface{}, oldObj interface{}) (areEqual bool)
}

return false
default:
// NOTE: when type is string, int, bool
var oVal interface{}

default: // when mergedObj's type is string, int, bool, or nil
if oldObj == nil && mergedObj != nil {
// compare the zero value of mergedObj's type to mergedObj
ref := reflect.ValueOf(mergedObj)
oVal = reflect.Zero(ref.Type()).Interface()
zero := reflect.Zero(ref.Type()).Interface()

return fmt.Sprint(zero) == fmt.Sprint(mergedObj)
}

if mergedObj == nil && oldObj != nil {
// compare the zero value of oldObj's type to oldObj
ref := reflect.ValueOf(oldObj)
zero := reflect.Zero(ref.Type()).Interface()

return fmt.Sprint(oVal) == fmt.Sprint(mergedObj)
return fmt.Sprint(zero) == fmt.Sprint(oldObj)
}

if !reflect.DeepEqual(fmt.Sprint(mergedObj), fmt.Sprint(oldObj)) {
Expand Down
27 changes: 27 additions & 0 deletions test/e2e/case31_policy_history_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -125,6 +125,33 @@ var _ = Describe("Test policy history messages when KubeAPI omits values in the
ExpectWithOffset(1, configlPlc).To(BeNil())
})
})

Describe("status should not toggle when a struct might be omitted", Ordered, func() {
const (
policyYAML = rsrcPath + "event-policy-emptystruct.yaml"
policyName = "test-policy-security-emptystruct"
configPolicyYAML = rsrcPath + "event-config-policy-emptystruct.yaml"
configPolicyName = "config-policy-event-emptystruct"
)

It("sets up a configuration policy with struct set to null", func() {
createConfigPolicyWithParent(policyYAML, policyName, configPolicyYAML)
})

It("checks the policy's history", func() {
doHistoryTest(policyYAML, policyName, configPolicyYAML, configPolicyName)
})

AfterAll(func() {
utils.Kubectl("delete", "policy", policyName, "-n", "managed", "--ignore-not-found")
configlPlc := utils.GetWithTimeout(clientManagedDynamic, gvrConfigPolicy,
configPolicyName, "managed", false, defaultTimeoutSeconds,
)
utils.Kubectl("delete", "event", "--field-selector=involvedObject.name="+policyName, "-n", "managed")
utils.Kubectl("delete", "event", "--field-selector=involvedObject.name="+configPolicyName, "-n", "managed")
ExpectWithOffset(1, configlPlc).To(BeNil())
})
})
})

func createConfigPolicyWithParent(parentPolicyYAML, parentPolicyName, configPolicyYAML string) {
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,39 @@
apiVersion: policy.open-cluster-management.io/v1
kind: ConfigurationPolicy
metadata:
name: config-policy-event-emptystruct
labels:
policy.open-cluster-management.io/policy: test-policy-security
ownerReferences:
- apiVersion: policy.open-cluster-management.io/v1
blockOwnerDeletion: false
controller: true
kind: Policy
name: test-policy-security-emptystruct
uid: 08bae967-4262-498a-84e9-d1f0e321b41e
spec:
pruneObjectBehavior: DeleteAll
remediationAction: enforce
namespaceSelector:
exclude:
- kube-public
include:
- default
- kube-system
object-templates:
- complianceType: musthave
objectDefinition:
action: DidTheThing
apiVersion: events.k8s.io/v1
eventTime: 2023-04-27T14:37:36.721589Z
kind: Event
metadata:
name: configpol-test-event
namespace: kube-system
note: Successfully did something
reason: Success
regarding: null
related: null
reportingController: ConfigPolicyTester
reportingInstance: configpol-history-test
type: Normal
42 changes: 42 additions & 0 deletions test/resources/case31_policy_history/event-policy-emptystruct.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,42 @@
apiVersion: policy.open-cluster-management.io/v1
kind: Policy
metadata:
name: test-policy-security-emptystruct
annotations:
policy.open-cluster.management.io/standards: NIST-CSF
policy.open-cluster.management.io/categories: PR.PT Protective Technology
policy.open-cluster.management.io/controls: PR.PT-3 Least Functionality
spec:
remediationAction: enforce
disabled: false
policy-templates:
- objectDefinition:
apiVersion: policy.open-cluster-management.io/v1
kind: ConfigurationPolicy
metadata:
name: config-policy-event-emptystruct
spec:
remediationAction: enforce
namespaceSelector:
exclude:
- kube-public
include:
- default
- kube-system
object-templates:
- complianceType: musthave
objectDefinition:
action: DidTheThing
apiVersion: events.k8s.io/v1
eventTime: 2023-04-27T14:37:36.721589Z
kind: Event
metadata:
name: configpol-test-event
namespace: kube-system
note: Successfully did something
reason: Success
regarding: null
related: null
reportingController: ConfigPolicyTester
reportingInstance: configpol-history-test
type: Normal

0 comments on commit dff741d

Please sign in to comment.