diff --git a/app/other/router/monitor.go b/app/other/router/monitor.go index 999f011d..b2d87556 100644 --- a/app/other/router/monitor.go +++ b/app/other/router/monitor.go @@ -13,7 +13,7 @@ import ( ) func init() { - routerNoCheckRole = append(routerNoCheckRole, registerMonitorRouter) + routerNoCheckRole = append(routerNoCheckRole, RegisterMonitorRouter) } // readyTimeout bounds the whole probe. It has to stay under whatever period @@ -21,8 +21,23 @@ func init() { // a queue of readiness checks. const readyTimeout = 2 * time.Second +// HealthPath and ReadyPath are the two probe routes, relative to APIPrefix. +// +// Exported for the same reason as the prefix: the rate limiter has to be told +// to skip them, and it is installed in a package that cannot import this one. +const ( + HealthPath = "/health" + ReadyPath = "/ready" +) + +// RegisterMonitorRouter mounts the metrics endpoint and the two probes on v1. +// +// Exported so that a test can put the real probes on a server of its own. The +// alternative - a test that re-implements the handler it means to check - is +// how a probe comes to be asserted against a copy of itself. +// // 无需认证的路由代码 -func registerMonitorRouter(v1 *gin.RouterGroup) { +func RegisterMonitorRouter(v1 *gin.RouterGroup) { v1.GET("/metrics", transfer.Handler(promhttp.Handler())) // 健康检查（存活） @@ -31,7 +46,7 @@ func registerMonitorRouter(v1 *gin.RouterGroup) { // you", and a process whose database is unreachable does not want // restarting - that turns one outage into a crash loop and throws away the // connection pool, the cache and every in-flight request along the way. - v1.GET("/health", func(c *gin.Context) { + v1.GET(HealthPath, func(c *gin.Context) { c.Status(http.StatusOK) }) @@ -41,7 +56,7 @@ func registerMonitorRouter(v1 *gin.RouterGroup) { // is unreachable, and from the moment shutdown begins. Nothing waits on // that second answer today, so it is readable rather than actionable - // the package comment in common/health says what it would take. - v1.GET("/ready", func(c *gin.Context) { + v1.GET(ReadyPath, func(c *gin.Context) { if health.Draining() { c.JSON(http.StatusServiceUnavailable, gin.H{ "status": "draining", diff --git a/app/other/router/router.go b/app/other/router/router.go index 8ed4306b..8897eb5b 100644 --- a/app/other/router/router.go +++ b/app/other/router/router.go @@ -10,6 +10,13 @@ var ( routerCheckRole = make([]func(v1 *gin.RouterGroup, authMiddleware *jwt.GinJWTMiddleware), 0) ) +// APIPrefix is the group every route below is registered under. +// +// Exported because the middleware chain in cmd/api has to name two of those +// routes in full - the rate limiter is installed on the engine and must skip +// the probes - and a prefix spelled in two places is a prefix that drifts. +const APIPrefix = "/api/v1" + // initRouter 路由示例 func initRouter(r *gin.Engine, authMiddleware *jwt.GinJWTMiddleware) *gin.Engine { @@ -24,7 +31,7 @@ func initRouter(r *gin.Engine, authMiddleware *jwt.GinJWTMiddleware) *gin.Engine // noCheckRoleRouter 无需认证的路由示例 func noCheckRoleRouter(r *gin.Engine) { // 可根据业务需求来设置接口版本 - v1 := r.Group("/api/v1") + v1 := r.Group(APIPrefix) for _, f := range routerNoCheckRole { f(v1) @@ -34,7 +41,7 @@ func noCheckRoleRouter(r *gin.Engine) { // checkRoleRouter 需要认证的路由示例 func checkRoleRouter(r *gin.Engine, authMiddleware *jwt.GinJWTMiddleware) { // 可根据业务需求来设置接口版本 - v1 := r.Group("/api/v1") + v1 := r.Group(APIPrefix) for _, f := range routerCheckRole { f(v1, authMiddleware) diff --git a/cmd/api/server.go b/cmd/api/server.go index 87507968..432788f6 100644 --- a/cmd/api/server.go +++ b/cmd/api/server.go @@ -28,6 +28,7 @@ import ( "go-admin/app/admin/models" "go-admin/app/admin/router" "go-admin/app/jobs" + otherrouter "go-admin/app/other/router" "go-admin/common/database" "go-admin/common/global" "go-admin/common/health" @@ -476,10 +477,38 @@ func initRouter() { r.Use(handler.TlsHandler()) } //r.Use(middleware.Metrics()) - r.Use(common.Sentinel()). + r.Use(exemptProbes(common.Sentinel())). Use(common.RequestId(pkg.TrafficKey)). Use(api.SetRequestLogger) common.InitMiddleware(r) } + +// probePaths are the two routes the rate limiter must not answer for. +var probePaths = map[string]bool{ + otherrouter.APIPrefix + otherrouter.HealthPath: true, + otherrouter.APIPrefix + otherrouter.ReadyPath: true, +} + +// exemptProbes wraps a middleware so the health and readiness routes skip it. +// +// The limiter is installed on the engine and the probes are routes like any +// other, so above the threshold they are answered with 429 as well. A liveness +// probe that collects 429s fails its threshold and the container is restarted, +// which takes capacity out of a deployment that is already short of it and +// pushes the rest closer to the threshold - the limiter working exactly as +// intended is what causes it. It is the argument common/health makes about +// restarting a process whose database is unreachable, applied to load. +// +// Wrapping rather than teaching the limiter about these paths: the limiter +// lives under common/, which may not import the package that registers them. +func exemptProbes(h gin.HandlerFunc) gin.HandlerFunc { + return func(c *gin.Context) { + if probePaths[c.FullPath()] { + c.Next() + return + } + h(c) + } +} diff --git a/cmd/api/shutdown_test.go b/cmd/api/shutdown_test.go index 1112fb65..74df27a5 100644 --- a/cmd/api/shutdown_test.go +++ b/cmd/api/shutdown_test.go @@ -1,9 +1,14 @@ package api import ( + "net/http" + "net/http/httptest" "testing" "time" + "github.com/gin-gonic/gin" + + otherrouter "go-admin/app/other/router" ext "go-admin/config" ) @@ -33,3 +38,65 @@ func TestDefaultBudgetIsTheConfiguredFallback(t *testing.T) { t.Errorf("defaultBudget = %+v, want the unconfigured budget %+v", got, want) } } + +// The rate limiter must not answer for the probes. +// +// It is installed on the engine, so without this the probes are limited like +// any other route and answer 429 above the threshold. A liveness probe that +// collects 429s fails its threshold and the container is restarted - taking +// capacity out of a deployment that is already short of it and pushing the +// rest closer to the threshold. The limiter working exactly as designed is +// what would cause it. +// +// The stand-in rejects everything rather than being a real limiter: what is +// under test is which requests reach it, and a real one would need the traffic +// to cross a threshold before it said anything. +func TestTheProbesSkipTheRateLimiter(t *testing.T) { + gin.SetMode(gin.TestMode) + + var reached []string + r := gin.New() + r.Use(exemptProbes(func(c *gin.Context) { + reached = append(reached, c.FullPath()) + c.AbortWithStatus(http.StatusTooManyRequests) + })) + v1 := r.Group(otherrouter.APIPrefix) + otherrouter.RegisterMonitorRouter(v1) + v1.GET("/business", func(c *gin.Context) { c.Status(http.StatusOK) }) + + for _, tc := range []struct { + path string + limited bool + }{ + {otherrouter.APIPrefix + otherrouter.HealthPath, false}, + {otherrouter.APIPrefix + otherrouter.ReadyPath, false}, + // Not a probe, and deliberately not exempt: the exemption is for the + // two routes an orchestrator acts on, not for everything under + // /api/v1 that happens to be unauthenticated. + {otherrouter.APIPrefix + "/metrics", true}, + {otherrouter.APIPrefix + "/business", true}, + } { + t.Run(tc.path, func(t *testing.T) { + w := httptest.NewRecorder() + r.ServeHTTP(w, httptest.NewRequest(http.MethodGet, tc.path, nil)) + + if tc.limited { + if w.Code != http.StatusTooManyRequests { + t.Errorf("answered %d, want the middleware's 429 - it was skipped for a route that is not a probe", w.Code) + } + return + } + if w.Code == http.StatusTooManyRequests { + t.Errorf("answered 429; a probe that can be rate-limited gets the container restarted under load") + } + }) + } + + // Said separately, because a probe could also answer 429 by itself: what + // has to be true is that the middleware never saw the request. + for _, p := range reached { + if probePaths[p] { + t.Errorf("the middleware ran for %s", p) + } + } +}