fix: record version with CRD name to properly output multiple versions (

#973) 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>
quarkiverse · Oct 8, 2024 · 805dc0a · 805dc0a
1 parent c08365d
commit 805dc0a
Show file tree

Hide file tree

Showing 11 changed files with 70 additions and 38 deletions.
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
@@ -10,6 +10,7 @@
 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 @@ private 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/...ent/src/main/java/io/quarkiverse/operatorsdk/common/CustomResourceAugmentedClassInfo.java b/...ent/src/main/java/io/quarkiverse/operatorsdk/common/CustomResourceAugmentedClassInfo.java
@@ -20,14 +20,12 @@ protected CustomResourceAugmentedClassInfo(ClassInfo classInfo, String associate
 
     @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/.../src/main/java/io/quarkiverse/operatorsdk/common/DependentResourceAugmentedClassInfo.java b/.../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/...loyment/src/main/java/io/quarkiverse/operatorsdk/common/ReconciledAugmentedClassInfo.java b/...loyment/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 ReconciledResourceAugmentedClassInfo asResourceTargeting() {
     }
 
     @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 static ReconciledAugmentedClassInfo createFor(ClassInfo resourceCI, Strin
         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/...src/main/java/io/quarkiverse/operatorsdk/common/ReconciledResourceAugmentedClassInfo.java b/...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 @@ protected boolean hasStatus(IndexView index) {
     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/...loyment/src/main/java/io/quarkiverse/operatorsdk/common/ReconcilerAugmentedClassInfo.java b/...loyment/src/main/java/io/quarkiverse/operatorsdk/common/ReconcilerAugmentedClassInfo.java
@@ -61,7 +61,7 @@ protected void doAugment(IndexView index, Logger log, Map<String, Object> contex
                             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/...src/main/java/io/quarkiverse/operatorsdk/common/ResourceAssociatedAugmentedClassInfo.java b/...src/main/java/io/quarkiverse/operatorsdk/common/ResourceAssociatedAugmentedClassInfo.java
@@ -13,11 +13,18 @@
 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 @@ protected void doAugment(IndexView index, Logger log, Map<String, Object> contex
                     + "' 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
@@ -56,7 +56,7 @@ boolean shouldApply() {
      * 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 @@ CRDGenerationInfo generate(OutputTargetBuildItem outputTarget,
         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 @@ private boolean needsGeneration(Map<String, CRDInfo> existingCRDInfos, Set<Strin
             // 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 @@ private boolean needsGeneration(Map<String, CRDInfo> existingCRDInfos, Set<Strin
     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 @@ public void withCustomResource(Class<? extends CustomResource<?, ?>> crClass, St
                 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/...eployment/src/main/java/io/quarkiverse/operatorsdk/deployment/CRDGenerationBuildStep.java b/...eployment/src/main/java/io/quarkiverse/operatorsdk/deployment/CRDGenerationBuildStep.java
@@ -52,22 +52,22 @@ GeneratedCRDInfoBuildItem generateCRDs(
                 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 @@ GeneratedCRDInfoBuildItem generateCRDs(
                         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/...oyment/src/main/java/io/quarkiverse/operatorsdk/deployment/ResourceControllerMapping.java b/...oyment/src/main/java/io/quarkiverse/operatorsdk/deployment/ResourceControllerMapping.java
@@ -17,8 +17,9 @@ public Map<String, ResourceInfo> getResourceInfos(String resourceFullName) {
         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 void add(io.fabric8.crdv2.generator.CustomResourceInfo info, String crdNa
             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);
     }
 }
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
@@ -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