Skip to content

Commit e758f04

Browse files
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 <yikim@redhat.com>
1 parent 92984ef commit e758f04

File tree

4 files changed

+177
-2
lines changed

4 files changed

+177
-2
lines changed

Makefile

+1-1
Original file line numberDiff line numberDiff line change
@@ -227,7 +227,7 @@ install-resources:
227227
-kubectl apply -k deploy/hubpermissions --kubeconfig=$(HUB_CONFIG)_e2e
228228
@if [ "$(KIND_VERSION)" != "minimum" ]; then \
229229
echo installing Gatekeeper on the managed cluster; \
230-
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 -; \
230+
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 -; \
231231
kubectl -n gatekeeper-system wait --for=condition=Available deployment/gatekeeper-audit --kubeconfig=$(MANAGED_CONFIG)_e2e; \
232232
fi
233233

controllers/templatesync/template_sync.go

+77-1
Original file line numberDiff line numberDiff line change
@@ -564,6 +564,20 @@ func (r *PolicyReconciler) Reconcile(ctx context.Context, request reconcile.Requ
564564
continue
565565
}
566566

567+
// Applicable for Gatekeeper versions v3.17 and later.
568+
if isGkConstraintTemplate {
569+
sentMsg, err := r.emitGKConstraintTemplateErrMsg(ctx, tObjectUnstructured,
570+
res, instance, tIndex, tName)
571+
if err != nil {
572+
return reconcile.Result{}, err
573+
}
574+
575+
if sentMsg {
576+
// Skip success event emitting
577+
continue
578+
}
579+
}
580+
567581
successMsg := fmt.Sprintf("Policy template %s created successfully", tName)
568582

569583
tLogger.Info("Policy template created successfully")
@@ -622,6 +636,25 @@ func (r *PolicyReconciler) Reconcile(ctx context.Context, request reconcile.Requ
622636
continue
623637
}
624638

639+
// Applicable for Gatekeeper versions v3.17 and later.
640+
if isGkConstraintTemplate {
641+
sentErrMsg, err := r.emitGKConstraintTemplateErrMsg(ctx, tObjectUnstructured,
642+
res, instance, tIndex, tName)
643+
if err != nil {
644+
return reconcile.Result{}, err
645+
}
646+
647+
if !sentErrMsg {
648+
tLogger.Info("Emitting status event for " + gvk.Kind)
649+
msg := fmt.Sprintf("%s %s was created successfully", gvk.Kind, tName)
650+
651+
emitErr := r.emitTemplateSuccess(ctx, instance, tIndex, tName, isClusterScoped, msg)
652+
if emitErr != nil {
653+
resultError = emitErr
654+
}
655+
}
656+
}
657+
625658
if len(dependencyFailures) > 0 {
626659
// template must be pending, need to delete it and error
627660
tLogger.Info("Dependencies were not satisfied for the policy template",
@@ -1091,8 +1124,12 @@ func (r *PolicyReconciler) cleanUpExcessTemplates(
10911124
r.setCreatedGkConstraint(false)
10921125
}
10931126
// Iterate over the ConstraintTemplates to gather the Constraints on the cluster
1127+
// In Gatekeeper v3.17 and later, ConstraintTemplates are created even if they contain errors.
1128+
// Only append to tmplGVRs if the ConstraintTemplate's status.created field is true.
10941129
for _, gkCT := range gkConstraintTemplateListv1.Items {
1095-
gkCT := gkCT
1130+
if !gkCT.Status.Created {
1131+
continue
1132+
}
10961133

10971134
tmplGVRs = append(tmplGVRs, gvrScoped{
10981135
gvr: schema.GroupVersionResource{
@@ -1740,3 +1777,42 @@ func getDupName(pol *policiesv1.Policy) string {
17401777

17411778
return ""
17421779
}
1780+
1781+
// emitGKConstraintTemplateErrMsg generates an error message based
1782+
// on the Gatekeeper ConstraintTemplate status.
1783+
// retrieves the "status.created" field from a Gatekeeper ConstraintTemplate.
1784+
// In Gatekeeper 3.17 and later, if a ConstraintTemplate contains a Rego syntax error,
1785+
// it is still created, but its status field will have "created: false" instead of failing outright.
1786+
// Returns true if an error message is emitted.
1787+
func (r *PolicyReconciler) emitGKConstraintTemplateErrMsg(
1788+
ctx context.Context,
1789+
tObjectUnstructured *unstructured.Unstructured,
1790+
res dynamic.ResourceInterface,
1791+
instance *policiesv1.Policy,
1792+
tIndex int,
1793+
tName string,
1794+
) (bool, error) {
1795+
name := tObjectUnstructured.GetName()
1796+
1797+
template, err := res.Get(ctx, name, metav1.GetOptions{})
1798+
if err != nil {
1799+
return false, err
1800+
}
1801+
1802+
created, ok, err := unstructured.NestedBool(template.Object, "status", "created")
1803+
if !ok || err != nil {
1804+
errMsg := fmt.Sprintf("Failed to retrieve status.created from ConstraintTemplate %s: %v", name, err)
1805+
_ = r.emitTemplateError(ctx, instance, tIndex, tName, true, errMsg)
1806+
1807+
return true, nil
1808+
}
1809+
1810+
if !created {
1811+
errMsg := fmt.Sprintf("Failed to create Gatekeeper ConstraintTemplate. Check the status of %s.", name)
1812+
_ = r.emitTemplateError(ctx, instance, tIndex, tName, true, errMsg)
1813+
1814+
return true, nil
1815+
}
1816+
1817+
return false, nil
1818+
}

test/e2e/case17_gatekeeper_sync_test.go

+67
Original file line numberDiff line numberDiff line change
@@ -705,3 +705,70 @@ var _ = Describe("Test Gatekeeper ConstraintTemplate and constraint sync", Order
705705
})
706706
})
707707
})
708+
709+
var _ = Describe("Test Gatekeeper ConstraintTemplate error", Ordered, Label("skip-minimum"), func() {
710+
const (
711+
yamlBasePath string = "../resources/case17_gatekeeper_sync/error-status"
712+
policyYaml string = yamlBasePath + "/policy.yaml"
713+
policyName string = "case17-invalid-syntax"
714+
)
715+
716+
BeforeAll(func() {
717+
if gkSyncDisabled {
718+
Skip("Gatekeeper sync is disabled--skipping Gatekeeper tests")
719+
}
720+
721+
Eventually(func(g Gomega) {
722+
deployments, err := clientManaged.AppsV1().Deployments("gatekeeper-system").
723+
List(context.TODO(), metav1.ListOptions{})
724+
g.Expect(err).ShouldNot(HaveOccurred())
725+
g.Expect(deployments.Items).ToNot(BeEmpty())
726+
727+
var available bool
728+
for _, deployment := range deployments.Items {
729+
for _, condition := range deployment.Status.Conditions {
730+
if condition.Reason == "MinimumReplicasAvailable" {
731+
available = condition.Status == "True"
732+
}
733+
}
734+
g.Expect(available).To(BeTrue())
735+
}
736+
}, defaultTimeoutSeconds, 1).Should(Succeed(), "Gatekeeper should be installed before the test")
737+
})
738+
739+
AfterAll(func() {
740+
_, err := kubectlHub("delete", "-f", policyYaml, "-n", clusterNamespaceOnHub)
741+
Expect(err).ShouldNot(HaveOccurred())
742+
})
743+
744+
It("Should the policy status is NonCompliant", func() {
745+
By("Creating policy " + policyName + " on the hub in ns:" + clusterNamespaceOnHub)
746+
_, err := kubectlHub("apply", "-f", policyYaml, "-n", clusterNamespaceOnHub)
747+
Expect(err).ShouldNot(HaveOccurred())
748+
749+
By("Waiting for policy status of the constraint to be compliant")
750+
Eventually(func(g Gomega) {
751+
plc := propagatorutils.GetWithTimeout(
752+
clientManagedDynamic,
753+
gvrPolicy,
754+
policyName,
755+
clusterNamespace,
756+
true,
757+
defaultTimeoutSeconds,
758+
)
759+
760+
managedPolicy := policyv1.Policy{}
761+
err := runtime.DefaultUnstructuredConverter.FromUnstructured(
762+
plc.Object, &managedPolicy,
763+
)
764+
g.Expect(err).ToNot(HaveOccurred())
765+
766+
g.Expect(string(managedPolicy.Status.ComplianceState)).To(Equal("NonCompliant"))
767+
768+
message := managedPolicy.Status.Details[0].History[0].Message
769+
g.Expect(message).To(
770+
Equal("NonCompliant; template-error; Failed to create Gatekeeper ConstraintTemplate. " +
771+
"Check the status of case17error."))
772+
}, defaultTimeoutSeconds, 1).Should(Succeed())
773+
})
774+
})
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,32 @@
1+
apiVersion: policy.open-cluster-management.io/v1
2+
kind: Policy
3+
metadata:
4+
name: case17-invalid-syntax
5+
spec:
6+
remediationAction: inform
7+
disabled: false
8+
policy-templates:
9+
- objectDefinition:
10+
apiVersion: templates.gatekeeper.sh/v1
11+
kind: ConstraintTemplate
12+
metadata:
13+
name: case17error
14+
spec:
15+
crd:
16+
spec:
17+
names:
18+
kind: Case17Error
19+
targets:
20+
- target: admission.k8s.gatekeeper.sh
21+
rego: |
22+
package case17error
23+
24+
violation[{"msg": msg}] {
25+
input.review.object.metadata.name != \"kube-root-ca.crt\"
26+
input.review.object.metadata.name != \"openshift-service-ca.crt\"
27+
provided := {label | input.review.object.metadata.labels[label]}
28+
required := {label | label := input.parameters.labels[_] invalid(missing})
29+
missing := required - provided
30+
count(missing) > 0
31+
msg := sprintf(\"you must provide labels: %v\", [missing])
32+
}

0 commit comments

Comments
 (0)