feat: move controller informer-related configuration to InformerConfig (#2455)

* feat: move controller informer-related configuration to InformerConfig

Signed-off-by: Chris Laprun <claprun@redhat.com>

* refactor: start isolating ResourceConfiguration

Signed-off-by: Chris Laprun <claprun@redhat.com>

* fix: initFromAnnotation now properly inits the current instance

Signed-off-by: Chris Laprun <claprun@redhat.com>

* fix: default onDeleteFilter implementation

Signed-off-by: Chris Laprun <claprun@redhat.com>

* fix: properly set default namespaces in controller case

Signed-off-by: Chris Laprun <claprun@redhat.com>

* refactor: remove KubernetesDependentInformerConfigBuilder

Signed-off-by: Chris Laprun <claprun@redhat.com>

* refactor: use InformerConfigHolder in more places, unifying handling

Signed-off-by: Chris Laprun <claprun@redhat.com>

* fix: properly propagate name to informer config

Signed-off-by: Chris Laprun <claprun@redhat.com>

* feat: add factory method to init builder from an extising configuation

Signed-off-by: Chris Laprun <claprun@redhat.com>

* fix: remove potentially problematic default implementation

Signed-off-by: Chris Laprun <claprun@redhat.com>

---------

Signed-off-by: Chris Laprun <claprun@redhat.com>

Author: metacosm

caffeine-bounded-cache-support/src/test/java/io/javaoperatorsdk/operator/processing/event/source/cache/sample/AbstractTestReconciler.java

@@ -78,7 +78,7 @@ public List<EventSource<?, P>> prepareEventSources(
             ConfigMap.class, Duration.ofMinutes(1), 1); // setting max size for testing purposes
 
     var es = new InformerEventSource<>(InformerConfiguration.from(ConfigMap.class, primaryClass())
-        .withItemStore(boundedItemStore)
+        .withInformerConfiguration(c -> c.withItemStore(boundedItemStore))
         .withSecondaryToPrimaryMapper(
             Mappers.fromOwnerReferences(context.getPrimaryResourceClass(),
                 this instanceof BoundedCacheClusterScopeTestReconciler))

operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/BaseConfigurationService.java

@@ -13,7 +13,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.ReconcilerUtils;
 import io.javaoperatorsdk.operator.api.config.Utils.Configurator;
 import io.javaoperatorsdk.operator.api.config.dependent.DependentResourceConfigurationResolver;
@@ -24,11 +23,9 @@
 import io.javaoperatorsdk.operator.api.reconciler.Workflow;
 import io.javaoperatorsdk.operator.api.reconciler.dependent.Dependent;
 import io.javaoperatorsdk.operator.api.reconciler.dependent.DependentResource;
+import io.javaoperatorsdk.operator.processing.dependent.kubernetes.InformerConfigHolder;
 import io.javaoperatorsdk.operator.processing.dependent.workflow.Condition;
 import io.javaoperatorsdk.operator.processing.event.rate.RateLimiter;
-import io.javaoperatorsdk.operator.processing.event.source.filter.GenericFilter;
-import io.javaoperatorsdk.operator.processing.event.source.filter.OnAddFilter;
-import io.javaoperatorsdk.operator.processing.event.source.filter.OnUpdateFilter;
 import io.javaoperatorsdk.operator.processing.retry.Retry;
 
 import static io.javaoperatorsdk.operator.api.config.ControllerConfiguration.CONTROLLER_NAME_AS_FIELD_MANAGER;
@@ -278,43 +275,20 @@ private <P extends HasMetadata> ResolvedControllerConfiguration<P> controllerCon
         fieldManager.equals(CONTROLLER_NAME_AS_FIELD_MANAGER) ? name
             : fieldManager;
 
-    var informerListLimitValue = valueOrDefaultFromAnnotation(annotation,
-        io.javaoperatorsdk.operator.api.reconciler.ControllerConfiguration::informerListLimit,
-        "informerListLimit");
-    final var informerListLimit =
-        informerListLimitValue == Constants.NO_LONG_VALUE_SET ? null
-            : informerListLimitValue;
+    InformerConfigHolder<P> informerConfig = InformerConfigHolder.builder(resourceClass)
+        .initFromAnnotation(annotation != null ? annotation.informerConfig() : null, context)
+        .buildForController();
 
     return new ResolvedControllerConfiguration<P>(
         resourceClass, name, generationAware,
         associatedReconcilerClass, retry, rateLimiter,
         ResolvedControllerConfiguration.getMaxReconciliationInterval(interval, timeUnit),
-        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,
-            "namespaces")),
         valueOrDefaultFromAnnotation(annotation,
             io.javaoperatorsdk.operator.api.reconciler.ControllerConfiguration::finalizerName,
             "finalizerName"),
-        valueOrDefaultFromAnnotation(annotation,
-            io.javaoperatorsdk.operator.api.reconciler.ControllerConfiguration::labelSelector,
-            "labelSelector"),
         null,
-        Utils.instantiate(
-            valueOrDefaultFromAnnotation(annotation,
-                io.javaoperatorsdk.operator.api.reconciler.ControllerConfiguration::itemStore,
-                "itemStore"),
-            ItemStore.class, context),
         dependentFieldManager,
-        this, informerListLimit);
+        this, informerConfig);
   }
 
 
@@ -326,6 +300,4 @@ protected boolean createIfNeeded() {
   public boolean checkCRDAndValidateLocalModel() {
     return Utils.shouldCheckCRDAndValidateLocalModel();
   }
-
-
 }

operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/ControllerConfigurationOverrider.java

@@ -10,51 +10,46 @@
 import io.fabric8.kubernetes.api.model.HasMetadata;
 import io.fabric8.kubernetes.client.informers.cache.ItemStore;
 import io.javaoperatorsdk.operator.api.config.dependent.DependentResourceSpec;
+import io.javaoperatorsdk.operator.processing.dependent.kubernetes.InformerConfigHolder;
 import io.javaoperatorsdk.operator.processing.event.rate.RateLimiter;
 import io.javaoperatorsdk.operator.processing.event.source.filter.GenericFilter;
 import io.javaoperatorsdk.operator.processing.event.source.filter.OnAddFilter;
 import io.javaoperatorsdk.operator.processing.event.source.filter.OnUpdateFilter;
 import io.javaoperatorsdk.operator.processing.retry.Retry;
 
-import static io.javaoperatorsdk.operator.api.reconciler.Constants.DEFAULT_NAMESPACES_SET;
-import static io.javaoperatorsdk.operator.api.reconciler.Constants.WATCH_CURRENT_NAMESPACE_SET;
 
 @SuppressWarnings({"rawtypes", "unused", "UnusedReturnValue"})
 public class ControllerConfigurationOverrider<R extends HasMetadata> {
 
+  private final ControllerConfiguration<R> original;
+  private String name;
   private String finalizer;
   private boolean generationAware;
-  private Set<String> namespaces;
   private Retry retry;
-  private String labelSelector;
-  private final ControllerConfiguration<R> original;
-  private Duration reconciliationMaxInterval;
-  private OnAddFilter<? super R> onAddFilter;
-  private OnUpdateFilter<? super R> onUpdateFilter;
-  private GenericFilter<? super R> genericFilter;
   private RateLimiter rateLimiter;
-  private Map<DependentResourceSpec, Object> configurations;
-  private ItemStore<R> itemStore;
-  private String name;
   private String fieldManager;
-  private Long informerListLimit;
+  private Duration reconciliationMaxInterval;
+  private Map<DependentResourceSpec, Object> configurations;
+  private final InformerConfigHolder<R>.Builder config;
 
   private ControllerConfigurationOverrider(ControllerConfiguration<R> original) {
     this.finalizer = original.getFinalizerName();
     this.generationAware = original.isGenerationAware();
-    this.namespaces = new HashSet<>(original.getNamespaces());
+    this.config = InformerConfigHolder.builder(original.getResourceClass())
+        .withName(name)
+        .withNamespaces(original.getNamespaces())
+        .withLabelSelector(original.getLabelSelector())
+        .withOnAddFilter(original.onAddFilter().orElse(null))
+        .withOnUpdateFilter(original.onUpdateFilter().orElse(null))
+        .withGenericFilter(original.genericFilter().orElse(null))
+        .withInformerListLimit(original.getInformerListLimit().orElse(null))
+        .withItemStore(original.getItemStore().orElse(null));
     this.retry = original.getRetry();
-    this.labelSelector = original.getLabelSelector();
     this.reconciliationMaxInterval = original.maxReconciliationInterval().orElse(null);
-    this.onAddFilter = original.onAddFilter().orElse(null);
-    this.onUpdateFilter = original.onUpdateFilter().orElse(null);
-    this.genericFilter = original.genericFilter().orElse(null);
     this.original = original;
     this.rateLimiter = original.getRateLimiter();
     this.name = original.getName();
     this.fieldManager = original.fieldManager();
-    this.informerListLimit = original.getInformerListLimit().orElse(null);
-    this.itemStore = original.getItemStore().orElse(null);
   }
 
   public ControllerConfigurationOverrider<R> withFinalizer(String finalizer) {
@@ -68,26 +63,36 @@ public ControllerConfigurationOverrider<R> withGenerationAware(boolean generatio
   }
 
   public ControllerConfigurationOverrider<R> watchingOnlyCurrentNamespace() {
-    this.namespaces = WATCH_CURRENT_NAMESPACE_SET;
+    config.withWatchCurrentNamespace();
     return this;
   }
 
   public ControllerConfigurationOverrider<R> addingNamespaces(String... namespaces) {
-    this.namespaces.addAll(List.of(namespaces));
+    if (namespaces != null && namespaces.length > 0) {
+      final var current = config.namespaces();
+      final var aggregated = new HashSet<String>(current.size() + namespaces.length);
+      aggregated.addAll(current);
+      aggregated.addAll(Set.of(namespaces));
+      config.withNamespaces(aggregated);
+    }
     return this;
   }
 
   public ControllerConfigurationOverrider<R> removingNamespaces(String... namespaces) {
-    List.of(namespaces).forEach(this.namespaces::remove);
-    if (this.namespaces.isEmpty()) {
-      this.namespaces = DEFAULT_NAMESPACES_SET;
+    if (namespaces != null && namespaces.length > 0) {
+      final var current = new HashSet<>(config.namespaces());
+      List.of(namespaces).forEach(current::remove);
+      if (current.isEmpty()) {
+        return watchingAllNamespaces();
+      } else {
+        config.withNamespaces(current);
+      }
     }
     return this;
   }
 
   public ControllerConfigurationOverrider<R> settingNamespaces(Set<String> newNamespaces) {
-    this.namespaces.clear();
-    this.namespaces.addAll(newNamespaces);
+    config.withNamespaces(newNamespaces);
     return this;
   }
 
@@ -96,13 +101,12 @@ public ControllerConfigurationOverrider<R> settingNamespaces(String... newNamesp
   }
 
   public ControllerConfigurationOverrider<R> settingNamespace(String namespace) {
-    this.namespaces.clear();
-    this.namespaces.add(namespace);
+    config.withNamespaces(Set.of(namespace));
     return this;
   }
 
   public ControllerConfigurationOverrider<R> watchingAllNamespaces() {
-    this.namespaces = DEFAULT_NAMESPACES_SET;
+    config.withWatchAllNamespaces();
     return this;
   }
 
@@ -117,7 +121,7 @@ public ControllerConfigurationOverrider<R> withRateLimiter(RateLimiter rateLimit
   }
 
   public ControllerConfigurationOverrider<R> withLabelSelector(String labelSelector) {
-    this.labelSelector = labelSelector;
+    config.withLabelSelector(labelSelector);
     return this;
   }
 
@@ -128,27 +132,28 @@ public ControllerConfigurationOverrider<R> withReconciliationMaxInterval(
   }
 
   public ControllerConfigurationOverrider<R> withOnAddFilter(OnAddFilter<R> onAddFilter) {
-    this.onAddFilter = onAddFilter;
+    config.withOnAddFilter(onAddFilter);
     return this;
   }
 
   public ControllerConfigurationOverrider<R> withOnUpdateFilter(OnUpdateFilter<R> onUpdateFilter) {
-    this.onUpdateFilter = onUpdateFilter;
+    config.withOnUpdateFilter(onUpdateFilter);
     return this;
   }
 
   public ControllerConfigurationOverrider<R> withGenericFilter(GenericFilter<R> genericFilter) {
-    this.genericFilter = genericFilter;
+    config.withGenericFilter(genericFilter);
     return this;
   }
 
   public ControllerConfigurationOverrider<R> withItemStore(ItemStore<R> itemStore) {
-    this.itemStore = itemStore;
+    config.withItemStore(itemStore);
     return this;
   }
 
   public ControllerConfigurationOverrider<R> withName(String name) {
     this.name = name;
+    config.withName(name);
     return this;
   }
 
@@ -168,7 +173,7 @@ public ControllerConfigurationOverrider<R> withFieldManager(
    */
   public ControllerConfigurationOverrider<R> withInformerListLimit(
       Long informerListLimit) {
-    this.informerListLimit = informerListLimit;
+    config.withInformerListLimit(informerListLimit);
     return this;
   }
 
@@ -192,9 +197,10 @@ public ControllerConfiguration<R> build() {
     return new ResolvedControllerConfiguration<>(original.getResourceClass(),
         name,
         generationAware, original.getAssociatedReconcilerClassName(), retry, rateLimiter,
-        reconciliationMaxInterval, onAddFilter, onUpdateFilter, genericFilter,
-        namespaces, finalizer, labelSelector, configurations, itemStore, fieldManager,
-        original.getConfigurationService(), informerListLimit,
+        reconciliationMaxInterval,
+        finalizer, configurations, fieldManager,
+        original.getConfigurationService(),
+        config.buildForController(),
         original.getWorkflowSpec().orElse(null));
   }
 

operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/DefaultResourceConfiguration.java

@@ -1,92 +1,41 @@
 package io.javaoperatorsdk.operator.api.config;
 
-import java.util.Optional;
-import java.util.Set;
 
 import io.fabric8.kubernetes.api.model.GenericKubernetesResource;
 import io.fabric8.kubernetes.api.model.HasMetadata;
-import io.fabric8.kubernetes.client.informers.cache.ItemStore;
 import io.javaoperatorsdk.operator.ReconcilerUtils;
-import io.javaoperatorsdk.operator.processing.event.source.filter.GenericFilter;
-import io.javaoperatorsdk.operator.processing.event.source.filter.OnAddFilter;
-import io.javaoperatorsdk.operator.processing.event.source.filter.OnUpdateFilter;
+import io.javaoperatorsdk.operator.processing.dependent.kubernetes.InformerConfigHolder;
 
 public class DefaultResourceConfiguration<R extends HasMetadata>
     implements ResourceConfiguration<R> {
 
   private final Class<R> resourceClass;
   private final String resourceTypeName;
-  private final OnAddFilter<? super R> onAddFilter;
-  private final OnUpdateFilter<? super R> onUpdateFilter;
-  private final GenericFilter<? super R> genericFilter;
-  private final String labelSelector;
-  private final Set<String> namespaces;
-  private final ItemStore<R> itemStore;
-  private final Long informerListLimit;
+  private final InformerConfigHolder<R> informerConfig;
 
   protected DefaultResourceConfiguration(Class<R> resourceClass,
-      Set<String> namespaces, String labelSelector, OnAddFilter<? super R> onAddFilter,
-      OnUpdateFilter<? super R> onUpdateFilter, GenericFilter<? super R> genericFilter,
-      ItemStore<R> itemStore, Long informerListLimit) {
+      InformerConfigHolder<R> informerConfig) {
     this.resourceClass = resourceClass;
     this.resourceTypeName = resourceClass.isAssignableFrom(GenericKubernetesResource.class)
         // in general this is irrelevant now for secondary resources it is used just by controller
         // where GenericKubernetesResource now does not apply
         ? GenericKubernetesResource.class.getSimpleName()
         : ReconcilerUtils.getResourceTypeName(resourceClass);
-    this.onAddFilter = onAddFilter;
-    this.onUpdateFilter = onUpdateFilter;
-    this.genericFilter = genericFilter;
-
-    this.namespaces = ResourceConfiguration.ensureValidNamespaces(namespaces);
-    this.labelSelector = ResourceConfiguration.ensureValidLabelSelector(labelSelector);
-    this.itemStore = itemStore;
-    this.informerListLimit = informerListLimit;
+    this.informerConfig = informerConfig;
   }
 
   @Override
   public String getResourceTypeName() {
     return resourceTypeName;
   }
 
-  @Override
-  public String getLabelSelector() {
-    return labelSelector;
-  }
-
-  @Override
-  public Set<String> getNamespaces() {
-    return namespaces;
-  }
-
   @Override
   public Class<R> getResourceClass() {
     return resourceClass;
   }
 
   @Override
-  public Optional<OnAddFilter<? super R>> onAddFilter() {
-    return Optional.ofNullable(onAddFilter);
-  }
-
-  @Override
-  public Optional<OnUpdateFilter<? super R>> onUpdateFilter() {
-    return Optional.ofNullable(onUpdateFilter);
+  public InformerConfigHolder<R> getInformerConfig() {
+    return informerConfig;
   }
-
-  @Override
-  public Optional<GenericFilter<? super R>> genericFilter() {
-    return Optional.ofNullable(genericFilter);
-  }
-
-  @Override
-  public Optional<ItemStore<R>> getItemStore() {
-    return Optional.ofNullable(itemStore);
-  }
-
-  @Override
-  public Optional<Long> getInformerListLimit() {
-    return Optional.ofNullable(informerListLimit);
-  }
-
 }