Skip to content

Commit d66afb6

Browse files
joelanfordclaude
andauthored
šŸ› Add support for build metadata precedence in bundle version comparison (#2273)
* Add support for build metadata precedence in bundle version comparison 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]> * Revert semantic changes to ClusterExtension version selection and improve 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 * Address code review feedback for build metadata precedence - Add documentation for AsLegacyRegistryV1Version method explaining the build metadata conversion logic - Fix AsLegacyRegistryV1Version to preserve original build metadata when Release field is not set - Add comprehensive test coverage for NewLegacyRegistryV1VersionRelease including edge cases for build metadata and release parsing - Add test coverage for AsLegacyRegistryV1Version conversion logic - Improve compare_test.go test structure with descriptive test names and assertion functions for better clarity - Add test case for non-release build metadata comparison --------- Co-authored-by: Claude <[email protected]>
1 parent 6aa4040 commit d66afb6

File tree

16 files changed

+913
-110
lines changed

16 files changed

+913
-110
lines changed
Lines changed: 137 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,137 @@
1+
package bundle
2+
3+
import (
4+
"errors"
5+
"fmt"
6+
"strings"
7+
8+
bsemver "github.com/blang/semver/v4"
9+
10+
slicesutil "github.com/operator-framework/operator-controller/internal/shared/util/slices"
11+
)
12+
13+
// NewLegacyRegistryV1VersionRelease parses a registry+v1 bundle version string and returns a
14+
// VersionRelease. Some registry+v1 bundles utilize the build metadata field of the semver version
15+
// as release information (a semver spec violation maintained for backward compatibility). When the
16+
// bundle version includes build metadata that is parsable as a release, the returned
17+
// VersionRelease has the build metadata extracted into the Release field, and the Version field
18+
// has its Build metadata cleared. When the bundle version includes build metadata that is NOT
19+
// parseable as a release, the returned VersionRelease has its Version set to the full semver
20+
// version (with build metadata) and its Release left empty.
21+
func NewLegacyRegistryV1VersionRelease(vStr string) (*VersionRelease, error) {
22+
vers, err := bsemver.Parse(vStr)
23+
if err != nil {
24+
return nil, err
25+
}
26+
27+
vr := &VersionRelease{
28+
Version: vers,
29+
}
30+
31+
rel, err := NewRelease(strings.Join(vr.Version.Build, "."))
32+
if err == nil {
33+
// If the version build metadata parses successfully as a release
34+
// then use it as a release and drop the build metadata
35+
//
36+
// If we don't parse the build metadata as a release successfully,
37+
// that doesn't mean we have an invalid version. It just means
38+
// that we have a valid semver version with valid build metadata,
39+
// but no release value. In this case, we return a VersionRelease
40+
// with:
41+
// - Version: the full version (with build metadata)
42+
// - Release: <empty>
43+
vr.Release = rel
44+
vr.Version.Build = nil
45+
}
46+
return vr, nil
47+
}
48+
49+
type VersionRelease struct {
50+
Version bsemver.Version
51+
Release Release
52+
}
53+
54+
// Compare compares two VersionRelease values. It returns:
55+
//
56+
// -1 if vr < other
57+
// 0 if vr == other
58+
// +1 if vr > other
59+
//
60+
// Comparison is done first by Version, then by Release if versions are equal.
61+
func (vr *VersionRelease) Compare(other VersionRelease) int {
62+
if vCmp := vr.Version.Compare(other.Version); vCmp != 0 {
63+
return vCmp
64+
}
65+
return vr.Release.Compare(other.Release)
66+
}
67+
68+
// AsLegacyRegistryV1Version converts a VersionRelease into a standard semver version.
69+
// If the VersionRelease's Release field is set, the returned semver version's build
70+
// metadata is set to the VersionRelease's Release. Otherwise, the build metadata is
71+
// set to the VersionRelease's Version field's build metadata.
72+
func (vr *VersionRelease) AsLegacyRegistryV1Version() bsemver.Version {
73+
v := bsemver.Version{
74+
Major: vr.Version.Major,
75+
Minor: vr.Version.Minor,
76+
Patch: vr.Version.Patch,
77+
Pre: vr.Version.Pre,
78+
Build: vr.Version.Build,
79+
}
80+
if len(vr.Release) > 0 {
81+
v.Build = slicesutil.Map(vr.Release, func(i bsemver.PRVersion) string { return i.String() })
82+
}
83+
return v
84+
}
85+
86+
type Release []bsemver.PRVersion
87+
88+
// Compare compares two Release values. It returns:
89+
//
90+
// -1 if r < other
91+
// 0 if r == other
92+
// +1 if r > other
93+
//
94+
// Comparison is done segment by segment from left to right. Numeric segments are
95+
// compared numerically, and alphanumeric segments are compared lexically in ASCII
96+
// sort order. A shorter release is considered less than a longer release if all
97+
// corresponding segments are equal.
98+
func (r Release) Compare(other Release) int {
99+
if len(r) == 0 && len(other) > 0 {
100+
return -1
101+
}
102+
if len(other) == 0 && len(r) > 0 {
103+
return 1
104+
}
105+
a := bsemver.Version{Pre: r}
106+
b := bsemver.Version{Pre: other}
107+
return a.Compare(b)
108+
}
109+
110+
// NewRelease parses a release string into a Release. The release string should be
111+
// a dot-separated sequence of non-empty identifiers, where each identifier contains
112+
// only ASCII alphanumerics and hyphens [0-9A-Za-z-]. Numeric identifiers (those
113+
// containing only digits) must not have leading zeros. An empty string returns a nil
114+
// Release. Returns an error if any segment is invalid.
115+
func NewRelease(relStr string) (Release, error) {
116+
if relStr == "" {
117+
return nil, nil
118+
}
119+
120+
var (
121+
segments = strings.Split(relStr, ".")
122+
r = make(Release, 0, len(segments))
123+
errs []error
124+
)
125+
for i, segment := range segments {
126+
prVer, err := bsemver.NewPRVersion(segment)
127+
if err != nil {
128+
errs = append(errs, fmt.Errorf("segment %d: %v", i, err))
129+
continue
130+
}
131+
r = append(r, prVer)
132+
}
133+
if err := errors.Join(errs...); err != nil {
134+
return nil, fmt.Errorf("invalid release %q: %v", relStr, err)
135+
}
136+
return r, nil
137+
}

0 commit comments

Comments
Ā (0)