Support explicit pkg.Release field with build metadata fallback

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

Author: rashmigottipati

internal/operator-controller/bundleutil/bundle.go

@@ -16,25 +16,42 @@ import (
 func GetVersionAndRelease(b declcfg.Bundle) (*bundle.VersionRelease, error) {
 	for _, p := range b.Properties {
 		if p.Type == property.TypePackage {
-			var pkg property.Package
-			if err := json.Unmarshal(p.Value, &pkg); err != nil {
-				return nil, fmt.Errorf("error unmarshalling package property: %w", err)
-			}
-
-			// TODO: For now, we assume that all bundles are registry+v1 bundles.
-			//   In the future, when we support other bundle formats, we should stop
-			//   using the legacy mechanism (i.e. using build metadata in the version)
-			//   to determine the bundle's release.
-			vr, err := bundle.NewLegacyRegistryV1VersionRelease(pkg.Version)
-			if err != nil {
-				return nil, err
-			}
-			return vr, nil
+			return parseVersionRelease(p.Value)
 		}
 	}
 	return nil, fmt.Errorf("no package property found in bundle %q", b.Name)
 }
 
+func parseVersionRelease(pkgData json.RawMessage) (*bundle.VersionRelease, error) {
+	var pkg property.Package
+	if err := json.Unmarshal(pkgData, &pkg); err != nil {
+		return nil, fmt.Errorf("error unmarshalling package property: %w", err)
+	}
+
+	// If the bundle has an explicit release field, use it
+	if pkg.Release != "" {
+		vers, err := bsemver.Parse(pkg.Version)
+		if err != nil {
+			return nil, fmt.Errorf("error parsing version %q: %w", pkg.Version, err)
+		}
+		rel, err := bundle.NewRelease(pkg.Release)
+		if err != nil {
+			return nil, fmt.Errorf("error parsing release %q: %w", pkg.Release, err)
+		}
+		return &bundle.VersionRelease{
+			Version: vers,
+			Release: rel,
+		}, nil
+	}
+
+	// Fall back to legacy registry+v1 behavior (release in build metadata)
+	vr, err := bundle.NewLegacyRegistryV1VersionRelease(pkg.Version)
+	if err != nil {
+		return nil, err
+	}
+	return vr, nil
+}
+
 // MetadataFor returns a BundleMetadata for the given bundle name and version.
 func MetadataFor(bundleName string, bundleVersion bsemver.Version) ocv1.BundleMetadata {
 	return ocv1.BundleMetadata{

internal/operator-controller/bundleutil/bundle_test.go

@@ -69,6 +69,44 @@ func TestGetVersionAndRelease(t *testing.T) {
 			pkgProperty: nil,
 			wantErr:     true,
 		},
+		{
+			name: "explicit release field - takes precedence over build metadata",
+			pkgProperty: &property.Property{
+				Type:  property.TypePackage,
+				Value: json.RawMessage(`{"version": "1.0.0+ignored", "release": "2"}`),
+			},
+			wantVersionRelease: &bundle.VersionRelease{
+				Version: bsemver.MustParse("1.0.0+ignored"),
+				Release: bundle.Release([]bsemver.PRVersion{
+					{VersionNum: 2, IsNum: true},
+				}),
+			},
+			wantErr: false,
+		},
+		{
+			name: "explicit release field - complex release",
+			pkgProperty: &property.Property{
+				Type:  property.TypePackage,
+				Value: json.RawMessage(`{"version": "2.1.0", "release": "1.alpha.3"}`),
+			},
+			wantVersionRelease: &bundle.VersionRelease{
+				Version: bsemver.MustParse("2.1.0"),
+				Release: bundle.Release([]bsemver.PRVersion{
+					{VersionNum: 1, IsNum: true},
+					{VersionStr: "alpha"},
+					{VersionNum: 3, IsNum: true},
+				}),
+			},
+			wantErr: false,
+		},
+		{
+			name: "explicit release field - invalid release",
+			pkgProperty: &property.Property{
+				Type:  property.TypePackage,
+				Value: json.RawMessage(`{"version": "1.0.0", "release": "001"}`),
+			},
+			wantErr: true,
+		},
 	}
 
 	for _, tc := range tests {
@@ -83,11 +121,12 @@ func TestGetVersionAndRelease(t *testing.T) {
 				Properties: properties,
 			}
 
-			_, err := bundleutil.GetVersionAndRelease(bundle)
+			actual, err := bundleutil.GetVersionAndRelease(bundle)
 			if tc.wantErr {
 				require.Error(t, err)
 			} else {
 				require.NoError(t, err)
+				require.Equal(t, tc.wantVersionRelease, actual)
 			}
 		})
 	}

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

@@ -10,6 +10,7 @@ import (
 	"github.com/operator-framework/operator-registry/alpha/declcfg"
 
 	"github.com/operator-framework/operator-controller/internal/operator-controller/bundleutil"
+	"github.com/operator-framework/operator-controller/internal/operator-controller/features"
 	slicesutil "github.com/operator-framework/operator-controller/internal/shared/util/slices"
 )
 
@@ -39,9 +40,22 @@ func newMastermindsRange(versionRange string) (bsemver.Range, error) {
 }
 
 // ByVersionAndRelease is a comparison function that compares bundles by
-// version and release. Bundles with lower versions/releases are
-// considered less than bundles with higher versions/releases.
+// version and release. When the CompositeVersionComparison feature gate is
+// enabled, it uses Bundle.Compare() (which reads pkg.Release from olm.package)
+// and falls back to build metadata comparison if equal. When disabled, it uses
+// version+release from build metadata (backward compatible).
 func ByVersionAndRelease(b1, b2 declcfg.Bundle) int {
+	if features.OperatorControllerFeatureGate.Enabled(features.CompositeVersionComparison) {
+		// Use CompositeVersion comparison (reads pkg.Release from olm.package)
+		result := b2.Compare(&b1)
+		if result != 0 {
+			return result
+		}
+		// If CompositeVersion comparison is equal, fall back to build metadata
+		return compareByBuildMetadata(b1, b2)
+	}
+
+	// Default: use version+release from build metadata (backward compatible)
 	vr1, err1 := bundleutil.GetVersionAndRelease(b1)
 	vr2, err2 := bundleutil.GetVersionAndRelease(b2)
 
@@ -54,6 +68,18 @@ func ByVersionAndRelease(b1, b2 declcfg.Bundle) int {
 	return vr2.Compare(*vr1)
 }
 
+// compareByBuildMetadata compares bundles using build metadata parsing.
+// This is used as a fallback when CompositeVersion comparison returns equal.
+func compareByBuildMetadata(b1, b2 declcfg.Bundle) int {
+	vr1, err1 := bundleutil.GetVersionAndRelease(b1)
+	vr2, err2 := bundleutil.GetVersionAndRelease(b2)
+
+	if err1 != nil || err2 != nil {
+		return compareErrors(err2, err1)
+	}
+	return vr2.Compare(*vr1)
+}
+
 func ByDeprecationFunc(deprecation declcfg.Deprecation) func(a, b declcfg.Bundle) int {
 	deprecatedBundles := sets.New[string]()
 	for _, entry := range deprecation.Entries {

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

@@ -2,6 +2,7 @@ package compare_test
 
 import (
 	"encoding/json"
+	"fmt"
 	"slices"
 	"testing"
 
@@ -13,6 +14,7 @@ import (
 	"github.com/operator-framework/operator-registry/alpha/property"
 
 	"github.com/operator-framework/operator-controller/internal/operator-controller/catalogmetadata/compare"
+	"github.com/operator-framework/operator-controller/internal/operator-controller/features"
 )
 
 func TestNewVersionRange(t *testing.T) {
@@ -138,13 +140,13 @@ func TestByVersionAndRelease(t *testing.T) {
 	t.Run("all bundles valid", func(t *testing.T) {
 		toSort := []declcfg.Bundle{b3_1, b2, b3_2, b1}
 		slices.SortStableFunc(toSort, compare.ByVersionAndRelease)
-		assert.Equal(t, []declcfg.Bundle{b1, b3_2, b3_1, b2}, toSort)
+		assert.Equal(t, []declcfg.Bundle{b1, b3_2, b3_1, b2}, toSort, "should sort descending: 1.0.0 > 1.0.0-alpha+2 > 1.0.0-alpha+1 > 0.0.1")
 	})
 
 	t.Run("some bundles are missing version", func(t *testing.T) {
 		toSort := []declcfg.Bundle{b3_1, b4noVersion, b2, b3_2, b5empty, b1}
 		slices.SortStableFunc(toSort, compare.ByVersionAndRelease)
-		assert.Equal(t, []declcfg.Bundle{b1, b3_2, b3_1, b2, b4noVersion, b5empty}, toSort)
+		assert.Equal(t, []declcfg.Bundle{b1, b3_2, b3_1, b2, b4noVersion, b5empty}, toSort, "bundles with invalid/missing versions should sort last")
 	})
 }
 
@@ -161,10 +163,67 @@ func TestByDeprecationFunc(t *testing.T) {
 	c := declcfg.Bundle{Name: "c"}
 	d := declcfg.Bundle{Name: "d"}
 
-	assert.Equal(t, 0, byDeprecation(a, b))
-	assert.Equal(t, 0, byDeprecation(b, a))
-	assert.Equal(t, 1, byDeprecation(a, c))
-	assert.Equal(t, -1, byDeprecation(c, a))
-	assert.Equal(t, 0, byDeprecation(c, d))
-	assert.Equal(t, 0, byDeprecation(d, c))
+	assert.Equal(t, 0, byDeprecation(a, b), "both deprecated bundles are equal")
+	assert.Equal(t, 0, byDeprecation(b, a), "both deprecated bundles are equal")
+	assert.Equal(t, 1, byDeprecation(a, c), "deprecated bundle 'a' should sort after non-deprecated 'c'")
+	assert.Equal(t, -1, byDeprecation(c, a), "non-deprecated bundle 'c' should sort before deprecated 'a'")
+	assert.Equal(t, 0, byDeprecation(c, d), "both non-deprecated bundles are equal")
+	assert.Equal(t, 0, byDeprecation(d, c), "both non-deprecated bundles are equal")
+}
+
+// TestByVersionAndRelease_WithCompositeVersionComparison tests the feature-gated hybrid comparison
+// for registry+v1 bundles. When the feature gate is enabled, Bundle.Compare() is called but returns
+// 0 for registry+v1 bundles (no explicit release field), so we fall back to build metadata parsing.
+// This maintains backward compatibility while preparing for future bundle formats with explicit release.
+func TestByVersionAndRelease_WithCompositeVersionComparison(t *testing.T) {
+	// Registry+v1 bundles: same version, different build metadata
+	b1 := declcfg.Bundle{
+		Name: "package1.v1.0.0+1",
+		Properties: []property.Property{
+			{Type: property.TypePackage, Value: json.RawMessage(`{"packageName": "package1", "version": "1.0.0+1"}`)},
+		},
+	}
+	b2 := declcfg.Bundle{
+		Name: "package1.v1.0.0+2",
+		Properties: []property.Property{
+			{Type: property.TypePackage, Value: json.RawMessage(`{"packageName": "package1", "version": "1.0.0+2"}`)},
+		},
+	}
+
+	prevEnabled := features.OperatorControllerFeatureGate.Enabled(features.CompositeVersionComparison)
+	t.Cleanup(func() {
+		require.NoError(t, features.OperatorControllerFeatureGate.Set(fmt.Sprintf("%s=%t", features.CompositeVersionComparison, prevEnabled)))
+	})
+
+	t.Run("feature gate disabled - uses build metadata only", func(t *testing.T) {
+		require.NoError(t, features.OperatorControllerFeatureGate.Set(fmt.Sprintf("%s=false", features.CompositeVersionComparison)))
+		result := compare.ByVersionAndRelease(b1, b2)
+		assert.Positive(t, result, "should sort by build metadata: 1.0.0+2 > 1.0.0+1")
+	})
+
+	t.Run("feature gate enabled - registry+v1 bundles still work via build metadata fallback", func(t *testing.T) {
+		require.NoError(t, features.OperatorControllerFeatureGate.Set(fmt.Sprintf("%s=true", features.CompositeVersionComparison)))
+		result := compare.ByVersionAndRelease(b1, b2)
+		assert.Positive(t, result, "Bundle.Compare() returns 0, fallback to build metadata: 1.0.0+2 > 1.0.0+1")
+	})
+
+	t.Run("bundles with explicit pkg.Release field", func(t *testing.T) {
+		// Bundle with explicit release=1
+		explicitR1 := declcfg.Bundle{
+			Name: "pkg.v2.0.0-r1",
+			Properties: []property.Property{
+				{Type: property.TypePackage, Value: json.RawMessage(`{"packageName": "pkg", "version": "2.0.0", "release": "1"}`)},
+			},
+		}
+		// Bundle with explicit release=2
+		explicitR2 := declcfg.Bundle{
+			Name: "pkg.v2.0.0-r2",
+			Properties: []property.Property{
+				{Type: property.TypePackage, Value: json.RawMessage(`{"packageName": "pkg", "version": "2.0.0", "release": "2"}`)},
+			},
+		}
+
+		result := compare.ByVersionAndRelease(explicitR1, explicitR2)
+		assert.Positive(t, result, "2.0.0+release.2 > 2.0.0+release.1 (explicit release field)")
+	})
 }

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

@@ -48,3 +48,21 @@ func InAnyChannel(channels ...declcfg.Channel) filter.Predicate[declcfg.Bundle]
 		return false
 	}
 }
+
+// SameVersionHigherRelease returns a predicate that matches bundles with the same
+// semantic version as the provided version-release, 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] {
+	return func(b declcfg.Bundle) bool {
+		actual, err := bundleutil.GetVersionAndRelease(b)
+		if err != nil {
+			return false
+		}
+
+		if expect.Version.Compare(actual.Version) != 0 {
+			return false
+		}
+
+		return expect.Release.Compare(actual.Release) < 0
+	}
+}