Skip to content
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

[BUG] OPM version parsing is not backward-compatible #910

Closed
sugarraysam opened this issue Jan 26, 2022 · 2 comments
Closed

[BUG] OPM version parsing is not backward-compatible #910

sugarraysam opened this issue Jan 26, 2022 · 2 comments

Comments

@sugarraysam
Copy link

In newer versions of OPM, bundle version is parsed using semver.Parse(...) from https://pkg.go.dev/github.com/blang/semver/v4#Parse.

This creates both a build error, but also an error when trying to list bundles from an older indexImage:

$ opm version
Version: version.Version{OpmVersion:"v1.19.5", GitCommit:"f478fdd", BuildDate:"2021-11-24T15:02:18Z", GoOs:"linux", GoArch:"amd64"}

$ opm alpha list bundles quay.io/osd-addons/rhods-index@sha256:dada41ed1f7be3b621f7ba42b596e1a3f150d9b78b16342064832a0517ffb498
WARN[0003] DEPRECATION NOTICE:
Sqlite-based catalogs and their related subcommands are deprecated. Support for
them will be removed in a future release. Please migrate your catalog workflows
to the new file-based catalog format.
FATA[0003] error parsing bundle "rhods-operator.1.1.0" version "v1.1.0": Invalid character(s) found in major number "v1"
(venv) exit 1

This error will be present when parsing either an indexImage metadata or the csv.Version bundle field.

In other words, semver does not support a prefix "v" character. At Red Hat, we rely extensively on OPM's tooling and this change makes many of our indexImages/bundleImages incompatible to use with newer OPM versions. Was this change intentional?

I am not sure when this breaking change was introduced, but I believe by simply changing this line with semver.ParseTolerant(...) we would solve the issue: https://github.com/operator-framework/operator-registry/blob/master/alpha/declcfg/declcfg_to_model.go#L124

@sugarraysam sugarraysam changed the title [BUG] OPM csv.Version parsing is not portable [BUG] OPM version parsing is not backward-compatible Jan 27, 2022
@joelanford
Copy link
Member

I think this is an unfortunate situation where we have not been careful to specify the required format and include regression tests.

Some parts of OPM (e.g. to generate a semver-based package manifest in replaces mode and to add a bundle with semver mode) and OLM (e.g. the VersionInRangePredicate in OLM's resolver) already expect to be able to parse the bundle version with semver.Parse, so the new opm alpha list commands, opm validate, and all of the new FBC-related code needs to use semver.Parse to ensure bundle version strings will work at runtime with other areas of opm and OLM.

I suspect that if you used semver-related features with bundles that specify a version with a "v" prefix, you'd encounter errors or unexpected behavior.

@sugarraysam
Copy link
Author

@joelanford thanks for your answer. We have maybe around ~20 faulty bundles so not the end of the world.

We can close this then as backporting won't help in the larger OLM ecosystem.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants