Skip to content

fix: NPE in handleRecentResourceUpdate for an unknown secondary resource - #3519

Draft
csviri wants to merge 1 commit into
operator-framework:mainfrom
csviri:fix/external-cache-recent-update-npe
Draft

fix: NPE in handleRecentResourceUpdate for an unknown secondary resource#3519
csviri wants to merge 1 commit into
operator-framework:mainfrom
csviri:fix/external-cache-recent-update-npe

Conversation

@csviri

@csviri csviri commented Jul 30, 2026

Copy link
Copy Markdown
Collaborator

ExternalResourceCachingEventSource.handleRecentResourceUpdate checks
that the primary has an entry in the cache, but then dereferences the
per-secondary lookup without checking it:

R actualResource = actualValues.get(resourceId);
if (actualResource.equals(previousVersionOfResource)) {

actualValues.get(resourceId) returns null whenever the primary has a
cache entry but that particular secondary id is not in it, which throws a
NullPointerException. This is reachable from
AbstractEventSourceHolderDependentResource.onUpdated for any
RecentOperationCacheFiller event source, e.g. after an update whose
resource id is not the one currently cached for that primary.

Skips the cache update when the resource is not tracked, which matches
the intent of the surrounding "only overwrite if we still hold the
version the caller saw" check.

Adds a regression test that fails with NullPointerException without this
change.

Part of #3517

`ExternalResourceCachingEventSource.handleRecentResourceUpdate` checks
that the primary has an entry in the cache, but then dereferences the
per-secondary lookup without checking it:

    R actualResource = actualValues.get(resourceId);
    if (actualResource.equals(previousVersionOfResource)) {

`actualValues.get(resourceId)` returns null whenever the primary has a
cache entry but that particular secondary id is not in it, which throws a
NullPointerException. This is reachable from
`AbstractEventSourceHolderDependentResource.onUpdated` for any
`RecentOperationCacheFiller` event source, e.g. after an update whose
resource id is not the one currently cached for that primary.

Skips the cache update when the resource is not tracked, which matches
the intent of the surrounding "only overwrite if we still hold the
version the caller saw" check.

Adds a regression test that fails with NullPointerException without this
change.
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.handleRecentResourceUpdate when a primary is cached but the specific secondary resource ID is not tracked, aligning behavior with the intended “only overwrite if the cached value matches the caller’s previous version” semantics.

Changes:

  • Guarded the equality check in handleRecentResourceUpdate to avoid dereferencing a missing cached secondary entry.
  • Added a regression test ensuring updates for unknown secondary resources are ignored (and do not throw).

Reviewed changes

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

File Description
operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/source/ExternalResourceCachingEventSource.java Adds a null check before comparing cached vs previous resource to prevent NPE and skip untracked secondary updates.
operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/event/source/ExternalResourceCachingEventSourceTest.java Adds a regression test covering the “unknown secondary ID” update scenario.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants