Skip to content

Commit 8e892ef

Browse files
committed
Validate the CEL expression on reconciliation.
If the .spec.resourceFilter is provided, and can't be parsed by the CEL environment, the resource will not be ready, and an appropriate error indicated. Signed-off-by: Kevin McDermott <[email protected]>
1 parent 6082afd commit 8e892ef

File tree

4 files changed

+67
-1
lines changed

4 files changed

+67
-1
lines changed

api/v1/condition_types.go

+4
Original file line numberDiff line numberDiff line change
@@ -28,4 +28,8 @@ const (
2828

2929
// TokenNotFoundReason represents the fact that receiver token can't be found.
3030
TokenNotFoundReason string = "TokenNotFound"
31+
32+
// InvalidCELExpressionReason represents the fact that the CEL resource
33+
// filter is invalid.
34+
InvalidCELExpressionReason string = "InvalidCELExpression"
3135
)

internal/controller/receiver_controller.go

+9
Original file line numberDiff line numberDiff line change
@@ -168,6 +168,15 @@ func (r *ReceiverReconciler) reconcile(ctx context.Context, obj *apiv1.Receiver)
168168
return ctrl.Result{Requeue: true}, err
169169
}
170170

171+
if filter := obj.Spec.ResourceFilter; filter != "" {
172+
err := server.ValidateCELExpression(filter)
173+
if err != nil {
174+
conditions.MarkFalse(obj, meta.ReadyCondition, apiv1.InvalidCELExpressionReason, "%s", err)
175+
obj.Status.WebhookPath = ""
176+
return ctrl.Result{}, err
177+
}
178+
}
179+
171180
webhookPath := obj.GetWebhookPath(token)
172181
msg := fmt.Sprintf("Receiver initialized for path: %s", webhookPath)
173182

internal/controller/receiver_controller_test.go

+37
Original file line numberDiff line numberDiff line change
@@ -144,6 +144,43 @@ func TestReceiverReconciler_Reconcile(t *testing.T) {
144144
g.Expect(resultR.Spec.Interval.Duration).To(BeIdenticalTo(10 * time.Minute))
145145
})
146146

147+
t.Run("fails with invalid CEL resource filter", func(t *testing.T) {
148+
g := NewWithT(t)
149+
g.Expect(k8sClient.Get(context.Background(), client.ObjectKeyFromObject(receiver), resultR)).To(Succeed())
150+
151+
// Incomplete CEL expression
152+
patch := []byte(`{"spec":{"resourceFilter":"has(resource.metadata.annotations"}}`)
153+
g.Expect(k8sClient.Patch(context.Background(), resultR, client.RawPatch(types.MergePatchType, patch))).To(Succeed())
154+
155+
g.Eventually(func() bool {
156+
_ = k8sClient.Get(context.Background(), client.ObjectKeyFromObject(receiver), resultR)
157+
return !conditions.IsReady(resultR)
158+
}, timeout, time.Second).Should(BeTrue())
159+
160+
g.Expect(conditions.GetReason(resultR, meta.ReadyCondition)).To(BeIdenticalTo(apiv1.InvalidCELExpressionReason))
161+
g.Expect(conditions.GetMessage(resultR, meta.ReadyCondition)).To(ContainSubstring("annotations"))
162+
163+
g.Expect(conditions.Has(resultR, meta.ReconcilingCondition)).To(BeTrue())
164+
g.Expect(conditions.GetReason(resultR, meta.ReconcilingCondition)).To(BeIdenticalTo(meta.ProgressingWithRetryReason))
165+
g.Expect(conditions.GetObservedGeneration(resultR, meta.ReconcilingCondition)).To(BeIdenticalTo(resultR.Generation))
166+
})
167+
168+
t.Run("recovers when the CEL expression is valid", func(t *testing.T) {
169+
g := NewWithT(t)
170+
// Incomplete CEL expression
171+
patch := []byte(`{"spec":{"resourceFilter":"has(resource.metadata.annotations)"}}`)
172+
g.Expect(k8sClient.Patch(context.Background(), resultR, client.RawPatch(types.MergePatchType, patch))).To(Succeed())
173+
174+
g.Eventually(func() bool {
175+
_ = k8sClient.Get(context.Background(), client.ObjectKeyFromObject(receiver), resultR)
176+
return conditions.IsReady(resultR)
177+
}, timeout, time.Second).Should(BeTrue())
178+
179+
g.Expect(conditions.GetObservedGeneration(resultR, meta.ReadyCondition)).To(BeIdenticalTo(resultR.Generation))
180+
g.Expect(resultR.Status.ObservedGeneration).To(BeIdenticalTo(resultR.Generation))
181+
g.Expect(conditions.Has(resultR, meta.ReconcilingCondition)).To(BeFalse())
182+
})
183+
147184
t.Run("fails with secret not found error", func(t *testing.T) {
148185
g := NewWithT(t)
149186

internal/server/cel.go

+17-1
Original file line numberDiff line numberDiff line change
@@ -32,7 +32,14 @@ import (
3232
"sigs.k8s.io/controller-runtime/pkg/client"
3333
)
3434

35-
func newCELEvaluator(expr string, req *http.Request) (resourcePredicate, error) {
35+
// ValidateCELEXpression accepts a CEL expression and will parse and check that
36+
// it's valid, if it's not valid an error is returned.
37+
func ValidateCELExpression(s string) error {
38+
_, err := newCELProgram(s)
39+
return err
40+
}
41+
42+
func newCELProgram(expr string) (cel.Program, error) {
3643
env, err := makeCELEnv()
3744
if err != nil {
3845
return nil, err
@@ -52,6 +59,15 @@ func newCELEvaluator(expr string, req *http.Request) (resourcePredicate, error)
5259
return nil, fmt.Errorf("expression %v failed to create a Program: %w", expr, err)
5360
}
5461

62+
return prg, nil
63+
}
64+
65+
func newCELEvaluator(expr string, req *http.Request) (resourcePredicate, error) {
66+
prg, err := newCELProgram(expr)
67+
if err != nil {
68+
return nil, err
69+
}
70+
5571
body := map[string]any{}
5672
// Only decodes the body for the expression if the body is JSON.
5773
// Technically you could generate several resources without any body.

0 commit comments

Comments
 (0)