fix: NPE in external resource caching event source when only a generic filter is set - #3518
Draft
csviri wants to merge 1 commit into
Draft
Conversation
…c filter is set
`acceptedByFiler` guards each of its three filter branches with
`onXFilter != null || genericFilter != null`, but the branch body
dereferences `onXFilter` unconditionally:
if (onAddFilter != null || genericFilter != null) {
... .anyMatch(r -> acceptedByGenericFiler(r) && onAddFilter.accept(r));
So configuring only a generic filter via `setGenericFilter(...)` throws a
NullPointerException as soon as a resource is added, deleted or updated.
All three branches (add / delete / update) are affected, which means
`PollingEventSource`, `PerResourcePollingEventSource` and
`CachingInboundEventSource` all break when used with a generic filter
only.
The existing `genericFilteringEvents` test missed this because it uses a
filter that returns `false`: `&&` short-circuits before the null
dereference. Only a generic filter that accepts a resource reaches the
NPE.
Each filter check is now null-safe (an absent filter accepts), which
preserves the previous behaviour whenever the specific filter is set.
Adds three regression tests, one per branch; they fail with
NullPointerException without this change.
openshift-ci
Bot
added
the
do-not-merge/work-in-progress
16 tasks
Contributor
There was a problem hiding this comment.
Pull request overview
Fixes a NullPointerException in ExternalResourceCachingEventSource when only a generic filter is configured (no add/delete/update-specific filter), ensuring add/delete/update paths remain null-safe and behave as before when specific filters are present.
Changes:
- Make add/delete/update filter evaluation null-safe by delegating to helper methods that treat missing specific filters as “accept”.
- Add regression tests covering add, delete, and update when only
setGenericFilter(...)is set.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/source/ExternalResourceCachingEventSource.java | Prevents NPE by making specific filter invocation null-safe in add/delete/update acceptance logic. |
| operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/event/source/ExternalResourceCachingEventSourceTest.java | Adds regression coverage for generic-filter-only configurations across add/delete/update branches. |
Comment on lines
159
to
+166
| private boolean acceptedByFiler(Map<ID, R> cachedResourceMap, Map<ID, R> newResourcesMap) { | ||
|
|
||
| var addedResources = new HashMap<>(newResourcesMap); | ||
| addedResources.keySet().removeAll(cachedResourceMap.keySet()); | ||
| if (onAddFilter != null || genericFilter != null) { | ||
| var anyAddAccepted = | ||
| addedResources.values().stream() | ||
| .anyMatch(r -> acceptedByGenericFiler(r) && onAddFilter.accept(r)); | ||
| .anyMatch(r -> acceptedByGenericFiler(r) && acceptedByOnAddFilter(r)); |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
acceptedByFilerguards each of its three filter branches withonXFilter != null || genericFilter != null, but the branch bodydereferences
onXFilterunconditionally:So configuring only a generic filter via
setGenericFilter(...)throws aNullPointerException as soon as a resource is added, deleted or updated.
All three branches (add / delete / update) are affected, which means
PollingEventSource,PerResourcePollingEventSourceandCachingInboundEventSourceall break when used with a generic filteronly.
The existing
genericFilteringEventstest missed this because it uses afilter that returns
false:&&short-circuits before the nulldereference. Only a generic filter that accepts a resource reaches the
NPE.
Each filter check is now null-safe (an absent filter accepts), which
preserves the previous behaviour whenever the specific filter is set.
Adds three regression tests, one per branch; they fail with
NullPointerException without this change.
Part of #3517