Fixed logic to choose whether to use filter or filters for a trigger (#…

…7286) * Fixed logic to choose whether to use filter or filters for a trigger Signed-off-by: Calum Murray <[email protected]> * Added test to make sure that empty filters does not override filter Signed-off-by: Calum Murray <[email protected]> --------- Signed-off-by: Calum Murray <[email protected]>
knative · Sep 25, 2023 · d6483cd · d6483cd
1 parent b704ab1
commit d6483cd
Show file tree

Hide file tree

Showing 2 changed files with 12 additions and 7 deletions.
diff --git a/pkg/broker/filter/filter_handler.go b/pkg/broker/filter/filter_handler.go
@@ -365,23 +365,19 @@ func (h *Handler) getTrigger(ref path.NamespacedNameUID) (*eventingv1.Trigger, e
 
 func (h *Handler) filterEvent(ctx context.Context, trigger *eventingv1.Trigger, event cloudevents.Event) eventfilter.FilterResult {
 	switch {
-	case feature.FromContext(ctx).IsEnabled(feature.NewTriggerFilters):
+	case feature.FromContext(ctx).IsEnabled(feature.NewTriggerFilters) && len(trigger.Spec.Filters) > 0:
 		logging.FromContext(ctx).Debugw("New trigger filters feature is enabled. Applying new filters.", zap.Any("filters", trigger.Spec.Filters))
 		filter, ok := h.filtersMap.Get(trigger)
 		if !ok {
-			// trigger filters haven't update in the map yet - need to create them on the fly
-			if len(trigger.Spec.Filters) == 0 {
-				logging.FromContext(ctx).Debug("Found no filters for trigger", zap.String("triggerFilterKey", fmt.Sprintf("%s.%s", trigger.Namespace, trigger.Name)))
-				return eventfilter.NoFilter
-			}
+			// trigger filters haven't updated in the map yet - need to create them on the fly
 			return applySubscriptionsAPIFilters(ctx, trigger, event)
 		}
 		return filter.Filter(ctx, event)
 	case trigger.Spec.Filter != nil:
 		logging.FromContext(ctx).Debugw("Applying attributes filter.", zap.Any("filter", trigger.Spec.Filter))
 		return applyAttributesFilter(ctx, trigger.Spec.Filter, event)
 	default:
-		logging.FromContext(ctx).Debugw("Found no filters in trigger", zap.Any("triggerSpec", trigger.Spec.Filter))
+		logging.FromContext(ctx).Debugw("Found no filters in trigger", zap.Any("triggerSpec", trigger.Spec))
 		return eventfilter.NoFilter
 	}
 }

diff --git a/pkg/broker/filter/filter_handler_test.go b/pkg/broker/filter/filter_handler_test.go
@@ -584,6 +584,15 @@ func TestReceiver_WithSubscriptionsAPI(t *testing.T) {
 			expectedEventCount:        true,
 			expectedEventDispatchTime: true,
 		},
+		"Dispatch failed - empty SubscriptionsAPI filter does not override Attributes Filter": {
+			triggers: []*eventingv1.Trigger{
+				makeTrigger(
+					withAttributesFilter(&eventingv1.TriggerFilter{
+						Attributes: map[string]string{"type": "some-other-type", "source": "some-other-source"},
+					})),
+			},
+			expectedEventCount: false,
+		},
 	}
 	for n, tc := range testCases {
 		t.Run(n, func(t *testing.T) {