Skip to content

Commit

Permalink
Validate the CEL expression on reconciliation.
Browse files Browse the repository at this point in the history
Signed-off-by: Kevin McDermott <[email protected]>
  • Loading branch information
bigkevmcd committed Nov 1, 2024
1 parent e5ab8e6 commit d5b54c5
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 d5b54c5

Please sign in to comment.