From e0a957b60ce742f7768d3469f3d74b1d0efbe8a1 Mon Sep 17 00:00:00 2001 From: Chris Laprun Date: Thu, 7 Nov 2024 15:47:14 +0100 Subject: [PATCH 1/5] refactor: move external CRD configuration to main CRD configuration Signed-off-by: Chris Laprun --- .../bundle/deployment/BundleProcessor.java | 4 ++-- .../operatorsdk/bundle/ExternalCRDsTest.java | 2 +- .../runtime/BundleGenerationConfiguration.java | 14 -------------- .../operatorsdk/runtime/CRDConfiguration.java | 14 ++++++++++++++ 4 files changed, 17 insertions(+), 17 deletions(-) diff --git a/bundle-generator/deployment/src/main/java/io/quarkiverse/operatorsdk/bundle/deployment/BundleProcessor.java b/bundle-generator/deployment/src/main/java/io/quarkiverse/operatorsdk/bundle/deployment/BundleProcessor.java index 46a280f0..15220535 100644 --- a/bundle-generator/deployment/src/main/java/io/quarkiverse/operatorsdk/bundle/deployment/BundleProcessor.java +++ b/bundle-generator/deployment/src/main/java/io/quarkiverse/operatorsdk/bundle/deployment/BundleProcessor.java @@ -115,9 +115,9 @@ private static String getBundleName(AnnotationInstance csvMetadata, String defau } @BuildStep(onlyIf = IsGenerationEnabled.class) - UnownedCRDInfoBuildItem unownedCRDInfo(BundleGenerationConfiguration bundleConfiguration, + UnownedCRDInfoBuildItem unownedCRDInfo(BuildTimeOperatorConfiguration operatorConfiguration, CurateOutcomeBuildItem appInfoBuildItem) { - final Optional> maybeExternalCRDs = bundleConfiguration.externalCRDLocations(); + final Optional> maybeExternalCRDs = operatorConfiguration.crd().externalCRDLocations(); final var crds = new CRDInfos(); if (maybeExternalCRDs.isPresent()) { final var moduleRoot = appInfoBuildItem.getApplicationModel().getApplicationModule().getModuleDir().toPath(); diff --git a/bundle-generator/deployment/src/test/java/io/quarkiverse/operatorsdk/bundle/ExternalCRDsTest.java b/bundle-generator/deployment/src/test/java/io/quarkiverse/operatorsdk/bundle/ExternalCRDsTest.java index 5e2cef04..753508b6 100644 --- a/bundle-generator/deployment/src/test/java/io/quarkiverse/operatorsdk/bundle/ExternalCRDsTest.java +++ b/bundle-generator/deployment/src/test/java/io/quarkiverse/operatorsdk/bundle/ExternalCRDsTest.java @@ -32,7 +32,7 @@ public class ExternalCRDsTest { .withApplicationRoot((jar) -> jar .addClasses(First.class, External.class, ExternalDependentResource.class, ReconcilerWithExternalCR.class)) - .overrideConfigKey("quarkus.operator-sdk.bundle.external-crd-locations", + .overrideConfigKey("quarkus.operator-sdk.crd.external-crd-locations", "src/test/external-crds/v1beta1spec.crd.yml, src/test/external-crds/external.crd.yml"); @ProdBuildResults diff --git a/bundle-generator/runtime/src/main/java/io/quarkiverse/operatorsdk/bundle/runtime/BundleGenerationConfiguration.java b/bundle-generator/runtime/src/main/java/io/quarkiverse/operatorsdk/bundle/runtime/BundleGenerationConfiguration.java index 5af0157c..f7334b68 100644 --- a/bundle-generator/runtime/src/main/java/io/quarkiverse/operatorsdk/bundle/runtime/BundleGenerationConfiguration.java +++ b/bundle-generator/runtime/src/main/java/io/quarkiverse/operatorsdk/bundle/runtime/BundleGenerationConfiguration.java @@ -58,18 +58,4 @@ public interface BundleGenerationConfiguration { * @since 6.8.0 */ Map bundles(); - - /** - * A comma-separated list of paths where external CRDs that need to be referenced for non-generated custom resources. - * Typical use cases where this might be needed include when custom resource implementations are located in a different - * module than the controller implementation or when the CRDs are not generated at all (e.g. in integration cases where your - * operator needs to deal with 3rd party custom resources). - * - *

- * Paths can be either absolute or relative, in which case they will be resolved from the current module root directory. - *

- * - * @since 6.8.4 - */ - Optional> externalCRDLocations(); } diff --git a/core/runtime/src/main/java/io/quarkiverse/operatorsdk/runtime/CRDConfiguration.java b/core/runtime/src/main/java/io/quarkiverse/operatorsdk/runtime/CRDConfiguration.java index 90faee8d..bd0ca5e9 100644 --- a/core/runtime/src/main/java/io/quarkiverse/operatorsdk/runtime/CRDConfiguration.java +++ b/core/runtime/src/main/java/io/quarkiverse/operatorsdk/runtime/CRDConfiguration.java @@ -64,4 +64,18 @@ public interface CRDConfiguration { * process. */ Optional> excludeResources(); + + /** + * A comma-separated list of paths where external CRDs that need to be referenced for non-generated custom resources. + * Typical use cases where this might be needed include when custom resource implementations are located in a different + * module than the controller implementation or when the CRDs are not generated at all (e.g. in integration cases where your + * operator needs to deal with 3rd party custom resources). + * + *

+ * Paths can be either absolute or relative, in which case they will be resolved from the current module root directory. + *

+ * + * @since 6.8.4 + */ + Optional> externalCRDLocations(); } From e84e8f732994b352a2efb31d8d321294c1694bfb Mon Sep 17 00:00:00 2001 From: Chris Laprun Date: Thu, 7 Nov 2024 15:59:24 +0100 Subject: [PATCH 2/5] refactor: load external CRDs from CRD generation step Fixes #867 Signed-off-by: Chris Laprun --- .../bundle/deployment/BundleProcessor.java | 37 +------------------ .../deployment/CRDGenerationBuildStep.java | 24 ++++++++++++ .../deployment/UnownedCRDInfoBuildItem.java | 2 +- .../operatorsdk/runtime/CRDUtils.java | 11 ++++++ ...quarkus-operator-sdk-bundle-generator.adoc | 19 ---------- ...bundle-generator_quarkus.operator-sdk.adoc | 19 ---------- .../pages/includes/quarkus-operator-sdk.adoc | 19 ++++++++++ ...kus-operator-sdk_quarkus.operator-sdk.adoc | 19 ++++++++++ 8 files changed, 75 insertions(+), 75 deletions(-) rename {bundle-generator/deployment/src/main/java/io/quarkiverse/operatorsdk/bundle => core/deployment/src/main/java/io/quarkiverse/operatorsdk}/deployment/UnownedCRDInfoBuildItem.java (86%) diff --git a/bundle-generator/deployment/src/main/java/io/quarkiverse/operatorsdk/bundle/deployment/BundleProcessor.java b/bundle-generator/deployment/src/main/java/io/quarkiverse/operatorsdk/bundle/deployment/BundleProcessor.java index 15220535..0e097e4d 100644 --- a/bundle-generator/deployment/src/main/java/io/quarkiverse/operatorsdk/bundle/deployment/BundleProcessor.java +++ b/bundle-generator/deployment/src/main/java/io/quarkiverse/operatorsdk/bundle/deployment/BundleProcessor.java @@ -11,9 +11,7 @@ import java.util.List; import java.util.Map; import java.util.Optional; -import java.util.Set; import java.util.function.BooleanSupplier; -import java.util.function.Predicate; import org.jboss.jandex.AnnotationInstance; import org.jboss.jandex.AnnotationValue; @@ -38,17 +36,14 @@ import io.quarkiverse.operatorsdk.common.ReconciledAugmentedClassInfo; import io.quarkiverse.operatorsdk.common.ReconcilerAugmentedClassInfo; import io.quarkiverse.operatorsdk.deployment.GeneratedCRDInfoBuildItem; +import io.quarkiverse.operatorsdk.deployment.UnownedCRDInfoBuildItem; import io.quarkiverse.operatorsdk.deployment.VersionBuildItem; import io.quarkiverse.operatorsdk.runtime.BuildTimeOperatorConfiguration; -import io.quarkiverse.operatorsdk.runtime.CRDInfo; -import io.quarkiverse.operatorsdk.runtime.CRDInfos; -import io.quarkiverse.operatorsdk.runtime.CRDUtils; import io.quarkus.deployment.annotations.BuildProducer; import io.quarkus.deployment.annotations.BuildStep; import io.quarkus.deployment.builditem.ApplicationInfoBuildItem; import io.quarkus.deployment.builditem.CombinedIndexBuildItem; import io.quarkus.deployment.builditem.GeneratedFileSystemResourceBuildItem; -import io.quarkus.deployment.pkg.builditem.CurateOutcomeBuildItem; import io.quarkus.deployment.pkg.builditem.JarBuildItem; import io.quarkus.deployment.pkg.builditem.OutputTargetBuildItem; import io.quarkus.kubernetes.deployment.KubernetesConfig; @@ -63,7 +58,6 @@ public class BundleProcessor { private static final DotName CSV_METADATA = DotName.createSimple(CSVMetadata.class.getName()); private static final String BUNDLE = "bundle"; private static final String DEFAULT_PROVIDER_NAME = System.getProperty("user.name"); - private static final Set EMPTY_SET = Set.of(); private static ReconcilerAugmentedClassInfo augmentReconcilerInfo( ReconcilerAugmentedClassInfo reconcilerInfo) { @@ -114,35 +108,6 @@ private static String getBundleName(AnnotationInstance csvMetadata, String defau } } - @BuildStep(onlyIf = IsGenerationEnabled.class) - UnownedCRDInfoBuildItem unownedCRDInfo(BuildTimeOperatorConfiguration operatorConfiguration, - CurateOutcomeBuildItem appInfoBuildItem) { - final Optional> maybeExternalCRDs = operatorConfiguration.crd().externalCRDLocations(); - final var crds = new CRDInfos(); - if (maybeExternalCRDs.isPresent()) { - final var moduleRoot = appInfoBuildItem.getApplicationModel().getApplicationModule().getModuleDir().toPath(); - maybeExternalCRDs.get().parallelStream() - .filter(Predicate.not(String::isBlank)) - .map(String::trim) - .forEach(crdLocation -> { - final var crdPath = moduleRoot.resolve(crdLocation); - final var crd = loadFrom(crdPath); - crds.addCRDInfoFor(crd.getCrdName(), crd.getCrdSpecVersion(), crd); - }); - } - return new UnownedCRDInfoBuildItem(crds); - } - - private CRDInfo loadFrom(Path crdPath) { - try { - final var crd = CRDUtils.loadFrom(crdPath); - final var crdName = crd.getMetadata().getName(); - return new CRDInfo(crdName, CRDUtils.DEFAULT_CRD_SPEC_VERSION, crdPath.toFile().getAbsolutePath(), EMPTY_SET); - } catch (IOException e) { - throw new RuntimeException(e); - } - } - @SuppressWarnings({ "unused" }) @BuildStep(onlyIf = IsGenerationEnabled.class) CSVMetadataBuildItem gatherCSVMetadata(KubernetesConfig kubernetesConfig, 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 5e262e9c..3c4f4e50 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 @@ -2,7 +2,10 @@ import java.util.Collections; import java.util.HashSet; +import java.util.List; import java.util.Map; +import java.util.Optional; +import java.util.function.Predicate; import org.jboss.logging.Logger; @@ -11,10 +14,12 @@ import io.quarkiverse.operatorsdk.runtime.CRDGenerationInfo; import io.quarkiverse.operatorsdk.runtime.CRDInfo; import io.quarkiverse.operatorsdk.runtime.CRDInfos; +import io.quarkiverse.operatorsdk.runtime.CRDUtils; import io.quarkus.deployment.annotations.BuildStep; import io.quarkus.deployment.builditem.CombinedIndexBuildItem; import io.quarkus.deployment.builditem.LaunchModeBuildItem; import io.quarkus.deployment.builditem.LiveReloadBuildItem; +import io.quarkus.deployment.pkg.builditem.CurateOutcomeBuildItem; import io.quarkus.deployment.pkg.builditem.OutputTargetBuildItem; class CRDGenerationBuildStep { @@ -94,4 +99,23 @@ GeneratedCRDInfoBuildItem generateCRDs( return new GeneratedCRDInfoBuildItem(crdInfo); } + + @BuildStep + UnownedCRDInfoBuildItem unownedCRDInfo(BuildTimeOperatorConfiguration operatorConfiguration, + CurateOutcomeBuildItem appInfoBuildItem) { + final Optional> maybeExternalCRDs = operatorConfiguration.crd().externalCRDLocations(); + final var crds = new CRDInfos(); + if (maybeExternalCRDs.isPresent()) { + final var moduleRoot = appInfoBuildItem.getApplicationModel().getApplicationModule().getModuleDir().toPath(); + maybeExternalCRDs.get().parallelStream() + .filter(Predicate.not(String::isBlank)) + .map(String::trim) + .forEach(crdLocation -> { + final var crdPath = moduleRoot.resolve(crdLocation); + final var crd = CRDUtils.loadFromAsCRDInfo(crdPath); + crds.addCRDInfoFor(crd.getCrdName(), crd.getCrdSpecVersion(), crd); + }); + } + return new UnownedCRDInfoBuildItem(crds); + } } diff --git a/bundle-generator/deployment/src/main/java/io/quarkiverse/operatorsdk/bundle/deployment/UnownedCRDInfoBuildItem.java b/core/deployment/src/main/java/io/quarkiverse/operatorsdk/deployment/UnownedCRDInfoBuildItem.java similarity index 86% rename from bundle-generator/deployment/src/main/java/io/quarkiverse/operatorsdk/bundle/deployment/UnownedCRDInfoBuildItem.java rename to core/deployment/src/main/java/io/quarkiverse/operatorsdk/deployment/UnownedCRDInfoBuildItem.java index 73a52ff8..d6a1b939 100644 --- a/bundle-generator/deployment/src/main/java/io/quarkiverse/operatorsdk/bundle/deployment/UnownedCRDInfoBuildItem.java +++ b/core/deployment/src/main/java/io/quarkiverse/operatorsdk/deployment/UnownedCRDInfoBuildItem.java @@ -1,4 +1,4 @@ -package io.quarkiverse.operatorsdk.bundle.deployment; +package io.quarkiverse.operatorsdk.deployment; import io.quarkiverse.operatorsdk.runtime.CRDInfos; import io.quarkus.builder.item.SimpleBuildItem; diff --git a/core/runtime/src/main/java/io/quarkiverse/operatorsdk/runtime/CRDUtils.java b/core/runtime/src/main/java/io/quarkiverse/operatorsdk/runtime/CRDUtils.java index bffba9eb..e50c9428 100644 --- a/core/runtime/src/main/java/io/quarkiverse/operatorsdk/runtime/CRDUtils.java +++ b/core/runtime/src/main/java/io/quarkiverse/operatorsdk/runtime/CRDUtils.java @@ -3,6 +3,7 @@ import java.io.IOException; import java.nio.file.Files; import java.nio.file.Path; +import java.util.Collections; import org.jboss.logging.Logger; @@ -56,6 +57,16 @@ public static CustomResourceDefinition loadFrom(Path crdPath) throws IOException return crd; } + public static CRDInfo loadFromAsCRDInfo(Path crdPath) { + try { + final var crd = loadFrom(crdPath); + final var crdName = crd.getMetadata().getName(); + return new CRDInfo(crdName, DEFAULT_CRD_SPEC_VERSION, crdPath.toFile().getAbsolutePath(), Collections.emptySet()); + } catch (IOException e) { + throw new RuntimeException(e); + } + } + private static void apply(KubernetesClient client, String v, Object crd) { switch (v) { case DEFAULT_CRD_SPEC_VERSION: diff --git a/docs/modules/ROOT/pages/includes/quarkus-operator-sdk-bundle-generator.adoc b/docs/modules/ROOT/pages/includes/quarkus-operator-sdk-bundle-generator.adoc index 805a8d6b..91eb43ff 100644 --- a/docs/modules/ROOT/pages/includes/quarkus-operator-sdk-bundle-generator.adoc +++ b/docs/modules/ROOT/pages/includes/quarkus-operator-sdk-bundle-generator.adoc @@ -110,25 +110,6 @@ endif::add-copy-button-to-env-var[] |Map | -a|icon:lock[title=Fixed at build time] [[quarkus-operator-sdk-bundle-generator_quarkus-operator-sdk-bundle-external-crd-locations]] [.property-path]##link:#quarkus-operator-sdk-bundle-generator_quarkus-operator-sdk-bundle-external-crd-locations[`quarkus.operator-sdk.bundle.external-crd-locations`]## - -[.description] --- -A comma-separated list of paths where external CRDs that need to be referenced for non-generated custom resources. Typical use cases where this might be needed include when custom resource implementations are located in a different module than the controller implementation or when the CRDs are not generated at all (e.g. in integration cases where your operator needs to deal with 3rd party custom resources). - -Paths can be either absolute or relative, in which case they will be resolved from the current module root directory. - - -ifdef::add-copy-button-to-env-var[] -Environment variable: env_var_with_copy_button:+++QUARKUS_OPERATOR_SDK_BUNDLE_EXTERNAL_CRD_LOCATIONS+++[] -endif::add-copy-button-to-env-var[] -ifndef::add-copy-button-to-env-var[] -Environment variable: `+++QUARKUS_OPERATOR_SDK_BUNDLE_EXTERNAL_CRD_LOCATIONS+++` -endif::add-copy-button-to-env-var[] --- -|list of string -| - |=== diff --git a/docs/modules/ROOT/pages/includes/quarkus-operator-sdk-bundle-generator_quarkus.operator-sdk.adoc b/docs/modules/ROOT/pages/includes/quarkus-operator-sdk-bundle-generator_quarkus.operator-sdk.adoc index 805a8d6b..91eb43ff 100644 --- a/docs/modules/ROOT/pages/includes/quarkus-operator-sdk-bundle-generator_quarkus.operator-sdk.adoc +++ b/docs/modules/ROOT/pages/includes/quarkus-operator-sdk-bundle-generator_quarkus.operator-sdk.adoc @@ -110,25 +110,6 @@ endif::add-copy-button-to-env-var[] |Map | -a|icon:lock[title=Fixed at build time] [[quarkus-operator-sdk-bundle-generator_quarkus-operator-sdk-bundle-external-crd-locations]] [.property-path]##link:#quarkus-operator-sdk-bundle-generator_quarkus-operator-sdk-bundle-external-crd-locations[`quarkus.operator-sdk.bundle.external-crd-locations`]## - -[.description] --- -A comma-separated list of paths where external CRDs that need to be referenced for non-generated custom resources. Typical use cases where this might be needed include when custom resource implementations are located in a different module than the controller implementation or when the CRDs are not generated at all (e.g. in integration cases where your operator needs to deal with 3rd party custom resources). - -Paths can be either absolute or relative, in which case they will be resolved from the current module root directory. - - -ifdef::add-copy-button-to-env-var[] -Environment variable: env_var_with_copy_button:+++QUARKUS_OPERATOR_SDK_BUNDLE_EXTERNAL_CRD_LOCATIONS+++[] -endif::add-copy-button-to-env-var[] -ifndef::add-copy-button-to-env-var[] -Environment variable: `+++QUARKUS_OPERATOR_SDK_BUNDLE_EXTERNAL_CRD_LOCATIONS+++` -endif::add-copy-button-to-env-var[] --- -|list of string -| - |=== diff --git a/docs/modules/ROOT/pages/includes/quarkus-operator-sdk.adoc b/docs/modules/ROOT/pages/includes/quarkus-operator-sdk.adoc index 73bfddb2..44eeb0ad 100644 --- a/docs/modules/ROOT/pages/includes/quarkus-operator-sdk.adoc +++ b/docs/modules/ROOT/pages/includes/quarkus-operator-sdk.adoc @@ -144,6 +144,25 @@ endif::add-copy-button-to-env-var[] |list of string | +a|icon:lock[title=Fixed at build time] [[quarkus-operator-sdk_quarkus-operator-sdk-crd-external-crd-locations]] [.property-path]##link:#quarkus-operator-sdk_quarkus-operator-sdk-crd-external-crd-locations[`quarkus.operator-sdk.crd.external-crd-locations`]## + +[.description] +-- +A comma-separated list of paths where external CRDs that need to be referenced for non-generated custom resources. Typical use cases where this might be needed include when custom resource implementations are located in a different module than the controller implementation or when the CRDs are not generated at all (e.g. in integration cases where your operator needs to deal with 3rd party custom resources). + +Paths can be either absolute or relative, in which case they will be resolved from the current module root directory. + + +ifdef::add-copy-button-to-env-var[] +Environment variable: env_var_with_copy_button:+++QUARKUS_OPERATOR_SDK_CRD_EXTERNAL_CRD_LOCATIONS+++[] +endif::add-copy-button-to-env-var[] +ifndef::add-copy-button-to-env-var[] +Environment variable: `+++QUARKUS_OPERATOR_SDK_CRD_EXTERNAL_CRD_LOCATIONS+++` +endif::add-copy-button-to-env-var[] +-- +|list of string +| + a|icon:lock[title=Fixed at build time] [[quarkus-operator-sdk_quarkus-operator-sdk-generation-aware]] [.property-path]##link:#quarkus-operator-sdk_quarkus-operator-sdk-generation-aware[`quarkus.operator-sdk.generation-aware`]## [.description] diff --git a/docs/modules/ROOT/pages/includes/quarkus-operator-sdk_quarkus.operator-sdk.adoc b/docs/modules/ROOT/pages/includes/quarkus-operator-sdk_quarkus.operator-sdk.adoc index 73bfddb2..44eeb0ad 100644 --- a/docs/modules/ROOT/pages/includes/quarkus-operator-sdk_quarkus.operator-sdk.adoc +++ b/docs/modules/ROOT/pages/includes/quarkus-operator-sdk_quarkus.operator-sdk.adoc @@ -144,6 +144,25 @@ endif::add-copy-button-to-env-var[] |list of string | +a|icon:lock[title=Fixed at build time] [[quarkus-operator-sdk_quarkus-operator-sdk-crd-external-crd-locations]] [.property-path]##link:#quarkus-operator-sdk_quarkus-operator-sdk-crd-external-crd-locations[`quarkus.operator-sdk.crd.external-crd-locations`]## + +[.description] +-- +A comma-separated list of paths where external CRDs that need to be referenced for non-generated custom resources. Typical use cases where this might be needed include when custom resource implementations are located in a different module than the controller implementation or when the CRDs are not generated at all (e.g. in integration cases where your operator needs to deal with 3rd party custom resources). + +Paths can be either absolute or relative, in which case they will be resolved from the current module root directory. + + +ifdef::add-copy-button-to-env-var[] +Environment variable: env_var_with_copy_button:+++QUARKUS_OPERATOR_SDK_CRD_EXTERNAL_CRD_LOCATIONS+++[] +endif::add-copy-button-to-env-var[] +ifndef::add-copy-button-to-env-var[] +Environment variable: `+++QUARKUS_OPERATOR_SDK_CRD_EXTERNAL_CRD_LOCATIONS+++` +endif::add-copy-button-to-env-var[] +-- +|list of string +| + a|icon:lock[title=Fixed at build time] [[quarkus-operator-sdk_quarkus-operator-sdk-generation-aware]] [.property-path]##link:#quarkus-operator-sdk_quarkus-operator-sdk-generation-aware[`quarkus.operator-sdk.generation-aware`]## [.description] From 964c9886ad767bf7d9750496340f4eebd6087d0a Mon Sep 17 00:00:00 2001 From: Chris Laprun Date: Thu, 7 Nov 2024 22:17:41 +0100 Subject: [PATCH 3/5] refactor: simplify add method Signed-off-by: Chris Laprun --- .../operatorsdk/deployment/CRDGeneration.java | 20 +++++++++++++------ .../deployment/CRDGenerationBuildStep.java | 13 ++++++------ .../operatorsdk/runtime/CRDInfos.java | 4 ++-- 3 files changed, 22 insertions(+), 15 deletions(-) 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 f25fa0c0..4957ee3d 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 @@ -4,6 +4,7 @@ import java.nio.file.Path; import java.nio.file.Paths; +import java.util.Collections; import java.util.HashSet; import java.util.Map; import java.util.Optional; @@ -13,6 +14,7 @@ import io.fabric8.crdv2.generator.CRDGenerator; import io.fabric8.crdv2.generator.CustomResourceInfo; +import io.fabric8.kubernetes.api.model.HasMetadata; import io.fabric8.kubernetes.client.CustomResource; import io.quarkiverse.operatorsdk.common.CustomResourceAugmentedClassInfo; import io.quarkiverse.operatorsdk.common.FileUtils; @@ -25,14 +27,20 @@ class CRDGeneration { private static final Logger log = Logger.getLogger(CRDGeneration.class.getName()); - private CRDGenerator generator; private final LaunchMode mode; private final CRDConfiguration crdConfiguration; + private final Set excludedCRDNames; + private final Set excludedResourceClassNames; + private CRDGenerator generator; private boolean needGeneration; - public CRDGeneration(CRDConfiguration crdConfig, LaunchMode mode) { + CRDGeneration(CRDConfiguration crdConfig, LaunchMode mode, CRDInfos externalCRDs) { this.crdConfiguration = crdConfig; this.mode = mode; + this.excludedCRDNames = externalCRDs.getCRDNameToInfoMappings().keySet(); + this.excludedResourceClassNames = crdConfig.excludeResources() + .map(excluded -> (Set) new HashSet<>(excluded)) + .orElse(Collections.emptySet()); } static boolean shouldGenerate(Optional configuredGenerate, Optional configuredApply, @@ -87,11 +95,11 @@ CRDGenerationInfo generate(OutputTargetBuildItem outputTarget, generated.add(crdName); initialVersionToCRDInfoMap - .forEach((version, crdInfo) -> { + .forEach((crdSpecVersion, crdInfo) -> { final var filePath = crdInfo.getFilePath(); - log.infov(" - {0} -> {1}", version, filePath); - converted.addCRDInfoFor(crdName, version, new CRDInfo(crdName, - version, filePath, crdInfo.getDependentClassNames())); + log.infov(" - {0} -> {1}", crdSpecVersion, filePath); + converted.addCRDInfo(new CRDInfo(crdName, + crdSpecVersion, filePath, crdInfo.getDependentClassNames())); }); }); } 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 3c4f4e50..5ac1e73d 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 @@ -69,13 +69,12 @@ GeneratedCRDInfoBuildItem generateCRDs( crdInfos = storedCRDInfos.getOrCreateCRDSpecVersionToInfoMapping(crId); } - // schedule the generation of associated primary resource CRD if required - if (crdGeneration.scheduleForGenerationIfNeeded((CustomResourceAugmentedClassInfo) crInfo, crdInfos, - changedClasses)) { - scheduledForGeneration.add(crId); + // schedule the generation of associated primary resource CRD if required + if (crdGeneration.scheduleForGenerationIfNeeded((CustomResourceAugmentedClassInfo) crInfo, crdInfos, + changedClasses)) { + scheduledForGeneration.add(crId); + } } - } - } }); // generate non-reconciler associated CRDs if requested @@ -113,7 +112,7 @@ UnownedCRDInfoBuildItem unownedCRDInfo(BuildTimeOperatorConfiguration operatorCo .forEach(crdLocation -> { final var crdPath = moduleRoot.resolve(crdLocation); final var crd = CRDUtils.loadFromAsCRDInfo(crdPath); - crds.addCRDInfoFor(crd.getCrdName(), crd.getCrdSpecVersion(), crd); + crds.addCRDInfo(crd); }); } return new UnownedCRDInfoBuildItem(crds); diff --git a/core/runtime/src/main/java/io/quarkiverse/operatorsdk/runtime/CRDInfos.java b/core/runtime/src/main/java/io/quarkiverse/operatorsdk/runtime/CRDInfos.java index 226596e4..5ffb6d68 100644 --- a/core/runtime/src/main/java/io/quarkiverse/operatorsdk/runtime/CRDInfos.java +++ b/core/runtime/src/main/java/io/quarkiverse/operatorsdk/runtime/CRDInfos.java @@ -40,8 +40,8 @@ public Map getCRDNameToInfoMappings() { .collect(Collectors.toMap(CRDInfo::getCrdName, Function.identity())); } - public void addCRDInfoFor(String crdName, String crdSpecVersion, CRDInfo crdInfo) { - getOrCreateCRDSpecVersionToInfoMapping(crdName).put(crdSpecVersion, crdInfo); + public void addCRDInfo(CRDInfo crdInfo) { + getOrCreateCRDSpecVersionToInfoMapping(crdInfo.getCrdName()).put(crdInfo.getCrdSpecVersion(), crdInfo); } // Needed by Quarkus: if this method isn't present, state is not properly set From 6240c2afd3bd1902baa656a2dfaf68538ab53c99 Mon Sep 17 00:00:00 2001 From: Chris Laprun Date: Thu, 7 Nov 2024 23:33:59 +0100 Subject: [PATCH 4/5] feat: only process CRs that are not excluded Signed-off-by: Chris Laprun --- .../CustomResourceAugmentedClassInfo.java | 17 ++-- .../operatorsdk/deployment/CRDGeneration.java | 18 +---- .../deployment/CRDGenerationBuildStep.java | 77 ++++++++++++++----- .../operatorsdk/runtime/CRDInfos.java | 4 + 4 files changed, 72 insertions(+), 44 deletions(-) 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 3478f65f..8862a1dd 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 @@ -2,7 +2,7 @@ import java.util.Map; import java.util.Optional; -import java.util.Set; +import java.util.function.Function; import org.jboss.jandex.ClassInfo; import org.jboss.jandex.IndexView; @@ -12,7 +12,7 @@ public class CustomResourceAugmentedClassInfo extends ReconciledResourceAugmentedClassInfo> { - public static final String EXISTING_CRDS_KEY = "existing-crds-key"; + public static final String KEEP_CR_PREDICATE_KEY = "keep-cr-predicate"; protected CustomResourceAugmentedClassInfo(ClassInfo classInfo, String associatedReconcilerName) { super(classInfo, Constants.CUSTOM_RESOURCE, 2, associatedReconcilerName); @@ -21,15 +21,16 @@ protected CustomResourceAugmentedClassInfo(ClassInfo classInfo, String associate @Override protected boolean doKeep(IndexView index, Logger log, Map context) { // only keep the information if the associated CRD hasn't already been generated - return Optional.ofNullable(context.get(EXISTING_CRDS_KEY)) - .map(value -> { - @SuppressWarnings("unchecked") - Set generated = (Set) value; - return !generated.contains(id()); - }) + return Optional.ofNullable(predicateFromContext(context)) + .map(predicate -> predicate.apply(this)) .orElse(true); } + @SuppressWarnings("unchecked") + private Function predicateFromContext(Map context) { + return (Function) context.get(KEEP_CR_PREDICATE_KEY); + } + @Override protected void doAugment(IndexView index, Logger log, Map context) { super.doAugment(index, log, context); 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 4957ee3d..59c48251 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 @@ -4,7 +4,6 @@ import java.nio.file.Path; import java.nio.file.Paths; -import java.util.Collections; import java.util.HashSet; import java.util.Map; import java.util.Optional; @@ -14,7 +13,6 @@ import io.fabric8.crdv2.generator.CRDGenerator; import io.fabric8.crdv2.generator.CustomResourceInfo; -import io.fabric8.kubernetes.api.model.HasMetadata; import io.fabric8.kubernetes.client.CustomResource; import io.quarkiverse.operatorsdk.common.CustomResourceAugmentedClassInfo; import io.quarkiverse.operatorsdk.common.FileUtils; @@ -29,18 +27,12 @@ class CRDGeneration { private static final Logger log = Logger.getLogger(CRDGeneration.class.getName()); private final LaunchMode mode; private final CRDConfiguration crdConfiguration; - private final Set excludedCRDNames; - private final Set excludedResourceClassNames; private CRDGenerator generator; private boolean needGeneration; - CRDGeneration(CRDConfiguration crdConfig, LaunchMode mode, CRDInfos externalCRDs) { + CRDGeneration(CRDConfiguration crdConfig, LaunchMode mode) { this.crdConfiguration = crdConfig; this.mode = mode; - this.excludedCRDNames = externalCRDs.getCRDNameToInfoMappings().keySet(); - this.excludedResourceClassNames = crdConfig.excludeResources() - .map(excluded -> (Set) new HashSet<>(excluded)) - .orElse(Collections.emptySet()); } static boolean shouldGenerate(Optional configuredGenerate, Optional configuredApply, @@ -148,13 +140,7 @@ boolean scheduleForGenerationIfNeeded(CustomResourceAugmentedClassInfo crInfo, return scheduleCurrent; } - public void withCustomResource(Class> 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()); - return; - } - + void withCustomResource(Class> crClass, String associatedControllerName) { try { // generator MUST be initialized before we start processing classes as initializing it // will reset the types information held by the generator 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 5ac1e73d..01070337 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 @@ -5,6 +5,8 @@ import java.util.List; import java.util.Map; import java.util.Optional; +import java.util.Set; +import java.util.function.Function; import java.util.function.Predicate; import org.jboss.logging.Logger; @@ -24,6 +26,8 @@ class CRDGenerationBuildStep { static final Logger log = Logger.getLogger(CRDGenerationBuildStep.class.getName()); + private static final String excludedCause = "it was explicitly excluded from generation"; + private static final String externalCause = "it is associated with an externally provided CRD"; @BuildStep GeneratedCRDInfoBuildItem generateCRDs( @@ -32,7 +36,8 @@ GeneratedCRDInfoBuildItem generateCRDs( LaunchModeBuildItem launchModeBuildItem, LiveReloadBuildItem liveReload, OutputTargetBuildItem outputTarget, - CombinedIndexBuildItem combinedIndexBuildItem) { + CombinedIndexBuildItem combinedIndexBuildItem, + UnownedCRDInfoBuildItem unownedCRDInfo) { final var crdConfig = operatorConfiguration.crd(); final boolean validateCustomResources = ConfigurationUtils.shouldValidateCustomResources(crdConfig.validate()); @@ -51,23 +56,32 @@ GeneratedCRDInfoBuildItem generateCRDs( final var changedClasses = ConfigurationUtils.getChangedClasses(liveReload); final var scheduledForGeneration = new HashSet(7); + final var excludedResourceClasses = crdConfig.excludeResources().map(Set::copyOf).orElseGet(Collections::emptySet); + final var externalCRDs = unownedCRDInfo.getCRDs(); + // predicate to decide whether or not to consider a given resource for generation + Function keepResourcePredicate = ( + CustomResourceAugmentedClassInfo crInfo) -> !isExcluded(crInfo, externalCRDs, excludedResourceClasses); + if (generate) { - reconcilers.getReconcilers().values().forEach(raci -> { - // add associated primary resource for CRD generation if it's a CR and it's owned by the reconciler - final ReconciledAugmentedClassInfo associatedResource = raci.associatedResourceInfo(); - if (associatedResource.isCR()) { - final var crInfo = associatedResource.asResourceTargeting(); - 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(crId); - } else { - // When we have a live reload, check if we need to regenerate the associated CRD - Map crdInfos = Collections.emptyMap(); - if (liveReload.isLiveReload()) { - crdInfos = storedCRDInfos.getOrCreateCRDSpecVersionToInfoMapping(crId); - } + reconcilers.getReconcilers().values().stream() + .map(ResourceAssociatedAugmentedClassInfo::associatedResourceInfo) + .filter(ReconciledAugmentedClassInfo::isCR) // only keep CRs + .map(CustomResourceAugmentedClassInfo.class::cast) + .filter(keepResourcePredicate::apply) + .forEach(associatedResource -> { + final var crInfo = associatedResource.asResourceTargeting(); + 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(associatedResource.getAssociatedReconcilerName().orElseThrow())) { + scheduledForGeneration.add(crId); + } else { + // When we have a live reload, check if we need to regenerate the associated CRD + Map crdInfos = Collections.emptyMap(); + if (liveReload.isLiveReload()) { + crdInfos = storedCRDInfos.getOrCreateCRDSpecVersionToInfoMapping(crId); + } // schedule the generation of associated primary resource CRD if required if (crdGeneration.scheduleForGenerationIfNeeded((CustomResourceAugmentedClassInfo) crInfo, crdInfos, @@ -75,13 +89,18 @@ GeneratedCRDInfoBuildItem generateCRDs( scheduledForGeneration.add(crId); } } - }); + }); // generate non-reconciler associated CRDs if requested if (crdConfig.generateAll()) { + // only process CRs that haven't been already considered and are not excluded + keepResourcePredicate = ( + CustomResourceAugmentedClassInfo crInfo) -> !scheduledForGeneration.contains(crInfo.id()) + && !isExcluded(crInfo, externalCRDs, excludedResourceClasses); + final Map context = Map.of(CustomResourceAugmentedClassInfo.KEEP_CR_PREDICATE_KEY, + keepResourcePredicate); ClassUtils.getProcessableSubClassesOf(Constants.CUSTOM_RESOURCE, combinedIndexBuildItem.getIndex(), log, - // pass already generated CRD names so that we can only keep the unhandled ones - Map.of(CustomResourceAugmentedClassInfo.EXISTING_CRDS_KEY, scheduledForGeneration)) + context) .map(CustomResourceAugmentedClassInfo.class::cast) .forEach(cr -> { crdGeneration.withCustomResource(cr.loadAssociatedClass(), null); @@ -117,4 +136,22 @@ UnownedCRDInfoBuildItem unownedCRDInfo(BuildTimeOperatorConfiguration operatorCo } return new UnownedCRDInfoBuildItem(crds); } + + /** + * Exclude all resources that shouldn't be generated because either they've been explicitly excluded or because they're + * supposed to be loaded directly from a specified CRD file + */ + private boolean isExcluded(CustomResourceAugmentedClassInfo crInfo, CRDInfos externalCRDs, + Set excludedResourceClassNames) { + final var crClassName = crInfo.classInfo().name().toString(); + final var excluded = excludedResourceClassNames.contains(crClassName); + final var external = externalCRDs.contains(crInfo.id()); + if (excluded || external) { + log.infov("CRD generation was skipped for ''{0}'' because {1}", crClassName, + external ? externalCause : excludedCause); + return true; + } else { + return false; + } + } } diff --git a/core/runtime/src/main/java/io/quarkiverse/operatorsdk/runtime/CRDInfos.java b/core/runtime/src/main/java/io/quarkiverse/operatorsdk/runtime/CRDInfos.java index 5ffb6d68..b912e738 100644 --- a/core/runtime/src/main/java/io/quarkiverse/operatorsdk/runtime/CRDInfos.java +++ b/core/runtime/src/main/java/io/quarkiverse/operatorsdk/runtime/CRDInfos.java @@ -49,4 +49,8 @@ public void addCRDInfo(CRDInfo crdInfo) { public Map> getInfos() { return infos; } + + public boolean contains(String crdId) { + return infos.containsKey(crdId); + } } From 2f50ae97a6c268181a2c8db2f633a4e9b456d8ec Mon Sep 17 00:00:00 2001 From: Chris Laprun Date: Fri, 8 Nov 2024 10:09:21 +0100 Subject: [PATCH 5/5] fix: clarify when id or full resource name is needed Signed-off-by: Chris Laprun --- .../common/ReconciledResourceAugmentedClassInfo.java | 9 ++++++--- .../operatorsdk/deployment/CRDGenerationBuildStep.java | 9 +++++---- 2 files changed, 11 insertions(+), 7 deletions(-) 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 2754ea06..21a33fe1 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 @@ -14,17 +14,20 @@ public class ReconciledResourceAugmentedClassInfo extends public static final String STATUS = "status"; protected boolean hasStatus; - private final String id; + private final Id id; + + public record Id(String fullResourceName, String version) { + } protected ReconciledResourceAugmentedClassInfo(ClassInfo classInfo, DotName extendedOrImplementedClass, int expectedParameterTypesCardinality, String associatedReconcilerName) { super(classInfo, extendedOrImplementedClass, expectedParameterTypesCardinality, associatedReconcilerName); - id = fullResourceName() + version(); + id = new Id(fullResourceName(), version()); } - public String id() { + public Id id() { return id; } 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 01070337..1273572a 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 @@ -54,7 +54,7 @@ GeneratedCRDInfoBuildItem generateCRDs( final var generate = CRDGeneration.shouldGenerate(crdConfig.generate(), crdConfig.apply(), launchMode); final var storedCRDInfos = stored; final var changedClasses = ConfigurationUtils.getChangedClasses(liveReload); - final var scheduledForGeneration = new HashSet(7); + final var scheduledForGeneration = new HashSet(7); final var excludedResourceClasses = crdConfig.excludeResources().map(Set::copyOf).orElseGet(Collections::emptySet); final var externalCRDs = unownedCRDInfo.getCRDs(); @@ -70,7 +70,7 @@ GeneratedCRDInfoBuildItem generateCRDs( .filter(keepResourcePredicate::apply) .forEach(associatedResource -> { final var crInfo = associatedResource.asResourceTargeting(); - final String crId = crInfo.id(); + final var 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 @@ -80,7 +80,7 @@ GeneratedCRDInfoBuildItem generateCRDs( // When we have a live reload, check if we need to regenerate the associated CRD Map crdInfos = Collections.emptyMap(); if (liveReload.isLiveReload()) { - crdInfos = storedCRDInfos.getOrCreateCRDSpecVersionToInfoMapping(crId); + crdInfos = storedCRDInfos.getOrCreateCRDSpecVersionToInfoMapping(crInfo.fullResourceName()); } // schedule the generation of associated primary resource CRD if required @@ -141,11 +141,12 @@ UnownedCRDInfoBuildItem unownedCRDInfo(BuildTimeOperatorConfiguration operatorCo * Exclude all resources that shouldn't be generated because either they've been explicitly excluded or because they're * supposed to be loaded directly from a specified CRD file */ + @SuppressWarnings("BooleanMethodIsAlwaysInverted") private boolean isExcluded(CustomResourceAugmentedClassInfo crInfo, CRDInfos externalCRDs, Set excludedResourceClassNames) { final var crClassName = crInfo.classInfo().name().toString(); final var excluded = excludedResourceClassNames.contains(crClassName); - final var external = externalCRDs.contains(crInfo.id()); + final var external = externalCRDs.contains(crInfo.fullResourceName()); if (excluded || external) { log.infov("CRD generation was skipped for ''{0}'' because {1}", crClassName, external ? externalCause : excludedCause);