From ac091cf968ef93f4bf5777255c0ca43eeccbb619 Mon Sep 17 00:00:00 2001 From: Arve Knudsen Date: Fri, 24 Jan 2025 14:45:39 +0100 Subject: [PATCH] Move HTTP middleware to dskit Signed-off-by: Arve Knudsen --- cmd/mimir/config-descriptor.json | 2 +- cmd/mimir/help-all.txt.tmpl | 2 +- .../configuration-parameters/index.md | 2 +- go.mod | 2 +- go.sum | 4 ++-- pkg/api/api.go | 5 ----- .../grafana/dskit/middleware/cluster.go | 20 +++++++++++++------ .../github.com/grafana/dskit/server/server.go | 18 ++++++++--------- vendor/modules.txt | 2 +- 9 files changed, 30 insertions(+), 27 deletions(-) rename pkg/api/middleware.go => vendor/github.com/grafana/dskit/middleware/cluster.go (58%) diff --git a/cmd/mimir/config-descriptor.json b/cmd/mimir/config-descriptor.json index c78050ff44..efe893de41 100644 --- a/cmd/mimir/config-descriptor.json +++ b/cmd/mimir/config-descriptor.json @@ -419,7 +419,7 @@ "kind": "field", "name": "register_instrumentation", "required": false, - "desc": "Register the intrumentation handlers (/metrics etc).", + "desc": "Register the instrumentation handlers (/metrics etc).", "fieldValue": null, "fieldDefaultValue": true, "fieldFlag": "server.register-instrumentation", diff --git a/cmd/mimir/help-all.txt.tmpl b/cmd/mimir/help-all.txt.tmpl index 4e66f4c1a8..38dc3ec040 100644 --- a/cmd/mimir/help-all.txt.tmpl +++ b/cmd/mimir/help-all.txt.tmpl @@ -3176,7 +3176,7 @@ Usage of ./cmd/mimir/mimir: -server.proxy-protocol-enabled [experimental] Enables PROXY protocol. -server.register-instrumentation - Register the intrumentation handlers (/metrics etc). (default true) + Register the instrumentation handlers (/metrics etc). (default true) -server.report-grpc-codes-in-instrumentation-label-enabled If set to true, gRPC statuses will be reported in instrumentation labels with their string representations. Otherwise, they will be reported as "error". (default true) -server.throughput.latency-cutoff duration diff --git a/docs/sources/mimir/configure/configuration-parameters/index.md b/docs/sources/mimir/configure/configuration-parameters/index.md index 70a0857d4e..c51f75bef1 100644 --- a/docs/sources/mimir/configure/configuration-parameters/index.md +++ b/docs/sources/mimir/configure/configuration-parameters/index.md @@ -622,7 +622,7 @@ grpc_tls_config: # CLI flag: -server.grpc-tls-ca-path [client_ca_file: | default = ""] -# (advanced) Register the intrumentation handlers (/metrics etc). +# (advanced) Register the instrumentation handlers (/metrics etc). # CLI flag: -server.register-instrumentation [register_instrumentation: | default = true] diff --git a/go.mod b/go.mod index 7ce21e3e5f..aeeec4abe5 100644 --- a/go.mod +++ b/go.mod @@ -22,7 +22,7 @@ require ( github.com/golang/snappy v0.0.4 github.com/google/gopacket v1.1.19 github.com/gorilla/mux v1.8.1 - github.com/grafana/dskit v0.0.0-20250124130032-aff6c876915b + github.com/grafana/dskit v0.0.0-20250124144421-f0ea018933c4 github.com/grafana/e2e v0.1.2-0.20240118170847-db90b84177fc github.com/hashicorp/golang-lru v1.0.2 // indirect github.com/influxdata/influxdb/v2 v2.7.11 diff --git a/go.sum b/go.sum index 36ee3843d7..58a1c88dbc 100644 --- a/go.sum +++ b/go.sum @@ -1271,8 +1271,8 @@ github.com/grafana-tools/sdk v0.0.0-20220919052116-6562121319fc h1:PXZQA2WCxe85T github.com/grafana-tools/sdk v0.0.0-20220919052116-6562121319fc/go.mod h1:AHHlOEv1+GGQ3ktHMlhuTUwo3zljV3QJbC0+8o2kn+4= github.com/grafana/alerting v0.0.0-20250113170557-b4ab2ba363a8 h1:mdI6P22PgFD7bQ0Yf4h8cfHSldak4nxogvlsTHZyZmc= github.com/grafana/alerting v0.0.0-20250113170557-b4ab2ba363a8/go.mod h1:QsnoKX/iYZxA4Cv+H+wC7uxutBD8qi8ZW5UJvD2TYmU= -github.com/grafana/dskit v0.0.0-20250124130032-aff6c876915b h1:y34FUoHHxGMbsX8nsGdCdJGfazKlGjf/7QhvgFMn0HA= -github.com/grafana/dskit v0.0.0-20250124130032-aff6c876915b/go.mod h1:SPLNCARd4xdjCkue0O6hvuoveuS1dGJjDnfxYe405YQ= +github.com/grafana/dskit v0.0.0-20250124144421-f0ea018933c4 h1:0CvC3frl7I1DnDZKd4WMib7Amb8yX/1/Wv8QopqMzzI= +github.com/grafana/dskit v0.0.0-20250124144421-f0ea018933c4/go.mod h1:SPLNCARd4xdjCkue0O6hvuoveuS1dGJjDnfxYe405YQ= github.com/grafana/e2e v0.1.2-0.20240118170847-db90b84177fc h1:BW+LjKJDz0So5LI8UZfW5neWeKpSkWqhmGjQFzcFfLM= github.com/grafana/e2e v0.1.2-0.20240118170847-db90b84177fc/go.mod h1:JVmqPBe8A/pZWwRoJW5ZjyALeY5OXMzPl7LrVXOdZAI= github.com/grafana/franz-go v0.0.0-20241009100846-782ba1442937 h1:fwwnG/NcygoS6XbAaEyK2QzMXI/BZIEJvQ3CD+7XZm8= diff --git a/pkg/api/api.go b/pkg/api/api.go index a89277a2bf..9e1df52a34 100644 --- a/pkg/api/api.go +++ b/pkg/api/api.go @@ -158,11 +158,6 @@ func (a *API) RegisterRoutesWithPrefix(prefix string, handler http.Handler, auth } func (a *API) newRoute(path string, handler http.Handler, isPrefix, auth, gzip bool, methods ...string) (route *mux.Route) { - if a.server.Cluster() != "" { - level.Debug(a.logger).Log("msg", "api: enabling cluster validation middleware", "path", path, "cluster", a.server.Cluster()) - handler = ClusterValidationMiddleware(a.server.Cluster(), a.logger).Wrap(handler) - } - // Propagate the consistency level on all HTTP routes. // They are not used everywhere, but for consistency and less surprise it's added everywhere. handler = querierapi.ConsistencyMiddleware().Wrap(handler) diff --git a/pkg/api/middleware.go b/vendor/github.com/grafana/dskit/middleware/cluster.go similarity index 58% rename from pkg/api/middleware.go rename to vendor/github.com/grafana/dskit/middleware/cluster.go index 147eeb7740..2e3f5ad9db 100644 --- a/pkg/api/middleware.go +++ b/vendor/github.com/grafana/dskit/middleware/cluster.go @@ -1,22 +1,25 @@ -package api +package middleware import ( "fmt" "net/http" + "strings" "github.com/go-kit/log" "github.com/go-kit/log/level" + "github.com/grafana/dskit/clusterutil" - "github.com/grafana/dskit/middleware" ) // ClusterValidationMiddleware validates that requests are for the correct cluster. -func ClusterValidationMiddleware(cluster string, logger log.Logger) middleware.Interface { - return middleware.Func(func(next http.Handler) http.Handler { +func ClusterValidationMiddleware(cluster string, logger log.Logger) Interface { + return Func(func(next http.Handler) http.Handler { return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { reqCluster := r.Header.Get(clusterutil.ClusterHeader) - if reqCluster != cluster { - level.Warn(logger).Log("msg", "rejecting request intended for wrong cluster", "cluster", cluster, "request_cluster", reqCluster) + if !auxilliaryPath(r.URL.Path) && reqCluster != cluster { + level.Warn(logger).Log("msg", "rejecting request intended for wrong cluster", + "cluster", cluster, "request_cluster", reqCluster, "header", clusterutil.ClusterHeader, + "url", r.URL) http.Error(w, fmt.Sprintf("request intended for cluster %q - this is cluster %q", reqCluster, cluster), http.StatusBadRequest) return @@ -26,3 +29,8 @@ func ClusterValidationMiddleware(cluster string, logger log.Logger) middleware.I }) }) } + +func auxilliaryPath(pth string) bool { + // Check suffix in case a path prefix is configured. + return strings.HasSuffix(pth, "/metrics") || strings.HasSuffix(pth, "/debug/pprof") +} diff --git a/vendor/github.com/grafana/dskit/server/server.go b/vendor/github.com/grafana/dskit/server/server.go index 8921b09be9..5a4d1a0db5 100644 --- a/vendor/github.com/grafana/dskit/server/server.go +++ b/vendor/github.com/grafana/dskit/server/server.go @@ -185,7 +185,7 @@ func (cfg *Config) RegisterFlags(f *flag.FlagSet) { f.StringVar(&cfg.GRPCListenAddress, "server.grpc-listen-address", "", "gRPC server listen address.") f.IntVar(&cfg.GRPCListenPort, "server.grpc-listen-port", 9095, "gRPC server listen port.") f.IntVar(&cfg.GRPCConnLimit, "server.grpc-conn-limit", 0, "Maximum number of simultaneous grpc connections, <=0 to disable") - f.BoolVar(&cfg.RegisterInstrumentation, "server.register-instrumentation", true, "Register the intrumentation handlers (/metrics etc).") + f.BoolVar(&cfg.RegisterInstrumentation, "server.register-instrumentation", true, "Register the instrumentation handlers (/metrics etc).") f.BoolVar(&cfg.ReportGRPCCodesInInstrumentationLabel, "server.report-grpc-codes-in-instrumentation-label-enabled", false, "If set to true, gRPC statuses will be reported in instrumentation labels with their string representations. Otherwise, they will be reported as \"error\".") f.DurationVar(&cfg.ServerGracefulShutdownTimeout, "server.graceful-shutdown-timeout", 30*time.Second, "Timeout for graceful shutdowns") f.DurationVar(&cfg.HTTPServerReadTimeout, "server.http-read-timeout", 30*time.Second, "Read timeout for entire HTTP request, including headers and body.") @@ -520,10 +520,14 @@ func BuildHTTPMiddleware(cfg Config, router *mux.Router, metrics *Metrics, logge logSourceIPs = nil } + if cfg.DoNotAddDefaultHTTPMiddleware { + return cfg.HTTPMiddleware, nil + } + defaultLogMiddleware := middleware.NewLogMiddleware(logger, cfg.LogRequestHeaders, cfg.LogRequestAtInfoLevel, logSourceIPs, strings.Split(cfg.LogRequestExcludeHeadersList, ",")) defaultLogMiddleware.DisableRequestSuccessLog = cfg.DisableRequestSuccessLog - defaultHTTPMiddleware := []middleware.Interface{ + httpMiddleware := []middleware.Interface{ middleware.RouteInjector{ RouteMatcher: router, }, @@ -543,14 +547,10 @@ func BuildHTTPMiddleware(cfg Config, router *mux.Router, metrics *Metrics, logge RequestThroughput: metrics.RequestThroughput, }, } - var httpMiddleware []middleware.Interface - if cfg.DoNotAddDefaultHTTPMiddleware { - httpMiddleware = cfg.HTTPMiddleware - } else { - httpMiddleware = append(defaultHTTPMiddleware, cfg.HTTPMiddleware...) + if cfg.Cluster != "" { + httpMiddleware = append(httpMiddleware, middleware.ClusterValidationMiddleware(cfg.Cluster, logger)) } - - return httpMiddleware, nil + return append(httpMiddleware, cfg.HTTPMiddleware...), nil } // Run the server; blocks until SIGTERM (if signal handling is enabled), an error is received, or Stop() is called. diff --git a/vendor/modules.txt b/vendor/modules.txt index 7c669993da..2e81e80658 100644 --- a/vendor/modules.txt +++ b/vendor/modules.txt @@ -644,7 +644,7 @@ github.com/grafana/alerting/receivers/webex github.com/grafana/alerting/receivers/webhook github.com/grafana/alerting/receivers/wecom github.com/grafana/alerting/templates -# github.com/grafana/dskit v0.0.0-20250124130032-aff6c876915b +# github.com/grafana/dskit v0.0.0-20250124144421-f0ea018933c4 ## explicit; go 1.21 github.com/grafana/dskit/backoff github.com/grafana/dskit/ballast