feat: @ControllerConfiguration annotation is optional (#2412)

Signed-off-by: Attila Mészáros <csviri@gmail.com> Co-authored-by: Chris Laprun <claprun@redhat.com>
operator-framework · Jul 9, 2024 · b34525c · b34525c
1 parent da7b033
commit b34525c
Show file tree

Hide file tree

Showing 3 changed files with 128 additions and 83 deletions.
diff --git a/...k-core/src/main/java/io/javaoperatorsdk/operator/api/config/BaseConfigurationService.java b/...k-core/src/main/java/io/javaoperatorsdk/operator/api/config/BaseConfigurationService.java
@@ -14,7 +14,6 @@
 import io.fabric8.kubernetes.api.model.HasMetadata;
 import io.fabric8.kubernetes.client.KubernetesClient;
 import io.fabric8.kubernetes.client.informers.cache.ItemStore;
-import io.javaoperatorsdk.operator.OperatorException;
 import io.javaoperatorsdk.operator.ReconcilerUtils;
 import io.javaoperatorsdk.operator.api.config.Utils.Configurator;
 import io.javaoperatorsdk.operator.api.config.dependent.DependentResourceConfigurationResolver;
@@ -33,7 +32,6 @@
 import io.javaoperatorsdk.operator.processing.retry.Retry;
 
 import static io.javaoperatorsdk.operator.api.config.ControllerConfiguration.CONTROLLER_NAME_AS_FIELD_MANAGER;
-import static io.javaoperatorsdk.operator.api.reconciler.Constants.DEFAULT_NAMESPACES_SET;
 
 public class BaseConfigurationService extends AbstractConfigurationService {
 
@@ -91,6 +89,7 @@ private static List<DependentResourceSpec> dependentResources(
           Utils.instantiate(dependent.deletePostcondition(), Condition.class, context),
           Utils.instantiate(dependent.activationCondition(), Condition.class, context),
           eventSourceName);
+      specsMap.put(dependentName, spec);
 
       // extract potential configuration
       DependentResourceConfigurationResolver.configureSpecFromConfigured(spec,
@@ -99,17 +98,24 @@ private static List<DependentResourceSpec> dependentResources(
 
       specsMap.put(dependentName, spec);
     }
+
     return specsMap.values().stream().toList();
   }
 
-  private static <T> T valueOrDefault(
+  @SuppressWarnings("unchecked")
+  private static <T> T valueOrDefaultFromAnnotation(
       io.javaoperatorsdk.operator.api.reconciler.ControllerConfiguration controllerConfiguration,
       Function<io.javaoperatorsdk.operator.api.reconciler.ControllerConfiguration, T> mapper,
-      T defaultValue) {
-    if (controllerConfiguration == null) {
-      return defaultValue;
-    } else {
-      return mapper.apply(controllerConfiguration);
+      String defaultMethodName) {
+    try {
+      if (controllerConfiguration == null) {
+        return (T) io.javaoperatorsdk.operator.api.reconciler.ControllerConfiguration.class
+            .getDeclaredMethod(defaultMethodName).getDefaultValue();
+      } else {
+        return mapper.apply(controllerConfiguration);
+      }
+    } catch (NoSuchMethodException e) {
+      throw new RuntimeException(e);
     }
   }
 
@@ -123,17 +129,18 @@ private static String getName(String name, Class<? extends DependentResource> de
 
   @SuppressWarnings("unused")
   private static <T> Configurator<T> configuratorFor(Class<T> instanceType,
-      Reconciler<?> reconciler) {
-    return instance -> configureFromAnnotatedReconciler(instance, reconciler);
+      Class<? extends Reconciler<?>> reconcilerClass) {
+    return instance -> configureFromAnnotatedReconciler(instance, reconcilerClass);
   }
 
   @SuppressWarnings({"unchecked", "rawtypes"})
-  private static void configureFromAnnotatedReconciler(Object instance, Reconciler<?> reconciler) {
+  private static void configureFromAnnotatedReconciler(Object instance,
+      Class<? extends Reconciler<?>> reconcilerClass) {
     if (instance instanceof AnnotationConfigurable configurable) {
       final Class<? extends Annotation> configurationClass =
           (Class<? extends Annotation>) Utils.getFirstTypeArgumentFromSuperClassOrInterface(
               instance.getClass(), AnnotationConfigurable.class);
-      final var configAnnotation = reconciler.getClass().getAnnotation(configurationClass);
+      final var configAnnotation = reconcilerClass.getAnnotation(configurationClass);
       if (configAnnotation != null) {
         configurable.initFrom(configAnnotation);
       }
@@ -190,101 +197,127 @@ protected ResourceClassResolver getResourceClassResolver() {
 
   @SuppressWarnings({"unchecked", "rawtypes"})
   protected <P extends HasMetadata> ControllerConfiguration<P> configFor(Reconciler<P> reconciler) {
-    final var annotation = reconciler.getClass().getAnnotation(
+    final Class<? extends Reconciler<P>> reconcilerClass =
+        (Class<? extends Reconciler<P>>) reconciler.getClass();
+    final var controllerAnnotation = reconcilerClass.getAnnotation(
         io.javaoperatorsdk.operator.api.reconciler.ControllerConfiguration.class);
 
-    if (annotation == null) {
-      throw new OperatorException(
-          "Missing mandatory @"
-              + io.javaoperatorsdk.operator.api.reconciler.ControllerConfiguration.class
-                  .getSimpleName()
-              +
-              " annotation for reconciler:  " + reconciler);
+    ResolvedControllerConfiguration<P> config =
+        controllerConfiguration(reconcilerClass, controllerAnnotation);
+
+    final var workflowAnnotation = reconcilerClass.getAnnotation(
+        io.javaoperatorsdk.operator.api.reconciler.Workflow.class);
+    if (workflowAnnotation != null) {
+      final var specs = dependentResources(workflowAnnotation, config);
+      WorkflowSpec workflowSpec = new WorkflowSpec() {
+        @Override
+        public List<DependentResourceSpec> getDependentResourceSpecs() {
+          return specs;
+        }
+
+        @Override
+        public boolean isExplicitInvocation() {
+          return workflowAnnotation.explicitInvocation();
+        }
+
+        @Override
+        public boolean handleExceptionsInReconciler() {
+          return workflowAnnotation.handleExceptionsInReconciler();
+        }
+
+      };
+      config.setWorkflowSpec(workflowSpec);
     }
-    Class<Reconciler<P>> reconcilerClass = (Class<Reconciler<P>>) reconciler.getClass();
+
+    return config;
+  }
+
+  @SuppressWarnings({"unchecked"})
+  private <P extends HasMetadata> ResolvedControllerConfiguration<P> controllerConfiguration(
+      Class<? extends Reconciler<P>> reconcilerClass,
+      io.javaoperatorsdk.operator.api.reconciler.ControllerConfiguration annotation) {
     final var resourceClass = getResourceClassResolver().getPrimaryResourceClass(reconcilerClass);
 
-    final var name = ReconcilerUtils.getNameFor(reconciler);
-    final var generationAware = valueOrDefault(
+    final var name = ReconcilerUtils.getNameFor(reconcilerClass);
+    final var generationAware = valueOrDefaultFromAnnotation(
         annotation,
         io.javaoperatorsdk.operator.api.reconciler.ControllerConfiguration::generationAwareEventProcessing,
-        true);
+        "generationAwareEventProcessing");
     final var associatedReconcilerClass =
-        ResolvedControllerConfiguration.getAssociatedReconcilerClassName(reconciler.getClass());
+        ResolvedControllerConfiguration.getAssociatedReconcilerClassName(reconcilerClass);
 
     final var context = Utils.contextFor(name);
-    final Class<? extends Retry> retryClass = annotation.retry();
+    final Class<? extends Retry> retryClass =
+        valueOrDefaultFromAnnotation(annotation,
+            io.javaoperatorsdk.operator.api.reconciler.ControllerConfiguration::retry,
+            "retry");
     final var retry = Utils.instantiateAndConfigureIfNeeded(retryClass, Retry.class,
-        context, configuratorFor(Retry.class, reconciler));
+        context, configuratorFor(Retry.class, reconcilerClass));
 
-    final Class<? extends RateLimiter> rateLimiterClass = annotation.rateLimiter();
+    @SuppressWarnings("rawtypes")
+    final Class<? extends RateLimiter> rateLimiterClass = valueOrDefaultFromAnnotation(annotation,
+        io.javaoperatorsdk.operator.api.reconciler.ControllerConfiguration::rateLimiter,
+        "rateLimiter");
     final var rateLimiter = Utils.instantiateAndConfigureIfNeeded(rateLimiterClass,
-        RateLimiter.class, context, configuratorFor(RateLimiter.class, reconciler));
+        RateLimiter.class, context, configuratorFor(RateLimiter.class, reconcilerClass));
 
-    final var reconciliationInterval = annotation.maxReconciliationInterval();
+    final var reconciliationInterval = valueOrDefaultFromAnnotation(annotation,
+        io.javaoperatorsdk.operator.api.reconciler.ControllerConfiguration::maxReconciliationInterval,
+        "maxReconciliationInterval");
     long interval = -1;
     TimeUnit timeUnit = null;
     if (reconciliationInterval != null && reconciliationInterval.interval() > 0) {
       interval = reconciliationInterval.interval();
       timeUnit = reconciliationInterval.timeUnit();
     }
 
+    var fieldManager = valueOrDefaultFromAnnotation(annotation,
+        io.javaoperatorsdk.operator.api.reconciler.ControllerConfiguration::fieldManager,
+        "fieldManager");
     final var dependentFieldManager =
-        annotation.fieldManager().equals(CONTROLLER_NAME_AS_FIELD_MANAGER) ? name
-            : annotation.fieldManager();
+        fieldManager.equals(CONTROLLER_NAME_AS_FIELD_MANAGER) ? name
+            : fieldManager;
 
+    var informerListLimitValue = valueOrDefaultFromAnnotation(annotation,
+        io.javaoperatorsdk.operator.api.reconciler.ControllerConfiguration::informerListLimit,
+        "informerListLimit");
     final var informerListLimit =
-        annotation.informerListLimit() == Constants.NO_LONG_VALUE_SET ? null
-            : annotation.informerListLimit();
+        informerListLimitValue == Constants.NO_LONG_VALUE_SET ? null
+            : informerListLimitValue;
 
-    final var config = new ResolvedControllerConfiguration<P>(
+    return new ResolvedControllerConfiguration<P>(
         resourceClass, name, generationAware,
         associatedReconcilerClass, retry, rateLimiter,
         ResolvedControllerConfiguration.getMaxReconciliationInterval(interval, timeUnit),
-        Utils.instantiate(annotation.onAddFilter(), OnAddFilter.class, context),
-        Utils.instantiate(annotation.onUpdateFilter(), OnUpdateFilter.class, context),
-        Utils.instantiate(annotation.genericFilter(), GenericFilter.class, context),
-        Set.of(valueOrDefault(annotation,
+        Utils.instantiate(valueOrDefaultFromAnnotation(annotation,
+            io.javaoperatorsdk.operator.api.reconciler.ControllerConfiguration::onAddFilter,
+            "onAddFilter"), OnAddFilter.class, context),
+        Utils.instantiate(valueOrDefaultFromAnnotation(annotation,
+            io.javaoperatorsdk.operator.api.reconciler.ControllerConfiguration::onUpdateFilter,
+            "onUpdateFilter"), OnUpdateFilter.class, context),
+        Utils.instantiate(valueOrDefaultFromAnnotation(annotation,
+            io.javaoperatorsdk.operator.api.reconciler.ControllerConfiguration::genericFilter,
+            "genericFilter"), GenericFilter.class, context),
+        Set.of(valueOrDefaultFromAnnotation(annotation,
             io.javaoperatorsdk.operator.api.reconciler.ControllerConfiguration::namespaces,
-            DEFAULT_NAMESPACES_SET.toArray(String[]::new))),
-        valueOrDefault(annotation,
+            "namespaces")),
+        valueOrDefaultFromAnnotation(annotation,
             io.javaoperatorsdk.operator.api.reconciler.ControllerConfiguration::finalizerName,
-            Constants.NO_VALUE_SET),
-        valueOrDefault(annotation,
+            "finalizerName"),
+        valueOrDefaultFromAnnotation(annotation,
             io.javaoperatorsdk.operator.api.reconciler.ControllerConfiguration::labelSelector,
-            Constants.NO_VALUE_SET),
+            "labelSelector"),
         null,
-        Utils.instantiate(annotation.itemStore(), ItemStore.class, context), dependentFieldManager,
+        Utils.instantiate(
+            valueOrDefaultFromAnnotation(annotation,
+                io.javaoperatorsdk.operator.api.reconciler.ControllerConfiguration::itemStore,
+                "itemStore"),
+            ItemStore.class, context),
+        dependentFieldManager,
         this, informerListLimit);
-
-
-    final var workflowAnnotation = reconciler.getClass().getAnnotation(
-        io.javaoperatorsdk.operator.api.reconciler.Workflow.class);
-    if (workflowAnnotation != null) {
-      final var specs = dependentResources(workflowAnnotation, config);
-      WorkflowSpec workflowSpec = new WorkflowSpec() {
-        @Override
-        public List<DependentResourceSpec> getDependentResourceSpecs() {
-          return specs;
-        }
-
-        @Override
-        public boolean isExplicitInvocation() {
-          return workflowAnnotation.explicitInvocation();
-        }
-
-        @Override
-        public boolean handleExceptionsInReconciler() {
-          return workflowAnnotation.handleExceptionsInReconciler();
-        }
-
-      };
-      config.setWorkflowSpec(workflowSpec);
-    }
-
-    return config;
   }
 
+
   protected boolean createIfNeeded() {
     return true;
   }
@@ -293,4 +326,6 @@ protected boolean createIfNeeded() {
   public boolean checkCRDAndValidateLocalModel() {
     return Utils.shouldCheckCRDAndValidateLocalModel();
   }
+
+
 }
diff --git a/...mework/src/test/java/io/javaoperatorsdk/operator/config/BaseConfigurationServiceTest.java b/...mework/src/test/java/io/javaoperatorsdk/operator/config/BaseConfigurationServiceTest.java
@@ -9,23 +9,17 @@
 import java.util.Optional;
 import java.util.concurrent.TimeUnit;
 
-import org.junit.jupiter.api.Assertions;
 import org.junit.jupiter.api.Test;
 
 import io.fabric8.kubernetes.api.model.ConfigMap;
 import io.fabric8.kubernetes.api.model.HasMetadata;
-import io.javaoperatorsdk.operator.OperatorException;
+import io.javaoperatorsdk.operator.ReconcilerUtils;
 import io.javaoperatorsdk.operator.api.config.AnnotationConfigurable;
 import io.javaoperatorsdk.operator.api.config.BaseConfigurationService;
 import io.javaoperatorsdk.operator.api.config.dependent.ConfigurationConverter;
 import io.javaoperatorsdk.operator.api.config.dependent.Configured;
 import io.javaoperatorsdk.operator.api.config.dependent.DependentResourceSpec;
-import io.javaoperatorsdk.operator.api.reconciler.Context;
-import io.javaoperatorsdk.operator.api.reconciler.ControllerConfiguration;
-import io.javaoperatorsdk.operator.api.reconciler.MaxReconciliationInterval;
-import io.javaoperatorsdk.operator.api.reconciler.Reconciler;
-import io.javaoperatorsdk.operator.api.reconciler.UpdateControl;
-import io.javaoperatorsdk.operator.api.reconciler.Workflow;
+import io.javaoperatorsdk.operator.api.reconciler.*;
 import io.javaoperatorsdk.operator.api.reconciler.dependent.Dependent;
 import io.javaoperatorsdk.operator.api.reconciler.dependent.DependentResource;
 import io.javaoperatorsdk.operator.api.reconciler.dependent.ReconcileResult;
@@ -41,6 +35,8 @@
 import io.javaoperatorsdk.operator.processing.retry.RetryExecution;
 import io.javaoperatorsdk.operator.sample.readonly.ReadOnlyDependent;
 
+import static io.javaoperatorsdk.operator.api.reconciler.MaxReconciliationInterval.DEFAULT_INTERVAL;
+import static org.assertj.core.api.Assertions.assertThat;
 import static org.junit.jupiter.api.Assertions.*;
 
 class BaseConfigurationServiceTest {
@@ -104,9 +100,24 @@ void getDependentResources() {
   }
 
   @Test
-  void missingAnnotationThrowsException() {
+  void missingAnnotationCreatesDefaultConfig() {
     final var reconciler = new MissingAnnotationReconciler();
-    Assertions.assertThrows(OperatorException.class, () -> configFor(reconciler));
+    var config = configFor(reconciler);
+
+    assertThat(config.getName()).isEqualTo(ReconcilerUtils.getNameFor(reconciler));
+    assertThat(config.getLabelSelector()).isEmpty();
+    assertThat(config.getRetry()).isInstanceOf(GenericRetry.class);
+    assertThat(config.getRateLimiter()).isInstanceOf(LinearRateLimiter.class);
+    assertThat(config.maxReconciliationInterval()).hasValue(Duration.ofHours(DEFAULT_INTERVAL));
+    assertThat(config.fieldManager()).isEqualTo(config.getName());
+    assertThat(config.getInformerListLimit()).isEmpty();
+    assertThat(config.onAddFilter()).isEmpty();
+    assertThat(config.onUpdateFilter()).isEmpty();
+    assertThat(config.genericFilter()).isEmpty();
+    assertThat(config.getNamespaces()).isEqualTo(Constants.DEFAULT_NAMESPACES_SET);
+    assertThat(config.getFinalizerName())
+        .isEqualTo(ReconcilerUtils.getDefaultFinalizerName(config.getResourceClass()));
+    assertThat(config.getItemStore()).isEmpty();
   }
 
   @SuppressWarnings("rawtypes")

diff --git a/.../src/main/java/io/javaoperatorsdk/operator/sample/WebPageManagedDependentsReconciler.java b/.../src/main/java/io/javaoperatorsdk/operator/sample/WebPageManagedDependentsReconciler.java
@@ -18,7 +18,6 @@
     @Dependent(type = IngressDependentResource.class,
         reconcilePrecondition = ExposedIngressCondition.class)
 })
-@ControllerConfiguration
 public class WebPageManagedDependentsReconciler
     implements Reconciler<WebPage>, Cleaner<WebPage> {