🐛 Add support for build metadata precedence in bundle version comparison #2273

joelanford · 2025-10-16T17:22:08Z

Description

This change fixes an issue to ensure that operator-controller properly handles and compares registry+v1 bundle versions that include build metadata as specified in the semver version.

The intention is that we only treat build metadata as a release value for registry+v1 bundles, which already have this precedent set. If/when operator-controller gains support for new bundle types, the intention is to avoid continuing the practice (and semver violation) of treating version build metadata as comparable/orderable.

Key changes:

Introduce VersionRelease type combining semver version with release metadata
Update bundle comparison logic to consider build metadata when present
Add exact version matching for pinned versions with build metadata
Replace GetVersion with GetVersionAndRelease across the codebase
Ensure successors are determined based on exact version+release matching

This is particularly important for registry+v1 bundles that encode release information in the build metadata field of their version strings.

🤖 Generated with Claude Code

The bug this fixes was originally reported by Red Hat in https://issues.redhat.com/browse/OCPBUGS-60424

Reviewer Checklist

API Go Documentation
Tests: Unit Tests (and E2E Tests, if appropriate)
Comprehensive Commit Messages
Links to related GitHub Issue(s)

netlify · 2025-10-16T17:22:18Z

✅ Deploy Preview for olmv1 ready!

Name	Link
🔨 Latest commit	`6372625`
🔍 Latest deploy log	https://app.netlify.com/projects/olmv1/deploys/68fa6e27a8b2e60009c90d9a
😎 Deploy Preview	https://deploy-preview-2273--olmv1.netlify.app
📱 Preview on mobile	Toggle QR Code... Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify project configuration.

Copilot

Pull Request Overview

This PR fixes bundle version comparison logic to properly handle registry+v1 bundles that use build metadata in their version strings. The implementation introduces a VersionRelease type that treats build metadata as comparable/orderable release information for registry+v1 bundles (a semver spec violation that exists for backward compatibility). The fix ensures exact version+release matching for pinned versions and successor determination.

Key changes:

Introduced VersionRelease type combining semver version with release metadata parsed from build metadata
Updated comparison and filtering logic to use exact version+release matching instead of semver-only matching
Replaced GetVersion with GetVersionAndRelease throughout the codebase

Reviewed Changes

Copilot reviewed 12 out of 12 changed files in this pull request and generated 1 comment.

Show a summary per file

File	Description
internal/shared/util/slices/slices.go	Added generic Map utility function
internal/operator-controller/resolve/resolver.go	Updated interface to use VersionRelease instead of semver.Version
internal/operator-controller/resolve/catalog.go	Updated to use new comparison functions and moved Map function to shared utilities
internal/operator-controller/controllers/clusterextension_controller.go	Added conversion to legacy registry+v1 version format when creating bundle metadata
internal/operator-controller/catalogmetadata/filter/successors_test.go	Added test case for exact version matching with build metadata
internal/operator-controller/catalogmetadata/filter/successors.go	Implemented exact version+release matching for installed bundle comparison
internal/operator-controller/catalogmetadata/filter/bundle_predicates_test.go	Updated test to use new InSemverRange function
internal/operator-controller/catalogmetadata/filter/bundle_predicates.go	Replaced Masterminds semver with blang semver and added exact version matching
internal/operator-controller/catalogmetadata/compare/compare_test.go	Added tests for NewVersionRange and updated existing tests for new comparison logic
internal/operator-controller/catalogmetadata/compare/compare.go	Implemented NewVersionRange with exact build metadata matching and renamed ByVersion to ByVersionAndRelease
internal/operator-controller/bundleutil/bundle_test.go	Added comprehensive tests for VersionRelease comparison logic
internal/operator-controller/bundleutil/bundle.go	Introduced VersionRelease, Release types and legacy registry+v1 conversion logic

_{Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.}

internal/operator-controller/catalogmetadata/compare/compare.go

internal/operator-controller/bundleutil/bundle.go

internal/operator-controller/catalogmetadata/compare/compare.go

pedjak · 2025-10-16T22:02:51Z

internal/shared/util/slices/slices.go

@@ -0,0 +1,9 @@
+package slices
+
+func Map[I any, O any](in []I, f func(I) O) []O {


this function is available in https://github.com/samber/lo/blob/master/slice.go#L26 - how about to use it instead?

Not strongly opinionated. I do notice that that would be a new dependency to pull in to the project, so I could go either way.

Anyone else have opinions?

Hearing no other opinions, my vote is to leave as is. With this commit moving this function into a new package in the shared/utils space, hopefully future contributions will find this location to add new generic slice methods, and we can revisit the question again when there is more to be gained from pulling in a new dep.

I'm of two minds on the subject. The reference is to what looks like a personal library, which normally garners a 'no' from me. But it has a mess of contributions, releases, and some kind of community, which relaxes that somewhat.

I'd prefer that this package didn't reuse the same name as a standard library (https://pkg.go.dev/slices).
But otherwise I'm fine with having a minimal utility library here.

But it has a mess of contributions, releases, and some kind of community, which relaxes that somewhat.

The library has 20.5k stars which is really a lot and is a very good signal.

codecov · 2025-10-17T04:06:46Z

Codecov Report

❌ Patch coverage is 91.48936% with 8 lines in your changes missing coverage. Please review.
✅ Project coverage is 71.39%. Comparing base (6604f2a) to head (6372625).
⚠️ Report is 21 commits behind head on main.

Files with missing lines	Patch %	Lines
...ernal/operator-controller/bundle/versionrelease.go	93.61%	1 Missing and 2 partials ⚠️
...ator-controller/catalogmetadata/compare/compare.go	83.33%	1 Missing and 2 partials ⚠️
...or-controller/catalogmetadata/filter/successors.go	66.66%	1 Missing ⚠️
internal/operator-controller/resolve/resolver.go	0.00%	1 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #2273      +/-   ##
==========================================
- Coverage   72.73%   71.39%   -1.34%     
==========================================
  Files          89       92       +3     
  Lines        8747     7052    -1695     
==========================================
- Hits         6362     5035    -1327     
+ Misses       1968     1601     -367     
+ Partials      417      416       -1

Flag	Coverage Δ
e2e	`46.12% <68.08%> (+6.93%)`	⬆️
experimental-e2e	`14.59% <0.00%> (-31.29%)`	⬇️
unit	`58.91% <79.78%> (+0.95%)`	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ 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.

internal/operator-controller/bundle/versionrelease.go

pedjak · 2025-10-20T08:00:15Z

internal/operator-controller/bundleutil/bundle.go

 )

-func GetVersion(b declcfg.Bundle) (*bsemver.Version, error) {
+func GetVersionAndRelease(b declcfg.Bundle) (*bundle.VersionRelease, error) {


would it make sense to move this function to the new bundle/versionsrelease.go file?

Not strongly opinionated here, but I do I kinda like bundle/versionsrelease.go as a pure implementation of the type without the baggage of the various kinds of things that we might source or derive a VersionRelease from.

internal/operator-controller/bundleutil/bundle_test.go

internal/operator-controller/catalogmetadata/compare/compare.go

pedjak · 2025-10-20T08:29:49Z

internal/operator-controller/catalogmetadata/compare/compare.go

+// ByVersionAndRelease is a comparison function that compares bundles by
+// version and release. Bundles with lower versions/releases are
+// considered less than bundles with higher versions/releases.
+func ByVersionAndRelease(b1, b2 declcfg.Bundle) int {


nit: how about that we implement Compare method on declcfg.Bundle instead?

The reason we don't have that right now is because declcfg.Bundle does not actually embed its own deprecation information. So we can't directly compare declcfg.Bundle along deprecation lines.

I suppose we could essentially combine ByVersionAndRelease and ByDeprecationFunc into a single comparison function, but:

It still wouldn't be a method on declcfg.Bundle

It would still require extra declcfg.Deprecation metadata to do the comparison.

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

camilamacedo86 · 2025-10-21T07:25:11Z

This one seems related to the bug that @grokspawn has been looking on.
I think it would be nice get his review here.

joelanford · 2025-10-22T18:43:40Z

crd-diff is failing due to the change in the description of the spec.source.catalog.version field. Assuming folks agree to the updated API semantics, we either need to override the failure or update the crd-diff check to ignore the description change.

I'm personally inclined to specifically override so that future description changes continue to be flagged as potentially breaking so that those changes get extra scrutiny.

internal/operator-controller/bundle/versionrelease.go

pedjak

/lgtm a few more functions could be documented.

grokspawn · 2025-10-23T13:39:57Z

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

+                          metadata (e.g., "1.0.0+20230101"). This ensures an exact match of both
+                          the semver version and the release. If you specify a version without build
+                          metadata (e.g., "1.0.0"), it will match all bundles with that version
+                          regardless of their release information.


We'll need to update this once the bundle release version feature lands, since we will need to mention both approaches, including how one is preferred in future. Right now the bundle release version approach is to use [version]-[release] to express the attribute combo in a string (though the source of truth is always the discrete version and release fields in the olm.package property).

Perhaps this is better resolved in a doc where we can go into more depth?

This change fixes an issue to ensure that operator-controller properly handles and compares registry+v1 bundle versions that include build metadata as specified in the semver version. The intention is that we only treat build metadata as a release value for registry+v1 bundles, which already have this precedent set. If/when operator-controller gains support for new bundle types, the intention is to avoid continuing the practice (and semver violation) of treating version build metadata as comparable/orderable. Key changes: - Introduce VersionRelease type combining semver version with release metadata - Update bundle comparison logic to consider build metadata when present - Add exact version matching for pinned versions with build metadata - Replace GetVersion with GetVersionAndRelease across the codebase - Ensure successors are determined based on exact version+release matching This is particularly important for registry+v1 bundles that encode release information in the build metadata field of their version strings. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]>

pedjak

/lgtm

openshift-ci · 2025-10-23T15:22:39Z

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: pedjak
Once this PR has been reviewed and has the lgtm label, please assign kevinrizza for approval. For more information see the Code Review Process.

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

Needs approval from an approver in each of these files:

OWNERS

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

joelanford · 2025-10-23T16:11:36Z

internal/operator-controller/bundle/versionrelease.go

+func NewLegacyRegistryV1VersionRelease(vStr string) (*VersionRelease, error) {
+	vers, err := bsemver.Parse(vStr)
+	if err != nil {
+		return nil, err


This is not back-compat. We should return a releaseless VersionRelease in this case.

…rove VersionRelease parsing This commit reverts the user-facing semantic changes to the ClusterExtension version field that were introduced to support exact version pinning with build metadata. The version field now ignores build metadata when matching versions, consistent with semver specification. Additionally, this commit modifies the VersionRelease parsing logic to be more tolerant of semver versions whose build metadata is not a valid release. When build metadata cannot be parsed as a release, the full version (including build metadata) is preserved in the Version field, with an empty Release field. Changes include: - Removed documentation about pinning to exact versions with build metadata - Removed exactVersionMatcher logic that enforced build metadata equality - Updated NewLegacyRegistryV1VersionRelease to tolerate non-release build metadata - Updated test expectations to reflect new behavior

openshift-ci · 2025-10-23T18:04:24Z

New changes are detected. LGTM label has been removed.

Copilot

Pull Request Overview

Copilot reviewed 16 out of 16 changed files in this pull request and generated 1 comment.

_{Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.}

Copilot · 2025-10-23T18:04:59Z

internal/operator-controller/catalogmetadata/compare/compare.go

 func compareErrors(err1 error, err2 error) int {
 	if err1 != nil && err2 == nil {
-		return 1
+		return -1


The comment on line 80 states 'The semantic is that errors are "less than" non-errors' but the implementation has been inverted from the original. The comment should be updated to match the new implementation or the implementation should be corrected to match the comment.

joelanford requested a review from a team as a code owner October 16, 2025 17:22

joelanford requested review from Copilot and removed request for a team October 16, 2025 17:22

openshift-ci bot requested review from anik120 and dtfranz October 16, 2025 17:22

Copilot AI reviewed Oct 16, 2025

View reviewed changes

internal/operator-controller/catalogmetadata/compare/compare.go Show resolved Hide resolved

joelanford force-pushed the build-metadata-precedence branch 3 times, most recently from c2cd023 to d352bc0 Compare October 16, 2025 18:24

pedjak reviewed Oct 16, 2025

View reviewed changes

joelanford force-pushed the build-metadata-precedence branch from d352bc0 to e8571b6 Compare October 17, 2025 03:54

joelanford force-pushed the build-metadata-precedence branch from e8571b6 to b398c34 Compare October 17, 2025 17:24

pedjak reviewed Oct 20, 2025

View reviewed changes

joelanford force-pushed the build-metadata-precedence branch from b398c34 to 2cf7680 Compare October 22, 2025 18:05

pedjak reviewed Oct 23, 2025

View reviewed changes

internal/operator-controller/bundle/versionrelease.go Show resolved Hide resolved

pedjak reviewed Oct 23, 2025

View reviewed changes

internal/operator-controller/bundle/versionrelease.go Show resolved Hide resolved

pedjak approved these changes Oct 23, 2025

View reviewed changes

openshift-ci bot assigned pedjak Oct 23, 2025

openshift-ci bot added the lgtm Indicates that a PR is ready to be merged. label Oct 23, 2025

grokspawn reviewed Oct 23, 2025

View reviewed changes

joelanford force-pushed the build-metadata-precedence branch from 2cf7680 to 72c3a34 Compare October 23, 2025 14:45

openshift-ci bot removed the lgtm Indicates that a PR is ready to be merged. label Oct 23, 2025

pedjak approved these changes Oct 23, 2025

View reviewed changes

openshift-ci bot added the lgtm Indicates that a PR is ready to be merged. label Oct 23, 2025

joelanford commented Oct 23, 2025

View reviewed changes

Copilot AI review requested due to automatic review settings October 23, 2025 18:04

openshift-ci bot removed the lgtm Indicates that a PR is ready to be merged. label Oct 23, 2025

Copilot AI reviewed Oct 23, 2025

View reviewed changes

		@@ -0,0 +1,9 @@
		package slices

		func Map[I any, O any](in []I, f func(I) O) []O {

🐛 Add support for build metadata precedence in bundle version comparison #2273

Are you sure you want to change the base?

🐛 Add support for build metadata precedence in bundle version comparison #2273

Conversation

joelanford commented Oct 16, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Reviewer Checklist

Uh oh!

netlify bot commented Oct 16, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

✅ Deploy Preview for olmv1 ready!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Reviewed Changes

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

joelanford Oct 17, 2025 • 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!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

codecov bot commented Oct 17, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

camilamacedo86 commented Oct 21, 2025

Uh oh!

joelanford commented Oct 22, 2025

Uh oh!

Uh oh!

Uh oh!

pedjak left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

pedjak left a comment

Choose a reason for hiding this comment

Uh oh!

openshift-ci bot commented Oct 23, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

openshift-ci bot commented Oct 23, 2025

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Uh oh!

Copilot AI Oct 23, 2025

Choose a reason for hiding this comment

joelanford commented Oct 16, 2025 •

edited

Loading

netlify bot commented Oct 16, 2025 •

edited

Loading

joelanford Oct 17, 2025 •

edited

Loading

codecov bot commented Oct 17, 2025 •

edited

Loading