From 9a1551f99ed94fee11820eca3b85e5cdeace896f Mon Sep 17 00:00:00 2001 From: Chris Laprun Date: Thu, 28 Oct 2021 20:30:49 +0200 Subject: [PATCH 1/2] fix: replace jandex plugin by Quarkus detection of beans.xml files This allows to not run into compatibility between index versions. --- .../src/main/resources/META-INF/beans.xml | 0 pom.xml | 18 ------------------ 2 files changed, 18 deletions(-) create mode 100644 operator-framework-core/src/main/resources/META-INF/beans.xml diff --git a/operator-framework-core/src/main/resources/META-INF/beans.xml b/operator-framework-core/src/main/resources/META-INF/beans.xml new file mode 100644 index 0000000000..e69de29bb2 diff --git a/pom.xml b/pom.xml index 3efc3b0f38..67b8b4efe4 100644 --- a/pom.xml +++ b/pom.xml @@ -67,7 +67,6 @@ 2.8.2 2.5.2 5.0.0 - 1.2.1 2.16.0 1.0 1.6.2 @@ -223,11 +222,6 @@ maven-install-plugin ${maven-install-plugin.version} - - org.jboss.jandex - jandex-maven-plugin - ${jandex-maven-plugin.version} - net.revelc.code.formatter formatter-maven-plugin @@ -268,18 +262,6 @@ - - org.jboss.jandex - jandex-maven-plugin - - - make-index - - jandex - - - - org.apache.maven.plugins maven-surefire-plugin From 8ad359b2b7f5b9bd8fd3e90fd7214cbd39799f14 Mon Sep 17 00:00:00 2001 From: Chris Laprun Date: Thu, 28 Oct 2021 21:02:03 +0200 Subject: [PATCH 2/2] 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. --- .../io/javaoperatorsdk/operator/Operator.java | 9 +-- .../operator/ControllerManagerTest.java | 76 +++++++++++++++++++ .../sample/simple/DuplicateCRController.java | 16 ++++ .../TestCustomResourceControllerV2.java | 16 ++++ .../sample/simple/TestCustomResourceV2.java | 12 +++ 5 files changed, 124 insertions(+), 5 deletions(-) create mode 100644 operator-framework-core/src/test/java/io/javaoperatorsdk/operator/ControllerManagerTest.java create mode 100644 operator-framework-core/src/test/java/io/javaoperatorsdk/operator/sample/simple/DuplicateCRController.java create mode 100644 operator-framework-core/src/test/java/io/javaoperatorsdk/operator/sample/simple/TestCustomResourceControllerV2.java create mode 100644 operator-framework-core/src/test/java/io/javaoperatorsdk/operator/sample/simple/TestCustomResourceV2.java diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/Operator.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/Operator.java index ae094b25c6..32bc6edd75 100644 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/Operator.java +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/Operator.java @@ -163,11 +163,10 @@ public void close() { } } - private static class ControllerManager implements LifecycleAware { + static class ControllerManager implements LifecycleAware { private final Map controllers = new HashMap<>(); private boolean started = false; - public synchronized void shouldStart() { if (started) { return; @@ -200,9 +199,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) { diff --git a/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/ControllerManagerTest.java b/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/ControllerManagerTest.java new file mode 100644 index 0000000000..d25378499e --- /dev/null +++ b/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 , U extends CustomResource> void checkException( + TestControllerConfiguration registered, + TestControllerConfiguration 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> + extends DefaultControllerConfiguration { + private final ResourceController controller; + + public TestControllerConfiguration(ResourceController controller, Class crClass) { + super(null, getControllerName(controller), + CustomResource.getCRDName(crClass), null, false, null, null, null, null, crClass, null); + this.controller = controller; + } + + static > String getControllerName( + ResourceController controller) { + return controller.getClass().getSimpleName() + "Controller"; + } + + private String getControllerName() { + return getControllerName(controller); + } + } +} diff --git a/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/sample/simple/DuplicateCRController.java b/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/sample/simple/DuplicateCRController.java new file mode 100644 index 0000000000..443c4c21cc --- /dev/null +++ b/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 { + + @Override + public UpdateControl createOrUpdateResource(TestCustomResource resource, + Context context) { + return UpdateControl.noUpdate(); + } +} diff --git a/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/sample/simple/TestCustomResourceControllerV2.java b/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/sample/simple/TestCustomResourceControllerV2.java new file mode 100644 index 0000000000..29a89381e2 --- /dev/null +++ b/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 { + + @Override + public UpdateControl createOrUpdateResource(TestCustomResourceV2 resource, + Context context) { + return UpdateControl.noUpdate(); + } +} diff --git a/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/sample/simple/TestCustomResourceV2.java b/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/sample/simple/TestCustomResourceV2.java new file mode 100644 index 0000000000..e02e359bcc --- /dev/null +++ b/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 { + +}