improve: dependent configuration improvements - context independent (#…

…2389) Signed-off-by: Attila Mészáros <[email protected]> Signed-off-by: Chris Laprun <[email protected]> Co-authored-by: Chris Laprun <[email protected]>
operator-framework · Aug 29, 2024 · 04f66a6 · 04f66a6
1 parent 74b0e54
commit 04f66a6
Show file tree

Hide file tree

Showing 102 changed files with 1,067 additions and 901 deletions.
diff --git a/...javaoperatorsdk/operator/processing/event/source/cache/sample/AbstractTestReconciler.java b/...javaoperatorsdk/operator/processing/event/source/cache/sample/AbstractTestReconciler.java
@@ -77,7 +77,7 @@ public List<EventSource> prepareEventSources(
         boundedItemStore(new KubernetesClientBuilder().build(),
             ConfigMap.class, Duration.ofMinutes(1), 1); // setting max size for testing purposes
 
-    var es = new InformerEventSource<>(InformerConfiguration.from(ConfigMap.class, context)
+    var es = new InformerEventSource<>(InformerConfiguration.from(ConfigMap.class, primaryClass())
         .withItemStore(boundedItemStore)
         .withSecondaryToPrimaryMapper(
             Mappers.fromOwnerReferences(context.getPrimaryResourceClass(),
@@ -104,4 +104,7 @@ public static <R extends HasMetadata> BoundedItemStore<R> boundedItemStore(
         .build();
     return CaffeineBoundedItemStores.boundedItemStore(client, rClass, cache);
   }
+
+  protected abstract Class<P> primaryClass();
+
 }
diff --git a/...essing/event/source/cache/sample/clusterscope/BoundedCacheClusterScopeTestReconciler.java b/...essing/event/source/cache/sample/clusterscope/BoundedCacheClusterScopeTestReconciler.java
@@ -7,4 +7,8 @@
 public class BoundedCacheClusterScopeTestReconciler extends
     AbstractTestReconciler<BoundedCacheClusterScopeTestCustomResource> {
 
+  @Override
+  protected Class<BoundedCacheClusterScopeTestCustomResource> primaryClass() {
+    return BoundedCacheClusterScopeTestCustomResource.class;
+  }
 }
diff --git a/...rator/processing/event/source/cache/sample/namespacescope/BoundedCacheTestReconciler.java b/...rator/processing/event/source/cache/sample/namespacescope/BoundedCacheTestReconciler.java
@@ -7,4 +7,8 @@
 public class BoundedCacheTestReconciler
     extends AbstractTestReconciler<BoundedCacheTestCustomResource> {
 
+  @Override
+  protected Class<BoundedCacheTestCustomResource> primaryClass() {
+    return BoundedCacheTestCustomResource.class;
+  }
 }
diff --git a/docs/documentation/v5-0-migration.md b/docs/documentation/v5-0-migration.md
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
@@ -17,6 +17,7 @@
 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;
 import io.javaoperatorsdk.operator.api.config.dependent.DependentResourceSpec;
 import io.javaoperatorsdk.operator.api.config.workflow.WorkflowSpec;
 import io.javaoperatorsdk.operator.api.reconciler.Constants;
@@ -194,7 +195,7 @@ public boolean handleExceptionsInReconciler() {
   @SuppressWarnings({"unchecked", "rawtypes"})
   private static List<DependentResourceSpec> dependentResources(
       Workflow annotation,
-      ControllerConfiguration<?> parent) {
+      ControllerConfiguration<?> controllerConfiguration) {
     final var dependents = annotation.dependents();
 
 
@@ -213,7 +214,7 @@ private static List<DependentResourceSpec> dependentResources(
             "A DependentResource named '" + dependentName + "' already exists: " + spec);
       }
 
-      final var name = parent.getName();
+      final var name = controllerConfiguration.getName();
 
       var eventSourceName = dependent.useEventSourceWithName();
       eventSourceName = Constants.NO_VALUE_SET.equals(eventSourceName) ? null : eventSourceName;
@@ -225,6 +226,12 @@ private static List<DependentResourceSpec> dependentResources(
           Utils.instantiate(dependent.deletePostcondition(), Condition.class, context),
           Utils.instantiate(dependent.activationCondition(), Condition.class, context),
           eventSourceName);
+
+      // extract potential configuration
+      DependentResourceConfigurationResolver.configureSpecFromConfigured(spec,
+          controllerConfiguration,
+          dependentType);
+
       specsMap.put(dependentName, spec);
     }
     return specsMap.values().stream().toList();

diff --git a/...rk-core/src/main/java/io/javaoperatorsdk/operator/api/config/ControllerConfiguration.java b/...rk-core/src/main/java/io/javaoperatorsdk/operator/api/config/ControllerConfiguration.java
@@ -6,6 +6,7 @@
 
 import io.fabric8.kubernetes.api.model.HasMetadata;
 import io.javaoperatorsdk.operator.ReconcilerUtils;
+import io.javaoperatorsdk.operator.api.config.dependent.DependentResourceSpec;
 import io.javaoperatorsdk.operator.api.config.workflow.WorkflowSpec;
 import io.javaoperatorsdk.operator.api.reconciler.MaxReconciliationInterval;
 import io.javaoperatorsdk.operator.processing.event.rate.LinearRateLimiter;
@@ -71,7 +72,6 @@ default Optional<Duration> maxReconciliationInterval() {
     return Optional.of(Duration.ofHours(MaxReconciliationInterval.DEFAULT_INTERVAL));
   }
 
-  @SuppressWarnings("unused")
   ConfigurationService getConfigurationService();
 
   @SuppressWarnings("unchecked")
@@ -86,7 +86,7 @@ default Class<P> getResourceClass() {
 
   @SuppressWarnings("unused")
   default Set<String> getEffectiveNamespaces() {
-    return ResourceConfiguration.super.getEffectiveNamespaces(getConfigurationService());
+    return ResourceConfiguration.super.getEffectiveNamespaces(this);
   }
 
   /**
@@ -100,4 +100,5 @@ default String fieldManager() {
     return getName();
   }
 
+  <C> C getConfigurationFor(DependentResourceSpec<?, P, C> spec);
 }
diff --git a/...rc/main/java/io/javaoperatorsdk/operator/api/config/ControllerConfigurationOverrider.java b/...rc/main/java/io/javaoperatorsdk/operator/api/config/ControllerConfigurationOverrider.java
@@ -19,7 +19,7 @@
 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"})
+@SuppressWarnings({"rawtypes", "unused", "UnusedReturnValue"})
 public class ControllerConfigurationOverrider<R extends HasMetadata> {
 
   private String finalizer;

diff --git a/...src/main/java/io/javaoperatorsdk/operator/api/config/ResolvedControllerConfiguration.java b/...src/main/java/io/javaoperatorsdk/operator/api/config/ResolvedControllerConfiguration.java
@@ -9,7 +9,6 @@
 
 import io.fabric8.kubernetes.api.model.HasMetadata;
 import io.fabric8.kubernetes.client.informers.cache.ItemStore;
-import io.javaoperatorsdk.operator.api.config.dependent.DependentResourceConfigurationProvider;
 import io.javaoperatorsdk.operator.api.config.dependent.DependentResourceSpec;
 import io.javaoperatorsdk.operator.api.config.workflow.WorkflowSpec;
 import io.javaoperatorsdk.operator.api.reconciler.Reconciler;
@@ -22,8 +21,7 @@
 @SuppressWarnings("rawtypes")
 public class ResolvedControllerConfiguration<P extends HasMetadata>
     extends DefaultResourceConfiguration<P>
-    implements io.javaoperatorsdk.operator.api.config.ControllerConfiguration<P>,
-    DependentResourceConfigurationProvider {
+    implements io.javaoperatorsdk.operator.api.config.ControllerConfiguration<P> {
 
   private final String name;
   private final boolean generationAware;
@@ -168,8 +166,15 @@ public ConfigurationService getConfigurationService() {
   }
 
   @Override
-  public Object getConfigurationFor(DependentResourceSpec spec) {
-    return configurations.get(spec);
+  @SuppressWarnings("unchecked")
+  public <C> C getConfigurationFor(DependentResourceSpec<?, P, C> spec) {
+    // first check if there's an overridden configuration at the controller level
+    var config = configurations.get(spec);
+    if (config == null) {
+      // if not, check the spec for configuration
+      config = spec.getConfiguration().orElse(null);
+    }
+    return (C) config;
   }
 
   @Override

diff --git a/...work-core/src/main/java/io/javaoperatorsdk/operator/api/config/ResourceConfiguration.java b/...work-core/src/main/java/io/javaoperatorsdk/operator/api/config/ResourceConfiguration.java
@@ -111,11 +111,12 @@ static Set<String> ensureValidNamespaces(Collection<String> namespaces) {
    *
    * @return a Set of namespace names the associated controller will watch
    */
-  default Set<String> getEffectiveNamespaces(ConfigurationService configurationService) {
+  default Set<String> getEffectiveNamespaces(ControllerConfiguration<?> controllerConfiguration) {
     var targetNamespaces = getNamespaces();
     if (watchCurrentNamespace()) {
       final String namespace =
-          configurationService.getKubernetesClient().getConfiguration().getNamespace();
+          controllerConfiguration.getConfigurationService().getKubernetesClient().getConfiguration()
+              .getNamespace();
       if (namespace == null) {
         throw new OperatorException(
             "Couldn't retrieve the currently connected namespace. Make sure it's correctly set in your ~/.kube/config file, using, e.g. 'kubectl config set-context <your context> --namespace=<your namespace>'");

diff --git a/...rc/main/java/io/javaoperatorsdk/operator/api/config/dependent/ConfigurationConverter.java b/...rc/main/java/io/javaoperatorsdk/operator/api/config/dependent/ConfigurationConverter.java
@@ -3,10 +3,9 @@
 import java.lang.annotation.Annotation;
 
 import io.javaoperatorsdk.operator.api.config.ControllerConfiguration;
-import io.javaoperatorsdk.operator.api.reconciler.dependent.managed.DependentResourceConfigurator;
 
-public interface ConfigurationConverter<A extends Annotation, C, D extends DependentResourceConfigurator<? extends C>> {
+public interface ConfigurationConverter<A extends Annotation, C> {
 
-  C configFrom(A configAnnotation, ControllerConfiguration<?> parentConfiguration,
-      Class<D> originatingClass);
+  C configFrom(A configAnnotation, DependentResourceSpec<?, ?, C> spec,
+      ControllerConfiguration<?> parentConfiguration);
 }
diff --git a/...javaoperatorsdk/operator/api/config/dependent/DependentResourceConfigurationResolver.java b/...javaoperatorsdk/operator/api/config/dependent/DependentResourceConfigurationResolver.java
@@ -8,7 +8,6 @@
 import io.javaoperatorsdk.operator.api.config.ControllerConfiguration;
 import io.javaoperatorsdk.operator.api.config.Utils;
 import io.javaoperatorsdk.operator.api.reconciler.dependent.DependentResource;
-import io.javaoperatorsdk.operator.api.reconciler.dependent.managed.DependentResourceConfigurator;
 
 @SuppressWarnings({"rawtypes", "unchecked"})
 public class DependentResourceConfigurationResolver {
@@ -20,39 +19,18 @@ private DependentResourceConfigurationResolver() {}
   private static final Map<Class<? extends ConfigurationConverter>, ConfigurationConverter> knownConverters =
       new HashMap<>();
 
-  public static <C extends ControllerConfiguration<? extends HasMetadata>> void configure(
-      DependentResource dependentResource, DependentResourceSpec spec, C parentConfiguration) {
-    if (dependentResource instanceof DependentResourceConfigurator configurator) {
-      final var config = configurationFor(spec, parentConfiguration);
-      configurator.configureWith(config);
-    }
-  }
-
-  public static <C extends ControllerConfiguration<? extends HasMetadata>> Object configurationFor(
-      DependentResourceSpec spec, C parentConfiguration) {
 
-    // first check if the parent configuration has potentially already resolved the configuration
-    if (parentConfiguration instanceof DependentResourceConfigurationProvider provider) {
-      final var configuration = provider.getConfigurationFor(spec);
-      if (configuration != null) {
-        return configuration;
-      }
-    }
-
-    // find Configured-annotated class if it exists
-    return extractConfigurationFromConfigured(spec.getDependentResourceClass(),
-        parentConfiguration);
-  }
-
-  public static <C extends ControllerConfiguration<? extends HasMetadata>> Object extractConfigurationFromConfigured(
-      Class<? extends DependentResource> dependentResourceClass, C parentConfiguration) {
+  public static <C extends ControllerConfiguration<?>> void configureSpecFromConfigured(
+      DependentResourceSpec spec,
+      C parentConfiguration,
+      Class<? extends DependentResource> dependentResourceClass) {
     var converterAnnotationPair = converters.get(dependentResourceClass);
 
     Annotation configAnnotation;
     if (converterAnnotationPair == null) {
       var configuredClassPair = getConfigured(dependentResourceClass);
       if (configuredClassPair == null) {
-        return null;
+        return;
       }
 
       // check if we already have a converter registered for the found Configured annotated class
@@ -76,7 +54,8 @@ public static <C extends ControllerConfiguration<? extends HasMetadata>> Object
 
     // always called even if the annotation is null so that implementations can provide default
     // values
-    return converter.configFrom(configAnnotation, parentConfiguration, dependentResourceClass);
+    final var config = converter.configFrom(configAnnotation, spec, parentConfiguration);
+    spec.setNullableConfiguration(config);
   }
 
   private static ConfiguredClassPair getConfigured(

diff --git a/...src/main/java/io/javaoperatorsdk/operator/api/config/dependent/DependentResourceSpec.java b/...src/main/java/io/javaoperatorsdk/operator/api/config/dependent/DependentResourceSpec.java
@@ -8,23 +8,17 @@
 import io.javaoperatorsdk.operator.api.reconciler.dependent.DependentResource;
 import io.javaoperatorsdk.operator.processing.dependent.workflow.Condition;
 
-public class DependentResourceSpec<R, P extends HasMetadata> {
+public class DependentResourceSpec<R, P extends HasMetadata, C> {
 
   private final Class<? extends DependentResource<R, P>> dependentResourceClass;
-
   private final String name;
-
   private final Set<String> dependsOn;
-
   private final Condition<?, ?> readyCondition;
-
   private final Condition<?, ?> reconcileCondition;
-
   private final Condition<?, ?> deletePostCondition;
-
   private final Condition<?, ?> activationCondition;
-
   private final String useEventSourceWithName;
+  private C nullableConfiguration;
 
   public DependentResourceSpec(Class<? extends DependentResource<R, P>> dependentResourceClass,
       String name, Set<String> dependsOn, Condition<?, ?> readyCondition,
@@ -62,7 +56,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);
   }
 
@@ -98,4 +92,13 @@ public Condition getActivationCondition() {
   public Optional<String> getUseEventSourceWithName() {
     return Optional.ofNullable(useEventSourceWithName);
   }
+
+  public Optional<C> getConfiguration() {
+    return Optional.ofNullable(nullableConfiguration);
+  }
+
+  protected void setNullableConfiguration(C configuration) {
+    this.nullableConfiguration = configuration;
+  }
+
 }