From d5df86c916998838d2b0aaf214d41dbb017e2097 Mon Sep 17 00:00:00 2001 From: grokspawn Date: Fri, 30 Oct 2026 13:21:42 -0500 Subject: [PATCH 1/3] CE explicitly signals source Signed-off-by: grokspawn --- cmd/operator-controller/main.go | 20 +- .../controllers/bundle_source.go | 70 ++++ .../controllers/catalog_bundle_source.go | 252 ++++++++++++++ .../clusterextension_controller.go | 56 +-- .../clusterextension_reconcile_steps.go | 258 ++------------ .../clusterextension_source_test.go | 327 ++++++++++++++++++ .../controllers/suite_test.go | 5 +- 7 files changed, 720 insertions(+), 268 deletions(-) create mode 100644 internal/operator-controller/controllers/bundle_source.go create mode 100644 internal/operator-controller/controllers/catalog_bundle_source.go create mode 100644 internal/operator-controller/controllers/clusterextension_source_test.go diff --git a/cmd/operator-controller/main.go b/cmd/operator-controller/main.go index 98461c743f..72a3c847b4 100644 --- a/cmd/operator-controller/main.go +++ b/cmd/operator-controller/main.go @@ -109,7 +109,7 @@ type boxcutterReconcilerConfigurator struct { mgr manager.Manager preflights []applier.Preflight regv1ManifestProvider applier.ManifestProvider - resolver resolve.Resolver + bundleSources controllers.BundleSources imageCache imageutil.Cache imagePuller imageutil.Puller finalizers crfinalizer.Finalizers @@ -119,7 +119,7 @@ type helmReconcilerConfigurator struct { mgr manager.Manager preflights []applier.Preflight regv1ManifestProvider applier.ManifestProvider - resolver resolve.Resolver + bundleSources controllers.BundleSources imageCache imageutil.Cache imagePuller imageutil.Puller finalizers crfinalizer.Finalizers @@ -471,8 +471,12 @@ func run() error { } } + bundleSources := controllers.BundleSources{ + ocv1.SourceTypeCatalog: &controllers.CatalogBundleSource{Resolver: resolver, Client: cl}, + } ceReconciler := &controllers.ClusterExtensionReconciler{ - Client: cl, + Client: cl, + BundleSources: bundleSources, } if features.OperatorControllerFeatureGate.Enabled(features.BoxcutterRuntime) { _, err = ceReconciler.SetupWithManagerForBoxcutter(mgr) @@ -499,7 +503,7 @@ func run() error { mgr: mgr, preflights: preflights, regv1ManifestProvider: regv1ManifestProvider, - resolver: resolver, + bundleSources: bundleSources, imageCache: imageCache, imagePuller: imagePuller, finalizers: clusterExtensionFinalizers, @@ -509,7 +513,7 @@ func run() error { mgr: mgr, preflights: preflights, regv1ManifestProvider: regv1ManifestProvider, - resolver: resolver, + bundleSources: bundleSources, imageCache: imageCache, imagePuller: imagePuller, finalizers: clusterExtensionFinalizers, @@ -635,6 +639,7 @@ func (c *boxcutterReconcilerConfigurator) Configure(ceReconciler *controllers.Cl RevisionGenerator: rg, FieldOwner: fieldOwner, } + ceReconciler.BundleSources = c.bundleSources ceReconciler.ReconcileSteps = []controllers.ReconcileStepFunc{ controllers.HandleFinalizers(c.finalizers), controllers.ValidateClusterExtension( @@ -642,7 +647,7 @@ func (c *boxcutterReconcilerConfigurator) Configure(ceReconciler *controllers.Cl ), controllers.MigrateStorage(storageMigrator), controllers.RetrieveRevisionStates(revisionStatesGetter), - controllers.ResolveBundle(c.resolver, c.mgr.GetClient()), + controllers.ResolveBundle(c.bundleSources), controllers.UnpackBundle(c.imagePuller, c.imageCache), controllers.ValidateInstallNamespace(coreClient), controllers.ApplyBundleWithBoxcutter(appl.Apply), @@ -693,13 +698,14 @@ func (c *helmReconcilerConfigurator) Configure(ceReconciler *controllers.Cluster TrackingCache: c.trackingCache, } revisionStatesGetter := &controllers.HelmRevisionStatesGetter{ActionClientGetter: acg} + ceReconciler.BundleSources = c.bundleSources ceReconciler.ReconcileSteps = []controllers.ReconcileStepFunc{ controllers.HandleFinalizers(c.finalizers), controllers.ValidateClusterExtension( controllers.ServiceAccountDeprecationWarning(), ), controllers.RetrieveRevisionStates(revisionStatesGetter), - controllers.ResolveBundle(c.resolver, c.mgr.GetClient()), + controllers.ResolveBundle(c.bundleSources), controllers.UnpackBundle(c.imagePuller, c.imageCache), controllers.ApplyBundle(appl), } diff --git a/internal/operator-controller/controllers/bundle_source.go b/internal/operator-controller/controllers/bundle_source.go new file mode 100644 index 0000000000..7444467480 --- /dev/null +++ b/internal/operator-controller/controllers/bundle_source.go @@ -0,0 +1,70 @@ +/* +Copyright 2026. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package controllers + +import ( + "context" + "fmt" + "reflect" + + "github.com/operator-framework/operator-registry/alpha/declcfg" + + ocv1 "github.com/operator-framework/operator-controller/api/v1" +) + +// BundleSource owns resolution and lifecycle policies for an installation source. +// Resolve includes rollout continuation and recovery from resolution errors. +// SetDeprecationStatus is called even on resolution errors, and again after a +// successful apply with the newly installed bundle name. +type BundleSource interface { + Resolve(context.Context, *ocv1.ClusterExtension, *RevisionStates) (BundleResolution, error) + SetDeprecationStatus(ext *ocv1.ClusterExtension, installedBundleName string, resolution BundleResolution) + ReconcileOnCatalogChange(*ocv1.ClusterExtension, *ocv1.ClusterCatalog) bool + CanMaintainWithoutContent(*ocv1.ClusterExtension, *RevisionStates, BundleResolution) bool +} + +// BundleResolution carries the selected revision and source-provided deprecation +// evidence. A nil Deprecation with DeprecationKnown true means known-empty data. +// Results returned alongside an error still carry evidence for status reporting. +type BundleResolution struct { + Revision *RevisionMetadata + Deprecation *declcfg.Deprecation + DeprecationKnown bool +} + +// BundleSources selects a source policy using spec.source.sourceType. +type BundleSources map[string]BundleSource + +// For returns the configured policy or an error before any source is invoked. +func (sources BundleSources) For(ext *ocv1.ClusterExtension) (BundleSource, error) { + source := sources[ext.Spec.Source.SourceType] + if source == nil || nilBundleSource(source) { + return nil, fmt.Errorf("unsupported sourceType %q: bundle source is not configured", ext.Spec.Source.SourceType) + } + return source, nil +} + +// Interfaces can contain typed nil implementations; reject these at lookup too. +func nilBundleSource(source BundleSource) bool { + v := reflect.ValueOf(source) + switch v.Kind() { + case reflect.Chan, reflect.Func, reflect.Interface, reflect.Map, reflect.Pointer, reflect.Slice: + return v.IsNil() + default: + return false + } +} diff --git a/internal/operator-controller/controllers/catalog_bundle_source.go b/internal/operator-controller/controllers/catalog_bundle_source.go new file mode 100644 index 0000000000..098662d9b3 --- /dev/null +++ b/internal/operator-controller/controllers/catalog_bundle_source.go @@ -0,0 +1,252 @@ +/* +Copyright 2026. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package controllers + +import ( + "context" + "fmt" + + apimeta "k8s.io/apimachinery/pkg/api/meta" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "sigs.k8s.io/controller-runtime/pkg/client" + "sigs.k8s.io/controller-runtime/pkg/log" + + ocv1 "github.com/operator-framework/operator-controller/api/v1" + "github.com/operator-framework/operator-controller/internal/operator-controller/bundleutil" + "github.com/operator-framework/operator-controller/internal/operator-controller/resolve" +) + +// CatalogBundleSource preserves Catalog resolution and lifecycle policies. +type CatalogBundleSource struct { + Resolver resolve.Resolver + Client client.Client +} + +var _ BundleSource = (*CatalogBundleSource)(nil) + +// Resolve continues the first rolling revision or resolves a Catalog bundle, +// retaining the existing catalog-deletion fallback and deprecation evidence rule. +func (s *CatalogBundleSource) Resolve(ctx context.Context, ext *ocv1.ClusterExtension, revisions *RevisionStates) (BundleResolution, error) { + if ext.Spec.Source.Catalog == nil { + return BundleResolution{}, fmt.Errorf("catalog source requires spec.source.catalog") + } + if len(revisions.RollingOut) > 0 { + return BundleResolution{Revision: revisions.RollingOut[0]}, nil + } + log.FromContext(ctx).V(1).Info("resolving bundle") + var bm *ocv1.BundleMetadata + if revisions.Installed != nil { + bm = &revisions.Installed.BundleMetadata + } + bundle, version, deprecation, err := s.Resolver.Resolve(ctx, ext, bm) + // Once the catalog resolver selects one package, it returns that catalog's + // deprecation data even when no bundle matches. An empty, non-nil deprecation + // means the selected catalog has no deprecation entries; nil on failure means + // no catalog package was selected. Keep the success check for resolver + // implementations that may return nil on success. + result := BundleResolution{Deprecation: deprecation, DeprecationKnown: err == nil || deprecation != nil} + if err != nil { + state := &reconcileState{revisionStates: revisions} + err = handleCatalogResolutionError(ctx, s.Client, state, ext, err) + result.Revision = state.resolvedRevisionMetadata + return result, err + } + result.Revision = &RevisionMetadata{ + Package: bundle.Package, + Image: bundle.Image, + // Preserve MetadataFor's feature-gated version/release normalization. + BundleMetadata: bundleutil.MetadataFor(bundle.Name, *version), + } + return result, nil +} + +// SetDeprecationStatus applies Catalog deprecation evidence, including on errors. +func (s *CatalogBundleSource) SetDeprecationStatus(ext *ocv1.ClusterExtension, installedBundleName string, resolution BundleResolution) { + SetDeprecationStatus(ext, installedBundleName, resolution.Deprecation, resolution.DeprecationKnown) +} + +// ReconcileOnCatalogChange preserves the existing fan-out to Catalog extensions. +// Catalog-to-extension provenance and selector-specific filtering remain unchanged. +func (s *CatalogBundleSource) ReconcileOnCatalogChange(*ocv1.ClusterExtension, *ocv1.ClusterCatalog) bool { + return true +} + +// CanMaintainWithoutContent preserves the original name/version comparison. +func (s *CatalogBundleSource) CanMaintainWithoutContent(_ *ocv1.ClusterExtension, revisions *RevisionStates, resolution BundleResolution) bool { + return revisions != nil && revisions.Installed != nil && resolution.Revision != nil && + resolution.Revision.Name == revisions.Installed.Name && + resolution.Revision.Version == revisions.Installed.Version +} + +// handleCatalogResolutionError owns recovery when Catalog resolution fails. +// +// Decision logic (evaluated in order): +// 1. No installed bundle → Retry (cannot proceed without any bundle) +// 2. Version change requested → Retry (cannot upgrade without catalog) +// 3. Cannot check catalog existence → Retry (API error, cannot safely decide) +// 4. Catalogs exist → Retry (transient error, catalog may be updating) +// 5. Catalogs deleted → Fallback to installed bundle (maintain current state) +// +// When falling back (case 5), we set the resolved bundle to the installed bundle and return +// no error, allowing the Apply step to run and maintain resources using the existing installation. +// The controller watches ClusterCatalog resources, so reconciliation will automatically resume +// when catalogs return, enabling upgrades. +func handleCatalogResolutionError(ctx context.Context, c client.Client, state *reconcileState, ext *ocv1.ClusterExtension, err error) error { + l := log.FromContext(ctx) + + // No installed bundle and resolution failed - cannot proceed + if state.revisionStates.Installed == nil { + msg := fmt.Sprintf("failed to resolve bundle: %v", err) + setStatusProgressing(ext, err) + setInstalledStatusFromRevisionStates(ext, state.revisionStates) + ensureFailureConditionsWithReason(ext, ocv1.ReasonRetrying, msg) + return err + } + + // Check if the spec is requesting a specific version that differs from installed + specVersion := "" + if ext.Spec.Source.Catalog != nil { + specVersion = ext.Spec.Source.Catalog.Version + } + installedVersion := state.revisionStates.Installed.Version + + // If spec requests a different version, we cannot fall back - must fail and retry + if specVersion != "" && specVersion != installedVersion { + msg := fmt.Sprintf("unable to upgrade to version %s: %v (currently installed: %s)", specVersion, err, installedVersion) + l.Error(err, "resolution failed and spec requests version change - cannot fall back", + "requestedVersion", specVersion, + "installedVersion", installedVersion) + setStatusProgressing(ext, err) + setInstalledStatusFromRevisionStates(ext, state.revisionStates) + ensureFailureConditionsWithReason(ext, ocv1.ReasonRetrying, msg) + return err + } + + // No version change requested - check if ClusterCatalogs exist + // Only fall back if ClusterCatalogs have been deleted + catalogsExist, catalogCheckErr := CheckCatalogsExist(ctx, c, ext) + if catalogCheckErr != nil { + msg := fmt.Sprintf("failed to resolve bundle: %v", err) + var catalogName string + if ext.Spec.Source.Catalog != nil { + catalogName = getCatalogNameFromSelector(ext.Spec.Source.Catalog.Selector) + } + l.Error(catalogCheckErr, "error checking if ClusterCatalogs exist, will retry resolution", + "resolutionError", err, + "packageName", getPackageName(ext), + "catalogName", catalogName) + setStatusProgressing(ext, err) + setInstalledStatusFromRevisionStates(ext, state.revisionStates) + ensureFailureConditionsWithReason(ext, ocv1.ReasonRetrying, msg) + return err + } + + if catalogsExist { + // ClusterCatalogs exist but resolution failed - likely a transient issue (ClusterCatalog updating, cache stale, etc.) + // Retry resolution instead of falling back + msg := fmt.Sprintf("failed to resolve bundle, retrying: %v", err) + var catalogName string + if ext.Spec.Source.Catalog != nil { + catalogName = getCatalogNameFromSelector(ext.Spec.Source.Catalog.Selector) + } + l.Error(err, "resolution failed but matching ClusterCatalogs exist - retrying instead of falling back", + "packageName", getPackageName(ext), + "catalogName", catalogName) + setStatusProgressing(ext, err) + setInstalledStatusFromRevisionStates(ext, state.revisionStates) + ensureFailureConditionsWithReason(ext, ocv1.ReasonRetrying, msg) + return err + } + + // ClusterCatalogs don't exist (deleted) - fall back to installed bundle to maintain current state. + // The controller watches ClusterCatalog resources, so when ClusterCatalogs become available again, + // a reconcile will be triggered automatically, allowing the extension to upgrade. + var catalogName string + if ext.Spec.Source.Catalog != nil { + catalogName = getCatalogNameFromSelector(ext.Spec.Source.Catalog.Selector) + } + l.Info("matching ClusterCatalogs unavailable or deleted - falling back to installed bundle to maintain workload", + "resolutionError", err.Error(), + "packageName", getPackageName(ext), + "catalogName", catalogName, + "installedBundle", state.revisionStates.Installed.Name, + "installedVersion", state.revisionStates.Installed.Version) + // Set installed status based on current revision states (needed before Apply runs) + setInstalledStatusFromRevisionStates(ext, state.revisionStates) + state.resolvedRevisionMetadata = state.revisionStates.Installed + // Return no error to allow Apply step to run and maintain resources. + // Apply will set Progressing=Succeeded when it completes successfully. + return nil +} + +// getCatalogNameFromSelector extracts the catalog name from the selector if available. +// Returns empty string if selector is nil or doesn't contain the metadata.name label. +func getCatalogNameFromSelector(selector *metav1.LabelSelector) string { + if selector == nil || selector.MatchLabels == nil { + return "" + } + return selector.MatchLabels["olm.operatorframework.io/metadata.name"] +} + +// getPackageName safely extracts the package name from the extension spec. +// Returns empty string if Catalog source is nil. +func getPackageName(ext *ocv1.ClusterExtension) string { + if ext.Spec.Source.Catalog == nil { + return "" + } + return ext.Spec.Source.Catalog.PackageName +} + +// CheckCatalogsExist checks if any ClusterCatalogs matching the extension's selector exist. +// Returns true if at least one matching ClusterCatalog exists, false if none exist. +// Treats "CRD doesn't exist" errors as "no ClusterCatalogs exist" (returns false, nil). +// Returns an error only if the check itself fails unexpectedly. +func CheckCatalogsExist(ctx context.Context, c client.Client, ext *ocv1.ClusterExtension) (bool, error) { + if ext.Spec.Source.SourceType != ocv1.SourceTypeCatalog || ext.Spec.Source.Catalog == nil { + return false, fmt.Errorf("catalog existence checks require a Catalog source with spec.source.catalog") + } + var catalogList *ocv1.ClusterCatalogList + var listErr error + + if ext.Spec.Source.Catalog.Selector == nil { + // No selector means all ClusterCatalogs match - check if any ClusterCatalogs exist at all + catalogList = &ocv1.ClusterCatalogList{} + listErr = c.List(ctx, catalogList, client.Limit(1)) + } else { + // Convert label selector to k8slabels.Selector + // Note: An empty LabelSelector matches everything by default + selector, err := metav1.LabelSelectorAsSelector(ext.Spec.Source.Catalog.Selector) + if err != nil { + return false, fmt.Errorf("invalid catalog selector: %w", err) + } + + // List ClusterCatalogs matching the selector (limit to 1 since we only care if any exist) + catalogList = &ocv1.ClusterCatalogList{} + listErr = c.List(ctx, catalogList, client.MatchingLabelsSelector{Selector: selector}, client.Limit(1)) + } + + if listErr != nil { + // Check if the error is because the ClusterCatalog CRD doesn't exist + // This can happen if catalogd is not installed, which means no ClusterCatalogs exist + if apimeta.IsNoMatchError(listErr) { + return false, nil + } + return false, fmt.Errorf("failed to list ClusterCatalogs: %w", listErr) + } + + return len(catalogList.Items) > 0, nil +} diff --git a/internal/operator-controller/controllers/clusterextension_controller.go b/internal/operator-controller/controllers/clusterextension_controller.go index 8099f29960..2160922a08 100644 --- a/internal/operator-controller/controllers/clusterextension_controller.go +++ b/internal/operator-controller/controllers/clusterextension_controller.go @@ -64,8 +64,8 @@ type reconcileState struct { revisionStates *RevisionStates resolvedRevisionMetadata *RevisionMetadata imageFS fs.FS - resolvedDeprecation *declcfg.Deprecation - hasCatalogData bool + bundleSource BundleSource + bundleResolution BundleResolution } // ReconcileStepFunc represents a single step in the ClusterExtension reconciliation process. @@ -103,6 +103,7 @@ func (steps *ReconcileSteps) Reconcile(ctx context.Context, ext *ocv1.ClusterExt type ClusterExtensionReconciler struct { client.Client ReconcileSteps ReconcileSteps + BundleSources BundleSources } type StorageMigrator interface { @@ -457,24 +458,23 @@ func (r *ClusterExtensionReconciler) newControllerBuilder(mgr ctrl.Manager) *ctr For(&ocv1.ClusterExtension{}). Named("controller-operator-cluster-extension-controller"). Watches(&ocv1.ClusterCatalog{}, - crhandler.EnqueueRequestsFromMapFunc(clusterExtensionRequestsForCatalog(mgr.GetClient(), mgr.GetLogger())), - builder.WithPredicates(predicate.Funcs{ - UpdateFunc: func(ue event.UpdateEvent) bool { - oldObject, isOldCatalog := ue.ObjectOld.(*ocv1.ClusterCatalog) - newObject, isNewCatalog := ue.ObjectNew.(*ocv1.ClusterCatalog) - - if !isOldCatalog || !isNewCatalog { - return true - } - - if oldObject.Status.ResolvedSource != nil && newObject.Status.ResolvedSource != nil { - if oldObject.Status.ResolvedSource.Image != nil && newObject.Status.ResolvedSource.Image != nil { - return oldObject.Status.ResolvedSource.Image.Ref != newObject.Status.ResolvedSource.Image.Ref - } - } - return true - }, - })) + crhandler.EnqueueRequestsFromMapFunc(clusterExtensionRequestsForCatalog(mgr.GetClient(), mgr.GetLogger(), r.BundleSources)), + builder.WithPredicates(predicate.Funcs{UpdateFunc: catalogResolutionInputsChanged})) +} + +// Preserve the existing catalog resolved-image update predicate. +func catalogResolutionInputsChanged(ue event.UpdateEvent) bool { + oldCatalog, oldOK := ue.ObjectOld.(*ocv1.ClusterCatalog) + newCatalog, newOK := ue.ObjectNew.(*ocv1.ClusterCatalog) + if !oldOK || !newOK { + return true + } + if oldCatalog.Status.ResolvedSource != nil && newCatalog.Status.ResolvedSource != nil { + if oldCatalog.Status.ResolvedSource.Image != nil && newCatalog.Status.ResolvedSource.Image != nil { + return oldCatalog.Status.ResolvedSource.Image.Ref != newCatalog.Status.ResolvedSource.Image.Ref + } + } + return true } func wrapErrorWithResolutionInfo(resolved ocv1.BundleMetadata, err error) error { @@ -495,11 +495,13 @@ func wrapErrorWithResolutionInfo(resolved ocv1.BundleMetadata, err error) error } // Generate reconcile requests for all cluster extensions affected by a catalog change -func clusterExtensionRequestsForCatalog(c client.Reader, logger logr.Logger) crhandler.MapFunc { - return func(ctx context.Context, _ client.Object) []reconcile.Request { - // no way of associating an extension to a catalog so create reconcile requests for everything - clusterExtensions := metav1.PartialObjectMetadataList{} - clusterExtensions.SetGroupVersionKind(ocv1.GroupVersion.WithKind("ClusterExtensionList")) +func clusterExtensionRequestsForCatalog(c client.Reader, logger logr.Logger, sources BundleSources) crhandler.MapFunc { + return func(ctx context.Context, obj client.Object) []reconcile.Request { + catalog, ok := obj.(*ocv1.ClusterCatalog) + if !ok { + return nil + } + clusterExtensions := ocv1.ClusterExtensionList{} err := c.List(ctx, &clusterExtensions) if err != nil { logger.Error(err, "unable to enqueue cluster extensions for catalog reconcile") @@ -507,6 +509,10 @@ func clusterExtensionRequestsForCatalog(c client.Reader, logger logr.Logger) crh } var requests []reconcile.Request for _, ext := range clusterExtensions.Items { + source, err := sources.For(&ext) + if err != nil || !source.ReconcileOnCatalogChange(&ext, catalog) { + continue + } requests = append(requests, reconcile.Request{ NamespacedName: types.NamespacedName{ Namespace: ext.GetNamespace(), diff --git a/internal/operator-controller/controllers/clusterextension_reconcile_steps.go b/internal/operator-controller/controllers/clusterextension_reconcile_steps.go index 8341c034e8..9a342f58b3 100644 --- a/internal/operator-controller/controllers/clusterextension_reconcile_steps.go +++ b/internal/operator-controller/controllers/clusterextension_reconcile_steps.go @@ -26,13 +26,10 @@ import ( metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" corev1client "k8s.io/client-go/kubernetes/typed/core/v1" ctrl "sigs.k8s.io/controller-runtime" - "sigs.k8s.io/controller-runtime/pkg/client" "sigs.k8s.io/controller-runtime/pkg/finalizer" "sigs.k8s.io/controller-runtime/pkg/log" ocv1 "github.com/operator-framework/operator-controller/api/v1" - "github.com/operator-framework/operator-controller/internal/operator-controller/bundleutil" - "github.com/operator-framework/operator-controller/internal/operator-controller/resolve" "github.com/operator-framework/operator-controller/internal/shared/labels" imageutil "github.com/operator-framework/operator-controller/internal/shared/util/image" ) @@ -126,227 +123,38 @@ func RetrieveRevisionStates(r RevisionStatesGetter) ReconcileStepFunc { } } -// ResolveBundle resolves the bundle to install or roll out for a ClusterExtension. -// It requires a controller-runtime client (in addition to the resolve.Resolver) to enable -// intelligent error handling when resolution fails. The client is used to check if ClusterCatalogs -// matching the extension's selector still exist in the cluster, allowing the controller to -// distinguish between "ClusterCatalog deleted" (fall back to installed bundle) and "transient failure" -// (retry resolution). This ensures workload resilience during ClusterCatalog outages while maintaining -// responsiveness during ClusterCatalog updates. -func ResolveBundle(r resolve.Resolver, c client.Client) ReconcileStepFunc { +// ResolveBundle selects the extension's source policy before resolving, including +// when a revision is already rolling out. Sources own recovery and status evidence. +func ResolveBundle(sources BundleSources) ReconcileStepFunc { return func(ctx context.Context, state *reconcileState, ext *ocv1.ClusterExtension) (*ctrl.Result, error) { - l := log.FromContext(ctx) - - // If already rolling out, use existing revision and set deprecation to Unknown (no catalog check) - if len(state.revisionStates.RollingOut) > 0 { - installedBundleName := "" - if state.revisionStates.Installed != nil { - installedBundleName = state.revisionStates.Installed.Name - } - SetDeprecationStatus(ext, installedBundleName, nil, false) - state.resolvedRevisionMetadata = state.revisionStates.RollingOut[0] - return nil, nil - } - - // Resolve a new bundle from the catalog - l.V(1).Info("resolving bundle") - var bm *ocv1.BundleMetadata - if state.revisionStates.Installed != nil { - bm = &state.revisionStates.Installed.BundleMetadata + source, err := sources.For(ext) + if err != nil { + setStatusProgressing(ext, err) + setInstalledStatusFromRevisionStates(ext, state.revisionStates) + ensureFailureConditionsWithReason(ext, ocv1.ReasonRetrying, fmt.Sprintf("failed to resolve bundle: %v", err)) + return nil, err } - resolvedBundle, resolvedBundleVersion, resolvedDeprecation, err := r.Resolve(ctx, ext, bm) - - // Get the installed bundle name for deprecation status. - // BundleDeprecated should reflect what's currently running, not what we're trying to install. + state.bundleSource = source + resolution, err := source.Resolve(ctx, ext, state.revisionStates) + state.bundleResolution = resolution installedBundleName := "" if state.revisionStates.Installed != nil { installedBundleName = state.revisionStates.Installed.Name } - // Once the catalog resolver selects one package, it returns that catalog's deprecation - // data even when no bundle matches. An empty, non-nil deprecation means the selected - // catalog has no deprecation entries; nil on failure means no catalog was selected. - // Keep the success check for other resolver implementations that may return nil. - hasCatalogData := err == nil || resolvedDeprecation != nil - state.resolvedDeprecation = resolvedDeprecation - state.hasCatalogData = hasCatalogData - SetDeprecationStatus(ext, installedBundleName, resolvedDeprecation, hasCatalogData) + source.SetDeprecationStatus(ext, installedBundleName, resolution) if err != nil { - return handleResolutionError(ctx, c, state, ext, err) - } - - state.resolvedRevisionMetadata = &RevisionMetadata{ - Package: resolvedBundle.Package, - Image: resolvedBundle.Image, - // MetadataFor accepts VersionRelease input and normalizes bundle metadata. - // - With BundleReleaseSupport enabled, it stores release information in BundleMetadata.Release - // (whether provided explicitly via pkg.Release or extracted from registry+v1 build metadata, - // e.g. "1.0.0+2" -> Release "2"). - // - Without BundleReleaseSupport, it preserves legacy behavior by embedding release information - // back into BundleMetadata.Version instead of emitting a separate Release field. - BundleMetadata: bundleutil.MetadataFor(resolvedBundle.Name, *resolvedBundleVersion), + setStatusProgressing(ext, err) + setInstalledStatusFromRevisionStates(ext, state.revisionStates) + ensureFailureConditionsWithReason(ext, ocv1.ReasonRetrying, fmt.Sprintf("failed to resolve bundle: %v", err)) + return nil, err } + state.resolvedRevisionMetadata = resolution.Revision return nil, nil } } -// handleResolutionError handles the case when bundle resolution fails. -// -// Decision logic (evaluated in order): -// 1. No installed bundle → Retry (cannot proceed without any bundle) -// 2. Version change requested → Retry (cannot upgrade without catalog) -// 3. Cannot check catalog existence → Retry (API error, cannot safely decide) -// 4. Catalogs exist → Retry (transient error, catalog may be updating) -// 5. Catalogs deleted → Fallback to installed bundle (maintain current state) -// -// When falling back (case 5), we set the resolved bundle to the installed bundle and return -// no error, allowing the Apply step to run and maintain resources using the existing installation. -// The controller watches ClusterCatalog resources, so reconciliation will automatically resume -// when catalogs return, enabling upgrades. -func handleResolutionError(ctx context.Context, c client.Client, state *reconcileState, ext *ocv1.ClusterExtension, err error) (*ctrl.Result, error) { - l := log.FromContext(ctx) - - // No installed bundle and resolution failed - cannot proceed - if state.revisionStates.Installed == nil { - msg := fmt.Sprintf("failed to resolve bundle: %v", err) - setStatusProgressing(ext, err) - setInstalledStatusFromRevisionStates(ext, state.revisionStates) - ensureFailureConditionsWithReason(ext, ocv1.ReasonRetrying, msg) - return nil, err - } - - // Check if the spec is requesting a specific version that differs from installed - specVersion := "" - if ext.Spec.Source.Catalog != nil { - specVersion = ext.Spec.Source.Catalog.Version - } - installedVersion := state.revisionStates.Installed.Version - - // If spec requests a different version, we cannot fall back - must fail and retry - if specVersion != "" && specVersion != installedVersion { - msg := fmt.Sprintf("unable to upgrade to version %s: %v (currently installed: %s)", specVersion, err, installedVersion) - l.Error(err, "resolution failed and spec requests version change - cannot fall back", - "requestedVersion", specVersion, - "installedVersion", installedVersion) - setStatusProgressing(ext, err) - setInstalledStatusFromRevisionStates(ext, state.revisionStates) - ensureFailureConditionsWithReason(ext, ocv1.ReasonRetrying, msg) - return nil, err - } - - // No version change requested - check if ClusterCatalogs exist - // Only fall back if ClusterCatalogs have been deleted - catalogsExist, catalogCheckErr := CheckCatalogsExist(ctx, c, ext) - if catalogCheckErr != nil { - msg := fmt.Sprintf("failed to resolve bundle: %v", err) - var catalogName string - if ext.Spec.Source.Catalog != nil { - catalogName = getCatalogNameFromSelector(ext.Spec.Source.Catalog.Selector) - } - l.Error(catalogCheckErr, "error checking if ClusterCatalogs exist, will retry resolution", - "resolutionError", err, - "packageName", getPackageName(ext), - "catalogName", catalogName) - setStatusProgressing(ext, err) - setInstalledStatusFromRevisionStates(ext, state.revisionStates) - ensureFailureConditionsWithReason(ext, ocv1.ReasonRetrying, msg) - return nil, err - } - - if catalogsExist { - // ClusterCatalogs exist but resolution failed - likely a transient issue (ClusterCatalog updating, cache stale, etc.) - // Retry resolution instead of falling back - msg := fmt.Sprintf("failed to resolve bundle, retrying: %v", err) - var catalogName string - if ext.Spec.Source.Catalog != nil { - catalogName = getCatalogNameFromSelector(ext.Spec.Source.Catalog.Selector) - } - l.Error(err, "resolution failed but matching ClusterCatalogs exist - retrying instead of falling back", - "packageName", getPackageName(ext), - "catalogName", catalogName) - setStatusProgressing(ext, err) - setInstalledStatusFromRevisionStates(ext, state.revisionStates) - ensureFailureConditionsWithReason(ext, ocv1.ReasonRetrying, msg) - return nil, err - } - - // ClusterCatalogs don't exist (deleted) - fall back to installed bundle to maintain current state. - // The controller watches ClusterCatalog resources, so when ClusterCatalogs become available again, - // a reconcile will be triggered automatically, allowing the extension to upgrade. - var catalogName string - if ext.Spec.Source.Catalog != nil { - catalogName = getCatalogNameFromSelector(ext.Spec.Source.Catalog.Selector) - } - l.Info("matching ClusterCatalogs unavailable or deleted - falling back to installed bundle to maintain workload", - "resolutionError", err.Error(), - "packageName", getPackageName(ext), - "catalogName", catalogName, - "installedBundle", state.revisionStates.Installed.Name, - "installedVersion", state.revisionStates.Installed.Version) - // Set installed status based on current revision states (needed before Apply runs) - setInstalledStatusFromRevisionStates(ext, state.revisionStates) - state.resolvedRevisionMetadata = state.revisionStates.Installed - // Return no error to allow Apply step to run and maintain resources. - // Apply will set Progressing=Succeeded when it completes successfully. - return nil, nil -} - -// getCatalogNameFromSelector extracts the catalog name from the selector if available. -// Returns empty string if selector is nil or doesn't contain the metadata.name label. -func getCatalogNameFromSelector(selector *metav1.LabelSelector) string { - if selector == nil || selector.MatchLabels == nil { - return "" - } - return selector.MatchLabels["olm.operatorframework.io/metadata.name"] -} - -// getPackageName safely extracts the package name from the extension spec. -// Returns empty string if Catalog source is nil. -func getPackageName(ext *ocv1.ClusterExtension) string { - if ext.Spec.Source.Catalog == nil { - return "" - } - return ext.Spec.Source.Catalog.PackageName -} - -// CheckCatalogsExist checks if any ClusterCatalogs matching the extension's selector exist. -// Returns true if at least one matching ClusterCatalog exists, false if none exist. -// Treats "CRD doesn't exist" errors as "no ClusterCatalogs exist" (returns false, nil). -// Returns an error only if the check itself fails unexpectedly. -func CheckCatalogsExist(ctx context.Context, c client.Client, ext *ocv1.ClusterExtension) (bool, error) { - var catalogList *ocv1.ClusterCatalogList - var listErr error - - if ext.Spec.Source.Catalog == nil || ext.Spec.Source.Catalog.Selector == nil { - // No selector means all ClusterCatalogs match - check if any ClusterCatalogs exist at all - catalogList = &ocv1.ClusterCatalogList{} - listErr = c.List(ctx, catalogList, client.Limit(1)) - } else { - // Convert label selector to k8slabels.Selector - // Note: An empty LabelSelector matches everything by default - selector, err := metav1.LabelSelectorAsSelector(ext.Spec.Source.Catalog.Selector) - if err != nil { - return false, fmt.Errorf("invalid catalog selector: %w", err) - } - - // List ClusterCatalogs matching the selector (limit to 1 since we only care if any exist) - catalogList = &ocv1.ClusterCatalogList{} - listErr = c.List(ctx, catalogList, client.MatchingLabelsSelector{Selector: selector}, client.Limit(1)) - } - - if listErr != nil { - // Check if the error is because the ClusterCatalog CRD doesn't exist - // This can happen if catalogd is not installed, which means no ClusterCatalogs exist - if apimeta.IsNoMatchError(listErr) { - return false, nil - } - return false, fmt.Errorf("failed to list ClusterCatalogs: %w", listErr) - } - - return len(catalogList.Items) > 0, nil -} - func UnpackBundle(i imageutil.Puller, cache imageutil.Cache) ReconcileStepFunc { return func(ctx context.Context, state *reconcileState, ext *ocv1.ClusterExtension) (*ctrl.Result, error) { l := log.FromContext(ctx) @@ -360,36 +168,20 @@ func UnpackBundle(i imageutil.Puller, cache imageutil.Cache) ReconcileStepFunc { l.V(1).Info("pulling bundle content") imageFS, _, _, err := i.Pull(ctx, ext.GetName(), state.resolvedRevisionMetadata.Image, cache) - // Check if resolved bundle matches installed bundle (no version change) - bundleUnchanged := state.revisionStates != nil && - state.revisionStates.Installed != nil && - state.resolvedRevisionMetadata.Name == state.revisionStates.Installed.Name && - state.resolvedRevisionMetadata.Version == state.revisionStates.Installed.Version - if err != nil { - if bundleUnchanged { - // Bundle hasn't changed and Pull failed (likely cache miss + catalog unavailable). - // This happens in fallback mode after catalog deletion. Set imageFS to nil so the - // applier can maintain the workload using existing Helm release or ClusterObjectSet. - l.V(1).Info("bundle content unavailable but version unchanged, maintaining current installation", + if state.bundleSource != nil && state.bundleSource.CanMaintainWithoutContent(ext, state.revisionStates, state.bundleResolution) { + l.V(1).Info("bundle content unavailable, maintaining current installation", "bundle", state.resolvedRevisionMetadata.Name, "version", state.resolvedRevisionMetadata.Version, "error", err.Error()) state.imageFS = nil return nil, nil } - // New bundle version but Pull failed - this is an error condition setStatusProgressing(ext, wrapErrorWithResolutionInfo(state.resolvedRevisionMetadata.BundleMetadata, err)) setInstalledStatusFromRevisionStates(ext, state.revisionStates) return nil, err } - if bundleUnchanged { - l.V(1).Info("bundle unchanged, using cached content for resource reconciliation", - "bundle", state.resolvedRevisionMetadata.Name, - "version", state.resolvedRevisionMetadata.Version) - } - state.imageFS = imageFS return nil, nil } @@ -461,13 +253,9 @@ func ApplyBundle(a Applier) ReconcileStepFunc { } setInstalledStatusFromRevisionStates(ext, state.revisionStates) - // After a successful rollout the installed bundle may have changed - // (e.g. upgrade from a deprecated to a non-deprecated version). - // Refresh deprecation conditions so they reflect the newly running - // bundle instead of the pre-upgrade bundle that was used during - // resolution. - if rolloutSucceeded && state.revisionStates.Installed != nil { - SetDeprecationStatus(ext, state.revisionStates.Installed.Name, state.resolvedDeprecation, state.hasCatalogData) + // Refresh source-specific deprecation conditions for the newly running bundle. + if rolloutSucceeded && state.revisionStates.Installed != nil && state.bundleSource != nil { + state.bundleSource.SetDeprecationStatus(ext, state.revisionStates.Installed.Name, state.bundleResolution) } // If there was an error applying the resolved bundle, diff --git a/internal/operator-controller/controllers/clusterextension_source_test.go b/internal/operator-controller/controllers/clusterextension_source_test.go new file mode 100644 index 0000000000..9b4c2995c4 --- /dev/null +++ b/internal/operator-controller/controllers/clusterextension_source_test.go @@ -0,0 +1,327 @@ +package controllers + +import ( + "context" + "errors" + "io/fs" + "testing" + + "github.com/blang/semver/v4" + "github.com/go-logr/logr" + "github.com/stretchr/testify/require" + apimeta "k8s.io/apimachinery/pkg/api/meta" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/runtime" + "sigs.k8s.io/controller-runtime/pkg/client" + "sigs.k8s.io/controller-runtime/pkg/client/fake" + "sigs.k8s.io/controller-runtime/pkg/event" + "sigs.k8s.io/controller-runtime/pkg/reconcile" + + "github.com/operator-framework/operator-registry/alpha/declcfg" + + ocv1 "github.com/operator-framework/operator-controller/api/v1" + "github.com/operator-framework/operator-controller/internal/operator-controller/resolve" + imageutil "github.com/operator-framework/operator-controller/internal/shared/util/image" +) + +// TestSource is an in-memory policy, not a production API source. In particular, +// it selects its own revision even while another revision is rolling out. +type TestSource struct { + result BundleResolution + err error + maintain bool + enqueue bool +} + +func (s *TestSource) Resolve(context.Context, *ocv1.ClusterExtension, *RevisionStates) (BundleResolution, error) { + return s.result, s.err +} + +func (s *TestSource) SetDeprecationStatus(ext *ocv1.ClusterExtension, installed string, result BundleResolution) { + status := metav1.ConditionUnknown + if result.DeprecationKnown { + status = metav1.ConditionFalse + } + SetStatusCondition(&ext.Status.Conditions, metav1.Condition{ + Type: ocv1.TypeDeprecated, Status: status, Reason: "TestSourcePolicy", + Message: installed, ObservedGeneration: ext.Generation, + }) +} + +func (s *TestSource) ReconcileOnCatalogChange(*ocv1.ClusterExtension, *ocv1.ClusterCatalog) bool { + return s.enqueue +} + +func (s *TestSource) CanMaintainWithoutContent(*ocv1.ClusterExtension, *RevisionStates, BundleResolution) bool { + return s.maintain +} + +type forbiddenCatalogClient struct{ client.Client } + +func (*forbiddenCatalogClient) List(context.Context, client.ObjectList, ...client.ListOption) error { + panic("source isolation violated: catalog client used") +} + +type successfulSourceApplier struct{} + +func (successfulSourceApplier) Apply(context.Context, fs.FS, *ocv1.ClusterExtension, map[string]string, map[string]string) (bool, string, error) { + return true, "", nil +} + +func sourceExtension(sourceType string) *ocv1.ClusterExtension { + ext := &ocv1.ClusterExtension{Spec: ocv1.ClusterExtensionSpec{Source: ocv1.SourceConfig{ + SourceType: sourceType, + }}} + if sourceType == ocv1.SourceTypeCatalog { + ext.Spec.Source.Catalog = &ocv1.CatalogFilter{PackageName: "example"} + } + return ext +} + +func sourceRevision(name, version string) *RevisionMetadata { + return &RevisionMetadata{Package: "example", Image: "example/bundle:latest", BundleMetadata: ocv1.BundleMetadata{Name: name, Version: version}} +} + +func forbiddenCatalogResolver(context.Context, *ocv1.ClusterExtension, *ocv1.BundleMetadata) (*declcfg.Bundle, *declcfg.VersionRelease, *declcfg.Deprecation, error) { + panic("source isolation violated: catalog resolver used") +} + +func TestSourceSelectionOwnsResolutionAndDeprecation(t *testing.T) { + resolutionErr := errors.New("test source failed") + for _, rolling := range []bool{false, true} { + for _, sourceErr := range []error{nil, resolutionErr, reconcile.TerminalError(resolutionErr)} { + t.Run(testingName(rolling, sourceErr), func(t *testing.T) { + installed := sourceRevision("old", "1.0.0") + target := sourceRevision("selected", "2.0.0") + state := &reconcileState{revisionStates: &RevisionStates{Installed: installed}} + if rolling { + state.revisionStates.RollingOut = []*RevisionMetadata{sourceRevision("rolling", "1.1.0")} + } + source := &TestSource{result: BundleResolution{Revision: target, DeprecationKnown: true}, err: sourceErr} + sources := BundleSources{ + "TestSource": source, + ocv1.SourceTypeCatalog: &CatalogBundleSource{ + Resolver: resolve.Func(forbiddenCatalogResolver), Client: &forbiddenCatalogClient{}, + }, + } + ext := sourceExtension("TestSource") + _, err := ResolveBundle(sources)(t.Context(), state, ext) + require.ErrorIs(t, err, sourceErr) + condition := apimeta.FindStatusCondition(ext.Status.Conditions, ocv1.TypeDeprecated) + require.NotNil(t, condition) + require.Equal(t, "TestSourcePolicy", condition.Reason) + require.Equal(t, metav1.ConditionFalse, condition.Status) + require.Equal(t, "old", condition.Message) + if sourceErr != nil { + require.Nil(t, state.resolvedRevisionMetadata) + require.Equal(t, installed.BundleMetadata, ext.Status.Install.Bundle) + return + } + require.Same(t, target, state.resolvedRevisionMetadata) + _, err = ApplyBundle(successfulSourceApplier{})(t.Context(), state, ext) + require.NoError(t, err) + require.Equal(t, "selected", apimeta.FindStatusCondition(ext.Status.Conditions, ocv1.TypeDeprecated).Message) + }) + } + } +} + +func testingName(rolling bool, err error) string { + name := "fresh" + if rolling { + name = "rolling" + } + if err != nil { + name += "/" + err.Error() + } + return name +} + +func TestSourceLookupRejectsUnknownAndNilBeforeRollout(t *testing.T) { + var typedNil *TestSource + for _, sourceType := range []string{"Unknown", "Nil", "TypedNil"} { + for _, rolling := range []bool{false, true} { + t.Run(sourceType+"/"+testingName(rolling, nil), func(t *testing.T) { + sources := BundleSources{ + "Nil": nil, "TypedNil": typedNil, + ocv1.SourceTypeCatalog: &CatalogBundleSource{Resolver: resolve.Func(forbiddenCatalogResolver), Client: &forbiddenCatalogClient{}}, + } + state := &reconcileState{revisionStates: &RevisionStates{}} + if rolling { + state.revisionStates.RollingOut = []*RevisionMetadata{sourceRevision("rolling", "1.0.0")} + } + _, err := ResolveBundle(sources)(t.Context(), state, sourceExtension(sourceType)) + require.ErrorContains(t, err, "unsupported sourceType "+`"`+sourceType+`"`) + require.Nil(t, state.resolvedRevisionMetadata) + }) + } + } +} + +func TestCatalogSourcePreservesFirstRollingRevision(t *testing.T) { + first, last := sourceRevision("first", "1.0.0"), sourceRevision("last", "2.0.0") + state := &reconcileState{revisionStates: &RevisionStates{RollingOut: []*RevisionMetadata{first, last}}} + ext := sourceExtension(ocv1.SourceTypeCatalog) + sources := BundleSources{ocv1.SourceTypeCatalog: &CatalogBundleSource{Resolver: resolve.Func(forbiddenCatalogResolver), Client: &forbiddenCatalogClient{}}} + _, err := ResolveBundle(sources)(t.Context(), state, ext) + require.NoError(t, err) + require.Same(t, first, state.resolvedRevisionMetadata) + require.False(t, state.bundleResolution.DeprecationKnown) + require.Equal(t, metav1.ConditionUnknown, apimeta.FindStatusCondition(ext.Status.Conditions, ocv1.TypeDeprecated).Status) +} + +func TestCatalogPolicyRequiresCatalogConfiguration(t *testing.T) { + for _, sourceType := range []string{ocv1.SourceTypeCatalog, "TestSource"} { + t.Run(sourceType, func(t *testing.T) { + ext := sourceExtension(sourceType) + ext.Spec.Source.Catalog = nil + exists, err := CheckCatalogsExist(t.Context(), &forbiddenCatalogClient{}, ext) + require.ErrorContains(t, err, "require a Catalog source") + require.False(t, exists) + }) + } + ext := sourceExtension(ocv1.SourceTypeCatalog) + ext.Spec.Source.Catalog = nil + source := &CatalogBundleSource{Resolver: resolve.Func(forbiddenCatalogResolver), Client: &forbiddenCatalogClient{}} + for _, rolling := range []bool{false, true} { + t.Run(testingName(rolling, nil), func(t *testing.T) { + state := &reconcileState{revisionStates: &RevisionStates{}} + if rolling { + state.revisionStates.RollingOut = []*RevisionMetadata{sourceRevision("rolling", "1.0.0")} + } + _, err := ResolveBundle(BundleSources{ocv1.SourceTypeCatalog: source})(t.Context(), state, ext) + require.ErrorContains(t, err, "catalog source requires spec.source.catalog") + require.Nil(t, state.resolvedRevisionMetadata) + }) + } +} + +func TestCatalogSourceDeprecationEvidence(t *testing.T) { + resolutionErr := errors.New("catalog resolution failed") + deprecation := &declcfg.Deprecation{Entries: []declcfg.DeprecationEntry{{ + Reference: declcfg.PackageScopedReference{Schema: declcfg.SchemaPackage, Name: "example"}, Message: "deprecated package", + }}} + for _, tc := range []struct { + name string + err error + deprecation *declcfg.Deprecation + known bool + status metav1.ConditionStatus + }{ + {name: "success empty", known: true, status: metav1.ConditionFalse}, + {name: "error without evidence", err: resolutionErr, status: metav1.ConditionUnknown}, + {name: "error with evidence", err: resolutionErr, deprecation: deprecation, known: true, status: metav1.ConditionTrue}, + } { + t.Run(tc.name, func(t *testing.T) { + ext := sourceExtension(ocv1.SourceTypeCatalog) + state := &reconcileState{revisionStates: &RevisionStates{}} + r := resolve.Func(func(context.Context, *ocv1.ClusterExtension, *ocv1.BundleMetadata) (*declcfg.Bundle, *declcfg.VersionRelease, *declcfg.Deprecation, error) { + return &declcfg.Bundle{Name: "example.v1", Package: "example", Image: "example/bundle:v1"}, &declcfg.VersionRelease{Version: semver.MustParse("1.0.0")}, tc.deprecation, tc.err + }) + _, err := ResolveBundle(BundleSources{ocv1.SourceTypeCatalog: &CatalogBundleSource{Resolver: r, Client: &forbiddenCatalogClient{}}})(t.Context(), state, ext) + require.ErrorIs(t, err, tc.err) + require.Equal(t, tc.known, state.bundleResolution.DeprecationKnown) + require.Equal(t, tc.status, apimeta.FindStatusCondition(ext.Status.Conditions, ocv1.TypePackageDeprecated).Status) + }) + } +} + +func TestCatalogSourceFallbackPreservesVersionStringPolicy(t *testing.T) { + sch := runtime.NewScheme() + require.NoError(t, ocv1.AddToScheme(sch)) + resolutionErr := errors.New("catalog unavailable") + for _, version := range []string{"", "1.0.0", ">=1.0.0"} { + for _, catalogExists := range []bool{false, true} { + t.Run(version+"/"+testingName(catalogExists, nil), func(t *testing.T) { + builder := fake.NewClientBuilder().WithScheme(sch) + if catalogExists { + builder.WithObjects(&ocv1.ClusterCatalog{ObjectMeta: metav1.ObjectMeta{Name: "catalog"}}) + } + ext := sourceExtension(ocv1.SourceTypeCatalog) + ext.Spec.Source.Catalog = &ocv1.CatalogFilter{PackageName: "example", Version: version} + installed := sourceRevision("installed", "1.0.0") + state := &reconcileState{revisionStates: &RevisionStates{Installed: installed}} + r := resolve.Func(func(context.Context, *ocv1.ClusterExtension, *ocv1.BundleMetadata) (*declcfg.Bundle, *declcfg.VersionRelease, *declcfg.Deprecation, error) { + return nil, nil, nil, resolutionErr + }) + _, err := ResolveBundle(BundleSources{ocv1.SourceTypeCatalog: &CatalogBundleSource{Resolver: r, Client: builder.Build()}})(t.Context(), state, ext) + if catalogExists || version == ">=1.0.0" { + require.ErrorIs(t, err, resolutionErr) + require.Nil(t, state.resolvedRevisionMetadata) + } else { + require.NoError(t, err) + require.Same(t, installed, state.resolvedRevisionMetadata) + } + }) + } + } +} + +func TestUnpackUsesSelectedSourceMaintenancePolicy(t *testing.T) { + pullErr := errors.New("content unavailable") + installed := sourceRevision("same", "1.0.0") + target := sourceRevision("same", "1.0.0") + target.Image, target.Package = "changed/image:v1", "changed-package" + for _, tc := range []struct { + name string + sourceType string + source BundleSource + wantError bool + }{ + {name: "catalog legacy name version", sourceType: ocv1.SourceTypeCatalog, source: &CatalogBundleSource{Resolver: resolve.Func(forbiddenCatalogResolver), Client: &forbiddenCatalogClient{}}}, + {name: "test rejects maintenance", sourceType: "TestSource", source: &TestSource{result: BundleResolution{Revision: target}}, wantError: true}, + {name: "test permits maintenance", sourceType: "TestSource", source: &TestSource{result: BundleResolution{Revision: target}, maintain: true}}, + } { + t.Run(tc.name, func(t *testing.T) { + state := &reconcileState{ + revisionStates: &RevisionStates{Installed: installed, RollingOut: []*RevisionMetadata{target}}, + } + ext := sourceExtension(tc.sourceType) + _, err := ResolveBundle(BundleSources{tc.sourceType: tc.source})(t.Context(), state, ext) + require.NoError(t, err) + _, err = UnpackBundle(&imageutil.FakePuller{Error: pullErr}, nil)(t.Context(), state, ext) + if tc.wantError { + require.ErrorIs(t, err, pullErr) + } else { + require.NoError(t, err) + require.Nil(t, state.imageFS) + } + }) + } +} + +func TestCatalogEventUsesSourcePolicy(t *testing.T) { + sch := runtime.NewScheme() + require.NoError(t, ocv1.AddToScheme(sch)) + objects := make([]client.Object, 0, 5) + for _, name := range []string{ocv1.SourceTypeCatalog, "TestSource", "InterestedTestSource", "Unknown", "Nil"} { + ext := sourceExtension(name) + ext.Name = name + objects = append(objects, ext) + } + c := fake.NewClientBuilder().WithScheme(sch).WithObjects(objects...).Build() + sources := BundleSources{ + ocv1.SourceTypeCatalog: &CatalogBundleSource{Client: &forbiddenCatalogClient{}}, + "TestSource": &TestSource{}, "InterestedTestSource": &TestSource{enqueue: true}, "Nil": nil, + } + requests := clusterExtensionRequestsForCatalog(c, logr.Discard(), sources)(t.Context(), &ocv1.ClusterCatalog{}) + names := make([]string, 0, len(requests)) + for _, request := range requests { + names = append(names, request.Name) + } + require.ElementsMatch(t, []string{ocv1.SourceTypeCatalog, "InterestedTestSource"}, names) + _, err := sources.For(sourceExtension("TestSource")) + require.NoError(t, err) +} + +func TestCatalogEventPredicatePreservesResolvedRefBehavior(t *testing.T) { + catalog := &ocv1.ClusterCatalog{Status: ocv1.ClusterCatalogStatus{ResolvedSource: &ocv1.ResolvedCatalogSource{ + Image: &ocv1.ResolvedImageSource{Ref: "example/catalog@sha256:old"}, + }}} + updated := catalog.DeepCopy() + updated.Labels = map[string]string{"changed": "label"} + require.False(t, catalogResolutionInputsChanged(event.UpdateEvent{ObjectOld: catalog, ObjectNew: updated})) + updated.Status.ResolvedSource.Image.Ref = "example/catalog@sha256:new" + require.True(t, catalogResolutionInputsChanged(event.UpdateEvent{ObjectOld: catalog, ObjectNew: updated})) + require.True(t, catalogResolutionInputsChanged(event.UpdateEvent{ObjectOld: &ocv1.ClusterCatalog{}, ObjectNew: updated})) +} diff --git a/internal/operator-controller/controllers/suite_test.go b/internal/operator-controller/controllers/suite_test.go index ba2b926f42..5de1a89729 100644 --- a/internal/operator-controller/controllers/suite_test.go +++ b/internal/operator-controller/controllers/suite_test.go @@ -138,7 +138,10 @@ func newClientAndReconciler(t *testing.T, opts ...reconcilerOption) (client.Clie controllers.RetrieveRevisionStates(d.RevisionStatesGetter), } if r := d.Resolver; r != nil { - reconciler.ReconcileSteps = append(reconciler.ReconcileSteps, controllers.ResolveBundle(r, cl)) + reconciler.BundleSources = controllers.BundleSources{ + ocv1.SourceTypeCatalog: &controllers.CatalogBundleSource{Resolver: r, Client: cl}, + } + reconciler.ReconcileSteps = append(reconciler.ReconcileSteps, controllers.ResolveBundle(reconciler.BundleSources)) } if i := d.ImagePuller; i != nil { reconciler.ReconcileSteps = append(reconciler.ReconcileSteps, controllers.UnpackBundle(i, d.ImageCache)) From d39e72c67ca3af4c5642e25eb12fdcdbd510915d Mon Sep 17 00:00:00 2001 From: grokspawn Date: Fri, 6 Nov 2026 16:25:35 -0600 Subject: [PATCH 2/3] Segregate deprecation evidence into optional source interfaces Split the mandatory source contract so deprecation becomes an optional, type-asserted capability instead of a required method with a sentinel-encoded knownness flag, following the review direction on this PR: - Rename the source contract to Resolver (from BundleSource) and the catalog implementation to CatalogSource. The mandatory surface is now: ValidateSource (fail-closed payload validation before rollout continuation and resolution), Resolve (steady-state only), RecoverFromResolutionError (invoked by the shared steps only when Resolve fails), CanMaintainWithoutContent, and ReconcileOnCatalogChange. The installed-bundle fallback action is the shared maintainInstalled helper; the catalog-specific existence gate stays in CatalogSource. - Slim resolve.Resolver: deprecation leaves the Resolve return value. Evidence is queried through new optional interfaces, resolve.PackageDeprecationSource, ChannelDeprecationSource, and BundleDeprecationSource, implemented by resolve.CatalogResolver and delegated by CatalogSource. - The shared ResolveBundle step owns engagement: validate the source payload, continue the first rolling revision without consulting resolution, call Resolve only at steady state, engage recovery only on failure, and collect deprecation evidence on every reconcile via type assertions. - A source that implements none of the deprecation interfaces manifests exactly like a source whose evaluation found no deprecations, so the DeprecationKnown/hasCatalogData flag is gone; a failed query from a capable source is the only unknown-evidence path. Evidence now also populates during rollouts. Deprecation ordering inside catalog resolution (ByDeprecationFunc) is unchanged. Direct sources (a later DBI PR) implement only Resolver and inherit this behavior structurally instead of answering catalog questions. Signed-off-by: grokspawn --- cmd/operator-controller/main.go | 16 +- .../controllers/bundle_source.go | 73 ++++--- ...log_bundle_source.go => catalog_source.go} | 185 ++++++++++-------- .../clusterextension_controller.go | 139 +++++++------ .../clusterextension_controller_test.go | 98 +++++++--- .../clusterextension_reconcile_steps.go | 105 ++++++++-- .../clusterextension_source_test.go | 176 ++++++++++++----- .../controllers/suite_test.go | 6 +- .../operator-controller/resolve/catalog.go | 97 +++++++-- .../resolve/catalog_test.go | 155 +++++++++++---- .../operator-controller/resolve/resolver.go | 38 +++- 11 files changed, 763 insertions(+), 325 deletions(-) rename internal/operator-controller/controllers/{catalog_bundle_source.go => catalog_source.go} (57%) diff --git a/cmd/operator-controller/main.go b/cmd/operator-controller/main.go index 72a3c847b4..98947096be 100644 --- a/cmd/operator-controller/main.go +++ b/cmd/operator-controller/main.go @@ -109,7 +109,7 @@ type boxcutterReconcilerConfigurator struct { mgr manager.Manager preflights []applier.Preflight regv1ManifestProvider applier.ManifestProvider - bundleSources controllers.BundleSources + bundleSources controllers.Resolvers imageCache imageutil.Cache imagePuller imageutil.Puller finalizers crfinalizer.Finalizers @@ -119,7 +119,7 @@ type helmReconcilerConfigurator struct { mgr manager.Manager preflights []applier.Preflight regv1ManifestProvider applier.ManifestProvider - bundleSources controllers.BundleSources + bundleSources controllers.Resolvers imageCache imageutil.Cache imagePuller imageutil.Puller finalizers crfinalizer.Finalizers @@ -471,12 +471,12 @@ func run() error { } } - bundleSources := controllers.BundleSources{ - ocv1.SourceTypeCatalog: &controllers.CatalogBundleSource{Resolver: resolver, Client: cl}, + bundleSources := controllers.Resolvers{ + ocv1.SourceTypeCatalog: &controllers.CatalogSource{Resolver: resolver, Client: cl}, } ceReconciler := &controllers.ClusterExtensionReconciler{ - Client: cl, - BundleSources: bundleSources, + Client: cl, + Resolvers: bundleSources, } if features.OperatorControllerFeatureGate.Enabled(features.BoxcutterRuntime) { _, err = ceReconciler.SetupWithManagerForBoxcutter(mgr) @@ -639,7 +639,7 @@ func (c *boxcutterReconcilerConfigurator) Configure(ceReconciler *controllers.Cl RevisionGenerator: rg, FieldOwner: fieldOwner, } - ceReconciler.BundleSources = c.bundleSources + ceReconciler.Resolvers = c.bundleSources ceReconciler.ReconcileSteps = []controllers.ReconcileStepFunc{ controllers.HandleFinalizers(c.finalizers), controllers.ValidateClusterExtension( @@ -698,7 +698,7 @@ func (c *helmReconcilerConfigurator) Configure(ceReconciler *controllers.Cluster TrackingCache: c.trackingCache, } revisionStatesGetter := &controllers.HelmRevisionStatesGetter{ActionClientGetter: acg} - ceReconciler.BundleSources = c.bundleSources + ceReconciler.Resolvers = c.bundleSources ceReconciler.ReconcileSteps = []controllers.ReconcileStepFunc{ controllers.HandleFinalizers(c.finalizers), controllers.ValidateClusterExtension( diff --git a/internal/operator-controller/controllers/bundle_source.go b/internal/operator-controller/controllers/bundle_source.go index 7444467480..d82d81ab3c 100644 --- a/internal/operator-controller/controllers/bundle_source.go +++ b/internal/operator-controller/controllers/bundle_source.go @@ -21,45 +21,70 @@ import ( "fmt" "reflect" - "github.com/operator-framework/operator-registry/alpha/declcfg" - ocv1 "github.com/operator-framework/operator-controller/api/v1" ) -// BundleSource owns resolution and lifecycle policies for an installation source. -// Resolve includes rollout continuation and recovery from resolution errors. -// SetDeprecationStatus is called even on resolution errors, and again after a -// successful apply with the newly installed bundle name. -type BundleSource interface { - Resolve(context.Context, *ocv1.ClusterExtension, *RevisionStates) (BundleResolution, error) - SetDeprecationStatus(ext *ocv1.ClusterExtension, installedBundleName string, resolution BundleResolution) - ReconcileOnCatalogChange(*ocv1.ClusterExtension, *ocv1.ClusterCatalog) bool - CanMaintainWithoutContent(*ocv1.ClusterExtension, *RevisionStates, BundleResolution) bool -} +// Resolver is the mandatory behavior of an object that can resolve a bundle +// from a source. The shared reconciliation steps own engagement policy: +// ValidateSource runs before rollout continuation and resolution, Resolve is +// called only at steady state with no rollout in progress, +// RecoverFromResolutionError is invoked only when Resolve fails, and +// CanMaintainWithoutContent is consulted only when bundle content cannot be +// pulled. Implementations must not hide preconditions or fallbacks inside +// Resolve. +// +// Deprecation evidence is deliberately not part of this contract: sources +// that can provide it implement the optional deprecation interfaces in the +// resolve package (see resolve.PackageDeprecationSource), and consumers use +// type assertions against those interfaces to collect evidence on every +// reconcile. A source without deprecation data is reported exactly like a +// source whose evaluation found no deprecations. +type Resolver interface { + // ValidateSource verifies the extension's source payload is complete + // enough to reconcile. It runs before rollout continuation and + // resolution, so malformed payloads fail closed without backend calls. + ValidateSource(ext *ocv1.ClusterExtension) error + + // Resolve selects the bundle to install or upgrade to. It is called only + // at steady state, with no rollout in progress. + Resolve(ctx context.Context, ext *ocv1.ClusterExtension, installedBundle *ocv1.BundleMetadata) (*RevisionMetadata, error) + + // RecoverFromResolutionError is invoked only when Resolve returned an + // error. Returning a nil error with a non-nil result authorizes + // maintaining the installed bundle (see maintainInstalled); returning an + // error classifies the failure for retry. + RecoverFromResolutionError(ctx context.Context, ext *ocv1.ClusterExtension, revisions *RevisionStates, resolveErr error) (*RevisionMetadata, error) -// BundleResolution carries the selected revision and source-provided deprecation -// evidence. A nil Deprecation with DeprecationKnown true means known-empty data. -// Results returned alongside an error still carry evidence for status reporting. -type BundleResolution struct { - Revision *RevisionMetadata - Deprecation *declcfg.Deprecation - DeprecationKnown bool + // CanMaintainWithoutContent authorizes maintaining the current + // installation when bundle content cannot be pulled. + CanMaintainWithoutContent(ext *ocv1.ClusterExtension, revisions *RevisionStates, resolved *RevisionMetadata) bool + + // ReconcileOnCatalogChange reports whether a ClusterCatalog event should + // enqueue reconciles for extensions served by this source. + ReconcileOnCatalogChange(ext *ocv1.ClusterExtension, catalog *ocv1.ClusterCatalog) bool } -// BundleSources selects a source policy using spec.source.sourceType. -type BundleSources map[string]BundleSource +// Resolvers selects a source policy using spec.source.sourceType. +type Resolvers map[string]Resolver // For returns the configured policy or an error before any source is invoked. -func (sources BundleSources) For(ext *ocv1.ClusterExtension) (BundleSource, error) { +func (sources Resolvers) For(ext *ocv1.ClusterExtension) (Resolver, error) { source := sources[ext.Spec.Source.SourceType] - if source == nil || nilBundleSource(source) { + if source == nil || nilResolver(source) { return nil, fmt.Errorf("unsupported sourceType %q: bundle source is not configured", ext.Spec.Source.SourceType) } return source, nil } +// maintainInstalled is the general installed-bundle fallback action: it +// targets the installed revision so the shared unpack and apply pipeline +// maintains the existing installation. +func maintainInstalled(revisions *RevisionStates) *RevisionMetadata { + return revisions.Installed +} + // Interfaces can contain typed nil implementations; reject these at lookup too. -func nilBundleSource(source BundleSource) bool { +func nilResolver(source Resolver) bool { v := reflect.ValueOf(source) switch v.Kind() { case reflect.Chan, reflect.Func, reflect.Interface, reflect.Map, reflect.Pointer, reflect.Slice: diff --git a/internal/operator-controller/controllers/catalog_bundle_source.go b/internal/operator-controller/controllers/catalog_source.go similarity index 57% rename from internal/operator-controller/controllers/catalog_bundle_source.go rename to internal/operator-controller/controllers/catalog_source.go index 098662d9b3..d4ce65488e 100644 --- a/internal/operator-controller/controllers/catalog_bundle_source.go +++ b/internal/operator-controller/controllers/catalog_source.go @@ -25,74 +25,51 @@ import ( "sigs.k8s.io/controller-runtime/pkg/client" "sigs.k8s.io/controller-runtime/pkg/log" + "github.com/operator-framework/operator-registry/alpha/declcfg" + ocv1 "github.com/operator-framework/operator-controller/api/v1" "github.com/operator-framework/operator-controller/internal/operator-controller/bundleutil" "github.com/operator-framework/operator-controller/internal/operator-controller/resolve" ) -// CatalogBundleSource preserves Catalog resolution and lifecycle policies. -type CatalogBundleSource struct { +// CatalogSource preserves Catalog resolution and lifecycle policies. +type CatalogSource struct { Resolver resolve.Resolver Client client.Client } -var _ BundleSource = (*CatalogBundleSource)(nil) +var ( + _ Resolver = (*CatalogSource)(nil) + _ resolve.PackageDeprecationSource = (*CatalogSource)(nil) + _ resolve.ChannelDeprecationSource = (*CatalogSource)(nil) + _ resolve.BundleDeprecationSource = (*CatalogSource)(nil) +) -// Resolve continues the first rolling revision or resolves a Catalog bundle, -// retaining the existing catalog-deletion fallback and deprecation evidence rule. -func (s *CatalogBundleSource) Resolve(ctx context.Context, ext *ocv1.ClusterExtension, revisions *RevisionStates) (BundleResolution, error) { +// ValidateSource rejects a missing Catalog payload before any rollout +// continuation, resolution, or catalog access. +func (s *CatalogSource) ValidateSource(ext *ocv1.ClusterExtension) error { if ext.Spec.Source.Catalog == nil { - return BundleResolution{}, fmt.Errorf("catalog source requires spec.source.catalog") - } - if len(revisions.RollingOut) > 0 { - return BundleResolution{Revision: revisions.RollingOut[0]}, nil + return fmt.Errorf("catalog source requires spec.source.catalog") } + return nil +} + +// Resolve selects a Catalog bundle for the extension. +func (s *CatalogSource) Resolve(ctx context.Context, ext *ocv1.ClusterExtension, installedBundle *ocv1.BundleMetadata) (*RevisionMetadata, error) { log.FromContext(ctx).V(1).Info("resolving bundle") - var bm *ocv1.BundleMetadata - if revisions.Installed != nil { - bm = &revisions.Installed.BundleMetadata - } - bundle, version, deprecation, err := s.Resolver.Resolve(ctx, ext, bm) - // Once the catalog resolver selects one package, it returns that catalog's - // deprecation data even when no bundle matches. An empty, non-nil deprecation - // means the selected catalog has no deprecation entries; nil on failure means - // no catalog package was selected. Keep the success check for resolver - // implementations that may return nil on success. - result := BundleResolution{Deprecation: deprecation, DeprecationKnown: err == nil || deprecation != nil} + bundle, version, err := s.Resolver.Resolve(ctx, ext, installedBundle) if err != nil { - state := &reconcileState{revisionStates: revisions} - err = handleCatalogResolutionError(ctx, s.Client, state, ext, err) - result.Revision = state.resolvedRevisionMetadata - return result, err + return nil, err } - result.Revision = &RevisionMetadata{ + return &RevisionMetadata{ Package: bundle.Package, Image: bundle.Image, // Preserve MetadataFor's feature-gated version/release normalization. BundleMetadata: bundleutil.MetadataFor(bundle.Name, *version), - } - return result, nil -} - -// SetDeprecationStatus applies Catalog deprecation evidence, including on errors. -func (s *CatalogBundleSource) SetDeprecationStatus(ext *ocv1.ClusterExtension, installedBundleName string, resolution BundleResolution) { - SetDeprecationStatus(ext, installedBundleName, resolution.Deprecation, resolution.DeprecationKnown) + }, nil } -// ReconcileOnCatalogChange preserves the existing fan-out to Catalog extensions. -// Catalog-to-extension provenance and selector-specific filtering remain unchanged. -func (s *CatalogBundleSource) ReconcileOnCatalogChange(*ocv1.ClusterExtension, *ocv1.ClusterCatalog) bool { - return true -} - -// CanMaintainWithoutContent preserves the original name/version comparison. -func (s *CatalogBundleSource) CanMaintainWithoutContent(_ *ocv1.ClusterExtension, revisions *RevisionStates, resolution BundleResolution) bool { - return revisions != nil && revisions.Installed != nil && resolution.Revision != nil && - resolution.Revision.Name == revisions.Installed.Name && - resolution.Revision.Version == revisions.Installed.Version -} - -// handleCatalogResolutionError owns recovery when Catalog resolution fails. +// RecoverFromResolutionError owns recovery when Catalog resolution fails. // // Decision logic (evaluated in order): // 1. No installed bundle → Retry (cannot proceed without any bundle) @@ -101,20 +78,21 @@ func (s *CatalogBundleSource) CanMaintainWithoutContent(_ *ocv1.ClusterExtension // 4. Catalogs exist → Retry (transient error, catalog may be updating) // 5. Catalogs deleted → Fallback to installed bundle (maintain current state) // -// When falling back (case 5), we set the resolved bundle to the installed bundle and return -// no error, allowing the Apply step to run and maintain resources using the existing installation. -// The controller watches ClusterCatalog resources, so reconciliation will automatically resume -// when catalogs return, enabling upgrades. -func handleCatalogResolutionError(ctx context.Context, c client.Client, state *reconcileState, ext *ocv1.ClusterExtension, err error) error { +// When falling back (case 5), we return the installed bundle and no error, +// allowing the Apply step to run and maintain resources using the existing +// installation. The controller watches ClusterCatalog resources, so +// reconciliation will automatically resume when catalogs return, enabling +// upgrades. +func (s *CatalogSource) RecoverFromResolutionError(ctx context.Context, ext *ocv1.ClusterExtension, revisions *RevisionStates, resolveErr error) (*RevisionMetadata, error) { l := log.FromContext(ctx) // No installed bundle and resolution failed - cannot proceed - if state.revisionStates.Installed == nil { - msg := fmt.Sprintf("failed to resolve bundle: %v", err) - setStatusProgressing(ext, err) - setInstalledStatusFromRevisionStates(ext, state.revisionStates) + if revisions.Installed == nil { + msg := fmt.Sprintf("failed to resolve bundle: %v", resolveErr) + setStatusProgressing(ext, resolveErr) + setInstalledStatusFromRevisionStates(ext, revisions) ensureFailureConditionsWithReason(ext, ocv1.ReasonRetrying, msg) - return err + return nil, resolveErr } // Check if the spec is requesting a specific version that differs from installed @@ -122,54 +100,54 @@ func handleCatalogResolutionError(ctx context.Context, c client.Client, state *r if ext.Spec.Source.Catalog != nil { specVersion = ext.Spec.Source.Catalog.Version } - installedVersion := state.revisionStates.Installed.Version + installedVersion := revisions.Installed.Version // If spec requests a different version, we cannot fall back - must fail and retry if specVersion != "" && specVersion != installedVersion { - msg := fmt.Sprintf("unable to upgrade to version %s: %v (currently installed: %s)", specVersion, err, installedVersion) - l.Error(err, "resolution failed and spec requests version change - cannot fall back", + msg := fmt.Sprintf("unable to upgrade to version %s: %v (currently installed: %s)", specVersion, resolveErr, installedVersion) + l.Error(resolveErr, "resolution failed and spec requests version change - cannot fall back", "requestedVersion", specVersion, "installedVersion", installedVersion) - setStatusProgressing(ext, err) - setInstalledStatusFromRevisionStates(ext, state.revisionStates) + setStatusProgressing(ext, resolveErr) + setInstalledStatusFromRevisionStates(ext, revisions) ensureFailureConditionsWithReason(ext, ocv1.ReasonRetrying, msg) - return err + return nil, resolveErr } // No version change requested - check if ClusterCatalogs exist // Only fall back if ClusterCatalogs have been deleted - catalogsExist, catalogCheckErr := CheckCatalogsExist(ctx, c, ext) + catalogsExist, catalogCheckErr := CheckCatalogsExist(ctx, s.Client, ext) if catalogCheckErr != nil { - msg := fmt.Sprintf("failed to resolve bundle: %v", err) + msg := fmt.Sprintf("failed to resolve bundle: %v", resolveErr) var catalogName string if ext.Spec.Source.Catalog != nil { catalogName = getCatalogNameFromSelector(ext.Spec.Source.Catalog.Selector) } l.Error(catalogCheckErr, "error checking if ClusterCatalogs exist, will retry resolution", - "resolutionError", err, + "resolutionError", resolveErr, "packageName", getPackageName(ext), "catalogName", catalogName) - setStatusProgressing(ext, err) - setInstalledStatusFromRevisionStates(ext, state.revisionStates) + setStatusProgressing(ext, resolveErr) + setInstalledStatusFromRevisionStates(ext, revisions) ensureFailureConditionsWithReason(ext, ocv1.ReasonRetrying, msg) - return err + return nil, resolveErr } if catalogsExist { // ClusterCatalogs exist but resolution failed - likely a transient issue (ClusterCatalog updating, cache stale, etc.) // Retry resolution instead of falling back - msg := fmt.Sprintf("failed to resolve bundle, retrying: %v", err) + msg := fmt.Sprintf("failed to resolve bundle, retrying: %v", resolveErr) var catalogName string if ext.Spec.Source.Catalog != nil { catalogName = getCatalogNameFromSelector(ext.Spec.Source.Catalog.Selector) } - l.Error(err, "resolution failed but matching ClusterCatalogs exist - retrying instead of falling back", + l.Error(resolveErr, "resolution failed but matching ClusterCatalogs exist - retrying instead of falling back", "packageName", getPackageName(ext), "catalogName", catalogName) - setStatusProgressing(ext, err) - setInstalledStatusFromRevisionStates(ext, state.revisionStates) + setStatusProgressing(ext, resolveErr) + setInstalledStatusFromRevisionStates(ext, revisions) ensureFailureConditionsWithReason(ext, ocv1.ReasonRetrying, msg) - return err + return nil, resolveErr } // ClusterCatalogs don't exist (deleted) - fall back to installed bundle to maintain current state. @@ -180,17 +158,62 @@ func handleCatalogResolutionError(ctx context.Context, c client.Client, state *r catalogName = getCatalogNameFromSelector(ext.Spec.Source.Catalog.Selector) } l.Info("matching ClusterCatalogs unavailable or deleted - falling back to installed bundle to maintain workload", - "resolutionError", err.Error(), + "resolutionError", resolveErr.Error(), "packageName", getPackageName(ext), "catalogName", catalogName, - "installedBundle", state.revisionStates.Installed.Name, - "installedVersion", state.revisionStates.Installed.Version) + "installedBundle", revisions.Installed.Name, + "installedVersion", revisions.Installed.Version) // Set installed status based on current revision states (needed before Apply runs) - setInstalledStatusFromRevisionStates(ext, state.revisionStates) - state.resolvedRevisionMetadata = state.revisionStates.Installed - // Return no error to allow Apply step to run and maintain resources. + setInstalledStatusFromRevisionStates(ext, revisions) + // Return the installed bundle so the Apply step runs and maintains resources. // Apply will set Progressing=Succeeded when it completes successfully. - return nil + return maintainInstalled(revisions), nil +} + +// GetPackageDeprecation returns catalog deprecation evidence for the desired +// package. CatalogSource requires a resolver that provides deprecations; +// production wiring always uses *resolve.CatalogResolver. +func (s *CatalogSource) GetPackageDeprecation(ctx context.Context, ext *ocv1.ClusterExtension) (*declcfg.Deprecation, error) { + deprecations, ok := s.Resolver.(resolve.PackageDeprecationSource) + if !ok { + return nil, fmt.Errorf("catalog source requires a resolver that provides package deprecations (got %T)", s.Resolver) + } + return deprecations.GetPackageDeprecation(ctx, ext) +} + +// GetChannelDeprecations returns catalog deprecation evidence for the desired +// channels; channel-scoped entries are filtered when the evidence is +// projected onto conditions. +func (s *CatalogSource) GetChannelDeprecations(ctx context.Context, ext *ocv1.ClusterExtension) (*declcfg.Deprecation, error) { + deprecations, ok := s.Resolver.(resolve.ChannelDeprecationSource) + if !ok { + return nil, fmt.Errorf("catalog source requires a resolver that provides channel deprecations (got %T)", s.Resolver) + } + return deprecations.GetChannelDeprecations(ctx, ext) +} + +// GetBundleDeprecations returns catalog deprecation evidence for the given +// bundles; bundle-scoped entries are filtered when the evidence is projected +// onto conditions. +func (s *CatalogSource) GetBundleDeprecations(ctx context.Context, ext *ocv1.ClusterExtension, bundles ...ocv1.BundleMetadata) (*declcfg.Deprecation, error) { + deprecations, ok := s.Resolver.(resolve.BundleDeprecationSource) + if !ok { + return nil, fmt.Errorf("catalog source requires a resolver that provides bundle deprecations (got %T)", s.Resolver) + } + return deprecations.GetBundleDeprecations(ctx, ext, bundles...) +} + +// ReconcileOnCatalogChange preserves the existing fan-out to Catalog extensions. +// Catalog-to-extension provenance and selector-specific filtering remain unchanged. +func (s *CatalogSource) ReconcileOnCatalogChange(*ocv1.ClusterExtension, *ocv1.ClusterCatalog) bool { + return true +} + +// CanMaintainWithoutContent preserves the original name/version comparison. +func (s *CatalogSource) CanMaintainWithoutContent(_ *ocv1.ClusterExtension, revisions *RevisionStates, resolved *RevisionMetadata) bool { + return revisions != nil && revisions.Installed != nil && resolved != nil && + resolved.Name == revisions.Installed.Name && + resolved.Version == revisions.Installed.Version } // getCatalogNameFromSelector extracts the catalog name from the selector if available. diff --git a/internal/operator-controller/controllers/clusterextension_controller.go b/internal/operator-controller/controllers/clusterextension_controller.go index 2160922a08..0911861e0e 100644 --- a/internal/operator-controller/controllers/clusterextension_controller.go +++ b/internal/operator-controller/controllers/clusterextension_controller.go @@ -64,8 +64,7 @@ type reconcileState struct { revisionStates *RevisionStates resolvedRevisionMetadata *RevisionMetadata imageFS fs.FS - bundleSource BundleSource - bundleResolution BundleResolution + bundleSource Resolver } // ReconcileStepFunc represents a single step in the ClusterExtension reconciliation process. @@ -103,7 +102,7 @@ func (steps *ReconcileSteps) Reconcile(ctx context.Context, ext *ocv1.ClusterExt type ClusterExtensionReconciler struct { client.Client ReconcileSteps ReconcileSteps - BundleSources BundleSources + Resolvers Resolvers } type StorageMigrator interface { @@ -206,31 +205,39 @@ func ensureFailureConditionsWithReason(ext *ocv1.ClusterExtension, reason v1alph } } -// SetDeprecationStatus updates deprecation conditions based on catalog metadata. +// DeprecationEvidence is the deprecation data collected from a source's +// optional deprecation interfaces (see resolve.PackageDeprecationSource and +// friends). Known is false when a capable source's query failed; sources +// that do not implement the interfaces produce known, empty evidence and are +// reported exactly like sources whose evaluation found no deprecations. +type DeprecationEvidence struct { + PackageDeprecation *declcfg.Deprecation + ChannelDeprecation *declcfg.Deprecation + BundleDeprecation *declcfg.Deprecation + Known bool +} + +// SetDeprecationStatus updates deprecation conditions based on source evidence. // // Behavior (following Kubernetes API conventions - conditions always present): // - IS deprecated -> condition True with Reason: Deprecated // - NOT deprecated -> condition False with Reason: NotDeprecated -// - Can't check (no catalog) -> condition Unknown with Reason: DeprecationStatusUnknown +// - Can't check (evidence unavailable) -> condition Unknown with Reason: DeprecationStatusUnknown // - No bundle installed -> BundleDeprecated Unknown with Reason: Absent // -// This keeps deprecation conditions focused on catalog data. Install/validation errors -// never appear here - they belong in Progressing/Installed conditions. -func SetDeprecationStatus(ext *ocv1.ClusterExtension, installedBundleName string, deprecation *declcfg.Deprecation, hasCatalogData bool) { - info := buildDeprecationInfo(ext, installedBundleName, deprecation) - packageMessages := collectDeprecationMessages(info.PackageEntries) - channelMessages := collectDeprecationMessages(info.ChannelEntries) - bundleMessages := collectDeprecationMessages(info.BundleEntries) - +// Sources without deprecation data produce the same conditions as sources +// whose evaluation found no deprecations. Install/validation errors never +// appear here - they belong in Progressing/Installed conditions. +func SetDeprecationStatus(ext *ocv1.ClusterExtension, installedBundleName string, evidence DeprecationEvidence) { // Strategy: Always set deprecation conditions (following Kubernetes API conventions). // SetStatusCondition preserves lastTransitionTime when status/reason/message haven't changed, // preventing infinite reconciliation loops. // - True = deprecated - // - False = not deprecated (verified via catalog) - // - Unknown = cannot verify (no catalog data or no bundle installed) + // - False = not deprecated (verified from source evidence) + // - Unknown = cannot verify (evidence unavailable or no bundle installed) - if !hasCatalogData { - // When catalog is unavailable, set all to Unknown. + if !evidence.Known { + // When evidence is unavailable, set all to Unknown. // BundleDeprecated uses Absent only when no bundle installed. bundleReason := ocv1.ReasonAbsent bundleMessage := "no bundle installed yet" @@ -269,7 +276,12 @@ func SetDeprecationStatus(ext *ocv1.ClusterExtension, installedBundleName string return } - // Handle catalog data available: set conditions to True when deprecated, False when not. + packageMessages := collectDeprecationMessages(packageDeprecationEntries(evidence.PackageDeprecation)) + channelMessages := collectDeprecationMessages(channelDeprecationEntries(ext, evidence.ChannelDeprecation)) + bundleEntries, bundleStatus := bundleDeprecationEntries(evidence.BundleDeprecation, installedBundleName) + bundleMessages := collectDeprecationMessages(bundleEntries) + + // Handle available evidence: set conditions to True when deprecated, False when not. messages := slices.Concat(packageMessages, channelMessages, bundleMessages) if len(messages) > 0 { SetStatusCondition(&ext.Status.Conditions, metav1.Condition{ @@ -326,7 +338,7 @@ func SetDeprecationStatus(ext *ocv1.ClusterExtension, installedBundleName string } // BundleDeprecated: Unknown when no bundle installed, True when deprecated, False when not - if info.BundleStatus == metav1.ConditionUnknown { + if bundleStatus == metav1.ConditionUnknown { SetStatusCondition(&ext.Status.Conditions, metav1.Condition{ Type: ocv1.TypeBundleDeprecated, Status: metav1.ConditionUnknown, @@ -364,55 +376,68 @@ func isDeprecationCondition(condType string) bool { } } -// deprecationInfo captures the deprecation data needed to update condition status. -type deprecationInfo struct { - PackageEntries []declcfg.DeprecationEntry - ChannelEntries []declcfg.DeprecationEntry - BundleEntries []declcfg.DeprecationEntry - BundleStatus metav1.ConditionStatus +// packageDeprecationEntries returns the package-scoped deprecation entries. +func packageDeprecationEntries(deprecation *declcfg.Deprecation) []declcfg.DeprecationEntry { + if deprecation == nil { + return nil + } + var entries []declcfg.DeprecationEntry + for _, entry := range deprecation.Entries { + if entry.Reference.Schema == declcfg.SchemaPackage { + entries = append(entries, entry) + } + } + return entries } -// buildDeprecationInfo filters the catalog deprecation data down to the package, channel, -// and bundle entries that matter for this ClusterExtension. An empty bundle name means -// nothing is installed yet, so we leave bundle status Unknown/Absent. -func buildDeprecationInfo(ext *ocv1.ClusterExtension, installedBundleName string, deprecation *declcfg.Deprecation) deprecationInfo { - info := deprecationInfo{BundleStatus: metav1.ConditionUnknown} +// channelDeprecationEntries returns the channel-scoped deprecation entries. +// When no channels are desired, any channel could be auto-selected, so all +// channel deprecations apply. +func channelDeprecationEntries(ext *ocv1.ClusterExtension, deprecation *declcfg.Deprecation) []declcfg.DeprecationEntry { + if deprecation == nil { + return nil + } channelSet := sets.New[string]() if ext.Spec.Source.Catalog != nil { channelSet.Insert(ext.Spec.Source.Catalog.Channels...) } + var entries []declcfg.DeprecationEntry + for _, entry := range deprecation.Entries { + if entry.Reference.Schema != declcfg.SchemaChannel { + continue + } + // Include channel deprecations if: + // 1. No channels specified (channelSet empty) - any channel could be auto-selected + // 2. The deprecated channel matches one of the specified channels + if len(channelSet) == 0 || channelSet.Has(entry.Reference.Name) { + entries = append(entries, entry) + } + } + return entries +} +// bundleDeprecationEntries returns the bundle-scoped deprecation entries that +// match the installed bundle, plus the status for the BundleDeprecated +// condition: Unknown when nothing is installed, True when the installed +// bundle is deprecated, False when it is not. +func bundleDeprecationEntries(deprecation *declcfg.Deprecation, installedBundleName string) ([]declcfg.DeprecationEntry, metav1.ConditionStatus) { + // installedBundleName is empty when nothing is installed. In that case we + // want to report the bundle deprecation condition as Unknown/Absent. + if installedBundleName == "" { + return nil, metav1.ConditionUnknown + } + var entries []declcfg.DeprecationEntry if deprecation != nil { for _, entry := range deprecation.Entries { - switch entry.Reference.Schema { - case declcfg.SchemaPackage: - info.PackageEntries = append(info.PackageEntries, entry) - case declcfg.SchemaChannel: - // Include channel deprecations if: - // 1. No channels specified (channelSet empty) - any channel could be auto-selected - // 2. The deprecated channel matches one of the specified channels - if len(channelSet) == 0 || channelSet.Has(entry.Reference.Name) { - info.ChannelEntries = append(info.ChannelEntries, entry) - } - case declcfg.SchemaBundle: - if installedBundleName != "" && entry.Reference.Name == installedBundleName { - info.BundleEntries = append(info.BundleEntries, entry) - } + if entry.Reference.Schema == declcfg.SchemaBundle && entry.Reference.Name == installedBundleName { + entries = append(entries, entry) } } } - - // installedBundleName is empty when nothing is installed. In that case we want - // to report the bundle deprecation condition as Unknown/Absent. - if installedBundleName != "" { - if len(info.BundleEntries) > 0 { - info.BundleStatus = metav1.ConditionTrue - } else { - info.BundleStatus = metav1.ConditionFalse - } + if len(entries) > 0 { + return entries, metav1.ConditionTrue } - - return info + return entries, metav1.ConditionFalse } // collectDeprecationMessages collects the non-empty deprecation messages from the provided entries. @@ -458,7 +483,7 @@ func (r *ClusterExtensionReconciler) newControllerBuilder(mgr ctrl.Manager) *ctr For(&ocv1.ClusterExtension{}). Named("controller-operator-cluster-extension-controller"). Watches(&ocv1.ClusterCatalog{}, - crhandler.EnqueueRequestsFromMapFunc(clusterExtensionRequestsForCatalog(mgr.GetClient(), mgr.GetLogger(), r.BundleSources)), + crhandler.EnqueueRequestsFromMapFunc(clusterExtensionRequestsForCatalog(mgr.GetClient(), mgr.GetLogger(), r.Resolvers)), builder.WithPredicates(predicate.Funcs{UpdateFunc: catalogResolutionInputsChanged})) } @@ -495,7 +520,7 @@ func wrapErrorWithResolutionInfo(resolved ocv1.BundleMetadata, err error) error } // Generate reconcile requests for all cluster extensions affected by a catalog change -func clusterExtensionRequestsForCatalog(c client.Reader, logger logr.Logger, sources BundleSources) crhandler.MapFunc { +func clusterExtensionRequestsForCatalog(c client.Reader, logger logr.Logger, sources Resolvers) crhandler.MapFunc { return func(ctx context.Context, obj client.Object) []reconcile.Request { catalog, ok := obj.(*ocv1.ClusterCatalog) if !ok { diff --git a/internal/operator-controller/controllers/clusterextension_controller_test.go b/internal/operator-controller/controllers/clusterextension_controller_test.go index eeaf5261fe..063d9ee792 100644 --- a/internal/operator-controller/controllers/clusterextension_controller_test.go +++ b/internal/operator-controller/controllers/clusterextension_controller_test.go @@ -43,6 +43,54 @@ import ( mockhelmclient "github.com/operator-framework/operator-controller/internal/testutil/mock/helmclient" ) +// fakeCatalogResolver adapts the four-value resolver results these tests were +// written against to the slimmed resolve.Resolver interface and the optional +// deprecation source interfaces. Each resolution attempt records the +// deprecation it returned, and deprecation queries replay the evidence of the +// most recent attempt: evidence is known when that attempt succeeded or +// produced deprecation data, matching the catalog resolver's historical +// contract. Before any attempt, evidence is unknown. +type fakeCatalogResolver struct { + fn func(ctx context.Context, ext *ocv1.ClusterExtension, installedBundle *ocv1.BundleMetadata) (*declcfg.Bundle, *declcfg.VersionRelease, *declcfg.Deprecation, error) + + attempted bool + lastDeprecation *declcfg.Deprecation + lastErr error +} + +// fakeResolver wraps fn so existing test bodies can keep returning the +// historical four-value resolver results. +func fakeResolver(fn func(ctx context.Context, ext *ocv1.ClusterExtension, installedBundle *ocv1.BundleMetadata) (*declcfg.Bundle, *declcfg.VersionRelease, *declcfg.Deprecation, error)) resolve.Resolver { + return &fakeCatalogResolver{fn: fn} +} + +func (f *fakeCatalogResolver) Resolve(ctx context.Context, ext *ocv1.ClusterExtension, installedBundle *ocv1.BundleMetadata) (*declcfg.Bundle, *declcfg.VersionRelease, error) { + bundle, version, deprecation, err := f.fn(ctx, ext, installedBundle) + f.attempted = true + f.lastDeprecation = deprecation + f.lastErr = err + return bundle, version, err +} + +func (f *fakeCatalogResolver) GetPackageDeprecation(context.Context, *ocv1.ClusterExtension) (*declcfg.Deprecation, error) { + return f.evidence() +} + +func (f *fakeCatalogResolver) GetChannelDeprecations(context.Context, *ocv1.ClusterExtension) (*declcfg.Deprecation, error) { + return f.evidence() +} + +func (f *fakeCatalogResolver) GetBundleDeprecations(context.Context, *ocv1.ClusterExtension, ...ocv1.BundleMetadata) (*declcfg.Deprecation, error) { + return f.evidence() +} + +func (f *fakeCatalogResolver) evidence() (*declcfg.Deprecation, error) { + if !f.attempted || (f.lastErr != nil && f.lastDeprecation == nil) { + return nil, errors.New("deprecation status unknown: catalog data unavailable") + } + return f.lastDeprecation, nil +} + // Describe: ClusterExtension Controller Test func TestClusterExtensionDoesNotExist(t *testing.T) { _, reconciler := newClientAndReconciler(t) @@ -127,7 +175,7 @@ func TestClusterExtensionShortCircuitsReconcileDuringDeletion(t *testing.T) { func TestClusterExtensionResolutionFails(t *testing.T) { pkgName := fmt.Sprintf("non-existent-%s", rand.String(6)) cl, reconciler := newClientAndReconciler(t, func(d *deps) { - d.Resolver = resolve.Func(func(ctx context.Context, ext *ocv1.ClusterExtension, installedBundle *ocv1.BundleMetadata) (*declcfg.Bundle, *declcfg.VersionRelease, *declcfg.Deprecation, error) { + d.Resolver = fakeResolver(func(ctx context.Context, ext *ocv1.ClusterExtension, installedBundle *ocv1.BundleMetadata) (*declcfg.Bundle, *declcfg.VersionRelease, *declcfg.Deprecation, error) { return nil, nil, nil, fmt.Errorf("no package %q found", pkgName) }) }) @@ -189,7 +237,7 @@ func TestClusterExtensionResolutionFailsWithDeprecationData(t *testing.T) { pkgName := fmt.Sprintf("deprecated-%s", rand.String(6)) deprecationMessage := "package marked deprecated in catalog" cl, reconciler := newClientAndReconciler(t, func(d *deps) { - d.Resolver = resolve.Func(func(ctx context.Context, ext *ocv1.ClusterExtension, installedBundle *ocv1.BundleMetadata) (*declcfg.Bundle, *declcfg.VersionRelease, *declcfg.Deprecation, error) { + d.Resolver = fakeResolver(func(ctx context.Context, ext *ocv1.ClusterExtension, installedBundle *ocv1.BundleMetadata) (*declcfg.Bundle, *declcfg.VersionRelease, *declcfg.Deprecation, error) { return nil, nil, &declcfg.Deprecation{ Entries: []declcfg.DeprecationEntry{{ Reference: declcfg.PackageScopedReference{Schema: declcfg.SchemaPackage}, @@ -254,7 +302,7 @@ func TestClusterExtensionUpgradeShowsInstalledBundleDeprecation(t *testing.T) { deprecationMessage := "v1.0.0 is deprecated, please upgrade to v2.0.0" cl, reconciler := newClientAndReconciler(t, func(d *deps) { - d.Resolver = resolve.Func(func(ctx context.Context, ext *ocv1.ClusterExtension, installedBundle *ocv1.BundleMetadata) (*declcfg.Bundle, *declcfg.VersionRelease, *declcfg.Deprecation, error) { + d.Resolver = fakeResolver(func(ctx context.Context, ext *ocv1.ClusterExtension, installedBundle *ocv1.BundleMetadata) (*declcfg.Bundle, *declcfg.VersionRelease, *declcfg.Deprecation, error) { v := declcfg.VersionRelease{ Version: bsemver.MustParse("2.0.0"), } @@ -354,7 +402,7 @@ func TestClusterExtensionUpgradeFromDeprecatedBundleClearsDeprecation(t *testing deprecationMessage := fmt.Sprintf("%s is deprecated. Uninstall and install v1.0.3 for support.", installedBundleName) cl, reconciler := newClientAndReconciler(t, func(d *deps) { - d.Resolver = resolve.Func(func(ctx context.Context, ext *ocv1.ClusterExtension, installedBundle *ocv1.BundleMetadata) (*declcfg.Bundle, *declcfg.VersionRelease, *declcfg.Deprecation, error) { + d.Resolver = fakeResolver(func(ctx context.Context, ext *ocv1.ClusterExtension, installedBundle *ocv1.BundleMetadata) (*declcfg.Bundle, *declcfg.VersionRelease, *declcfg.Deprecation, error) { v := declcfg.VersionRelease{ Version: bsemver.MustParse("1.0.3"), } @@ -450,7 +498,7 @@ func TestClusterExtensionResolutionFailsWithoutCatalogDeprecationData(t *testing catalogName := fmt.Sprintf("test-catalog-%s", rand.String(6)) installedBundleName := fmt.Sprintf("%s.v1.0.0", pkgName) cl, reconciler := newClientAndReconciler(t, func(d *deps) { - d.Resolver = resolve.Func(func(ctx context.Context, ext *ocv1.ClusterExtension, installedBundle *ocv1.BundleMetadata) (*declcfg.Bundle, *declcfg.VersionRelease, *declcfg.Deprecation, error) { + d.Resolver = fakeResolver(func(ctx context.Context, ext *ocv1.ClusterExtension, installedBundle *ocv1.BundleMetadata) (*declcfg.Bundle, *declcfg.VersionRelease, *declcfg.Deprecation, error) { return nil, nil, nil, fmt.Errorf("no bundles found for package %q", pkgName) }) @@ -571,7 +619,7 @@ func TestClusterExtensionResolutionSuccessfulUnpackFails(t *testing.T) { } }, func(d *deps) { - d.Resolver = resolve.Func(func(ctx context.Context, ext *ocv1.ClusterExtension, installedBundle *ocv1.BundleMetadata) (*declcfg.Bundle, *declcfg.VersionRelease, *declcfg.Deprecation, error) { + d.Resolver = fakeResolver(func(ctx context.Context, ext *ocv1.ClusterExtension, installedBundle *ocv1.BundleMetadata) (*declcfg.Bundle, *declcfg.VersionRelease, *declcfg.Deprecation, error) { v := declcfg.VersionRelease{ Version: bsemver.MustParse("1.0.0"), } @@ -648,7 +696,7 @@ func TestClusterExtensionResolutionAndUnpackSuccessfulApplierFails(t *testing.T) d.ImagePuller = &imageutil.FakePuller{ ImageFS: fstest.MapFS{}, } - d.Resolver = resolve.Func(func(ctx context.Context, ext *ocv1.ClusterExtension, installedBundle *ocv1.BundleMetadata) (*declcfg.Bundle, *declcfg.VersionRelease, *declcfg.Deprecation, error) { + d.Resolver = fakeResolver(func(ctx context.Context, ext *ocv1.ClusterExtension, installedBundle *ocv1.BundleMetadata) (*declcfg.Bundle, *declcfg.VersionRelease, *declcfg.Deprecation, error) { v := declcfg.VersionRelease{ Version: bsemver.MustParse("1.0.0"), } @@ -760,7 +808,7 @@ func TestClusterExtensionBoxcutterApplierFailsDoesNotLeakDeprecationErrors(t *te d.RevisionStatesGetter = newMockRevisionStatesGetter(gomock.NewController(t), &controllers.RevisionStates{ RollingOut: []*controllers.RevisionMetadata{{}}, }, nil) - d.Resolver = resolve.Func(func(ctx context.Context, ext *ocv1.ClusterExtension, installedBundle *ocv1.BundleMetadata) (*declcfg.Bundle, *declcfg.VersionRelease, *declcfg.Deprecation, error) { + d.Resolver = fakeResolver(func(ctx context.Context, ext *ocv1.ClusterExtension, installedBundle *ocv1.BundleMetadata) (*declcfg.Bundle, *declcfg.VersionRelease, *declcfg.Deprecation, error) { v := declcfg.VersionRelease{ Version: bsemver.MustParse("1.0.0"), } @@ -1086,7 +1134,7 @@ func TestClusterExtensionApplierFailsWithBundleInstalled(t *testing.T) { d.ImagePuller = &imageutil.FakePuller{ ImageFS: fstest.MapFS{}, } - d.Resolver = resolve.Func(func(ctx context.Context, ext *ocv1.ClusterExtension, installedBundle *ocv1.BundleMetadata) (*declcfg.Bundle, *declcfg.VersionRelease, *declcfg.Deprecation, error) { + d.Resolver = fakeResolver(func(ctx context.Context, ext *ocv1.ClusterExtension, installedBundle *ocv1.BundleMetadata) (*declcfg.Bundle, *declcfg.VersionRelease, *declcfg.Deprecation, error) { v := declcfg.VersionRelease{ Version: bsemver.MustParse("1.0.0"), } @@ -1172,7 +1220,7 @@ func TestClusterExtensionManagerFailed(t *testing.T) { d.ImagePuller = &imageutil.FakePuller{ ImageFS: fstest.MapFS{}, } - d.Resolver = resolve.Func(func(ctx context.Context, ext *ocv1.ClusterExtension, installedBundle *ocv1.BundleMetadata) (*declcfg.Bundle, *declcfg.VersionRelease, *declcfg.Deprecation, error) { + d.Resolver = fakeResolver(func(ctx context.Context, ext *ocv1.ClusterExtension, installedBundle *ocv1.BundleMetadata) (*declcfg.Bundle, *declcfg.VersionRelease, *declcfg.Deprecation, error) { v := declcfg.VersionRelease{ Version: bsemver.MustParse("1.0.0"), } @@ -1244,7 +1292,7 @@ func TestClusterExtensionManagedContentCacheWatchFail(t *testing.T) { d.ImagePuller = &imageutil.FakePuller{ ImageFS: fstest.MapFS{}, } - d.Resolver = resolve.Func(func(ctx context.Context, ext *ocv1.ClusterExtension, installedBundle *ocv1.BundleMetadata) (*declcfg.Bundle, *declcfg.VersionRelease, *declcfg.Deprecation, error) { + d.Resolver = fakeResolver(func(ctx context.Context, ext *ocv1.ClusterExtension, installedBundle *ocv1.BundleMetadata) (*declcfg.Bundle, *declcfg.VersionRelease, *declcfg.Deprecation, error) { v := declcfg.VersionRelease{ Version: bsemver.MustParse("1.0.0"), } @@ -1318,7 +1366,7 @@ func TestClusterExtensionInstallationSucceeds(t *testing.T) { d.ImagePuller = &imageutil.FakePuller{ ImageFS: fstest.MapFS{}, } - d.Resolver = resolve.Func(func(ctx context.Context, ext *ocv1.ClusterExtension, installedBundle *ocv1.BundleMetadata) (*declcfg.Bundle, *declcfg.VersionRelease, *declcfg.Deprecation, error) { + d.Resolver = fakeResolver(func(ctx context.Context, ext *ocv1.ClusterExtension, installedBundle *ocv1.BundleMetadata) (*declcfg.Bundle, *declcfg.VersionRelease, *declcfg.Deprecation, error) { v := declcfg.VersionRelease{ Version: bsemver.MustParse("1.0.0"), } @@ -1393,7 +1441,7 @@ func TestClusterExtensionDeleteFinalizerFails(t *testing.T) { d.ImagePuller = &imageutil.FakePuller{ ImageFS: fstest.MapFS{}, } - d.Resolver = resolve.Func(func(ctx context.Context, ext *ocv1.ClusterExtension, installedBundle *ocv1.BundleMetadata) (*declcfg.Bundle, *declcfg.VersionRelease, *declcfg.Deprecation, error) { + d.Resolver = fakeResolver(func(ctx context.Context, ext *ocv1.ClusterExtension, installedBundle *ocv1.BundleMetadata) (*declcfg.Bundle, *declcfg.VersionRelease, *declcfg.Deprecation, error) { v := declcfg.VersionRelease{ Version: bsemver.MustParse("1.0.0"), } @@ -2308,7 +2356,7 @@ func TestSetDeprecationStatus(t *testing.T) { require.Failf(t, "test case must set catalogDataProvided when deprecation is supplied", "test case %q", tc.name) } hasCatalogData := tc.catalogDataProvided && tc.hasCatalogData - controllers.SetDeprecationStatus(tc.clusterExtension, tc.bundle.Name, tc.deprecation, hasCatalogData) + controllers.SetDeprecationStatus(tc.clusterExtension, tc.bundle.Name, controllers.DeprecationEvidence{PackageDeprecation: tc.deprecation, ChannelDeprecation: tc.deprecation, BundleDeprecation: tc.deprecation, Known: hasCatalogData}) // TODO: we should test for unexpected changes to lastTransitionTime. We only expect // lastTransitionTime to change when the status of the condition changes. assert.Empty(t, cmp.Diff(tc.expectedClusterExtension, tc.clusterExtension, cmpopts.IgnoreFields(metav1.Condition{}, "LastTransitionTime"))) @@ -2410,7 +2458,7 @@ func TestSetDeprecationStatus_NoInfiniteReconcileLoop(t *testing.T) { } // First reconcile: should add/update conditions - controllers.SetDeprecationStatus(ext, tt.installedBundleName, tt.deprecation, tt.hasCatalogData) + controllers.SetDeprecationStatus(ext, tt.installedBundleName, controllers.DeprecationEvidence{PackageDeprecation: tt.deprecation, ChannelDeprecation: tt.deprecation, BundleDeprecation: tt.deprecation, Known: tt.hasCatalogData}) firstReconcileConditions := make([]metav1.Condition, len(ext.Status.Conditions)) copy(firstReconcileConditions, ext.Status.Conditions) @@ -2421,7 +2469,7 @@ func TestSetDeprecationStatus_NoInfiniteReconcileLoop(t *testing.T) { "First reconcile should have %d deprecation conditions", tt.expectConditionsCount) // Second reconcile: should preserve lastTransitionTime (no changes) - controllers.SetDeprecationStatus(ext, tt.installedBundleName, tt.deprecation, tt.hasCatalogData) + controllers.SetDeprecationStatus(ext, tt.installedBundleName, controllers.DeprecationEvidence{PackageDeprecation: tt.deprecation, ChannelDeprecation: tt.deprecation, BundleDeprecation: tt.deprecation, Known: tt.hasCatalogData}) secondReconcileConditions := ext.Status.Conditions @@ -2442,7 +2490,7 @@ func TestSetDeprecationStatus_NoInfiniteReconcileLoop(t *testing.T) { } // Third reconcile: verify it remains stable - controllers.SetDeprecationStatus(ext, tt.installedBundleName, tt.deprecation, tt.hasCatalogData) + controllers.SetDeprecationStatus(ext, tt.installedBundleName, controllers.DeprecationEvidence{PackageDeprecation: tt.deprecation, ChannelDeprecation: tt.deprecation, BundleDeprecation: tt.deprecation, Known: tt.hasCatalogData}) thirdReconcileConditions := ext.Status.Conditions require.Len(t, thirdReconcileConditions, len(secondReconcileConditions), @@ -2480,11 +2528,11 @@ func TestSetDeprecationStatus_StatusChangesOnlyWhenNeeded(t *testing.T) { } // First reconcile: add deprecation condition - controllers.SetDeprecationStatus(ext, "test.v1.0.0", deprecation, true) + controllers.SetDeprecationStatus(ext, "test.v1.0.0", controllers.DeprecationEvidence{PackageDeprecation: deprecation, ChannelDeprecation: deprecation, BundleDeprecation: deprecation, Known: true}) statusAfterFirstReconcile := ext.Status.DeepCopy() // Second reconcile: same deprecation state - controllers.SetDeprecationStatus(ext, "test.v1.0.0", deprecation, true) + controllers.SetDeprecationStatus(ext, "test.v1.0.0", controllers.DeprecationEvidence{PackageDeprecation: deprecation, ChannelDeprecation: deprecation, BundleDeprecation: deprecation, Known: true}) statusAfterSecondReconcile := ext.Status.DeepCopy() // Status should be semantically equal (DeepEqual would return true) @@ -2492,7 +2540,7 @@ func TestSetDeprecationStatus_StatusChangesOnlyWhenNeeded(t *testing.T) { "Status should not change when deprecation state is unchanged") // Scenario 2: Deprecation is resolved (package no longer deprecated) - controllers.SetDeprecationStatus(ext, "test.v1.0.0", nil, true) + controllers.SetDeprecationStatus(ext, "test.v1.0.0", controllers.DeprecationEvidence{}) statusAfterResolution := ext.Status.DeepCopy() // Status should have changed (conditions removed) @@ -2500,7 +2548,7 @@ func TestSetDeprecationStatus_StatusChangesOnlyWhenNeeded(t *testing.T) { "Status should change when deprecation is resolved") // Scenario 3: Verify resolution is stable - controllers.SetDeprecationStatus(ext, "test.v1.0.0", nil, true) + controllers.SetDeprecationStatus(ext, "test.v1.0.0", controllers.DeprecationEvidence{}) statusAfterFourthReconcile := ext.Status.DeepCopy() require.True(t, equality.Semantic.DeepEqual(statusAfterResolution, statusAfterFourthReconcile), @@ -2635,7 +2683,7 @@ func TestResolutionFallbackToInstalledBundle(t *testing.T) { resolveAttempt := 0 cl, reconciler := newClientAndReconciler(t, func(d *deps) { // First reconcile: catalog available, second reconcile: catalog unavailable - d.Resolver = resolve.Func(func(_ context.Context, _ *ocv1.ClusterExtension, _ *ocv1.BundleMetadata) (*declcfg.Bundle, *declcfg.VersionRelease, *declcfg.Deprecation, error) { + d.Resolver = fakeResolver(func(_ context.Context, _ *ocv1.ClusterExtension, _ *ocv1.BundleMetadata) (*declcfg.Bundle, *declcfg.VersionRelease, *declcfg.Deprecation, error) { resolveAttempt++ if resolveAttempt == 1 { // First reconcile: catalog available, resolve to version 1.0.0 @@ -2732,7 +2780,7 @@ func TestResolutionFallbackToInstalledBundle(t *testing.T) { t.Run("fails when version upgrade requested without catalog", func(t *testing.T) { cl, reconciler := newClientAndReconciler(t, func(d *deps) { - d.Resolver = resolve.Func(func(_ context.Context, _ *ocv1.ClusterExtension, _ *ocv1.BundleMetadata) (*declcfg.Bundle, *declcfg.VersionRelease, *declcfg.Deprecation, error) { + d.Resolver = fakeResolver(func(_ context.Context, _ *ocv1.ClusterExtension, _ *ocv1.BundleMetadata) (*declcfg.Bundle, *declcfg.VersionRelease, *declcfg.Deprecation, error) { return nil, nil, nil, fmt.Errorf("catalog unavailable") }) d.RevisionStatesGetter = newMockRevisionStatesGetter(gomock.NewController(t), &controllers.RevisionStates{ @@ -2789,7 +2837,7 @@ func TestResolutionFallbackToInstalledBundle(t *testing.T) { resolveAttempt := 0 cl, reconciler := newClientAndReconciler(t, func(d *deps) { // First attempt: catalog unavailable, then becomes available - d.Resolver = resolve.Func(func(_ context.Context, _ *ocv1.ClusterExtension, _ *ocv1.BundleMetadata) (*declcfg.Bundle, *declcfg.VersionRelease, *declcfg.Deprecation, error) { + d.Resolver = fakeResolver(func(_ context.Context, _ *ocv1.ClusterExtension, _ *ocv1.BundleMetadata) (*declcfg.Bundle, *declcfg.VersionRelease, *declcfg.Deprecation, error) { resolveAttempt++ if resolveAttempt == 1 { // First reconcile: catalog unavailable @@ -2884,7 +2932,7 @@ func TestResolutionFallbackToInstalledBundle(t *testing.T) { t.Run("retries when catalogs exist but resolution fails", func(t *testing.T) { cl, reconciler := newClientAndReconciler(t, func(d *deps) { // Resolver fails (transient issue) - d.Resolver = resolve.Func(func(_ context.Context, _ *ocv1.ClusterExtension, _ *ocv1.BundleMetadata) (*declcfg.Bundle, *declcfg.VersionRelease, *declcfg.Deprecation, error) { + d.Resolver = fakeResolver(func(_ context.Context, _ *ocv1.ClusterExtension, _ *ocv1.BundleMetadata) (*declcfg.Bundle, *declcfg.VersionRelease, *declcfg.Deprecation, error) { return nil, nil, nil, fmt.Errorf("transient catalog issue: cache stale") }) d.RevisionStatesGetter = newMockRevisionStatesGetter(gomock.NewController(t), &controllers.RevisionStates{ diff --git a/internal/operator-controller/controllers/clusterextension_reconcile_steps.go b/internal/operator-controller/controllers/clusterextension_reconcile_steps.go index 9a342f58b3..08cc222925 100644 --- a/internal/operator-controller/controllers/clusterextension_reconcile_steps.go +++ b/internal/operator-controller/controllers/clusterextension_reconcile_steps.go @@ -30,6 +30,7 @@ import ( "sigs.k8s.io/controller-runtime/pkg/log" ocv1 "github.com/operator-framework/operator-controller/api/v1" + "github.com/operator-framework/operator-controller/internal/operator-controller/resolve" "github.com/operator-framework/operator-controller/internal/shared/labels" imageutil "github.com/operator-framework/operator-controller/internal/shared/util/image" ) @@ -123,9 +124,12 @@ func RetrieveRevisionStates(r RevisionStatesGetter) ReconcileStepFunc { } } -// ResolveBundle selects the extension's source policy before resolving, including -// when a revision is already rolling out. Sources own recovery and status evidence. -func ResolveBundle(sources BundleSources) ReconcileStepFunc { +// ResolveBundle selects the extension's source policy, then resolves a +// bundle. The shared step owns engagement: the source payload is validated +// before rollout continuation, Resolve is called only at steady state, +// recovery runs only when Resolve fails, and deprecation evidence is +// collected from the source on every reconcile. +func ResolveBundle(sources Resolvers) ReconcileStepFunc { return func(ctx context.Context, state *reconcileState, ext *ocv1.ClusterExtension) (*ctrl.Result, error) { source, err := sources.For(ext) if err != nil { @@ -135,26 +139,95 @@ func ResolveBundle(sources BundleSources) ReconcileStepFunc { return nil, err } state.bundleSource = source - resolution, err := source.Resolve(ctx, ext, state.revisionStates) - state.bundleResolution = resolution - installedBundleName := "" - if state.revisionStates.Installed != nil { - installedBundleName = state.revisionStates.Installed.Name + + var resolveErr error + if err := source.ValidateSource(ext); err != nil { + // Malformed source payloads fail closed before rollout + // continuation, resolution, or any backend call. + resolveErr = err + } else if len(state.revisionStates.RollingOut) > 0 { + // Steady-state gate: continue the first rolling revision without + // consulting resolution. + state.resolvedRevisionMetadata = state.revisionStates.RollingOut[0] + } else { + var installedBundle *ocv1.BundleMetadata + if state.revisionStates.Installed != nil { + installedBundle = &state.revisionStates.Installed.BundleMetadata + } + var resolved *RevisionMetadata + resolved, resolveErr = source.Resolve(ctx, ext, installedBundle) + if resolveErr != nil { + resolved, resolveErr = source.RecoverFromResolutionError(ctx, ext, state.revisionStates, resolveErr) + } + state.resolvedRevisionMetadata = resolved } - source.SetDeprecationStatus(ext, installedBundleName, resolution) + // Deprecation evidence is collected from the source on every + // reconcile, including rollout continuation and resolution failures. + SetDeprecationStatus(ext, installedBundleName(state.revisionStates), deprecationEvidenceFromSource(ctx, source, ext, state.revisionStates)) - if err != nil { - setStatusProgressing(ext, err) + if resolveErr != nil { + setStatusProgressing(ext, resolveErr) setInstalledStatusFromRevisionStates(ext, state.revisionStates) - ensureFailureConditionsWithReason(ext, ocv1.ReasonRetrying, fmt.Sprintf("failed to resolve bundle: %v", err)) - return nil, err + ensureFailureConditionsWithReason(ext, ocv1.ReasonRetrying, fmt.Sprintf("failed to resolve bundle: %v", resolveErr)) + return nil, resolveErr } - state.resolvedRevisionMetadata = resolution.Revision return nil, nil } } +// deprecationEvidenceFromSource collects deprecation evidence from the +// source's optional deprecation interfaces. Sources that implement none of +// them have no deprecation data and are reported exactly like sources whose +// evaluation found no deprecations. A failing query from a capable source +// makes the evidence unknown for every dimension. +func deprecationEvidenceFromSource(ctx context.Context, source Resolver, ext *ocv1.ClusterExtension, revisions *RevisionStates) DeprecationEvidence { + l := log.FromContext(ctx) + evidence := DeprecationEvidence{Known: true} + if pds, ok := source.(resolve.PackageDeprecationSource); ok { + deprecation, err := pds.GetPackageDeprecation(ctx, ext) + if err != nil { + l.V(1).Info("package deprecation evidence unavailable", "error", err.Error()) + return DeprecationEvidence{} + } + evidence.PackageDeprecation = deprecation + } + if cds, ok := source.(resolve.ChannelDeprecationSource); ok { + deprecation, err := cds.GetChannelDeprecations(ctx, ext) + if err != nil { + l.V(1).Info("channel deprecation evidence unavailable", "error", err.Error()) + return DeprecationEvidence{} + } + evidence.ChannelDeprecation = deprecation + } + if bds, ok := source.(resolve.BundleDeprecationSource); ok { + var bundles []ocv1.BundleMetadata + if revisions != nil { + if revisions.Installed != nil { + bundles = append(bundles, revisions.Installed.BundleMetadata) + } + for _, rolling := range revisions.RollingOut { + bundles = append(bundles, rolling.BundleMetadata) + } + } + deprecation, err := bds.GetBundleDeprecations(ctx, ext, bundles...) + if err != nil { + l.V(1).Info("bundle deprecation evidence unavailable", "error", err.Error()) + return DeprecationEvidence{} + } + evidence.BundleDeprecation = deprecation + } + return evidence +} + +// installedBundleName returns the name of the installed bundle, if any. +func installedBundleName(revisions *RevisionStates) string { + if revisions != nil && revisions.Installed != nil { + return revisions.Installed.Name + } + return "" +} + func UnpackBundle(i imageutil.Puller, cache imageutil.Cache) ReconcileStepFunc { return func(ctx context.Context, state *reconcileState, ext *ocv1.ClusterExtension) (*ctrl.Result, error) { l := log.FromContext(ctx) @@ -169,7 +242,7 @@ func UnpackBundle(i imageutil.Puller, cache imageutil.Cache) ReconcileStepFunc { imageFS, _, _, err := i.Pull(ctx, ext.GetName(), state.resolvedRevisionMetadata.Image, cache) if err != nil { - if state.bundleSource != nil && state.bundleSource.CanMaintainWithoutContent(ext, state.revisionStates, state.bundleResolution) { + if state.bundleSource != nil && state.bundleSource.CanMaintainWithoutContent(ext, state.revisionStates, state.resolvedRevisionMetadata) { l.V(1).Info("bundle content unavailable, maintaining current installation", "bundle", state.resolvedRevisionMetadata.Name, "version", state.resolvedRevisionMetadata.Version, @@ -255,7 +328,7 @@ func ApplyBundle(a Applier) ReconcileStepFunc { // Refresh source-specific deprecation conditions for the newly running bundle. if rolloutSucceeded && state.revisionStates.Installed != nil && state.bundleSource != nil { - state.bundleSource.SetDeprecationStatus(ext, state.revisionStates.Installed.Name, state.bundleResolution) + SetDeprecationStatus(ext, state.revisionStates.Installed.Name, deprecationEvidenceFromSource(ctx, state.bundleSource, ext, state.revisionStates)) } // If there was an error applying the resolved bundle, diff --git a/internal/operator-controller/controllers/clusterextension_source_test.go b/internal/operator-controller/controllers/clusterextension_source_test.go index 9b4c2995c4..15764eef35 100644 --- a/internal/operator-controller/controllers/clusterextension_source_test.go +++ b/internal/operator-controller/controllers/clusterextension_source_test.go @@ -24,38 +24,56 @@ import ( imageutil "github.com/operator-framework/operator-controller/internal/shared/util/image" ) -// TestSource is an in-memory policy, not a production API source. In particular, -// it selects its own revision even while another revision is rolling out. +// TestSource is an in-memory policy, not a production API source. It provides +// no deprecation evidence: sources without deprecation data are reported +// exactly like sources whose evaluation found none. type TestSource struct { - result BundleResolution + result *RevisionMetadata err error maintain bool enqueue bool } -func (s *TestSource) Resolve(context.Context, *ocv1.ClusterExtension, *RevisionStates) (BundleResolution, error) { +func (s *TestSource) ValidateSource(*ocv1.ClusterExtension) error { + return nil +} + +func (s *TestSource) Resolve(context.Context, *ocv1.ClusterExtension, *ocv1.BundleMetadata) (*RevisionMetadata, error) { return s.result, s.err } -func (s *TestSource) SetDeprecationStatus(ext *ocv1.ClusterExtension, installed string, result BundleResolution) { - status := metav1.ConditionUnknown - if result.DeprecationKnown { - status = metav1.ConditionFalse - } - SetStatusCondition(&ext.Status.Conditions, metav1.Condition{ - Type: ocv1.TypeDeprecated, Status: status, Reason: "TestSourcePolicy", - Message: installed, ObservedGeneration: ext.Generation, - }) +func (s *TestSource) RecoverFromResolutionError(_ context.Context, _ *ocv1.ClusterExtension, _ *RevisionStates, resolveErr error) (*RevisionMetadata, error) { + return nil, resolveErr } func (s *TestSource) ReconcileOnCatalogChange(*ocv1.ClusterExtension, *ocv1.ClusterCatalog) bool { return s.enqueue } -func (s *TestSource) CanMaintainWithoutContent(*ocv1.ClusterExtension, *RevisionStates, BundleResolution) bool { +func (s *TestSource) CanMaintainWithoutContent(*ocv1.ClusterExtension, *RevisionStates, *RevisionMetadata) bool { return s.maintain } +// deprecatingResolver is a resolver fake that also provides deprecation +// evidence, standing in for *resolve.CatalogResolver. +type deprecatingResolver struct { + resolve.Func + deprecation *declcfg.Deprecation + queryErr error +} + +func (r deprecatingResolver) GetPackageDeprecation(context.Context, *ocv1.ClusterExtension) (*declcfg.Deprecation, error) { + return r.deprecation, r.queryErr +} + +func (r deprecatingResolver) GetChannelDeprecations(context.Context, *ocv1.ClusterExtension) (*declcfg.Deprecation, error) { + return r.deprecation, r.queryErr +} + +func (r deprecatingResolver) GetBundleDeprecations(context.Context, *ocv1.ClusterExtension, ...ocv1.BundleMetadata) (*declcfg.Deprecation, error) { + return r.deprecation, r.queryErr +} + type forbiddenCatalogClient struct{ client.Client } func (*forbiddenCatalogClient) List(context.Context, client.ObjectList, ...client.ListOption) error { @@ -82,36 +100,68 @@ func sourceRevision(name, version string) *RevisionMetadata { return &RevisionMetadata{Package: "example", Image: "example/bundle:latest", BundleMetadata: ocv1.BundleMetadata{Name: name, Version: version}} } -func forbiddenCatalogResolver(context.Context, *ocv1.ClusterExtension, *ocv1.BundleMetadata) (*declcfg.Bundle, *declcfg.VersionRelease, *declcfg.Deprecation, error) { +func forbiddenCatalogResolver(context.Context, *ocv1.ClusterExtension, *ocv1.BundleMetadata) (*declcfg.Bundle, *declcfg.VersionRelease, error) { panic("source isolation violated: catalog resolver used") } -func TestSourceSelectionOwnsResolutionAndDeprecation(t *testing.T) { +// TestSourceWithoutDeprecationDataManifestsAsNotDeprecated covers the contract +// for sources that implement none of the deprecation interfaces: absence of +// the capability manifests exactly like an evaluation that found no +// deprecations, on success and on resolution failure alike. +func TestSourceWithoutDeprecationDataManifestsAsNotDeprecated(t *testing.T) { + resolutionErr := errors.New("test source failed") + for _, tc := range []struct { + name string + err error + }{ + {name: "success"}, + {name: "resolution error", err: resolutionErr}, + } { + t.Run(tc.name, func(t *testing.T) { + installed := sourceRevision("old", "1.0.0") + state := &reconcileState{revisionStates: &RevisionStates{Installed: installed}} + source := &TestSource{result: sourceRevision("selected", "2.0.0"), err: tc.err} + ext := sourceExtension("TestSource") + _, err := ResolveBundle(Resolvers{"TestSource": source})(t.Context(), state, ext) + require.ErrorIs(t, err, tc.err) + for _, condType := range []string{ocv1.TypeDeprecated, ocv1.TypePackageDeprecated, ocv1.TypeChannelDeprecated, ocv1.TypeBundleDeprecated} { + condition := apimeta.FindStatusCondition(ext.Status.Conditions, condType) + require.NotNil(t, condition) + require.Equal(t, metav1.ConditionFalse, condition.Status) + require.Equal(t, ocv1.ReasonNotDeprecated, condition.Reason) + } + }) + } +} + +func TestSourceSelectionOwnsResolution(t *testing.T) { resolutionErr := errors.New("test source failed") for _, rolling := range []bool{false, true} { for _, sourceErr := range []error{nil, resolutionErr, reconcile.TerminalError(resolutionErr)} { t.Run(testingName(rolling, sourceErr), func(t *testing.T) { installed := sourceRevision("old", "1.0.0") target := sourceRevision("selected", "2.0.0") + rollingRevision := sourceRevision("rolling", "1.1.0") state := &reconcileState{revisionStates: &RevisionStates{Installed: installed}} if rolling { - state.revisionStates.RollingOut = []*RevisionMetadata{sourceRevision("rolling", "1.1.0")} + state.revisionStates.RollingOut = []*RevisionMetadata{rollingRevision} } - source := &TestSource{result: BundleResolution{Revision: target, DeprecationKnown: true}, err: sourceErr} - sources := BundleSources{ + source := &TestSource{result: target, err: sourceErr} + sources := Resolvers{ "TestSource": source, - ocv1.SourceTypeCatalog: &CatalogBundleSource{ + ocv1.SourceTypeCatalog: &CatalogSource{ Resolver: resolve.Func(forbiddenCatalogResolver), Client: &forbiddenCatalogClient{}, }, } ext := sourceExtension("TestSource") _, err := ResolveBundle(sources)(t.Context(), state, ext) + if rolling { + // Rollout continuation is selected without consulting resolution. + require.NoError(t, err) + require.Same(t, rollingRevision, state.resolvedRevisionMetadata) + return + } require.ErrorIs(t, err, sourceErr) - condition := apimeta.FindStatusCondition(ext.Status.Conditions, ocv1.TypeDeprecated) - require.NotNil(t, condition) - require.Equal(t, "TestSourcePolicy", condition.Reason) - require.Equal(t, metav1.ConditionFalse, condition.Status) - require.Equal(t, "old", condition.Message) if sourceErr != nil { require.Nil(t, state.resolvedRevisionMetadata) require.Equal(t, installed.BundleMetadata, ext.Status.Install.Bundle) @@ -120,7 +170,6 @@ func TestSourceSelectionOwnsResolutionAndDeprecation(t *testing.T) { require.Same(t, target, state.resolvedRevisionMetadata) _, err = ApplyBundle(successfulSourceApplier{})(t.Context(), state, ext) require.NoError(t, err) - require.Equal(t, "selected", apimeta.FindStatusCondition(ext.Status.Conditions, ocv1.TypeDeprecated).Message) }) } } @@ -142,9 +191,9 @@ func TestSourceLookupRejectsUnknownAndNilBeforeRollout(t *testing.T) { for _, sourceType := range []string{"Unknown", "Nil", "TypedNil"} { for _, rolling := range []bool{false, true} { t.Run(sourceType+"/"+testingName(rolling, nil), func(t *testing.T) { - sources := BundleSources{ + sources := Resolvers{ "Nil": nil, "TypedNil": typedNil, - ocv1.SourceTypeCatalog: &CatalogBundleSource{Resolver: resolve.Func(forbiddenCatalogResolver), Client: &forbiddenCatalogClient{}}, + ocv1.SourceTypeCatalog: &CatalogSource{Resolver: resolve.Func(forbiddenCatalogResolver), Client: &forbiddenCatalogClient{}}, } state := &reconcileState{revisionStates: &RevisionStates{}} if rolling { @@ -162,12 +211,32 @@ func TestCatalogSourcePreservesFirstRollingRevision(t *testing.T) { first, last := sourceRevision("first", "1.0.0"), sourceRevision("last", "2.0.0") state := &reconcileState{revisionStates: &RevisionStates{RollingOut: []*RevisionMetadata{first, last}}} ext := sourceExtension(ocv1.SourceTypeCatalog) - sources := BundleSources{ocv1.SourceTypeCatalog: &CatalogBundleSource{Resolver: resolve.Func(forbiddenCatalogResolver), Client: &forbiddenCatalogClient{}}} + sources := Resolvers{ocv1.SourceTypeCatalog: &CatalogSource{Resolver: resolve.Func(forbiddenCatalogResolver), Client: &forbiddenCatalogClient{}}} _, err := ResolveBundle(sources)(t.Context(), state, ext) require.NoError(t, err) require.Same(t, first, state.resolvedRevisionMetadata) - require.False(t, state.bundleResolution.DeprecationKnown) - require.Equal(t, metav1.ConditionUnknown, apimeta.FindStatusCondition(ext.Status.Conditions, ocv1.TypeDeprecated).Status) +} + +// TestDeprecationEvidenceCollectedDuringRollout covers deprecation evidence +// collection while a rollout is in progress: resolution is not consulted +// (the resolver panics if called), and evidence still populates conditions. +func TestDeprecationEvidenceCollectedDuringRollout(t *testing.T) { + deprecation := &declcfg.Deprecation{Entries: []declcfg.DeprecationEntry{{ + Reference: declcfg.PackageScopedReference{Schema: declcfg.SchemaPackage, Name: "example"}, Message: "deprecated package", + }}} + rolling := sourceRevision("rolling", "1.1.0") + state := &reconcileState{revisionStates: &RevisionStates{RollingOut: []*RevisionMetadata{rolling}}} + ext := sourceExtension(ocv1.SourceTypeCatalog) + r := deprecatingResolver{ + Func: resolve.Func(forbiddenCatalogResolver), + deprecation: deprecation, + } + _, err := ResolveBundle(Resolvers{ocv1.SourceTypeCatalog: &CatalogSource{Resolver: r, Client: &forbiddenCatalogClient{}}})(t.Context(), state, ext) + require.NoError(t, err) + require.Same(t, rolling, state.resolvedRevisionMetadata) + condition := apimeta.FindStatusCondition(ext.Status.Conditions, ocv1.TypePackageDeprecated) + require.Equal(t, metav1.ConditionTrue, condition.Status) + require.Equal(t, "deprecated package", condition.Message) } func TestCatalogPolicyRequiresCatalogConfiguration(t *testing.T) { @@ -182,14 +251,14 @@ func TestCatalogPolicyRequiresCatalogConfiguration(t *testing.T) { } ext := sourceExtension(ocv1.SourceTypeCatalog) ext.Spec.Source.Catalog = nil - source := &CatalogBundleSource{Resolver: resolve.Func(forbiddenCatalogResolver), Client: &forbiddenCatalogClient{}} + source := &CatalogSource{Resolver: resolve.Func(forbiddenCatalogResolver), Client: &forbiddenCatalogClient{}} for _, rolling := range []bool{false, true} { t.Run(testingName(rolling, nil), func(t *testing.T) { state := &reconcileState{revisionStates: &RevisionStates{}} if rolling { state.revisionStates.RollingOut = []*RevisionMetadata{sourceRevision("rolling", "1.0.0")} } - _, err := ResolveBundle(BundleSources{ocv1.SourceTypeCatalog: source})(t.Context(), state, ext) + _, err := ResolveBundle(Resolvers{ocv1.SourceTypeCatalog: source})(t.Context(), state, ext) require.ErrorContains(t, err, "catalog source requires spec.source.catalog") require.Nil(t, state.resolvedRevisionMetadata) }) @@ -205,22 +274,25 @@ func TestCatalogSourceDeprecationEvidence(t *testing.T) { name string err error deprecation *declcfg.Deprecation - known bool + queryErr error status metav1.ConditionStatus }{ - {name: "success empty", known: true, status: metav1.ConditionFalse}, - {name: "error without evidence", err: resolutionErr, status: metav1.ConditionUnknown}, - {name: "error with evidence", err: resolutionErr, deprecation: deprecation, known: true, status: metav1.ConditionTrue}, + {name: "success empty", status: metav1.ConditionFalse}, + {name: "error without evidence", err: resolutionErr, queryErr: errors.New("catalog data unavailable"), status: metav1.ConditionUnknown}, + {name: "error with evidence", err: resolutionErr, deprecation: deprecation, status: metav1.ConditionTrue}, } { t.Run(tc.name, func(t *testing.T) { ext := sourceExtension(ocv1.SourceTypeCatalog) state := &reconcileState{revisionStates: &RevisionStates{}} - r := resolve.Func(func(context.Context, *ocv1.ClusterExtension, *ocv1.BundleMetadata) (*declcfg.Bundle, *declcfg.VersionRelease, *declcfg.Deprecation, error) { - return &declcfg.Bundle{Name: "example.v1", Package: "example", Image: "example/bundle:v1"}, &declcfg.VersionRelease{Version: semver.MustParse("1.0.0")}, tc.deprecation, tc.err - }) - _, err := ResolveBundle(BundleSources{ocv1.SourceTypeCatalog: &CatalogBundleSource{Resolver: r, Client: &forbiddenCatalogClient{}}})(t.Context(), state, ext) + r := deprecatingResolver{ + Func: resolve.Func(func(context.Context, *ocv1.ClusterExtension, *ocv1.BundleMetadata) (*declcfg.Bundle, *declcfg.VersionRelease, error) { + return &declcfg.Bundle{Name: "example.v1", Package: "example", Image: "example/bundle:v1"}, &declcfg.VersionRelease{Version: semver.MustParse("1.0.0")}, tc.err + }), + deprecation: tc.deprecation, + queryErr: tc.queryErr, + } + _, err := ResolveBundle(Resolvers{ocv1.SourceTypeCatalog: &CatalogSource{Resolver: r, Client: &forbiddenCatalogClient{}}})(t.Context(), state, ext) require.ErrorIs(t, err, tc.err) - require.Equal(t, tc.known, state.bundleResolution.DeprecationKnown) require.Equal(t, tc.status, apimeta.FindStatusCondition(ext.Status.Conditions, ocv1.TypePackageDeprecated).Status) }) } @@ -241,10 +313,10 @@ func TestCatalogSourceFallbackPreservesVersionStringPolicy(t *testing.T) { ext.Spec.Source.Catalog = &ocv1.CatalogFilter{PackageName: "example", Version: version} installed := sourceRevision("installed", "1.0.0") state := &reconcileState{revisionStates: &RevisionStates{Installed: installed}} - r := resolve.Func(func(context.Context, *ocv1.ClusterExtension, *ocv1.BundleMetadata) (*declcfg.Bundle, *declcfg.VersionRelease, *declcfg.Deprecation, error) { - return nil, nil, nil, resolutionErr + r := resolve.Func(func(context.Context, *ocv1.ClusterExtension, *ocv1.BundleMetadata) (*declcfg.Bundle, *declcfg.VersionRelease, error) { + return nil, nil, resolutionErr }) - _, err := ResolveBundle(BundleSources{ocv1.SourceTypeCatalog: &CatalogBundleSource{Resolver: r, Client: builder.Build()}})(t.Context(), state, ext) + _, err := ResolveBundle(Resolvers{ocv1.SourceTypeCatalog: &CatalogSource{Resolver: r, Client: builder.Build()}})(t.Context(), state, ext) if catalogExists || version == ">=1.0.0" { require.ErrorIs(t, err, resolutionErr) require.Nil(t, state.resolvedRevisionMetadata) @@ -265,19 +337,19 @@ func TestUnpackUsesSelectedSourceMaintenancePolicy(t *testing.T) { for _, tc := range []struct { name string sourceType string - source BundleSource + source Resolver wantError bool }{ - {name: "catalog legacy name version", sourceType: ocv1.SourceTypeCatalog, source: &CatalogBundleSource{Resolver: resolve.Func(forbiddenCatalogResolver), Client: &forbiddenCatalogClient{}}}, - {name: "test rejects maintenance", sourceType: "TestSource", source: &TestSource{result: BundleResolution{Revision: target}}, wantError: true}, - {name: "test permits maintenance", sourceType: "TestSource", source: &TestSource{result: BundleResolution{Revision: target}, maintain: true}}, + {name: "catalog legacy name version", sourceType: ocv1.SourceTypeCatalog, source: &CatalogSource{Resolver: resolve.Func(forbiddenCatalogResolver), Client: &forbiddenCatalogClient{}}}, + {name: "test rejects maintenance", sourceType: "TestSource", source: &TestSource{result: target}, wantError: true}, + {name: "test permits maintenance", sourceType: "TestSource", source: &TestSource{result: target, maintain: true}}, } { t.Run(tc.name, func(t *testing.T) { state := &reconcileState{ revisionStates: &RevisionStates{Installed: installed, RollingOut: []*RevisionMetadata{target}}, } ext := sourceExtension(tc.sourceType) - _, err := ResolveBundle(BundleSources{tc.sourceType: tc.source})(t.Context(), state, ext) + _, err := ResolveBundle(Resolvers{tc.sourceType: tc.source})(t.Context(), state, ext) require.NoError(t, err) _, err = UnpackBundle(&imageutil.FakePuller{Error: pullErr}, nil)(t.Context(), state, ext) if tc.wantError { @@ -300,8 +372,8 @@ func TestCatalogEventUsesSourcePolicy(t *testing.T) { objects = append(objects, ext) } c := fake.NewClientBuilder().WithScheme(sch).WithObjects(objects...).Build() - sources := BundleSources{ - ocv1.SourceTypeCatalog: &CatalogBundleSource{Client: &forbiddenCatalogClient{}}, + sources := Resolvers{ + ocv1.SourceTypeCatalog: &CatalogSource{Client: &forbiddenCatalogClient{}}, "TestSource": &TestSource{}, "InterestedTestSource": &TestSource{enqueue: true}, "Nil": nil, } requests := clusterExtensionRequestsForCatalog(c, logr.Discard(), sources)(t.Context(), &ocv1.ClusterCatalog{}) diff --git a/internal/operator-controller/controllers/suite_test.go b/internal/operator-controller/controllers/suite_test.go index 5de1a89729..16bc0ffb67 100644 --- a/internal/operator-controller/controllers/suite_test.go +++ b/internal/operator-controller/controllers/suite_test.go @@ -138,10 +138,10 @@ func newClientAndReconciler(t *testing.T, opts ...reconcilerOption) (client.Clie controllers.RetrieveRevisionStates(d.RevisionStatesGetter), } if r := d.Resolver; r != nil { - reconciler.BundleSources = controllers.BundleSources{ - ocv1.SourceTypeCatalog: &controllers.CatalogBundleSource{Resolver: r, Client: cl}, + reconciler.Resolvers = controllers.Resolvers{ + ocv1.SourceTypeCatalog: &controllers.CatalogSource{Resolver: r, Client: cl}, } - reconciler.ReconcileSteps = append(reconciler.ReconcileSteps, controllers.ResolveBundle(reconciler.BundleSources)) + reconciler.ReconcileSteps = append(reconciler.ReconcileSteps, controllers.ResolveBundle(reconciler.Resolvers)) } if i := d.ImagePuller; i != nil { reconciler.ReconcileSteps = append(reconciler.ReconcileSteps, controllers.UnpackBundle(i, d.ImageCache)) diff --git a/internal/operator-controller/resolve/catalog.go b/internal/operator-controller/resolve/catalog.go index b6b44186cd..e2d7b66685 100644 --- a/internal/operator-controller/resolve/catalog.go +++ b/internal/operator-controller/resolve/catalog.go @@ -32,41 +32,36 @@ type CatalogResolver struct { Validations []ValidationFunc } -// Resolve returns a Bundle from a catalog that needs to get installed on the cluster. -func (r *CatalogResolver) Resolve(ctx context.Context, ext *ocv1.ClusterExtension, installedBundle *ocv1.BundleMetadata) (*declcfg.Bundle, *declcfg.VersionRelease, *declcfg.Deprecation, error) { +// Resolve returns a Bundle from a catalog that needs to get installed on the +// cluster. Deprecation data is deliberately not part of the result: it is +// available through the deprecation source methods implemented by +// CatalogResolver. +func (r *CatalogResolver) Resolve(ctx context.Context, ext *ocv1.ClusterExtension, installedBundle *ocv1.BundleMetadata) (*declcfg.Bundle, *declcfg.VersionRelease, error) { l := log.FromContext(ctx) packageName := ext.Spec.Source.Catalog.PackageName versionRange := ext.Spec.Source.Catalog.Version channels := ext.Spec.Source.Catalog.Channels // Unless overridden, select all catalogs. - var selector = labels.Everything() - var err error - if ext.Spec.Source.Catalog != nil { - selector, err = metav1.LabelSelectorAsSelector(ext.Spec.Source.Catalog.Selector) - if err != nil { - return nil, nil, nil, fmt.Errorf("desired catalog selector is invalid: %w", err) - } - // A nothing (empty) selector selects everything - if selector == labels.Nothing() { - selector = labels.Everything() - } + selector, err := desiredCatalogSelector(ext) + if err != nil { + return nil, nil, err } var versionRangeConstraints bsemver.Range if versionRange != "" { versionRangeConstraints, err = compare.NewVersionRange(versionRange) if err != nil { - return nil, nil, nil, fmt.Errorf("desired version range %q is invalid: %w", versionRange, err) + return nil, nil, fmt.Errorf("desired version range %q is invalid: %w", versionRange, err) } } cat, packageFBC, err := r.selectPackage(ctx, packageName, selector) if err != nil { - return nil, nil, nil, err + return nil, nil, err } if cat == nil { - return nil, nil, nil, fmt.Errorf("package %q not found in any eligible catalog", packageName) + return nil, nil, fmt.Errorf("package %q not found in any eligible catalog", packageName) } // A non-nil deprecation means that one catalog's package data was read, @@ -75,9 +70,9 @@ func (r *CatalogResolver) Resolve(ctx context.Context, ext *ocv1.ClusterExtensio if len(packageFBC.Deprecations) > 0 { deprecation = &packageFBC.Deprecations[0] } - failed := func(err error) (*declcfg.Bundle, *declcfg.VersionRelease, *declcfg.Deprecation, error) { + failed := func(err error) (*declcfg.Bundle, *declcfg.VersionRelease, error) { l.Info("resolution failed", "catalog", cat.Name, "error", err) - return nil, nil, deprecation, fmt.Errorf("resolving package %q from catalog %q: %w", packageName, cat.Name, err) + return nil, nil, fmt.Errorf("resolving package %q from catalog %q: %w", packageName, cat.Name, err) } var predicates []filterutil.Predicate[declcfg.Bundle] @@ -130,7 +125,71 @@ func (r *CatalogResolver) Resolve(ctx context.Context, ext *ocv1.ClusterExtensio } l.V(4).Info("resolution succeeded", "catalog", cat.Name, "bundle", resolvedBundle.Name) - return resolvedBundle, resolvedBundleVersion, deprecation, nil + return resolvedBundle, resolvedBundleVersion, nil +} + +// GetPackageDeprecation returns the deprecation data recorded for the +// extension's desired package. It implements PackageDeprecationSource. +func (r *CatalogResolver) GetPackageDeprecation(ctx context.Context, ext *ocv1.ClusterExtension) (*declcfg.Deprecation, error) { + return r.packageDeprecation(ctx, ext) +} + +// GetChannelDeprecations returns the deprecation data recorded for the +// extension's desired package; channel-scoped entries are filtered when the +// evidence is projected onto conditions. It implements ChannelDeprecationSource. +func (r *CatalogResolver) GetChannelDeprecations(ctx context.Context, ext *ocv1.ClusterExtension) (*declcfg.Deprecation, error) { + return r.packageDeprecation(ctx, ext) +} + +// GetBundleDeprecations returns the deprecation data recorded for the +// extension's desired package; bundle-scoped entries are filtered when the +// evidence is projected onto conditions. It implements BundleDeprecationSource. +func (r *CatalogResolver) GetBundleDeprecations(ctx context.Context, ext *ocv1.ClusterExtension, _ ...ocv1.BundleMetadata) (*declcfg.Deprecation, error) { + return r.packageDeprecation(ctx, ext) +} + +// packageDeprecation reads the desired package's deprecation data from the +// selected catalog. A non-nil result with no entries means the package was +// found and has no deprecations; an error means no deprecation evidence +// could be established. +func (r *CatalogResolver) packageDeprecation(ctx context.Context, ext *ocv1.ClusterExtension) (*declcfg.Deprecation, error) { + if ext.Spec.Source.Catalog == nil { + return nil, fmt.Errorf("catalog deprecations require spec.source.catalog") + } + packageName := ext.Spec.Source.Catalog.PackageName + selector, err := desiredCatalogSelector(ext) + if err != nil { + return nil, err + } + cat, packageFBC, err := r.selectPackage(ctx, packageName, selector) + if err != nil { + return nil, err + } + if cat == nil { + return nil, fmt.Errorf("package %q not found in any eligible catalog", packageName) + } + if len(packageFBC.Deprecations) == 0 { + return &declcfg.Deprecation{Schema: declcfg.SchemaDeprecation, Package: packageName}, nil + } + return &packageFBC.Deprecations[0], nil +} + +// desiredCatalogSelector returns the catalog selector for the extension. +// An absent Catalog payload or empty selector selects every catalog. +func desiredCatalogSelector(ext *ocv1.ClusterExtension) (labels.Selector, error) { + selector := labels.Everything() + if ext.Spec.Source.Catalog == nil { + return selector, nil + } + selector, err := metav1.LabelSelectorAsSelector(ext.Spec.Source.Catalog.Selector) + if err != nil { + return nil, fmt.Errorf("desired catalog selector is invalid: %w", err) + } + // A nothing (empty) selector selects everything + if selector == labels.Nothing() { + selector = labels.Everything() + } + return selector, nil } type resolutionError struct { diff --git a/internal/operator-controller/resolve/catalog_test.go b/internal/operator-controller/resolve/catalog_test.go index 52c2663316..af0bd114e0 100644 --- a/internal/operator-controller/resolve/catalog_test.go +++ b/internal/operator-controller/resolve/catalog_test.go @@ -25,7 +25,7 @@ func TestInvalidClusterExtensionVersionRange(t *testing.T) { r := CatalogResolver{} pkgName := randPkg() ce := buildFooClusterExtension(pkgName, []string{}, "foobar", ocv1.UpgradeConstraintPolicyCatalogProvided) - _, _, _, err := r.Resolve(context.Background(), ce, nil) + _, _, err := r.Resolve(context.Background(), ce, nil) assert.EqualError(t, err, `desired version range "foobar" is invalid: improper constraint: "foobar"`) } @@ -35,7 +35,7 @@ func TestErrorWalkingCatalogs(t *testing.T) { }} pkgName := randPkg() ce := buildFooClusterExtension(pkgName, []string{}, "", ocv1.UpgradeConstraintPolicyCatalogProvided) - _, _, _, err := r.Resolve(context.Background(), ce, nil) + _, _, err := r.Resolve(context.Background(), ce, nil) assert.EqualError(t, err, "error listing catalogs: fake error") } @@ -58,10 +58,11 @@ func TestErrorGettingPackage(t *testing.T) { return w.GetPackage(ctx, cat, name) }} ce := buildFooClusterExtension(pkgName, []string{}, "", ocv1.UpgradeConstraintPolicyCatalogProvided) - _, _, deprecation, err := r.Resolve(context.Background(), ce, nil) + _, _, err := r.Resolve(context.Background(), ce, nil) require.EqualError(t, err, fmt.Sprintf(`error getting package %q from catalog "b": fake error`, pkgName)) assert.Equal(t, []string{"a", "b"}, consulted) - assert.Nil(t, deprecation) + _, deprecationErr := r.GetPackageDeprecation(context.Background(), ce) + require.ErrorContains(t, deprecationErr, `error getting package`) } func TestPackageDoesNotExist(t *testing.T) { @@ -79,7 +80,7 @@ func TestPackageDoesNotExist(t *testing.T) { r := CatalogResolver{ListCatalogsFunc: w.ListCatalogs, GetPackageFunc: w.GetPackage} pkgName := randPkg() ce := buildFooClusterExtension(pkgName, []string{}, "", ocv1.UpgradeConstraintPolicyCatalogProvided) - _, _, _, err := r.Resolve(context.Background(), ce, nil) + _, _, err := r.Resolve(context.Background(), ce, nil) assert.EqualError(t, err, fmt.Sprintf(`package %q not found in any eligible catalog`, pkgName)) } @@ -98,10 +99,12 @@ func TestPackageExists(t *testing.T) { } r := CatalogResolver{ListCatalogsFunc: w.ListCatalogs, GetPackageFunc: w.GetPackage} ce := buildFooClusterExtension(pkgName, []string{}, "", ocv1.UpgradeConstraintPolicyCatalogProvided) - gotBundle, gotVersion, gotDeprecation, err := r.Resolve(context.Background(), ce, nil) + gotBundle, gotVersion, err := r.Resolve(context.Background(), ce, nil) require.NoError(t, err) assert.Equal(t, genBundle(pkgName, "3.0.0"), *gotBundle) assert.Equal(t, declcfg.VersionRelease{Version: bsemver.MustParse("3.0.0")}, *gotVersion) + gotDeprecation, deprecationErr := r.GetPackageDeprecation(context.Background(), ce) + require.NoError(t, deprecationErr) assert.Equal(t, ptr.To(packageDeprecation(pkgName)), gotDeprecation) } @@ -132,9 +135,11 @@ func TestValidationFailed(t *testing.T) { }, } ce := buildFooClusterExtension(pkgName, []string{}, "", ocv1.UpgradeConstraintPolicyCatalogProvided) - _, _, deprecation, err := r.Resolve(context.Background(), ce, nil) + _, _, err := r.Resolve(context.Background(), ce, nil) require.ErrorContains(t, err, `from catalog "c": validating bundle`) assert.Equal(t, []string{"c"}, consulted) + deprecation, deprecationErr := r.GetPackageDeprecation(context.Background(), ce) + require.NoError(t, deprecationErr) assert.Equal(t, ptr.To(packageDeprecation(pkgName)), deprecation) } @@ -153,8 +158,10 @@ func TestVersionDoesNotExist(t *testing.T) { } r := CatalogResolver{ListCatalogsFunc: w.ListCatalogs, GetPackageFunc: w.GetPackage} ce := buildFooClusterExtension(pkgName, []string{}, "4.0.0", ocv1.UpgradeConstraintPolicyCatalogProvided) - _, _, deprecation, err := r.Resolve(context.Background(), ce, nil) + _, _, err := r.Resolve(context.Background(), ce, nil) require.EqualError(t, err, fmt.Sprintf(`resolving package %q from catalog "c": no bundles found for package %q matching version "4.0.0"`, pkgName, pkgName)) + deprecation, deprecationErr := r.GetPackageDeprecation(context.Background(), ce) + require.NoError(t, deprecationErr) assert.Equal(t, ptr.To(packageDeprecation(pkgName)), deprecation) } @@ -173,10 +180,12 @@ func TestVersionExists(t *testing.T) { } r := CatalogResolver{ListCatalogsFunc: w.ListCatalogs, GetPackageFunc: w.GetPackage} ce := buildFooClusterExtension(pkgName, []string{}, ">=1.0.0 <2.0.0", ocv1.UpgradeConstraintPolicyCatalogProvided) - gotBundle, gotVersion, gotDeprecation, err := r.Resolve(context.Background(), ce, nil) + gotBundle, gotVersion, err := r.Resolve(context.Background(), ce, nil) require.NoError(t, err) assert.Equal(t, genBundle(pkgName, "1.0.2"), *gotBundle) assert.Equal(t, declcfg.VersionRelease{Version: bsemver.MustParse("1.0.2")}, *gotVersion) + gotDeprecation, deprecationErr := r.GetPackageDeprecation(context.Background(), ce) + require.NoError(t, deprecationErr) assert.Equal(t, ptr.To(packageDeprecation(pkgName)), gotDeprecation) } @@ -195,7 +204,7 @@ func TestChannelDoesNotExist(t *testing.T) { } r := CatalogResolver{ListCatalogsFunc: w.ListCatalogs, GetPackageFunc: w.GetPackage} ce := buildFooClusterExtension(pkgName, []string{"stable"}, "", ocv1.UpgradeConstraintPolicyCatalogProvided) - _, _, _, err := r.Resolve(context.Background(), ce, nil) + _, _, err := r.Resolve(context.Background(), ce, nil) assert.EqualError(t, err, fmt.Sprintf(`resolving package %q from catalog "c": no bundles found for package %q in channels [stable]`, pkgName, pkgName)) } @@ -214,10 +223,12 @@ func TestChannelExists(t *testing.T) { } r := CatalogResolver{ListCatalogsFunc: w.ListCatalogs, GetPackageFunc: w.GetPackage} ce := buildFooClusterExtension(pkgName, []string{"beta"}, "", ocv1.UpgradeConstraintPolicyCatalogProvided) - gotBundle, gotVersion, gotDeprecation, err := r.Resolve(context.Background(), ce, nil) + gotBundle, gotVersion, err := r.Resolve(context.Background(), ce, nil) require.NoError(t, err) assert.Equal(t, genBundle(pkgName, "1.0.2"), *gotBundle) assert.Equal(t, declcfg.VersionRelease{Version: bsemver.MustParse("1.0.2")}, *gotVersion) + gotDeprecation, deprecationErr := r.GetPackageDeprecation(context.Background(), ce) + require.NoError(t, deprecationErr) assert.Equal(t, ptr.To(packageDeprecation(pkgName)), gotDeprecation) } @@ -236,7 +247,7 @@ func TestChannelExistsButNotVersion(t *testing.T) { } r := CatalogResolver{ListCatalogsFunc: w.ListCatalogs, GetPackageFunc: w.GetPackage} ce := buildFooClusterExtension(pkgName, []string{"beta"}, "3.0.0", ocv1.UpgradeConstraintPolicyCatalogProvided) - _, _, _, err := r.Resolve(context.Background(), ce, nil) + _, _, err := r.Resolve(context.Background(), ce, nil) assert.EqualError(t, err, fmt.Sprintf(`resolving package %q from catalog "c": no bundles found for package %q matching version "3.0.0" in channels [beta]`, pkgName, pkgName)) } @@ -255,7 +266,7 @@ func TestVersionExistsButNotChannel(t *testing.T) { } r := CatalogResolver{ListCatalogsFunc: w.ListCatalogs, GetPackageFunc: w.GetPackage} ce := buildFooClusterExtension(pkgName, []string{"stable"}, "1.0.0", ocv1.UpgradeConstraintPolicyCatalogProvided) - _, _, _, err := r.Resolve(context.Background(), ce, nil) + _, _, err := r.Resolve(context.Background(), ce, nil) assert.EqualError(t, err, fmt.Sprintf(`resolving package %q from catalog "c": no bundles found for package %q matching version "1.0.0" in channels [stable]`, pkgName, pkgName)) } @@ -274,10 +285,12 @@ func TestChannelAndVersionExist(t *testing.T) { } r := CatalogResolver{ListCatalogsFunc: w.ListCatalogs, GetPackageFunc: w.GetPackage} ce := buildFooClusterExtension(pkgName, []string{"alpha"}, "0.1.0", ocv1.UpgradeConstraintPolicyCatalogProvided) - gotBundle, gotVersion, gotDeprecation, err := r.Resolve(context.Background(), ce, nil) + gotBundle, gotVersion, err := r.Resolve(context.Background(), ce, nil) require.NoError(t, err) assert.Equal(t, genBundle(pkgName, "0.1.0"), *gotBundle) assert.Equal(t, declcfg.VersionRelease{Version: bsemver.MustParse("0.1.0")}, *gotVersion) + gotDeprecation, deprecationErr := r.GetPackageDeprecation(context.Background(), ce) + require.NoError(t, deprecationErr) assert.Equal(t, ptr.To(packageDeprecation(pkgName)), gotDeprecation) } @@ -296,10 +309,12 @@ func TestPreferNonDeprecated(t *testing.T) { } r := CatalogResolver{ListCatalogsFunc: w.ListCatalogs, GetPackageFunc: w.GetPackage} ce := buildFooClusterExtension(pkgName, []string{}, ">=0.1.0 <=1.0.0", ocv1.UpgradeConstraintPolicyCatalogProvided) - gotBundle, gotVersion, gotDeprecation, err := r.Resolve(context.Background(), ce, nil) + gotBundle, gotVersion, err := r.Resolve(context.Background(), ce, nil) require.NoError(t, err) assert.Equal(t, genBundle(pkgName, "0.1.0"), *gotBundle) assert.Equal(t, declcfg.VersionRelease{Version: bsemver.MustParse("0.1.0")}, *gotVersion) + gotDeprecation, deprecationErr := r.GetPackageDeprecation(context.Background(), ce) + require.NoError(t, deprecationErr) assert.Equal(t, ptr.To(packageDeprecation(pkgName)), gotDeprecation) } @@ -318,10 +333,12 @@ func TestAcceptDeprecated(t *testing.T) { } r := CatalogResolver{ListCatalogsFunc: w.ListCatalogs, GetPackageFunc: w.GetPackage} ce := buildFooClusterExtension(pkgName, []string{}, ">=1.0.0 <=1.0.1", ocv1.UpgradeConstraintPolicyCatalogProvided) - gotBundle, gotVersion, gotDeprecation, err := r.Resolve(context.Background(), ce, nil) + gotBundle, gotVersion, err := r.Resolve(context.Background(), ce, nil) require.NoError(t, err) assert.Equal(t, genBundle(pkgName, "1.0.1"), *gotBundle) assert.Equal(t, declcfg.VersionRelease{Version: bsemver.MustParse("1.0.1")}, *gotVersion) + gotDeprecation, deprecationErr := r.GetPackageDeprecation(context.Background(), ce) + require.NoError(t, deprecationErr) assert.Equal(t, ptr.To(packageDeprecation(pkgName)), gotDeprecation) } @@ -401,21 +418,24 @@ func TestPackageVariationsBetweenCatalogs(t *testing.T) { t.Run("same-priority packages are ambiguous despite non-deprecated candidates", func(t *testing.T) { ce := buildFooClusterExtension(pkgName, []string{}, ">=1.0.0 <=1.0.3", ocv1.UpgradeConstraintPolicyCatalogProvided) - gotBundle, gotVersion, gotDeprecation, err := r.Resolve(context.Background(), ce, nil) + gotBundle, gotVersion, err := r.Resolve(context.Background(), ce, nil) require.ErrorContains(t, err, "in multiple catalogs with the same priority 0: [b c d e f]") assert.Nil(t, gotBundle) assert.Nil(t, gotVersion) - assert.Nil(t, gotDeprecation) + _, deprecationErr := r.GetPackageDeprecation(context.Background(), ce) + require.ErrorContains(t, deprecationErr, "in multiple catalogs with the same priority 0") }) t.Run("highest semver bundle is chosen when candidates are all from the same catalog", func(t *testing.T) { ce := buildFooClusterExtension(pkgName, []string{}, ">=1.0.4 <=1.0.5", ocv1.UpgradeConstraintPolicyCatalogProvided) ce.Spec.Source.Catalog.Selector = &metav1.LabelSelector{MatchLabels: map[string]string{"olm.operatorframework.io/metadata.name": "f"}} - gotBundle, gotVersion, gotDeprecation, err := r.Resolve(context.Background(), ce, nil) + gotBundle, gotVersion, err := r.Resolve(context.Background(), ce, nil) require.NoError(t, err) // Bundles within one catalog for a package will be sorted by semver and deprecation and the best is returned assert.Equal(t, genBundle(pkgName, "1.0.5").Name, gotBundle.Name) assert.Equal(t, declcfg.VersionRelease{Version: bsemver.MustParse("1.0.5")}, *gotVersion) + gotDeprecation, deprecationErr := r.GetPackageDeprecation(context.Background(), ce) + require.NoError(t, deprecationErr) assert.Equal(t, &declcfg.Deprecation{Schema: declcfg.SchemaDeprecation, Package: pkgName}, gotDeprecation) }) } @@ -440,10 +460,12 @@ func TestUpgradeFoundLegacy(t *testing.T) { Version: "0.1.0", } // 0.1.0 => 1.0.2 would not be allowed using semver semantics - gotBundle, gotVersion, gotDeprecation, err := r.Resolve(context.Background(), ce, installedBundle) + gotBundle, gotVersion, err := r.Resolve(context.Background(), ce, installedBundle) require.NoError(t, err) assert.Equal(t, genBundle(pkgName, "1.0.2"), *gotBundle) assert.Equal(t, declcfg.VersionRelease{Version: bsemver.MustParse("1.0.2")}, *gotVersion) + gotDeprecation, deprecationErr := r.GetPackageDeprecation(context.Background(), ce) + require.NoError(t, deprecationErr) assert.Equal(t, ptr.To(packageDeprecation(pkgName)), gotDeprecation) } @@ -467,7 +489,7 @@ func TestUpgradeNotFoundLegacy(t *testing.T) { Version: "0.1.0", } // 0.1.0 only upgrades to 1.0.x with its legacy upgrade edges, so this fails. - _, _, _, err := r.Resolve(context.Background(), ce, installedBundle) + _, _, err := r.Resolve(context.Background(), ce, installedBundle) assert.EqualError(t, err, fmt.Sprintf(`resolving package %q from catalog "c": error upgrading from currently installed version "0.1.0": no bundles found for package %q matching version "<1.0.0 >=2.0.0"`, pkgName, pkgName)) } @@ -492,10 +514,12 @@ func TestDowngradeFound(t *testing.T) { } // 1.0.2 => 0.1.0 is a downgrade, but it is allowed because of the upgrade constraint policy. // note: we chose 0.1.0 because 1.0.0 and 1.0.1 are deprecated. - gotBundle, gotVersion, gotDeprecation, err := r.Resolve(context.Background(), ce, installedBundle) + gotBundle, gotVersion, err := r.Resolve(context.Background(), ce, installedBundle) require.NoError(t, err) assert.Equal(t, genBundle(pkgName, "0.1.0"), *gotBundle) assert.Equal(t, declcfg.VersionRelease{Version: bsemver.MustParse("0.1.0")}, *gotVersion) + gotDeprecation, deprecationErr := r.GetPackageDeprecation(context.Background(), ce) + require.NoError(t, deprecationErr) assert.Equal(t, ptr.To(packageDeprecation(pkgName)), gotDeprecation) } @@ -519,7 +543,7 @@ func TestDowngradeNotFound(t *testing.T) { Version: "1.0.2", } // Downgrades are allowed via the upgrade constraint policy, but there is no bundle in the specified range. - _, _, _, err := r.Resolve(context.Background(), ce, installedBundle) + _, _, err := r.Resolve(context.Background(), ce, installedBundle) assert.EqualError(t, err, fmt.Sprintf(`resolving package %q from catalog "c": error upgrading from currently installed version "1.0.2": no bundles found for package %q matching version ">0.1.0 <1.0.0"`, pkgName, pkgName)) } @@ -667,7 +691,7 @@ func TestInvalidClusterExtensionCatalogMatchExpressions(t *testing.T) { }, }, } - _, _, _, err := r.Resolve(context.Background(), ce, nil) + _, _, err := r.Resolve(context.Background(), ce, nil) assert.EqualError(t, err, "desired catalog selector is invalid: \"bad\" is not a valid label selector operator") } @@ -693,7 +717,7 @@ func TestInvalidClusterExtensionCatalogMatchLabelsName(t *testing.T) { }, }, } - _, _, _, err := r.Resolve(context.Background(), ce, nil) + _, _, err := r.Resolve(context.Background(), ce, nil) assert.ErrorContains(t, err, "desired catalog selector is invalid: key: Invalid value:") } @@ -719,7 +743,7 @@ func TestInvalidClusterExtensionCatalogMatchLabelsValue(t *testing.T) { }, }, } - _, _, _, err := r.Resolve(context.Background(), ce, nil) + _, _, err := r.Resolve(context.Background(), ce, nil) assert.ErrorContains(t, err, "desired catalog selector is invalid: values[0][name]: Invalid value:") } @@ -739,7 +763,7 @@ func TestClusterExtensionMatchLabel(t *testing.T) { MatchLabels: map[string]string{"olm.operatorframework.io/metadata.name": "b"}, } - _, _, _, err := r.Resolve(context.Background(), ce, nil) + _, _, err := r.Resolve(context.Background(), ce, nil) require.NoError(t, err) } @@ -759,7 +783,7 @@ func TestClusterExtensionNoMatchLabel(t *testing.T) { MatchLabels: map[string]string{"olm.operatorframework.io/metadata.name": "a"}, } - _, _, _, err := r.Resolve(context.Background(), ce, nil) + _, _, err := r.Resolve(context.Background(), ce, nil) require.Error(t, err) require.ErrorContains(t, err, fmt.Sprintf("package %q not found in any eligible catalog", pkgName)) } @@ -803,18 +827,22 @@ func TestUnequalPriority(t *testing.T) { }} ce := buildFooClusterExtension(pkgName, []string{}, "", ocv1.UpgradeConstraintPolicyCatalogProvided) - _, gotVersion, deprecation, err := r.Resolve(context.Background(), ce, nil) + _, gotVersion, err := r.Resolve(context.Background(), ce, nil) require.NoError(t, err) require.Equal(t, declcfg.VersionRelease{Version: bsemver.MustParse("1.0.0")}, *gotVersion) assert.Equal(t, []string{"a"}, consulted) + deprecation, deprecationErr := r.GetPackageDeprecation(context.Background(), ce) + require.NoError(t, deprecationErr) assert.Equal(t, &declcfg.Deprecation{Schema: declcfg.SchemaDeprecation, Package: pkgName}, deprecation) // The lower-priority catalog has 1.1.0, but the selected catalog cannot satisfy it. consulted = nil ce.Spec.Source.Catalog.Version = "1.1.0" - _, _, deprecation, err = r.Resolve(context.Background(), ce, nil) + _, _, err = r.Resolve(context.Background(), ce, nil) require.ErrorContains(t, err, `from catalog "a": no bundles found`) assert.Equal(t, []string{"a"}, consulted) + deprecation, deprecationErr = r.GetPackageDeprecation(context.Background(), ce) + require.NoError(t, deprecationErr) assert.Equal(t, &declcfg.Deprecation{Schema: declcfg.SchemaDeprecation, Package: pkgName}, deprecation) // Only a genuinely absent package permits a lower-priority catalog. @@ -822,7 +850,7 @@ func TestUnequalPriority(t *testing.T) { return &declcfg.DeclarativeConfig{}, &ocv1.ClusterCatalogSpec{Priority: 1}, nil } consulted = nil - _, gotVersion, _, err = r.Resolve(context.Background(), ce, nil) + _, gotVersion, err = r.Resolve(context.Background(), ce, nil) require.NoError(t, err) assert.Equal(t, declcfg.VersionRelease{Version: bsemver.MustParse("1.1.0")}, *gotVersion) assert.Equal(t, []string{"a", "b"}, consulted) @@ -832,9 +860,11 @@ func TestUnequalPriority(t *testing.T) { return &declcfg.DeclarativeConfig{Channels: []declcfg.Channel{{Package: pkgName, Name: "alpha"}}}, &ocv1.ClusterCatalogSpec{Priority: 1}, nil } consulted = nil - _, _, deprecation, err = r.Resolve(context.Background(), ce, nil) + _, _, err = r.Resolve(context.Background(), ce, nil) require.ErrorContains(t, err, `from catalog "a": no bundles found`) assert.Equal(t, []string{"a"}, consulted) + deprecation, deprecationErr = r.GetPackageDeprecation(context.Background(), ce) + require.NoError(t, deprecationErr) assert.Equal(t, &declcfg.Deprecation{Schema: declcfg.SchemaDeprecation, Package: pkgName}, deprecation) } @@ -855,12 +885,13 @@ func TestMultiplePriority(t *testing.T) { // "a" has an eligible bundle; "c" has no bundles but still creates an ambiguity. ce := buildFooClusterExtension(pkgName, nil, "3.0.0", ocv1.UpgradeConstraintPolicyCatalogProvided) - gotBundle, gotVersion, gotDeprecation, err := r.Resolve(context.Background(), ce, nil) + gotBundle, gotVersion, err := r.Resolve(context.Background(), ce, nil) require.Error(t, err) require.ErrorContains(t, err, "in multiple catalogs with the same priority 1: [a c]") assert.Nil(t, gotBundle) assert.Nil(t, gotVersion) - assert.Nil(t, gotDeprecation) + _, deprecationErr := r.GetPackageDeprecation(context.Background(), ce) + require.ErrorContains(t, deprecationErr, "in multiple catalogs with the same priority 1") } func priorityCatalog(name string, priority int32) ocv1.ClusterCatalog { @@ -902,10 +933,12 @@ func TestMultipleChannels(t *testing.T) { } r := CatalogResolver{ListCatalogsFunc: w.ListCatalogs, GetPackageFunc: w.GetPackage} ce := buildFooClusterExtension(pkgName, []string{"beta", "alpha"}, "", ocv1.UpgradeConstraintPolicyCatalogProvided) - gotBundle, gotVersion, gotDeprecation, err := r.Resolve(context.Background(), ce, nil) + gotBundle, gotVersion, err := r.Resolve(context.Background(), ce, nil) require.NoError(t, err) assert.Equal(t, genBundle(pkgName, "2.0.0"), *gotBundle) assert.Equal(t, declcfg.VersionRelease{Version: bsemver.MustParse("2.0.0")}, *gotVersion) + gotDeprecation, deprecationErr := r.GetPackageDeprecation(context.Background(), ce) + require.NoError(t, deprecationErr) assert.Equal(t, ptr.To(packageDeprecation(pkgName)), gotDeprecation) } @@ -935,7 +968,7 @@ func TestAllCatalogsDisabled(t *testing.T) { } ce := buildFooClusterExtension(pkgName, []string{}, ">=1.0.0", ocv1.UpgradeConstraintPolicyCatalogProvided) - _, _, _, err := r.Resolve(context.Background(), ce, nil) + _, _, err := r.Resolve(context.Background(), ce, nil) require.Error(t, err) assert.Contains(t, err.Error(), "not found in any eligible catalog") } @@ -976,9 +1009,57 @@ func TestSomeCatalogsDisabled(t *testing.T) { } ce := buildFooClusterExtension(pkgName, []string{}, ">=1.0.0", ocv1.UpgradeConstraintPolicyCatalogProvided) - gotBundle, gotVersion, _, err := r.Resolve(context.Background(), ce, nil) + gotBundle, gotVersion, err := r.Resolve(context.Background(), ce, nil) require.NoError(t, err) require.NotNil(t, gotBundle) require.Equal(t, declcfg.VersionRelease{Version: bsemver.MustParse("3.0.0")}, *gotVersion) require.Equal(t, []string{"enabledCatalog"}, consulted) } + +// TestDeprecationSources covers the optional deprecation source methods +// implemented by CatalogResolver: each returns the selected package's +// deprecation data, and a package that cannot be found means no evidence +// can be established. +func TestDeprecationSources(t *testing.T) { + pkgName := randPkg() + w := staticCatalogs{ + "a": func() (*declcfg.DeclarativeConfig, *ocv1.ClusterCatalogSpec, error) { + return &declcfg.DeclarativeConfig{}, nil, nil + }, + "b": func() (*declcfg.DeclarativeConfig, *ocv1.ClusterCatalogSpec, error) { + return &declcfg.DeclarativeConfig{}, nil, nil + }, + "c": func() (*declcfg.DeclarativeConfig, *ocv1.ClusterCatalogSpec, error) { + return genPackage(pkgName), nil, nil + }, + } + r := CatalogResolver{ListCatalogsFunc: w.ListCatalogs, GetPackageFunc: w.GetPackage} + ce := buildFooClusterExtension(pkgName, []string{}, "", ocv1.UpgradeConstraintPolicyCatalogProvided) + expected := ptr.To(packageDeprecation(pkgName)) + + t.Run("package", func(t *testing.T) { + got, err := r.GetPackageDeprecation(context.Background(), ce) + require.NoError(t, err) + assert.Equal(t, expected, got) + }) + t.Run("channels", func(t *testing.T) { + got, err := r.GetChannelDeprecations(context.Background(), ce) + require.NoError(t, err) + assert.Equal(t, expected, got) + }) + t.Run("bundles", func(t *testing.T) { + got, err := r.GetBundleDeprecations(context.Background(), ce, ocv1.BundleMetadata{Name: bundleName(pkgName, "1.0.0")}) + require.NoError(t, err) + assert.Equal(t, expected, got) + }) + t.Run("missing package means unknown evidence", func(t *testing.T) { + empty := staticCatalogs{ + "a": func() (*declcfg.DeclarativeConfig, *ocv1.ClusterCatalogSpec, error) { + return &declcfg.DeclarativeConfig{}, nil, nil + }, + } + missingResolver := CatalogResolver{ListCatalogsFunc: empty.ListCatalogs, GetPackageFunc: empty.GetPackage} + _, err := missingResolver.GetPackageDeprecation(context.Background(), ce) + require.ErrorContains(t, err, `not found in any eligible catalog`) + }) +} diff --git a/internal/operator-controller/resolve/resolver.go b/internal/operator-controller/resolve/resolver.go index ef7543b5c8..030d71b2fe 100644 --- a/internal/operator-controller/resolve/resolver.go +++ b/internal/operator-controller/resolve/resolver.go @@ -8,12 +8,44 @@ import ( ocv1 "github.com/operator-framework/operator-controller/api/v1" ) +// Resolver selects a bundle from a source. Callers own engagement policy: +// Resolve is called only when the source's preconditions hold (for example, +// no rollout is in progress), and recovery from returned errors happens in +// the caller. Implementations must not build their own fallbacks. type Resolver interface { - Resolve(ctx context.Context, ext *ocv1.ClusterExtension, installedBundle *ocv1.BundleMetadata) (*declcfg.Bundle, *declcfg.VersionRelease, *declcfg.Deprecation, error) + Resolve(ctx context.Context, ext *ocv1.ClusterExtension, installedBundle *ocv1.BundleMetadata) (*declcfg.Bundle, *declcfg.VersionRelease, error) } -type Func func(ctx context.Context, ext *ocv1.ClusterExtension, installedBundle *ocv1.BundleMetadata) (*declcfg.Bundle, *declcfg.VersionRelease, *declcfg.Deprecation, error) +type Func func(ctx context.Context, ext *ocv1.ClusterExtension, installedBundle *ocv1.BundleMetadata) (*declcfg.Bundle, *declcfg.VersionRelease, error) -func (f Func) Resolve(ctx context.Context, ext *ocv1.ClusterExtension, installedBundle *ocv1.BundleMetadata) (*declcfg.Bundle, *declcfg.VersionRelease, *declcfg.Deprecation, error) { +func (f Func) Resolve(ctx context.Context, ext *ocv1.ClusterExtension, installedBundle *ocv1.BundleMetadata) (*declcfg.Bundle, *declcfg.VersionRelease, error) { return f(ctx, ext, installedBundle) } + +// PackageDeprecationSource is an optional capability: deprecation evidence +// for the extension's desired package. Each source extracts the values it +// needs from the extension itself, so shared reconciliation never has to +// interpret source-specific fields. +// +// A source that cannot provide deprecation data simply does not implement +// these interfaces: consumers must use type assertions and report the +// absence of the capability exactly like an evaluation that found no +// deprecations. The interfaces carry evidence only; consumers must never use +// them to decide control flow such as source selection, fallback, or +// maintenance. +type PackageDeprecationSource interface { + GetPackageDeprecation(ctx context.Context, ext *ocv1.ClusterExtension) (*declcfg.Deprecation, error) +} + +// ChannelDeprecationSource is an optional capability: deprecation evidence +// for the extension's desired channels. +type ChannelDeprecationSource interface { + GetChannelDeprecations(ctx context.Context, ext *ocv1.ClusterExtension) (*declcfg.Deprecation, error) +} + +// BundleDeprecationSource is an optional capability: deprecation evidence +// for the given bundles (for example, the installed bundle and any bundles +// in a rollout in progress). +type BundleDeprecationSource interface { + GetBundleDeprecations(ctx context.Context, ext *ocv1.ClusterExtension, bundles ...ocv1.BundleMetadata) (*declcfg.Deprecation, error) +} From 4d9209fa4f4b1eceed86046927cbb6c80ef692c8 Mon Sep 17 00:00:00 2001 From: grokspawn Date: Fri, 6 Nov 2026 16:45:41 -0600 Subject: [PATCH 3/3] Merge upstream main; migrate PR-owned files to operator-registry v2 Upstream #3080 moved operator-registry to the /v2 module path after this branch's base, so PR CI (which builds a merge with current main) could not compile the PR-owned files that still imported the v1 path. This merges current main (77754d81) and migrates catalog_source.go and clusterextension_source_test.go to github.com/operator-framework/operator-registry/v2/alpha/declcfg. Signed-off-by: grokspawn --- internal/operator-controller/controllers/catalog_source.go | 2 +- .../controllers/clusterextension_source_test.go | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/internal/operator-controller/controllers/catalog_source.go b/internal/operator-controller/controllers/catalog_source.go index d4ce65488e..e0886b8d67 100644 --- a/internal/operator-controller/controllers/catalog_source.go +++ b/internal/operator-controller/controllers/catalog_source.go @@ -25,7 +25,7 @@ import ( "sigs.k8s.io/controller-runtime/pkg/client" "sigs.k8s.io/controller-runtime/pkg/log" - "github.com/operator-framework/operator-registry/alpha/declcfg" + "github.com/operator-framework/operator-registry/v2/alpha/declcfg" ocv1 "github.com/operator-framework/operator-controller/api/v1" "github.com/operator-framework/operator-controller/internal/operator-controller/bundleutil" diff --git a/internal/operator-controller/controllers/clusterextension_source_test.go b/internal/operator-controller/controllers/clusterextension_source_test.go index 15764eef35..d9c2def219 100644 --- a/internal/operator-controller/controllers/clusterextension_source_test.go +++ b/internal/operator-controller/controllers/clusterextension_source_test.go @@ -17,7 +17,7 @@ import ( "sigs.k8s.io/controller-runtime/pkg/event" "sigs.k8s.io/controller-runtime/pkg/reconcile" - "github.com/operator-framework/operator-registry/alpha/declcfg" + "github.com/operator-framework/operator-registry/v2/alpha/declcfg" ocv1 "github.com/operator-framework/operator-controller/api/v1" "github.com/operator-framework/operator-controller/internal/operator-controller/resolve"