Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged
merged 2 commits into from
Sep 25, 2023
Merged

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

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
c35a207
Fixed logic to choose whether to use filter or filters for a trigger
Cali0707 Sep 22, 2023
ac3c95c
Added test to make sure that empty filters does not override filter
Cali0707 Sep 25, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 3 additions & 7 deletions pkg/broker/filter/filter_handler.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
}
Expand Down
9 changes: 9 additions & 0 deletions pkg/broker/filter/filter_handler_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down