bundle version increased type safety / serialization support by grokspawn · Pull Request #1938 · operator-framework/operator-registry

grokspawn · 2026-03-23T20:37:38Z

Description of the change:
This is a deliberate re-scoping of the CompositeVersion type to co-locate all the bits of dependency on use of the underlying blang semver PRVersion slice, by making this an explicit type.
The original implementation was evaluating the tradeoffs between re-use of the semver.Parse method versus re-implementing just the semver.org pre-release version info for validation.

api-diff is intended to fail here, and will need to be overridden (since we are un-exporting the fields to make this an opaque type).

Motivation for the change:

Reviewer Checklist

Implementation matches the proposed design, or proposal is updated to match implementation
Sufficient unit test coverage
Sufficient end-to-end test coverage
Docs updated or added to /docs
Commit messages sensible and descriptive

Signed-off-by: grokspawn <jordan@nimblewidget.com>

openshift-ci · 2026-03-23T20:37:42Z

Skipping CI for Draft Pull Request.
If you want CI signal for your change, please convert it to an actual PR.
You can still manually trigger a test run with /test all

codecov · 2026-03-23T20:42:02Z

Codecov Report

❌ Patch coverage is 96.77419% with 1 line in your changes missing coverage. Please review.
✅ Project coverage is 57.74%. Comparing base (6ea0f39) to head (ef468c4).
⚠️ Report is 4 commits behind head on master.

Files with missing lines	Patch %	Lines
alpha/declcfg/declcfg.go	96.77%	0 Missing and 1 partial ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #1938      +/-   ##
==========================================
+ Coverage   57.50%   57.74%   +0.24%     
==========================================
  Files         139      139              
  Lines       13339    13382      +43     
==========================================
+ Hits         7671     7728      +57     
+ Misses       4486     4468      -18     
- Partials     1182     1186       +4

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Signed-off-by: grokspawn <jordan@nimblewidget.com>

tmshort · 2026-03-26T18:14:13Z

alpha/declcfg/declcfg.go

+
+	// Validate against CRD constraint from operators.coreos.com/v1alpha1 ClusterServiceVersion
+	// Maximum length of 20 characters
+	if len(relStr) > 20 {


Is there a value we can get that isn't a magic number?

There isn't one defined presently, but I'm happy to define a constant here for it. The reference is in the type here, but we're deliberately not using that type in this repo (to de-couple v0 & v1 uses), so we just need to be consistent.

tmshort · 2026-03-26T18:22:39Z

/approve

Signed-off-by: grokspawn <jordan@nimblewidget.com>

tmshort · 2026-03-26T19:39:52Z

/approve
Still approve after change

openshift-ci · 2026-03-26T19:39:59Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: tmshort

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Details

Needs approval from an approver in each of these files:

~~OWNERS~~ [tmshort]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

joelanford · 2026-03-26T19:42:56Z

alpha/declcfg/declcfg.go

+	SchemaChannel           = "olm.channel"
+	SchemaBundle            = "olm.bundle"
+	SchemaDeprecation       = "olm.deprecations"
+	VersionReleaseMaxLength = 20


Does this need to be exported? Do we expect callers to need to know it, or is it an internal detail of a validation that happens somewhere?

joelanford · 2026-03-26T19:46:34Z

alpha/declcfg/declcfg.go

+	b := semver.Version{Pre: other}
+	return a.Compare(b)
+}
+


I think you'll want a String() method on the Release type. I know @pedjak had added one in his operator-controller PR when we were thinking it would include the release plumbing.

joelanford · 2026-03-26T19:47:41Z

alpha/declcfg/declcfg.go

-		r, err = semver.Parse(fmt.Sprintf("0.0.0-%s", props.Packages[0].Release))
+		r, err = NewRelease(props.Packages[0].Release)
 		if err != nil {
 			return nil, fmt.Errorf("error parsing bundle %q release version %q: %v", b.Name, props.Packages[0].Release, err)


Suggested change

return nil, fmt.Errorf("error parsing bundle %q release version %q: %v", b.Name, props.Packages[0].Release, err)

return nil, fmt.Errorf("error parsing bundle %q release %q: %v", b.Name, props.Packages[0].Release, err)

joelanford · 2026-03-26T19:50:50Z

alpha/declcfg/declcfg.go

+	Version semver.Version `json:"version"`
+	Release Release        `json:"release,omitempty"`


Do we have specific needs around JSON marshaling/unmarshaling yet?

A few questions come to mind:

Should we not omit an empty release, so that an empty release is still explicitly marshalled?

Should the CompositeVersion be marshalled/unmarshalled as to/from an JSON string instead of a JSON object?

joelanford · 2026-03-26T19:52:50Z

alpha/declcfg/declcfg_test.go

For organizational purposes, WDYT about separate files:

compositeversion.go

compositeversion_test.go

It seems a little odd for this to be the only contents of a file calleddeclcfg_test.go

joelanford · 2026-03-26T19:56:41Z

alpha/declcfg/declcfg.go

+	return r, nil
+}
+
+type CompositeVersion struct {


Naming is the hardest thing in CompSci right? So here goes:

Should this be VersionRelease? My pitch is:

RPM uses NVR to abbreviate their name/version/release tuple. I could very easily see us defining an actual NameVersionRelease type that includes the package name.

Kubernetes has GroupVersion, GroupKind, and GroupVersionKind, which are structs that represent the respective combos of group, version, and kind.

ensure that new types minimize internal access and maximize safeness

17f2fa5

Signed-off-by: grokspawn <jordan@nimblewidget.com>

openshift-ci bot added the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Mar 23, 2026

grokspawn marked this pull request as ready for review March 23, 2026 20:56

openshift-ci bot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Mar 23, 2026

openshift-ci bot requested review from anik120 and pedjak March 23, 2026 20:56

grokspawn added the go-apidiff-override label Mar 23, 2026

provide serialization support

6121538

Signed-off-by: grokspawn <jordan@nimblewidget.com>

grokspawn force-pushed the release-types-play branch from 9df00dd to 6121538 Compare March 24, 2026 21:14

grokspawn mentioned this pull request Mar 25, 2026

🐛 Add bundle-version and package-name annotations to CER phase objects operator-framework/operator-controller#2580

Merged

4 tasks

grokspawn changed the title ~~ensure that new types minimize internal access and maximize safeness~~ bundle version increased type safety / serialization support Mar 25, 2026

tmshort reviewed Mar 26, 2026

View reviewed changes

openshift-ci bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Mar 26, 2026

review comments

ef468c4

Signed-off-by: grokspawn <jordan@nimblewidget.com>

joelanford reviewed Mar 26, 2026

View reviewed changes

	return nil, fmt.Errorf("error parsing bundle %q release version %q: %v", b.Name, props.Packages[0].Release, err)
	return nil, fmt.Errorf("error parsing bundle %q release %q: %v", b.Name, props.Packages[0].Release, err)

		Version semver.Version `json:"version"`
		Release Release `json:"release,omitempty"`

Conversation

grokspawn commented Mar 23, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

openshift-ci bot commented Mar 23, 2026

Uh oh!

codecov bot commented Mar 23, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

tmshort Mar 26, 2026

Choose a reason for hiding this comment

Uh oh!

grokspawn Mar 26, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

tmshort commented Mar 26, 2026

Uh oh!

tmshort commented Mar 26, 2026

Uh oh!

openshift-ci bot commented Mar 26, 2026

Uh oh!

joelanford Mar 26, 2026

Choose a reason for hiding this comment

Uh oh!

joelanford Mar 26, 2026

Choose a reason for hiding this comment

Uh oh!

joelanford Mar 26, 2026

Choose a reason for hiding this comment

Uh oh!

joelanford Mar 26, 2026

Choose a reason for hiding this comment

Uh oh!

joelanford Mar 26, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

joelanford Mar 26, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

grokspawn commented Mar 23, 2026 •

edited

Loading

codecov bot commented Mar 23, 2026 •

edited

Loading

grokspawn Mar 26, 2026 •

edited

Loading

joelanford Mar 26, 2026 •

edited

Loading