diff --git a/api/v1/condition_types.go b/api/v1/condition_types.go index 66f9a3375..910b1c04c 100644 --- a/api/v1/condition_types.go +++ b/api/v1/condition_types.go @@ -28,4 +28,8 @@ const ( // TokenNotFoundReason represents the fact that receiver token can't be found. TokenNotFoundReason string = "TokenNotFound" + + // InvalidCELExpressionReason represents the fact that the CEL resource + // filter is invalid. + InvalidCELExpressionReason string = "InvalidCELExpression" ) diff --git a/internal/controller/receiver_controller.go b/internal/controller/receiver_controller.go index 50f85dd2f..056810889 100644 --- a/internal/controller/receiver_controller.go +++ b/internal/controller/receiver_controller.go @@ -168,6 +168,15 @@ func (r *ReceiverReconciler) reconcile(ctx context.Context, obj *apiv1.Receiver) return ctrl.Result{Requeue: true}, err } + if filter := obj.Spec.ResourceFilter; filter != "" { + err := server.ValidateCELExpression(filter) + if err != nil { + conditions.MarkFalse(obj, meta.ReadyCondition, apiv1.InvalidCELExpressionReason, "%s", err) + obj.Status.WebhookPath = "" + return ctrl.Result{}, err + } + } + webhookPath := obj.GetWebhookPath(token) msg := fmt.Sprintf("Receiver initialized for path: %s", webhookPath) diff --git a/internal/controller/receiver_controller_test.go b/internal/controller/receiver_controller_test.go index 7d80953b3..676abae08 100644 --- a/internal/controller/receiver_controller_test.go +++ b/internal/controller/receiver_controller_test.go @@ -144,6 +144,43 @@ func TestReceiverReconciler_Reconcile(t *testing.T) { g.Expect(resultR.Spec.Interval.Duration).To(BeIdenticalTo(10 * time.Minute)) }) + t.Run("fails with invalid CEL resource filter", func(t *testing.T) { + g := NewWithT(t) + g.Expect(k8sClient.Get(context.Background(), client.ObjectKeyFromObject(receiver), resultR)).To(Succeed()) + + // Incomplete CEL expression + patch := []byte(`{"spec":{"resourceFilter":"has(resource.metadata.annotations"}}`) + g.Expect(k8sClient.Patch(context.Background(), resultR, client.RawPatch(types.MergePatchType, patch))).To(Succeed()) + + g.Eventually(func() bool { + _ = k8sClient.Get(context.Background(), client.ObjectKeyFromObject(receiver), resultR) + return !conditions.IsReady(resultR) + }, timeout, time.Second).Should(BeTrue()) + + g.Expect(conditions.GetReason(resultR, meta.ReadyCondition)).To(BeIdenticalTo(apiv1.InvalidCELExpressionReason)) + g.Expect(conditions.GetMessage(resultR, meta.ReadyCondition)).To(ContainSubstring("annotations")) + + g.Expect(conditions.Has(resultR, meta.ReconcilingCondition)).To(BeTrue()) + g.Expect(conditions.GetReason(resultR, meta.ReconcilingCondition)).To(BeIdenticalTo(meta.ProgressingWithRetryReason)) + g.Expect(conditions.GetObservedGeneration(resultR, meta.ReconcilingCondition)).To(BeIdenticalTo(resultR.Generation)) + }) + + t.Run("recovers when the CEL expression is valid", func(t *testing.T) { + g := NewWithT(t) + // Incomplete CEL expression + patch := []byte(`{"spec":{"resourceFilter":"has(resource.metadata.annotations)"}}`) + g.Expect(k8sClient.Patch(context.Background(), resultR, client.RawPatch(types.MergePatchType, patch))).To(Succeed()) + + g.Eventually(func() bool { + _ = k8sClient.Get(context.Background(), client.ObjectKeyFromObject(receiver), resultR) + return conditions.IsReady(resultR) + }, timeout, time.Second).Should(BeTrue()) + + g.Expect(conditions.GetObservedGeneration(resultR, meta.ReadyCondition)).To(BeIdenticalTo(resultR.Generation)) + g.Expect(resultR.Status.ObservedGeneration).To(BeIdenticalTo(resultR.Generation)) + g.Expect(conditions.Has(resultR, meta.ReconcilingCondition)).To(BeFalse()) + }) + t.Run("fails with secret not found error", func(t *testing.T) { g := NewWithT(t) diff --git a/internal/server/cel.go b/internal/server/cel.go index e02ceea98..683e29c77 100644 --- a/internal/server/cel.go +++ b/internal/server/cel.go @@ -32,7 +32,14 @@ import ( "sigs.k8s.io/controller-runtime/pkg/client" ) -func newCELEvaluator(expr string, req *http.Request) (resourcePredicate, error) { +// ValidateCELEXpression accepts a CEL expression and will parse and check that +// it's valid, if it's not valid an error is returned. +func ValidateCELExpression(s string) error { + _, err := newCELProgram(s) + return err +} + +func newCELProgram(expr string) (cel.Program, error) { env, err := makeCELEnv() if err != nil { return nil, err @@ -52,6 +59,15 @@ func newCELEvaluator(expr string, req *http.Request) (resourcePredicate, error) return nil, fmt.Errorf("expression %v failed to create a Program: %w", expr, err) } + return prg, nil +} + +func newCELEvaluator(expr string, req *http.Request) (resourcePredicate, error) { + prg, err := newCELProgram(expr) + if err != nil { + return nil, err + } + body := map[string]any{} // Only decodes the body for the expression if the body is JSON. // Technically you could generate several resources without any body.