Skip to content

fix: NPE in external resource caching event source when only a generic filter is set - #3518

Merged
csviri merged 2 commits into
operator-framework:mainfrom
csviri:fix/external-cache-generic-filter-npe
Aug 3, 2026
Merged

fix: NPE in external resource caching event source when only a generic filter is set#3518
csviri merged 2 commits into
operator-framework:mainfrom
csviri:fix/external-cache-generic-filter-npe

Conversation

@csviri

@csviri csviri commented Jul 30, 2026

Copy link
Copy Markdown
Collaborator

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.

Part of #3517

Copilot AI review requested due to automatic review settings July 30, 2026 09:04
@openshift-ci openshift-ci Bot added the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Jul 30, 2026

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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
var anyAddAccepted =
addedResources.values().stream()
.anyMatch(r -> acceptedByGenericFiler(r) && onAddFilter.accept(r));
.anyMatch(r -> acceptedByGenericFiler(r) && acceptedByOnAddFilter(r));
Copilot AI review requested due to automatic review settings August 3, 2026 07:48
@csviri
csviri marked this pull request as ready for review August 3, 2026 07:48
@openshift-ci openshift-ci Bot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Aug 3, 2026

@csviri csviri left a comment

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

csviri added 2 commits August 3, 2026 09:49
…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.
Signed-off-by: Attila Mészáros <a_meszaros@apple.com>
@csviri
csviri force-pushed the fix/external-cache-generic-filter-npe branch from efbdee9 to 1a97b9d Compare August 3, 2026 07:49
@openshift-ci
openshift-ci Bot requested review from metacosm and xstefank August 3, 2026 07:49

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated no new comments.

Copilot AI review requested due to automatic review settings August 3, 2026 07:50

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated no new comments.

@csviri
csviri merged commit 3e85d04 into operator-framework:main Aug 3, 2026
28 checks passed
csviri added a commit to csviri/java-operator-sdk that referenced this pull request Aug 3, 2026
…c filter is set (operator-framework#3518)

`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.

Signed-off-by: Attila Mészáros <a_meszaros@apple.com>
# Conflicts:
#	operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/event/source/ExternalResourceCachingEventSourceTest.java
csviri added a commit to csviri/java-operator-sdk that referenced this pull request Aug 3, 2026
…c filter is set (operator-framework#3518)

`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.

Signed-off-by: Attila Mészáros <a_meszaros@apple.com>
# Conflicts:
#	operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/event/source/ExternalResourceCachingEventSourceTest.java
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants