fix: ReconcilerUtils.isFinalizerValid should account for NO_FINALIZER (operator-framework#819)

metacosm · web-flow · commit bd242ea6f37c · 2022-01-13T09:28:31.000+01:00
Also restores ReconcilerUtils.getDefaultReconcilerName(String) Fixes operator-framework#818
diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/ReconcilerUtils.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/ReconcilerUtils.java
@@ -37,7 +37,7 @@ public void setApiVersion(String s) {
         throw new UnsupportedOperationException();
       }
     };
-    return validator.isFinalizerValid(finalizer);
+    return Constants.NO_FINALIZER.equals(finalizer) || validator.isFinalizerValid(finalizer);
   }
 
   public static String getResourceTypeName(Class<? extends HasMetadata> resourceClass) {
@@ -49,7 +49,10 @@ public static String getResourceTypeName(Class<? extends HasMetadata> resourceCl
   }
 
   public static String getDefaultFinalizerName(Class<? extends HasMetadata> resourceClass) {
-    var resourceName = getResourceTypeName(resourceClass);
+    return getDefaultFinalizerName(getResourceTypeName(resourceClass));
+  }
+
+  public static String getDefaultFinalizerName(String resourceName) {
     // resource names for historic resources such as Pods are missing periods and therefore do not
     // constitute valid domain names as mandated by Kubernetes so generate one that does
     if (resourceName.indexOf('.') < 0) {
diff --git a/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/ReconcilerUtilsTest.java b/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/ReconcilerUtilsTest.java
@@ -3,6 +3,7 @@
 import org.junit.jupiter.api.Test;
 
 import io.fabric8.kubernetes.api.model.Pod;
+import io.javaoperatorsdk.operator.api.reconciler.Constants;
 import io.javaoperatorsdk.operator.sample.simple.TestCustomReconciler;
 import io.javaoperatorsdk.operator.sample.simple.TestCustomResource;
 
@@ -33,4 +34,9 @@ void defaultFinalizerShouldWork() {
     assertTrue(isFinalizerValid(getDefaultFinalizerName(Pod.class)));
     assertTrue(isFinalizerValid(getDefaultFinalizerName(TestCustomResource.class)));
   }
+
+  @Test
+  void noFinalizerMarkerShouldWork() {
+    assertTrue(isFinalizerValid(Constants.NO_FINALIZER));
+  }
 }
diff --git a/operator-framework/src/main/java/io/javaoperatorsdk/operator/config/runtime/AnnotationConfiguration.java b/operator-framework/src/main/java/io/javaoperatorsdk/operator/config/runtime/AnnotationConfiguration.java
@@ -6,7 +6,6 @@
 import io.fabric8.kubernetes.api.model.HasMetadata;
 import io.javaoperatorsdk.operator.ReconcilerUtils;
 import io.javaoperatorsdk.operator.api.config.ConfigurationService;
-import io.javaoperatorsdk.operator.api.reconciler.Constants;
 import io.javaoperatorsdk.operator.api.reconciler.ControllerConfiguration;
 import io.javaoperatorsdk.operator.api.reconciler.Reconciler;
 import io.javaoperatorsdk.operator.processing.event.source.controller.ResourceEventFilter;
@@ -35,7 +34,7 @@ public String getFinalizer() {
       return ReconcilerUtils.getDefaultFinalizerName(getResourceClass());
     } else {
       final var finalizer = annotation.finalizerName();
-      if (Constants.NO_FINALIZER.equals(finalizer) || ReconcilerUtils.isFinalizerValid(finalizer)) {
+      if (ReconcilerUtils.isFinalizerValid(finalizer)) {
         return finalizer;
       } else {
         throw new IllegalArgumentException(finalizer

Original file line number	Diff line number	Diff line change
`@@ -37,7 +37,7 @@ public void setApiVersion(String s) {`
`37`	`37`	`throw new UnsupportedOperationException();`
`38`	`38`	`}`
`39`	`39`	`};`
`40`		`- return validator.isFinalizerValid(finalizer);`
	`40`	`+ return Constants.NO_FINALIZER.equals(finalizer) \|\| validator.isFinalizerValid(finalizer);`
`41`	`41`	`}`
`42`	`42`
`43`	`43`	`public static String getResourceTypeName(Class<? extends HasMetadata> resourceClass) {`
`@@ -49,7 +49,10 @@ public static String getResourceTypeName(Class<? extends HasMetadata> resourceCl`
`49`	`49`	`}`
`50`	`50`
`51`	`51`	`public static String getDefaultFinalizerName(Class<? extends HasMetadata> resourceClass) {`
`52`		`- var resourceName = getResourceTypeName(resourceClass);`
	`52`	`+ return getDefaultFinalizerName(getResourceTypeName(resourceClass));`
	`53`	`+ }`
	`54`	`+`
	`55`	`+ public static String getDefaultFinalizerName(String resourceName) {`
`53`	`56`	`// resource names for historic resources such as Pods are missing periods and therefore do not`
`54`	`57`	`// constitute valid domain names as mandated by Kubernetes so generate one that does`
`55`	`58`	`if (resourceName.indexOf('.') < 0) {`