Skip to content

Commit

Permalink
Validate the CEL expression on reconciliation.
Browse files Browse the repository at this point in the history
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 <bigkevmcd@gmail.com>
  • Loading branch information
bigkevmcd committed Nov 1, 2024
1 parent 6082afd commit 18c1f31
Show file tree
Hide file tree
Showing 4 changed files with 67 additions and 1 deletion.
4 changes: 4 additions & 0 deletions api/v1/condition_types.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
)
9 changes: 9 additions & 0 deletions internal/controller/receiver_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)

Expand Down
37 changes: 37 additions & 0 deletions internal/controller/receiver_controller_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)

Expand Down
18 changes: 17 additions & 1 deletion internal/server/cel.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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.
Expand Down

0 comments on commit 18c1f31

Please sign in to comment.