Address review comments

* Don't pass log through - get it from the context * Use ContextEval - need to set a timeout on the context that's passed Signed-off-by: Kevin McDermott <[email protected]>
fluxcd · Nov 10, 2024 · 348c595 · 348c595
1 parent 9d7f593
commit 348c595
Show file tree

Hide file tree

Showing 4 changed files with 14 additions and 12 deletions.
diff --git a/docs/spec/v1/receivers.md b/docs/spec/v1/receivers.md
@@ -757,15 +757,15 @@ If the body of the incoming hook looks like this:
 
 This simple example would match `ImageRepositories` containing the name `hello-world`.
 
-If you want do do more complex processing:
+If you want to do more complex processing:
 
 ```yaml
   resourceFilter: has(resource.metadata.annotations) && request.body.tag.split('/').last().split(":").first() == resource.metadata.annotations['update-image']
 ```
 
 This would look for an annotation "update-image" on the resource, and match it to the `hello-world` part of the tag name.
 
-**NOTE**: Currently the `resource` value in the CEL expression only provides the object metadata, this means you can access things like `resource.metadata.labels` and `resource.metadata.annotations` and `resource.metadata.name`.
+**Note:** Currently the `resource` value in the CEL expression only provides the object metadata, this means you can access things like `resource.metadata.labels` and `resource.metadata.annotations` and `resource.metadata.name`.
 
 There are a number of functions available to the CEL expressions beyond the basic CEL functionality.
 

diff --git a/internal/controller/receiver_controller.go b/internal/controller/receiver_controller.go
@@ -40,7 +40,6 @@ import (
 	helper "github.com/fluxcd/pkg/runtime/controller"
 	"github.com/fluxcd/pkg/runtime/patch"
 	"github.com/fluxcd/pkg/runtime/predicates"
-	"github.com/go-logr/logr"
 
 	apiv1 "github.com/fluxcd/notification-controller/api/v1"
 	"github.com/fluxcd/notification-controller/internal/server"
@@ -153,12 +152,14 @@ func (r *ReceiverReconciler) Reconcile(ctx context.Context, req ctrl.Request) (r
 		return ctrl.Result{}, nil
 	}
 
-	return r.reconcile(ctx, obj, log)
+	return r.reconcile(ctx, obj)
 }
 
 // reconcile steps through the actual reconciliation tasks for the object, it returns early on the first step that
 // produces an error.
-func (r *ReceiverReconciler) reconcile(ctx context.Context, obj *apiv1.Receiver, logger logr.Logger) (ctrl.Result, error) {
+func (r *ReceiverReconciler) reconcile(ctx context.Context, obj *apiv1.Receiver) (ctrl.Result, error) {
+	log := ctrl.LoggerFrom(ctx)
+
 	// Mark the resource as under reconciliation.
 	conditions.MarkReconciling(obj, meta.ProgressingReason, "Reconciliation in progress")
 

diff --git a/internal/server/cel.go b/internal/server/cel.go
@@ -17,6 +17,7 @@ limitations under the License.
 package server
 
 import (
+	"context"
 	"encoding/json"
 	"fmt"
 	"mime"
@@ -54,7 +55,7 @@ func newCELProgram(expr string) (cel.Program, error) {
 		return nil, fmt.Errorf("expression %v check failed: %w", expr, issues.Err())
 	}
 
-	prg, err := env.Program(checked, cel.EvalOptions(cel.OptOptimize))
+	prg, err := env.Program(checked, cel.EvalOptions(cel.OptOptimize), cel.InterruptCheckFrequency(100))
 	if err != nil {
 		return nil, fmt.Errorf("expression %v failed to create a Program: %w", expr, err)
 	}
@@ -77,13 +78,13 @@ func newCELEvaluator(expr string, req *http.Request) (resourcePredicate, error)
 		}
 	}
 
-	return func(obj client.Object) (*bool, error) {
+	return func(ctx context.Context, obj client.Object) (*bool, error) {
 		data, err := clientObjectToMap(obj)
 		if err != nil {
 			return nil, err
 		}
 
-		out, _, err := prg.Eval(map[string]any{
+		out, _, err := prg.ContextEval(ctx, map[string]any{
 			"resource": data,
 			"request": map[string]any{
 				"body": body,

diff --git a/internal/server/receiver_handlers.go b/internal/server/receiver_handlers.go
@@ -115,7 +115,7 @@ func (s *ReceiverServer) handlePayload(w http.ResponseWriter, r *http.Request) {
 		return
 	}
 
-	var evaluator func(client.Object) (*bool, error)
+	var evaluator func(context.Context, client.Object) (*bool, error)
 	if receiver.Spec.ResourceFilter != "" {
 		evaluator, err = newCELEvaluator(receiver.Spec.ResourceFilter, r)
 		if err != nil {
@@ -150,12 +150,12 @@ func (s *ReceiverServer) notifySingleResource(ctx context.Context, logger logr.L
 
 func (s *ReceiverServer) notifyResource(ctx context.Context, logger logr.Logger, resource *metav1.PartialObjectMetadata, predicate resourcePredicate) error {
 	if predicate != nil {
-		accept, err := predicate(resource)
+		accept, err := predicate(ctx, resource)
 		if err != nil {
 			return err
 		}
 		if !*accept {
-			logger.Info(fmt.Sprintf("resource '%s/%s.%s' NOT annotated because CEL expression returned false", resource.Kind, resource.Name, resource.Namespace))
+			logger.V(1).Info(fmt.Sprintf("resource '%s/%s.%s' NOT annotated because CEL expression returned false", resource.Kind, resource.Name, resource.Namespace))
 			return nil
 		}
 	}
@@ -514,7 +514,7 @@ func (s *ReceiverServer) token(ctx context.Context, receiver apiv1.Receiver) (st
 	return token, nil
 }
 
-type resourcePredicate func(client.Object) (*bool, error)
+type resourcePredicate func(context.Context, client.Object) (*bool, error)
 
 // requestReconciliation requests reconciliation of all the resources matching the given CrossNamespaceObjectReference by annotating them accordingly.
 func (s *ReceiverServer) requestReconciliation(ctx context.Context, logger logr.Logger, resource apiv1.CrossNamespaceObjectReference, defaultNamespace string, resourcePredicate resourcePredicate) error {