address review feedback

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

Author: rashmigottipati

api/v1/clusterextension_types.go

@@ -467,16 +467,27 @@ type BundleMetadata struct {
 	// +kubebuilder:validation:XValidation:rule="self.matches(\"^([0-9]+)(\\\\.[0-9]+)?(\\\\.[0-9]+)?(-([-0-9A-Za-z]+(\\\\.[-0-9A-Za-z]+)*))?(\\\\+([-0-9A-Za-z]+(-\\\\.[-0-9A-Za-z]+)*))?\")",message="version must be well-formed semver"
 	Version string `json:"version"`
 
-	// release is an optional field that references the release value for this bundle.
-	// The release follows pre-release/build metadata syntax as defined in https://semver.org/,
-	// consisting of dot-separated identifiers where numeric identifiers must not have leading zeros.
+	// release is an optional field that identifies a specific release of this bundle's version.
+	// A release represents a re-publication of the same version, typically used to deliver
+	// packaging or metadata changes without changing the version number. When multiple
+	// releases exist for the same version, higher releases are preferred. An unset release
+	// is less preferred than all other release values.
+	//
+	// The value consists of dot-separated identifiers, where each identifier is either a
+	// numeric value (without leading zeros) or an alphanumeric string (e.g., "2", "1.el9",
+	// "3.alpha.1"). Releases are compared identifier by identifier: numeric identifiers are
+	// compared as integers, alphanumeric identifiers are compared lexically, and numeric
+	// identifiers always sort before alphanumeric identifiers.
+	//
 	// For bundles with explicit pkg.Release metadata, this field contains that release value.
-	// For registry+v1 bundles, this field contains the release extracted from version's build metadata (e.g., '2' from '1.0.0+2').
-	// This field may be omitted if there is no explicit release and the version contains no parseable build metadata.
+	// For registry+v1 bundles lacking an explicit release value, this field contains the release
+	// extracted from version's build metadata (e.g., '2' from '1.0.0+2').
+	// This field is omitted when the bundle's release value is unset.
 	//
 	// +optional
-	// +kubebuilder:validation:XValidation:rule="self.matches(\"^$|^(0|[1-9][0-9]*|[0-9]*[A-Za-z-][0-9A-Za-z-]*)(\\\\.(0|[1-9][0-9]*|[0-9]*[A-Za-z-][0-9A-Za-z-]*))*$\")",message="release must be empty or well-formed pre-release/build metadata (dot-separated identifiers, numeric parts without leading zeros)"
-	Release string `json:"release,omitempty"`
+	// <opcon:experimental>
+	// +kubebuilder:validation:XValidation:rule="self.matches(\"^$|^(0|[1-9][0-9]*|[0-9]*[A-Za-z-][0-9A-Za-z-]*)(\\\\.(0|[1-9][0-9]*|[0-9]*[A-Za-z-][0-9A-Za-z-]*))*$\")",message="release must be empty or consist of dot-separated identifiers (numeric without leading zeros, or alphanumeric)"
+	Release *string `json:"release,omitempty"`
 }
 
 // RevisionStatus defines the observed state of a ClusterObjectSet.

api/v1/zz_generated.deepcopy.go

@@ -67,7 +67,7 @@ _Appears in:_
 | --- | --- | --- | --- |
 | `name` _string_ | name is required and follows the DNS subdomain standard as defined in [RFC 1123].<br />It must contain only lowercase alphanumeric characters, hyphens (-) or periods (.),<br />start and end with an alphanumeric character, and be no longer than 253 characters. |  | Required: \{\} <br /> |
 | `version` _string_ | version is required and references the version that this bundle represents.<br />It follows the semantic versioning standard as defined in https://semver.org/. |  | Required: \{\} <br /> |
-| `release` _string_ | release is an optional field that references the release value for this bundle.<br />The release follows pre-release/build metadata syntax as defined in https://semver.org/,<br />consisting of dot-separated identifiers where numeric identifiers must not have leading zeros.<br />For bundles with explicit pkg.Release metadata, this field contains that release value.<br />For registry+v1 bundles, this field contains the release extracted from version's build metadata (e.g., '2' from '1.0.0+2').<br />This field may be omitted if there is no explicit release and the version contains no parseable build metadata. |  | Optional: \{\} <br /> |
+| `release` _string_ | release is an optional field that identifies a specific release of this bundle's version.<br />A release represents a re-publication of the same version, typically used to deliver<br />packaging or metadata changes without changing the version number. When multiple<br />releases exist for the same version, higher releases are preferred. An unset release<br />is less preferred than all other release values.<br />The value consists of dot-separated identifiers, where each identifier is either a<br />numeric value (without leading zeros) or an alphanumeric string (e.g., "2", "1.el9",<br />"3.alpha.1"). Releases are compared identifier by identifier: numeric identifiers are<br />compared as integers, alphanumeric identifiers are compared lexically, and numeric<br />identifiers always sort before alphanumeric identifiers.<br />For bundles with explicit pkg.Release metadata, this field contains that release value.<br />For registry+v1 bundles lacking an explicit release value, this field contains the release<br />extracted from version's build metadata (e.g., '2' from '1.0.0+2').<br />This field is omitted when the bundle's release value is unset.<br /><opcon:experimental> |  | Optional: \{\} <br /> |
 
 
 #### CRDUpgradeSafetyEnforcement

applyconfigurations/api/v1/bundlemetadata.go

@@ -690,17 +690,26 @@ spec:
                           rule: self.matches("^[a-z0-9]([-a-z0-9]*[a-z0-9])?(\\.[a-z0-9]([-a-z0-9]*[a-z0-9])?)*$")
                       release:
                         description: |-
-                          release is an optional field that references the release value for this bundle.
-                          The release follows pre-release/build metadata syntax as defined in https://semver.org/,
-                          consisting of dot-separated identifiers where numeric identifiers must not have leading zeros.
+                          release is an optional field that identifies a specific release of this bundle's version.
+                          A release represents a re-publication of the same version, typically used to deliver
+                          packaging or metadata changes without changing the version number. When multiple
+                          releases exist for the same version, higher releases are preferred. An unset release
+                          is less preferred than all other release values.
+
+                          The value consists of dot-separated identifiers, where each identifier is either a
+                          numeric value (without leading zeros) or an alphanumeric string (e.g., "2", "1.el9",
+                          "3.alpha.1"). Releases are compared identifier by identifier: numeric identifiers are
+                          compared as integers, alphanumeric identifiers are compared lexically, and numeric
+                          identifiers always sort before alphanumeric identifiers.
+
                           For bundles with explicit pkg.Release metadata, this field contains that release value.
-                          For registry+v1 bundles, this field contains the release extracted from version's build metadata (e.g., '2' from '1.0.0+2').
-                          This field may be omitted if there is no explicit release and the version contains no parseable build metadata.
+                          For registry+v1 bundles lacking an explicit release value, this field contains the release
+                          extracted from version's build metadata (e.g., '2' from '1.0.0+2').
+                          This field is omitted when the bundle's release value is unset.
                         type: string
                         x-kubernetes-validations:
-                        - message: release must be empty or well-formed pre-release/build
-                            metadata (dot-separated identifiers, numeric parts without
-                            leading zeros)
+                        - message: release must be empty or consist of dot-separated
+                            identifiers (numeric without leading zeros, or alphanumeric)
                           rule: self.matches("^$|^(0|[1-9][0-9]*|[0-9]*[A-Za-z-][0-9A-Za-z-]*)(\\.(0|[1-9][0-9]*|[0-9]*[A-Za-z-][0-9A-Za-z-]*))*$")
                       version:
                         description: |-

docs/api-reference/olmv1-api-reference.md

@@ -556,20 +556,6 @@ spec:
                             hyphens (-) or periods (.), start and end with an alphanumeric
                             character, and be no longer than 253 characters
                           rule: self.matches("^[a-z0-9]([-a-z0-9]*[a-z0-9])?(\\.[a-z0-9]([-a-z0-9]*[a-z0-9])?)*$")
-                      release:
-                        description: |-
-                          release is an optional field that references the release value for this bundle.
-                          The release follows pre-release/build metadata syntax as defined in https://semver.org/,
-                          consisting of dot-separated identifiers where numeric identifiers must not have leading zeros.
-                          For bundles with explicit pkg.Release metadata, this field contains that release value.
-                          For registry+v1 bundles, this field contains the release extracted from version's build metadata (e.g., '2' from '1.0.0+2').
-                          This field may be omitted if there is no explicit release and the version contains no parseable build metadata.
-                        type: string
-                        x-kubernetes-validations:
-                        - message: release must be empty or well-formed pre-release/build
-                            metadata (dot-separated identifiers, numeric parts without
-                            leading zeros)
-                          rule: self.matches("^$|^(0|[1-9][0-9]*|[0-9]*[A-Za-z-][0-9A-Za-z-]*)(\\.(0|[1-9][0-9]*|[0-9]*[A-Za-z-][0-9A-Za-z-]*))*$")
                       version:
                         description: |-
                           version is required and references the version that this bundle represents.

helm/olmv1/base/operator-controller/crd/experimental/olm.operatorframework.io_clusterextensions.yaml

@@ -85,6 +85,19 @@ func (vr *VersionRelease) AsLegacyRegistryV1Version() bsemver.Version {
 
 type Release []bsemver.PRVersion
 
+// String returns the string representation of the release.
+// Returns an empty string if the release is nil or empty.
+func (r Release) String() string {
+	if len(r) == 0 {
+		return ""
+	}
+	parts := make([]string, len(r))
+	for i, pr := range r {
+		parts[i] = pr.String()
+	}
+	return strings.Join(parts, ".")
+}
+
 // Compare compares two Release values. It returns:
 //
 //	-1 if r < other

helm/olmv1/base/operator-controller/crd/standard/olm.operatorframework.io_clusterextensions.yaml

@@ -3,7 +3,6 @@ package bundleutil
 import (
 	"encoding/json"
 	"fmt"
-	"strings"
 
 	bsemver "github.com/blang/semver/v4"
 
@@ -13,7 +12,6 @@ import (
 	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"
-	slicesutil "github.com/operator-framework/operator-controller/internal/shared/util/slices"
 )
 
 func GetVersionAndRelease(b declcfg.Bundle) (*bundle.VersionRelease, error) {
@@ -52,6 +50,11 @@ func parseVersionRelease(pkgData json.RawMessage) (*bundle.VersionRelease, error
 	}
 
 	// Fall back to legacy registry+v1 behavior (release in build metadata)
+	//
+	// TODO: For now, we assume that all bundles are registry+v1 bundles.
+	//   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.
 	vr, err := bundle.NewLegacyRegistryV1VersionRelease(pkg.Version)
 	if err != nil {
 		return nil, err
@@ -65,9 +68,9 @@ func MetadataFor(bundleName string, vr bundle.VersionRelease) ocv1.BundleMetadat
 		Name:    bundleName,
 		Version: vr.Version.String(),
 	}
-	if len(vr.Release) > 0 {
-		parts := slicesutil.Map(vr.Release, func(pr bsemver.PRVersion) string { return pr.String() })
-		bm.Release = strings.Join(parts, ".")
+	if vr.Release != nil {
+		relStr := vr.Release.String()
+		bm.Release = &relStr
 	}
 	return bm
 }

internal/operator-controller/bundle/versionrelease.go

@@ -213,7 +213,8 @@ func TestMetadataFor(t *testing.T) {
 		result := bundleutil.MetadataFor("test-bundle", vr)
 		require.Equal(t, "test-bundle", result.Name)
 		require.Equal(t, "1.0.0", result.Version)
-		require.Equal(t, "2", result.Release)
+		require.NotNil(t, result.Release)
+		require.Equal(t, "2", *result.Release)
 	})
 
 	t.Run("without release", func(t *testing.T) {
@@ -224,6 +225,18 @@ func TestMetadataFor(t *testing.T) {
 		result := bundleutil.MetadataFor("test-bundle", vr)
 		require.Equal(t, "test-bundle", result.Name)
 		require.Equal(t, "1.0.0", result.Version)
-		require.Empty(t, result.Release)
+		require.Nil(t, result.Release)
+	})
+
+	t.Run("with explicit empty release", func(t *testing.T) {
+		vr := bundle.VersionRelease{
+			Version: bsemver.MustParse("1.0.0"),
+			Release: bundle.Release([]bsemver.PRVersion{}),
+		}
+		result := bundleutil.MetadataFor("test-bundle", vr)
+		require.Equal(t, "test-bundle", result.Name)
+		require.Equal(t, "1.0.0", result.Version)
+		require.NotNil(t, result.Release)
+		require.Empty(t, *result.Release)
 	})
 }

internal/operator-controller/bundleutil/bundle.go

@@ -14,13 +14,13 @@ import (
 
 func SuccessorsOf(installedBundle ocv1.BundleMetadata, channels ...declcfg.Channel) (filter.Predicate[declcfg.Bundle], error) {
 	// Construct VersionRelease from BundleMetadata.
-	// If the Release field is populated, parse version and release separately.
-	// Otherwise, parse release from version build metadata (registry+v1 legacy format).
+	// If the Release field is not nil (even if empty string), use it as the explicit release.
+	// If the Release field is nil, parse release from version build metadata (registry+v1 legacy format).
 	var installedVersionRelease *bundle.VersionRelease
 	var err error
 
-	if installedBundle.Release != "" {
-		// Bundle has explicit release field - parse version and release from separate fields.
+	if installedBundle.Release != 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
 		// 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
@@ -29,9 +29,9 @@ func SuccessorsOf(installedBundle ocv1.BundleMetadata, channels ...declcfg.Chann
 		if err != nil {
 			return nil, fmt.Errorf("failed to parse installed bundle version %q: %w", installedBundle.Version, err)
 		}
-		release, err := bundle.NewRelease(installedBundle.Release)
+		release, err := bundle.NewRelease(*installedBundle.Release)
 		if err != nil {
-			return nil, fmt.Errorf("failed to parse installed bundle release %q: %w", installedBundle.Release, err)
+			return nil, fmt.Errorf("failed to parse installed bundle release %q: %w", *installedBundle.Release, err)
 		}
 		installedVersionRelease = &bundle.VersionRelease{
 			Version: version,