-
Notifications
You must be signed in to change notification settings - Fork 68
🐛 Add support for build metadata precedence in bundle version comparison #2273
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,129 @@ | ||
| package bundle | ||
|
|
||
| import ( | ||
| "errors" | ||
| "fmt" | ||
| "strings" | ||
|
|
||
| bsemver "github.com/blang/semver/v4" | ||
|
|
||
| slicesutil "github.com/operator-framework/operator-controller/internal/shared/util/slices" | ||
| ) | ||
|
|
||
| // NewLegacyRegistryV1VersionRelease parses a registry+v1 bundle version string and returns a | ||
| // VersionRelease. Some registry+v1 bundles utilize the build metadata field of the semver version | ||
| // as release information (a semver spec violation maintained for backward compatibility). When the | ||
| // bundle version includes build metadata that is parsable as a release, the returned | ||
| // VersionRelease has the build metadata extracted into the Release field, and the Version field | ||
| // has its Build metadata cleared. When the bundle version includes build metadata that is NOT | ||
| // parseable as a release, the returned VersionRelease has its Version set to the full semver | ||
| // version (with build metadata) and its Release left empty. | ||
| func NewLegacyRegistryV1VersionRelease(vStr string) (*VersionRelease, error) { | ||
| vers, err := bsemver.Parse(vStr) | ||
| if err != nil { | ||
| return nil, err | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is not back-compat. We should return a releaseless |
||
| } | ||
|
|
||
| vr := &VersionRelease{ | ||
| Version: vers, | ||
| } | ||
|
|
||
| rel, err := NewRelease(strings.Join(vr.Version.Build, ".")) | ||
| if err == nil { | ||
| // If the version build metadata parses successfully as a release | ||
| // then use it as a release and drop the build metadata | ||
| // | ||
| // If we don't parse the build metadata as a release successfully, | ||
| // that doesn't mean we have an invalid version. It just means | ||
| // that we have a valid semver version with valid build metadata, | ||
| // but no release value. In this case, we return a VersionRelease | ||
| // with: | ||
| // - Version: the full version (with build metadata) | ||
| // - Release: <empty> | ||
| vr.Release = rel | ||
| vr.Version.Build = nil | ||
| } | ||
| return vr, nil | ||
| } | ||
|
|
||
| type VersionRelease struct { | ||
| Version bsemver.Version | ||
| Release Release | ||
| } | ||
|
|
||
| // Compare compares two VersionRelease values. It returns: | ||
| // | ||
| // -1 if vr < other | ||
| // 0 if vr == other | ||
| // +1 if vr > other | ||
| // | ||
| // Comparison is done first by Version, then by Release if versions are equal. | ||
| func (vr *VersionRelease) Compare(other VersionRelease) int { | ||
joelanford marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| if vCmp := vr.Version.Compare(other.Version); vCmp != 0 { | ||
| return vCmp | ||
| } | ||
| return vr.Release.Compare(other.Release) | ||
| } | ||
|
|
||
| func (vr *VersionRelease) AsLegacyRegistryV1Version() bsemver.Version { | ||
| return bsemver.Version{ | ||
| Major: vr.Version.Major, | ||
| Minor: vr.Version.Minor, | ||
| Patch: vr.Version.Patch, | ||
| Pre: vr.Version.Pre, | ||
| Build: slicesutil.Map(vr.Release, func(i bsemver.PRVersion) string { return i.String() }), | ||
| } | ||
| } | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @joelanford it might have a small edge case here. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Maybe related: #2273 (comment) |
||
|
|
||
| type Release []bsemver.PRVersion | ||
|
|
||
| // Compare compares two Release values. It returns: | ||
| // | ||
| // -1 if r < other | ||
| // 0 if r == other | ||
| // +1 if r > other | ||
| // | ||
| // Comparison is done segment by segment from left to right. Numeric segments are | ||
| // compared numerically, and alphanumeric segments are compared lexically in ASCII | ||
| // sort order. A shorter release is considered less than a longer release if all | ||
| // corresponding segments are equal. | ||
| func (r Release) Compare(other Release) int { | ||
pedjak marked this conversation as resolved.
Show resolved
Hide resolved
pedjak marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| if len(r) == 0 && len(other) > 0 { | ||
| return -1 | ||
| } | ||
| if len(other) == 0 && len(r) > 0 { | ||
| return 1 | ||
| } | ||
| a := bsemver.Version{Pre: r} | ||
| b := bsemver.Version{Pre: other} | ||
| return a.Compare(b) | ||
| } | ||
|
|
||
| // NewRelease parses a release string into a Release. The release string should be | ||
| // a dot-separated sequence of non-empty identifiers, where each identifier contains | ||
| // only ASCII alphanumerics and hyphens [0-9A-Za-z-]. Numeric identifiers (those | ||
| // containing only digits) must not have leading zeros. An empty string returns a nil | ||
| // Release. Returns an error if any segment is invalid. | ||
| func NewRelease(relStr string) (Release, error) { | ||
pedjak marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| if relStr == "" { | ||
| return nil, nil | ||
| } | ||
|
|
||
| var ( | ||
| segments = strings.Split(relStr, ".") | ||
| r = make(Release, 0, len(segments)) | ||
| errs []error | ||
| ) | ||
| for i, segment := range segments { | ||
| prVer, err := bsemver.NewPRVersion(segment) | ||
| if err != nil { | ||
| errs = append(errs, fmt.Errorf("segment %d: %v", i, err)) | ||
| continue | ||
| } | ||
| r = append(r, prVer) | ||
| } | ||
| if err := errors.Join(errs...); err != nil { | ||
| return nil, fmt.Errorf("invalid release %q: %v", relStr, err) | ||
| } | ||
| return r, nil | ||
| } | ||
Uh oh!
There was an error while loading. Please reload this page.