Repository: quarkiverse/quarkus-operator-sdk (java)

Issue:
CRD Generator is not generating multiple custom resource versions in case one version is tied to a reconciler
Hi,

Fabric8 CRD generator supports multiple versions of the same kind:
https://github.com/fabric8io/kubernetes-client/blob/main/doc/CRD-generator.md#multiple-custom-resource-versions

This does not work for us. We have two versions (v1 and v2) of the same custom resource. We set [quarkus.operator-sdk.crd.generate-all](https://docs.quarkiverse.io/quarkus-operator-sdk/dev/includes/quarkus-operator-sdk.html#quarkus-operator-sdk_quarkus-operator-sdk-crd-generate-all) to true. When no version is tied to a reconciler, both versions will be generated, but when we use one version (v2) by the reconciler, only this version is generated and the other (v1) is ignored.

Is this intended behavior or a bug?

note: Quarkus platform version is 3.10.2

Pull request description:
fix: record version with CRD name to properly output multiple versions
Also fixes an issue with improperly recorded reconciler name and
handling of generic kubernetes resources.

Fixes #886

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


Patch:
diff --git a/common-deployment/src/main/java/io/quarkiverse/operatorsdk/common/Constants.java b/common-deployment/src/main/java/io/quarkiverse/operatorsdk/common/Constants.java
index 18fec24f..a0ee10f9 100644
--- a/common-deployment/src/main/java/io/quarkiverse/operatorsdk/common/Constants.java
+++ b/common-deployment/src/main/java/io/quarkiverse/operatorsdk/common/Constants.java
@@ -10,6 +10,7 @@ import io.javaoperatorsdk.operator.api.reconciler.ControllerConfiguration;
 import io.javaoperatorsdk.operator.api.reconciler.Ignore;
 import io.javaoperatorsdk.operator.api.reconciler.Reconciler;
 import io.javaoperatorsdk.operator.api.reconciler.dependent.DependentResource;
+import io.javaoperatorsdk.operator.processing.dependent.kubernetes.GenericKubernetesDependentResource;
 import io.quarkiverse.operatorsdk.annotations.AdditionalRBACRoleRefs;
 import io.quarkiverse.operatorsdk.annotations.AdditionalRBACRules;
 import io.quarkiverse.operatorsdk.annotations.RBACCRoleRef;
@@ -25,6 +26,8 @@ public class Constants {
     public static final DotName HAS_METADATA = DotName.createSimple(HasMetadata.class.getName());
     public static final DotName CONTROLLER_CONFIGURATION = DotName.createSimple(ControllerConfiguration.class.getName());
     public static final DotName DEPENDENT_RESOURCE = DotName.createSimple(DependentResource.class.getName());
+    public static final DotName GENERIC_KUBERNETES_DEPENDENT_RESOURCE = DotName
+            .createSimple(GenericKubernetesDependentResource.class.getName());
     public static final DotName CONFIGURED = DotName.createSimple(Configured.class.getName());
     public static final DotName ANNOTATION_CONFIGURABLE = DotName.createSimple(AnnotationConfigurable.class.getName());
     public static final DotName OBJECT = DotName.createSimple(Object.class.getName());
diff --git a/common-deployment/src/main/java/io/quarkiverse/operatorsdk/common/CustomResourceAugmentedClassInfo.java b/common-deployment/src/main/java/io/quarkiverse/operatorsdk/common/CustomResourceAugmentedClassInfo.java
index f0a31c86..3478f65f 100644
--- a/common-deployment/src/main/java/io/quarkiverse/operatorsdk/common/CustomResourceAugmentedClassInfo.java
+++ b/common-deployment/src/main/java/io/quarkiverse/operatorsdk/common/CustomResourceAugmentedClassInfo.java
@@ -20,14 +20,12 @@ public class CustomResourceAugmentedClassInfo extends ReconciledResourceAugmente
 
     @Override
     protected boolean doKeep(IndexView index, Logger log, Map<String, Object> context) {
-
         // only keep the information if the associated CRD hasn't already been generated
-        final var fullName = fullResourceName();
         return Optional.ofNullable(context.get(EXISTING_CRDS_KEY))
                 .map(value -> {
                     @SuppressWarnings("unchecked")
                     Set<String> generated = (Set<String>) value;
-                    return !generated.contains(fullName);
+                    return !generated.contains(id());
                 })
                 .orElse(true);
     }
diff --git a/common-deployment/src/main/java/io/quarkiverse/operatorsdk/common/DependentResourceAugmentedClassInfo.java b/common-deployment/src/main/java/io/quarkiverse/operatorsdk/common/DependentResourceAugmentedClassInfo.java
index 9fbf286f..75d771cb 100644
--- a/common-deployment/src/main/java/io/quarkiverse/operatorsdk/common/DependentResourceAugmentedClassInfo.java
+++ b/common-deployment/src/main/java/io/quarkiverse/operatorsdk/common/DependentResourceAugmentedClassInfo.java
@@ -16,24 +16,26 @@ public class DependentResourceAugmentedClassInfo extends ResourceAssociatedAugme
     private final AnnotationInstance dependentAnnotationFromController;
 
     public DependentResourceAugmentedClassInfo(ClassInfo classInfo) {
-        this(classInfo, null);
+        this(classInfo, null, null);
     }
 
-    private DependentResourceAugmentedClassInfo(ClassInfo classInfo, AnnotationInstance dependentAnnotationFromController) {
+    private DependentResourceAugmentedClassInfo(ClassInfo classInfo, AnnotationInstance dependentAnnotationFromController,
+            String reconcilerName) {
         super(classInfo, DEPENDENT_RESOURCE, 2,
                 Optional.ofNullable(dependentAnnotationFromController)
                         .map(a -> a.value("name"))
                         .map(AnnotationValue::asString)
                         .filter(Predicate.not(String::isBlank))
                         // note that this should match DependentResource.getDefaultNameFor implementation)
-                        .orElse(classInfo.name().toString()));
+                        .orElse(classInfo.name().toString()),
+                reconcilerName);
         this.dependentAnnotationFromController = dependentAnnotationFromController;
     }
 
     public static DependentResourceAugmentedClassInfo createFor(ClassInfo classInfo,
             AnnotationInstance dependentAnnotationFromController, IndexView index, Logger log,
-            Map<String, Object> context) {
-        final var info = new DependentResourceAugmentedClassInfo(classInfo, dependentAnnotationFromController);
+            Map<String, Object> context, String reconcilerName) {
+        final var info = new DependentResourceAugmentedClassInfo(classInfo, dependentAnnotationFromController, reconcilerName);
         info.augmentIfKept(index, log, context);
         return info;
     }
diff --git a/common-deployment/src/main/java/io/quarkiverse/operatorsdk/common/ReconciledAugmentedClassInfo.java b/common-deployment/src/main/java/io/quarkiverse/operatorsdk/common/ReconciledAugmentedClassInfo.java
index 2416594b..7887bc43 100644
--- a/common-deployment/src/main/java/io/quarkiverse/operatorsdk/common/ReconciledAugmentedClassInfo.java
+++ b/common-deployment/src/main/java/io/quarkiverse/operatorsdk/common/ReconciledAugmentedClassInfo.java
@@ -1,9 +1,7 @@
 package io.quarkiverse.operatorsdk.common;
 
 import static io.quarkiverse.operatorsdk.common.ClassLoadingUtils.loadClass;
-import static io.quarkiverse.operatorsdk.common.Constants.CUSTOM_RESOURCE;
-import static io.quarkiverse.operatorsdk.common.Constants.HAS_METADATA;
-import static io.quarkiverse.operatorsdk.common.Constants.OBJECT;
+import static io.quarkiverse.operatorsdk.common.Constants.*;
 
 import java.util.Map;
 import java.util.Optional;
@@ -65,14 +63,21 @@ public class ReconciledAugmentedClassInfo<T> extends SelectiveAugmentedClassInfo
     }
 
     @SuppressWarnings("rawtypes")
-    public static ReconciledAugmentedClassInfo createFor(ClassInfo resourceCI, String reconcilerName,
+    public static ReconciledAugmentedClassInfo createFor(ResourceAssociatedAugmentedClassInfo parent, ClassInfo resourceCI,
+            String reconcilerName,
             IndexView index, Logger log, Map<String, Object> context) {
         var isResource = false;
         var isCR = false;
+        var isGenericKubernetesResource = false;
         try {
             isResource = ClassUtils.isImplementationOf(index, resourceCI, HAS_METADATA);
             if (isResource) {
                 isCR = JandexUtil.isSubclassOf(index, resourceCI, CUSTOM_RESOURCE);
+                if (!isCR) {
+                    // check if the target resource is a generic one
+                    isGenericKubernetesResource = JandexUtil.isSubclassOf(index, parent.classInfo(),
+                            GENERIC_KUBERNETES_DEPENDENT_RESOURCE);
+                }
             }
         } catch (BuildException e) {
             log.errorv(
@@ -83,7 +88,8 @@ public class ReconciledAugmentedClassInfo<T> extends SelectiveAugmentedClassInfo
         ReconciledAugmentedClassInfo reconciledInfo;
         if (isCR) {
             reconciledInfo = new CustomResourceAugmentedClassInfo(resourceCI, reconcilerName);
-        } else if (isResource) {
+        } else if (isResource && !isGenericKubernetesResource) {
+            // only record detailed information if the target resource is not generic
             reconciledInfo = new ReconciledResourceAugmentedClassInfo<>(resourceCI, HAS_METADATA, 0,
                     reconcilerName);
         } else {
diff --git a/common-deployment/src/main/java/io/quarkiverse/operatorsdk/common/ReconciledResourceAugmentedClassInfo.java b/common-deployment/src/main/java/io/quarkiverse/operatorsdk/common/ReconciledResourceAugmentedClassInfo.java
index 59148464..2754ea06 100644
--- a/common-deployment/src/main/java/io/quarkiverse/operatorsdk/common/ReconciledResourceAugmentedClassInfo.java
+++ b/common-deployment/src/main/java/io/quarkiverse/operatorsdk/common/ReconciledResourceAugmentedClassInfo.java
@@ -1,6 +1,7 @@
 package io.quarkiverse.operatorsdk.common;
 
 import java.util.Map;
+import java.util.Objects;
 
 import org.jboss.jandex.ClassInfo;
 import org.jboss.jandex.DotName;
@@ -13,12 +14,18 @@ public class ReconciledResourceAugmentedClassInfo<T extends HasMetadata> extends
 
     public static final String STATUS = "status";
     protected boolean hasStatus;
+    private final String id;
 
     protected ReconciledResourceAugmentedClassInfo(ClassInfo classInfo,
             DotName extendedOrImplementedClass, int expectedParameterTypesCardinality,
             String associatedReconcilerName) {
         super(classInfo, extendedOrImplementedClass, expectedParameterTypesCardinality,
                 associatedReconcilerName);
+        id = fullResourceName() + version();
+    }
+
+    public String id() {
+        return id;
     }
 
     public String fullResourceName() {
@@ -55,4 +62,16 @@ public class ReconciledResourceAugmentedClassInfo<T extends HasMetadata> extends
     public boolean hasNonVoidStatus() {
         return hasStatus;
     }
+
+    @Override
+    public boolean equals(Object o) {
+        if (!(o instanceof ReconciledResourceAugmentedClassInfo<?> that))
+            return false;
+        return Objects.equals(id, that.id);
+    }
+
+    @Override
+    public int hashCode() {
+        return Objects.hashCode(id);
+    }
 }
diff --git a/common-deployment/src/main/java/io/quarkiverse/operatorsdk/common/ReconcilerAugmentedClassInfo.java b/common-deployment/src/main/java/io/quarkiverse/operatorsdk/common/ReconcilerAugmentedClassInfo.java
index 6f29133c..39f31b43 100644
--- a/common-deployment/src/main/java/io/quarkiverse/operatorsdk/common/ReconcilerAugmentedClassInfo.java
+++ b/common-deployment/src/main/java/io/quarkiverse/operatorsdk/common/ReconcilerAugmentedClassInfo.java
@@ -61,7 +61,7 @@ public class ReconcilerAugmentedClassInfo extends ResourceAssociatedAugmentedCla
                             dependentConfig.value("type"),
                             DependentResource.class, index);
                     final var dependent = DependentResourceAugmentedClassInfo.createFor(dependentType, dependentConfig, index,
-                            log, context);
+                            log, context, nameOrFailIfUnset());
                     final var dependentName = dependent.nameOrFailIfUnset();
                     final var dependentTypeName = dependentType.name().toString();
                     if (dependentResources.containsKey(dependentName)) {
diff --git a/common-deployment/src/main/java/io/quarkiverse/operatorsdk/common/ResourceAssociatedAugmentedClassInfo.java b/common-deployment/src/main/java/io/quarkiverse/operatorsdk/common/ResourceAssociatedAugmentedClassInfo.java
index 093827df..026a41af 100644
--- a/common-deployment/src/main/java/io/quarkiverse/operatorsdk/common/ResourceAssociatedAugmentedClassInfo.java
+++ b/common-deployment/src/main/java/io/quarkiverse/operatorsdk/common/ResourceAssociatedAugmentedClassInfo.java
@@ -13,11 +13,18 @@ import org.jboss.logging.Logger;
 public class ResourceAssociatedAugmentedClassInfo extends SelectiveAugmentedClassInfo {
     private final String name;
     private ReconciledAugmentedClassInfo<?> resourceInfo;
+    private final String reconcilerName;
 
     protected ResourceAssociatedAugmentedClassInfo(ClassInfo classInfo,
             DotName extendedOrImplementedClass, int expectedParameterTypesCardinality, String name) {
+        this(classInfo, extendedOrImplementedClass, expectedParameterTypesCardinality, name, null);
+    }
+
+    protected ResourceAssociatedAugmentedClassInfo(ClassInfo classInfo,
+            DotName extendedOrImplementedClass, int expectedParameterTypesCardinality, String name, String reconcilerName) {
         super(classInfo, extendedOrImplementedClass, expectedParameterTypesCardinality);
         this.name = name;
+        this.reconcilerName = reconcilerName != null ? reconcilerName : name;
     }
 
     public DotName resourceTypeName() {
@@ -42,7 +49,7 @@ public class ResourceAssociatedAugmentedClassInfo extends SelectiveAugmentedClas
                     + "' has not been found in the Jandex index so it cannot be introspected. Please index your classes with Jandex.");
         }
 
-        resourceInfo = ReconciledAugmentedClassInfo.createFor(primaryCI, name, index, log, context);
+        resourceInfo = ReconciledAugmentedClassInfo.createFor(this, primaryCI, reconcilerName, index, log, context);
     }
 
     public ReconciledAugmentedClassInfo<?> associatedResourceInfo() {
diff --git a/core/deployment/src/main/java/io/quarkiverse/operatorsdk/deployment/CRDGeneration.java b/core/deployment/src/main/java/io/quarkiverse/operatorsdk/deployment/CRDGeneration.java
index 9cf6fdb7..f8b423e7 100644
--- a/core/deployment/src/main/java/io/quarkiverse/operatorsdk/deployment/CRDGeneration.java
+++ b/core/deployment/src/main/java/io/quarkiverse/operatorsdk/deployment/CRDGeneration.java
@@ -56,7 +56,7 @@ class CRDGeneration {
      * Generates the CRD in the location specified by the output target, using the specified CRD
      * generation configuration only if generation has been requested by call
      * {@link #scheduleForGenerationIfNeeded(CustomResourceAugmentedClassInfo, Map, Set)} or
-     * {@link #withCustomResource(Class, String, String)}
+     * {@link #withCustomResource(Class, String)}
      *
      * @param outputTarget the {@link OutputTargetBuildItem} specifying where the CRDs
      *        should be generated
@@ -101,7 +101,7 @@ class CRDGeneration {
         return new CRDGenerationInfo(shouldApply(), validateCustomResources, converted, generated);
     }
 
-    private boolean needsGeneration(Map<String, CRDInfo> existingCRDInfos, Set<String> changedClassNames, String targetCRName) {
+    private boolean needsGeneration(Map<String, CRDInfo> existingCRDInfos, Set<String> changedClassNames) {
         final boolean[] generateCurrent = { true }; // request CRD generation by default
         crdConfiguration.versions().forEach(v -> {
             final var crd = existingCRDInfos.get(v);
@@ -122,7 +122,7 @@ class CRDGeneration {
             // we've looked at all the changed classes and none have been changed for this CR/version: do not regenerate CRD
             log.infov(
                     "''{0}'' CRD generation was skipped for ''{1}'' because no changes impacting the CRD were detected",
-                    v, targetCRName);
+                    v, crd.getCrdName());
             generateCurrent[0] = false;
         });
         return generateCurrent[0];
@@ -131,21 +131,19 @@ class CRDGeneration {
     boolean scheduleForGenerationIfNeeded(CustomResourceAugmentedClassInfo crInfo,
             Map<String, CRDInfo> existingCRDInfos, Set<String> changedClasses) {
         var scheduleCurrent = true;
-        final String targetCRName = crInfo.asResourceTargeting().fullResourceName();
 
         if (existingCRDInfos != null && !existingCRDInfos.isEmpty()) {
-            scheduleCurrent = needsGeneration(existingCRDInfos, changedClasses, targetCRName);
+            scheduleCurrent = needsGeneration(existingCRDInfos, changedClasses);
         }
 
         if (scheduleCurrent) {
-            withCustomResource(crInfo.loadAssociatedClass(), targetCRName, crInfo.getAssociatedReconcilerName().orElse(null));
+            withCustomResource(crInfo.loadAssociatedClass(), crInfo.getAssociatedReconcilerName().orElse(null));
         }
 
         return scheduleCurrent;
     }
 
-    public void withCustomResource(Class<? extends CustomResource<?, ?>> crClass, String crdName,
-            String associatedControllerName) {
+    public void withCustomResource(Class<? extends CustomResource<?, ?>> crClass, String associatedControllerName) {
         // first check if the CR is not filtered out
         if (crdConfiguration.excludeResources().map(excluded -> excluded.contains(crClass.getName())).orElse(false)) {
             log.infov("CRD generation was skipped for ''{0}'' because it was excluded from generation", crClass.getName());
@@ -159,7 +157,7 @@ class CRDGeneration {
                 generator = new CRDGenerator().withParallelGenerationEnabled(crdConfiguration.generateInParallel());
             }
             final var info = CustomResourceInfo.fromClass(crClass);
-            crMappings.add(info, crdName, associatedControllerName);
+            crMappings.add(info, associatedControllerName);
             generator.customResources(info);
             needGeneration = true;
         } catch (Exception e) {
diff --git a/core/deployment/src/main/java/io/quarkiverse/operatorsdk/deployment/CRDGenerationBuildStep.java b/core/deployment/src/main/java/io/quarkiverse/operatorsdk/deployment/CRDGenerationBuildStep.java
index 1e5082ec..6c3cdd99 100644
--- a/core/deployment/src/main/java/io/quarkiverse/operatorsdk/deployment/CRDGenerationBuildStep.java
+++ b/core/deployment/src/main/java/io/quarkiverse/operatorsdk/deployment/CRDGenerationBuildStep.java
@@ -52,22 +52,22 @@ class CRDGenerationBuildStep {
                 final ReconciledAugmentedClassInfo<?> associatedResource = raci.associatedResourceInfo();
                 if (associatedResource.isCR()) {
                     final var crInfo = associatedResource.asResourceTargeting();
-                    final String targetCRName = crInfo.fullResourceName();
+                    final String crId = crInfo.id();
 
                     // if the primary resource is unowned, mark it as "scheduled" (i.e. already handled) so that it doesn't get considered if all CRDs generation is requested
                     if (!operatorConfiguration.isControllerOwningPrimary(raci.nameOrFailIfUnset())) {
-                        scheduledForGeneration.add(targetCRName);
+                        scheduledForGeneration.add(crId);
                     } else {
                         // When we have a live reload, check if we need to regenerate the associated CRD
                         Map<String, CRDInfo> crdInfos = Collections.emptyMap();
                         if (liveReload.isLiveReload()) {
-                            crdInfos = storedCRDInfos.getCRDInfosFor(targetCRName);
+                            crdInfos = storedCRDInfos.getCRDInfosFor(crId);
                         }
 
                         // schedule the generation of associated primary resource CRD if required
                         if (crdGeneration.scheduleForGenerationIfNeeded((CustomResourceAugmentedClassInfo) crInfo, crdInfos,
                                 changedClasses)) {
-                            scheduledForGeneration.add(targetCRName);
+                            scheduledForGeneration.add(crId);
                         }
                     }
                 }
@@ -80,9 +80,8 @@ class CRDGenerationBuildStep {
                         Map.of(CustomResourceAugmentedClassInfo.EXISTING_CRDS_KEY, scheduledForGeneration))
                         .map(CustomResourceAugmentedClassInfo.class::cast)
                         .forEach(cr -> {
-                            final var targetCRName = cr.fullResourceName();
-                            crdGeneration.withCustomResource(cr.loadAssociatedClass(), targetCRName, null);
-                            log.infov("Will generate CRD for non-reconciler bound resource: {0}", targetCRName);
+                            crdGeneration.withCustomResource(cr.loadAssociatedClass(), null);
+                            log.infov("Will generate CRD for non-reconciler bound resource: {0}", cr.fullResourceName());
                         });
             }
         }
diff --git a/core/deployment/src/main/java/io/quarkiverse/operatorsdk/deployment/ResourceControllerMapping.java b/core/deployment/src/main/java/io/quarkiverse/operatorsdk/deployment/ResourceControllerMapping.java
index 07106b7d..a9ed3798 100644
--- a/core/deployment/src/main/java/io/quarkiverse/operatorsdk/deployment/ResourceControllerMapping.java
+++ b/core/deployment/src/main/java/io/quarkiverse/operatorsdk/deployment/ResourceControllerMapping.java
@@ -17,8 +17,9 @@ public class ResourceControllerMapping {
         return infos;
     }
 
-    public void add(io.fabric8.crdv2.generator.CustomResourceInfo info, String crdName, String associatedControllerName) {
+    public void add(io.fabric8.crdv2.generator.CustomResourceInfo info, String associatedControllerName) {
         final var version = info.version();
+        final var crdName = info.crdName();
         final var versionsForCR = resourceFullNameToVersionToInfos.computeIfAbsent(crdName, s -> new HashMap<>());
         final var cri = versionsForCR.get(version);
         if (cri != null) {
@@ -37,17 +38,16 @@ public class ResourceControllerMapping {
             throw new IllegalStateException(msg);
         }
 
-        final var converted = augment(info, crdName, associatedControllerName);
+        final var converted = augment(info, associatedControllerName);
         versionsForCR.put(version, converted);
     }
 
-    private static ResourceInfo augment(io.fabric8.crdv2.generator.CustomResourceInfo info,
-            String crdName, String associatedControllerName) {
+    private static ResourceInfo augment(io.fabric8.crdv2.generator.CustomResourceInfo info, String associatedControllerName) {
         return new ResourceInfo(
                 info.group(), info.version(), info.kind(), info.singular(), info.plural(), info.shortNames(),
                 info.storage(),
                 info.served(), info.scope(), info.crClassName(),
-                info.statusClassName().map(ClassUtils::isStatusNotVoid).orElse(false), crdName,
+                info.statusClassName().map(ClassUtils::isStatusNotVoid).orElse(false), info.crdName(),
                 associatedControllerName);
     }
 }


Test patch:
diff --git a/core/deployment/src/test/java/io/quarkiverse/operatorsdk/test/AllCRDGenerationTest.java b/core/deployment/src/test/java/io/quarkiverse/operatorsdk/test/AllCRDGenerationTest.java
index eb840bc4..191119aa 100644
--- a/core/deployment/src/test/java/io/quarkiverse/operatorsdk/test/AllCRDGenerationTest.java
+++ b/core/deployment/src/test/java/io/quarkiverse/operatorsdk/test/AllCRDGenerationTest.java
@@ -28,8 +28,8 @@ public class AllCRDGenerationTest {
     static final QuarkusProdModeTest config = new QuarkusProdModeTest()
             .setApplicationName("test")
             .withApplicationRoot(
-                    (jar) -> jar.addClasses(SimpleReconciler.class, SimpleCR.class, SimpleSpec.class, SimpleStatus.class,
-                            External.class, ExternalV1.class, SimpleReconcilerV2.class, SimpleCRV2.class))
+                    (jar) -> jar.addClasses(SimpleCR.class, SimpleSpec.class, SimpleStatus.class,
+                            External.class, ExternalV1.class, SimpleCRV2.class, SimpleReconcilerV2.class))
             .overrideConfigKey("quarkus.operator-sdk.crd.generate-all", "true");
 
     @ProdBuildResults