From 4519cc61bc906ccca82e94112e83dec8d64b1b06 Mon Sep 17 00:00:00 2001 From: Rashmi Gottipati Date: Thu, 9 Apr 2026 16:37:11 -0400 Subject: [PATCH 1/2] adds support for parsing explicit `pkg.Release` field Signed-off-by: Rashmi Gottipati --- api/v1/clusterextension_types.go | 11 ++ applyconfigurations/api/v1/bundlemetadata.go | 15 ++ docs/api-reference/olmv1-api-reference.md | 1 + helm/experimental.yaml | 1 + ...peratorframework.io_clusterextensions.yaml | 14 ++ ...peratorframework.io_clusterextensions.yaml | 14 ++ .../operator-controller/bundleutil/bundle.go | 65 ++++++--- .../bundleutil/bundle_test.go | 137 +++++++++++++++++- .../catalogmetadata/filter/successors.go | 38 ++++- .../catalogmetadata/filter/successors_test.go | 20 ++- .../controllers/boxcutter_reconcile_steps.go | 2 + .../clusterextension_controller.go | 1 + .../clusterextension_controller_test.go | 2 + .../clusterextension_reconcile_steps.go | 11 +- .../operator-controller/features/features.go | 12 ++ internal/operator-controller/labels/labels.go | 6 + manifests/experimental-e2e.yaml | 15 ++ manifests/experimental.yaml | 15 ++ manifests/standard-e2e.yaml | 14 ++ manifests/standard.yaml | 14 ++ 20 files changed, 371 insertions(+), 37 deletions(-) diff --git a/api/v1/clusterextension_types.go b/api/v1/clusterextension_types.go index cf5946a408..9fa524e15a 100644 --- a/api/v1/clusterextension_types.go +++ b/api/v1/clusterextension_types.go @@ -466,6 +466,17 @@ type BundleMetadata struct { // +required // +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. + // 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. + // + // +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"` } // RevisionStatus defines the observed state of a ClusterObjectSet. diff --git a/applyconfigurations/api/v1/bundlemetadata.go b/applyconfigurations/api/v1/bundlemetadata.go index acf9d152f8..d61ffb00f6 100644 --- a/applyconfigurations/api/v1/bundlemetadata.go +++ b/applyconfigurations/api/v1/bundlemetadata.go @@ -29,6 +29,13 @@ type BundleMetadataApplyConfiguration struct { // version is required and references the version that this bundle represents. // It follows the semantic versioning standard as defined in https://semver.org/. Version *string `json:"version,omitempty"` + // 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. + Release *string `json:"release,omitempty"` } // BundleMetadataApplyConfiguration constructs a declarative configuration of the BundleMetadata type for use with @@ -52,3 +59,11 @@ func (b *BundleMetadataApplyConfiguration) WithVersion(value string) *BundleMeta b.Version = &value return b } + +// WithRelease sets the Release field in the declarative configuration to the given value +// and returns the receiver, so that objects can be built by chaining "With" function invocations. +// If called multiple times, the Release field is set to the value of the last call. +func (b *BundleMetadataApplyConfiguration) WithRelease(value string) *BundleMetadataApplyConfiguration { + b.Release = &value + return b +} diff --git a/docs/api-reference/olmv1-api-reference.md b/docs/api-reference/olmv1-api-reference.md index 5b5eeb2361..e9a8090ccd 100644 --- a/docs/api-reference/olmv1-api-reference.md +++ b/docs/api-reference/olmv1-api-reference.md @@ -67,6 +67,7 @@ _Appears in:_ | --- | --- | --- | --- | | `name` _string_ | name is required and follows the DNS subdomain standard as defined in [RFC 1123].
It must contain only lowercase alphanumeric characters, hyphens (-) or periods (.),
start and end with an alphanumeric character, and be no longer than 253 characters. | | Required: \{\}
| | `version` _string_ | version is required and references the version that this bundle represents.
It follows the semantic versioning standard as defined in https://semver.org/. | | Required: \{\}
| +| `release` _string_ | 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. | | Optional: \{\}
| #### CRDUpgradeSafetyEnforcement diff --git a/helm/experimental.yaml b/helm/experimental.yaml index b158389d48..05bb45f4d8 100644 --- a/helm/experimental.yaml +++ b/helm/experimental.yaml @@ -14,6 +14,7 @@ options: - HelmChartSupport - BoxcutterRuntime - DeploymentConfig + - BundleReleaseSupport disabled: - WebhookProviderOpenshiftServiceCA # List of enabled experimental features for catalogd diff --git a/helm/olmv1/base/operator-controller/crd/experimental/olm.operatorframework.io_clusterextensions.yaml b/helm/olmv1/base/operator-controller/crd/experimental/olm.operatorframework.io_clusterextensions.yaml index aebb9e72be..a1844b7551 100644 --- a/helm/olmv1/base/operator-controller/crd/experimental/olm.operatorframework.io_clusterextensions.yaml +++ b/helm/olmv1/base/operator-controller/crd/experimental/olm.operatorframework.io_clusterextensions.yaml @@ -688,6 +688,20 @@ 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. diff --git a/helm/olmv1/base/operator-controller/crd/standard/olm.operatorframework.io_clusterextensions.yaml b/helm/olmv1/base/operator-controller/crd/standard/olm.operatorframework.io_clusterextensions.yaml index aca133f732..8f2737a298 100644 --- a/helm/olmv1/base/operator-controller/crd/standard/olm.operatorframework.io_clusterextensions.yaml +++ b/helm/olmv1/base/operator-controller/crd/standard/olm.operatorframework.io_clusterextensions.yaml @@ -556,6 +556,20 @@ 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. diff --git a/internal/operator-controller/bundleutil/bundle.go b/internal/operator-controller/bundleutil/bundle.go index 2771c52593..b5e8b784dd 100644 --- a/internal/operator-controller/bundleutil/bundle.go +++ b/internal/operator-controller/bundleutil/bundle.go @@ -3,6 +3,7 @@ package bundleutil import ( "encoding/json" "fmt" + "strings" bsemver "github.com/blang/semver/v4" @@ -11,34 +12,62 @@ 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) { 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) } -// MetadataFor returns a BundleMetadata for the given bundle name and version. -func MetadataFor(bundleName string, bundleVersion bsemver.Version) ocv1.BundleMetadata { - return ocv1.BundleMetadata{ +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) + } + + // When BundleReleaseSupport is enabled and bundle has explicit release field, use it. + // Note: Build metadata is preserved here 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 + // interprets build metadata AS the release value for registry+v1 bundles. + if features.OperatorControllerFeatureGate.Enabled(features.BundleReleaseSupport) && 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/release. +func MetadataFor(bundleName string, vr bundle.VersionRelease) ocv1.BundleMetadata { + bm := ocv1.BundleMetadata{ Name: bundleName, - Version: bundleVersion.String(), + 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, ".") } + return bm } diff --git a/internal/operator-controller/bundleutil/bundle_test.go b/internal/operator-controller/bundleutil/bundle_test.go index 1cdabad054..e82af4d8c9 100644 --- a/internal/operator-controller/bundleutil/bundle_test.go +++ b/internal/operator-controller/bundleutil/bundle_test.go @@ -2,6 +2,7 @@ package bundleutil_test import ( "encoding/json" + "fmt" "testing" bsemver "github.com/blang/semver/v4" @@ -12,6 +13,7 @@ import ( "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/features" ) func TestGetVersionAndRelease(t *testing.T) { @@ -83,12 +85,145 @@ 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) } }) } } + +// TestGetVersionAndRelease_WithBundleReleaseSupport tests the feature-gated parsing behavior. +// Explicitly sets the gate to test both enabled and disabled paths. +func TestGetVersionAndRelease_WithBundleReleaseSupport(t *testing.T) { + t.Run("gate enabled - parses explicit release field", func(t *testing.T) { + // Enable the feature gate for this test + prevEnabled := features.OperatorControllerFeatureGate.Enabled(features.BundleReleaseSupport) + require.NoError(t, features.OperatorControllerFeatureGate.Set("BundleReleaseSupport=true")) + t.Cleanup(func() { + require.NoError(t, features.OperatorControllerFeatureGate.Set(fmt.Sprintf("BundleReleaseSupport=%t", prevEnabled))) + }) + + tests := []struct { + name string + pkgProperty *property.Property + wantVersionRelease *bundle.VersionRelease + wantErr bool + }{ + { + 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"), // Build metadata preserved - serves its proper semver purpose + 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 { + t.Run(tc.name, func(t *testing.T) { + bundle := declcfg.Bundle{ + Name: "test-bundle", + Properties: []property.Property{*tc.pkgProperty}, + } + + actual, err := bundleutil.GetVersionAndRelease(bundle) + if tc.wantErr { + require.Error(t, err) + } else { + require.NoError(t, err) + require.Equal(t, tc.wantVersionRelease, actual) + } + }) + } + }) + + t.Run("gate disabled - ignores explicit release field, uses build metadata", func(t *testing.T) { + // Disable the feature gate for this test + prevEnabled := features.OperatorControllerFeatureGate.Enabled(features.BundleReleaseSupport) + require.NoError(t, features.OperatorControllerFeatureGate.Set("BundleReleaseSupport=false")) + t.Cleanup(func() { + require.NoError(t, features.OperatorControllerFeatureGate.Set(fmt.Sprintf("BundleReleaseSupport=%t", prevEnabled))) + }) + + // When gate disabled, explicit release field is ignored and parsing falls back to legacy behavior + bundleWithExplicitRelease := declcfg.Bundle{ + Name: "test-bundle", + Properties: []property.Property{ + { + Type: property.TypePackage, + Value: json.RawMessage(`{"version": "1.0.0+2", "release": "999"}`), + }, + }, + } + + actual, err := bundleutil.GetVersionAndRelease(bundleWithExplicitRelease) + require.NoError(t, err) + + // Should parse build metadata (+2), not explicit release field (999) + expected := &bundle.VersionRelease{ + Version: bsemver.MustParse("1.0.0"), + Release: bundle.Release([]bsemver.PRVersion{ + {VersionNum: 2, IsNum: true}, + }), + } + require.Equal(t, expected, actual) + }) +} + +func TestMetadataFor(t *testing.T) { + t.Run("with release", func(t *testing.T) { + vr := bundle.VersionRelease{ + Version: bsemver.MustParse("1.0.0"), + Release: bundle.Release([]bsemver.PRVersion{{VersionNum: 2, IsNum: true}}), + } + 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) + }) + + t.Run("without release", func(t *testing.T) { + vr := bundle.VersionRelease{ + Version: bsemver.MustParse("1.0.0"), + Release: nil, + } + 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) + }) +} diff --git a/internal/operator-controller/catalogmetadata/filter/successors.go b/internal/operator-controller/catalogmetadata/filter/successors.go index 975c8cb39f..c0aa27e317 100644 --- a/internal/operator-controller/catalogmetadata/filter/successors.go +++ b/internal/operator-controller/catalogmetadata/filter/successors.go @@ -13,14 +13,36 @@ import ( ) 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) + // 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). + var installedVersionRelease *bundle.VersionRelease + var err error + + if installedBundle.Release != "" { + // Bundle has explicit release field - 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 + // semver like "1.0.0+git.abc+2". + version, err := bsemver.Parse(installedBundle.Version) + if err != nil { + return nil, fmt.Errorf("failed to parse installed bundle version %q: %w", installedBundle.Version, err) + } + release, err := bundle.NewRelease(installedBundle.Release) + if err != nil { + return nil, fmt.Errorf("failed to parse installed bundle release %q: %w", installedBundle.Release, err) + } + installedVersionRelease = &bundle.VersionRelease{ + Version: version, + Release: release, + } + } else { + // Legacy registry+v1: release embedded in version's build metadata + installedVersionRelease, err = bundle.NewLegacyRegistryV1VersionRelease(installedBundle.Version) + if err != nil { + return nil, fmt.Errorf("failed to get version and release of installed bundle: %w", err) + } } successorsPredicate, err := legacySuccessor(installedBundle, channels...) diff --git a/internal/operator-controller/catalogmetadata/filter/successors_test.go b/internal/operator-controller/catalogmetadata/filter/successors_test.go index d22a1fdb2f..441cab4e4b 100644 --- a/internal/operator-controller/catalogmetadata/filter/successors_test.go +++ b/internal/operator-controller/catalogmetadata/filter/successors_test.go @@ -4,7 +4,6 @@ import ( "slices" "testing" - bsemver "github.com/blang/semver/v4" "github.com/google/go-cmp/cmp" "github.com/google/go-cmp/cmp/cmpopts" "github.com/stretchr/testify/assert" @@ -14,11 +13,22 @@ import ( "github.com/operator-framework/operator-registry/alpha/property" 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/shared/util/filter" ) +// 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) bundle.VersionRelease { + vr, err := bundle.NewLegacyRegistryV1VersionRelease(versionStr) + if err != nil { + panic(err) + } + return *vr +} + func TestSuccessorsPredicate(t *testing.T) { const testPackageName = "test-package" channelSet := map[string]declcfg.Channel{ @@ -122,7 +132,7 @@ func TestSuccessorsPredicate(t *testing.T) { }{ { name: "respect replaces directive from catalog", - installedBundle: bundleutil.MetadataFor("test-package.v2.0.0", bsemver.MustParse("2.0.0")), + installedBundle: bundleutil.MetadataFor("test-package.v2.0.0", mustVersionRelease("2.0.0")), expectedResult: []declcfg.Bundle{ // Must only have two bundle: // - the one which replaces the current version @@ -133,7 +143,7 @@ func TestSuccessorsPredicate(t *testing.T) { }, { name: "respect skips directive from catalog", - installedBundle: bundleutil.MetadataFor("test-package.v2.2.1", bsemver.MustParse("2.2.1")), + installedBundle: bundleutil.MetadataFor("test-package.v2.2.1", mustVersionRelease("2.2.1")), expectedResult: []declcfg.Bundle{ // Must only have two bundle: // - the one which skips the current version @@ -144,7 +154,7 @@ func TestSuccessorsPredicate(t *testing.T) { }, { name: "respect skipRange directive from catalog", - installedBundle: bundleutil.MetadataFor("test-package.v2.3.0", bsemver.MustParse("2.3.0")), + installedBundle: bundleutil.MetadataFor("test-package.v2.3.0", mustVersionRelease("2.3.0")), expectedResult: []declcfg.Bundle{ // Must only have two bundle: // - the one which is skipRanges the current version @@ -155,7 +165,7 @@ func TestSuccessorsPredicate(t *testing.T) { }, { name: "installed bundle matcher is exact", - installedBundle: bundleutil.MetadataFor("test-package.v2.0.0+1", bsemver.MustParse("2.0.0+1")), + installedBundle: bundleutil.MetadataFor("test-package.v2.0.0+1", mustVersionRelease("2.0.0+1")), expectedResult: []declcfg.Bundle{ // Must only have two bundle: // - the one which is skips the current version diff --git a/internal/operator-controller/controllers/boxcutter_reconcile_steps.go b/internal/operator-controller/controllers/boxcutter_reconcile_steps.go index 63b8c7ddb2..965f0ac84c 100644 --- a/internal/operator-controller/controllers/boxcutter_reconcile_steps.go +++ b/internal/operator-controller/controllers/boxcutter_reconcile_steps.go @@ -69,6 +69,7 @@ func (d *BoxcutterRevisionStatesGetter) GetRevisionStates(ctx context.Context, e BundleMetadata: ocv1.BundleMetadata{ Name: rev.Annotations[labels.BundleNameKey], Version: rev.Annotations[labels.BundleVersionKey], + Release: rev.Annotations[labels.BundleReleaseKey], }, } @@ -104,6 +105,7 @@ func ApplyBundleWithBoxcutter(apply func(ctx context.Context, contentFS fs.FS, e labels.PackageNameKey: state.resolvedRevisionMetadata.Package, labels.BundleVersionKey: state.resolvedRevisionMetadata.Version, labels.BundleReferenceKey: state.resolvedRevisionMetadata.Image, + labels.BundleReleaseKey: state.resolvedRevisionMetadata.Release, } objLbls := map[string]string{ labels.OwnerKindKey: ocv1.ClusterExtensionKind, diff --git a/internal/operator-controller/controllers/clusterextension_controller.go b/internal/operator-controller/controllers/clusterextension_controller.go index 22b6768514..5251f50d59 100644 --- a/internal/operator-controller/controllers/clusterextension_controller.go +++ b/internal/operator-controller/controllers/clusterextension_controller.go @@ -540,6 +540,7 @@ func (d *HelmRevisionStatesGetter) GetRevisionStates(ctx context.Context, ext *o BundleMetadata: ocv1.BundleMetadata{ Name: rel.Labels[labels.BundleNameKey], Version: rel.Labels[labels.BundleVersionKey], + Release: rel.Labels[labels.BundleReleaseKey], }, } break diff --git a/internal/operator-controller/controllers/clusterextension_controller_test.go b/internal/operator-controller/controllers/clusterextension_controller_test.go index b86c500764..5eef430d72 100644 --- a/internal/operator-controller/controllers/clusterextension_controller_test.go +++ b/internal/operator-controller/controllers/clusterextension_controller_test.go @@ -2596,6 +2596,7 @@ func TestGetInstalledBundleHistory(t *testing.T) { labels.BundleNameKey: "test-ext", labels.BundleVersionKey: "1.0", labels.BundleReferenceKey: "bundle-ref", + labels.BundleReleaseKey: "2", }, }, }, @@ -2604,6 +2605,7 @@ func TestGetInstalledBundleHistory(t *testing.T) { BundleMetadata: ocv1.BundleMetadata{ Name: "test-ext", Version: "1.0", + Release: "2", }, Image: "bundle-ref", }, nil, diff --git a/internal/operator-controller/controllers/clusterextension_reconcile_steps.go b/internal/operator-controller/controllers/clusterextension_reconcile_steps.go index 48507a2b7c..3aaf04429c 100644 --- a/internal/operator-controller/controllers/clusterextension_reconcile_steps.go +++ b/internal/operator-controller/controllers/clusterextension_reconcile_steps.go @@ -191,11 +191,11 @@ 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()), + // MetadataFor accepts VersionRelease and populates both Version and Release fields. + // - With BundleReleaseSupport enabled + explicit pkg.Release: Release from pkg.Release field + // - Registry+v1 bundles (e.g., "1.0.0+2"): Release extracted from build metadata (e.g., "2") + // - Both result in separate Version and Release fields in BundleMetadata for roundtripping + BundleMetadata: bundleutil.MetadataFor(resolvedBundle.Name, *resolvedBundleVersion), } return nil, nil } @@ -413,6 +413,7 @@ func ApplyBundle(a Applier) ReconcileStepFunc { labels.PackageNameKey: state.resolvedRevisionMetadata.Package, labels.BundleVersionKey: state.resolvedRevisionMetadata.Version, labels.BundleReferenceKey: state.resolvedRevisionMetadata.Image, + labels.BundleReleaseKey: state.resolvedRevisionMetadata.Release, } objLbls := map[string]string{ labels.OwnerKindKey: ocv1.ClusterExtensionKind, diff --git a/internal/operator-controller/features/features.go b/internal/operator-controller/features/features.go index 0f99c1b28e..01e4fe4486 100644 --- a/internal/operator-controller/features/features.go +++ b/internal/operator-controller/features/features.go @@ -19,6 +19,7 @@ const ( HelmChartSupport featuregate.Feature = "HelmChartSupport" BoxcutterRuntime featuregate.Feature = "BoxcutterRuntime" DeploymentConfig featuregate.Feature = "DeploymentConfig" + BundleReleaseSupport featuregate.Feature = "BundleReleaseSupport" ) var operatorControllerFeatureGates = map[featuregate.Feature]featuregate.FeatureSpec{ @@ -89,6 +90,17 @@ var operatorControllerFeatureGates = map[featuregate.Feature]featuregate.Feature PreRelease: featuregate.Alpha, LockToDefault: false, }, + + // BundleReleaseSupport enables parsing of the explicit pkg.Release field + // from the olm.package property. When enabled, bundles with an explicit + // pkg.Release field have their release value parsed separately from the version, + // allowing build metadata to serve its proper semver purpose (e.g., git commit). + // When disabled, release values are parsed from version build metadata (registry+v1 legacy). + BundleReleaseSupport: { + Default: false, + PreRelease: featuregate.Alpha, + LockToDefault: false, + }, } var OperatorControllerFeatureGate featuregate.MutableFeatureGate = featuregate.NewFeatureGate() diff --git a/internal/operator-controller/labels/labels.go b/internal/operator-controller/labels/labels.go index 805c34c3b0..8872a462f2 100644 --- a/internal/operator-controller/labels/labels.go +++ b/internal/operator-controller/labels/labels.go @@ -28,6 +28,12 @@ const ( // associated with a ClusterObjectSet. BundleVersionKey = "olm.operatorframework.io/bundle-version" + // BundleReleaseKey is the label key used to record the bundle release value + // associated with a ClusterObjectSet. 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'). + BundleReleaseKey = "olm.operatorframework.io/bundle-release" + // BundleReferenceKey is the label key used to record an external reference // (such as an image or catalog reference) to the bundle for a // ClusterObjectSet. diff --git a/manifests/experimental-e2e.yaml b/manifests/experimental-e2e.yaml index 4a27cf2056..14945915ee 100644 --- a/manifests/experimental-e2e.yaml +++ b/manifests/experimental-e2e.yaml @@ -1300,6 +1300,20 @@ 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. @@ -2759,6 +2773,7 @@ spec: - --feature-gates=HelmChartSupport=true - --feature-gates=BoxcutterRuntime=true - --feature-gates=DeploymentConfig=true + - --feature-gates=BundleReleaseSupport=true - --feature-gates=WebhookProviderOpenshiftServiceCA=false - --tls-cert=/var/certs/tls.crt - --tls-key=/var/certs/tls.key diff --git a/manifests/experimental.yaml b/manifests/experimental.yaml index 9a8fa0b406..f22f8148ec 100644 --- a/manifests/experimental.yaml +++ b/manifests/experimental.yaml @@ -1261,6 +1261,20 @@ 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. @@ -2665,6 +2679,7 @@ spec: - --feature-gates=HelmChartSupport=true - --feature-gates=BoxcutterRuntime=true - --feature-gates=DeploymentConfig=true + - --feature-gates=BundleReleaseSupport=true - --feature-gates=WebhookProviderOpenshiftServiceCA=false - --tls-cert=/var/certs/tls.crt - --tls-key=/var/certs/tls.key diff --git a/manifests/standard-e2e.yaml b/manifests/standard-e2e.yaml index a3a72e89f9..4f6ca0dcdd 100644 --- a/manifests/standard-e2e.yaml +++ b/manifests/standard-e2e.yaml @@ -1168,6 +1168,20 @@ 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. diff --git a/manifests/standard.yaml b/manifests/standard.yaml index ad02f96e2e..f7022b47c9 100644 --- a/manifests/standard.yaml +++ b/manifests/standard.yaml @@ -1129,6 +1129,20 @@ 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. From d13ce7116e838245b34c9112cf22f587485e666d Mon Sep 17 00:00:00 2001 From: Rashmi Gottipati Date: Fri, 10 Apr 2026 21:55:25 -0400 Subject: [PATCH 2/2] address review feedback Signed-off-by: Rashmi Gottipati --- api/v1/clusterextension_types.go | 25 +++++++++++++------ api/v1/zz_generated.deepcopy.go | 9 +++++-- applyconfigurations/api/v1/bundlemetadata.go | 22 ++++++++++++---- docs/api-reference/olmv1-api-reference.md | 2 +- ...peratorframework.io_clusterextensions.yaml | 25 +++++++++++++------ ...peratorframework.io_clusterextensions.yaml | 14 ----------- .../bundle/versionrelease.go | 13 ++++++++++ .../operator-controller/bundleutil/bundle.go | 13 ++++++---- .../bundleutil/bundle_test.go | 17 +++++++++++-- .../catalogmetadata/filter/successors.go | 12 ++++----- .../controllers/boxcutter_reconcile_steps.go | 11 ++++++-- .../clusterextension_controller.go | 5 +++- .../clusterextension_controller_test.go | 3 ++- .../clusterextension_reconcile_steps.go | 6 ++++- manifests/experimental-e2e.yaml | 25 +++++++++++++------ manifests/experimental.yaml | 25 +++++++++++++------ manifests/standard-e2e.yaml | 14 ----------- manifests/standard.yaml | 14 ----------- 18 files changed, 156 insertions(+), 99 deletions(-) diff --git a/api/v1/clusterextension_types.go b/api/v1/clusterextension_types.go index 9fa524e15a..5e67d3b4eb 100644 --- a/api/v1/clusterextension_types.go +++ b/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"` + // + // +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. diff --git a/api/v1/zz_generated.deepcopy.go b/api/v1/zz_generated.deepcopy.go index 8d1cea0245..fd27626648 100644 --- a/api/v1/zz_generated.deepcopy.go +++ b/api/v1/zz_generated.deepcopy.go @@ -46,6 +46,11 @@ func (in *Assertion) DeepCopy() *Assertion { // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *BundleMetadata) DeepCopyInto(out *BundleMetadata) { *out = *in + if in.Release != nil { + in, out := &in.Release, &out.Release + *out = new(string) + **out = **in + } } // DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new BundleMetadata. @@ -312,7 +317,7 @@ func (in *ClusterExtensionInstallConfig) DeepCopy() *ClusterExtensionInstallConf // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *ClusterExtensionInstallStatus) DeepCopyInto(out *ClusterExtensionInstallStatus) { *out = *in - out.Bundle = in.Bundle + in.Bundle.DeepCopyInto(&out.Bundle) } // DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new ClusterExtensionInstallStatus. @@ -397,7 +402,7 @@ func (in *ClusterExtensionStatus) DeepCopyInto(out *ClusterExtensionStatus) { if in.Install != nil { in, out := &in.Install, &out.Install *out = new(ClusterExtensionInstallStatus) - **out = **in + (*in).DeepCopyInto(*out) } if in.ActiveRevisions != nil { in, out := &in.ActiveRevisions, &out.ActiveRevisions diff --git a/applyconfigurations/api/v1/bundlemetadata.go b/applyconfigurations/api/v1/bundlemetadata.go index d61ffb00f6..f6ba9dfdab 100644 --- a/applyconfigurations/api/v1/bundlemetadata.go +++ b/applyconfigurations/api/v1/bundlemetadata.go @@ -29,12 +29,24 @@ type BundleMetadataApplyConfiguration struct { // version is required and references the version that this bundle represents. // It follows the semantic versioning standard as defined in https://semver.org/. Version *string `json:"version,omitempty"` - // 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. + // + // Release *string `json:"release,omitempty"` } diff --git a/docs/api-reference/olmv1-api-reference.md b/docs/api-reference/olmv1-api-reference.md index e9a8090ccd..48c7417fab 100644 --- a/docs/api-reference/olmv1-api-reference.md +++ b/docs/api-reference/olmv1-api-reference.md @@ -67,7 +67,7 @@ _Appears in:_ | --- | --- | --- | --- | | `name` _string_ | name is required and follows the DNS subdomain standard as defined in [RFC 1123].
It must contain only lowercase alphanumeric characters, hyphens (-) or periods (.),
start and end with an alphanumeric character, and be no longer than 253 characters. | | Required: \{\}
| | `version` _string_ | version is required and references the version that this bundle represents.
It follows the semantic versioning standard as defined in https://semver.org/. | | Required: \{\}
| -| `release` _string_ | 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. | | Optional: \{\}
| +| `release` _string_ | 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 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: \{\}
| #### CRDUpgradeSafetyEnforcement diff --git a/helm/olmv1/base/operator-controller/crd/experimental/olm.operatorframework.io_clusterextensions.yaml b/helm/olmv1/base/operator-controller/crd/experimental/olm.operatorframework.io_clusterextensions.yaml index a1844b7551..541b6c6424 100644 --- a/helm/olmv1/base/operator-controller/crd/experimental/olm.operatorframework.io_clusterextensions.yaml +++ b/helm/olmv1/base/operator-controller/crd/experimental/olm.operatorframework.io_clusterextensions.yaml @@ -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: |- diff --git a/helm/olmv1/base/operator-controller/crd/standard/olm.operatorframework.io_clusterextensions.yaml b/helm/olmv1/base/operator-controller/crd/standard/olm.operatorframework.io_clusterextensions.yaml index 8f2737a298..aca133f732 100644 --- a/helm/olmv1/base/operator-controller/crd/standard/olm.operatorframework.io_clusterextensions.yaml +++ b/helm/olmv1/base/operator-controller/crd/standard/olm.operatorframework.io_clusterextensions.yaml @@ -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. diff --git a/internal/operator-controller/bundle/versionrelease.go b/internal/operator-controller/bundle/versionrelease.go index 03fbc8e507..48ab9d0a16 100644 --- a/internal/operator-controller/bundle/versionrelease.go +++ b/internal/operator-controller/bundle/versionrelease.go @@ -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 diff --git a/internal/operator-controller/bundleutil/bundle.go b/internal/operator-controller/bundleutil/bundle.go index b5e8b784dd..21085a3489 100644 --- a/internal/operator-controller/bundleutil/bundle.go +++ b/internal/operator-controller/bundleutil/bundle.go @@ -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 } diff --git a/internal/operator-controller/bundleutil/bundle_test.go b/internal/operator-controller/bundleutil/bundle_test.go index e82af4d8c9..89b74387af 100644 --- a/internal/operator-controller/bundleutil/bundle_test.go +++ b/internal/operator-controller/bundleutil/bundle_test.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) }) } diff --git a/internal/operator-controller/catalogmetadata/filter/successors.go b/internal/operator-controller/catalogmetadata/filter/successors.go index c0aa27e317..2732750d7a 100644 --- a/internal/operator-controller/catalogmetadata/filter/successors.go +++ b/internal/operator-controller/catalogmetadata/filter/successors.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, diff --git a/internal/operator-controller/controllers/boxcutter_reconcile_steps.go b/internal/operator-controller/controllers/boxcutter_reconcile_steps.go index 965f0ac84c..dc64017525 100644 --- a/internal/operator-controller/controllers/boxcutter_reconcile_steps.go +++ b/internal/operator-controller/controllers/boxcutter_reconcile_steps.go @@ -69,9 +69,12 @@ func (d *BoxcutterRevisionStatesGetter) GetRevisionStates(ctx context.Context, e BundleMetadata: ocv1.BundleMetadata{ Name: rev.Annotations[labels.BundleNameKey], Version: rev.Annotations[labels.BundleVersionKey], - Release: rev.Annotations[labels.BundleReleaseKey], }, } + // Only set Release if the annotation key exists (to distinguish "not set" from "explicitly empty") + if releaseValue, ok := rev.Annotations[labels.BundleReleaseKey]; ok { + rm.Release = &releaseValue + } if apimeta.IsStatusConditionTrue(rev.Status.Conditions, ocv1.ClusterObjectSetTypeSucceeded) { rs.Installed = rm @@ -100,12 +103,16 @@ func MigrateStorage(m StorageMigrator) ReconcileStepFunc { func ApplyBundleWithBoxcutter(apply func(ctx context.Context, contentFS fs.FS, ext *ocv1.ClusterExtension, objectLabels, revisionAnnotations map[string]string) (bool, string, error)) ReconcileStepFunc { return func(ctx context.Context, state *reconcileState, ext *ocv1.ClusterExtension) (*ctrl.Result, error) { l := log.FromContext(ctx) + releaseValue := "" + if state.resolvedRevisionMetadata.Release != nil { + releaseValue = *state.resolvedRevisionMetadata.Release + } revisionAnnotations := map[string]string{ labels.BundleNameKey: state.resolvedRevisionMetadata.Name, labels.PackageNameKey: state.resolvedRevisionMetadata.Package, labels.BundleVersionKey: state.resolvedRevisionMetadata.Version, labels.BundleReferenceKey: state.resolvedRevisionMetadata.Image, - labels.BundleReleaseKey: state.resolvedRevisionMetadata.Release, + labels.BundleReleaseKey: releaseValue, } objLbls := map[string]string{ labels.OwnerKindKey: ocv1.ClusterExtensionKind, diff --git a/internal/operator-controller/controllers/clusterextension_controller.go b/internal/operator-controller/controllers/clusterextension_controller.go index 5251f50d59..6645d392f3 100644 --- a/internal/operator-controller/controllers/clusterextension_controller.go +++ b/internal/operator-controller/controllers/clusterextension_controller.go @@ -540,9 +540,12 @@ func (d *HelmRevisionStatesGetter) GetRevisionStates(ctx context.Context, ext *o BundleMetadata: ocv1.BundleMetadata{ Name: rel.Labels[labels.BundleNameKey], Version: rel.Labels[labels.BundleVersionKey], - Release: rel.Labels[labels.BundleReleaseKey], }, } + // Only set Release if the label key exists (to distinguish "not set" from "explicitly empty") + if releaseValue, ok := rel.Labels[labels.BundleReleaseKey]; ok { + rs.Installed.Release = &releaseValue + } break } } diff --git a/internal/operator-controller/controllers/clusterextension_controller_test.go b/internal/operator-controller/controllers/clusterextension_controller_test.go index 5eef430d72..2726fe7c8c 100644 --- a/internal/operator-controller/controllers/clusterextension_controller_test.go +++ b/internal/operator-controller/controllers/clusterextension_controller_test.go @@ -22,6 +22,7 @@ import ( "k8s.io/apimachinery/pkg/types" "k8s.io/apimachinery/pkg/util/rand" "k8s.io/client-go/kubernetes/fake" + "k8s.io/utils/ptr" ctrl "sigs.k8s.io/controller-runtime" "sigs.k8s.io/controller-runtime/pkg/client" crfinalizer "sigs.k8s.io/controller-runtime/pkg/finalizer" @@ -2605,7 +2606,7 @@ func TestGetInstalledBundleHistory(t *testing.T) { BundleMetadata: ocv1.BundleMetadata{ Name: "test-ext", Version: "1.0", - Release: "2", + Release: ptr.To("2"), }, Image: "bundle-ref", }, nil, diff --git a/internal/operator-controller/controllers/clusterextension_reconcile_steps.go b/internal/operator-controller/controllers/clusterextension_reconcile_steps.go index 3aaf04429c..287afb4e05 100644 --- a/internal/operator-controller/controllers/clusterextension_reconcile_steps.go +++ b/internal/operator-controller/controllers/clusterextension_reconcile_steps.go @@ -408,12 +408,16 @@ func UnpackBundle(i imageutil.Puller, cache imageutil.Cache) ReconcileStepFunc { func ApplyBundle(a Applier) ReconcileStepFunc { return func(ctx context.Context, state *reconcileState, ext *ocv1.ClusterExtension) (*ctrl.Result, error) { l := log.FromContext(ctx) + releaseValue := "" + if state.resolvedRevisionMetadata.Release != nil { + releaseValue = *state.resolvedRevisionMetadata.Release + } revisionAnnotations := map[string]string{ labels.BundleNameKey: state.resolvedRevisionMetadata.Name, labels.PackageNameKey: state.resolvedRevisionMetadata.Package, labels.BundleVersionKey: state.resolvedRevisionMetadata.Version, labels.BundleReferenceKey: state.resolvedRevisionMetadata.Image, - labels.BundleReleaseKey: state.resolvedRevisionMetadata.Release, + labels.BundleReleaseKey: releaseValue, } objLbls := map[string]string{ labels.OwnerKindKey: ocv1.ClusterExtensionKind, diff --git a/manifests/experimental-e2e.yaml b/manifests/experimental-e2e.yaml index 14945915ee..cd3b1b7a02 100644 --- a/manifests/experimental-e2e.yaml +++ b/manifests/experimental-e2e.yaml @@ -1302,17 +1302,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: |- diff --git a/manifests/experimental.yaml b/manifests/experimental.yaml index f22f8148ec..9ae4a5348c 100644 --- a/manifests/experimental.yaml +++ b/manifests/experimental.yaml @@ -1263,17 +1263,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: |- diff --git a/manifests/standard-e2e.yaml b/manifests/standard-e2e.yaml index 4f6ca0dcdd..a3a72e89f9 100644 --- a/manifests/standard-e2e.yaml +++ b/manifests/standard-e2e.yaml @@ -1168,20 +1168,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. diff --git a/manifests/standard.yaml b/manifests/standard.yaml index f7022b47c9..ad02f96e2e 100644 --- a/manifests/standard.yaml +++ b/manifests/standard.yaml @@ -1129,20 +1129,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.