From cde6df110134c7ac662cecb5493d54ab979e1ea3 Mon Sep 17 00:00:00 2001 From: "David L. Chandler" Date: Tue, 24 Mar 2026 15:57:02 -0400 Subject: [PATCH 1/4] strict semver tags like 2.1.3 work, not just tags like v2.1.3 Signed-off-by: David L. Chandler --- githubutils/repo.go | 6 ++--- securityscanutils/predicate.go | 11 +++++---- securityscanutils/predicate_test.go | 36 +++++++++++++++++++++++++++++ 3 files changed, 45 insertions(+), 8 deletions(-) diff --git a/githubutils/repo.go b/githubutils/repo.go index c5377cfa..fecbbd1d 100644 --- a/githubutils/repo.go +++ b/githubutils/repo.go @@ -357,15 +357,15 @@ 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()) + verA, err := semver.NewVersion(rA.GetTagName()) if err != nil { return false } - verB, err := versionutils.ParseVersion(rB.GetTagName()) + verB, err := semver.NewVersion(rB.GetTagName()) if err != nil { return false } - return verA.MustIsGreaterThan(*verB) + return verA.GreaterThan(verB) }) } diff --git a/securityscanutils/predicate.go b/securityscanutils/predicate.go index 67259ade..5ec1fcf1 100644 --- a/securityscanutils/predicate.go +++ b/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 } diff --git a/securityscanutils/predicate_test.go b/securityscanutils/predicate_test.go index 1b37bac5..00ee9b4a 100644 --- a/securityscanutils/predicate_test.go +++ b/securityscanutils/predicate_test.go @@ -85,4 +85,40 @@ var _ = Describe("Predicate", func() { }, true), ) }) + + Context("latestPatchRepositoryReleasePredicate without v prefix", func() { + var releasePredicate githubutils.RepositoryReleasePredicate + + BeforeEach(func() { + releaseSet := []*github.RepositoryRelease{ + { + TagName: github.String("2.0.1"), + }, + { + TagName: github.String("2.1.2"), + }, + { + TagName: github.String("2.1.3"), + }, + } + + releasePredicate = securityscanutils.NewLatestPatchRepositoryReleasePredicate(releaseSet) + }) + + DescribeTable( + "Returns true/false based on release properties", + func(release *github.RepositoryRelease, expectedResult bool) { + Expect(releasePredicate.Apply(release)).To(Equal(expectedResult)) + }, + Entry("release is latest patch for 2.0.x", &github.RepositoryRelease{ + TagName: github.String("2.0.1"), + }, true), + Entry("release is not latest patch for 2.1.x", &github.RepositoryRelease{ + TagName: github.String("2.1.2"), + }, false), + Entry("release is latest patch for 2.1.x", &github.RepositoryRelease{ + TagName: github.String("2.1.3"), + }, true), + ) + }) }) From 33eb1186c2ca8f40dd2b972e28779efccad2610b Mon Sep 17 00:00:00 2001 From: "David L. Chandler" Date: Tue, 24 Mar 2026 16:00:56 -0400 Subject: [PATCH 2/4] tested Signed-off-by: David L. Chandler --- githubutils/repo.go | 11 +- securityscanutils/predicate_test.go | 209 ++++++++++++++++++++-------- 2 files changed, 159 insertions(+), 61 deletions(-) diff --git a/githubutils/repo.go b/githubutils/repo.go index fecbbd1d..9ed1098e 100644 --- a/githubutils/repo.go +++ b/githubutils/repo.go @@ -357,13 +357,14 @@ 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 := semver.NewVersion(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 := semver.NewVersion(rB.GetTagName()) - if err != nil { - return false + if errB != nil { + return true } return verA.GreaterThan(verB) }) diff --git a/securityscanutils/predicate_test.go b/securityscanutils/predicate_test.go index 00ee9b4a..adfc1d17 100644 --- a/securityscanutils/predicate_test.go +++ b/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,74 +63,159 @@ 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)) }, - { - TagName: github.String("v1.0.2"), + 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)) }, - { - TagName: github.String("v1.0.3"), + 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)) }, - } + 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), + ) - releasePredicate = securityscanutils.NewLatestPatchRepositoryReleasePredicate(releaseSet) + 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)) + }, + 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), + ) }) - 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), - ) - }) + 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("latestPatchRepositoryReleasePredicate without v prefix", func() { - var releasePredicate githubutils.RepositoryReleasePredicate + 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()) + }) + }) - BeforeEach(func() { - releaseSet := []*github.RepositoryRelease{ - { - TagName: github.String("2.0.1"), - }, - { - TagName: github.String("2.1.2"), - }, - { - TagName: github.String("2.1.3"), - }, + 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()) + }) + }) + }) + + 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 latest patch for 2.0.x", &github.RepositoryRelease{ - TagName: github.String("2.0.1"), - }, true), - Entry("release is not latest patch for 2.1.x", &github.RepositoryRelease{ - TagName: github.String("2.1.2"), - }, false), - Entry("release is latest patch for 2.1.x", &github.RepositoryRelease{ - TagName: github.String("2.1.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"})) + }) }) }) From ba369d7066a9b21694227f796b652d428a2291c8 Mon Sep 17 00:00:00 2001 From: "David L. Chandler" Date: Tue, 24 Mar 2026 16:25:53 -0400 Subject: [PATCH 3/4] changelog Signed-off-by: David L. Chandler --- changelog/v0.30.1/fix-unprefix-semver.yaml | 9 +++++++++ 1 file changed, 9 insertions(+) create mode 100644 changelog/v0.30.1/fix-unprefix-semver.yaml diff --git a/changelog/v0.30.1/fix-unprefix-semver.yaml b/changelog/v0.30.1/fix-unprefix-semver.yaml new file mode 100644 index 00000000..c5d24a99 --- /dev/null +++ b/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 From 9af48edcfd2e4c3ff98bb44227a76bb1f3e1ddae Mon Sep 17 00:00:00 2001 From: "David L. Chandler" Date: Tue, 24 Mar 2026 17:36:36 -0400 Subject: [PATCH 4/4] a new test that fails without this branch Signed-off-by: David L. Chandler --- securityscanutils/predicate_test.go | 25 +++++++++++++++++++++++++ 1 file changed, 25 insertions(+) diff --git a/securityscanutils/predicate_test.go b/securityscanutils/predicate_test.go index adfc1d17..54bde2ac 100644 --- a/securityscanutils/predicate_test.go +++ b/securityscanutils/predicate_test.go @@ -165,6 +165,31 @@ var _ = Describe("Predicate", func() { 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() {