From dffdb7f54c981815fc1573ccda6736410ad0df67 Mon Sep 17 00:00:00 2001 From: Mahendra Paipuri Date: Tue, 14 Jan 2025 15:24:13 +0100 Subject: [PATCH] feat: Handle pre and post releases in semver.Compare Signed-off-by: Mahendra Paipuri --- pkg/plugin/permissions.go | 3 +- pkg/plugin/resources.go | 23 ++++++++++++-- pkg/plugin/resources_test.go | 58 ++++++++++++++++++++++++++++++++++++ 3 files changed, 80 insertions(+), 4 deletions(-) diff --git a/pkg/plugin/permissions.go b/pkg/plugin/permissions.go index 06aa00b..9e65f51 100644 --- a/pkg/plugin/permissions.go +++ b/pkg/plugin/permissions.go @@ -10,7 +10,6 @@ import ( "github.com/mahendrapaipuri/authlib/authn" "github.com/mahendrapaipuri/authlib/authz" "github.com/mahendrapaipuri/authlib/cache" - "golang.org/x/mod/semver" ) // HasAccess verifies if the current request context has access to certain action. @@ -77,7 +76,7 @@ func (app *App) GetAuthZClient(req *http.Request) (authz.EnforcementClient, erro // So this check will fail for Grafana < 11.1.0 // Set VerifierConfig{DisableTypHeaderCheck: true} for those cases disableTypHeaderCheck := false - if semver.Compare(app.grafanaSemVer, "v11.1.0") == -1 { + if semverCompare(app.grafanaSemVer, "v11.1.0") == -1 { disableTypHeaderCheck = true } diff --git a/pkg/plugin/resources.go b/pkg/plugin/resources.go index c1c4641..87772ad 100644 --- a/pkg/plugin/resources.go +++ b/pkg/plugin/resources.go @@ -33,7 +33,7 @@ const ( // convertPanelIDs returns panel IDs based on Grafana version. func (app *App) convertPanelIDs(ids []string) []string { // For Grafana < 11.3.0, we can use the IDs as such - if semver.Compare(app.grafanaSemVer, "v11.3.0") == -1 { + if semverCompare(app.grafanaSemVer, "v11.3.0") == -1 { return ids } @@ -107,7 +107,7 @@ func (app *App) updateConfig(req *http.Request, conf *config.Config) { func (app *App) featureTogglesEnabled(ctx context.Context) bool { // If Grafana <= 10.4.3, we use cookies to make request. Moreover feature toggles are // not available for these Grafana versions. - if semver.Compare(app.grafanaSemVer, "v10.4.3") <= -1 { + if semverCompare(app.grafanaSemVer, "v10.4.3") <= -1 { return false } @@ -393,3 +393,22 @@ func (app *App) registerRoutes(mux *http.ServeMux) { mux.HandleFunc("/report", app.handleReport) mux.HandleFunc("/healthz", app.handleHealth) } + +// semverCompare compares the semantic version of Grafana versions. +// Grafana uses "+" as post release suffix and "-" as pre-release +// suffixes. We take that into account when calling upstream semver +// package. +func semverCompare(a, b string) int { + switch { + case strings.HasPrefix(a, b+"+"): + return 1 + case strings.HasPrefix(b, a+"+"): + return -1 + case strings.HasPrefix(a, b+"-"): + return -1 + case strings.HasPrefix(b, a+"-"): + return 1 + } + + return semver.Compare(a, b) +} diff --git a/pkg/plugin/resources_test.go b/pkg/plugin/resources_test.go index 8f8eaab..4234efb 100644 --- a/pkg/plugin/resources_test.go +++ b/pkg/plugin/resources_test.go @@ -110,3 +110,61 @@ func TestReportResource(t *testing.T) { }) }) } + +func TestSemverComapre(t *testing.T) { + Convey("When comparing semantic versions", t, func() { + tests := []struct { + name string + a, b string + expected int + }{ + { + name: "regular sem ver comparison", + a: "v1.2.3", + b: "v1.2.5", + expected: -1, + }, + { + name: "regular sem ver with pre-release comparison", + a: "v1.2.3", + b: "v1.2.3-rc0", + expected: 1, + }, + { + name: "regular sem ver with pre-release comparison with inverse order", + a: "v1.2.3-rc1", + b: "v1.2.3", + expected: -1, + }, + { + name: "regular sem ver with post-release comparison", + a: "v1.2.3", + b: "v1.2.3+security-01", + expected: -1, + }, + { + name: "regular sem ver with post-release comparison with inverse order", + a: "v1.2.3+security-01", + b: "v1.2.3", + expected: 1, + }, + { + name: "comparison with zero version", + a: "v0.0.0", + b: "v1.2.5", + expected: -1, + }, + { + name: "comparison with zero version with inverse order", + a: "v1.2.3", + b: "v0.0.0", + expected: 1, + }, + } + + for _, test := range tests { + got := semverCompare(test.a, test.b) + So(got, ShouldEqual, test.expected) + } + }) +}