fix: improve duplicated controller detection, add tests

While we originally planned to make it possible to register controllers
with the same CR but with different version (see #637), that behavior
should actually be forbidden since only one CR version can be served,
see #94 for more details.

Author: metacosm

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

@@ -159,11 +159,10 @@ public <R extends CustomResource> void register(
     }
   }
 
-  private static class ControllerManager implements Closeable {
+  static class ControllerManager implements Closeable {
     private final Map<String, ConfiguredController> controllers = new HashMap<>();
     private boolean started = false;
 
-
     public synchronized void shouldStart() {
       if (started) {
         return;
@@ -201,9 +200,9 @@ public synchronized void add(ConfiguredController configuredController) {
       final var crdName = configuration.getCRDName();
       final var existing = controllers.get(crdName);
       if (existing != null) {
-        throw new OperatorException("Cannot register controller " + configuration.getName()
-            + ": another controller (" + existing.getConfiguration().getName()
-            + ") is already registered for CRD " + crdName);
+        throw new OperatorException("Cannot register controller '" + configuration.getName()
+            + "': another controller named '" + existing.getConfiguration().getName()
+            + "' is already registered for CRD '" + crdName + "'");
       }
       this.controllers.put(crdName, configuredController);
       if (started) {

operator-framework-core/src/test/java/io/javaoperatorsdk/operator/ControllerManagerTest.java

@@ -0,0 +1,76 @@
+package io.javaoperatorsdk.operator;
+
+import org.junit.Test;
+
+import io.fabric8.kubernetes.client.CustomResource;
+import io.javaoperatorsdk.operator.Operator.ControllerManager;
+import io.javaoperatorsdk.operator.api.ResourceController;
+import io.javaoperatorsdk.operator.api.config.DefaultControllerConfiguration;
+import io.javaoperatorsdk.operator.processing.ConfiguredController;
+import io.javaoperatorsdk.operator.sample.simple.DuplicateCRController;
+import io.javaoperatorsdk.operator.sample.simple.TestCustomResource;
+import io.javaoperatorsdk.operator.sample.simple.TestCustomResourceController;
+import io.javaoperatorsdk.operator.sample.simple.TestCustomResourceControllerV2;
+import io.javaoperatorsdk.operator.sample.simple.TestCustomResourceV2;
+
+import static org.junit.jupiter.api.Assertions.assertThrows;
+import static org.junit.jupiter.api.Assertions.assertTrue;
+
+public class ControllerManagerTest {
+
+  @Test
+  public void shouldNotAddMultipleControllersForSameCustomResource() {
+    final var registered = new TestControllerConfiguration<>(new TestCustomResourceController(null),
+        TestCustomResource.class);
+    final var duplicated =
+        new TestControllerConfiguration<>(new DuplicateCRController(), TestCustomResource.class);
+
+    checkException(registered, duplicated);
+  }
+
+  @Test
+  public void addingMultipleControllersForCustomResourcesWithDifferentVersionsShouldNotWork() {
+    final var registered = new TestControllerConfiguration<>(new TestCustomResourceController(null),
+        TestCustomResource.class);
+    final var duplicated = new TestControllerConfiguration<>(new TestCustomResourceControllerV2(),
+        TestCustomResourceV2.class);
+
+    checkException(registered, duplicated);
+
+  }
+
+  private <T extends CustomResource<?, ?>, U extends CustomResource<?, ?>> void checkException(
+      TestControllerConfiguration<T> registered,
+      TestControllerConfiguration<U> duplicated) {
+    final var exception = assertThrows(OperatorException.class, () -> {
+      final var controllerManager = new ControllerManager();
+      controllerManager.add(new ConfiguredController<>(registered.controller, registered, null));
+      controllerManager.add(new ConfiguredController<>(duplicated.controller, duplicated, null));
+    });
+    final var msg = exception.getMessage();
+    assertTrue(
+        msg.contains("Cannot register controller '" + duplicated.getControllerName() + "'")
+            && msg.contains(registered.getControllerName())
+            && msg.contains(registered.getCRDName()));
+  }
+
+  private static class TestControllerConfiguration<R extends CustomResource<?, ?>>
+      extends DefaultControllerConfiguration<R> {
+    private final ResourceController<R> controller;
+
+    public TestControllerConfiguration(ResourceController<R> controller, Class<R> crClass) {
+      super(null, getControllerName(controller),
+          CustomResource.getCRDName(crClass), null, false, null, null, null, null, crClass, null);
+      this.controller = controller;
+    }
+
+    static <R extends CustomResource<?, ?>> String getControllerName(
+        ResourceController<R> controller) {
+      return controller.getClass().getSimpleName() + "Controller";
+    }
+
+    private String getControllerName() {
+      return getControllerName(controller);
+    }
+  }
+}

operator-framework-core/src/test/java/io/javaoperatorsdk/operator/sample/simple/DuplicateCRController.java

@@ -0,0 +1,16 @@
+package io.javaoperatorsdk.operator.sample.simple;
+
+import io.javaoperatorsdk.operator.api.Context;
+import io.javaoperatorsdk.operator.api.Controller;
+import io.javaoperatorsdk.operator.api.ResourceController;
+import io.javaoperatorsdk.operator.api.UpdateControl;
+
+@Controller
+public class DuplicateCRController implements ResourceController<TestCustomResource> {
+
+  @Override
+  public UpdateControl<TestCustomResource> createOrUpdateResource(TestCustomResource resource,
+      Context<TestCustomResource> context) {
+    return UpdateControl.noUpdate();
+  }
+}

operator-framework-core/src/test/java/io/javaoperatorsdk/operator/sample/simple/TestCustomResourceControllerV2.java

@@ -0,0 +1,16 @@
+package io.javaoperatorsdk.operator.sample.simple;
+
+import io.javaoperatorsdk.operator.api.Context;
+import io.javaoperatorsdk.operator.api.Controller;
+import io.javaoperatorsdk.operator.api.ResourceController;
+import io.javaoperatorsdk.operator.api.UpdateControl;
+
+@Controller
+public class TestCustomResourceControllerV2 implements ResourceController<TestCustomResourceV2> {
+
+  @Override
+  public UpdateControl<TestCustomResourceV2> createOrUpdateResource(TestCustomResourceV2 resource,
+      Context<TestCustomResourceV2> context) {
+    return UpdateControl.noUpdate();
+  }
+}

operator-framework-core/src/test/java/io/javaoperatorsdk/operator/sample/simple/TestCustomResourceV2.java

@@ -0,0 +1,12 @@
+package io.javaoperatorsdk.operator.sample.simple;
+
+import io.fabric8.kubernetes.client.CustomResource;
+import io.fabric8.kubernetes.model.annotation.Group;
+import io.fabric8.kubernetes.model.annotation.Version;
+
+@Group("sample.javaoperatorsdk.io")
+@Version("v2")
+public class TestCustomResourceV2
+    extends CustomResource<TestCustomResourceSpec, TestCustomResourceStatus> {
+
+}