feat: introduce AnnotationDependentResourceConfigurator concept

The idea is to be able to configure dependents using annotations
directly so that we can remove the special handling of
KubernetesDependentResource from AnnotationControllerConfiguration. This
results in dependent resources being instantiated and configured
directly when processed from the annotations in the managed case, thus
rendering the DependentResourceFactory concept obsolete. This should
also lead to further simplification later.

Author: metacosm

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

@@ -2,7 +2,6 @@
 
 import java.lang.annotation.Annotation;
 import java.time.Duration;
-import java.util.Arrays;
 import java.util.Collections;
 import java.util.LinkedHashMap;
 import java.util.List;
@@ -15,21 +14,17 @@
 import io.javaoperatorsdk.operator.OperatorException;
 import io.javaoperatorsdk.operator.ReconcilerUtils;
 import io.javaoperatorsdk.operator.api.config.dependent.DependentResourceSpec;
-import io.javaoperatorsdk.operator.api.reconciler.Constants;
 import io.javaoperatorsdk.operator.api.reconciler.ControllerConfiguration;
 import io.javaoperatorsdk.operator.api.reconciler.Reconciler;
 import io.javaoperatorsdk.operator.api.reconciler.dependent.Dependent;
 import io.javaoperatorsdk.operator.api.reconciler.dependent.DependentResource;
-import io.javaoperatorsdk.operator.processing.dependent.kubernetes.KubernetesDependent;
-import io.javaoperatorsdk.operator.processing.dependent.kubernetes.KubernetesDependentResource;
-import io.javaoperatorsdk.operator.processing.dependent.kubernetes.KubernetesDependentResourceConfig;
+import io.javaoperatorsdk.operator.api.reconciler.dependent.managed.AnnotationDependentResourceConfigurator;
 import io.javaoperatorsdk.operator.processing.dependent.workflow.Condition;
 import io.javaoperatorsdk.operator.processing.event.rate.RateLimiter;
 import io.javaoperatorsdk.operator.processing.event.source.controller.ResourceEventFilter;
 import io.javaoperatorsdk.operator.processing.event.source.controller.ResourceEventFilters;
 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.OnDeleteFilter;
 import io.javaoperatorsdk.operator.processing.event.source.filter.OnUpdateFilter;
 import io.javaoperatorsdk.operator.processing.retry.Retry;
 
@@ -159,7 +154,7 @@ public Retry getRetry() {
 
 
   @SuppressWarnings("unchecked")
-  private <T> void configureFromAnnotatedReconciler(T instance) {
+  private void configureFromAnnotatedReconciler(Object instance) {
     if (instance instanceof AnnotationConfigurable) {
       AnnotationConfigurable configurable = (AnnotationConfigurable) instance;
       final Class<? extends Annotation> configurationClass =
@@ -172,6 +167,22 @@ private <T> void configureFromAnnotatedReconciler(T instance) {
     }
   }
 
+  @SuppressWarnings("unchecked")
+  private void configureFromCustomAnnotation(Object instance) {
+    if (instance instanceof AnnotationDependentResourceConfigurator) {
+      AnnotationDependentResourceConfigurator configurator =
+          (AnnotationDependentResourceConfigurator) instance;
+      final Class<? extends Annotation> configurationClass =
+          (Class<? extends Annotation>) Utils.getFirstTypeArgumentFromInterface(
+              instance.getClass(), AnnotationDependentResourceConfigurator.class);
+      final var configAnnotation = instance.getClass().getAnnotation(configurationClass);
+      // always called even if the annotation is null so that implementations can provide default
+      // values
+      final var config = configurator.configFrom(configAnnotation, this);
+      configurator.configureWith(config);
+    }
+  }
+
   @Override
   @SuppressWarnings("unchecked")
   public Optional<OnAddFilter<P>> onAddFilter() {
@@ -209,20 +220,22 @@ public List<DependentResourceSpec> getDependentResources() {
 
       final var specsMap = new LinkedHashMap<String, DependentResourceSpec>(dependents.length);
       for (Dependent dependent : dependents) {
-        Object config = null;
         final Class<? extends DependentResource> dependentType = dependent.type();
-        if (KubernetesDependentResource.class.isAssignableFrom(dependentType)) {
-          config = createKubernetesResourceConfig(dependentType);
-        }
 
         final var name = getName(dependent, dependentType);
         var spec = specsMap.get(name);
         if (spec != null) {
           throw new IllegalArgumentException(
               "A DependentResource named '" + name + "' already exists: " + spec);
         }
+
+        final var dependentResource = Utils.instantiateAndConfigureIfNeeded(dependentType,
+            DependentResource.class,
+            Utils.contextFor(this, dependentType, Dependent.class),
+            this::configureFromCustomAnnotation);
+
         final var context = Utils.contextFor(this, dependentType, null);
-        spec = new DependentResourceSpec(dependentType, config, name,
+        spec = new DependentResourceSpec(dependentResource, name,
             Set.of(dependent.dependsOn()),
             Utils.instantiate(dependent.readyPostcondition(), Condition.class, context),
             Utils.instantiate(dependent.reconcilePrecondition(), Condition.class, context),
@@ -243,47 +256,6 @@ private String getName(Dependent dependent, Class<? extends DependentResource> d
     return name;
   }
 
-  @SuppressWarnings({"rawtypes", "unchecked"})
-  private Object createKubernetesResourceConfig(Class<? extends DependentResource> dependentType) {
-
-    Object config;
-    final var kubeDependent = dependentType.getAnnotation(KubernetesDependent.class);
-
-    var namespaces = getNamespaces();
-    var configuredNS = false;
-    String labelSelector = null;
-    OnAddFilter<? extends HasMetadata> onAddFilter = null;
-    OnUpdateFilter<? extends HasMetadata> onUpdateFilter = null;
-    OnDeleteFilter<? extends HasMetadata> onDeleteFilter = null;
-    GenericFilter<? extends HasMetadata> genericFilter = null;
-    if (kubeDependent != null) {
-      if (!Arrays.equals(KubernetesDependent.DEFAULT_NAMESPACES,
-          kubeDependent.namespaces())) {
-        namespaces = Set.of(kubeDependent.namespaces());
-        configuredNS = true;
-      }
-
-      final var fromAnnotation = kubeDependent.labelSelector();
-      labelSelector = Constants.NO_VALUE_SET.equals(fromAnnotation) ? null : fromAnnotation;
-
-
-      final var context =
-          Utils.contextFor(this, dependentType, null);
-      onAddFilter = Utils.instantiate(kubeDependent.onAddFilter(), OnAddFilter.class, context);
-      onUpdateFilter =
-          Utils.instantiate(kubeDependent.onUpdateFilter(), OnUpdateFilter.class, context);
-      onDeleteFilter =
-          Utils.instantiate(kubeDependent.onDeleteFilter(), OnDeleteFilter.class, context);
-      genericFilter =
-          Utils.instantiate(kubeDependent.genericFilter(), GenericFilter.class, context);
-    }
-
-    config =
-        new KubernetesDependentResourceConfig(namespaces, labelSelector, configuredNS, onAddFilter,
-            onUpdateFilter, onDeleteFilter, genericFilter);
-
-    return config;
-  }
 
   public static <T> T valueOrDefault(
       ControllerConfiguration controllerConfiguration,

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

@@ -140,8 +140,9 @@ default ObjectMapper getObjectMapper() {
     return Serialization.jsonMapper();
   }
 
+  @Deprecated(forRemoval = true)
   default DependentResourceFactory dependentResourceFactory() {
-    return new DependentResourceFactory() {};
+    return null;
   }
 
   default Optional<LeaderElectionConfiguration> getLeaderElectionConfiguration() {

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

@@ -11,6 +11,8 @@
 
 import io.fabric8.kubernetes.api.model.HasMetadata;
 import io.javaoperatorsdk.operator.api.config.dependent.DependentResourceSpec;
+import io.javaoperatorsdk.operator.api.reconciler.dependent.DependentResource;
+import io.javaoperatorsdk.operator.api.reconciler.dependent.managed.DependentResourceConfigurator;
 import io.javaoperatorsdk.operator.processing.dependent.kubernetes.KubernetesDependentResourceConfig;
 import io.javaoperatorsdk.operator.processing.event.rate.RateLimiter;
 import io.javaoperatorsdk.operator.processing.event.source.controller.ResourceEventFilter;
@@ -159,42 +161,58 @@ public ControllerConfigurationOverrider<R> withGenericFilter(GenericFilter<R> ge
     return this;
   }
 
+  @SuppressWarnings("unchecked")
   public ControllerConfigurationOverrider<R> replacingNamedDependentResourceConfig(String name,
       Object dependentResourceConfig) {
 
     var current = namedDependentResourceSpecs.get(name);
     if (current == null) {
       throw new IllegalArgumentException("Cannot find a DependentResource named: " + name);
     }
-    replaceConfig(name, dependentResourceConfig, current);
-    return this;
-  }
 
-  private void replaceConfig(String name, Object newConfig, DependentResourceSpec<?, ?> current) {
-    namedDependentResourceSpecs.put(name,
-        new DependentResourceSpec<>(current.getDependentResourceClass(), newConfig, name,
-            current.getDependsOn(), current.getReadyCondition(), current.getReconcileCondition(),
-            current.getDeletePostCondition()));
+    var dependentResource = current.getDependentResource();
+    if (dependentResource instanceof DependentResourceConfigurator) {
+      var configurator = (DependentResourceConfigurator) dependentResource;
+      configurator.configureWith(dependentResourceConfig);
+    }
+
+    return this;
   }
 
   @SuppressWarnings("unchecked")
   public ControllerConfiguration<R> build() {
+    // todo: this should be abstracted by introducing an interface to deal with listening to
+    // namespace changes as possibly other things than the informers might be interested in reacting
+    // to such changes
     // propagate namespaces if needed
-    final List<DependentResourceSpec> newDependentSpecs;
+
     final var hasModifiedNamespaces = !original.getNamespaces().equals(namespaces);
-    newDependentSpecs = namedDependentResourceSpecs.entrySet().stream()
-        .map(drsEntry -> {
-          final var spec = drsEntry.getValue();
-
-          // if the spec has a config and it's a KubernetesDependentResourceConfig, update the
-          // namespaces if needed, otherwise, just return the existing spec
-          final Optional<?> maybeConfig = spec.getDependentResourceConfiguration();
-          return maybeConfig.filter(KubernetesDependentResourceConfig.class::isInstance)
-              .map(KubernetesDependentResourceConfig.class::cast)
-              .filter(Predicate.not(KubernetesDependentResourceConfig::wereNamespacesConfigured))
-              .map(c -> updateSpec(drsEntry.getKey(), spec, c))
-              .orElse(drsEntry.getValue());
-        }).collect(Collectors.toUnmodifiableList());
+    final var newDependentSpecs = namedDependentResourceSpecs.values().stream()
+        .map(spec -> {
+          // if the dependent resource has a config and it's a KubernetesDependentResourceConfig,
+          // update
+          // the namespaces if needed, otherwise, do nothing
+          DependentResource dependent = spec.getDependentResource();
+          Optional<DependentResourceSpec> updated = Optional.empty();
+          if (hasModifiedNamespaces && dependent instanceof DependentResourceConfigurator) {
+            DependentResourceConfigurator configurator = (DependentResourceConfigurator) dependent;
+            final Optional<?> config = configurator.configuration();
+            updated = config.filter(KubernetesDependentResourceConfig.class::isInstance)
+                .map(KubernetesDependentResourceConfig.class::cast)
+                .filter(Predicate.not(KubernetesDependentResourceConfig::wereNamespacesConfigured))
+                .map(c -> {
+                  // update the namespaces of the config, configure the dependent with it and update
+                  // the spec
+                  c.setNamespaces(namespaces);
+                  configurator.configureWith(c);
+                  return new DependentResourceSpec(dependent, spec.getName(), spec.getDependsOn(),
+                      spec.getReadyCondition(), spec.getReconcileCondition(),
+                      spec.getDeletePostCondition());
+                });
+          }
+
+          return updated.orElse(spec);
+        }).collect(Collectors.toList());
 
     return new DefaultControllerConfiguration<>(
         original.getAssociatedReconcilerClassName(),
@@ -215,14 +233,6 @@ public ControllerConfiguration<R> build() {
         newDependentSpecs);
   }
 
-  @SuppressWarnings({"rawtypes", "unchecked"})
-  private DependentResourceSpec<?, ?> updateSpec(String name, DependentResourceSpec spec,
-      KubernetesDependentResourceConfig c) {
-    return new DependentResourceSpec(spec.getDependentResourceClass(),
-        c.setNamespaces(namespaces), name, spec.getDependsOn(), spec.getReadyCondition(),
-        spec.getReconcileCondition(), spec.getDeletePostCondition());
-  }
-
   public static <R extends HasMetadata> ControllerConfigurationOverrider<R> override(
       ControllerConfiguration<R> original) {
     return new ControllerConfigurationOverrider<>(original);

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

@@ -4,14 +4,14 @@
 import java.util.Optional;
 import java.util.Set;
 
+import io.fabric8.kubernetes.api.model.HasMetadata;
 import io.javaoperatorsdk.operator.api.reconciler.dependent.DependentResource;
+import io.javaoperatorsdk.operator.api.reconciler.dependent.managed.DependentResourceConfigurator;
 import io.javaoperatorsdk.operator.processing.dependent.workflow.Condition;
 
-public class DependentResourceSpec<T extends DependentResource<?, ?>, C> {
+public class DependentResourceSpec<R, P extends HasMetadata, C> {
 
-  private final Class<T> dependentResourceClass;
-
-  private final C dependentResourceConfig;
+  private final DependentResource<R, P> dependentResource;
 
   private final String name;
 
@@ -23,24 +23,29 @@ public class DependentResourceSpec<T extends DependentResource<?, ?>, C> {
 
   private final Condition<?, ?> deletePostCondition;
 
-  public DependentResourceSpec(Class<T> dependentResourceClass, C dependentResourceConfig,
+  public DependentResourceSpec(DependentResource<R, P> dependentResource,
       String name, Set<String> dependsOn, Condition<?, ?> readyCondition,
       Condition<?, ?> reconcileCondition, Condition<?, ?> deletePostCondition) {
-    this.dependentResourceClass = dependentResourceClass;
-    this.dependentResourceConfig = dependentResourceConfig;
+    this.dependentResource = dependentResource;
     this.name = name;
     this.dependsOn = dependsOn;
     this.readyCondition = readyCondition;
     this.reconcileCondition = reconcileCondition;
     this.deletePostCondition = deletePostCondition;
   }
 
-  public Class<T> getDependentResourceClass() {
-    return dependentResourceClass;
+  @SuppressWarnings("unchecked")
+  public Class<DependentResource<R, P>> getDependentResourceClass() {
+    return (Class<DependentResource<R, P>>) dependentResource.getClass();
   }
 
+  @SuppressWarnings({"unchecked", "rawtypes"})
   public Optional<C> getDependentResourceConfiguration() {
-    return Optional.ofNullable(dependentResourceConfig);
+    if (dependentResource instanceof DependentResourceConfigurator) {
+      var configurator = (DependentResourceConfigurator) dependentResource;
+      return configurator.configuration();
+    }
+    return Optional.empty();
   }
 
   public String getName() {
@@ -50,8 +55,7 @@ public String getName() {
   @Override
   public String toString() {
     return "DependentResourceSpec{ name='" + name +
-        "', type=" + dependentResourceClass.getCanonicalName() +
-        ", config=" + dependentResourceConfig + '}';
+        "', type=" + getDependentResourceClass().getCanonicalName() + '}';
   }
 
   @Override
@@ -62,7 +66,7 @@ public boolean equals(Object o) {
     if (o == null || getClass() != o.getClass()) {
       return false;
     }
-    DependentResourceSpec<?, ?> that = (DependentResourceSpec<?, ?>) o;
+    DependentResourceSpec<?, ?, ?> that = (DependentResourceSpec<?, ?, ?>) o;
     return name.equals(that.name);
   }
 
@@ -89,4 +93,8 @@ public Condition getReconcileCondition() {
   public Condition getDeletePostCondition() {
     return deletePostCondition;
   }
+
+  public DependentResource<R, P> getDependentResource() {
+    return dependentResource;
+  }
 }

operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/reconciler/dependent/DependentResourceFactory.java

@@ -1,23 +1,18 @@
 package io.javaoperatorsdk.operator.api.reconciler.dependent;
 
-import java.lang.reflect.InvocationTargetException;
-
+import io.javaoperatorsdk.operator.api.config.Utils;
 import io.javaoperatorsdk.operator.api.config.dependent.DependentResourceSpec;
 
+@Deprecated
+@SuppressWarnings({"rawtypes", "unchecked"})
 public interface DependentResourceFactory {
 
-  default <T extends DependentResource<?, ?>> T createFrom(DependentResourceSpec<T, ?> spec) {
+  default DependentResource createFrom(DependentResourceSpec spec) {
     return createFrom(spec.getDependentResourceClass());
   }
 
-  default <T extends DependentResource<?, ?>> T createFrom(Class<T> dependentResourceClass) {
-    try {
-      return dependentResourceClass.getConstructor().newInstance();
-    } catch (InstantiationException | NoSuchMethodException | IllegalAccessException
-        | InvocationTargetException e) {
-      throw new IllegalArgumentException("Cannot instantiate DependentResource "
-          + dependentResourceClass.getCanonicalName(), e);
-    }
+  default <T extends DependentResource> T createFrom(Class<T> dependentResourceClass) {
+    return (T) Utils.instantiate(dependentResourceClass, DependentResource.class, null);
   }
 
 }

operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/reconciler/dependent/managed/AnnotationDependentResourceConfigurator.java

@@ -0,0 +1,11 @@
+package io.javaoperatorsdk.operator.api.reconciler.dependent.managed;
+
+import java.lang.annotation.Annotation;
+
+import io.javaoperatorsdk.operator.api.config.ControllerConfiguration;
+
+public interface AnnotationDependentResourceConfigurator<A extends Annotation, C>
+    extends DependentResourceConfigurator<C> {
+
+  C configFrom(A annotation, ControllerConfiguration<?> parentConfiguration);
+}

operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/reconciler/dependent/managed/DependentResourceConfigurator.java

@@ -1,5 +1,9 @@
 package io.javaoperatorsdk.operator.api.reconciler.dependent.managed;
 
+import java.util.Optional;
+
 public interface DependentResourceConfigurator<C> {
   void configureWith(C config);
+
+  Optional<C> configuration();
 }