diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/AnnotationControllerConfiguration.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/AnnotationControllerConfiguration.java index 19535c6d3e..ddd69dff53 100644 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/AnnotationControllerConfiguration.java +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/AnnotationControllerConfiguration.java @@ -66,7 +66,8 @@ public boolean isGenerationAware() { @Override public Set getNamespaces() { - return Set.of(valueOrDefault(annotation, ControllerConfiguration::namespaces, new String[] {})); + return Set.of(valueOrDefault(annotation, ControllerConfiguration::namespaces, + new String[] {Constants.WATCH_ALL_NAMESPACES})); } @Override diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/ControllerConfigurationOverrider.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/ControllerConfigurationOverrider.java index 57b8640a8b..f045a4a2e3 100644 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/ControllerConfigurationOverrider.java +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/ControllerConfigurationOverrider.java @@ -19,7 +19,7 @@ public class ControllerConfigurationOverrider { private String finalizer; private boolean generationAware; - private final Set namespaces; + private Set namespaces; private RetryConfiguration retry; private String labelSelector; private ResourceEventFilter customResourcePredicate; @@ -52,8 +52,8 @@ public ControllerConfigurationOverrider withGenerationAware(boolean generatio return this; } - public ControllerConfigurationOverrider withCurrentNamespace() { - namespaces.clear(); + public ControllerConfigurationOverrider watchingOnlyCurrentNamespace() { + this.namespaces = ResourceConfiguration.CURRENT_NAMESPACE_ONLY; return this; } @@ -64,6 +64,9 @@ public ControllerConfigurationOverrider addingNamespaces(String... namespaces public ControllerConfigurationOverrider removingNamespaces(String... namespaces) { List.of(namespaces).forEach(this.namespaces::remove); + if (this.namespaces.isEmpty()) { + this.namespaces = ResourceConfiguration.DEFAULT_NAMESPACES; + } return this; } @@ -73,6 +76,11 @@ public ControllerConfigurationOverrider settingNamespace(String namespace) { return this; } + public ControllerConfigurationOverrider watchingAllNamespaces() { + this.namespaces = ResourceConfiguration.DEFAULT_NAMESPACES; + return this; + } + public ControllerConfigurationOverrider withRetry(RetryConfiguration retry) { this.retry = retry; return this; diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/DefaultResourceConfiguration.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/DefaultResourceConfiguration.java index 4959b827ad..7ee116ac60 100644 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/DefaultResourceConfiguration.java +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/DefaultResourceConfiguration.java @@ -1,6 +1,5 @@ package io.javaoperatorsdk.operator.api.config; -import java.util.Collections; import java.util.Set; import io.fabric8.kubernetes.api.model.HasMetadata; @@ -10,21 +9,22 @@ public class DefaultResourceConfiguration private final String labelSelector; private final Set namespaces; - private final boolean watchAllNamespaces; private final Class resourceClass; public DefaultResourceConfiguration(String labelSelector, Class resourceClass, String... namespaces) { this(labelSelector, resourceClass, - namespaces != null ? Set.of(namespaces) : Collections.emptySet()); + namespaces == null || namespaces.length == 0 ? ResourceConfiguration.DEFAULT_NAMESPACES + : Set.of(namespaces)); } public DefaultResourceConfiguration(String labelSelector, Class resourceClass, Set namespaces) { this.labelSelector = labelSelector; this.resourceClass = resourceClass; - this.namespaces = namespaces != null ? namespaces : Collections.emptySet(); - this.watchAllNamespaces = this.namespaces.isEmpty(); + this.namespaces = + namespaces == null || namespaces.isEmpty() ? ResourceConfiguration.DEFAULT_NAMESPACES + : namespaces; } @Override @@ -42,11 +42,6 @@ public Set getNamespaces() { return namespaces; } - @Override - public boolean watchAllNamespaces() { - return watchAllNamespaces; - } - @Override public Class getResourceClass() { return resourceClass; diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/ResourceConfiguration.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/ResourceConfiguration.java index 2d947050e7..fe85476735 100644 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/ResourceConfiguration.java +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/ResourceConfiguration.java @@ -10,15 +10,18 @@ public interface ResourceConfiguration { + Set DEFAULT_NAMESPACES = Set.of(Constants.WATCH_ALL_NAMESPACES); + Set CURRENT_NAMESPACE_ONLY = Set.of(Constants.WATCH_CURRENT_NAMESPACE); + default String getResourceTypeName() { return ReconcilerUtils.getResourceTypeName(getResourceClass()); } /** * Retrieves the label selector that is used to filter which resources are actually watched by the - * associated event source. See - * https://kubernetes.io/docs/concepts/overview/working-with-objects/labels/ for more details on - * syntax. + * associated event source. See the official documentation on the + * topic + * for more details on syntax. * * @return the label selector filtering watched resources */ @@ -32,7 +35,7 @@ default Class getResourceClass() { } default Set getNamespaces() { - return Collections.emptySet(); + return DEFAULT_NAMESPACES; } default boolean watchAllNamespaces() { @@ -40,7 +43,8 @@ default boolean watchAllNamespaces() { } static boolean allNamespacesWatched(Set namespaces) { - return namespaces == null || namespaces.isEmpty(); + failIfNotValid(namespaces); + return DEFAULT_NAMESPACES.equals(namespaces); } default boolean watchCurrentNamespace() { @@ -48,9 +52,24 @@ default boolean watchCurrentNamespace() { } static boolean currentNamespaceWatched(Set namespaces) { - return namespaces != null - && namespaces.size() == 1 - && namespaces.contains(Constants.WATCH_CURRENT_NAMESPACE); + failIfNotValid(namespaces); + return CURRENT_NAMESPACE_ONLY.equals(namespaces); + } + + static void failIfNotValid(Set namespaces) { + if (namespaces != null && !namespaces.isEmpty()) { + final var present = namespaces.contains(Constants.WATCH_CURRENT_NAMESPACE) + || namespaces.contains(Constants.WATCH_ALL_NAMESPACES); + if (!present || namespaces.size() == 1) { + return; + } + } + + throw new IllegalArgumentException( + "Must specify namespaces. To watch all namespaces, use only '" + + Constants.WATCH_ALL_NAMESPACES + + "'. To watch only the namespace in which the operator is deployed, use only '" + + Constants.WATCH_CURRENT_NAMESPACE + "'"); } /** diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/reconciler/Constants.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/reconciler/Constants.java index 285bcb16cb..7d7d44ec32 100644 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/reconciler/Constants.java +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/reconciler/Constants.java @@ -4,6 +4,7 @@ public final class Constants { public static final String NO_VALUE_SET = ""; public static final String WATCH_CURRENT_NAMESPACE = "JOSDK_WATCH_CURRENT"; + public static final String WATCH_ALL_NAMESPACES = "JOSDK_ALL_NAMESPACES"; public static final long NO_RECONCILIATION_MAX_INTERVAL = -1L; private Constants() {} diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/reconciler/ControllerConfiguration.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/reconciler/ControllerConfiguration.java index 3bd5b93a61..a5e1459cb7 100644 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/reconciler/ControllerConfiguration.java +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/reconciler/ControllerConfiguration.java @@ -39,7 +39,7 @@ * * @return the list of namespaces this controller monitors */ - String[] namespaces() default {}; + String[] namespaces() default Constants.WATCH_ALL_NAMESPACES; /** * Optional label selector used to identify the set of custom resources the controller will acc diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/kubernetes/KubernetesDependent.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/kubernetes/KubernetesDependent.java index fbebe34648..8ce04d752e 100644 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/kubernetes/KubernetesDependent.java +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/kubernetes/KubernetesDependent.java @@ -11,7 +11,6 @@ @Target({ElementType.TYPE}) public @interface KubernetesDependent { String SAME_AS_PARENT = "JOSDK_SAME_AS_PARENT"; - String WATCH_ALL_NAMESPACES = "JOSDK_ALL_NAMESPACES"; String[] DEFAULT_NAMESPACES = {SAME_AS_PARENT}; diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/kubernetes/KubernetesDependentResourceConfig.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/kubernetes/KubernetesDependentResourceConfig.java index 48a93cb102..5f325bbfcc 100644 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/kubernetes/KubernetesDependentResourceConfig.java +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/kubernetes/KubernetesDependentResourceConfig.java @@ -37,11 +37,7 @@ public KubernetesDependentResourceConfig setLabelSelector(String labelSelector) } public Set namespaces() { - if (!namespaces.contains(KubernetesDependent.WATCH_ALL_NAMESPACES)) { - return namespaces; - } else { - return Collections.emptySet(); - } + return namespaces; } public String labelSelector() { diff --git a/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/OperatorTest.java b/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/OperatorTest.java index f4de6b2141..2a1b48f019 100644 --- a/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/OperatorTest.java +++ b/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/OperatorTest.java @@ -10,20 +10,17 @@ import io.fabric8.kubernetes.client.KubernetesClient; import io.javaoperatorsdk.operator.api.config.AbstractConfigurationService; import io.javaoperatorsdk.operator.api.config.ConfigurationServiceProvider; -import io.javaoperatorsdk.operator.api.config.ControllerConfiguration; +import io.javaoperatorsdk.operator.api.config.MockControllerConfiguration; import io.javaoperatorsdk.operator.api.reconciler.Context; import io.javaoperatorsdk.operator.api.reconciler.Reconciler; import io.javaoperatorsdk.operator.api.reconciler.UpdateControl; import static org.assertj.core.api.AssertionsForClassTypes.assertThat; -import static org.mockito.Mockito.mock; -import static org.mockito.Mockito.when; @SuppressWarnings("rawtypes") class OperatorTest { private final KubernetesClient kubernetesClient = MockKubernetesClient.client(ConfigMap.class); - private final ControllerConfiguration configuration = mock(ControllerConfiguration.class); private final Operator operator = new Operator(kubernetesClient); private final FooReconciler fooReconciler = new FooReconciler(); @@ -35,10 +32,9 @@ static void setUpConfigurationServiceProvider() { @Test @DisplayName("should register `Reconciler` to Controller") - @SuppressWarnings("unchecked") public void shouldRegisterReconcilerToController() { // given - when(configuration.getResourceClass()).thenReturn(ConfigMap.class); + final var configuration = MockControllerConfiguration.forResource(ConfigMap.class); // when operator.register(fooReconciler, configuration); diff --git a/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/api/config/ControllerConfigurationOverriderTest.java b/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/api/config/ControllerConfigurationOverriderTest.java index d0bb8916cb..497fafb897 100644 --- a/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/api/config/ControllerConfigurationOverriderTest.java +++ b/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/api/config/ControllerConfigurationOverriderTest.java @@ -6,6 +6,7 @@ import org.junit.jupiter.api.Test; import io.fabric8.kubernetes.api.model.ConfigMap; +import io.javaoperatorsdk.operator.api.reconciler.Constants; import io.javaoperatorsdk.operator.api.reconciler.Context; import io.javaoperatorsdk.operator.api.reconciler.ControllerConfiguration; import io.javaoperatorsdk.operator.api.reconciler.Reconciler; @@ -107,6 +108,63 @@ private io.javaoperatorsdk.operator.api.config.ControllerConfiguration create return new AnnotationControllerConfiguration<>(reconciler); } + @ControllerConfiguration(namespaces = "foo") + private static class WatchCurrentReconciler implements Reconciler { + + @Override + public UpdateControl reconcile(ConfigMap resource, Context context) + throws Exception { + return null; + } + } + + @Test + void overridingNamespacesShouldWork() { + var configuration = createConfiguration(new WatchCurrentReconciler()); + assertEquals(Set.of("foo"), configuration.getNamespaces()); + assertFalse(configuration.watchAllNamespaces()); + assertFalse(configuration.watchCurrentNamespace()); + + configuration = ControllerConfigurationOverrider.override(configuration) + .addingNamespaces("foo", "bar") + .build(); + assertEquals(Set.of("foo", "bar"), configuration.getNamespaces()); + assertFalse(configuration.watchAllNamespaces()); + assertFalse(configuration.watchCurrentNamespace()); + + configuration = ControllerConfigurationOverrider.override(configuration) + .removingNamespaces("bar") + .build(); + assertEquals(Set.of("foo"), configuration.getNamespaces()); + assertFalse(configuration.watchAllNamespaces()); + assertFalse(configuration.watchCurrentNamespace()); + + configuration = ControllerConfigurationOverrider.override(configuration) + .removingNamespaces("foo") + .build(); + assertTrue(configuration.watchAllNamespaces()); + assertFalse(configuration.watchCurrentNamespace()); + + configuration = ControllerConfigurationOverrider.override(configuration) + .settingNamespace("foo") + .build(); + assertFalse(configuration.watchAllNamespaces()); + assertFalse(configuration.watchCurrentNamespace()); + assertEquals(Set.of("foo"), configuration.getNamespaces()); + + configuration = ControllerConfigurationOverrider.override(configuration) + .watchingOnlyCurrentNamespace() + .build(); + assertFalse(configuration.watchAllNamespaces()); + assertTrue(configuration.watchCurrentNamespace()); + + configuration = ControllerConfigurationOverrider.override(configuration) + .watchingAllNamespaces() + .build(); + assertTrue(configuration.watchAllNamespaces()); + assertFalse(configuration.watchCurrentNamespace()); + } + @Test void configuredDependentShouldNotChangeOnParentOverrideEvenWhenInitialConfigIsSame() { var configuration = createConfiguration(new OverriddenNSOnDepReconciler()); @@ -140,7 +198,7 @@ void dependentShouldWatchAllNamespacesIfParentDoesAsWell() { var config = extractFirstDependentKubernetesResourceConfig(configuration); // check that the DependentResource inherits the controller's configuration if applicable - assertEquals(0, config.namespaces().size()); + assertTrue(ResourceConfiguration.allNamespacesWatched(config.namespaces())); // override the NS final var newNS = "bar"; @@ -160,7 +218,7 @@ void shouldBePossibleToForceDependentToWatchAllNamespaces() { var config = extractFirstDependentKubernetesResourceConfig(configuration); // check that the DependentResource inherits the controller's configuration if applicable - assertEquals(0, config.namespaces().size()); + assertTrue(ResourceConfiguration.allNamespacesWatched(config.namespaces())); // override the NS final var newNS = "bar"; @@ -169,7 +227,7 @@ void shouldBePossibleToForceDependentToWatchAllNamespaces() { // check that dependent config is still configured to watch all NS config = extractFirstDependentKubernetesResourceConfig(configuration); - assertEquals(0, config.namespaces().size()); + assertTrue(ResourceConfiguration.allNamespacesWatched(config.namespaces())); } @Test @@ -224,7 +282,8 @@ void replaceNamedDependentResourceConfigShouldWork() { final var dependentResourceName = DependentResource.defaultNameFor(ReadOnlyDependent.class); assertTrue(dependents.stream().anyMatch(dr -> dr.getName().equals(dependentResourceName))); - var dependentSpec = dependents.stream().filter(dr -> dr.getName().equals(dependentResourceName)) + var dependentSpec = dependents.stream() + .filter(dr -> dr.getName().equals(dependentResourceName)) .findFirst().get(); assertEquals(ReadOnlyDependent.class, dependentSpec.getDependentResourceClass()); var maybeConfig = dependentSpec.getDependentResourceConfiguration(); @@ -292,7 +351,7 @@ public ReadOnlyDependent() { } } - @KubernetesDependent(namespaces = KubernetesDependent.WATCH_ALL_NAMESPACES) + @KubernetesDependent(namespaces = Constants.WATCH_ALL_NAMESPACES) private static class WatchAllNSDependent extends KubernetesDependentResource { diff --git a/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/api/config/MockControllerConfiguration.java b/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/api/config/MockControllerConfiguration.java new file mode 100644 index 0000000000..a58e3b5b73 --- /dev/null +++ b/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/api/config/MockControllerConfiguration.java @@ -0,0 +1,18 @@ +package io.javaoperatorsdk.operator.api.config; + +import io.fabric8.kubernetes.api.model.HasMetadata; + +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.when; + +public class MockControllerConfiguration { + @SuppressWarnings({"unchecked", "rawtypes"}) + public static ControllerConfiguration forResource( + Class resourceType) { + final ControllerConfiguration configuration = mock(ControllerConfiguration.class); + when(configuration.getResourceClass()).thenReturn(resourceType); + when(configuration.getNamespaces()).thenReturn(ResourceConfiguration.DEFAULT_NAMESPACES); + when(configuration.getEffectiveNamespaces()).thenCallRealMethod(); + return configuration; + } +} diff --git a/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/api/config/ResourceConfigurationTest.java b/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/api/config/ResourceConfigurationTest.java new file mode 100644 index 0000000000..013eec9cc0 --- /dev/null +++ b/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/api/config/ResourceConfigurationTest.java @@ -0,0 +1,71 @@ +package io.javaoperatorsdk.operator.api.config; + +import java.util.Collections; +import java.util.Set; + +import org.junit.jupiter.api.Test; + +import io.javaoperatorsdk.operator.api.reconciler.Constants; + +import static org.junit.jupiter.api.Assertions.*; + +class ResourceConfigurationTest { + + @Test + void allNamespacesWatched() { + assertThrows(IllegalArgumentException.class, + () -> ResourceConfiguration.allNamespacesWatched(null)); + assertThrows(IllegalArgumentException.class, () -> ResourceConfiguration.allNamespacesWatched( + Set.of(Constants.WATCH_CURRENT_NAMESPACE, Constants.WATCH_ALL_NAMESPACES, "foo"))); + assertThrows(IllegalArgumentException.class, () -> ResourceConfiguration.allNamespacesWatched( + Collections.emptySet())); + assertFalse(ResourceConfiguration.allNamespacesWatched(Set.of("foo", "bar"))); + assertTrue(ResourceConfiguration.allNamespacesWatched(Set.of(Constants.WATCH_ALL_NAMESPACES))); + assertFalse(ResourceConfiguration.allNamespacesWatched(Set.of("foo"))); + assertFalse( + ResourceConfiguration.allNamespacesWatched(Set.of(Constants.WATCH_CURRENT_NAMESPACE))); + } + + @Test + void currentNamespaceWatched() { + assertThrows(IllegalArgumentException.class, + () -> ResourceConfiguration.currentNamespaceWatched(null)); + assertThrows(IllegalArgumentException.class, + () -> ResourceConfiguration.currentNamespaceWatched( + Set.of(Constants.WATCH_CURRENT_NAMESPACE, Constants.WATCH_ALL_NAMESPACES, "foo"))); + assertThrows(IllegalArgumentException.class, + () -> ResourceConfiguration.currentNamespaceWatched(Collections.emptySet())); + assertFalse(ResourceConfiguration.currentNamespaceWatched(Set.of("foo", "bar"))); + assertFalse( + ResourceConfiguration.currentNamespaceWatched(Set.of(Constants.WATCH_ALL_NAMESPACES))); + assertFalse(ResourceConfiguration.currentNamespaceWatched(Set.of("foo"))); + assertTrue( + ResourceConfiguration.currentNamespaceWatched(Set.of(Constants.WATCH_CURRENT_NAMESPACE))); + } + + @Test + void nullLabelSelectorByDefault() { + assertNull(new ResourceConfiguration<>() {}.getLabelSelector()); + } + + @Test + void shouldWatchAllNamespacesByDefault() { + assertTrue(new ResourceConfiguration<>() {}.watchAllNamespaces()); + } + + @Test + void failIfNotValid() { + assertThrows(IllegalArgumentException.class, () -> ResourceConfiguration.failIfNotValid(null)); + assertThrows(IllegalArgumentException.class, + () -> ResourceConfiguration.failIfNotValid(Collections.emptySet())); + assertThrows(IllegalArgumentException.class, () -> ResourceConfiguration.failIfNotValid( + Set.of(Constants.WATCH_CURRENT_NAMESPACE, Constants.WATCH_ALL_NAMESPACES, "foo"))); + assertThrows(IllegalArgumentException.class, () -> ResourceConfiguration.failIfNotValid( + Set.of(Constants.WATCH_CURRENT_NAMESPACE, "foo"))); + assertThrows(IllegalArgumentException.class, () -> ResourceConfiguration.failIfNotValid( + Set.of(Constants.WATCH_ALL_NAMESPACES, "foo"))); + + // should work + ResourceConfiguration.failIfNotValid(Set.of("foo", "bar")); + } +} diff --git a/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/ControllerTest.java b/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/ControllerTest.java index 26fd763467..67e9112b27 100644 --- a/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/ControllerTest.java +++ b/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/ControllerTest.java @@ -5,7 +5,7 @@ import io.fabric8.kubernetes.api.model.Secret; import io.javaoperatorsdk.operator.MockKubernetesClient; import io.javaoperatorsdk.operator.api.config.ConfigurationServiceProvider; -import io.javaoperatorsdk.operator.api.config.ControllerConfiguration; +import io.javaoperatorsdk.operator.api.config.MockControllerConfiguration; import io.javaoperatorsdk.operator.api.reconciler.Cleaner; import io.javaoperatorsdk.operator.api.reconciler.Reconciler; import io.javaoperatorsdk.operator.sample.simple.TestCustomResource; @@ -13,17 +13,15 @@ import static org.assertj.core.api.Assertions.assertThat; import static org.mockito.Mockito.*; -@SuppressWarnings("unchecked") +@SuppressWarnings({"unchecked", "rawtypes"}) class ControllerTest { - final ControllerConfiguration configuration = mock(ControllerConfiguration.class); final Reconciler reconciler = mock(Reconciler.class); @Test void crdShouldNotBeCheckedForNativeResources() { final var client = MockKubernetesClient.client(Secret.class); - - when(configuration.getResourceClass()).thenReturn(Secret.class); + final var configuration = MockControllerConfiguration.forResource(Secret.class); final var controller = new Controller(reconciler, configuration, client); controller.start(); @@ -33,7 +31,7 @@ void crdShouldNotBeCheckedForNativeResources() { @Test void crdShouldNotBeCheckedForCustomResourcesIfDisabled() { final var client = MockKubernetesClient.client(TestCustomResource.class); - when(configuration.getResourceClass()).thenReturn(TestCustomResource.class); + final var configuration = MockControllerConfiguration.forResource(TestCustomResource.class); try { ConfigurationServiceProvider.overrideCurrent(o -> o.checkingCRDAndValidateLocalModel(false)); @@ -49,10 +47,10 @@ void crdShouldNotBeCheckedForCustomResourcesIfDisabled() { @Test void usesFinalizerIfThereIfReconcilerImplementsCleaner() { Reconciler reconciler = mock(Reconciler.class, withSettings().extraInterfaces(Cleaner.class)); - when(configuration.getResourceClass()).thenReturn(TestCustomResource.class); + final var configuration = MockControllerConfiguration.forResource(Secret.class); - final var controller = new Controller(reconciler, - configuration, MockKubernetesClient.client(TestCustomResource.class)); + final var controller = new Controller(reconciler, configuration, + MockKubernetesClient.client(Secret.class)); assertThat(controller.useFinalizer()).isTrue(); } diff --git a/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/event/EventSourceManagerTest.java b/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/event/EventSourceManagerTest.java index 42a9a75447..f09605d095 100644 --- a/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/event/EventSourceManagerTest.java +++ b/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/event/EventSourceManagerTest.java @@ -7,7 +7,7 @@ import io.fabric8.kubernetes.api.model.HasMetadata; import io.javaoperatorsdk.operator.MockKubernetesClient; import io.javaoperatorsdk.operator.OperatorException; -import io.javaoperatorsdk.operator.api.config.ControllerConfiguration; +import io.javaoperatorsdk.operator.api.config.MockControllerConfiguration; import io.javaoperatorsdk.operator.api.reconciler.Reconciler; import io.javaoperatorsdk.operator.processing.Controller; import io.javaoperatorsdk.operator.processing.event.source.CachingEventSource; @@ -140,8 +140,7 @@ void retrievingAnEventSourceWhenMultipleAreRegisteredForATypeShouldRequireAQuali } private EventSourceManager initManager() { - final ControllerConfiguration configuration = mock(ControllerConfiguration.class); - when(configuration.getResourceClass()).thenReturn(HasMetadata.class); + final var configuration = MockControllerConfiguration.forResource(HasMetadata.class); final Controller controller = new Controller(mock(Reconciler.class), configuration, MockKubernetesClient.client(HasMetadata.class)); return new EventSourceManager(controller); diff --git a/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/event/ReconciliationDispatcherTest.java b/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/event/ReconciliationDispatcherTest.java index d1b6883075..6afec04543 100644 --- a/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/event/ReconciliationDispatcherTest.java +++ b/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/event/ReconciliationDispatcherTest.java @@ -21,6 +21,7 @@ import io.javaoperatorsdk.operator.api.config.Cloner; import io.javaoperatorsdk.operator.api.config.ConfigurationServiceProvider; import io.javaoperatorsdk.operator.api.config.ControllerConfiguration; +import io.javaoperatorsdk.operator.api.config.MockControllerConfiguration; import io.javaoperatorsdk.operator.api.config.RetryConfiguration; import io.javaoperatorsdk.operator.api.reconciler.*; import io.javaoperatorsdk.operator.processing.Controller; @@ -64,15 +65,13 @@ static void classSetup() { * equals will fail on the two equal but NOT identical TestCustomResources because equals is not * implemented on TestCustomResourceSpec or TestCustomResourceStatus */ - ConfigurationServiceProvider.overrideCurrent(overrider -> { - overrider.checkingCRDAndValidateLocalModel(false) - .withResourceCloner(new Cloner() { - @Override - public R clone(R object) { - return object; - } - }); - }); + ConfigurationServiceProvider.overrideCurrent(overrider -> overrider + .checkingCRDAndValidateLocalModel(false).withResourceCloner(new Cloner() { + @Override + public R clone(R object) { + return object; + } + })); } @AfterAll @@ -92,17 +91,19 @@ private ReconciliationDispatcher init(R customResourc Reconciler reconciler, ControllerConfiguration configuration, CustomResourceFacade customResourceFacade, boolean useFinalizer) { - configuration = configuration == null ? mock(ControllerConfiguration.class) : configuration; + final Class resourceClass = (Class) customResource.getClass(); + configuration = configuration == null ? MockControllerConfiguration.forResource(resourceClass) + : configuration; when(configuration.getFinalizerName()).thenReturn(DEFAULT_FINALIZER); when(configuration.getName()).thenReturn("EventDispatcherTestController"); - when(configuration.getResourceClass()).thenReturn((Class) customResource.getClass()); + when(configuration.getResourceClass()).thenReturn(resourceClass); when(configuration.getRetryConfiguration()).thenReturn(RetryConfiguration.DEFAULT); when(configuration.reconciliationMaxInterval()) .thenReturn(Optional.of(Duration.ofHours(RECONCILIATION_MAX_INTERVAL))); Controller controller = new Controller<>(reconciler, configuration, - MockKubernetesClient.client(customResource.getClass())) { + MockKubernetesClient.client(resourceClass)) { @Override public boolean useFinalizer() { return useFinalizer; @@ -114,7 +115,7 @@ public boolean useFinalizer() { } @Test - void addFinalizerOnNewResource() throws Exception { + void addFinalizerOnNewResource() { assertFalse(testCustomResource.hasFinalizer(DEFAULT_FINALIZER)); reconciliationDispatcher.handleExecution(executionScopeWithCREvent(testCustomResource)); verify(reconciler, never()) @@ -126,7 +127,7 @@ void addFinalizerOnNewResource() throws Exception { } @Test - void callCreateOrUpdateOnNewResourceIfFinalizerSet() throws Exception { + void callCreateOrUpdateOnNewResourceIfFinalizerSet() { testCustomResource.addFinalizer(DEFAULT_FINALIZER); reconciliationDispatcher.handleExecution(executionScopeWithCREvent(testCustomResource)); verify(reconciler, times(1)) @@ -134,7 +135,7 @@ void callCreateOrUpdateOnNewResourceIfFinalizerSet() throws Exception { } @Test - void updatesOnlyStatusSubResourceIfFinalizerSet() throws Exception { + void updatesOnlyStatusSubResourceIfFinalizerSet() { testCustomResource.addFinalizer(DEFAULT_FINALIZER); reconciler.reconcile = (r, c) -> UpdateControl.updateStatus(testCustomResource); @@ -146,7 +147,7 @@ void updatesOnlyStatusSubResourceIfFinalizerSet() throws Exception { } @Test - void updatesBothResourceAndStatusIfFinalizerSet() throws Exception { + void updatesBothResourceAndStatusIfFinalizerSet() { testCustomResource.addFinalizer(DEFAULT_FINALIZER); reconciler.reconcile = (r, c) -> UpdateControl.updateResourceAndStatus(testCustomResource); @@ -187,7 +188,7 @@ void patchesStatus() { } @Test - void callCreateOrUpdateOnModifiedResourceIfFinalizerSet() throws Exception { + void callCreateOrUpdateOnModifiedResourceIfFinalizerSet() { testCustomResource.addFinalizer(DEFAULT_FINALIZER); reconciliationDispatcher.handleExecution(executionScopeWithCREvent(testCustomResource)); @@ -261,7 +262,7 @@ void doesNotRemovesTheSetFinalizerIfTheDeleteNotMethodInstructsIt() { } @Test - void doesNotUpdateTheResourceIfNoUpdateUpdateControlIfFinalizerSet() throws Exception { + void doesNotUpdateTheResourceIfNoUpdateUpdateControlIfFinalizerSet() { testCustomResource.addFinalizer(DEFAULT_FINALIZER); reconciler.reconcile = (r, c) -> UpdateControl.noUpdate(); @@ -272,7 +273,7 @@ void doesNotUpdateTheResourceIfNoUpdateUpdateControlIfFinalizerSet() throws Exce } @Test - void addsFinalizerIfNotMarkedForDeletionAndEmptyCustomResourceReturned() throws Exception { + void addsFinalizerIfNotMarkedForDeletionAndEmptyCustomResourceReturned() { removeFinalizers(testCustomResource); reconciler.reconcile = (r, c) -> UpdateControl.noUpdate(); @@ -295,8 +296,7 @@ void doesNotCallDeleteIfMarkedForDeletionButNotOurFinalizer() { } @Test - void executeControllerRegardlessGenerationInNonGenerationAwareModeIfFinalizerSet() - throws Exception { + void executeControllerRegardlessGenerationInNonGenerationAwareModeIfFinalizerSet() { testCustomResource.addFinalizer(DEFAULT_FINALIZER); reconciliationDispatcher.handleExecution(executionScopeWithCREvent(testCustomResource)); reconciliationDispatcher.handleExecution(executionScopeWithCREvent(testCustomResource)); @@ -305,7 +305,7 @@ void executeControllerRegardlessGenerationInNonGenerationAwareModeIfFinalizerSet } @Test - void propagatesRetryInfoToContextIfFinalizerSet() throws Exception { + void propagatesRetryInfoToContextIfFinalizerSet() { testCustomResource.addFinalizer(DEFAULT_FINALIZER); reconciliationDispatcher.handleExecution( @@ -334,7 +334,7 @@ public boolean isLastAttempt() { } @Test - void setReScheduleToPostExecutionControlFromUpdateControl() throws Exception { + void setReScheduleToPostExecutionControlFromUpdateControl() { testCustomResource.addFinalizer(DEFAULT_FINALIZER); reconciler.reconcile = @@ -368,7 +368,7 @@ void setObservedGenerationForStatusIfNeeded() throws Exception { Reconciler reconciler = mock(Reconciler.class); ControllerConfiguration config = - mock(ControllerConfiguration.class); + MockControllerConfiguration.forResource(ObservedGenCustomResource.class); CustomResourceFacade facade = mock(CustomResourceFacade.class); var dispatcher = init(observedGenResource, reconciler, config, facade, true); @@ -389,8 +389,7 @@ void updatesObservedGenerationOnNoUpdateUpdateControl() throws Exception { var observedGenResource = createObservedGenCustomResource(); Reconciler reconciler = mock(Reconciler.class); - ControllerConfiguration config = - mock(ControllerConfiguration.class); + final var config = MockControllerConfiguration.forResource(ObservedGenCustomResource.class); CustomResourceFacade facade = mock(CustomResourceFacade.class); when(config.isGenerationAware()).thenReturn(true); when(reconciler.reconcile(any(), any())) @@ -410,8 +409,7 @@ void updateObservedGenerationOnCustomResourceUpdate() throws Exception { var observedGenResource = createObservedGenCustomResource(); Reconciler reconciler = mock(Reconciler.class); - ControllerConfiguration config = - mock(ControllerConfiguration.class); + final var config = MockControllerConfiguration.forResource(ObservedGenCustomResource.class); CustomResourceFacade facade = mock(CustomResourceFacade.class); when(config.isGenerationAware()).thenReturn(true); when(reconciler.reconcile(any(), any())) @@ -428,7 +426,7 @@ void updateObservedGenerationOnCustomResourceUpdate() throws Exception { } @Test - void callErrorStatusHandlerIfImplemented() throws Exception { + void callErrorStatusHandlerIfImplemented() { testCustomResource.addFinalizer(DEFAULT_FINALIZER); reconciler.reconcile = (r, c) -> { @@ -460,7 +458,7 @@ public boolean isLastAttempt() { } @Test - void callErrorStatusHandlerEvenOnFirstError() throws Exception { + void callErrorStatusHandlerEvenOnFirstError() { testCustomResource.addFinalizer(DEFAULT_FINALIZER); reconciler.reconcile = (r, c) -> { @@ -523,7 +521,7 @@ void errorHandlerCanInstructNoRetryNoUpdate() { } @Test - void schedulesReconciliationIfMaxDelayIsSet() throws Exception { + void schedulesReconciliationIfMaxDelayIsSet() { testCustomResource.addFinalizer(DEFAULT_FINALIZER); reconciler.reconcile = (r, c) -> UpdateControl.noUpdate(); @@ -536,7 +534,7 @@ void schedulesReconciliationIfMaxDelayIsSet() throws Exception { } @Test - void canSkipSchedulingMaxDelayIf() throws Exception { + void canSkipSchedulingMaxDelayIf() { testCustomResource.addFinalizer(DEFAULT_FINALIZER); reconciler.reconcile = (r, c) -> UpdateControl.noUpdate(); @@ -573,6 +571,7 @@ public ExecutionScope executionScopeWithCREvent(T res private class TestReconciler implements Reconciler, Cleaner, ErrorStatusHandler { + private BiFunction> reconcile; private BiFunction cleanup; private ErrorStatusHandler errorHandler; diff --git a/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/event/source/informer/InformerEventSourceTest.java b/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/event/source/informer/InformerEventSourceTest.java index 7ac6779523..8563fe4d01 100644 --- a/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/event/source/informer/InformerEventSourceTest.java +++ b/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/event/source/informer/InformerEventSourceTest.java @@ -14,6 +14,7 @@ import io.fabric8.kubernetes.client.dsl.MixedOperation; import io.fabric8.kubernetes.client.informers.SharedIndexInformer; import io.fabric8.kubernetes.client.informers.cache.Indexer; +import io.javaoperatorsdk.operator.api.config.ResourceConfiguration; import io.javaoperatorsdk.operator.api.config.informer.InformerConfiguration; import io.javaoperatorsdk.operator.processing.event.EventHandler; import io.javaoperatorsdk.operator.processing.event.ResourceID; @@ -23,6 +24,7 @@ import static org.mockito.ArgumentMatchers.any; import static org.mockito.Mockito.*; +@SuppressWarnings({"rawtypes", "unchecked"}) class InformerEventSourceTest { private static final String PREV_RESOURCE_VERSION = "0"; @@ -30,16 +32,17 @@ class InformerEventSourceTest { private static final String NEXT_RESOURCE_VERSION = "2"; private InformerEventSource informerEventSource; - private KubernetesClient clientMock = mock(KubernetesClient.class); - private TemporaryResourceCache temporaryResourceCacheMock = + private final KubernetesClient clientMock = mock(KubernetesClient.class); + private final TemporaryResourceCache temporaryResourceCacheMock = mock(TemporaryResourceCache.class); - private EventHandler eventHandlerMock = mock(EventHandler.class); - private MixedOperation crClientMock = mock(MixedOperation.class); - private FilterWatchListMultiDeletable specificResourceClientMock = + private final EventHandler eventHandlerMock = mock(EventHandler.class); + private final MixedOperation crClientMock = mock(MixedOperation.class); + private final FilterWatchListMultiDeletable specificResourceClientMock = mock(FilterWatchListMultiDeletable.class); - private FilterWatchListDeletable labeledResourceClientMock = mock(FilterWatchListDeletable.class); - private SharedIndexInformer informer = mock(SharedIndexInformer.class); - private InformerConfiguration informerConfiguration = + private final FilterWatchListDeletable labeledResourceClientMock = + mock(FilterWatchListDeletable.class); + private final SharedIndexInformer informer = mock(SharedIndexInformer.class); + private final InformerConfiguration informerConfiguration = mock(InformerConfiguration.class); @BeforeEach @@ -51,6 +54,8 @@ void setup() { when(labeledResourceClientMock.runnableInformer(0)).thenReturn(informer); when(informer.getIndexer()).thenReturn(mock(Indexer.class)); + when(informerConfiguration.getEffectiveNamespaces()) + .thenReturn(ResourceConfiguration.DEFAULT_NAMESPACES); when(informerConfiguration.getSecondaryToPrimaryMapper()) .thenReturn(mock(SecondaryToPrimaryMapper.class));