mirror of
https://github.com/Rain-kl/OpenFlare.git
synced 2026-10-08 16:46:37 +08:00
fix(logs): collect access logs regardless of plugin order
Global Router.Use middleware is applied at HTTP Start instead of being snapshotted when each route is registered, so risk_control still wraps admin APIs that mount earlier. Access-log collection is enabled by default on SQLite/Postgres, not only ClickHouse.
This commit is contained in:
@@ -80,12 +80,12 @@ func TestRouterExtension(t *testing.T) {
|
|||||||
for _, route := range routes {
|
for _, route := range routes {
|
||||||
if route.Method == "GET" && route.Path == "/api/v1/orders" {
|
if route.Method == "GET" && route.Path == "/api/v1/orders" {
|
||||||
foundOrderGet = true
|
foundOrderGet = true
|
||||||
assert.Equal(t, []any{mGlobal, mAPI, "api_extra_middleware"}, route.Middlewares)
|
assert.Equal(t, []any{mAPI, "api_extra_middleware"}, route.Middlewares)
|
||||||
assert.Equal(t, []any{hList}, route.Handlers)
|
assert.Equal(t, []any{hList}, route.Handlers)
|
||||||
}
|
}
|
||||||
if route.Method == "PUT" && route.Path == "/api/v1/admin/users/:id" {
|
if route.Method == "PUT" && route.Path == "/api/v1/admin/users/:id" {
|
||||||
foundUserPut = true
|
foundUserPut = true
|
||||||
assert.Equal(t, []any{mGlobal, mAPI, "api_extra_middleware", mAdmin}, route.Middlewares)
|
assert.Equal(t, []any{mAPI, "api_extra_middleware", mAdmin}, route.Middlewares)
|
||||||
assert.Equal(t, []any{hUserPut}, route.Handlers)
|
assert.Equal(t, []any{hUserPut}, route.Handlers)
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
@@ -93,6 +93,21 @@ func TestRouterExtension(t *testing.T) {
|
|||||||
assert.True(t, foundUserPut)
|
assert.True(t, foundUserPut)
|
||||||
}
|
}
|
||||||
|
|
||||||
|
func TestRouterGlobalMiddlewareIsNotSnapshottedOntoRoutes(t *testing.T) {
|
||||||
|
r := extpoints.NewRouterRegistry()
|
||||||
|
r.GET("/before", "handler")
|
||||||
|
r.Use("late_global")
|
||||||
|
r.GET("/after", "handler")
|
||||||
|
|
||||||
|
for _, route := range r.Routes() {
|
||||||
|
if len(route.Middlewares) != 0 {
|
||||||
|
t.Errorf("route %s %s Middlewares = %v, want none (globals live on Router.Middlewares)",
|
||||||
|
route.Method, route.Path, route.Middlewares)
|
||||||
|
}
|
||||||
|
}
|
||||||
|
assert.Equal(t, []any{"late_global"}, r.Middlewares())
|
||||||
|
}
|
||||||
|
|
||||||
func TestRouterWhitelist(t *testing.T) {
|
func TestRouterWhitelist(t *testing.T) {
|
||||||
r := extpoints.NewRouterRegistry()
|
r := extpoints.NewRouterRegistry()
|
||||||
require.NotNil(t, r)
|
require.NotNil(t, r)
|
||||||
|
|||||||
@@ -95,11 +95,12 @@ func (r *RouterRegistry) addRoute(method, fullPath string, handlers ...any) Rout
|
|||||||
|
|
||||||
r.nextID++
|
r.nextID++
|
||||||
rd := RouteDefinition{
|
rd := RouteDefinition{
|
||||||
ID: r.nextID,
|
ID: r.nextID,
|
||||||
Method: strings.ToUpper(method),
|
Method: strings.ToUpper(method),
|
||||||
Path: fullPath,
|
Path: fullPath,
|
||||||
Handlers: handlers,
|
Handlers: handlers,
|
||||||
Middlewares: append([]any(nil), r.middlewares...),
|
// Global Router.Use middlewares are applied at HTTP Start from
|
||||||
|
// Router.Middlewares(), so late-registered plugins still wrap earlier routes.
|
||||||
}
|
}
|
||||||
r.routes = append(r.routes, rd)
|
r.routes = append(r.routes, rd)
|
||||||
return rd
|
return rd
|
||||||
@@ -241,9 +242,7 @@ func (g *RouterGroup) addRoute(method, fullPath string, handlers ...any) RouteDe
|
|||||||
g.registry.mu.Lock()
|
g.registry.mu.Lock()
|
||||||
defer g.registry.mu.Unlock()
|
defer g.registry.mu.Unlock()
|
||||||
|
|
||||||
allMiddlewares := make([]any, 0, len(g.registry.middlewares)+len(g.middlewares))
|
allMiddlewares := append([]any(nil), g.middlewares...)
|
||||||
allMiddlewares = append(allMiddlewares, g.registry.middlewares...)
|
|
||||||
allMiddlewares = append(allMiddlewares, g.middlewares...)
|
|
||||||
|
|
||||||
g.registry.nextID++
|
g.registry.nextID++
|
||||||
rd := RouteDefinition{
|
rd := RouteDefinition{
|
||||||
|
|||||||
@@ -42,10 +42,8 @@ func SetChDBForTest(db *gorm.DB) {
|
|||||||
}
|
}
|
||||||
|
|
||||||
func getDB(ctx context.Context) *gorm.DB {
|
func getDB(ctx context.Context) *gorm.DB {
|
||||||
if c, ok := ctx.(*core.Context); ok && c != nil {
|
if s, err := core.InjectFrom[contracts.DBService](ctx); err == nil && s != nil {
|
||||||
if s, err := core.Inject[contracts.DBService](c); err == nil && s != nil {
|
return s.DB(ctx)
|
||||||
return s.DB(ctx)
|
|
||||||
}
|
|
||||||
}
|
}
|
||||||
dbMu.RLock()
|
dbMu.RLock()
|
||||||
s := dbSvc
|
s := dbSvc
|
||||||
|
|||||||
@@ -91,7 +91,9 @@ func (p *Plugin) Apply(ctx *core.Context) error {
|
|||||||
var dbCfg rcDBConfig
|
var dbCfg rcDBConfig
|
||||||
_ = ctx.Config().Bind("database", &dbCfg)
|
_ = ctx.Config().Bind("database", &dbCfg)
|
||||||
|
|
||||||
SetAccessLogEnabled(chCfg.Enabled)
|
// Access logs persist on the active log database (SQLite / Postgres / ClickHouse).
|
||||||
|
// Collection is independent of ClickHouse being enabled.
|
||||||
|
SetAccessLogEnabled(true)
|
||||||
logstore.SetDefaultDatabases(dbCfg.Enabled, chCfg.Enabled)
|
logstore.SetDefaultDatabases(dbCfg.Enabled, chCfg.Enabled)
|
||||||
|
|
||||||
core.Bind[contracts.DBService](ctx, logstore.SetDBService)
|
core.Bind[contracts.DBService](ctx, logstore.SetDBService)
|
||||||
|
|||||||
@@ -35,6 +35,8 @@ func TestRiskControlPluginUnit(t *testing.T) {
|
|||||||
setting, ok := ctx.Settings().Get("risk_control.enable_access_log")
|
setting, ok := ctx.Settings().Get("risk_control.enable_access_log")
|
||||||
require.True(t, ok)
|
require.True(t, ok)
|
||||||
assert.Equal(t, true, setting.Default)
|
assert.Equal(t, true, setting.Default)
|
||||||
|
assert.True(t, risk_control.IsAccessLogEnabled(),
|
||||||
|
"access log collection must be on even when ClickHouse is disabled")
|
||||||
|
|
||||||
require.NoError(t, ctx.Dispose())
|
require.NoError(t, ctx.Dispose())
|
||||||
assert.False(t, customMWCalled) // not dispatched via gin engine here
|
assert.False(t, customMWCalled) // not dispatched via gin engine here
|
||||||
|
|||||||
@@ -252,27 +252,42 @@ func (p *Plugin) mountContextRoutes(ctx context.Context) error {
|
|||||||
}
|
}
|
||||||
p.engine.Use(appContextMiddleware(ctx, p.coreCtx.Root()))
|
p.engine.Use(appContextMiddleware(ctx, p.coreCtx.Root()))
|
||||||
SetWhitelist(p.coreCtx.Router().Whitelist())
|
SetWhitelist(p.coreCtx.Router().Whitelist())
|
||||||
|
|
||||||
|
globalMW, err := toGinHandlers(p.coreCtx.Router().Middlewares())
|
||||||
|
if err != nil {
|
||||||
|
return fmt.Errorf("driver_http: invalid global middleware: %w", err)
|
||||||
|
}
|
||||||
|
|
||||||
for _, rd := range p.coreCtx.Router().Routes() {
|
for _, rd := range p.coreCtx.Router().Routes() {
|
||||||
allHandlers := make([]gin.HandlerFunc, 0, len(rd.Middlewares)+len(rd.Handlers))
|
routeMW, convErr := toGinHandlers(rd.Middlewares)
|
||||||
for _, m := range rd.Middlewares {
|
if convErr != nil {
|
||||||
gh, err := toGinHandler(m)
|
return fmt.Errorf("driver_http: invalid middleware for route %s %s: %w", rd.Method, rd.Path, convErr)
|
||||||
if err != nil {
|
|
||||||
return fmt.Errorf("driver_http: invalid middleware for route %s %s: %w", rd.Method, rd.Path, err)
|
|
||||||
}
|
|
||||||
allHandlers = append(allHandlers, gh)
|
|
||||||
}
|
}
|
||||||
for _, h := range rd.Handlers {
|
handlers, convErr := toGinHandlers(rd.Handlers)
|
||||||
gh, err := toGinHandler(h)
|
if convErr != nil {
|
||||||
if err != nil {
|
return fmt.Errorf("driver_http: invalid handler for route %s %s: %w", rd.Method, rd.Path, convErr)
|
||||||
return fmt.Errorf("driver_http: invalid handler for route %s %s: %w", rd.Method, rd.Path, err)
|
|
||||||
}
|
|
||||||
allHandlers = append(allHandlers, gh)
|
|
||||||
}
|
}
|
||||||
|
allHandlers := make([]gin.HandlerFunc, 0, len(globalMW)+len(routeMW)+len(handlers))
|
||||||
|
allHandlers = append(allHandlers, globalMW...)
|
||||||
|
allHandlers = append(allHandlers, routeMW...)
|
||||||
|
allHandlers = append(allHandlers, handlers...)
|
||||||
p.engine.Handle(rd.Method, rd.Path, allHandlers...)
|
p.engine.Handle(rd.Method, rd.Path, allHandlers...)
|
||||||
}
|
}
|
||||||
return nil
|
return nil
|
||||||
}
|
}
|
||||||
|
|
||||||
|
func toGinHandlers(hs []any) ([]gin.HandlerFunc, error) {
|
||||||
|
out := make([]gin.HandlerFunc, 0, len(hs))
|
||||||
|
for _, h := range hs {
|
||||||
|
gh, err := toGinHandler(h)
|
||||||
|
if err != nil {
|
||||||
|
return nil, err
|
||||||
|
}
|
||||||
|
out = append(out, gh)
|
||||||
|
}
|
||||||
|
return out, nil
|
||||||
|
}
|
||||||
|
|
||||||
//nolint:contextcheck // middleware must wrap the gin request context, not Start's ctx
|
//nolint:contextcheck // middleware must wrap the gin request context, not Start's ctx
|
||||||
func appContextMiddleware(_ context.Context, appCtx *core.Context) gin.HandlerFunc {
|
func appContextMiddleware(_ context.Context, appCtx *core.Context) gin.HandlerFunc {
|
||||||
return func(c *gin.Context) {
|
return func(c *gin.Context) {
|
||||||
|
|||||||
@@ -152,6 +152,41 @@ func TestHTTPDriverLifecycle(t *testing.T) {
|
|||||||
require.NoError(t, err)
|
require.NoError(t, err)
|
||||||
}
|
}
|
||||||
|
|
||||||
|
func TestHTTPDriverAppliesGlobalMiddlewareRegisteredAfterRoutes(t *testing.T) {
|
||||||
|
ctx := core.NewContext(context.Background())
|
||||||
|
ctx.Config().SetSource(core.NewMapSource(nil))
|
||||||
|
require.NoError(t, ctx.Config().Resolve())
|
||||||
|
|
||||||
|
ctx.Router().GET("/early", func(c *gin.Context) {
|
||||||
|
c.Status(http.StatusOK)
|
||||||
|
})
|
||||||
|
|
||||||
|
var called atomic.Bool
|
||||||
|
ctx.Router().Use(func(c *gin.Context) {
|
||||||
|
called.Store(true)
|
||||||
|
c.Next()
|
||||||
|
})
|
||||||
|
|
||||||
|
httpPlugin := driver_http.New(driver_http.WithAddr("127.0.0.1:0"))
|
||||||
|
require.NoError(t, httpPlugin.Apply(ctx))
|
||||||
|
d, ok := ctx.Driver(core.DriverTypeHTTP)
|
||||||
|
require.True(t, ok)
|
||||||
|
require.NoError(t, d.Start(context.Background()))
|
||||||
|
t.Cleanup(func() {
|
||||||
|
stopCtx, cancel := context.WithTimeout(context.Background(), 2*time.Second)
|
||||||
|
defer cancel()
|
||||||
|
_ = d.Stop(stopCtx)
|
||||||
|
})
|
||||||
|
|
||||||
|
resp, err := http.Get(fmt.Sprintf("http://%s/early", httpPlugin.Addr()))
|
||||||
|
require.NoError(t, err)
|
||||||
|
defer resp.Body.Close()
|
||||||
|
require.Equal(t, http.StatusOK, resp.StatusCode)
|
||||||
|
if !called.Load() {
|
||||||
|
t.Fatal("Router.Use middleware registered after the route must still run at HTTP Start")
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
func TestAsynqWorkerDriverLifecycle(t *testing.T) {
|
func TestAsynqWorkerDriverLifecycle(t *testing.T) {
|
||||||
mr, err := miniredis.Run()
|
mr, err := miniredis.Run()
|
||||||
require.NoError(t, err)
|
require.NoError(t, err)
|
||||||
|
|||||||
Reference in New Issue
Block a user