Skip to content

Commit c07ac53

Browse files
JustinKuliopenshift-merge-robot
authored andcommitted
Emit template-error if template already exists
Previously, if a Policy defined a template that already exists, it assumed that another Policy had defined it, and tried to get its owner reference in order to give a helpful error message. But if the template was created outside of a Policy, then this process would panic. Refs: - https://issues.redhat.com/browse/ACM-3416 Signed-off-by: Justin Kulikauskas <jkulikau@redhat.com>
1 parent 8654c6f commit c07ac53

File tree

3 files changed

+70
-6
lines changed

3 files changed

+70
-6
lines changed

controllers/templatesync/template_sync.go

+25-6
Original file line numberDiff line numberDiff line change
@@ -424,14 +424,33 @@ func (r *PolicyReconciler) Reconcile(ctx context.Context, request reconcile.Requ
424424
continue
425425
}
426426

427-
refName := eObject.GetOwnerReferences()[0].Name
427+
refName := ""
428+
429+
for _, ownerref := range eObject.GetOwnerReferences() {
430+
refName = ownerref.Name
431+
432+
break // just get the first ownerReference, if there are any at all
433+
}
434+
428435
// violation if object reference and policy don't match
429436
if instance.GetName() != refName {
430-
errMsg := fmt.Sprintf(
431-
"Template name must be unique. Policy template with kind: %s name: %s already exists in policy %s",
432-
tObjectUnstructured.Object["kind"],
433-
tName,
434-
refName)
437+
var errMsg string
438+
439+
if refName == "" {
440+
errMsg = fmt.Sprintf(
441+
"Template name must be unique. Policy template with "+
442+
"kind: %s name: %s already exists outside of a Policy",
443+
tObjectUnstructured.Object["kind"],
444+
tName)
445+
} else {
446+
errMsg = fmt.Sprintf(
447+
"Template name must be unique. Policy template with "+
448+
"kind: %s name: %s already exists in policy %s",
449+
tObjectUnstructured.Object["kind"],
450+
tName,
451+
refName)
452+
}
453+
435454
resultError = errors.NewBadRequest(errMsg)
436455

437456
r.emitTemplateError(instance, tIndex, tName, errMsg)

test/e2e/case10_error_test.go

+28
Original file line numberDiff line numberDiff line change
@@ -217,6 +217,34 @@ var _ = Describe("Test error handling", func() {
217217
1,
218218
).Should(BeFalse())
219219
})
220+
It("should throw a noncompliance event if the template already exists outside of a policy", func() {
221+
By("Creating the ConfigurationPolicy on the managed cluster directly")
222+
_, err := kubectlManaged(
223+
"apply",
224+
"--filename=../resources/case10_template_sync_error_test/working-policy-configpol.yaml",
225+
"--namespace="+clusterNamespace,
226+
)
227+
Expect(err).Should(BeNil())
228+
229+
managedPlc := utils.GetWithTimeout(
230+
clientManagedDynamic,
231+
gvrConfigurationPolicy,
232+
"case10-config-policy",
233+
clusterNamespace,
234+
true,
235+
defaultTimeoutSeconds)
236+
ExpectWithOffset(1, managedPlc).NotTo(BeNil())
237+
238+
hubApplyPolicy("case10-test-policy",
239+
"../resources/case10_template_sync_error_test/working-policy.yaml")
240+
241+
By("Checking for the error event")
242+
Eventually(
243+
checkForEvent("case10-test-policy", "already exists outside of a Policy"),
244+
defaultTimeoutSeconds,
245+
1,
246+
).Should(BeTrue())
247+
})
220248
})
221249

222250
// Checks for an event on the managed cluster
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,17 @@
1+
apiVersion: policy.open-cluster-management.io/v1
2+
kind: ConfigurationPolicy
3+
metadata:
4+
name: case10-config-policy
5+
spec:
6+
remediationAction: inform
7+
object-templates:
8+
- complianceType: musthave
9+
objectDefinition:
10+
apiVersion: v1
11+
kind: Pod
12+
metadata:
13+
name: nginx-pod-e2e
14+
namespace: default
15+
spec:
16+
containers:
17+
- name: nginx

0 commit comments

Comments
 (0)