From 9160d2f2e216ca6aad73cecfde4313791e5b3404 Mon Sep 17 00:00:00 2001 From: Lucas Lemos Date: Sun, 18 Feb 2024 16:23:59 -0300 Subject: [PATCH 1/9] fix: healthcheck middleware not working with route group --- middleware/healthcheck/healthcheck.go | 15 ++++++-- middleware/healthcheck/healthcheck_test.go | 44 ++++++++++++++++++++++ 2 files changed, 56 insertions(+), 3 deletions(-) diff --git a/middleware/healthcheck/healthcheck.go b/middleware/healthcheck/healthcheck.go index 14ff33430c..3744a737c0 100644 --- a/middleware/healthcheck/healthcheck.go +++ b/middleware/healthcheck/healthcheck.go @@ -1,6 +1,9 @@ package healthcheck import ( + "fmt" + "regexp" + "github.com/gofiber/fiber/v2" ) @@ -27,6 +30,11 @@ func healthCheckerHandler(checker HealthChecker) fiber.Handler { func New(config ...Config) fiber.Handler { cfg := defaultConfig(config...) + baseRegexp := `\%s$` + + readinessIdentifier := regexp.MustCompile(fmt.Sprintf(baseRegexp, cfg.ReadinessEndpoint)) + livenessIdentifier := regexp.MustCompile(fmt.Sprintf(baseRegexp, cfg.LivenessEndpoint)) + isLiveHandler := healthCheckerHandler(cfg.LivenessProbe) isReadyHandler := healthCheckerHandler(cfg.ReadinessProbe) @@ -40,10 +48,11 @@ func New(config ...Config) fiber.Handler { return c.Next() } - switch c.Path() { - case cfg.ReadinessEndpoint: + if readinessIdentifier.Match([]byte(c.Path())) { return isReadyHandler(c) - case cfg.LivenessEndpoint: + } + + if livenessIdentifier.Match([]byte(c.Path())) { return isLiveHandler(c) } diff --git a/middleware/healthcheck/healthcheck_test.go b/middleware/healthcheck/healthcheck_test.go index df0165f158..03b29da08f 100644 --- a/middleware/healthcheck/healthcheck_test.go +++ b/middleware/healthcheck/healthcheck_test.go @@ -10,6 +10,50 @@ import ( "github.com/valyala/fasthttp" ) +func Test_HealthCheck_Strict_Routing_Default(t *testing.T) { + t.Parallel() + + app := fiber.New(fiber.Config{ + StrictRouting: true, + }) + + app.Use(New()) + + req, err := app.Test(httptest.NewRequest(fiber.MethodGet, "/v1/readyz", nil)) + utils.AssertEqual(t, nil, err) + utils.AssertEqual(t, fiber.StatusOK, req.StatusCode) + + req, err = app.Test(httptest.NewRequest(fiber.MethodGet, "/v1/livez", nil)) + utils.AssertEqual(t, nil, err) + utils.AssertEqual(t, fiber.StatusOK, req.StatusCode) +} + +func Test_HealthCheck_Group_Default(t *testing.T) { + t.Parallel() + + app := fiber.New() + app.Group("/v1", New()) + v2 := app.Group("/v2/") + customer := v2.Group("/customer/") + customer.Use(New()) + + req, err := app.Test(httptest.NewRequest(fiber.MethodGet, "/v1/readyz", nil)) + utils.AssertEqual(t, nil, err) + utils.AssertEqual(t, fiber.StatusOK, req.StatusCode) + + req, err = app.Test(httptest.NewRequest(fiber.MethodGet, "/v1/livez", nil)) + utils.AssertEqual(t, nil, err) + utils.AssertEqual(t, fiber.StatusOK, req.StatusCode) + + req, err = app.Test(httptest.NewRequest(fiber.MethodGet, "/v2/customer/readyz", nil)) + utils.AssertEqual(t, nil, err) + utils.AssertEqual(t, fiber.StatusOK, req.StatusCode) + + req, err = app.Test(httptest.NewRequest(fiber.MethodGet, "/v2/customer/livez", nil)) + utils.AssertEqual(t, nil, err) + utils.AssertEqual(t, fiber.StatusOK, req.StatusCode) +} + func Test_HealthCheck_Default(t *testing.T) { t.Parallel() From 207f9c5f3aef7764d3c936492c6477e4b8fbe5c0 Mon Sep 17 00:00:00 2001 From: Lucas Lemos Date: Sun, 18 Feb 2024 16:55:52 -0300 Subject: [PATCH 2/9] perf: change verification method to improve perf --- middleware/healthcheck/healthcheck.go | 19 ++++++++----------- 1 file changed, 8 insertions(+), 11 deletions(-) diff --git a/middleware/healthcheck/healthcheck.go b/middleware/healthcheck/healthcheck.go index 3744a737c0..bb8c8b2613 100644 --- a/middleware/healthcheck/healthcheck.go +++ b/middleware/healthcheck/healthcheck.go @@ -1,8 +1,7 @@ package healthcheck import ( - "fmt" - "regexp" + "strings" "github.com/gofiber/fiber/v2" ) @@ -30,11 +29,6 @@ func healthCheckerHandler(checker HealthChecker) fiber.Handler { func New(config ...Config) fiber.Handler { cfg := defaultConfig(config...) - baseRegexp := `\%s$` - - readinessIdentifier := regexp.MustCompile(fmt.Sprintf(baseRegexp, cfg.ReadinessEndpoint)) - livenessIdentifier := regexp.MustCompile(fmt.Sprintf(baseRegexp, cfg.LivenessEndpoint)) - isLiveHandler := healthCheckerHandler(cfg.LivenessProbe) isReadyHandler := healthCheckerHandler(cfg.ReadinessProbe) @@ -48,11 +42,14 @@ func New(config ...Config) fiber.Handler { return c.Next() } - if readinessIdentifier.Match([]byte(c.Path())) { - return isReadyHandler(c) - } + // Find the start of the last segment + lastSlashIndex := strings.LastIndex(c.Path(), "/") + lastSegment := c.Path()[lastSlashIndex:] // +1 to skip the slash itself - if livenessIdentifier.Match([]byte(c.Path())) { + // Direct string comparison for the last segment of the path + if lastSegment == cfg.ReadinessEndpoint { + return isReadyHandler(c) + } else if lastSegment == cfg.LivenessEndpoint { return isLiveHandler(c) } From ec9554143bd95bc69c9fe80907682010d9674952 Mon Sep 17 00:00:00 2001 From: Juan Calderon-Perez <835733+gaby@users.noreply.github.com> Date: Sun, 18 Feb 2024 14:59:18 -0500 Subject: [PATCH 3/9] Update healthcheck_test.go --- middleware/healthcheck/healthcheck_test.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/middleware/healthcheck/healthcheck_test.go b/middleware/healthcheck/healthcheck_test.go index 03b29da08f..5c4bc01ada 100644 --- a/middleware/healthcheck/healthcheck_test.go +++ b/middleware/healthcheck/healthcheck_test.go @@ -33,8 +33,8 @@ func Test_HealthCheck_Group_Default(t *testing.T) { app := fiber.New() app.Group("/v1", New()) - v2 := app.Group("/v2/") - customer := v2.Group("/customer/") + v2Group := app.Group("/v2/") + customer := v2Group.Group("/customer/") customer.Use(New()) req, err := app.Test(httptest.NewRequest(fiber.MethodGet, "/v1/readyz", nil)) From 816ed8ab4f07670c88a1ff8829139725a9e5a942 Mon Sep 17 00:00:00 2001 From: Lucas Lemos Date: Sun, 18 Feb 2024 17:04:00 -0300 Subject: [PATCH 4/9] test: add not matching route test for strict routing --- middleware/healthcheck/healthcheck_test.go | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/middleware/healthcheck/healthcheck_test.go b/middleware/healthcheck/healthcheck_test.go index 5c4bc01ada..c05e4cc84f 100644 --- a/middleware/healthcheck/healthcheck_test.go +++ b/middleware/healthcheck/healthcheck_test.go @@ -26,6 +26,14 @@ func Test_HealthCheck_Strict_Routing_Default(t *testing.T) { req, err = app.Test(httptest.NewRequest(fiber.MethodGet, "/v1/livez", nil)) utils.AssertEqual(t, nil, err) utils.AssertEqual(t, fiber.StatusOK, req.StatusCode) + + req, err = app.Test(httptest.NewRequest(fiber.MethodGet, "/v1/livez/", nil)) + utils.AssertEqual(t, nil, err) + utils.AssertEqual(t, fiber.StatusNotFound, req.StatusCode) + + req, err = app.Test(httptest.NewRequest(fiber.MethodGet, "/v1/readyz/", nil)) + utils.AssertEqual(t, nil, err) + utils.AssertEqual(t, fiber.StatusNotFound, req.StatusCode) } func Test_HealthCheck_Group_Default(t *testing.T) { From 1b1bc46432d0907e44579ff87d42ee87c4c6ac96 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ren=C3=A9=20Werner?= Date: Sun, 18 Feb 2024 21:18:35 +0100 Subject: [PATCH 5/9] add more test cases --- middleware/healthcheck/healthcheck_test.go | 89 ++++++++++------------ 1 file changed, 41 insertions(+), 48 deletions(-) diff --git a/middleware/healthcheck/healthcheck_test.go b/middleware/healthcheck/healthcheck_test.go index c05e4cc84f..d68a280780 100644 --- a/middleware/healthcheck/healthcheck_test.go +++ b/middleware/healthcheck/healthcheck_test.go @@ -10,6 +10,18 @@ import ( "github.com/valyala/fasthttp" ) +func shouldWork(app *fiber.App, t *testing.T, path string) { + req, err := app.Test(httptest.NewRequest(fiber.MethodGet, path, nil)) + utils.AssertEqual(t, nil, err) + utils.AssertEqual(t, fiber.StatusOK, req.StatusCode) +} + +func shouldNotWork(app *fiber.App, t *testing.T, path string) { + req, err := app.Test(httptest.NewRequest(fiber.MethodGet, path, nil)) + utils.AssertEqual(t, nil, err) + utils.AssertEqual(t, fiber.StatusNotFound, req.StatusCode) +} + func Test_HealthCheck_Strict_Routing_Default(t *testing.T) { t.Parallel() @@ -19,21 +31,12 @@ func Test_HealthCheck_Strict_Routing_Default(t *testing.T) { app.Use(New()) - req, err := app.Test(httptest.NewRequest(fiber.MethodGet, "/v1/readyz", nil)) - utils.AssertEqual(t, nil, err) - utils.AssertEqual(t, fiber.StatusOK, req.StatusCode) - - req, err = app.Test(httptest.NewRequest(fiber.MethodGet, "/v1/livez", nil)) - utils.AssertEqual(t, nil, err) - utils.AssertEqual(t, fiber.StatusOK, req.StatusCode) - - req, err = app.Test(httptest.NewRequest(fiber.MethodGet, "/v1/livez/", nil)) - utils.AssertEqual(t, nil, err) - utils.AssertEqual(t, fiber.StatusNotFound, req.StatusCode) - - req, err = app.Test(httptest.NewRequest(fiber.MethodGet, "/v1/readyz/", nil)) - utils.AssertEqual(t, nil, err) - utils.AssertEqual(t, fiber.StatusNotFound, req.StatusCode) + shouldWork(app, t, "/readyz") + shouldWork(app, t, "/livez") + shouldNotWork(app, t, "/readyz/") + shouldNotWork(app, t, "/livez/") + shouldNotWork(app, t, "/notDefined/readyz") + shouldNotWork(app, t, "/notDefined/livez") } func Test_HealthCheck_Group_Default(t *testing.T) { @@ -45,21 +48,18 @@ func Test_HealthCheck_Group_Default(t *testing.T) { customer := v2Group.Group("/customer/") customer.Use(New()) - req, err := app.Test(httptest.NewRequest(fiber.MethodGet, "/v1/readyz", nil)) - utils.AssertEqual(t, nil, err) - utils.AssertEqual(t, fiber.StatusOK, req.StatusCode) - - req, err = app.Test(httptest.NewRequest(fiber.MethodGet, "/v1/livez", nil)) - utils.AssertEqual(t, nil, err) - utils.AssertEqual(t, fiber.StatusOK, req.StatusCode) - - req, err = app.Test(httptest.NewRequest(fiber.MethodGet, "/v2/customer/readyz", nil)) - utils.AssertEqual(t, nil, err) - utils.AssertEqual(t, fiber.StatusOK, req.StatusCode) - - req, err = app.Test(httptest.NewRequest(fiber.MethodGet, "/v2/customer/livez", nil)) - utils.AssertEqual(t, nil, err) - utils.AssertEqual(t, fiber.StatusOK, req.StatusCode) + shouldWork(app, t, "/v1/readyz") + shouldWork(app, t, "/v1/livez") + shouldWork(app, t, "/v1/readyz/") + shouldWork(app, t, "/v1/livez/") + shouldWork(app, t, "/v2/customer/readyz") + shouldWork(app, t, "/v2/customer/livez") + shouldWork(app, t, "/v2/customer/readyz/") + shouldWork(app, t, "/v2/customer/livez/") + shouldNotWork(app, t, "/v2/customer/readyz/") + shouldNotWork(app, t, "/v2/customer/livez/") + shouldNotWork(app, t, "/notDefined/readyz") + shouldNotWork(app, t, "/notDefined/livez") } func Test_HealthCheck_Default(t *testing.T) { @@ -68,13 +68,12 @@ func Test_HealthCheck_Default(t *testing.T) { app := fiber.New() app.Use(New()) - req, err := app.Test(httptest.NewRequest(fiber.MethodGet, "/readyz", nil)) - utils.AssertEqual(t, nil, err) - utils.AssertEqual(t, fiber.StatusOK, req.StatusCode) - - req, err = app.Test(httptest.NewRequest(fiber.MethodGet, "/livez", nil)) - utils.AssertEqual(t, nil, err) - utils.AssertEqual(t, fiber.StatusOK, req.StatusCode) + shouldWork(app, t, "/readyz") + shouldWork(app, t, "/livez") + shouldWork(app, t, "/readyz/") + shouldWork(app, t, "/livez/") + shouldNotWork(app, t, "/notDefined/readyz") + shouldNotWork(app, t, "/notDefined/livez") } func Test_HealthCheck_Custom(t *testing.T) { @@ -105,12 +104,9 @@ func Test_HealthCheck_Custom(t *testing.T) { })) // Live should return 200 with GET request - req, err := app.Test(httptest.NewRequest(fiber.MethodGet, "/live", nil)) - utils.AssertEqual(t, nil, err) - utils.AssertEqual(t, fiber.StatusOK, req.StatusCode) - + shouldWork(app, t, "/live") // Live should return 404 with POST request - req, err = app.Test(httptest.NewRequest(fiber.MethodPost, "/live", nil)) + req, err := app.Test(httptest.NewRequest(fiber.MethodPost, "/live", nil)) utils.AssertEqual(t, nil, err) utils.AssertEqual(t, fiber.StatusNotFound, req.StatusCode) @@ -127,9 +123,7 @@ func Test_HealthCheck_Custom(t *testing.T) { time.Sleep(1 * time.Second) // Ready should return 200 with GET request after the channel is closed - req, err = app.Test(httptest.NewRequest(fiber.MethodGet, "/ready", nil)) - utils.AssertEqual(t, nil, err) - utils.AssertEqual(t, fiber.StatusOK, req.StatusCode) + shouldWork(app, t, "/ready") } func Test_HealthCheck_Next(t *testing.T) { @@ -143,9 +137,8 @@ func Test_HealthCheck_Next(t *testing.T) { }, })) - req, err := app.Test(httptest.NewRequest(fiber.MethodGet, "/livez", nil)) - utils.AssertEqual(t, nil, err) - utils.AssertEqual(t, fiber.StatusNotFound, req.StatusCode) + shouldNotWork(app, t, "/readyz") + shouldNotWork(app, t, "/livez") } func Benchmark_HealthCheck(b *testing.B) { From 0b92ab881af80d13fbe0b526b9d18a06969fc803 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ren=C3=A9=20Werner?= Date: Sun, 18 Feb 2024 21:32:57 +0100 Subject: [PATCH 6/9] correct tests --- middleware/healthcheck/healthcheck.go | 24 ++++++++++--------- middleware/healthcheck/healthcheck_test.go | 28 ++++++++++++++++++++-- 2 files changed, 39 insertions(+), 13 deletions(-) diff --git a/middleware/healthcheck/healthcheck.go b/middleware/healthcheck/healthcheck.go index bb8c8b2613..09d1c4283b 100644 --- a/middleware/healthcheck/healthcheck.go +++ b/middleware/healthcheck/healthcheck.go @@ -1,9 +1,8 @@ package healthcheck import ( - "strings" - "github.com/gofiber/fiber/v2" + "github.com/gofiber/fiber/v2/utils" ) // HealthChecker defines a function to check liveness or readiness of the application @@ -42,15 +41,18 @@ func New(config ...Config) fiber.Handler { return c.Next() } - // Find the start of the last segment - lastSlashIndex := strings.LastIndex(c.Path(), "/") - lastSegment := c.Path()[lastSlashIndex:] // +1 to skip the slash itself - - // Direct string comparison for the last segment of the path - if lastSegment == cfg.ReadinessEndpoint { - return isReadyHandler(c) - } else if lastSegment == cfg.LivenessEndpoint { - return isLiveHandler(c) + prefixCount := len(utils.TrimRight(c.Route().Path, '/')) + if len(c.Path()) >= prefixCount { + checkPath := c.Path()[prefixCount:] + if !c.App().Config().StrictRouting { + checkPath = utils.TrimRight(checkPath, '/') + } + switch checkPath { + case cfg.ReadinessEndpoint: + return isReadyHandler(c) + case cfg.LivenessEndpoint: + return isLiveHandler(c) + } } return c.Next() diff --git a/middleware/healthcheck/healthcheck_test.go b/middleware/healthcheck/healthcheck_test.go index d68a280780..5d0f8c80f7 100644 --- a/middleware/healthcheck/healthcheck_test.go +++ b/middleware/healthcheck/healthcheck_test.go @@ -11,15 +11,17 @@ import ( ) func shouldWork(app *fiber.App, t *testing.T, path string) { + t.Helper() req, err := app.Test(httptest.NewRequest(fiber.MethodGet, path, nil)) utils.AssertEqual(t, nil, err) - utils.AssertEqual(t, fiber.StatusOK, req.StatusCode) + utils.AssertEqual(t, fiber.StatusOK, req.StatusCode, "path: "+path+" should match") } func shouldNotWork(app *fiber.App, t *testing.T, path string) { + t.Helper() req, err := app.Test(httptest.NewRequest(fiber.MethodGet, path, nil)) utils.AssertEqual(t, nil, err) - utils.AssertEqual(t, fiber.StatusNotFound, req.StatusCode) + utils.AssertEqual(t, fiber.StatusNotFound, req.StatusCode, "path: "+path+" should not match") } func Test_HealthCheck_Strict_Routing_Default(t *testing.T) { @@ -56,10 +58,32 @@ func Test_HealthCheck_Group_Default(t *testing.T) { shouldWork(app, t, "/v2/customer/livez") shouldWork(app, t, "/v2/customer/readyz/") shouldWork(app, t, "/v2/customer/livez/") + shouldNotWork(app, t, "/notDefined/readyz") + shouldNotWork(app, t, "/notDefined/livez") + shouldNotWork(app, t, "/notDefined/readyz/") + shouldNotWork(app, t, "/notDefined/livez/") + + // strict routing + app = fiber.New(fiber.Config{ + StrictRouting: true, + }) + app.Group("/v1", New()) + v2Group = app.Group("/v2/") + customer = v2Group.Group("/customer/") + customer.Use(New()) + + shouldWork(app, t, "/v1/readyz") + shouldWork(app, t, "/v1/livez") + shouldNotWork(app, t, "/v1/readyz/") + shouldNotWork(app, t, "/v1/livez/") + shouldWork(app, t, "/v2/customer/readyz") + shouldWork(app, t, "/v2/customer/livez") shouldNotWork(app, t, "/v2/customer/readyz/") shouldNotWork(app, t, "/v2/customer/livez/") shouldNotWork(app, t, "/notDefined/readyz") shouldNotWork(app, t, "/notDefined/livez") + shouldNotWork(app, t, "/notDefined/readyz/") + shouldNotWork(app, t, "/notDefined/livez/") } func Test_HealthCheck_Default(t *testing.T) { From 19282aba207c942e9f9071b7647f90ff37cb7007 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ren=C3=A9=20Werner?= Date: Sun, 18 Feb 2024 21:38:07 +0100 Subject: [PATCH 7/9] correct test helpers --- middleware/healthcheck/healthcheck_test.go | 84 +++++++++++----------- 1 file changed, 42 insertions(+), 42 deletions(-) diff --git a/middleware/healthcheck/healthcheck_test.go b/middleware/healthcheck/healthcheck_test.go index 5d0f8c80f7..f51439bd50 100644 --- a/middleware/healthcheck/healthcheck_test.go +++ b/middleware/healthcheck/healthcheck_test.go @@ -10,14 +10,14 @@ import ( "github.com/valyala/fasthttp" ) -func shouldWork(app *fiber.App, t *testing.T, path string) { +func shouldWork(t *testing.T, app *fiber.App, path string) { t.Helper() req, err := app.Test(httptest.NewRequest(fiber.MethodGet, path, nil)) utils.AssertEqual(t, nil, err) utils.AssertEqual(t, fiber.StatusOK, req.StatusCode, "path: "+path+" should match") } -func shouldNotWork(app *fiber.App, t *testing.T, path string) { +func shouldNotWork(t *testing.T, app *fiber.App, path string) { t.Helper() req, err := app.Test(httptest.NewRequest(fiber.MethodGet, path, nil)) utils.AssertEqual(t, nil, err) @@ -33,12 +33,12 @@ func Test_HealthCheck_Strict_Routing_Default(t *testing.T) { app.Use(New()) - shouldWork(app, t, "/readyz") - shouldWork(app, t, "/livez") - shouldNotWork(app, t, "/readyz/") - shouldNotWork(app, t, "/livez/") - shouldNotWork(app, t, "/notDefined/readyz") - shouldNotWork(app, t, "/notDefined/livez") + shouldWork(t, app, "/readyz") + shouldWork(t, app, "/livez") + shouldNotWork(t, app, "/readyz/") + shouldNotWork(t, app, "/livez/") + shouldNotWork(t, app, "/notDefined/readyz") + shouldNotWork(t, app, "/notDefined/livez") } func Test_HealthCheck_Group_Default(t *testing.T) { @@ -50,18 +50,18 @@ func Test_HealthCheck_Group_Default(t *testing.T) { customer := v2Group.Group("/customer/") customer.Use(New()) - shouldWork(app, t, "/v1/readyz") - shouldWork(app, t, "/v1/livez") - shouldWork(app, t, "/v1/readyz/") - shouldWork(app, t, "/v1/livez/") - shouldWork(app, t, "/v2/customer/readyz") - shouldWork(app, t, "/v2/customer/livez") - shouldWork(app, t, "/v2/customer/readyz/") - shouldWork(app, t, "/v2/customer/livez/") - shouldNotWork(app, t, "/notDefined/readyz") - shouldNotWork(app, t, "/notDefined/livez") - shouldNotWork(app, t, "/notDefined/readyz/") - shouldNotWork(app, t, "/notDefined/livez/") + shouldWork(t, app, "/v1/readyz") + shouldWork(t, app, "/v1/livez") + shouldWork(t, app, "/v1/readyz/") + shouldWork(t, app, "/v1/livez/") + shouldWork(t, app, "/v2/customer/readyz") + shouldWork(t, app, "/v2/customer/livez") + shouldWork(t, app, "/v2/customer/readyz/") + shouldWork(t, app, "/v2/customer/livez/") + shouldNotWork(t, app, "/notDefined/readyz") + shouldNotWork(t, app, "/notDefined/livez") + shouldNotWork(t, app, "/notDefined/readyz/") + shouldNotWork(t, app, "/notDefined/livez/") // strict routing app = fiber.New(fiber.Config{ @@ -72,18 +72,18 @@ func Test_HealthCheck_Group_Default(t *testing.T) { customer = v2Group.Group("/customer/") customer.Use(New()) - shouldWork(app, t, "/v1/readyz") - shouldWork(app, t, "/v1/livez") - shouldNotWork(app, t, "/v1/readyz/") - shouldNotWork(app, t, "/v1/livez/") - shouldWork(app, t, "/v2/customer/readyz") - shouldWork(app, t, "/v2/customer/livez") - shouldNotWork(app, t, "/v2/customer/readyz/") - shouldNotWork(app, t, "/v2/customer/livez/") - shouldNotWork(app, t, "/notDefined/readyz") - shouldNotWork(app, t, "/notDefined/livez") - shouldNotWork(app, t, "/notDefined/readyz/") - shouldNotWork(app, t, "/notDefined/livez/") + shouldWork(t, app, "/v1/readyz") + shouldWork(t, app, "/v1/livez") + shouldNotWork(t, app, "/v1/readyz/") + shouldNotWork(t, app, "/v1/livez/") + shouldWork(t, app, "/v2/customer/readyz") + shouldWork(t, app, "/v2/customer/livez") + shouldNotWork(t, app, "/v2/customer/readyz/") + shouldNotWork(t, app, "/v2/customer/livez/") + shouldNotWork(t, app, "/notDefined/readyz") + shouldNotWork(t, app, "/notDefined/livez") + shouldNotWork(t, app, "/notDefined/readyz/") + shouldNotWork(t, app, "/notDefined/livez/") } func Test_HealthCheck_Default(t *testing.T) { @@ -92,12 +92,12 @@ func Test_HealthCheck_Default(t *testing.T) { app := fiber.New() app.Use(New()) - shouldWork(app, t, "/readyz") - shouldWork(app, t, "/livez") - shouldWork(app, t, "/readyz/") - shouldWork(app, t, "/livez/") - shouldNotWork(app, t, "/notDefined/readyz") - shouldNotWork(app, t, "/notDefined/livez") + shouldWork(t, app, "/readyz") + shouldWork(t, app, "/livez") + shouldWork(t, app, "/readyz/") + shouldWork(t, app, "/livez/") + shouldNotWork(t, app, "/notDefined/readyz") + shouldNotWork(t, app, "/notDefined/livez") } func Test_HealthCheck_Custom(t *testing.T) { @@ -128,7 +128,7 @@ func Test_HealthCheck_Custom(t *testing.T) { })) // Live should return 200 with GET request - shouldWork(app, t, "/live") + shouldWork(t, app, "/live") // Live should return 404 with POST request req, err := app.Test(httptest.NewRequest(fiber.MethodPost, "/live", nil)) utils.AssertEqual(t, nil, err) @@ -147,7 +147,7 @@ func Test_HealthCheck_Custom(t *testing.T) { time.Sleep(1 * time.Second) // Ready should return 200 with GET request after the channel is closed - shouldWork(app, t, "/ready") + shouldWork(t, app, "/ready") } func Test_HealthCheck_Next(t *testing.T) { @@ -161,8 +161,8 @@ func Test_HealthCheck_Next(t *testing.T) { }, })) - shouldNotWork(app, t, "/readyz") - shouldNotWork(app, t, "/livez") + shouldNotWork(t, app, "/readyz") + shouldNotWork(t, app, "/livez") } func Benchmark_HealthCheck(b *testing.B) { From b6de02af03b5210a4e2a5cf73098518a88d2c3fe Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ren=C3=A9=20Werner?= Date: Sun, 18 Feb 2024 22:12:55 +0100 Subject: [PATCH 8/9] correct tests --- middleware/healthcheck/healthcheck.go | 9 +- middleware/healthcheck/healthcheck_test.go | 170 +++++++++++++-------- 2 files changed, 115 insertions(+), 64 deletions(-) diff --git a/middleware/healthcheck/healthcheck.go b/middleware/healthcheck/healthcheck.go index 09d1c4283b..4a95949116 100644 --- a/middleware/healthcheck/healthcheck.go +++ b/middleware/healthcheck/healthcheck.go @@ -44,13 +44,14 @@ func New(config ...Config) fiber.Handler { prefixCount := len(utils.TrimRight(c.Route().Path, '/')) if len(c.Path()) >= prefixCount { checkPath := c.Path()[prefixCount:] + checkPathTrimmed := checkPath if !c.App().Config().StrictRouting { - checkPath = utils.TrimRight(checkPath, '/') + checkPathTrimmed = utils.TrimRight(checkPath, '/') } - switch checkPath { - case cfg.ReadinessEndpoint: + switch true { + case checkPath == cfg.ReadinessEndpoint || checkPathTrimmed == cfg.ReadinessEndpoint: return isReadyHandler(c) - case cfg.LivenessEndpoint: + case checkPath == cfg.LivenessEndpoint || checkPathTrimmed == cfg.LivenessEndpoint: return isLiveHandler(c) } } diff --git a/middleware/healthcheck/healthcheck_test.go b/middleware/healthcheck/healthcheck_test.go index f51439bd50..6718d052bd 100644 --- a/middleware/healthcheck/healthcheck_test.go +++ b/middleware/healthcheck/healthcheck_test.go @@ -1,27 +1,29 @@ package healthcheck import ( - "net/http/httptest" - "testing" - "time" - + "fmt" "github.com/gofiber/fiber/v2" "github.com/gofiber/fiber/v2/utils" "github.com/valyala/fasthttp" + "net/http/httptest" + "testing" ) -func shouldWork(t *testing.T, app *fiber.App, path string) { +func shouldGiveStatus(t *testing.T, app *fiber.App, path string, expectedStatus int) { t.Helper() req, err := app.Test(httptest.NewRequest(fiber.MethodGet, path, nil)) utils.AssertEqual(t, nil, err) - utils.AssertEqual(t, fiber.StatusOK, req.StatusCode, "path: "+path+" should match") + utils.AssertEqual(t, expectedStatus, req.StatusCode, "path: "+path+" should match "+fmt.Sprint(expectedStatus)) } -func shouldNotWork(t *testing.T, app *fiber.App, path string) { +func shouldGiveOK(t *testing.T, app *fiber.App, path string) { t.Helper() - req, err := app.Test(httptest.NewRequest(fiber.MethodGet, path, nil)) - utils.AssertEqual(t, nil, err) - utils.AssertEqual(t, fiber.StatusNotFound, req.StatusCode, "path: "+path+" should not match") + shouldGiveStatus(t, app, path, fiber.StatusOK) +} + +func shouldGiveNotFound(t *testing.T, app *fiber.App, path string) { + t.Helper() + shouldGiveStatus(t, app, path, fiber.StatusNotFound) } func Test_HealthCheck_Strict_Routing_Default(t *testing.T) { @@ -33,12 +35,12 @@ func Test_HealthCheck_Strict_Routing_Default(t *testing.T) { app.Use(New()) - shouldWork(t, app, "/readyz") - shouldWork(t, app, "/livez") - shouldNotWork(t, app, "/readyz/") - shouldNotWork(t, app, "/livez/") - shouldNotWork(t, app, "/notDefined/readyz") - shouldNotWork(t, app, "/notDefined/livez") + shouldGiveOK(t, app, "/readyz") + shouldGiveOK(t, app, "/livez") + shouldGiveNotFound(t, app, "/readyz/") + shouldGiveNotFound(t, app, "/livez/") + shouldGiveNotFound(t, app, "/notDefined/readyz") + shouldGiveNotFound(t, app, "/notDefined/livez") } func Test_HealthCheck_Group_Default(t *testing.T) { @@ -50,18 +52,25 @@ func Test_HealthCheck_Group_Default(t *testing.T) { customer := v2Group.Group("/customer/") customer.Use(New()) - shouldWork(t, app, "/v1/readyz") - shouldWork(t, app, "/v1/livez") - shouldWork(t, app, "/v1/readyz/") - shouldWork(t, app, "/v1/livez/") - shouldWork(t, app, "/v2/customer/readyz") - shouldWork(t, app, "/v2/customer/livez") - shouldWork(t, app, "/v2/customer/readyz/") - shouldWork(t, app, "/v2/customer/livez/") - shouldNotWork(t, app, "/notDefined/readyz") - shouldNotWork(t, app, "/notDefined/livez") - shouldNotWork(t, app, "/notDefined/readyz/") - shouldNotWork(t, app, "/notDefined/livez/") + v3Group := app.Group("/v3/") + v3Group.Group("/todos/", New(Config{ReadinessEndpoint: "/readyz/", LivenessEndpoint: "/livez/"})) + + shouldGiveOK(t, app, "/v1/readyz") + shouldGiveOK(t, app, "/v1/livez") + shouldGiveOK(t, app, "/v1/readyz/") + shouldGiveOK(t, app, "/v1/livez/") + shouldGiveOK(t, app, "/v2/customer/readyz") + shouldGiveOK(t, app, "/v2/customer/livez") + shouldGiveOK(t, app, "/v2/customer/readyz/") + shouldGiveOK(t, app, "/v2/customer/livez/") + shouldGiveNotFound(t, app, "/v3/todos/readyz") + shouldGiveNotFound(t, app, "/v3/todos/livez") + shouldGiveOK(t, app, "/v3/todos/readyz/") + shouldGiveOK(t, app, "/v3/todos/livez/") + shouldGiveNotFound(t, app, "/notDefined/readyz") + shouldGiveNotFound(t, app, "/notDefined/livez") + shouldGiveNotFound(t, app, "/notDefined/readyz/") + shouldGiveNotFound(t, app, "/notDefined/livez/") // strict routing app = fiber.New(fiber.Config{ @@ -72,18 +81,25 @@ func Test_HealthCheck_Group_Default(t *testing.T) { customer = v2Group.Group("/customer/") customer.Use(New()) - shouldWork(t, app, "/v1/readyz") - shouldWork(t, app, "/v1/livez") - shouldNotWork(t, app, "/v1/readyz/") - shouldNotWork(t, app, "/v1/livez/") - shouldWork(t, app, "/v2/customer/readyz") - shouldWork(t, app, "/v2/customer/livez") - shouldNotWork(t, app, "/v2/customer/readyz/") - shouldNotWork(t, app, "/v2/customer/livez/") - shouldNotWork(t, app, "/notDefined/readyz") - shouldNotWork(t, app, "/notDefined/livez") - shouldNotWork(t, app, "/notDefined/readyz/") - shouldNotWork(t, app, "/notDefined/livez/") + v3Group = app.Group("/v3/") + v3Group.Group("/todos/", New(Config{ReadinessEndpoint: "/readyz/", LivenessEndpoint: "/livez/"})) + + shouldGiveOK(t, app, "/v1/readyz") + shouldGiveOK(t, app, "/v1/livez") + shouldGiveNotFound(t, app, "/v1/readyz/") + shouldGiveNotFound(t, app, "/v1/livez/") + shouldGiveOK(t, app, "/v2/customer/readyz") + shouldGiveOK(t, app, "/v2/customer/livez") + shouldGiveNotFound(t, app, "/v2/customer/readyz/") + shouldGiveNotFound(t, app, "/v2/customer/livez/") + shouldGiveNotFound(t, app, "/v3/todos/readyz") + shouldGiveNotFound(t, app, "/v3/todos/livez") + shouldGiveOK(t, app, "/v3/todos/readyz/") + shouldGiveOK(t, app, "/v3/todos/livez/") + shouldGiveNotFound(t, app, "/notDefined/readyz") + shouldGiveNotFound(t, app, "/notDefined/livez") + shouldGiveNotFound(t, app, "/notDefined/readyz/") + shouldGiveNotFound(t, app, "/notDefined/livez/") } func Test_HealthCheck_Default(t *testing.T) { @@ -92,12 +108,12 @@ func Test_HealthCheck_Default(t *testing.T) { app := fiber.New() app.Use(New()) - shouldWork(t, app, "/readyz") - shouldWork(t, app, "/livez") - shouldWork(t, app, "/readyz/") - shouldWork(t, app, "/livez/") - shouldNotWork(t, app, "/notDefined/readyz") - shouldNotWork(t, app, "/notDefined/livez") + shouldGiveOK(t, app, "/readyz") + shouldGiveOK(t, app, "/livez") + shouldGiveOK(t, app, "/readyz/") + shouldGiveOK(t, app, "/livez/") + shouldGiveNotFound(t, app, "/notDefined/readyz") + shouldGiveNotFound(t, app, "/notDefined/livez") } func Test_HealthCheck_Custom(t *testing.T) { @@ -106,11 +122,6 @@ func Test_HealthCheck_Custom(t *testing.T) { app := fiber.New() c1 := make(chan struct{}, 1) - go func() { - time.Sleep(1 * time.Second) - c1 <- struct{}{} - }() - app.Use(New(Config{ LivenessProbe: func(c *fiber.Ctx) bool { return true @@ -128,7 +139,7 @@ func Test_HealthCheck_Custom(t *testing.T) { })) // Live should return 200 with GET request - shouldWork(t, app, "/live") + shouldGiveOK(t, app, "/live") // Live should return 404 with POST request req, err := app.Test(httptest.NewRequest(fiber.MethodPost, "/live", nil)) utils.AssertEqual(t, nil, err) @@ -140,14 +151,53 @@ func Test_HealthCheck_Custom(t *testing.T) { utils.AssertEqual(t, fiber.StatusNotFound, req.StatusCode) // Ready should return 503 with GET request before the channel is closed - req, err = app.Test(httptest.NewRequest(fiber.MethodGet, "/ready", nil)) - utils.AssertEqual(t, nil, err) - utils.AssertEqual(t, fiber.StatusServiceUnavailable, req.StatusCode) - - time.Sleep(1 * time.Second) + shouldGiveStatus(t, app, "/ready", fiber.StatusServiceUnavailable) // Ready should return 200 with GET request after the channel is closed - shouldWork(t, app, "/ready") + c1 <- struct{}{} + shouldGiveOK(t, app, "/ready") +} + +func Test_HealthCheck_Custom_Nested(t *testing.T) { + t.Parallel() + + app := fiber.New() + + c1 := make(chan struct{}, 1) + + app.Use(New(Config{ + LivenessProbe: func(c *fiber.Ctx) bool { + return true + }, + LivenessEndpoint: "/probe/live", + ReadinessProbe: func(c *fiber.Ctx) bool { + select { + case <-c1: + return true + default: + return false + } + }, + ReadinessEndpoint: "/probe/ready", + })) + + shouldGiveOK(t, app, "/probe/live") + shouldGiveStatus(t, app, "/probe/ready", fiber.StatusServiceUnavailable) + shouldGiveOK(t, app, "/probe/live/") + shouldGiveStatus(t, app, "/probe/ready/", fiber.StatusServiceUnavailable) + shouldGiveNotFound(t, app, "/probe/livez") + shouldGiveNotFound(t, app, "/probe/readyz") + shouldGiveNotFound(t, app, "/probe/livez/") + shouldGiveNotFound(t, app, "/probe/readyz/") + shouldGiveNotFound(t, app, "/livez") + shouldGiveNotFound(t, app, "/readyz") + shouldGiveNotFound(t, app, "/readyz/") + shouldGiveNotFound(t, app, "/livez/") + + c1 <- struct{}{} + shouldGiveOK(t, app, "/probe/ready") + c1 <- struct{}{} + shouldGiveOK(t, app, "/probe/ready/") } func Test_HealthCheck_Next(t *testing.T) { @@ -161,8 +211,8 @@ func Test_HealthCheck_Next(t *testing.T) { }, })) - shouldNotWork(t, app, "/readyz") - shouldNotWork(t, app, "/livez") + shouldGiveNotFound(t, app, "/readyz") + shouldGiveNotFound(t, app, "/livez") } func Benchmark_HealthCheck(b *testing.B) { From ed3cc2757840b538bb4d3748bfe1e61f9860f293 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ren=C3=A9=20Werner?= Date: Sun, 18 Feb 2024 22:16:38 +0100 Subject: [PATCH 9/9] correct tests --- middleware/healthcheck/healthcheck.go | 2 +- middleware/healthcheck/healthcheck_test.go | 5 +++-- 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/middleware/healthcheck/healthcheck.go b/middleware/healthcheck/healthcheck.go index 4a95949116..c9d6a6476b 100644 --- a/middleware/healthcheck/healthcheck.go +++ b/middleware/healthcheck/healthcheck.go @@ -48,7 +48,7 @@ func New(config ...Config) fiber.Handler { if !c.App().Config().StrictRouting { checkPathTrimmed = utils.TrimRight(checkPath, '/') } - switch true { + switch { case checkPath == cfg.ReadinessEndpoint || checkPathTrimmed == cfg.ReadinessEndpoint: return isReadyHandler(c) case checkPath == cfg.LivenessEndpoint || checkPathTrimmed == cfg.LivenessEndpoint: diff --git a/middleware/healthcheck/healthcheck_test.go b/middleware/healthcheck/healthcheck_test.go index 6718d052bd..84fbb43da0 100644 --- a/middleware/healthcheck/healthcheck_test.go +++ b/middleware/healthcheck/healthcheck_test.go @@ -2,11 +2,12 @@ package healthcheck import ( "fmt" + "net/http/httptest" + "testing" + "github.com/gofiber/fiber/v2" "github.com/gofiber/fiber/v2/utils" "github.com/valyala/fasthttp" - "net/http/httptest" - "testing" ) func shouldGiveStatus(t *testing.T, app *fiber.App, path string, expectedStatus int) {