Allows the `v` char in the version string #5343

cardil · 2021-11-06T11:47:54Z

Description of the change:

Fixes #5342

Checklist

If the pull request includes user-facing changes, extra documentation is required:

Add a new changelog fragment in changelog/fragments (see changelog/fragments/00-template.yaml)
Add or update relevant sections of the docs website in website/content/en/docs

varshaprasad96

/lgtm

internal/cmd/operator-sdk/generate/internal/genutil.go

asmacdo · 2021-11-08T19:50:07Z

@cardil thanks for the PR! Once the changelog fragment is added, we should be good to merge this :)

joelanford · 2021-11-10T18:29:14Z

Before we merge this, I just wanted to double check a few things.

Are we talking about the version derived from csv.spec.version?
If so, are we sure OLM supports versions that include a v prefix?

/hold

(feel free to unhold if the above questions are sufficiently answered)

jmrodri · 2022-01-14T20:50:40Z

/cc @jmrodri get an answer to Joe's comment above

varshaprasad96

Based on what Joe said, even if csv.Spec.Version accepts string with v, I think with this change we would still error here:

operator-sdk/internal/generate/clusterserviceversion/clusterserviceversion.go

Line 161 in 43b5394

if base.Spec.Version.Version, err = semver.Parse(g.Version); err != nil {

I think olm may accept spec.Version with v prefixed. Looks like csv.spec.Version is of type OperatorVersion, and atleast while Umarshalling it they sanitize the string. Nevertheless, it would also be helpful to have a test to verify if we need to change semver.Parse() to semver.ParseTolerant() while generating csv with this change.

internal/cmd/operator-sdk/generate/internal/genutil.go

openshift-bot · 2022-04-14T23:52:21Z

Issues go stale after 90d of inactivity.

Mark the issue as fresh by commenting /remove-lifecycle stale.
Stale issues rot after an additional 30d of inactivity and eventually close.
Exclude this issue from closing by commenting /lifecycle frozen.

If this issue is safe to close now please do so with /close.

/lifecycle stale

cardil · 2022-04-26T10:58:24Z

/remove-lifecycle stale

cardil · 2022-04-26T12:15:55Z

At @joelanford:

With the latest changes to this PR, I'm removing the v prefix if there is any. This means any other behavior of operator-sdk is unchanged.

joelanford · 2022-05-04T03:34:38Z

changelog/fragments/allow-v-in-version.yaml

+      Allows the `v` char as a prefix in version string. The `v` char in 
+      version string, is extremely common, for example the `git describe`
+      command produces an output like this: `v0.2.0-13-ga74dac1`.


Could you expand on (a) which versions are allowed to have the v prefix and (b) what commands will translate those versions to non-v-prefixed versions and (c) which output artifacts will drop the v prefix and which won't?

IIRC there's a handful of places where versions are specified by users and a separate handful where versions are written/pushed by the operator-sdk commands or the project Makefile.

Hope the new description is more informative. Please, re-evaluate @joelanford.

@joelanford please take a look once again. Thanks!

- Adding test to cover version validation - Stripping `v` prefix in version Signed-off-by: Chris Suszyński <ksuszyns@redhat.com>

…-v-in-version Conflicts fixed: * internal/cmd/operator-sdk/generate/bundle/bundle.go

cardil · 2022-08-25T12:04:29Z

Ping!

cardil · 2022-10-10T20:27:55Z

Can I get re-review?

varshaprasad96 · 2023-01-04T18:19:28Z

/approve
/lgtm
/ok-to-test
The change looks good to me. @joelanford any objections in merging this?

openshift-bot · 2023-04-05T01:01:01Z

Issues go stale after 90d of inactivity.

Mark the issue as fresh by commenting /remove-lifecycle stale.
Stale issues rot after an additional 30d of inactivity and eventually close.
Exclude this issue from closing by commenting /lifecycle frozen.

If this issue is safe to close now please do so with /close.

/lifecycle stale

openshift-bot · 2023-05-05T08:30:43Z

Stale issues rot after 30d of inactivity.

Mark the issue as fresh by commenting /remove-lifecycle rotten.
Rotten issues close after an additional 30d of inactivity.
Exclude this issue from closing by commenting /lifecycle frozen.

If this issue is safe to close now please do so with /close.

/lifecycle rotten
/remove-lifecycle stale

openshift-bot · 2023-06-05T00:00:25Z

Rotten issues close after 30d of inactivity.

Reopen the issue by commenting /reopen.
Mark the issue as fresh by commenting /remove-lifecycle rotten.
Exclude this issue from closing again by commenting /lifecycle frozen.

/close

openshift-ci · 2023-06-05T00:00:29Z

@openshift-bot: Closed this PR.

In response to this:

Rotten issues close after 30d of inactivity.

Reopen the issue by commenting /reopen.
Mark the issue as fresh by commenting /remove-lifecycle rotten.
Exclude this issue from closing again by commenting /lifecycle frozen.

/close

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

openshift-ci bot requested review from camilamacedo86 and jmccormick2001 November 6, 2021 11:48

cardil force-pushed the bugfix/5342-allow-v-in-version branch from 7b49670 to 32e676b Compare November 6, 2021 11:58

varshaprasad96 approved these changes Nov 8, 2021

View reviewed changes

openshift-ci bot assigned varshaprasad96 Nov 8, 2021

openshift-ci bot added the lgtm Indicates that a PR is ready to be merged. label Nov 8, 2021

camilamacedo86 reviewed Nov 8, 2021

View reviewed changes

internal/cmd/operator-sdk/generate/internal/genutil.go Outdated Show resolved Hide resolved

openshift-ci bot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Nov 10, 2021

varshaprasad96 reviewed Jan 14, 2022

View reviewed changes

internal/cmd/operator-sdk/generate/internal/genutil.go Show resolved Hide resolved

openshift-ci bot added the lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. label Apr 14, 2022

openshift-ci bot removed lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. lgtm Indicates that a PR is ready to be merged. labels Apr 26, 2022

cardil force-pushed the bugfix/5342-allow-v-in-version branch 2 times, most recently from 544e68e to b2fc3b4 Compare April 26, 2022 11:10

cardil mentioned this pull request Apr 26, 2022

Remove the semver v strip wavesoftware/passless-operator#14

Open

cardil force-pushed the bugfix/5342-allow-v-in-version branch from b2fc3b4 to 08a9d1c Compare April 26, 2022 12:11

cardil requested review from asmacdo, camilamacedo86 and varshaprasad96 April 26, 2022 12:16

cardil changed the title ~~Allow v char in version string~~ Allows the v char in the version string Apr 26, 2022

joelanford reviewed May 4, 2022

View reviewed changes

openshift-ci bot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Jun 13, 2022

cardil force-pushed the bugfix/5342-allow-v-in-version branch from 08a9d1c to 24f8b1b Compare June 22, 2022 13:49

openshift-ci bot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Jun 22, 2022

Allow to use v char in version string

813607c

- Adding test to cover version validation - Stripping `v` prefix in version Signed-off-by: Chris Suszyński <ksuszyns@redhat.com>

cardil force-pushed the bugfix/5342-allow-v-in-version branch from 24f8b1b to 813607c Compare June 22, 2022 13:57

cardil requested a review from joelanford June 22, 2022 14:03

openshift-ci bot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Jun 26, 2022

Merge remote-tracking branch 'upstream/master' into bugfix/5342-allow…

9862846

…-v-in-version Conflicts fixed: * internal/cmd/operator-sdk/generate/bundle/bundle.go

openshift-ci bot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Jul 13, 2022

openshift-ci bot added ok-to-test Indicates a non-member PR verified by an org member that is safe to test. lgtm Indicates that a PR is ready to be merged. labels Jan 4, 2023

openshift-ci bot added the lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. label Apr 5, 2023

openshift-ci bot added lifecycle/rotten Denotes an issue or PR that has aged beyond stale and will be auto-closed. and removed lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. labels May 5, 2023

openshift-ci bot closed this Jun 5, 2023

Uh oh!

Allows the v char in the version string #5343

Allows the v char in the version string #5343

Uh oh!

Conversation

cardil commented Nov 6, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

varshaprasad96 left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

asmacdo commented Nov 8, 2021

Uh oh!

joelanford commented Nov 10, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

jmrodri commented Jan 14, 2022

Uh oh!

varshaprasad96 left a comment • 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!

Uh oh!

openshift-bot commented Apr 14, 2022

Uh oh!

cardil commented Apr 26, 2022

Uh oh!

cardil commented Apr 26, 2022

Uh oh!

joelanford May 4, 2022

Choose a reason for hiding this comment

Uh oh!

cardil Jun 22, 2022

Choose a reason for hiding this comment

Uh oh!

cardil Jul 13, 2022

Choose a reason for hiding this comment

Uh oh!

cardil commented Aug 25, 2022

Uh oh!

cardil commented Oct 10, 2022

Uh oh!

varshaprasad96 commented Jan 4, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

openshift-bot commented Apr 5, 2023

Uh oh!

openshift-bot commented May 5, 2023

Uh oh!

openshift-bot commented Jun 5, 2023

Uh oh!

openshift-ci bot commented Jun 5, 2023

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

7 participants

Allows the `v` char in the version string #5343

Allows the `v` char in the version string #5343

cardil commented Nov 6, 2021 •

edited

Loading

joelanford commented Nov 10, 2021 •

edited

Loading

varshaprasad96 left a comment •

edited

Loading

varshaprasad96 commented Jan 4, 2023 •

edited

Loading