Skip to content

Commit 4d7c4da

Browse files
Handle hub template errors
The plan is to add template support to more policy types. Currently it is restricted to ConfigurationPolicies. This removes that restriction and reports hub template errors that would be added by the propagator, so that each policy controller does not need to duplicate the logic to emit those events themselves. Refs: - https://issues.redhat.com/browse/ACM-10858 Signed-off-by: Justin Kulikauskas <[email protected]>
1 parent a74d55e commit 4d7c4da

File tree

3 files changed

+16
-22
lines changed

3 files changed

+16
-22
lines changed

controllers/templatesync/template_sync.go

Lines changed: 10 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -361,13 +361,11 @@ func (r *PolicyReconciler) Reconcile(ctx context.Context, request reconcile.Requ
361361
continue
362362
}
363363

364-
var tName string
365-
if tMetaObj, ok := object.(metav1.Object); ok {
366-
tName = tMetaObj.GetName()
367-
}
364+
metaObj, ok := object.(metav1.Object)
365+
tName := metaObj.GetName()
368366

369-
if tName == "" {
370-
errMsg := fmt.Sprintf("Failed to get name from policy template at index %v", tIndex)
367+
if !ok || tName == "" {
368+
errMsg := fmt.Sprintf("Failed to parse or get name from policy template at index %v", tIndex)
371369
resultError = k8serrors.NewBadRequest(errMsg)
372370

373371
_ = r.emitTemplateError(ctx, instance, tIndex,
@@ -463,21 +461,15 @@ func (r *PolicyReconciler) Reconcile(ctx context.Context, request reconcile.Requ
463461
continue
464462
}
465463

466-
// reject if not configuration policy and has template strings, and don't requeue
467-
if gvk.Kind != "ConfigurationPolicy" {
468-
// if not configuration policies, do a simple check for templates {{hub and reject
469-
// only checking for hub and not {{ as they could be valid cases where they are valid chars.
470-
if strings.Contains(string(policyT.ObjectDefinition.Raw), "{{hub ") {
471-
errMsg := fmt.Sprintf("Templates are not supported for kind : %s", gvk.Kind)
472-
473-
_ = r.emitTemplateError(ctx, instance, tIndex, tName, isClusterScoped, errMsg)
464+
// check for hub template error
465+
if errAnno := metaObj.GetAnnotations()["policy.open-cluster-management.io/hub-templates-error"]; errAnno != "" {
466+
_ = r.emitTemplateError(ctx, instance, tIndex, tName, isClusterScoped, errAnno)
474467

475-
tLogger.Error(k8serrors.NewBadRequest(errMsg), "Failed to process the policy template")
468+
tLogger.Error(k8serrors.NewBadRequest(errAnno), "Failed to process the policy template")
476469

477-
policyUserErrorsCounter.WithLabelValues(instance.Name, tName, "format-error").Inc()
470+
policyUserErrorsCounter.WithLabelValues(instance.Name, tName, "format-error").Inc()
478471

479-
continue
480-
}
472+
continue
481473
}
482474

483475
dependencyFailures := r.processDependencies(ctx, dClient, discoveryClient, templateDeps, tLogger)

test/e2e/case10_error_test.go

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -96,7 +96,7 @@ var _ = Describe("Test error handling", func() {
9696

9797
By("Checking for event with missing name err on managed cluster in ns:" + clusterNamespace)
9898
Eventually(
99-
checkForEvent("case10-template-name-error", "template-error; Failed to get name from policy"),
99+
checkForEvent("case10-template-name-error", "template-error; Failed to parse or get name from policy"),
100100
defaultTimeoutSeconds,
101101
1,
102102
).Should(BeTrue())
@@ -255,7 +255,7 @@ var _ = Describe("Test error handling", func() {
255255
return found
256256
}, defaultTimeoutSeconds*2, 1).Should(BeFalse())
257257
})
258-
It("should throw a noncompliance event if a non-configurationpolicy uses a hub template", func() {
258+
It("should throw a noncompliance event for hub template errors", func() {
259259
By("Deploying a test policy CRD")
260260
_, err := kubectlManaged("apply", "-f", yamlBasePath+"mock-crd.yaml")
261261
Expect(err).ToNot(HaveOccurred())
@@ -266,11 +266,11 @@ var _ = Describe("Test error handling", func() {
266266
})
267267

268268
hubApplyPolicy("case10-bad-hubtemplate",
269-
yamlBasePath+"non-config-hubtemplate.yaml")
269+
yamlBasePath+"error-hubtemplate.yaml")
270270

271271
By("Checking for the error event")
272272
Eventually(
273-
checkForEvent("case10-bad-hubtemplate", "Templates are not supported for kind"),
273+
checkForEvent("case10-bad-hubtemplate", "must be aboveground"),
274274
defaultTimeoutSeconds,
275275
1,
276276
).Should(BeTrue())

test/resources/case10_template_sync_error_test/non-config-hubtemplate.yaml renamed to test/resources/case10_template_sync_error_test/error-hubtemplate.yaml

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -15,5 +15,7 @@ spec:
1515
kind: MockPolicy
1616
metadata:
1717
name: case10-bad-hubtemplate-policy
18+
annotations:
19+
policy.open-cluster-management.io/hub-templates-error: "must be aboveground"
1820
spec:
1921
foo: 'I come from {{hub the land down under hub}}'

0 commit comments

Comments
 (0)