Controller name as finalizer by default

.github/workflows/ci.yml

@@ -29,7 +29,7 @@ jobs:
       - name: Set up Minikube
         uses: manusa/actions-setup-minikube@v2.0.1
         with:
-          minikube version: 'v1.15.0'
+          minikube version: 'v1.15.1'
           kubernetes version: ${{ matrix.kubernetes }}
           driver: 'docker'
       - name: Run integration tests

operator-framework/src/main/java/io/javaoperatorsdk/operator/ControllerUtils.java

@@ -16,23 +16,27 @@
 public class ControllerUtils {
 
     private final static double JAVA_VERSION = Double.parseDouble(System.getProperty("java.specification.version"));
-
-    private final static Logger log = LoggerFactory.getLogger(ControllerUtils.class);
+    private static final String FINALIZER_NAME_SUFFIX = "/finalizer";
 
     // this is just to support testing, this way we don't try to create class multiple times in memory with same name.
     // note that other solution is to add a random string to doneable class name
     private static Map<Class<? extends CustomResource>, Class<? extends CustomResourceDoneable<? extends CustomResource>>>
             doneableClassCache = new HashMap<>();
 
-    static String getDefaultFinalizer(ResourceController controller) {
-        return getAnnotation(controller).finalizerName();
+    static String getFinalizer(ResourceController controller) {
+        final String annotationFinalizerName = getAnnotation(controller).finalizerName();
+        if (!Controller.NULL.equals(annotationFinalizerName)) {
+            return annotationFinalizerName;
+        }
+        final String crdName = getAnnotation(controller).crdName() + FINALIZER_NAME_SUFFIX;
+        return crdName;
     }
 
     static boolean getGenerationEventProcessing(ResourceController controller) {
         return getAnnotation(controller).generationAwareEventProcessing();
     }
 
-    static <R extends CustomResource> Class<R> getCustomResourceClass(ResourceController controller) {
+    static <R extends CustomResource> Class<R> getCustomResourceClass(ResourceController<R> controller) {
         return (Class<R>) getAnnotation(controller).customResourceClass();
     }
 
@@ -79,10 +83,7 @@ private static Controller getAnnotation(ResourceController controller) {
         return controller.getClass().getAnnotation(Controller.class);
     }
 
-    public static boolean hasDefaultFinalizer(CustomResource resource, String finalizer) {
-        if (resource.getMetadata().getFinalizers() != null) {
-            return resource.getMetadata().getFinalizers().contains(finalizer);
-        }
-        return false;
+    public static boolean hasGivenFinalizer(CustomResource resource, String finalizer) {
+        return resource.getMetadata().getFinalizers() != null && resource.getMetadata().getFinalizers().contains(finalizer);
     }
 }

operator-framework/src/main/java/io/javaoperatorsdk/operator/Operator.java

@@ -55,7 +55,7 @@ private <R extends CustomResource> void registerController(ResourceController<R>
         Class<R> resClass = ControllerUtils.getCustomResourceClass(controller);
         CustomResourceDefinitionContext crd = getCustomResourceDefinitionForController(controller);
         KubernetesDeserializer.registerCustomKind(crd.getVersion(), crd.getKind(), resClass);
-        String finalizer = ControllerUtils.getDefaultFinalizer(controller);
+        String finalizer = ControllerUtils.getFinalizer(controller);
         MixedOperation client = k8sClient.customResources(crd, resClass, CustomResourceList.class, ControllerUtils.getCustomResourceDoneableClass(controller));
         EventDispatcher eventDispatcher = new EventDispatcher(controller,
                 finalizer, new EventDispatcher.CustomResourceFacade(client), ControllerUtils.getGenerationEventProcessing(controller));

operator-framework/src/main/java/io/javaoperatorsdk/operator/api/Controller.java

@@ -10,14 +10,17 @@
 @Retention(RetentionPolicy.RUNTIME)
 @Target({ElementType.TYPE})
 public @interface Controller {
-
-    String DEFAULT_FINALIZER = "operator.default.finalizer";
+    String NULL = "";
 
     String crdName();
 
     Class<? extends CustomResource> customResourceClass();
 
-    String finalizerName() default DEFAULT_FINALIZER;
+    /**
+     * Optional finalizer name, if it is not,
+     * the crdName will be used as the name of the finalizer too.
+     */
+    String finalizerName() default NULL;
 
     /**
      * If true, will dispatch new event to the controller if generation increased since the last processing, otherwise will

operator-framework/src/main/java/io/javaoperatorsdk/operator/api/ResourceController.java

@@ -6,7 +6,7 @@ public interface ResourceController<R extends CustomResource> {
 
     /**
      * The implementation should delete the associated component(s). Note that this is method is called when an object
-     * is marked for deletion. After its executed the default finalizer is automatically removed by the framework;
+     * is marked for deletion. After its executed the custom resource finalizer is automatically removed by the framework;
      * unless the return value is false - note that this is almost never the case.
      * Its important to have the implementation also idempotent, in the current implementation to cover all edge cases
      * actually will be executed mostly twice.
@@ -27,5 +27,4 @@ public interface ResourceController<R extends CustomResource> {
      * <b>However we will always call an update if there is no finalizer on object and its not marked for deletion.</b>
      */
     UpdateControl<R> createOrUpdateResource(R resource, Context<R> context);
-
 }

operator-framework/src/main/java/io/javaoperatorsdk/operator/processing/EventDispatcher.java

@@ -21,17 +21,17 @@ public class EventDispatcher {
     private final static Logger log = LoggerFactory.getLogger(EventDispatcher.class);
 
     private final ResourceController controller;
-    private final String resourceDefaultFinalizer;
+    private final String resourceFinalizer;
     private final CustomResourceFacade customResourceFacade;
     private final boolean generationAware;
     private final Map<String, Long> lastGenerationProcessedSuccessfully = new ConcurrentHashMap<>();
 
     public EventDispatcher(ResourceController controller,
-                           String defaultFinalizer,
+                           String finalizer,
                            CustomResourceFacade customResourceFacade, boolean generationAware) {
         this.controller = controller;
         this.customResourceFacade = customResourceFacade;
-        this.resourceDefaultFinalizer = defaultFinalizer;
+        this.resourceFinalizer = finalizer;
         this.generationAware = generationAware;
     }
 
@@ -43,24 +43,24 @@ public void handleEvent(CustomResourceEvent event) {
             log.error("Received error for resource: {}", resource.getMetadata().getName());
             return;
         }
-        if (markedForDeletion(resource) && !ControllerUtils.hasDefaultFinalizer(resource, resourceDefaultFinalizer)) {
-            log.debug("Skipping event dispatching since its marked for deletion but has no default finalizer: {}", event);
+        if (markedForDeletion(resource) && !ControllerUtils.hasGivenFinalizer(resource, resourceFinalizer)) {
+            log.debug("Skipping event dispatching since its marked for deletion but does not have finalizer: {}", event);
             return;
         }
         Context context = new DefaultContext(new RetryInfo(event.getRetryCount(), event.getRetryExecution().isLastExecution()));
         if (markedForDeletion(resource)) {
             boolean removeFinalizer = controller.deleteResource(resource, context);
-            boolean hasDefaultFinalizer = ControllerUtils.hasDefaultFinalizer(resource, resourceDefaultFinalizer);
-            if (removeFinalizer && hasDefaultFinalizer) {
-                removeDefaultFinalizer(resource);
+            boolean hasFinalizer = ControllerUtils.hasGivenFinalizer(resource, resourceFinalizer);
+            if (removeFinalizer && hasFinalizer) {
+                removeFinalizer(resource);
             } else {
-                log.debug("Skipping finalizer remove. removeFinalizer: {}, hasDefaultFinalizer: {} ",
-                        removeFinalizer, hasDefaultFinalizer);
+                log.debug("Skipping finalizer remove. removeFinalizer: {}, hasFinalizer: {} ",
+                        removeFinalizer, hasFinalizer);
             }
             cleanup(resource);
         } else {
-            if (!ControllerUtils.hasDefaultFinalizer(resource, resourceDefaultFinalizer) && !markedForDeletion(resource)) {
-                /*  We always add the default finalizer if missing and not marked for deletion.
+            if (!ControllerUtils.hasGivenFinalizer(resource, resourceFinalizer) && !markedForDeletion(resource)) {
+                /*  We always add a finalizer if missing and not marked for deletion.
                     We execute the controller processing only for processing the event sent as a results
                     of the finalizer add. This will make sure that the resources are not created before
                     there is a finalizer.
@@ -118,9 +118,9 @@ private void updateCustomResource(CustomResource updatedResource) {
     }
 
 
-    private void removeDefaultFinalizer(CustomResource resource) {
+    private void removeFinalizer(CustomResource resource) {
         log.debug("Removing finalizer on resource {}:", resource);
-        resource.getMetadata().getFinalizers().remove(resourceDefaultFinalizer);
+        resource.getMetadata().getFinalizers().remove(resourceFinalizer);
         customResourceFacade.replaceWithLock(resource);
     }
 
@@ -130,12 +130,12 @@ private void replace(CustomResource resource) {
     }
 
     private void addFinalizerIfNotPresent(CustomResource resource) {
-        if (!ControllerUtils.hasDefaultFinalizer(resource, resourceDefaultFinalizer) && !markedForDeletion(resource)) {
-            log.info("Adding default finalizer to {}", resource.getMetadata());
+        if (!ControllerUtils.hasGivenFinalizer(resource, resourceFinalizer) && !markedForDeletion(resource)) {
+            log.info("Adding finalizer {} to {}", resourceFinalizer, resource.getMetadata());
             if (resource.getMetadata().getFinalizers() == null) {
                 resource.getMetadata().setFinalizers(new ArrayList<>(1));
             }
-            resource.getMetadata().getFinalizers().add(resourceDefaultFinalizer);
+            resource.getMetadata().getFinalizers().add(resourceFinalizer);
         }
     }
 

operator-framework/src/test/java/io/javaoperatorsdk/operator/ControllerExecutionIT.java

@@ -21,7 +21,6 @@
 @TestInstance(TestInstance.Lifecycle.PER_METHOD)
 public class ControllerExecutionIT {
 
-    private final static Logger log = LoggerFactory.getLogger(ControllerExecutionIT.class);
     public static final String TEST_CUSTOM_RESOURCE_NAME = "test-custom-resource";
     private IntegrationTestSupport integrationTestSupport = new IntegrationTestSupport();
 

operator-framework/src/test/java/io/javaoperatorsdk/operator/ControllerUtilsTest.java

@@ -1,23 +1,46 @@
 package io.javaoperatorsdk.operator;
 
-import io.javaoperatorsdk.operator.sample.TestCustomResource;
-import io.javaoperatorsdk.operator.sample.TestCustomResourceController;
 import io.fabric8.kubernetes.client.CustomResourceDoneable;
+import io.javaoperatorsdk.operator.api.Context;
 import io.javaoperatorsdk.operator.api.Controller;
+import io.javaoperatorsdk.operator.api.ResourceController;
+import io.javaoperatorsdk.operator.api.UpdateControl;
+import io.javaoperatorsdk.operator.sample.TestCustomResource;
+import io.javaoperatorsdk.operator.sample.TestCustomResourceController;
 import org.junit.jupiter.api.Assertions;
 import org.junit.jupiter.api.Test;
 
-import static org.junit.jupiter.api.Assertions.assertEquals;
-import static org.junit.jupiter.api.Assertions.assertTrue;
+import static org.junit.jupiter.api.Assertions.*;
 
 class ControllerUtilsTest {
 
+    public static final String CUSTOM_FINALIZER_NAME = "a.custom/finalizer";
+
     @Test
     public void returnsValuesFromControllerAnnotationFinalizer() {
-        Assertions.assertEquals(Controller.DEFAULT_FINALIZER, ControllerUtils.getDefaultFinalizer(new TestCustomResourceController(null)));
+        Assertions.assertEquals(TestCustomResourceController.CRD_NAME + "/finalizer", ControllerUtils.getFinalizer(new TestCustomResourceController(null)));
         assertEquals(TestCustomResource.class, ControllerUtils.getCustomResourceClass(new TestCustomResourceController(null)));
         Assertions.assertEquals(TestCustomResourceController.CRD_NAME, ControllerUtils.getCrdName(new TestCustomResourceController(null)));
-        assertEquals(false, ControllerUtils.getGenerationEventProcessing(new TestCustomResourceController(null)));
+        assertFalse(ControllerUtils.getGenerationEventProcessing(new TestCustomResourceController(null)));
         assertTrue(CustomResourceDoneable.class.isAssignableFrom(ControllerUtils.getCustomResourceDoneableClass(new TestCustomResourceController(null))));
     }
+
+    @Controller(crdName = "test.crd", customResourceClass = TestCustomResource.class, finalizerName = CUSTOM_FINALIZER_NAME)
+    static class TestCustomFinalizerController implements ResourceController<TestCustomResource> {
+
+        @Override
+        public boolean deleteResource(TestCustomResource resource, Context<TestCustomResource> context) {
+            return false;
+        }
+
+        @Override
+        public UpdateControl<TestCustomResource> createOrUpdateResource(TestCustomResource resource, Context<TestCustomResource> context) {
+            return null;
+        }
+    }
+
+    @Test
+    public void returnCustomerFinalizerNameIfSet() {
+        assertEquals(CUSTOM_FINALIZER_NAME, ControllerUtils.getFinalizer(new TestCustomFinalizerController()));
+    }
 }

operator-framework/src/test/java/io/javaoperatorsdk/operator/EventDispatcherTest.java

@@ -20,15 +20,16 @@
 
 class EventDispatcherTest {
 
+    public static final String FINALIZER_NAME = "finalizer";
     private CustomResource testCustomResource;
     private EventDispatcher eventDispatcher;
-    private ResourceController<CustomResource> controller = mock(ResourceController.class);
-    private EventDispatcher.CustomResourceFacade customResourceFacade = mock(EventDispatcher.CustomResourceFacade.class);
+    private final ResourceController<CustomResource> controller = mock(ResourceController.class);
+    private final EventDispatcher.CustomResourceFacade customResourceFacade = mock(EventDispatcher.CustomResourceFacade.class);
 
     @BeforeEach
     void setup() {
         eventDispatcher = new EventDispatcher(controller,
-                Controller.DEFAULT_FINALIZER, customResourceFacade, false);
+                FINALIZER_NAME, customResourceFacade, false);
 
         testCustomResource = getResource();
 
@@ -45,7 +46,7 @@ void callCreateOrUpdateOnNewResource() {
 
     @Test
     void updatesOnlyStatusSubResource() {
-        testCustomResource.getMetadata().getFinalizers().add(Controller.DEFAULT_FINALIZER);
+        testCustomResource.getMetadata().getFinalizers().add(FINALIZER_NAME);
         when(controller.createOrUpdateResource(eq(testCustomResource), any()))
                 .thenReturn(UpdateControl.updateStatusSubResource(testCustomResource));
 
@@ -63,17 +64,17 @@ void callCreateOrUpdateOnModifiedResource() {
     }
 
     @Test
-    void adsDefaultFinalizerOnCreateIfNotThere() {
+    void addsFinalizerOnCreateIfNotThere() {
         eventDispatcher.handleEvent(customResourceEvent(Watcher.Action.MODIFIED, testCustomResource));
         verify(controller, times(1))
                 .createOrUpdateResource(argThat(testCustomResource ->
-                        testCustomResource.getMetadata().getFinalizers().contains(Controller.DEFAULT_FINALIZER)), any());
+                        testCustomResource.getMetadata().getFinalizers().contains(FINALIZER_NAME)), any());
     }
 
     @Test
     void callsDeleteIfObjectHasFinalizerAndMarkedForDelete() {
         testCustomResource.getMetadata().setDeletionTimestamp("2019-8-10");
-        testCustomResource.getMetadata().getFinalizers().add(Controller.DEFAULT_FINALIZER);
+        testCustomResource.getMetadata().getFinalizers().add(FINALIZER_NAME);
 
         eventDispatcher.handleEvent(customResourceEvent(Watcher.Action.MODIFIED, testCustomResource));
 
@@ -84,7 +85,7 @@ void callsDeleteIfObjectHasFinalizerAndMarkedForDelete() {
      * Note that there could be more finalizers. Out of our control.
      */
     @Test
-    void callDeleteOnControllerIfMarkedForDeletionButThereIsNoDefaultFinalizer() {
+    void callDeleteOnControllerIfMarkedForDeletionButThereIsNoFinalizer() {
         markForDeletion(testCustomResource);
 
         eventDispatcher.handleEvent(customResourceEvent(Watcher.Action.MODIFIED, testCustomResource));
@@ -93,7 +94,7 @@ void callDeleteOnControllerIfMarkedForDeletionButThereIsNoDefaultFinalizer() {
     }
 
     @Test
-    void removesDefaultFinalizerOnDelete() {
+    void removesFinalizerOnDelete() {
         markForDeletion(testCustomResource);
 
         eventDispatcher.handleEvent(customResourceEvent(Watcher.Action.MODIFIED, testCustomResource));
@@ -172,9 +173,8 @@ void doesNotMarkNewGenerationInCaseOfException() {
                 .thenThrow(new IllegalStateException("Exception for testing purposes"))
                 .thenReturn(UpdateControl.noUpdate());
 
-        Assertions.assertThrows(IllegalStateException.class, () -> {
-            eventDispatcher.handleEvent(customResourceEvent(Watcher.Action.MODIFIED, testCustomResource));
-        });
+        Assertions.assertThrows(IllegalStateException.class, () ->
+                eventDispatcher.handleEvent(customResourceEvent(Watcher.Action.MODIFIED, testCustomResource)));
         eventDispatcher.handleEvent(customResourceEvent(Watcher.Action.MODIFIED, testCustomResource));
 
         verify(controller, times(2)).createOrUpdateResource(eq(testCustomResource), any());
@@ -183,7 +183,7 @@ void doesNotMarkNewGenerationInCaseOfException() {
 
     void generationAwareMode() {
         eventDispatcher = new EventDispatcher(controller,
-                Controller.DEFAULT_FINALIZER, customResourceFacade, true);
+                FINALIZER_NAME, customResourceFacade, true);
     }
 
     private void markForDeletion(CustomResource customResource) {
@@ -202,7 +202,7 @@ CustomResource getResource() {
                 .withDeletionGracePeriodSeconds(10L)
                 .withGeneration(10L)
                 .withName("name")
-                .withFinalizers(Controller.DEFAULT_FINALIZER)
+                .withFinalizers(FINALIZER_NAME)
                 .withNamespace("namespace")
                 .withResourceVersion("resourceVersion")
                 .withSelfLink("selfLink")