Skip to content

Commit 06186f4

Browse files
Deduplicate objects to watch
This version of the dependency-watches library can panic if passed duplicate object IDs along with an object ID for a kind that is not present on the cluster. This avoids triggering that situation, and might be easier to backport than changes to the library itself. Refs: - https://issues.redhat.com/browse/ACM-14091 Signed-off-by: Justin Kulikauskas <jkulikau@redhat.com>
1 parent 389b32f commit 06186f4

File tree

3 files changed

+99
-3
lines changed

3 files changed

+99
-3
lines changed

controllers/templatesync/template_sync.go

+11-3
Original file line numberDiff line numberDiff line change
@@ -879,12 +879,20 @@ func (r *PolicyReconciler) Reconcile(ctx context.Context, request reconcile.Requ
879879
}
880880

881881
if len(allDeps) != 0 || len(childTemplates) != 0 {
882-
objectsToWatch := make([]depclient.ObjectIdentifier, 0, len(allDeps)+len(childTemplates))
882+
watchSet := make(map[depclient.ObjectIdentifier]struct{})
883+
883884
for depID := range allDeps {
884-
objectsToWatch = append(objectsToWatch, depID)
885+
watchSet[depID] = struct{}{}
886+
}
887+
888+
for _, childID := range childTemplates {
889+
watchSet[childID] = struct{}{}
885890
}
886891

887-
objectsToWatch = append(objectsToWatch, childTemplates...)
892+
objectsToWatch := make([]depclient.ObjectIdentifier, 0, len(watchSet))
893+
for depID := range watchSet {
894+
objectsToWatch = append(objectsToWatch, depID)
895+
}
888896

889897
err = r.DynamicWatcher.AddOrUpdateWatcher(policyObjectID, objectsToWatch...)
890898
} else {

test/e2e/case12_ordering_test.go

+27
Original file line numberDiff line numberDiff line change
@@ -381,4 +381,31 @@ var _ = Describe("Test dependency logic in template sync", Ordered, func() {
381381
By("Checking if policy status is consistently pending")
382382
Consistently(checkCompliance(case12PolicyName), "15s", 1).Should(Equal("Pending"))
383383
})
384+
// This test case is meant for a specific panic that was being caused by some Policies.
385+
It("Should function properly despite a nonexistent dependency kind", func() {
386+
const (
387+
testPolicyName string = "case12-test-nonexist-dep"
388+
testPolicyYaml string = "../resources/case12_ordering/case12-plc-nonexist-dep.yaml"
389+
templateName string = "case12-config-policy"
390+
)
391+
392+
By("Creating a policy on hub cluster in ns:" + clusterNamespaceOnHub)
393+
hubPolicyApplyAndDeferCleanup(testPolicyYaml, testPolicyName)
394+
395+
By("Generating a noncompliant event on the first template")
396+
generateEventOnPolicy(testPolicyName, templateName, "not yet successful", "NonCompliant")
397+
398+
By("Checking if policy status is noncompliant")
399+
Eventually(checkCompliance(testPolicyName), defaultTimeoutSeconds, 1).Should(Equal("NonCompliant"))
400+
401+
// Sleep to avoid the status-sync from reconciling again before the template-sync has a
402+
// chance to reconcile and (possibly) panic.
403+
time.Sleep(5 * time.Second)
404+
405+
By("Generating a compliant event on the first template")
406+
generateEventOnPolicy(testPolicyName, templateName, "all systems go", "Compliant")
407+
408+
By("Checking if policy status is Compliant")
409+
Eventually(checkCompliance(testPolicyName), defaultTimeoutSeconds, 1).Should(Equal("Compliant"))
410+
})
384411
})
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,61 @@
1+
apiVersion: policy.open-cluster-management.io/v1
2+
kind: Policy
3+
metadata:
4+
name: case12-test-nonexist-dep
5+
labels:
6+
policy.open-cluster-management.io/cluster-name: managed
7+
policy.open-cluster-management.io/cluster-namespace: managed
8+
policy.open-cluster-management.io/root-policy: case12-test-policy-nonexist-dep
9+
spec:
10+
remediationAction: inform
11+
disabled: false
12+
policy-templates:
13+
- objectDefinition:
14+
apiVersion: policy.open-cluster-management.io/v1
15+
kind: ConfigurationPolicy
16+
metadata:
17+
name: case12-config-policy
18+
spec:
19+
remediationAction: inform
20+
object-templates:
21+
- complianceType: musthave
22+
objectDefinition:
23+
apiVersion: v1
24+
kind: Pod
25+
metadata:
26+
name: nginx-pod-e2e
27+
namespace: default
28+
spec:
29+
containers:
30+
- name: nginx
31+
- extraDependencies:
32+
- apiVersion: policy.open-cluster-management.io/v1
33+
kind: ConfigurationPolicy
34+
name: case12-config-policy
35+
namespace: ""
36+
compliance: Compliant
37+
- apiVersion: policy.open-cluster-management.io/v1
38+
kind: NonexistPolicy
39+
name: nonexist
40+
namespace: ""
41+
compliance: Compliant
42+
ignorePending: true
43+
objectDefinition:
44+
apiVersion: policy.open-cluster-management.io/v1
45+
kind: ConfigurationPolicy
46+
metadata:
47+
name: case12-config-policy-2
48+
spec:
49+
remediationAction: inform
50+
object-templates:
51+
- complianceType: musthave
52+
objectDefinition:
53+
apiVersion: v1
54+
kind: Pod
55+
metadata:
56+
name: nginx-pod-e2e
57+
namespace: default
58+
spec:
59+
containers:
60+
- name: nginx
61+

0 commit comments

Comments
 (0)