Fix ConstraintTemplate creation with invalid rego

Ref: https://issues.redhat.com/browse/ACM-15588
ConstraintTemplate is created and compliant even with invalid rego syntax
Signed-off-by: yiraeChristineKim <[email protected]>

Author: yiraeChristineKim

Makefile

@@ -227,7 +227,7 @@ install-resources:
 	-kubectl apply -k deploy/hubpermissions --kubeconfig=$(HUB_CONFIG)_e2e
 	@if [ "$(KIND_VERSION)" != "minimum" ]; then \
 		echo installing Gatekeeper on the managed cluster; \
-		curl -L https://raw.githubusercontent.com/stolostron/gatekeeper/release-3.15/deploy/gatekeeper.yaml | sed 's/- --disable-cert-rotation/- --disable-cert-rotation\n        - --audit-interval=10/g' | kubectl apply --kubeconfig=$(MANAGED_CONFIG)_e2e -f -; \
+		curl -L https://raw.githubusercontent.com/stolostron/gatekeeper/release-3.17/deploy/gatekeeper.yaml | sed 's/- --disable-cert-rotation/- --disable-cert-rotation\n        - --audit-interval=10/g' | kubectl apply --kubeconfig=$(MANAGED_CONFIG)_e2e -f -; \
 		kubectl -n gatekeeper-system wait --for=condition=Available deployment/gatekeeper-audit --kubeconfig=$(MANAGED_CONFIG)_e2e; \
 	fi
 

controllers/templatesync/template_sync.go

@@ -564,6 +564,20 @@ func (r *PolicyReconciler) Reconcile(ctx context.Context, request reconcile.Requ
 					continue
 				}
 
+				// Applicable for Gatekeeper versions v3.17 and later.
+				if isGkConstraintTemplate {
+					sentMsg, err := r.emitGKConstraintTemplateErrMsg(ctx, tObjectUnstructured,
+						res, instance, tIndex, tName)
+					if err != nil {
+						return reconcile.Result{}, err
+					}
+
+					if sentMsg {
+						// Skip success event emitting
+						continue
+					}
+				}
+
 				successMsg := fmt.Sprintf("Policy template %s created successfully", tName)
 
 				tLogger.Info("Policy template created successfully")
@@ -622,6 +636,25 @@ func (r *PolicyReconciler) Reconcile(ctx context.Context, request reconcile.Requ
 			continue
 		}
 
+		// Applicable for Gatekeeper versions v3.17 and later.
+		if isGkConstraintTemplate {
+			sentErrMsg, err := r.emitGKConstraintTemplateErrMsg(ctx, tObjectUnstructured,
+				res, instance, tIndex, tName)
+			if err != nil {
+				return reconcile.Result{}, err
+			}
+
+			if !sentErrMsg {
+				tLogger.Info("Emitting status event for " + gvk.Kind)
+				msg := fmt.Sprintf("%s %s was created successfully", gvk.Kind, tName)
+
+				emitErr := r.emitTemplateSuccess(ctx, instance, tIndex, tName, isClusterScoped, msg)
+				if emitErr != nil {
+					resultError = emitErr
+				}
+			}
+		}
+
 		if len(dependencyFailures) > 0 {
 			// template must be pending, need to delete it and error
 			tLogger.Info("Dependencies were not satisfied for the policy template",
@@ -1091,8 +1124,12 @@ func (r *PolicyReconciler) cleanUpExcessTemplates(
 				r.setCreatedGkConstraint(false)
 			}
 			// Iterate over the ConstraintTemplates to gather the Constraints on the cluster
+			// In Gatekeeper v3.17 and later, ConstraintTemplates are created even if they contain errors.
+			// Only append to tmplGVRs if the ConstraintTemplate's status.created field is true.
 			for _, gkCT := range gkConstraintTemplateListv1.Items {
-				gkCT := gkCT
+				if !gkCT.Status.Created {
+					continue
+				}
 
 				tmplGVRs = append(tmplGVRs, gvrScoped{
 					gvr: schema.GroupVersionResource{
@@ -1740,3 +1777,42 @@ func getDupName(pol *policiesv1.Policy) string {
 
 	return ""
 }
+
+// emitGKConstraintTemplateErrMsg generates an error message based
+// on the Gatekeeper ConstraintTemplate status.
+// retrieves the "status.created" field from a Gatekeeper ConstraintTemplate.
+// In Gatekeeper 3.17 and later, if a ConstraintTemplate contains a Rego syntax error,
+// it is still created, but its status field will have "created: false" instead of failing outright.
+// Returns true if an error message is emitted.
+func (r *PolicyReconciler) emitGKConstraintTemplateErrMsg(
+	ctx context.Context,
+	tObjectUnstructured *unstructured.Unstructured,
+	res dynamic.ResourceInterface,
+	instance *policiesv1.Policy,
+	tIndex int,
+	tName string,
+) (bool, error) {
+	name := tObjectUnstructured.GetName()
+
+	template, err := res.Get(ctx, name, metav1.GetOptions{})
+	if err != nil {
+		return false, err
+	}
+
+	created, ok, err := unstructured.NestedBool(template.Object, "status", "created")
+	if !ok || err != nil {
+		errMsg := fmt.Sprintf("Failed to retrieve status.created from ConstraintTemplate %s: %v", name, err)
+		_ = r.emitTemplateError(ctx, instance, tIndex, tName, true, errMsg)
+
+		return true, nil
+	}
+
+	if !created {
+		errMsg := fmt.Sprintf("Failed to create Gatekeeper ConstraintTemplate. Check the status of %s.", name)
+		_ = r.emitTemplateError(ctx, instance, tIndex, tName, true, errMsg)
+
+		return true, nil
+	}
+
+	return false, nil
+}

test/e2e/case17_gatekeeper_sync_test.go

@@ -705,3 +705,70 @@ var _ = Describe("Test Gatekeeper ConstraintTemplate and constraint sync", Order
 		})
 	})
 })
+
+var _ = Describe("Test Gatekeeper ConstraintTemplate error", Ordered, Label("skip-minimum"), func() {
+	const (
+		yamlBasePath string = "../resources/case17_gatekeeper_sync/error-status"
+		policyYaml   string = yamlBasePath + "/policy.yaml"
+		policyName   string = "case17-invalid-syntax"
+	)
+
+	BeforeAll(func() {
+		if gkSyncDisabled {
+			Skip("Gatekeeper sync is disabled--skipping Gatekeeper tests")
+		}
+
+		Eventually(func(g Gomega) {
+			deployments, err := clientManaged.AppsV1().Deployments("gatekeeper-system").
+				List(context.TODO(), metav1.ListOptions{})
+			g.Expect(err).ShouldNot(HaveOccurred())
+			g.Expect(deployments.Items).ToNot(BeEmpty())
+
+			var available bool
+			for _, deployment := range deployments.Items {
+				for _, condition := range deployment.Status.Conditions {
+					if condition.Reason == "MinimumReplicasAvailable" {
+						available = condition.Status == "True"
+					}
+				}
+				g.Expect(available).To(BeTrue())
+			}
+		}, defaultTimeoutSeconds, 1).Should(Succeed(), "Gatekeeper should be installed before the test")
+	})
+
+	AfterAll(func() {
+		_, err := kubectlHub("delete", "-f", policyYaml, "-n", clusterNamespaceOnHub)
+		Expect(err).ShouldNot(HaveOccurred())
+	})
+
+	It("Should the policy status is NonCompliant", func() {
+		By("Creating policy " + policyName + " on the hub in ns:" + clusterNamespaceOnHub)
+		_, err := kubectlHub("apply", "-f", policyYaml, "-n", clusterNamespaceOnHub)
+		Expect(err).ShouldNot(HaveOccurred())
+
+		By("Waiting for policy status of the constraint to be compliant")
+		Eventually(func(g Gomega) {
+			plc := propagatorutils.GetWithTimeout(
+				clientManagedDynamic,
+				gvrPolicy,
+				policyName,
+				clusterNamespace,
+				true,
+				defaultTimeoutSeconds,
+			)
+
+			managedPolicy := policyv1.Policy{}
+			err := runtime.DefaultUnstructuredConverter.FromUnstructured(
+				plc.Object, &managedPolicy,
+			)
+			g.Expect(err).ToNot(HaveOccurred())
+
+			g.Expect(string(managedPolicy.Status.ComplianceState)).To(Equal("NonCompliant"))
+
+			message := managedPolicy.Status.Details[0].History[0].Message
+			g.Expect(message).To(
+				Equal("NonCompliant; template-error; Failed to create Gatekeeper ConstraintTemplate. " +
+					"Check the status of case17error."))
+		}, defaultTimeoutSeconds, 1).Should(Succeed())
+	})
+})

test/resources/case17_gatekeeper_sync/error-status/policy.yaml

@@ -0,0 +1,32 @@
+apiVersion: policy.open-cluster-management.io/v1
+kind: Policy
+metadata:
+  name: case17-invalid-syntax
+spec:
+  remediationAction: inform
+  disabled: false
+  policy-templates:
+    - objectDefinition:
+        apiVersion: templates.gatekeeper.sh/v1
+        kind: ConstraintTemplate
+        metadata:
+          name: case17error
+        spec:
+          crd:
+            spec:
+              names:
+                kind: Case17Error
+          targets:
+            - target: admission.k8s.gatekeeper.sh
+              rego: |
+                package case17error
+
+                violation[{"msg": msg}] {
+                  input.review.object.metadata.name != \"kube-root-ca.crt\"
+                  input.review.object.metadata.name != \"openshift-service-ca.crt\"
+                  provided := {label | input.review.object.metadata.labels[label]}
+                  required := {label | label := input.parameters.labels[_] invalid(missing})
+                  missing := required - provided
+                  count(missing) > 0
+                  msg := sprintf(\"you must provide labels: %v\", [missing])
+                }