address review comments

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

Author: rashmigottipati

internal/operator-controller/bundleutil/bundle.go

@@ -22,15 +22,15 @@ func GetVersionAndRelease(b declcfg.Bundle) (*declcfg.VersionRelease, error) {
 	return nil, fmt.Errorf("no package property found in bundle %q", b.Name)
 }
 
-// newLegacyRegistryV1VersionRelease parses a registry+v1 bundle version string and returns a
+// ParseLegacyVersionRelease parses a registry+v1 bundle version string and returns a
 // VersionRelease. Some registry+v1 bundles utilize the build metadata field of the semver version
 // as release information (a semver spec violation maintained for backward compatibility). When the
 // bundle version includes build metadata that is parsable as a release, the returned
 // VersionRelease has the build metadata extracted into the Release field, and the Version field
 // has its Build metadata cleared. When the bundle version includes build metadata that is NOT
 // parseable as a release, the returned VersionRelease has its Version set to the full semver
 // version (with build metadata) and its Release left empty.
-func newLegacyRegistryV1VersionRelease(vStr string) (*declcfg.VersionRelease, error) {
+func ParseLegacyVersionRelease(vStr string) (*declcfg.VersionRelease, error) {
 	vers, err := bsemver.Parse(vStr)
 	if err != nil {
 		return nil, err
@@ -93,13 +93,13 @@ func parseVersionRelease(pkgData json.RawMessage) (*declcfg.VersionRelease, erro
 	//   In the future, for supporting other bundle formats, we should not
 	//   use the legacy registry+v1 mechanism (i.e. using build metadata in
 	//   the version) to determine the bundle's release.
-	return newLegacyRegistryV1VersionRelease(pkg.Version)
+	return ParseLegacyVersionRelease(pkg.Version)
 }
 
 // parseExplicitRelease parses version and release from separate fields.
 // Build metadata is preserved in the version because with an explicit release field,
 // build metadata serves its proper semver purpose (e.g., git commit, build number).
-// In contrast, newLegacyRegistryV1VersionRelease clears build metadata because it
+// In contrast, ParseLegacyVersionRelease clears build metadata because it
 // interprets build metadata AS the release value for registry+v1 bundles.
 func parseExplicitRelease(version, releaseStr string) (*declcfg.VersionRelease, error) {
 	vers, err := bsemver.Parse(version)

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

@@ -8,6 +8,7 @@ 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/bundleutil"
 	"github.com/operator-framework/operator-controller/internal/shared/util/filter"
 )
 
@@ -17,11 +18,15 @@ import (
 func parseInstalledBundleVersionRelease(installedBundle ocv1.BundleMetadata) (*declcfg.VersionRelease, error) {
 	// Handle legacy registry+v1 format: release embedded in version's build metadata
 	if installedBundle.Release == nil {
-		return newLegacyRegistryV1VersionRelease(installedBundle.Version)
+		vr, err := bundleutil.ParseLegacyVersionRelease(installedBundle.Version)
+		if err != nil {
+			return nil, fmt.Errorf("failed to get version and release of installed bundle: %w", err)
+		}
+		return vr, nil
 	}
 
 	// Bundle has explicit release field (or explicitly empty) - parse version and release from separate fields.
-	// Note: We can't use newLegacyRegistryV1VersionRelease here because the version might
+	// Note: We can't use ParseLegacyVersionRelease here because the version might
 	// already contain build metadata (e.g., "1.0.0+git.abc"), which serves its proper
 	// semver purpose when using explicit pkg.Release. Concatenating would create invalid
 	// semver like "1.0.0+git.abc+2".
@@ -48,35 +53,6 @@ func parseInstalledBundleVersionRelease(installedBundle ocv1.BundleMetadata) (*d
 	}, nil
 }
 
-// newLegacyRegistryV1VersionRelease parses a registry+v1 bundle version string and returns a
-// VersionRelease. Some registry+v1 bundles utilize the build metadata field of the semver version
-// as release information (a semver spec violation maintained for backward compatibility).
-func newLegacyRegistryV1VersionRelease(vStr string) (*declcfg.VersionRelease, error) {
-	vers, err := bsemver.Parse(vStr)
-	if err != nil {
-		return nil, fmt.Errorf("failed to get version and release of installed bundle: %w", err)
-	}
-
-	vr := &declcfg.VersionRelease{
-		Version: vers,
-	}
-
-	buildMetadata := ""
-	if len(vr.Version.Build) > 0 {
-		buildMetadata = vr.Version.Build[0]
-		for i := 1; i < len(vr.Version.Build); i++ {
-			buildMetadata += "." + vr.Version.Build[i]
-		}
-	}
-
-	rel, err := declcfg.NewRelease(buildMetadata)
-	if err == nil && len(rel) > 0 {
-		vr.Release = rel
-		vr.Version.Build = nil
-	}
-	return vr, nil
-}
-
 func SuccessorsOf(installedBundle ocv1.BundleMetadata, channels ...declcfg.Channel) (filter.Predicate[declcfg.Bundle], error) {
 	installedVersionRelease, err := parseInstalledBundleVersionRelease(installedBundle)
 	if err != nil {

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

@@ -21,7 +21,7 @@ import (
 // mustVersionRelease is a test helper that parses a version string into a VersionRelease.
 // For registry+v1 bundles, build metadata is interpreted as release (e.g., "1.0.0+2" -> Version: 1.0.0, Release: 2).
 func mustVersionRelease(versionStr string) declcfg.VersionRelease {
-	vr, err := newLegacyRegistryV1VersionRelease(versionStr)
+	vr, err := bundleutil.ParseLegacyVersionRelease(versionStr)
 	if err != nil {
 		panic(err)
 	}