From c9eb8f03c0eec20d53130989af3c97d5b160ec7e Mon Sep 17 00:00:00 2001 From: Hidde Beydals Date: Wed, 28 Oct 2020 02:00:21 +0100 Subject: [PATCH] Switch to Masterminds/semver and pkg/version libs Co-authored-by: Illia Ovchynnikov Signed-off-by: Hidde Beydals --- controllers/gitrepository_controller_test.go | 11 +++- go.mod | 3 +- go.sum | 4 +- internal/helm/repository.go | 64 ++++++++++---------- pkg/git/checkout.go | 36 ++++++----- 5 files changed, 65 insertions(+), 53 deletions(-) diff --git a/controllers/gitrepository_controller_test.go b/controllers/gitrepository_controller_test.go index 4b64d93b..5e9ad82c 100644 --- a/controllers/gitrepository_controller_test.go +++ b/controllers/gitrepository_controller_test.go @@ -216,11 +216,18 @@ var _ = Describe("GitRepositoryReconciler", func() { expectStatus: corev1.ConditionTrue, expectRevision: "0.2.0", }), + Entry("mixed semver range", refTestCase{ + reference: &sourcev1.GitRepositoryRef{SemVer: ">=0.1.0 <1.0.0"}, + createRefs: []string{"refs/tags/0.1.0", "refs/tags/v0.1.1", "refs/tags/v0.2.0", "refs/tags/1.0.0"}, + waitForReason: sourcev1.GitOperationSucceedReason, + expectStatus: corev1.ConditionTrue, + expectRevision: "v0.2.0", + }), Entry("semver invalid", refTestCase{ - reference: &sourcev1.GitRepositoryRef{SemVer: "v1.0.0"}, + reference: &sourcev1.GitRepositoryRef{SemVer: "1.2.3.4"}, waitForReason: sourcev1.GitOperationFailedReason, expectStatus: corev1.ConditionFalse, - expectMessage: "semver parse range error", + expectMessage: "semver parse range error: improper constraint: 1.2.3.4", }), Entry("semver no match", refTestCase{ reference: &sourcev1.GitRepositoryRef{SemVer: "1.0.0"}, diff --git a/go.mod b/go.mod index c99567e3..fcc37c7b 100644 --- a/go.mod +++ b/go.mod @@ -5,7 +5,7 @@ go 1.15 replace github.com/fluxcd/source-controller/api => ./api require ( - github.com/blang/semver/v4 v4.0.0 + github.com/Masterminds/semver/v3 v3.1.0 github.com/fluxcd/pkg/apis/meta v0.0.2 github.com/fluxcd/pkg/gittestserver v0.0.2 github.com/fluxcd/pkg/helmtestserver v0.0.1 @@ -13,6 +13,7 @@ require ( github.com/fluxcd/pkg/runtime v0.1.0 github.com/fluxcd/pkg/ssh v0.0.5 github.com/fluxcd/pkg/untar v0.0.5 + github.com/fluxcd/pkg/version v0.0.1 github.com/fluxcd/source-controller/api v0.1.1 github.com/go-git/go-billy/v5 v5.0.0 github.com/go-git/go-git/v5 v5.1.0 diff --git a/go.sum b/go.sum index 68e7cb46..6a84ae1a 100644 --- a/go.sum +++ b/go.sum @@ -87,8 +87,6 @@ github.com/bitly/go-simplejson v0.5.0/go.mod h1:cXHtHw4XUPsvGaxgjIAn8PhEWG9NfngE github.com/blang/semver v3.1.0+incompatible/go.mod h1:kRBLl5iJ+tD4TcOOxsy/0fnwebNt5EWlYSAyrTnjyyk= github.com/blang/semver v3.5.0+incompatible h1:CGxCgetQ64DKk7rdZ++Vfnb1+ogGNnB17OJKJXD2Cfs= github.com/blang/semver v3.5.0+incompatible/go.mod h1:kRBLl5iJ+tD4TcOOxsy/0fnwebNt5EWlYSAyrTnjyyk= -github.com/blang/semver/v4 v4.0.0 h1:1PFHFE6yCCTv8C1TeyNNarDzntLi7wMI5i/pzqYIsAM= -github.com/blang/semver/v4 v4.0.0/go.mod h1:IbckMUScFkM3pff0VJDNKRiT6TG/YpiHIM2yvyW5YoQ= github.com/bmizerany/assert v0.0.0-20160611221934-b7ed37b82869 h1:DDGfHa7BWjL4YnC6+E63dPcxHo2sUxDIu8g3QgEJdRY= github.com/bmizerany/assert v0.0.0-20160611221934-b7ed37b82869/go.mod h1:Ekp36dRnpXw/yCqJaO+ZrUyxD+3VXMFFr56k5XYrpB4= github.com/bshuster-repo/logrus-logstash-hook v0.4.1 h1:pgAtgj+A31JBVtEHu2uHuEx0n+2ukqUJnS2vVe5pQNA= @@ -220,6 +218,8 @@ github.com/fluxcd/pkg/testserver v0.0.2 h1:SoaMtO9cE5p/wl2zkGudzflnEHd9mk68CGjZO github.com/fluxcd/pkg/testserver v0.0.2/go.mod h1:pgUZTh9aQ44FSTQo+5NFlh7YMbUfdz1B80DalW7k96Y= github.com/fluxcd/pkg/untar v0.0.5 h1:UGI3Ch1UIEIaqQvMicmImL1s9npQa64DJ/ozqHKB7gk= github.com/fluxcd/pkg/untar v0.0.5/go.mod h1:O6V9+rtl8c1mHBafgqFlJN6zkF1HS5SSYn7RpQJ/nfw= +github.com/fluxcd/pkg/version v0.0.1 h1:/8asQoDXSThz3csiwi4Qo8Zb6blAxLXbtxNgeMJ9bCg= +github.com/fluxcd/pkg/version v0.0.1/go.mod h1:WAF4FEEA9xyhngF8TDxg3UPu5fA1qhEYV8Pmi2Il01Q= github.com/flynn/go-shlex v0.0.0-20150515145356-3f9db97f8568 h1:BHsljHzVlRcyQhjrss6TZTdY2VfCqZPbv5k3iBFa2ZQ= github.com/flynn/go-shlex v0.0.0-20150515145356-3f9db97f8568/go.mod h1:xEzjJPgXI435gkrCt3MPfRiAkVrwSbHsst4LCFVfpJc= github.com/franela/goblin v0.0.0-20200105215937-c9ffbefa60db/go.mod h1:7dvUGVsVBjqR7JHJk0brhHOZYGmfBYOrK0ZhYMEtBr4= diff --git a/internal/helm/repository.go b/internal/helm/repository.go index 762a4cb5..ee945379 100644 --- a/internal/helm/repository.go +++ b/internal/helm/repository.go @@ -25,10 +25,12 @@ import ( "sort" "strings" - "github.com/blang/semver/v4" + "github.com/Masterminds/semver/v3" "helm.sh/helm/v3/pkg/getter" "helm.sh/helm/v3/pkg/repo" "sigs.k8s.io/yaml" + + "github.com/fluxcd/pkg/version" ) // ChartRepository represents a Helm chart repository, and the configuration @@ -63,7 +65,7 @@ func NewChartRepository(repositoryURL string, providers getter.Providers, opts [ // Get returns the repo.ChartVersion for the given name, the version is expected // to be a semver.Constraints compatible string. If version is empty, the latest // stable version will be returned and prerelease versions will be ignored. -func (r *ChartRepository) Get(name, version string) (*repo.ChartVersion, error) { +func (r *ChartRepository) Get(name, ver string) (*repo.ChartVersion, error) { cvs, ok := r.Index.Entries[name] if !ok { return nil, repo.ErrNoChartName @@ -73,71 +75,69 @@ func (r *ChartRepository) Get(name, version string) (*repo.ChartVersion, error) } // Check for exact matches first - if len(version) != 0 { + if len(ver) != 0 { for _, cv := range cvs { - if version == cv.Version { + if ver == cv.Version { return cv, nil } } } // Continue to look for a (semantic) version match - latestStable := len(version) == 0 || version == "*" - var match semver.Range + verConstraint, err := semver.NewConstraint("*") + if err != nil { + return nil, err + } + latestStable := len(ver) == 0 || ver == "*" if !latestStable { - rng, err := semver.ParseRange(version) + verConstraint, err = semver.NewConstraint(ver) if err != nil { return nil, err } - match = rng } - var filteredVersions semver.Versions - lookup := make(map[string]*repo.ChartVersion) + + // Filter out chart versions that doesn't satisfy constraints if any, + // parse semver and build a lookup table + var matchedVersions semver.Collection + lookup := make(map[*semver.Version]*repo.ChartVersion) for _, cv := range cvs { - v, err := semver.ParseTolerant(cv.Version) + v, err := version.ParseVersion(cv.Version) if err != nil { continue } - if match != nil && !match(v) { + if !verConstraint.Check(v) { continue } - filteredVersions = append(filteredVersions, v) - lookup[v.String()] = cv + + matchedVersions = append(matchedVersions, v) + lookup[v] = cv } - if len(filteredVersions) == 0 { - return nil, fmt.Errorf("no chart version found for %s-%s", name, version) + if len(matchedVersions) == 0 { + return nil, fmt.Errorf("no chart version found for %s-%s", name, ver) } // Sort versions - sort.SliceStable(filteredVersions, func(i, j int) bool { + sort.SliceStable(matchedVersions, func(i, j int) bool { // Reverse return !(func() bool { - left := filteredVersions[i] - right := filteredVersions[j] + left := matchedVersions[i] + right := matchedVersions[j] - if !left.EQ(right) { - return left.LT(right) + if !left.Equal(right) { + return left.LessThan(right) } // Having chart creation timestamp at our disposal, we put package with the // same version into a chronological order. This is especially important for // versions that differ only by build metadata, because it is not considered // a part of the comparable version in Semver - return lookup[left.String()].Created.Before(lookup[right.String()].Created) + return lookup[left].Created.Before(lookup[right].Created) })() }) - latest := filteredVersions[0] - if latestStable { - for _, v := range filteredVersions { - if len(v.Pre) == 0 { - latest = v - break - } - } - } - return lookup[latest.String()], nil + latest := matchedVersions[0] + return lookup[latest], nil } // DownloadChart confirms the given repo.ChartVersion has a downloadable URL, diff --git a/pkg/git/checkout.go b/pkg/git/checkout.go index 103f49f5..c15ca97d 100644 --- a/pkg/git/checkout.go +++ b/pkg/git/checkout.go @@ -22,12 +22,14 @@ import ( "sort" "time" - "github.com/blang/semver/v4" + "github.com/Masterminds/semver/v3" "github.com/go-git/go-git/v5" "github.com/go-git/go-git/v5/plumbing" "github.com/go-git/go-git/v5/plumbing/object" "github.com/go-git/go-git/v5/plumbing/transport" + "github.com/fluxcd/pkg/version" + sourcev1 "github.com/fluxcd/source-controller/api/v1beta1" ) @@ -166,7 +168,7 @@ type CheckoutSemVer struct { } func (c *CheckoutSemVer) Checkout(ctx context.Context, path, url string, auth transport.AuthMethod) (*object.Commit, string, error) { - rng, err := semver.ParseRange(c.semVer) + verConstraint, err := semver.NewConstraint(c.semVer) if err != nil { return nil, "", fmt.Errorf("semver parse range error: %w", err) } @@ -208,26 +210,28 @@ func (c *CheckoutSemVer) Checkout(ctx context.Context, path, url string, auth tr return nil }) - svTags := make(map[string]string) - var svers []semver.Version + var matchedVersions semver.Collection for tag, _ := range tags { - v, _ := semver.ParseTolerant(tag) - if rng(v) { - svers = append(svers, v) - svTags[v.String()] = tag + v, err := version.ParseVersion(tag) + if err != nil { + continue } + if !verConstraint.Check(v) { + continue + } + matchedVersions = append(matchedVersions, v) } - if len(svers) == 0 { + if len(matchedVersions) == 0 { return nil, "", fmt.Errorf("no match found for semver: %s", c.semVer) } // Sort versions - sort.SliceStable(svers, func(i, j int) bool { - left := svers[i] - right := svers[j] + sort.SliceStable(matchedVersions, func(i, j int) bool { + left := matchedVersions[i] + right := matchedVersions[j] - if !left.EQ(right) { - return left.LT(right) + if !left.Equal(right) { + return left.LessThan(right) } // Having tag target timestamps at our disposal, we further try to sort @@ -236,8 +240,8 @@ func (c *CheckoutSemVer) Checkout(ctx context.Context, path, url string, auth tr // a part of the comparable version in Semver return tagTimestamps[left.String()].Before(tagTimestamps[right.String()]) }) - v := svers[len(svers)-1] - t := svTags[v.String()] + v := matchedVersions[len(matchedVersions)-1] + t := v.Original() w, err := repo.Worktree() if err != nil {