NPE in `ManagedInformerEventSource.eventFilteringUpdateAndCacheResource` when patch target was already deleted on the API server

[Donnerbart]

Bug Report

What did you do?

ResourceOperations.removeFinalizer ultimately calls client.resource(r).edit(unaryOperator) via ManagedInformerEventSource.eventFilteringUpdateAndCacheResource. When that edit() returns null (fabric8's behavior on HTTP 404 when the target is gone), the result is fed unguarded into TemporaryResourceCache.putResource, which calls ResourceID.fromResource(null) and throws.

The code path that handles this null is missing; see ManagedInformerEventSource.java:101-103 on current main (5.3.4-SNAPSHOT). For comparison, removeFinalizer's own retry-precondition guard already null-checks the resource and returns gracefully (ResourceOperations.java:636-639); this one does not.

Concrete observed reproduction: a controller implementing Reconciler + Cleaner, using KubernetesMockServer (fabric8 7.6.1) in test teardown. The mock server honours finalizers, so the typical sequence we observed was:

1. Test calls client.resources(...).inNamespace(ns).delete().
2. Mock server sets deletionTimestamp and keeps the resource (finalizer present).
3. Informer emits an update event; JOSDK runs handleCleanup; removeFinalizer patches the finalizer off successfully.
4. Mock server actually removes the resource.
5. A subsequent cleanup pass runs (the dispatcher had a superseding event queued — could be the delete event or another update; logs show deleteEvent=false, so in this case it was an update event). This pass's patch reaches the API server after the resource is gone; edit() returns null; NPE.

We have not characterized this on a real cluster — no production observations either way. The bug is reported because the code path is unguarded against a documented null return from fabric8, and we have a deterministic in-process reproducer (below).

What did you expect to see?

A clean return (no exception), matching the existing precondition guard in ResourceOperations.removeFinalizer's retry path:

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

What did you see instead? Under which circumstances?

NullPointerException thrown from TemporaryResourceCache.putResource, surfaced as a WARN in EventProcessor ("Uncaught error during event processing ... another reconciliation will be attempted because a superseding event has been received").

java.lang.NullPointerException: Cannot invoke "io.fabric8.kubernetes.api.model.HasMetadata.getMetadata()" because "resource" is null
  at io.javaoperatorsdk.operator.processing.event.ResourceID.fromResource(ResourceID.java:28)
  at io.javaoperatorsdk.operator.processing.event.source.informer.TemporaryResourceCache.putResource(TemporaryResourceCache.java:180)
  at io.javaoperatorsdk.operator.processing.event.source.informer.ManagedInformerEventSource.eventFilteringUpdateAndCacheResource(ManagedInformerEventSource.java:103)
  at io.javaoperatorsdk.operator.api.reconciler.ResourceOperations.resourcePatch(ResourceOperations.java:567)
  at io.javaoperatorsdk.operator.api.reconciler.ResourceOperations.jsonPatchPrimary(ResourceOperations.java:409)
  at io.javaoperatorsdk.operator.api.reconciler.ResourceOperations.conflictRetryingPatchPrimary(ResourceOperations.java:668)
  at io.javaoperatorsdk.operator.api.reconciler.ResourceOperations.removeFinalizer(ResourceOperations.java:630)

Triggering circumstances:

1. client.resources(...).inNamespace(ns).delete() is called against a resource whose reconciler manages a finalizer.
2. Mock server (and real API server) honour the finalizer: deletionTimestamp is set, resource still present.
3. Informer emits an update event with deletionTimestamp set.
4. JOSDK runs handleCleanup and removeFinalizer patches the finalizer off successfully.
5. Mock server removes the resource.
6. A second cleanup pass is dispatched (superseding event already queued). This time edit() returns null and the NPE fires.

The relevant call site (ManagedInformerEventSource.eventFilteringUpdateAndCacheResource, current main / 5.3.4-SNAPSHOT):

updatedResource = updateMethod.apply(resourceToUpdate);   // can return null
log.debug("Resource update successful");
handleRecentResourceUpdate(id, updatedResource, resourceToUpdate);  // NPE: no null guard

handleRecentResourceUpdate forwards to temporaryResourceCache.putResource(null), which fails at ResourceID.fromResource(null).

Environment

Kubernetes cluster type:

vanilla; reproducer is also deterministic against fabric8 KubernetesMockServer.

$ Mention java-operator-sdk version from pom.xml file

Originally observed on 5.3.3; reproducer (below) also confirmed against current main / 5.3.4-SNAPSHOT.

$ java -version

openjdk version "21"

$ kubectl version

N/A (reproduced via KubernetesMockServer and via direct unit test; not cluster-dependent).

Possible Solution

Null-guard the call in ManagedInformerEventSource.eventFilteringUpdateAndCacheResource:

updatedResource = updateMethod.apply(resourceToUpdate);
if (updatedResource != null) {
  log.debug("Resource update successful");
  handleRecentResourceUpdate(id, updatedResource, resourceToUpdate);
} else {
  log.debug("Update returned null, skipping cache update");
}
return updatedResource;

The existing finally block already null-checks updatedResource for version tracking, so only this direct call path needs the guard.

Additional context

Reproducer test (no cluster required), demonstrating the call path from removeFinalizer all the way down to the NPE:

@Test
void removeFinalizerHandlesAlreadyDeletedTargetGracefully() {
  // When the underlying patch returns null because the target resource is already gone
  // on the API server (fabric8's edit() returns null on 404), removeFinalizer must
  // return cleanly rather than throw.
  var resource = TestUtils.testCustomResource1();
  resource.getMetadata().setResourceVersion("1");
  resource.addFinalizer(FINALIZER_NAME);
  when(context.getPrimaryResource()).thenReturn(resource);

  var client = MockKubernetesClient.client(TestCustomResource.class);
  NamespaceableResource<TestCustomResource> single = mock();
  when(single.edit(any(UnaryOperator.class))).thenReturn(null);
  when(client.resource(any(TestCustomResource.class))).thenReturn(single);

  var eventSource = new ControllerEventSource<>(testController(client));
  eventSource.start();
  try {
    when(context.getClient()).thenReturn(client);
    var eventSourceRetriever = mock(EventSourceRetriever.class);
    when(context.eventSourceRetriever()).thenReturn(eventSourceRetriever);
    when(eventSourceRetriever.getControllerEventSource()).thenReturn(eventSource);

    var result = resourceOperations.removeFinalizer(FINALIZER_NAME);

    assertThat(result).isNull();
  } finally {
    eventSource.stop();
  }
}

A second, narrower reproducer at the ManagedInformerEventSource layer:

@Test
void eventFilteringUpdateHandlesNullFromUpdateMethodGracefully() {
  withRealTemporaryResourceCache();
  var result = informerEventSource.eventFilteringUpdateAndCacheResource(testDeployment(), r -> null);
  assertThat(result).isNull();
}

Both fail today with the NPE shown above and pass with the proposed null-guard.

Related prior issue (different flow, same class of bug): #1987 — fixed in the old ReconciliationDispatcher flow; the 5.x rewrite moved the logic to ResourceOperations + ManagedInformerEventSource and regressed null-safety on the cache-update side.

[Donnerbart]

Updated cause analysis. The "fabric8 returns null on 404" framing in the original report is wrong. After tracing the actual path, the null does come back from client.resource(r).edit(unaryOperator), but via an empty response body, not a 404. Details below.

Minimal trigger surface: a single client.resources(...).delete() against a CR whose reconciler implements Cleaner, while the operator is running against KubernetesMockServer. No external setup or special event ordering required. The race is internal to the operator/mock interaction during cleanup: when the cleanup PATCH that removes the last finalizer happens to be the one that lands cleanly (vs. early-returning because another pass already removed it, vs. hitting a conflict), the mock's empty-body branch fires and the NPE follows.

Real-world stack

Observed as intermittent failures across our unit-test suite. Tests using KubernetesMockServer with a Cleaner-implementing reconciler hit it on cleanup; not every run, but often enough to be visibly flaky. Log from a unit test failure:

WARN  i.j.o.p.event.EventProcessor - Uncaught error during event processing ExecutionScope{resource=ResourceID{name='hivemq-unit-test-platform', namespace='hivemqplatformoperatormetricstest'}, retryInfo=null, deleteEvent=false, isDeleteFinalStateUnknown=false} - but another reconciliation will be attempted because a superseding event has been received or another retry attempt is pending.
java.lang.NullPointerException: Cannot invoke "io.fabric8.kubernetes.api.model.HasMetadata.getMetadata()" because "resource" is null
    at io.javaoperatorsdk.operator.processing.event.ResourceID.fromResource(ResourceID.java:28)
    at io.javaoperatorsdk.operator.processing.event.source.informer.TemporaryResourceCache.putResource(TemporaryResourceCache.java:180)
    at io.javaoperatorsdk.operator.processing.event.source.informer.ManagedInformerEventSource.handleRecentResourceUpdate(ManagedInformerEventSource.java:184)
    at io.javaoperatorsdk.operator.processing.event.source.informer.ManagedInformerEventSource.eventFilteringUpdateAndCacheResource(ManagedInformerEventSource.java:103)
    at io.javaoperatorsdk.operator.api.reconciler.ResourceOperations.resourcePatch(ResourceOperations.java:567)
    at io.javaoperatorsdk.operator.api.reconciler.ResourceOperations.jsonPatchPrimary(ResourceOperations.java:409)
    at io.javaoperatorsdk.operator.api.reconciler.ResourceOperations.conflictRetryingPatchPrimary(ResourceOperations.java:668)
    at io.javaoperatorsdk.operator.api.reconciler.ResourceOperations.removeFinalizer(ResourceOperations.java:630)
    at io.javaoperatorsdk.operator.api.reconciler.ResourceOperations.removeFinalizer(ResourceOperations.java:614)
    at io.javaoperatorsdk.operator.processing.event.ReconciliationDispatcher.handleCleanup(ReconciliationDispatcher.java:313)
    at io.javaoperatorsdk.operator.processing.event.ReconciliationDispatcher.handleDispatch(ReconciliationDispatcher.java:108)
    at io.javaoperatorsdk.operator.processing.event.ReconciliationDispatcher.handleExecution(ReconciliationDispatcher.java:75)
    at io.javaoperatorsdk.operator.processing.event.EventProcessor$ReconcilerExecutor.run(EventProcessor.java:535)
    at java.base/java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1090)
    at java.base/java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:614)
    at java.base/java.lang.Thread.run(Thread.java:1474)

Actual trigger, three layers

1. KubernetesMockServer (kubernetes-server-mock 7.6.1, PatchHandler.java:89-96): when a PATCH leaves the resource marked for deletion AND with zero finalizers (i.e. the moment removeFinalizer removes the last finalizer on a deletion-pending resource), the mock returns HTTP 202 with an empty body and deletes the resource:

if (deserializedResource.isMarkedForDeletion() && deserializedResource.getFinalizers().isEmpty()) {
  // Delete the resource.
  updatedAsString = null;
  deserializedResource = null;
}
...
return new MockResponse().setResponseCode(HTTP_ACCEPTED).setBody(Utils.getNonNullOrElse(updatedAsString, ""));

2. fabric8 (kubernetes-client 7.6.1): 202 is a 2xx, so OperationSupport.assertResponseCode does not throw. The empty body is then handed to KubernetesSerialization.unmarshal, which (since the input does not start with { or [) falls into parseYaml. parseYaml initialises result = null and finds zero documents to iterate, so it returns null. Net effect: client.resource(r).edit(unaryOperator) returns null cleanly, no exception thrown. So edit() does throw on 404 (as called out in the PR thread on #3332); it just also has this empty-body path that returns null.

3. JOSDK (ManagedInformerEventSource.java:101-103): that null is forwarded into handleRecentResourceUpdate → TemporaryResourceCache.putResource(null) → ResourceID.fromResource(null) → NPE.

Correction to the original sequence in the issue body

The original report described the failing pass as a "second cleanup pass" after a successful first one, implying a queued superseding event was the trigger. That framing is misleading. The failing pass is whichever cleanup pass ends up being the one that PATCH-removes the last finalizer; the "superseding event has been received" line in the WARN is the delete event the mock emits asynchronously after deleting the resource, not evidence of a prior successful pass.

Where should the fix live?

IT result, for the K8s-vs-mock question: 0 NPEs across 57 IT runs against real k3s on JOSDK 5.3.3 + fabric8 7.6.1; consistent reproduction in our unit tests against KubernetesMockServer. Looks like a mock-only divergence, not a production bug.

Given that, the fix could live in:

• KubernetesMockServer: the mock's empty-body branch (PatchHandler:89-96) is the primary divergence from real K8s and affects every JOSDK user with mock-based tests. Will file upstream against fabric8.
• fabric8 client: an unmarshal of an empty body returning null via parseYaml is a silent surprise; an explicit null from edit() on a successful (2xx) response is the deeper API quirk and may or may not be intentional.
• JOSDK: ResourceOperations.resourcePatch(R, UnaryOperator<R>) is a public API. Users can pass arbitrary UnaryOperator<R> lambdas, and a null return currently drops into an opaque NPE inside the cache layer rather than surfacing where the user can act on it. Fix NPE in ManagedInformerEventSource #3332 takes the simplest shape: skip the cache update at eventFilteringUpdateAndCacheResource:101-103 and return null up the stack. Tradeoffs:
  • Pro: localized, low-risk, ships the flaky-test fix immediately for everyone using the mock server. Matches the existing precondition pattern at ResourceOperations.java:636-639.
  • Con: silently propagates null all the way up to ReconciliationDispatcher.handleCleanup through public methods (resourcePatch, jsonPatchPrimary, removeFinalizer) that are typed as returning P and not documented to return null. I have not confirmed every caller handles null gracefully.
  • Alternative shapes: (a) fail loud with Objects.requireNonNull(updatedResource, ...) so user-lambda bugs surface immediately, at the cost of not fixing the mock flakiness without a separate change; (b) translate null to the input resource at the ResourceOperations layer, mirroring the existing r == null precondition guard, so the public-API contract stays "non-null P"; (c) hybrid: null-guard at the IES level (no NPE) plus translate at the public-API level (no null escapes the public surface).

I can refactor #3332 along any of the alternatives above if preferred.

[shawkins]

Alternative shapes

I'd vote to keep things simple in the JOSDK and use Objects.requireNonNull or an assertion.

fabric8 client: an unmarshal of an empty body returning null via parseYaml is a silent surprise;

That is coming straight from the SnakeYAML handling - could be a good for a discussion item or an issue in their project.

KubernetesMockServer: the mock's empty-body branch (PatchHandler:89-96) is the primary divergence from real K8s and affects every JOSDK user with mock-based tests. Will file upstream against fabric8.

Sounds good. It's been a slow process, but ideally we'd like to encourage people to use kube-api-test instead of the crud mock.

[csviri]

I would also vote for use Objects.requireNonNull. Since this is an update it is reasonable to expect a resource or exception in case something went wrong.
@Donnerbart if you could prepare the PR would be awesome, otherwise I can do that.

thank you @Donnerbart and @shawkins !