fix: make sure a DependentResourceSpec is always created complete (#1391)

Author: metacosm

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

@@ -277,14 +277,7 @@ public List<DependentResourceSpec> getDependentResources() {
 
   private Condition<?, ?> instantiateConditionIfNotVoid(Class<? extends Condition> condition) {
     if (condition != VoidCondition.class) {
-      try {
-        return condition.getDeclaredConstructor().newInstance();
-      } catch (InstantiationException
-          | IllegalAccessException
-          | InvocationTargetException
-          | NoSuchMethodException e) {
-        throw new OperatorException(e);
-      }
+      return instantiateAndConfigureIfNeeded(condition, Condition.class);
     }
     return null;
   }

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

@@ -172,7 +172,9 @@ public ControllerConfigurationOverrider<R> replacingNamedDependentResourceConfig
 
   private void replaceConfig(String name, Object newConfig, DependentResourceSpec<?, ?> current) {
     namedDependentResourceSpecs.put(name,
-        new DependentResourceSpec<>(current.getDependentResourceClass(), newConfig, name));
+        new DependentResourceSpec<>(current.getDependentResourceClass(), newConfig, name,
+            current.getDependsOn(), current.getReadyCondition(), current.getReconcileCondition(),
+            current.getDeletePostCondition()));
   }
 
   @SuppressWarnings("unchecked")

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

@@ -1,6 +1,5 @@
 package io.javaoperatorsdk.operator.api.config.dependent;
 
-import java.util.Collections;
 import java.util.Objects;
 import java.util.Optional;
 import java.util.Set;
@@ -24,12 +23,6 @@ public class DependentResourceSpec<T extends DependentResource<?, ?>, C> {
 
   private final Condition<?, ?> deletePostCondition;
 
-  public DependentResourceSpec(Class<T> dependentResourceClass, C dependentResourceConfig,
-      String name) {
-    this(dependentResourceClass, dependentResourceConfig, name, Collections.emptySet(), null, null,
-        null);
-  }
-
   public DependentResourceSpec(Class<T> dependentResourceClass, C dependentResourceConfig,
       String name, Set<String> dependsOn, Condition<?, ?> readyCondition,
       Condition<?, ?> reconcileCondition, Condition<?, ?> deletePostCondition) {

operator-framework-core/src/test/java/io/javaoperatorsdk/operator/api/config/ControllerConfigurationOverriderTest.java

@@ -17,6 +17,7 @@
 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.processing.dependent.workflow.Condition;
 
 import static org.junit.jupiter.api.Assertions.assertEquals;
 import static org.junit.jupiter.api.Assertions.assertFalse;
@@ -91,6 +92,7 @@ public Optional<Object> getSecondaryResource(ConfigMap primary) {
     }
   }
 
+  @SuppressWarnings("rawtypes")
   private KubernetesDependentResourceConfig extractFirstDependentKubernetesResourceConfig(
       io.javaoperatorsdk.operator.api.config.ControllerConfiguration<?> configuration) {
     return (KubernetesDependentResourceConfig) extractDependentKubernetesResourceConfig(
@@ -191,6 +193,7 @@ void configuredDependentShouldNotChangeOnParentOverrideEvenWhenInitialConfigIsSa
     assertEquals(Set.of(OverriddenNSDependent.DEP_NS), config.namespaces());
   }
 
+  @SuppressWarnings("unchecked")
   @Test
   void dependentShouldWatchAllNamespacesIfParentDoesAsWell() {
     var configuration = createConfiguration(new WatchAllNamespacesReconciler());
@@ -211,6 +214,7 @@ void dependentShouldWatchAllNamespacesIfParentDoesAsWell() {
     assertEquals(Set.of(newNS), config.namespaces());
   }
 
+  @SuppressWarnings("unchecked")
   @Test
   void shouldBePossibleToForceDependentToWatchAllNamespaces() {
     var configuration = createConfiguration(new DependentWatchesAllNSReconciler());
@@ -272,6 +276,7 @@ void alreadyOverriddenDependentNamespacesShouldNotBePropagated() {
     assertEquals(Set.of(OverriddenNSDependent.DEP_NS), config.namespaces());
   }
 
+  @SuppressWarnings({"rawtypes", "unchecked"})
   @Test
   void replaceNamedDependentResourceConfigShouldWork() {
     var configuration = createConfiguration(new OneDepReconciler());
@@ -284,7 +289,7 @@ void replaceNamedDependentResourceConfigShouldWork() {
 
     var dependentSpec = dependents.stream()
         .filter(dr -> dr.getName().equals(dependentResourceName))
-        .findFirst().get();
+        .findFirst().orElseThrow();
     assertEquals(ReadOnlyDependent.class, dependentSpec.getDependentResourceClass());
     var maybeConfig = dependentSpec.getDependentResourceConfiguration();
     assertTrue(maybeConfig.isPresent());
@@ -306,12 +311,14 @@ void replaceNamedDependentResourceConfigShouldWork() {
         .build();
     dependents = overridden.getDependentResources();
     dependentSpec = dependents.stream().filter(dr -> dr.getName().equals(dependentResourceName))
-        .findFirst().get();
+        .findFirst().orElseThrow();
     config = (KubernetesDependentResourceConfig) dependentSpec.getDependentResourceConfiguration()
         .orElseThrow();
     assertEquals(1, config.namespaces().size());
     assertEquals(labelSelector, config.labelSelector());
     assertEquals(Set.of(overriddenNS), config.namespaces());
+    // check that we still have the proper workflow configuration
+    assertTrue(dependentSpec.getReadyCondition() instanceof TestCondition);
   }
 
   @ControllerConfiguration(dependents = @Dependent(type = ReadOnlyDependent.class))
@@ -332,8 +339,18 @@ public UpdateControl<ConfigMap> reconcile(ConfigMap resource, Context<ConfigMap>
     }
   }
 
+  private static class TestCondition implements Condition<ConfigMap, ConfigMap> {
+
+    @Override
+    public boolean isMet(DependentResource<ConfigMap, ConfigMap> dependentResource,
+        ConfigMap primary, Context<ConfigMap> context) {
+      return true;
+    }
+  }
+
   @ControllerConfiguration(namespaces = OneDepReconciler.CONFIGURED_NS,
-      dependents = @Dependent(type = ReadOnlyDependent.class))
+      dependents = @Dependent(type = ReadOnlyDependent.class,
+          readyPostcondition = TestCondition.class))
   private static class OneDepReconciler implements Reconciler<ConfigMap> {
 
     private static final String CONFIGURED_NS = "foo";