Read spec.release field from the olm.package property for proper version ordering and comparison

Signed-off-by: Rashmi Gottipati <rgottipa@redhat.com>

Author: rashmigottipati

internal/operator-controller/catalogmetadata/compare/compare.go

@@ -9,7 +9,6 @@ import (
 
 	"github.com/operator-framework/operator-registry/alpha/declcfg"
 
-	"github.com/operator-framework/operator-controller/internal/operator-controller/bundleutil"
 	slicesutil "github.com/operator-framework/operator-controller/internal/shared/util/slices"
 )
 
@@ -42,16 +41,16 @@ func newMastermindsRange(versionRange string) (bsemver.Range, error) {
 // version and release. Bundles with lower versions/releases are
 // considered less than bundles with higher versions/releases.
 func ByVersionAndRelease(b1, b2 declcfg.Bundle) int {
-	vr1, err1 := bundleutil.GetVersionAndRelease(b1)
-	vr2, err2 := bundleutil.GetVersionAndRelease(b2)
+	cv1, err1 := b1.CompositeVersion()
+	cv2, err2 := b2.CompositeVersion()
 
 	// We don't really expect errors, because we expect well-formed/validated
 	// FBC as input. However, just in case we'll check the errors and sort
 	// invalid bundles as "lower" than valid bundles.
 	if err1 != nil || err2 != nil {
 		return compareErrors(err2, err1)
 	}
-	return vr2.Compare(*vr1)
+	return cv2.Compare(cv1)
 }
 
 func ByDeprecationFunc(deprecation declcfg.Deprecation) func(a, b declcfg.Bundle) int {

internal/operator-controller/catalogmetadata/filter/bundle_predicates.go

@@ -5,21 +5,19 @@ import (
 
 	"github.com/operator-framework/operator-registry/alpha/declcfg"
 
-	"github.com/operator-framework/operator-controller/internal/operator-controller/bundle"
-	"github.com/operator-framework/operator-controller/internal/operator-controller/bundleutil"
 	"github.com/operator-framework/operator-controller/internal/shared/util/filter"
 )
 
 // ExactVersionRelease returns a predicate that matches bundles with an exact
 // version and release match. Both the semver version and the release must match
 // exactly for the predicate to return true.
-func ExactVersionRelease(expect bundle.VersionRelease) filter.Predicate[declcfg.Bundle] {
+func ExactVersionRelease(expect declcfg.CompositeVersion) filter.Predicate[declcfg.Bundle] {
 	return func(b declcfg.Bundle) bool {
-		actual, err := bundleutil.GetVersionAndRelease(b)
+		actual, err := b.CompositeVersion()
 		if err != nil {
 			return false
 		}
-		return expect.Compare(*actual) == 0
+		return expect.Compare(actual) == 0
 	}
 }
 
@@ -28,11 +26,11 @@ func ExactVersionRelease(expect bundle.VersionRelease) filter.Predicate[declcfg.
 // ignoring the release metadata.
 func InSemverRange(versionRange bsemver.Range) filter.Predicate[declcfg.Bundle] {
 	return func(b declcfg.Bundle) bool {
-		vr, err := bundleutil.GetVersionAndRelease(b)
+		cv, err := b.CompositeVersion()
 		if err != nil {
 			return false
 		}
-		return versionRange(vr.Version)
+		return versionRange(cv.Version)
 	}
 }
 
@@ -50,11 +48,11 @@ func InAnyChannel(channels ...declcfg.Channel) filter.Predicate[declcfg.Bundle]
 }
 
 // SameVersionHigherRelease returns a predicate that matches bundles with the same
-// semantic version as the provided version-release, but with a higher release value.
+// semantic version as the provided composite version, but with a higher release value.
 // This is used to identify re-released bundles (e.g., 2.0.0+2 when 2.0.0+1 is installed).
-func SameVersionHigherRelease(expect bundle.VersionRelease) filter.Predicate[declcfg.Bundle] {
+func SameVersionHigherRelease(expect declcfg.CompositeVersion) filter.Predicate[declcfg.Bundle] {
 	return func(b declcfg.Bundle) bool {
-		actual, err := bundleutil.GetVersionAndRelease(b)
+		actual, err := b.CompositeVersion()
 		if err != nil {
 			return false
 		}
@@ -63,6 +61,7 @@ func SameVersionHigherRelease(expect bundle.VersionRelease) filter.Predicate[dec
 			return false
 		}
 
-		return expect.Release.Compare(actual.Release) < 0
+		// Use CompositeVersion.Compare which handles release comparison correctly
+		return expect.Compare(actual) < 0
 	}
 }

internal/operator-controller/catalogmetadata/filter/successors.go

@@ -8,43 +8,39 @@ import (
 	"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/bundle"
-	"github.com/operator-framework/operator-controller/internal/operator-controller/features"
 	"github.com/operator-framework/operator-controller/internal/shared/util/filter"
 )
 
-func SuccessorsOf(installedBundle ocv1.BundleMetadata, channels ...declcfg.Channel) (filter.Predicate[declcfg.Bundle], error) {
-	// TODO: We do not have an explicit field in our BundleMetadata for a bundle's release value.
-	//    Legacy registry+v1 bundles embed the release value inside their versions as build metadata
-	//    (in violation of the semver spec). If/when we add explicit release metadata to bundles and/or
-	//    we support a new bundle format, we need to revisit the assumption that all bundles are
-	//    registry+v1 and embed release in build metadata.
-	installedVersionRelease, err := bundle.NewLegacyRegistryV1VersionRelease(installedBundle.Version)
-	if err != nil {
-		return nil, fmt.Errorf("failed to get version and release of installed bundle: %v", err)
+func SuccessorsOf(installedBundle ocv1.BundleMetadata, allBundles []declcfg.Bundle, channels ...declcfg.Channel) (filter.Predicate[declcfg.Bundle], error) {
+	// Find the installed bundle in the catalog to get its CompositeVersion (with explicit Release field)
+	var installedCompositeVersion *declcfg.CompositeVersion
+	for _, b := range allBundles {
+		if b.Name == installedBundle.Name {
+			cv, err := b.CompositeVersion()
+			if err != nil {
+				return nil, fmt.Errorf("failed to get composite version for installed bundle %q: %v", installedBundle.Name, err)
+			}
+			installedCompositeVersion = cv
+			break
+		}
+	}
+
+	// If installed bundle not found in catalog, we can't determine successors based on release
+	if installedCompositeVersion == nil {
+		// Fall back to legacy successor logic only (no release awareness)
+		return legacySuccessor(installedBundle, channels...)
 	}
 
 	successorsPredicate, err := legacySuccessor(installedBundle, channels...)
 	if err != nil {
 		return nil, fmt.Errorf("getting successorsPredicate: %w", err)
 	}
 
-	// Build the final predicate based on feature gate status
-	predicates := []filter.Predicate[declcfg.Bundle]{
+	// Bundle matches if it's a channel successor or current version (no upgrade)
+	return filter.Or(
 		successorsPredicate,
-		ExactVersionRelease(*installedVersionRelease),
-	}
-
-	// If release version priority is enabled, also consider bundles
-	// with the same semantic version but higher release as valid successors.
-	// This allows upgrades to re-released bundles (e.g., 2.0.0+1 -> 2.0.0+2).
-	if features.OperatorControllerFeatureGate.Enabled(features.ReleaseVersionPriority) {
-		predicates = append(predicates, SameVersionHigherRelease(*installedVersionRelease))
-	}
-
-	// Bundle matches if it's a channel successor, current version (no upgrade),
-	// or (when feature gate enabled) same version with higher release
-	return filter.Or(predicates...), nil
+		ExactVersionRelease(*installedCompositeVersion),
+	), nil
 }
 
 func legacySuccessor(installedBundle ocv1.BundleMetadata, channels ...declcfg.Channel) (filter.Predicate[declcfg.Bundle], error) {

internal/operator-controller/controllers/clusterextension_reconcile_steps.go

@@ -189,11 +189,7 @@ func ResolveBundle(r resolve.Resolver, c client.Client) ReconcileStepFunc {
 		state.resolvedRevisionMetadata = &RevisionMetadata{
 			Package: resolvedBundle.Package,
 			Image:   resolvedBundle.Image,
-			// TODO: Right now, operator-controller only supports registry+v1 bundles and has no concept
-			//   of a "release" field. If/when we add a release field concept or a new bundle format
-			//   we need to re-evaluate use of `AsLegacyRegistryV1Version` so that we avoid propagating
-			//   registry+v1's semver spec violations of treating build metadata as orderable.
-			BundleMetadata: bundleutil.MetadataFor(resolvedBundle.Name, resolvedBundleVersion.AsLegacyRegistryV1Version()),
+			BundleMetadata: bundleutil.MetadataFor(resolvedBundle.Name, resolvedBundleVersion.Version),
 		}
 		return nil, nil
 	}

internal/operator-controller/resolve/catalog.go

@@ -17,8 +17,6 @@ import (
 	"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/bundle"
-	"github.com/operator-framework/operator-controller/internal/operator-controller/bundleutil"
 	"github.com/operator-framework/operator-controller/internal/operator-controller/catalogmetadata/compare"
 	"github.com/operator-framework/operator-controller/internal/operator-controller/catalogmetadata/filter"
 	filterutil "github.com/operator-framework/operator-controller/internal/shared/util/filter"
@@ -39,7 +37,7 @@ type foundBundle struct {
 }
 
 // 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, *bundle.VersionRelease, *declcfg.Deprecation, error) {
+func (r *CatalogResolver) Resolve(ctx context.Context, ext *ocv1.ClusterExtension, installedBundle *ocv1.BundleMetadata) (*declcfg.Bundle, *declcfg.CompositeVersion, *declcfg.Deprecation, error) {
 	l := log.FromContext(ctx)
 	packageName := ext.Spec.Source.Catalog.PackageName
 	versionRange := ext.Spec.Source.Catalog.Version
@@ -111,7 +109,7 @@ func (r *CatalogResolver) Resolve(ctx context.Context, ext *ocv1.ClusterExtensio
 		}
 
 		if ext.Spec.Source.Catalog.UpgradeConstraintPolicy != ocv1.UpgradeConstraintPolicySelfCertified && installedBundle != nil {
-			successorPredicate, err := filter.SuccessorsOf(*installedBundle, packageFBC.Channels...)
+			successorPredicate, err := filter.SuccessorsOf(*installedBundle, packageFBC.Bundles, packageFBC.Channels...)
 			if err != nil {
 				return fmt.Errorf("error finding upgrade edges: %w", err)
 			}
@@ -190,7 +188,7 @@ func (r *CatalogResolver) Resolve(ctx context.Context, ext *ocv1.ClusterExtensio
 		}
 	}
 	resolvedBundle := resolvedBundles[0].bundle
-	resolvedBundleVersion, err := bundleutil.GetVersionAndRelease(*resolvedBundle)
+	resolvedBundleVersion, err := resolvedBundle.CompositeVersion()
 	if err != nil {
 		return nil, nil, nil, fmt.Errorf("error getting resolved bundle version for bundle %q: %w", resolvedBundle.Name, err)
 	}

internal/operator-controller/resolve/resolver.go

@@ -6,15 +6,14 @@ import (
 	"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/bundle"
 )
 
 type Resolver interface {
-	Resolve(ctx context.Context, ext *ocv1.ClusterExtension, installedBundle *ocv1.BundleMetadata) (*declcfg.Bundle, *bundle.VersionRelease, *declcfg.Deprecation, error)
+	Resolve(ctx context.Context, ext *ocv1.ClusterExtension, installedBundle *ocv1.BundleMetadata) (*declcfg.Bundle, *declcfg.CompositeVersion, *declcfg.Deprecation, error)
 }
 
-type Func func(ctx context.Context, ext *ocv1.ClusterExtension, installedBundle *ocv1.BundleMetadata) (*declcfg.Bundle, *bundle.VersionRelease, *declcfg.Deprecation, error)
+type Func func(ctx context.Context, ext *ocv1.ClusterExtension, installedBundle *ocv1.BundleMetadata) (*declcfg.Bundle, *declcfg.CompositeVersion, *declcfg.Deprecation, error)
 
-func (f Func) Resolve(ctx context.Context, ext *ocv1.ClusterExtension, installedBundle *ocv1.BundleMetadata) (*declcfg.Bundle, *bundle.VersionRelease, *declcfg.Deprecation, error) {
+func (f Func) Resolve(ctx context.Context, ext *ocv1.ClusterExtension, installedBundle *ocv1.BundleMetadata) (*declcfg.Bundle, *declcfg.CompositeVersion, *declcfg.Deprecation, error) {
 	return f(ctx, ext, installedBundle)
 }