fix: Fix NPE in ReconciliationDispatcher when custom resource cannot be retrieved on finalizer removal retry

Author: Donnerbart

Diff for: operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/ReconciliationDispatcher.java

@@ -294,12 +294,23 @@ private PostExecutionControl<P> handleCleanup(P resource,
       // cleanup is finished, nothing left to done
       final var finalizerName = configuration().getFinalizerName();
       if (deleteControl.isRemoveFinalizer() && resource.hasFinalizer(finalizerName)) {
-        P customResource = conflictRetryingUpdate(resource, r -> r.removeFinalizer(finalizerName));
+        P customResource = conflictRetryingUpdate(resource, r -> {
+          // the operator might not be allowed to retrieve the resource on a retry, e.g. when its
+          // permissions are removed by deleting the namespace concurrently
+          if (r == null) {
+            log.warn(
+                "Could not remove finalizer on null resource: {} with version: {}",
+                getUID(resource),
+                getVersion(resource));
+            return false;
+          }
+          return r.removeFinalizer(finalizerName);
+        });
         return PostExecutionControl.customResourceFinalizerRemoved(customResource);
       }
     }
     log.debug(
-        "Skipping finalizer remove for resource: {} with version: {}. delete control: {}, uses finalizer: {} ",
+        "Skipping finalizer remove for resource: {} with version: {}. delete control: {}, uses finalizer: {}",
         getUID(resource),
         getVersion(resource),
         deleteControl,

Diff for: operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/event/ReconciliationDispatcherTest.java

@@ -240,6 +240,24 @@ void retriesFinalizerRemovalWithFreshResource() {
     verify(customResourceFacade, times(1)).getResource(any(), any());
   }
 
+  @Test
+  void nullResourceIsGracefullyHandledOnFinalizerRemovalRetry() {
+    // simulate the operator not able or not be allowed to get the custom resource during the retry
+    // of the finalizer removal
+    testCustomResource.addFinalizer(DEFAULT_FINALIZER);
+    markForDeletion(testCustomResource);
+    when(customResourceFacade.updateResource(any()))
+        .thenThrow(new KubernetesClientException(null, 409, null));
+    when(customResourceFacade.getResource(any(), any())).thenReturn(null);
+
+    var postExecControl =
+        reconciliationDispatcher.handleExecution(executionScopeWithCREvent(testCustomResource));
+
+    assertThat(postExecControl.isFinalizerRemoved()).isTrue();
+    verify(customResourceFacade, times(1)).updateResource(testCustomResource);
+    verify(customResourceFacade, times(1)).getResource(any(), any());
+  }
+
   @Test
   void throwsExceptionIfFinalizerRemovalRetryExceeded() {
     testCustomResource.addFinalizer(DEFAULT_FINALIZER);
@@ -307,8 +325,6 @@ void doesNotAddFinalizerIfConfiguredNotTo() {
     assertEquals(0, testCustomResource.getMetadata().getFinalizers().size());
   }
 
-
-
   @Test
   void doesNotRemovesTheSetFinalizerIfTheDeleteNotMethodInstructsIt() {
     testCustomResource.addFinalizer(DEFAULT_FINALIZER);
@@ -590,7 +606,6 @@ void errorStatusHandlerCanPatchResource() {
     reconciler.errorHandler =
         (r, ri, e) -> ErrorStatusUpdateControl.patchStatus(testCustomResource);
 
-
     reconciliationDispatcher.handleExecution(
         new ExecutionScope(null).setResource(testCustomResource));
 
@@ -673,7 +688,6 @@ TestCustomResource createResourceWithFinalizer() {
     return resourceWithFinalizer;
   }
 
-
   private void removeFinalizers(CustomResource customResource) {
     customResource.getMetadata().getFinalizers().clear();
   }