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

chore(deps): Upgrade the version of semver package from v1 to v3 #625

Merged
merged 1 commit into from
Sep 28, 2023

Conversation

umi0410
Copy link
Contributor

@umi0410 umi0410 commented Sep 22, 2023

Reason for the upgrade

I attempted to use the semver update strategy with the x-0 constraint.

  • x-0 signifies any tags that adhere to semantic versioning, whether or not they have prerelease tags.

However, the constraint wasn't considering tags with prerelease versions as valid tags.

There was a fix1 in Masterminds/semver package, which argocd-image-updater uses for semver checks. This fix was included in the v3.2.0 release2 of Masterminds/semver and the lastest release is v3.2.1.

Therefore I have upgraded the version of Masterminds/semver package from v1 .5.0to v3.2.1.

Testing

I have verified that the x-0 constraint correctly handles prerelease tags by the following test code.
The test code includes multiple test cases to cover various scenarios.

Details

package tag

import (
	"fmt"
	"github.com/Masterminds/semver/v3"
	"github.com/stretchr/testify/assert"
	"strconv"
	"testing"
	"time"
)

func Test_Semver(t *testing.T) {
	for i, tt := range []struct {
		imageList  []string
		expected   string
		constraint string
	}{
		{
			imageList: []string{"v1.2.3", "v1.2.3-rc1"},
			expected:  "v1.2.3",
		},
		{
			imageList: []string{"v1.2", "v1.2.3"},
			expected:  "v1.2.3",
		},
		{
			imageList: []string{"v1.2.3-rc1", "v1.2.3"},
			expected:  "v1.2.3",
		},
		{
			imageList: []string{"v1.2.3-rc1", "v1.2.3-rc2"},
			expected:  "v1.2.3-rc2",
		},
		{
			imageList: []string{"v1.2.3-rc2", "v1.2.3-rc12"},
			// This is because if prelease tags are string,
			// they are sorted by the order of strings.
			expected: "v1.2.3-rc2",
		},
		{
			imageList:  []string{"v1.2.3-1", "v1.2.3"},
			expected:   "v1.2.3",
			constraint: "x-0",
		},
		{
			imageList:  []string{"v1.2.3-rc1", "v1.2.3-1"},
			expected:   "v1.2.3-rc1",
			constraint: "x-0",
		},
		{
			imageList:  []string{"v1.2.3-1", "v1.2.3-rc1"},
			expected:   "v1.2.3-rc1",
			constraint: "x-0",
		},
	} {
		t.Run("tt "+strconv.Itoa(i), func(t *testing.T) {
			var (
				v   *semver.Constraints
				err error
			)
			if tt.constraint != "" {
				v, err = semver.NewConstraint(tt.constraint)
				assert.NoError(t, err)
			}

			il := NewImageTagList()
			for _, tag := range tt.imageList {
				version, err := semver.NewVersion(tag)
				assert.NoError(t, err)
				if v != nil && !v.Check(version) {
					t.Logf("Constraint failed. constraint=%s, version=%s", tt.constraint, tag)
					continue
				}
				t.Logf("Constraint qualified. constraint=%s, version=%s", tt.constraint, tag)
				il.Add(NewImageTag(tag, time.Now(), ""))
			}
			sorted := il.SortBySemVer()
			answer := sorted[len(sorted)-1].TagName
			assert.Equal(t, tt.expected, answer, fmt.Sprintf("expected %s, but %s returned. sorted version: %+v", tt.expected, answer, sorted))
		})
	}
}

Footnotes

  1. https://github.com/Masterminds/semver/pull/176

  2. https://github.com/Masterminds/semver/releases/tag/v3.2.0

@umi0410 umi0410 force-pushed the chore-bump-semver-package branch from 0d18b16 to 22954c8 Compare September 22, 2023 18:36
@codecov-commenter
Copy link

codecov-commenter commented Sep 22, 2023

Codecov Report

Merging #625 (22954c8) into master (d5a8f94) will decrease coverage by 0.29%.
Report is 4 commits behind head on master.
The diff coverage is 75.00%.

@@            Coverage Diff             @@
##           master     #625      +/-   ##
==========================================
- Coverage   65.63%   65.35%   -0.29%     
==========================================
  Files          22       22              
  Lines        2069     2084      +15     
==========================================
+ Hits         1358     1362       +4     
- Misses        577      588      +11     
  Partials      134      134              
Files Changed Coverage Δ
pkg/argocd/git.go 66.82% <0.00%> (ø)
pkg/image/version.go 40.69% <ø> (ø)
pkg/tag/semver.go 100.00% <ø> (ø)
pkg/tag/tag.go 82.53% <ø> (ø)
pkg/argocd/gitcreds.go 76.19% <83.33%> (ø)
pkg/argocd/update.go 66.54% <83.33%> (+0.49%) ⬆️
pkg/registry/client.go 12.75% <100.00%> (-0.76%) ⬇️

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

Copy link
Contributor

@jannfis jannfis left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, thanks!

@jannfis jannfis merged commit 82d59ef into argoproj-labs:master Sep 28, 2023
mcfearsome pushed a commit to mcfearsome/argocd-image-updater that referenced this pull request Oct 5, 2023
@umi0410 umi0410 deleted the chore-bump-semver-package branch December 3, 2023 10:10
dlactin pushed a commit to dlactin/argocd-image-updater that referenced this pull request May 9, 2024
sribiere-jellysmack pushed a commit to sribiere-jellysmack/argocd-image-updater that referenced this pull request Aug 13, 2024
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

Successfully merging this pull request may close these issues.

3 participants