Allow strict semver versions like '2.1.3'; do not require a 'v' prefix

changelog/v0.30.1/fix-unprefix-semver.yaml

@@ -0,0 +1,9 @@
+changelog:
+  - type: FIX
+    description: |
+      Use semver library instead of versionutils.ParseVersion in SortReleasesBySemver
+      and NewLatestPatchRepositoryReleasePredicate so that release tags without a 'v'
+      prefix (e.g. "2.0.1") are parsed correctly. Previously these tags were silently
+      skipped, causing the GitHub issue writer to never create issues for affected releases.
+    issueLink: https://github.com/solo-io/gloo-gateway/issues/1927
+    resolvesIssue: false

githubutils/repo.go

@@ -357,15 +357,16 @@ func DownloadFile(url string, w io.Writer) error {
 func SortReleasesBySemver(releases []*github.RepositoryRelease) {
 	sort.Slice(releases, func(i, j int) bool {
 		rA, rB := releases[i], releases[j]
-		verA, err := versionutils.ParseVersion(rA.GetTagName())
-		if err != nil {
+		verA, errA := semver.NewVersion(rA.GetTagName())
+		verB, errB := semver.NewVersion(rB.GetTagName())
+		// Push non-semver tags to the end
+		if errA != nil {
 			return false
 		}
-		verB, err := versionutils.ParseVersion(rB.GetTagName())
-		if err != nil {
-			return false
+		if errB != nil {
+			return true
 		}
-		return verA.MustIsGreaterThan(*verB)
+		return verA.GreaterThan(verB)
 	})
 }
 

securityscanutils/predicate.go

@@ -5,7 +5,6 @@ import (
 	"github.com/google/go-github/v32/github"
 	"github.com/solo-io/go-utils/githubutils"
 	"github.com/solo-io/go-utils/log"
-	"github.com/solo-io/go-utils/versionutils"
 )
 
 // The securityScanRepositoryReleasePredicate is responsible for defining which
@@ -70,17 +69,19 @@ func NewLatestPatchRepositoryReleasePredicate(releases []*github.RepositoryRelea
 	recentMajor := -1
 	recentMinor := -1
 	for _, release := range releases {
-		version, err := versionutils.ParseVersion(release.GetTagName())
+		version, err := semver.NewVersion(release.GetTagName())
 		if err != nil {
 			continue
 		}
-		if version.Major == recentMajor && version.Minor == recentMinor {
+		major := int(version.Major())
+		minor := int(version.Minor())
+		if major == recentMajor && minor == recentMinor {
 			continue
 		}
 
 		// This is the largest patch release
-		recentMajor = version.Major
-		recentMinor = version.Minor
+		recentMajor = major
+		recentMinor = minor
 		latestPatchReleasesByTagName[release.GetTagName()] = release
 	}
 

securityscanutils/predicate_test.go

@@ -11,6 +11,18 @@ import (
 	"github.com/solo-io/go-utils/securityscanutils"
 )
 
+func release(tag string) *github.RepositoryRelease {
+	return &github.RepositoryRelease{TagName: github.String(tag)}
+}
+
+func releases(tags ...string) []*github.RepositoryRelease {
+	out := make([]*github.RepositoryRelease, len(tags))
+	for i, t := range tags {
+		out[i] = release(t)
+	}
+	return out
+}
+
 var _ = Describe("Predicate", func() {
 	Context("securityScanRepositoryReleasePredicate", func() {
 		DescribeTable(
@@ -51,38 +63,184 @@ var _ = Describe("Predicate", func() {
 	})
 
 	Context("latestPatchRepositoryReleasePredicate", func() {
-		var releasePredicate githubutils.RepositoryReleasePredicate
 
-		BeforeEach(func() {
-			releaseSet := []*github.RepositoryRelease{
-				{
-					TagName: github.String("v1.0.1"),
+		Context("with v-prefixed tags", func() {
+			DescribeTable("single minor series",
+				func(tag string, expected bool) {
+					pred := securityscanutils.NewLatestPatchRepositoryReleasePredicate(
+						releases("v1.0.1", "v1.0.2", "v1.0.3"))
+					Expect(pred.Apply(release(tag))).To(Equal(expected))
+				},
+				Entry("latest patch is accepted", "v1.0.3", true),
+				Entry("older patch is rejected", "v1.0.2", false),
+				Entry("oldest patch is rejected", "v1.0.1", false),
+				Entry("unknown version is rejected", "v3.0.0", false),
+			)
+
+			DescribeTable("multiple minor series",
+				func(tag string, expected bool) {
+					pred := securityscanutils.NewLatestPatchRepositoryReleasePredicate(
+						releases("v1.0.0", "v1.0.1", "v1.1.0", "v1.1.1", "v2.0.0"))
+					Expect(pred.Apply(release(tag))).To(Equal(expected))
+				},
+				Entry("latest patch of 1.0.x", "v1.0.1", true),
+				Entry("older patch of 1.0.x", "v1.0.0", false),
+				Entry("latest patch of 1.1.x", "v1.1.1", true),
+				Entry("older patch of 1.1.x", "v1.1.0", false),
+				Entry("latest patch of 2.0.x", "v2.0.0", true),
+			)
+		})
+
+		Context("without v prefix", func() {
+			DescribeTable("single minor series",
+				func(tag string, expected bool) {
+					pred := securityscanutils.NewLatestPatchRepositoryReleasePredicate(
+						releases("2.0.0", "2.0.1", "2.0.2"))
+					Expect(pred.Apply(release(tag))).To(Equal(expected))
 				},
-				{
-					TagName: github.String("v1.0.2"),
+				Entry("latest patch is accepted", "2.0.2", true),
+				Entry("older patch is rejected", "2.0.1", false),
+				Entry("oldest patch is rejected", "2.0.0", false),
+			)
+
+			DescribeTable("multiple minor series",
+				func(tag string, expected bool) {
+					pred := securityscanutils.NewLatestPatchRepositoryReleasePredicate(
+						releases("2.0.1", "2.1.2", "2.1.3"))
+					Expect(pred.Apply(release(tag))).To(Equal(expected))
 				},
-				{
-					TagName: github.String("v1.0.3"),
+				Entry("latest patch of 2.0.x", "2.0.1", true),
+				Entry("older patch of 2.1.x", "2.1.2", false),
+				Entry("latest patch of 2.1.x", "2.1.3", true),
+			)
+
+			DescribeTable("multiple major series",
+				func(tag string, expected bool) {
+					pred := securityscanutils.NewLatestPatchRepositoryReleasePredicate(
+						releases("1.0.5", "1.1.3", "2.0.1", "2.1.0"))
+					Expect(pred.Apply(release(tag))).To(Equal(expected))
 				},
+				Entry("latest patch of 1.0.x", "1.0.5", true),
+				Entry("latest patch of 1.1.x", "1.1.3", true),
+				Entry("latest patch of 2.0.x", "2.0.1", true),
+				Entry("latest patch of 2.1.x", "2.1.0", true),
+			)
+		})
+
+		Context("unsorted input", func() {
+			It("sorts internally and picks correct latest patches", func() {
+				// provide releases in random order
+				pred := securityscanutils.NewLatestPatchRepositoryReleasePredicate(
+					releases("1.0.0", "1.0.3", "1.0.1", "1.1.0", "1.0.2", "1.1.2", "1.1.1"))
+				Expect(pred.Apply(release("1.0.3"))).To(BeTrue(), "1.0.3 should be latest of 1.0.x")
+				Expect(pred.Apply(release("1.1.2"))).To(BeTrue(), "1.1.2 should be latest of 1.1.x")
+				Expect(pred.Apply(release("1.0.2"))).To(BeFalse())
+				Expect(pred.Apply(release("1.1.1"))).To(BeFalse())
+				Expect(pred.Apply(release("1.0.0"))).To(BeFalse())
+			})
+		})
+
+		Context("non-semver tags are skipped", func() {
+			It("ignores invalid tags and still picks correct latest patches", func() {
+				pred := securityscanutils.NewLatestPatchRepositoryReleasePredicate(
+					releases("not-a-version", "2.0.1", "also-bad", "2.0.2"))
+				Expect(pred.Apply(release("2.0.2"))).To(BeTrue())
+				Expect(pred.Apply(release("2.0.1"))).To(BeFalse())
+				Expect(pred.Apply(release("not-a-version"))).To(BeFalse())
+			})
+		})
+
+		Context("empty release set", func() {
+			It("rejects everything", func() {
+				pred := securityscanutils.NewLatestPatchRepositoryReleasePredicate(releases())
+				Expect(pred.Apply(release("1.0.0"))).To(BeFalse())
+				Expect(pred.Apply(release("v1.0.0"))).To(BeFalse())
+			})
+		})
+
+		Context("single release", func() {
+			It("accepts the only release", func() {
+				pred := securityscanutils.NewLatestPatchRepositoryReleasePredicate(
+					releases("3.5.7"))
+				Expect(pred.Apply(release("3.5.7"))).To(BeTrue())
+			})
+		})
+
+		// Regression: before switching from versionutils.ParseVersion to semver.NewVersion,
+		// non-v-prefixed tags silently failed parsing, producing an empty predicate map.
+		// This is the bug visible in the log as:
+		//   GithubIssueWriter configured with Predicate: &{releasesByTagName:map[]}
+		// With an empty map the predicate rejects every release, so no GitHub issues
+		// are created even when github-issue-minor output is configured.
+		Context("non-v-prefixed tags must not produce an empty predicate (regression)", func() {
+			It("accepts latest patches for strict-semver tags like those used by github-issue-minor repos", func() {
+				// Simulate the exact flow from initializeRepoConfiguration (securityscan.go:201-203):
+				//   issuePredicate = NewLatestPatchRepositoryReleasePredicate(releasesToScan)
+				// using tags that lack the v prefix, as seen in production.
+				pred := securityscanutils.NewLatestPatchRepositoryReleasePredicate(
+					releases("2.0.0", "2.0.1", "2.1.0", "2.1.2", "2.1.3"))
+
+				// The predicate must NOT be a no-op: latest patches must be accepted
+				Expect(pred.Apply(release("2.0.1"))).To(BeTrue(), "latest patch of 2.0.x must be accepted")
+				Expect(pred.Apply(release("2.1.3"))).To(BeTrue(), "latest patch of 2.1.x must be accepted")
+
+				// Older patches must still be rejected
+				Expect(pred.Apply(release("2.0.0"))).To(BeFalse())
+				Expect(pred.Apply(release("2.1.2"))).To(BeFalse())
+				Expect(pred.Apply(release("2.1.0"))).To(BeFalse())
+			})
+		})
+	})
+
+	Context("SortReleasesBySemver", func() {
+		tags := func(rels []*github.RepositoryRelease) []string {
+			out := make([]string, len(rels))
+			for i, r := range rels {
+				out[i] = r.GetTagName()
 			}
+			return out
+		}
 
-			releasePredicate = securityscanutils.NewLatestPatchRepositoryReleasePredicate(releaseSet)
+		It("sorts v-prefixed tags descending", func() {
+			rels := releases("v1.0.0", "v2.0.0", "v1.1.0", "v1.0.1")
+			githubutils.SortReleasesBySemver(rels)
+			Expect(tags(rels)).To(Equal([]string{"v2.0.0", "v1.1.0", "v1.0.1", "v1.0.0"}))
 		})
 
-		DescribeTable(
-			"Returns true/false based on release properties",
-			func(release *github.RepositoryRelease, expectedResult bool) {
-				Expect(releasePredicate.Apply(release)).To(Equal(expectedResult))
-			},
-			Entry("release is not in release set", &github.RepositoryRelease{
-				TagName: github.String("v3.0.0"), // not in the original release set
-			}, false),
-			Entry("release is not latest patch release", &github.RepositoryRelease{
-				TagName: github.String("v1.0.1"),
-			}, false),
-			Entry("release is latest patch release", &github.RepositoryRelease{
-				TagName: github.String("v1.0.3"),
-			}, true),
-		)
+		It("sorts non-prefixed tags descending", func() {
+			rels := releases("1.0.0", "2.0.0", "1.1.0", "1.0.1")
+			githubutils.SortReleasesBySemver(rels)
+			Expect(tags(rels)).To(Equal([]string{"2.0.0", "1.1.0", "1.0.1", "1.0.0"}))
+		})
+
+		It("sorts a realistic release set", func() {
+			rels := releases("2.0.1", "2.1.3", "2.1.2", "2.0.0", "2.1.0", "2.1.1")
+			githubutils.SortReleasesBySemver(rels)
+			Expect(tags(rels)).To(Equal([]string{"2.1.3", "2.1.2", "2.1.1", "2.1.0", "2.0.1", "2.0.0"}))
+		})
+
+		It("handles pre-release tags", func() {
+			rels := releases("2.0.0", "2.1.0-beta.1", "2.1.0-beta.2", "2.1.0")
+			githubutils.SortReleasesBySemver(rels)
+			Expect(tags(rels)).To(Equal([]string{"2.1.0", "2.1.0-beta.2", "2.1.0-beta.1", "2.0.0"}))
+		})
+
+		It("pushes non-semver tags to the end", func() {
+			rels := releases("bad-tag", "2.0.0", "1.0.0")
+			githubutils.SortReleasesBySemver(rels)
+			Expect(tags(rels)).To(Equal([]string{"2.0.0", "1.0.0", "bad-tag"}))
+		})
+
+		It("handles empty list", func() {
+			rels := releases()
+			githubutils.SortReleasesBySemver(rels)
+			Expect(rels).To(BeEmpty())
+		})
+
+		It("handles single element", func() {
+			rels := releases("1.0.0")
+			githubutils.SortReleasesBySemver(rels)
+			Expect(tags(rels)).To(Equal([]string{"1.0.0"}))
+		})
 	})
 })