explicitly read pkg.Release, falling back to build metadata parsing when comparison returns equal

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

Author: rashmigottipati

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) {
@@ -168,3 +170,59 @@ func TestByDeprecationFunc(t *testing.T) {
 	assert.Equal(t, 0, byDeprecation(c, d))
 	assert.Equal(t, 0, byDeprecation(d, c))
 }
+
+// TestByVersionAndRelease_WithCompositeVersionComparison tests the feature-gated hybrid comparison:
+// When disabled: uses build metadata (backward compatible)
+// When enabled: uses Bundle.Compare() with build metadata fallback for registry+v1
+func TestByVersionAndRelease_WithCompositeVersionComparison(t *testing.T) {
+	// Registry+v1 bundles: same version, different build metadata
+	registryV1_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"}`)},
+		},
+	}
+	registryV1_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"}`)},
+		},
+	}
+
+	// New format bundles: same version, different .spec.release
+	newFormat_b1 := declcfg.Bundle{
+		Name: "package2.v1.0.0-rel.1",
+		Properties: []property.Property{
+			{Type: property.TypePackage, Value: json.RawMessage(`{"packageName": "package2", "version": "1.0.0", "release": "1"}`)},
+		},
+	}
+	newFormat_b2 := declcfg.Bundle{
+		Name: "package2.v1.0.0-rel.2",
+		Properties: []property.Property{
+			{Type: property.TypePackage, Value: json.RawMessage(`{"packageName": "package2", "version": "1.0.0", "release": "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", func(t *testing.T) {
+		require.NoError(t, features.OperatorControllerFeatureGate.Set(fmt.Sprintf("%s=false", features.CompositeVersionComparison)))
+		result := compare.ByVersionAndRelease(registryV1_b1, registryV1_b2)
+		assert.Greater(t, result, 0, "should sort by build metadata: 1.0.0+2 > 1.0.0+1")
+	})
+
+	t.Run("feature gate enabled - registry+v1 bundles use build metadata fallback", func(t *testing.T) {
+		require.NoError(t, features.OperatorControllerFeatureGate.Set(fmt.Sprintf("%s=true", features.CompositeVersionComparison)))
+		result := compare.ByVersionAndRelease(registryV1_b1, registryV1_b2)
+		assert.Greater(t, result, 0, "should fallback to build metadata: 1.0.0+2 > 1.0.0+1")
+	})
+
+	t.Run("feature gate enabled - new format bundles use .spec.release", func(t *testing.T) {
+		require.NoError(t, features.OperatorControllerFeatureGate.Set(fmt.Sprintf("%s=true", features.CompositeVersionComparison)))
+		result := compare.ByVersionAndRelease(newFormat_b1, newFormat_b2)
+		assert.Greater(t, result, 0, "should use .spec.release: release=2 > release=1")
+	})
+}

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

@@ -35,10 +35,10 @@ func SuccessorsOf(installedBundle ocv1.BundleMetadata, channels ...declcfg.Chann
 		ExactVersionRelease(*installedVersionRelease),
 	}
 
-	// If release version priority is enabled, also consider bundles
+	// If CompositeVersionComparison 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) {
+	if features.OperatorControllerFeatureGate.Enabled(features.CompositeVersionComparison) {
 		predicates = append(predicates, SameVersionHigherRelease(*installedVersionRelease))
 	}
 

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

@@ -243,14 +243,14 @@ func TestLegacySuccessor(t *testing.T) {
 	assert.False(t, f(emptyBundle))
 }
 
-// TestSuccessorsOf_WithReleaseVersionPriority_FeatureGateDisabled verifies higher releases
-// are NOT successors when ReleaseVersionPriority gate is disabled (testing the default behavior).
-func TestSuccessorsOf_WithReleaseVersionPriority_FeatureGateDisabled(t *testing.T) {
+// TestSuccessorsOf_WithCompositeVersionComparison_FeatureGateDisabled verifies higher releases
+// are NOT successors when CompositeVersionComparison gate is disabled (testing the default behavior).
+func TestSuccessorsOf_WithCompositeVersionComparison_FeatureGateDisabled(t *testing.T) {
 	// Explicitly disable the feature gate for this test
-	prevEnabled := features.OperatorControllerFeatureGate.Enabled(features.ReleaseVersionPriority)
-	require.NoError(t, features.OperatorControllerFeatureGate.Set(fmt.Sprintf("%s=false", features.ReleaseVersionPriority)))
+	prevEnabled := features.OperatorControllerFeatureGate.Enabled(features.CompositeVersionComparison)
+	require.NoError(t, features.OperatorControllerFeatureGate.Set(fmt.Sprintf("%s=false", features.CompositeVersionComparison)))
 	t.Cleanup(func() {
-		require.NoError(t, features.OperatorControllerFeatureGate.Set(fmt.Sprintf("%s=%t", features.ReleaseVersionPriority, prevEnabled)))
+		require.NoError(t, features.OperatorControllerFeatureGate.Set(fmt.Sprintf("%s=%t", features.CompositeVersionComparison, prevEnabled)))
 	})
 
 	channel := declcfg.Channel{
@@ -282,14 +282,14 @@ func TestSuccessorsOf_WithReleaseVersionPriority_FeatureGateDisabled(t *testing.
 	assert.False(t, predicate(higherRelease))
 }
 
-// TestSuccessorsOf_WithReleaseVersionPriority_FeatureGateEnabled verifies higher releases
-// as valid successors when ReleaseVersionPriority gate is enabled.
-func TestSuccessorsOf_WithReleaseVersionPriority_FeatureGateEnabled(t *testing.T) {
+// TestSuccessorsOf_WithCompositeVersionComparison_FeatureGateEnabled verifies higher releases
+// as valid successors when CompositeVersionComparison gate is enabled.
+func TestSuccessorsOf_WithCompositeVersionComparison_FeatureGateEnabled(t *testing.T) {
 	// Enable the feature gate for this test
-	prevEnabled := features.OperatorControllerFeatureGate.Enabled(features.ReleaseVersionPriority)
-	require.NoError(t, features.OperatorControllerFeatureGate.Set(fmt.Sprintf("%s=true", features.ReleaseVersionPriority)))
+	prevEnabled := features.OperatorControllerFeatureGate.Enabled(features.CompositeVersionComparison)
+	require.NoError(t, features.OperatorControllerFeatureGate.Set(fmt.Sprintf("%s=true", features.CompositeVersionComparison)))
 	t.Cleanup(func() {
-		require.NoError(t, features.OperatorControllerFeatureGate.Set(fmt.Sprintf("%s=%t", features.ReleaseVersionPriority, prevEnabled)))
+		require.NoError(t, features.OperatorControllerFeatureGate.Set(fmt.Sprintf("%s=%t", features.CompositeVersionComparison, prevEnabled)))
 	})
 
 	channel := declcfg.Channel{

internal/operator-controller/features/features.go

@@ -17,9 +17,9 @@ const (
 	WebhookProviderCertManager        featuregate.Feature = "WebhookProviderCertManager"
 	WebhookProviderOpenshiftServiceCA featuregate.Feature = "WebhookProviderOpenshiftServiceCA"
 	HelmChartSupport                  featuregate.Feature = "HelmChartSupport"
-	BoxcutterRuntime                  featuregate.Feature = "BoxcutterRuntime"
-	DeploymentConfig                  featuregate.Feature = "DeploymentConfig"
-	ReleaseVersionPriority            featuregate.Feature = "ReleaseVersionPriority"
+	BoxcutterRuntime           featuregate.Feature = "BoxcutterRuntime"
+	DeploymentConfig           featuregate.Feature = "DeploymentConfig"
+	CompositeVersionComparison featuregate.Feature = "CompositeVersionComparison"
 )
 
 var operatorControllerFeatureGates = map[featuregate.Feature]featuregate.FeatureSpec{
@@ -91,11 +91,12 @@ var operatorControllerFeatureGates = map[featuregate.Feature]featuregate.Feature
 		LockToDefault: false,
 	},
 
-	// ReleaseVersionPriority enables considering bundles with higher release versions
-	// as valid upgrade successors. When enabled, bundles with the same semantic version
-	// but higher release values (e.g., 2.0.0+2 as a successor to 2.0.0+1) are included
-	// in the upgrade candidate set during resolution.
-	ReleaseVersionPriority: {
+	// CompositeVersionComparison enables bundle comparison using CompositeVersion
+	// from operator-registry, which reads the explicit pkg.Release field from the
+	// olm.package property. When this comparison returns equal, build metadata is
+	// used as a tiebreaker. This supports both new bundle formats (with explicit
+	// release) and registry+v1 bundles (with build metadata).
+	CompositeVersionComparison: {
 		Default:       false,
 		PreRelease:    featuregate.Alpha,
 		LockToDefault: false,