feat: handle clustered resource on secondary to primary mapper init (#2321)

Fixes #2311

Overriding getPrimaryResourceType should allow to make things work even
in deeper hierarchies.

Signed-off-by: Chris Laprun <[email protected]>
Signed-off-by: Attila Mészáros <[email protected]>

Author: metacosm

Diff for: operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/Utils.java

@@ -103,21 +103,22 @@ static boolean getBooleanFromSystemPropsOrDefault(String propertyName, boolean d
       return defaultValue;
     } else {
       property = property.trim().toLowerCase();
-      switch (property) {
-        case "true":
-          return true;
-        case "false":
-          return false;
-        default:
-          return defaultValue;
-      }
+      return switch (property) {
+        case "true" -> true;
+        case "false" -> false;
+        default -> defaultValue;
+      };
     }
   }
 
   public static Class<?> getFirstTypeArgumentFromExtendedClass(Class<?> clazz) {
+    return getTypeArgumentFromExtendedClassByIndex(clazz, 0);
+  }
+
+  public static Class<?> getTypeArgumentFromExtendedClassByIndex(Class<?> clazz, int index) {
     try {
       Type type = clazz.getGenericSuperclass();
-      return (Class<?>) ((ParameterizedType) type).getActualTypeArguments()[0];
+      return (Class<?>) ((ParameterizedType) type).getActualTypeArguments()[index];
     } catch (Exception e) {
       throw new RuntimeException(GENERIC_PARAMETER_TYPE_ERROR_PREFIX
           + clazz.getSimpleName()
@@ -186,27 +187,31 @@ private static Optional<? extends Class<?>> extractType(Class<?> clazz,
 
   public static Class<?> getFirstTypeArgumentFromSuperClassOrInterface(Class<?> clazz,
       Class<?> expectedImplementedInterface) {
+    return getTypeArgumentFromSuperClassOrInterfaceByIndex(clazz, expectedImplementedInterface, 0);
+  }
+
+  public static Class<?> getTypeArgumentFromSuperClassOrInterfaceByIndex(Class<?> clazz,
+      Class<?> expectedImplementedInterface, int index) {
     // first check super class if it exists
     try {
       final Class<?> superclass = clazz.getSuperclass();
       if (!superclass.equals(Object.class)) {
         try {
-          return getFirstTypeArgumentFromExtendedClass(clazz);
+          return getTypeArgumentFromExtendedClassByIndex(clazz, index);
         } catch (Exception e) {
           // try interfaces
           try {
-            return getFirstTypeArgumentFromInterface(clazz, expectedImplementedInterface);
+            return getTypeArgumentFromInterfaceByIndex(clazz, expectedImplementedInterface, index);
           } catch (Exception ex) {
             // try on the parent
-            return getFirstTypeArgumentFromSuperClassOrInterface(superclass,
-                expectedImplementedInterface);
+            return getTypeArgumentFromSuperClassOrInterfaceByIndex(superclass,
+                expectedImplementedInterface, index);
           }
         }
       }
-      return getFirstTypeArgumentFromInterface(clazz, expectedImplementedInterface);
+      return getTypeArgumentFromInterfaceByIndex(clazz, expectedImplementedInterface, index);
     } catch (Exception e) {
-      throw new OperatorException(
-          GENERIC_PARAMETER_TYPE_ERROR_PREFIX + clazz.getSimpleName(), e);
+      throw new OperatorException(GENERIC_PARAMETER_TYPE_ERROR_PREFIX + clazz.getSimpleName(), e);
     }
   }
 

Diff for: operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/kubernetes/GenericKubernetesDependentResource.java

@@ -2,6 +2,7 @@
 
 import io.fabric8.kubernetes.api.model.GenericKubernetesResource;
 import io.fabric8.kubernetes.api.model.HasMetadata;
+import io.javaoperatorsdk.operator.api.config.Utils;
 import io.javaoperatorsdk.operator.api.config.informer.InformerConfiguration;
 import io.javaoperatorsdk.operator.processing.GroupVersionKind;
 
@@ -23,8 +24,14 @@ protected InformerConfiguration.InformerConfigurationBuilder<GenericKubernetesRe
     return InformerConfiguration.from(groupVersionKind);
   }
 
+  @SuppressWarnings("unchecked")
+  @Override
+  protected Class<P> getPrimaryResourceType() {
+    return (Class<P>) Utils.getFirstTypeArgumentFromExtendedClass(getClass());
+  }
+
   @SuppressWarnings("unused")
-  public GroupVersionKindPlural getGroupVersionKind() {
+  public GroupVersionKind getGroupVersionKind() {
     return groupVersionKind;
   }
 }

Diff for: operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/kubernetes/KubernetesDependentResource.java

@@ -9,9 +9,11 @@
 import org.slf4j.LoggerFactory;
 
 import io.fabric8.kubernetes.api.model.HasMetadata;
+import io.fabric8.kubernetes.api.model.Namespaced;
 import io.fabric8.kubernetes.client.dsl.Resource;
 import io.javaoperatorsdk.operator.OperatorException;
 import io.javaoperatorsdk.operator.ReconcilerUtils;
+import io.javaoperatorsdk.operator.api.config.Utils;
 import io.javaoperatorsdk.operator.api.config.dependent.Configured;
 import io.javaoperatorsdk.operator.api.config.informer.InformerConfiguration;
 import io.javaoperatorsdk.operator.api.reconciler.Constants;
@@ -36,8 +38,12 @@ public abstract class KubernetesDependentResource<R extends HasMetadata, P exten
     implements DependentResourceConfigurator<KubernetesDependentResourceConfig<R>> {
 
   private static final Logger log = LoggerFactory.getLogger(KubernetesDependentResource.class);
-  private final ResourceUpdaterMatcher<R> updaterMatcher;
   private final boolean garbageCollected = this instanceof GarbageCollected;
+  @SuppressWarnings("unchecked")
+  private final ResourceUpdaterMatcher<R> updaterMatcher = this instanceof ResourceUpdaterMatcher
+      ? (ResourceUpdaterMatcher<R>) this
+      : GenericResourceUpdaterMatcher.updaterMatcherFor(resourceType());
+  private final boolean clustered;
   private KubernetesDependentResourceConfig<R> kubernetesDependentResourceConfig;
   private volatile Boolean useSSA;
 
@@ -46,16 +52,23 @@ public KubernetesDependentResource(Class<R> resourceType) {
     this(resourceType, null);
   }
 
-  @SuppressWarnings("unchecked")
   public KubernetesDependentResource(Class<R> resourceType, String name) {
     super(resourceType, name);
+    final var primaryResourceType = getPrimaryResourceType();
+    clustered = !Namespaced.class.isAssignableFrom(primaryResourceType);
+  }
 
-    updaterMatcher = this instanceof ResourceUpdaterMatcher
-        ? (ResourceUpdaterMatcher<R>) this
-        : GenericResourceUpdaterMatcher.updaterMatcherFor(resourceType);
+  protected KubernetesDependentResource(Class<R> resourceType, String name,
+      boolean primaryIsClustered) {
+    super(resourceType, name);
+    clustered = primaryIsClustered;
   }
 
   @SuppressWarnings("unchecked")
+  protected Class<P> getPrimaryResourceType() {
+    return (Class<P>) Utils.getTypeArgumentFromExtendedClassByIndex(getClass(), 1);
+  }
+
   @Override
   public void configureWith(KubernetesDependentResourceConfig<R> config) {
     this.kubernetesDependentResourceConfig = config;
@@ -87,7 +100,7 @@ private SecondaryToPrimaryMapper<R> getSecondaryToPrimaryMapper() {
     if (this instanceof SecondaryToPrimaryMapper) {
       return (SecondaryToPrimaryMapper<R>) this;
     } else if (garbageCollected) {
-      return Mappers.fromOwnerReferences(false);
+      return Mappers.fromOwnerReferences(clustered);
     } else if (useNonOwnerRefBasedSecondaryToPrimaryMapping()) {
       return Mappers.fromDefaultAnnotations();
     } else {

Diff for: operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/dependent/workflow/AbstractWorkflowExecutorTest.java

@@ -39,6 +39,11 @@ public TestDependent(String name) {
       super(ConfigMap.class, name);
     }
 
+    @Override
+    protected Class<TestCustomResource> getPrimaryResourceType() {
+      return TestCustomResource.class;
+    }
+
     @Override
     public ReconcileResult<ConfigMap> reconcile(TestCustomResource primary,
         Context<TestCustomResource> context) {

Diff for: operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/bulkdependent/ConfigMapDeleterBulkDependentResource.java

@@ -34,20 +34,24 @@ public ConfigMapDeleterBulkDependentResource() {
     super(ConfigMap.class);
   }
 
+  @Override
+  protected Class<BulkDependentTestCustomResource> getPrimaryResourceType() {
+    return BulkDependentTestCustomResource.class;
+  }
+
   @Override
   public Map<String, ConfigMap> desiredResources(BulkDependentTestCustomResource primary,
       Context<BulkDependentTestCustomResource> context) {
     var number = primary.getSpec().getNumberOfResources();
     Map<String, ConfigMap> res = new HashMap<>();
     for (int i = 0; i < number; i++) {
       var key = Integer.toString(i);
-      res.put(key, desired(primary, key, context));
+      res.put(key, desired(primary, key));
     }
     return res;
   }
 
-  public ConfigMap desired(BulkDependentTestCustomResource primary, String key,
-      Context<BulkDependentTestCustomResource> context) {
+  public ConfigMap desired(BulkDependentTestCustomResource primary, String key) {
     ConfigMap configMap = new ConfigMap();
     configMap.setMetadata(new ObjectMetaBuilder()
         .withName(primary.getMetadata().getName() + INDEX_DELIMITER + key)

Diff for: operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/complexdependent/dependent/BaseDependentResource.java

@@ -16,6 +16,11 @@ public BaseDependentResource(Class<R> resourceType, String component) {
     this.component = component;
   }
 
+  @Override
+  protected Class<ComplexDependentCustomResource> getPrimaryResourceType() {
+    return ComplexDependentCustomResource.class;
+  }
+
   protected String name(ComplexDependentCustomResource primary) {
     return String.format("%s-%s", component, primary.getSpec().getProjectId());
   }