GeezerVison

The World Wide Web, restored to 800 pixels of glory.

GeezerVison is showing github.com — fix: do not NPE when adding a finalizer to a resource deleted mid-retry by csviri · Pull Request #3530 · operator-framework/java-operator-sdk

Conversation

csviri commented Jul 30, 2026

Copy link

Copy Markdown

Collaborator

conflictRetryingPatchPrimary / conflictRetryingPatch re-read the
resource from the API server after a 409 or 422 and then re-evaluate the
precondition:

resource = operation.inNamespace(ns).withName(name).get();

get() returns null if the resource was deleted in the meantime, so the
next iteration calls the precondition with null. removeFinalizer
anticipates this:

r -> {
  if (r == null) {
    log.warn("Cannot remove finalizer since resource not exists.");
    return false;
  }
  return r.hasFinalizer(finalizerName);
}

but addFinalizer passes r -> !r.hasFinalizer(finalizerName), which
throws a NullPointerException instead of exiting cleanly.

Gives addFinalizer the same null guard, in both ResourceOperations and
the deprecated PrimaryUpdateAndCacheUtils.

No test is added: reaching the retry path requires stubbing the client to
answer 409/422 and then 404 through the whole fabric8 DSL chain, which the
existing unit tests are not set up for.

Part of #3517

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

Copilot AI review requested due to automatic review settings

July 30, 2026 09:05

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

csviri mentioned this pull request

Jul 30, 2026

Correctness AI check: collection of possible issues and related PRs #3517

Closed

13 of 16 tasks

Copilot started reviewing on behalf of csviri

July 30, 2026 09:06

View session

Copilot AI reviewed

Jul 30, 2026

View reviewed changes

Copilot AI left a comment

Copy link

Copy Markdown

Contributor

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

Pull request overview

Prevents a NullPointerException when addFinalizer is executed through the conflict-retry patch path and the primary resource is deleted between retries (i.e., the re-read get() returns null). This makes addFinalizer behave consistently with existing removeFinalizer handling in ResourceOperations.

Changes:

Add a null-guard to the addFinalizer precondition in ResourceOperations to avoid dereferencing a deleted resource during retry.
Add the same null-guard to the deprecated PrimaryUpdateAndCacheUtils.addFinalizer retry precondition (and log when the resource is gone).

Reviewed changes

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

File	Description
operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/reconciler/ResourceOperations.java	Adds null-guarded precondition for `addFinalizer` during conflict-retry patch.
operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/reconciler/PrimaryUpdateAndCacheUtils.java	Adds null-guarded precondition for deprecated `addFinalizer` during conflict-retry patch.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

Comment thread

...ore/src/main/java/io/javaoperatorsdk/operator/api/reconciler/PrimaryUpdateAndCacheUtils.java Resolved

Uh oh!

There was an error while loading. Please reload this page.

Comment thread

...mework-core/src/main/java/io/javaoperatorsdk/operator/api/reconciler/ResourceOperations.java Resolved

Uh oh!

There was an error while loading. Please reload this page.

Copilot AI review requested due to automatic review settings

August 3, 2026 08:33

csviri marked this pull request as ready for review

August 3, 2026 08:34

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

openshift-ci Bot requested review from metacosm and xstefank

August 3, 2026 08:34

csviri and others added 2 commits

August 3, 2026 10:34


          fix: do not NPE when adding a finalizer to a resource deleted mid-retry

8905de2

`conflictRetryingPatchPrimary` / `conflictRetryingPatch` re-read the
resource from the API server after a 409 or 422 and then re-evaluate the
precondition:

    resource = operation.inNamespace(ns).withName(name).get();

`get()` returns null if the resource was deleted in the meantime, so the
next iteration calls the precondition with null. `removeFinalizer`
anticipates this:

    r -> {
      if (r == null) {
        log.warn("Cannot remove finalizer since resource not exists.");
        return false;
      }
      return r.hasFinalizer(finalizerName);
    }

but `addFinalizer` passes `r -> !r.hasFinalizer(finalizerName)`, which
throws a NullPointerException instead of exiting cleanly.

Gives `addFinalizer` the same null guard, in both `ResourceOperations` and
the deprecated `PrimaryUpdateAndCacheUtils`.

No test is added: reaching the retry path requires stubbing the client to
answer 409/422 and then 404 through the whole fabric8 DSL chain, which the
existing unit tests are not set up for.


          Apply suggestions from code review

3b0a833

Co-authored-by: Copilot Autofix powered by AI <[email protected]>

csviri force-pushed the fix/add-finalizer-null-resource branch from e92fb53 to 3b0a833 Compare

August 3, 2026 08:34

Copilot started reviewing on behalf of csviri

August 3, 2026 08:37

View session

Copilot AI reviewed

Aug 3, 2026

View reviewed changes

Copilot AI left a comment

Copy link

Copy Markdown

Contributor

There was a problem hiding this comment.

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.

Suppressed comments (2)

operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/reconciler/ResourceOperations.java:1094

addFinalizer can now return null when the resource is deleted mid-retry (because the retry loop re-fetches with .get() which may return null, and the precondition returns false in that case). The Javadoc currently says it returns the updated resource, which no longer fully describes the behavior; please document the possible null return so callers know they must handle it.

        r -> {
          if (r == null) {
            log.warn("Cannot add finalizer since resource no longer exists.");
            return false;
          }
          return !r.hasFinalizer(finalizerName);
        },

operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/reconciler/PrimaryUpdateAndCacheUtils.java:298

addFinalizer can now return null when the resource is deleted mid-retry (the retry loop re-fetches with .get() which may return null, and the precondition returns false). The Javadoc currently states it returns the updated resource, which is no longer complete; please document the possible null return value for callers.

        r -> {
          if (r == null) {
            log.warn("Cannot add finalizer since resource no longer exists.");
            return false;
          }
          return !r.hasFinalizer(finalizerName);
        });

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

csviri commented

Aug 3, 2026

View reviewed changes

csviri left a comment

Copy link

Copy Markdown

Collaborator Author

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

LGTM

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

csviri merged commit 96e59cd into operator-framework:main

Aug 3, 2026

28 checks passed

Uh oh!

There was an error while loading. Please reload this page.

Reviewers

Copilot code review Copilot

Copilot review effort

Applies to this pull request for everyone.Learn more about Copilot code review.

Copilot left review comments

metacosm

Awaiting requested review from metacosm

xstefank

Awaiting requested review from xstefank

Uh oh!

There was an error while loading. Please reload this page.

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Uh oh!

There was an error while loading. Please reload this page.

2 participants

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.

You are visitor 01402503 — GeezerVison does not store the pages you look up.
Best viewed with Netscape Navigator 4.7 or Internet Explorer 6 at 800×600. No cookies required.