Skip to content

Commit 4519cc6

Browse files
adds support for parsing explicit pkg.Release field
Signed-off-by: Rashmi Gottipati <rgottipa@redhat.com>
1 parent b6dfd40 commit 4519cc6

File tree

20 files changed

+371
-37
lines changed

20 files changed

+371
-37
lines changed

api/v1/clusterextension_types.go

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -466,6 +466,17 @@ type BundleMetadata struct {
466466
// +required
467467
// +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"
468468
Version string `json:"version"`
469+
470+
// release is an optional field that references the release value for this bundle.
471+
// The release follows pre-release/build metadata syntax as defined in https://semver.org/,
472+
// consisting of dot-separated identifiers where numeric identifiers must not have leading zeros.
473+
// For bundles with explicit pkg.Release metadata, this field contains that release value.
474+
// For registry+v1 bundles, this field contains the release extracted from version's build metadata (e.g., '2' from '1.0.0+2').
475+
// This field may be omitted if there is no explicit release and the version contains no parseable build metadata.
476+
//
477+
// +optional
478+
// +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)"
479+
Release string `json:"release,omitempty"`
469480
}
470481

471482
// RevisionStatus defines the observed state of a ClusterObjectSet.

applyconfigurations/api/v1/bundlemetadata.go

Lines changed: 15 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

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

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -67,6 +67,7 @@ _Appears in:_
6767
| --- | --- | --- | --- |
6868
| `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 /> |
6969
| `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 /> |
70+
| `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 /> |
7071

7172

7273
#### CRDUpgradeSafetyEnforcement

helm/experimental.yaml

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@ options:
1414
- HelmChartSupport
1515
- BoxcutterRuntime
1616
- DeploymentConfig
17+
- BundleReleaseSupport
1718
disabled:
1819
- WebhookProviderOpenshiftServiceCA
1920
# List of enabled experimental features for catalogd

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

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -688,6 +688,20 @@ spec:
688688
hyphens (-) or periods (.), start and end with an alphanumeric
689689
character, and be no longer than 253 characters
690690
rule: self.matches("^[a-z0-9]([-a-z0-9]*[a-z0-9])?(\\.[a-z0-9]([-a-z0-9]*[a-z0-9])?)*$")
691+
release:
692+
description: |-
693+
release is an optional field that references the release value for this bundle.
694+
The release follows pre-release/build metadata syntax as defined in https://semver.org/,
695+
consisting of dot-separated identifiers where numeric identifiers must not have leading zeros.
696+
For bundles with explicit pkg.Release metadata, this field contains that release value.
697+
For registry+v1 bundles, this field contains the release extracted from version's build metadata (e.g., '2' from '1.0.0+2').
698+
This field may be omitted if there is no explicit release and the version contains no parseable build metadata.
699+
type: string
700+
x-kubernetes-validations:
701+
- message: release must be empty or well-formed pre-release/build
702+
metadata (dot-separated identifiers, numeric parts without
703+
leading zeros)
704+
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-]*))*$")
691705
version:
692706
description: |-
693707
version is required and references the version that this bundle represents.

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

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -556,6 +556,20 @@ spec:
556556
hyphens (-) or periods (.), start and end with an alphanumeric
557557
character, and be no longer than 253 characters
558558
rule: self.matches("^[a-z0-9]([-a-z0-9]*[a-z0-9])?(\\.[a-z0-9]([-a-z0-9]*[a-z0-9])?)*$")
559+
release:
560+
description: |-
561+
release is an optional field that references the release value for this bundle.
562+
The release follows pre-release/build metadata syntax as defined in https://semver.org/,
563+
consisting of dot-separated identifiers where numeric identifiers must not have leading zeros.
564+
For bundles with explicit pkg.Release metadata, this field contains that release value.
565+
For registry+v1 bundles, this field contains the release extracted from version's build metadata (e.g., '2' from '1.0.0+2').
566+
This field may be omitted if there is no explicit release and the version contains no parseable build metadata.
567+
type: string
568+
x-kubernetes-validations:
569+
- message: release must be empty or well-formed pre-release/build
570+
metadata (dot-separated identifiers, numeric parts without
571+
leading zeros)
572+
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-]*))*$")
559573
version:
560574
description: |-
561575
version is required and references the version that this bundle represents.

internal/operator-controller/bundleutil/bundle.go

Lines changed: 47 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@ package bundleutil
33
import (
44
"encoding/json"
55
"fmt"
6+
"strings"
67

78
bsemver "github.com/blang/semver/v4"
89

@@ -11,34 +12,62 @@ import (
1112

1213
ocv1 "github.com/operator-framework/operator-controller/api/v1"
1314
"github.com/operator-framework/operator-controller/internal/operator-controller/bundle"
15+
"github.com/operator-framework/operator-controller/internal/operator-controller/features"
16+
slicesutil "github.com/operator-framework/operator-controller/internal/shared/util/slices"
1417
)
1518

1619
func GetVersionAndRelease(b declcfg.Bundle) (*bundle.VersionRelease, error) {
1720
for _, p := range b.Properties {
1821
if p.Type == property.TypePackage {
19-
var pkg property.Package
20-
if err := json.Unmarshal(p.Value, &pkg); err != nil {
21-
return nil, fmt.Errorf("error unmarshalling package property: %w", err)
22-
}
23-
24-
// TODO: For now, we assume that all bundles are registry+v1 bundles.
25-
// In the future, when we support other bundle formats, we should stop
26-
// using the legacy mechanism (i.e. using build metadata in the version)
27-
// to determine the bundle's release.
28-
vr, err := bundle.NewLegacyRegistryV1VersionRelease(pkg.Version)
29-
if err != nil {
30-
return nil, err
31-
}
32-
return vr, nil
22+
return parseVersionRelease(p.Value)
3323
}
3424
}
3525
return nil, fmt.Errorf("no package property found in bundle %q", b.Name)
3626
}
3727

38-
// MetadataFor returns a BundleMetadata for the given bundle name and version.
39-
func MetadataFor(bundleName string, bundleVersion bsemver.Version) ocv1.BundleMetadata {
40-
return ocv1.BundleMetadata{
28+
func parseVersionRelease(pkgData json.RawMessage) (*bundle.VersionRelease, error) {
29+
var pkg property.Package
30+
if err := json.Unmarshal(pkgData, &pkg); err != nil {
31+
return nil, fmt.Errorf("error unmarshalling package property: %w", err)
32+
}
33+
34+
// When BundleReleaseSupport is enabled and bundle has explicit release field, use it.
35+
// Note: Build metadata is preserved here because with an explicit release field,
36+
// build metadata serves its proper semver purpose (e.g., git commit, build number).
37+
// In contrast, NewLegacyRegistryV1VersionRelease clears build metadata because it
38+
// interprets build metadata AS the release value for registry+v1 bundles.
39+
if features.OperatorControllerFeatureGate.Enabled(features.BundleReleaseSupport) && pkg.Release != "" {
40+
vers, err := bsemver.Parse(pkg.Version)
41+
if err != nil {
42+
return nil, fmt.Errorf("error parsing version %q: %w", pkg.Version, err)
43+
}
44+
rel, err := bundle.NewRelease(pkg.Release)
45+
if err != nil {
46+
return nil, fmt.Errorf("error parsing release %q: %w", pkg.Release, err)
47+
}
48+
return &bundle.VersionRelease{
49+
Version: vers,
50+
Release: rel,
51+
}, nil
52+
}
53+
54+
// Fall back to legacy registry+v1 behavior (release in build metadata)
55+
vr, err := bundle.NewLegacyRegistryV1VersionRelease(pkg.Version)
56+
if err != nil {
57+
return nil, err
58+
}
59+
return vr, nil
60+
}
61+
62+
// MetadataFor returns a BundleMetadata for the given bundle name and version/release.
63+
func MetadataFor(bundleName string, vr bundle.VersionRelease) ocv1.BundleMetadata {
64+
bm := ocv1.BundleMetadata{
4165
Name: bundleName,
42-
Version: bundleVersion.String(),
66+
Version: vr.Version.String(),
67+
}
68+
if len(vr.Release) > 0 {
69+
parts := slicesutil.Map(vr.Release, func(pr bsemver.PRVersion) string { return pr.String() })
70+
bm.Release = strings.Join(parts, ".")
4371
}
72+
return bm
4473
}

internal/operator-controller/bundleutil/bundle_test.go

Lines changed: 136 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@ package bundleutil_test
22

33
import (
44
"encoding/json"
5+
"fmt"
56
"testing"
67

78
bsemver "github.com/blang/semver/v4"
@@ -12,6 +13,7 @@ import (
1213

1314
"github.com/operator-framework/operator-controller/internal/operator-controller/bundle"
1415
"github.com/operator-framework/operator-controller/internal/operator-controller/bundleutil"
16+
"github.com/operator-framework/operator-controller/internal/operator-controller/features"
1517
)
1618

1719
func TestGetVersionAndRelease(t *testing.T) {
@@ -83,12 +85,145 @@ func TestGetVersionAndRelease(t *testing.T) {
8385
Properties: properties,
8486
}
8587

86-
_, err := bundleutil.GetVersionAndRelease(bundle)
88+
actual, err := bundleutil.GetVersionAndRelease(bundle)
8789
if tc.wantErr {
8890
require.Error(t, err)
8991
} else {
9092
require.NoError(t, err)
93+
require.Equal(t, tc.wantVersionRelease, actual)
9194
}
9295
})
9396
}
9497
}
98+
99+
// TestGetVersionAndRelease_WithBundleReleaseSupport tests the feature-gated parsing behavior.
100+
// Explicitly sets the gate to test both enabled and disabled paths.
101+
func TestGetVersionAndRelease_WithBundleReleaseSupport(t *testing.T) {
102+
t.Run("gate enabled - parses explicit release field", func(t *testing.T) {
103+
// Enable the feature gate for this test
104+
prevEnabled := features.OperatorControllerFeatureGate.Enabled(features.BundleReleaseSupport)
105+
require.NoError(t, features.OperatorControllerFeatureGate.Set("BundleReleaseSupport=true"))
106+
t.Cleanup(func() {
107+
require.NoError(t, features.OperatorControllerFeatureGate.Set(fmt.Sprintf("BundleReleaseSupport=%t", prevEnabled)))
108+
})
109+
110+
tests := []struct {
111+
name string
112+
pkgProperty *property.Property
113+
wantVersionRelease *bundle.VersionRelease
114+
wantErr bool
115+
}{
116+
{
117+
name: "explicit release field - takes precedence over build metadata",
118+
pkgProperty: &property.Property{
119+
Type: property.TypePackage,
120+
Value: json.RawMessage(`{"version": "1.0.0+ignored", "release": "2"}`),
121+
},
122+
wantVersionRelease: &bundle.VersionRelease{
123+
Version: bsemver.MustParse("1.0.0+ignored"), // Build metadata preserved - serves its proper semver purpose
124+
Release: bundle.Release([]bsemver.PRVersion{
125+
{VersionNum: 2, IsNum: true},
126+
}),
127+
},
128+
wantErr: false,
129+
},
130+
{
131+
name: "explicit release field - complex release",
132+
pkgProperty: &property.Property{
133+
Type: property.TypePackage,
134+
Value: json.RawMessage(`{"version": "2.1.0", "release": "1.alpha.3"}`),
135+
},
136+
wantVersionRelease: &bundle.VersionRelease{
137+
Version: bsemver.MustParse("2.1.0"),
138+
Release: bundle.Release([]bsemver.PRVersion{
139+
{VersionNum: 1, IsNum: true},
140+
{VersionStr: "alpha"},
141+
{VersionNum: 3, IsNum: true},
142+
}),
143+
},
144+
wantErr: false,
145+
},
146+
{
147+
name: "explicit release field - invalid release",
148+
pkgProperty: &property.Property{
149+
Type: property.TypePackage,
150+
Value: json.RawMessage(`{"version": "1.0.0", "release": "001"}`),
151+
},
152+
wantErr: true,
153+
},
154+
}
155+
156+
for _, tc := range tests {
157+
t.Run(tc.name, func(t *testing.T) {
158+
bundle := declcfg.Bundle{
159+
Name: "test-bundle",
160+
Properties: []property.Property{*tc.pkgProperty},
161+
}
162+
163+
actual, err := bundleutil.GetVersionAndRelease(bundle)
164+
if tc.wantErr {
165+
require.Error(t, err)
166+
} else {
167+
require.NoError(t, err)
168+
require.Equal(t, tc.wantVersionRelease, actual)
169+
}
170+
})
171+
}
172+
})
173+
174+
t.Run("gate disabled - ignores explicit release field, uses build metadata", func(t *testing.T) {
175+
// Disable the feature gate for this test
176+
prevEnabled := features.OperatorControllerFeatureGate.Enabled(features.BundleReleaseSupport)
177+
require.NoError(t, features.OperatorControllerFeatureGate.Set("BundleReleaseSupport=false"))
178+
t.Cleanup(func() {
179+
require.NoError(t, features.OperatorControllerFeatureGate.Set(fmt.Sprintf("BundleReleaseSupport=%t", prevEnabled)))
180+
})
181+
182+
// When gate disabled, explicit release field is ignored and parsing falls back to legacy behavior
183+
bundleWithExplicitRelease := declcfg.Bundle{
184+
Name: "test-bundle",
185+
Properties: []property.Property{
186+
{
187+
Type: property.TypePackage,
188+
Value: json.RawMessage(`{"version": "1.0.0+2", "release": "999"}`),
189+
},
190+
},
191+
}
192+
193+
actual, err := bundleutil.GetVersionAndRelease(bundleWithExplicitRelease)
194+
require.NoError(t, err)
195+
196+
// Should parse build metadata (+2), not explicit release field (999)
197+
expected := &bundle.VersionRelease{
198+
Version: bsemver.MustParse("1.0.0"),
199+
Release: bundle.Release([]bsemver.PRVersion{
200+
{VersionNum: 2, IsNum: true},
201+
}),
202+
}
203+
require.Equal(t, expected, actual)
204+
})
205+
}
206+
207+
func TestMetadataFor(t *testing.T) {
208+
t.Run("with release", func(t *testing.T) {
209+
vr := bundle.VersionRelease{
210+
Version: bsemver.MustParse("1.0.0"),
211+
Release: bundle.Release([]bsemver.PRVersion{{VersionNum: 2, IsNum: true}}),
212+
}
213+
result := bundleutil.MetadataFor("test-bundle", vr)
214+
require.Equal(t, "test-bundle", result.Name)
215+
require.Equal(t, "1.0.0", result.Version)
216+
require.Equal(t, "2", result.Release)
217+
})
218+
219+
t.Run("without release", func(t *testing.T) {
220+
vr := bundle.VersionRelease{
221+
Version: bsemver.MustParse("1.0.0"),
222+
Release: nil,
223+
}
224+
result := bundleutil.MetadataFor("test-bundle", vr)
225+
require.Equal(t, "test-bundle", result.Name)
226+
require.Equal(t, "1.0.0", result.Version)
227+
require.Empty(t, result.Release)
228+
})
229+
}

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

Lines changed: 30 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -13,14 +13,36 @@ import (
1313
)
1414

1515
func SuccessorsOf(installedBundle ocv1.BundleMetadata, channels ...declcfg.Channel) (filter.Predicate[declcfg.Bundle], error) {
16-
// TODO: We do not have an explicit field in our BundleMetadata for a bundle's release value.
17-
// Legacy registry+v1 bundles embed the release value inside their versions as build metadata
18-
// (in violation of the semver spec). If/when we add explicit release metadata to bundles and/or
19-
// we support a new bundle format, we need to revisit the assumption that all bundles are
20-
// registry+v1 and embed release in build metadata.
21-
installedVersionRelease, err := bundle.NewLegacyRegistryV1VersionRelease(installedBundle.Version)
22-
if err != nil {
23-
return nil, fmt.Errorf("failed to get version and release of installed bundle: %v", err)
16+
// Construct VersionRelease from BundleMetadata.
17+
// If the Release field is populated, parse version and release separately.
18+
// Otherwise, parse release from version build metadata (registry+v1 legacy format).
19+
var installedVersionRelease *bundle.VersionRelease
20+
var err error
21+
22+
if installedBundle.Release != "" {
23+
// Bundle has explicit release field - parse version and release from separate fields.
24+
// Note: We can't use NewLegacyRegistryV1VersionRelease here because the version might
25+
// already contain build metadata (e.g., "1.0.0+git.abc"), which serves its proper
26+
// semver purpose when using explicit pkg.Release. Concatenating would create invalid
27+
// semver like "1.0.0+git.abc+2".
28+
version, err := bsemver.Parse(installedBundle.Version)
29+
if err != nil {
30+
return nil, fmt.Errorf("failed to parse installed bundle version %q: %w", installedBundle.Version, err)
31+
}
32+
release, err := bundle.NewRelease(installedBundle.Release)
33+
if err != nil {
34+
return nil, fmt.Errorf("failed to parse installed bundle release %q: %w", installedBundle.Release, err)
35+
}
36+
installedVersionRelease = &bundle.VersionRelease{
37+
Version: version,
38+
Release: release,
39+
}
40+
} else {
41+
// Legacy registry+v1: release embedded in version's build metadata
42+
installedVersionRelease, err = bundle.NewLegacyRegistryV1VersionRelease(installedBundle.Version)
43+
if err != nil {
44+
return nil, fmt.Errorf("failed to get version and release of installed bundle: %w", err)
45+
}
2446
}
2547

2648
successorsPredicate, err := legacySuccessor(installedBundle, channels...)

0 commit comments

Comments
 (0)