From 1777468df4565ae73d7b0b227da45041511a9293 Mon Sep 17 00:00:00 2001 From: Julien Duchesne Date: Mon, 20 Jan 2025 12:53:27 -0500 Subject: [PATCH] frontend: Allow blocking raw http requests This is a setting that can be set in user overrides like this: ``` user1: blocked_requests: - path: /api/v1/series - method: DELETE - query_params foo: bar ``` Or a combination of these. Each entry is an AND condition --- cmd/mimir/config-descriptor.json | 10 ++ .../configuration-parameters/index.md | 3 + .../mimir/manage/mimir-runbooks/_index.md | 13 ++ pkg/frontend/querymiddleware/limits.go | 3 + pkg/frontend/querymiddleware/limits_test.go | 9 ++ .../{blocker.go => query_blocker.go} | 0 ...{blocker_test.go => query_blocker_test.go} | 0 .../querymiddleware/request_blocker.go | 84 ++++++++++++ .../querymiddleware/request_blocker_test.go | 126 ++++++++++++++++++ pkg/frontend/querymiddleware/roundtrip.go | 5 + pkg/util/globalerror/user.go | 1 + pkg/util/validation/blocked_request.go | 9 ++ pkg/util/validation/limits.go | 6 + tools/doc-generator/parse/parser.go | 6 + 14 files changed, 275 insertions(+) rename pkg/frontend/querymiddleware/{blocker.go => query_blocker.go} (100%) rename pkg/frontend/querymiddleware/{blocker_test.go => query_blocker_test.go} (100%) create mode 100644 pkg/frontend/querymiddleware/request_blocker.go create mode 100644 pkg/frontend/querymiddleware/request_blocker_test.go create mode 100644 pkg/util/validation/blocked_request.go diff --git a/cmd/mimir/config-descriptor.json b/cmd/mimir/config-descriptor.json index f8a5964216..fbfbe2af75 100644 --- a/cmd/mimir/config-descriptor.json +++ b/cmd/mimir/config-descriptor.json @@ -4327,6 +4327,16 @@ "fieldType": "blocked_queries_config...", "fieldCategory": "experimental" }, + { + "kind": "field", + "name": "blocked_requests", + "required": false, + "desc": "List of http requests to block.", + "fieldValue": null, + "fieldDefaultValue": null, + "fieldType": "blocked_requests_config...", + "fieldCategory": "experimental" + }, { "kind": "field", "name": "align_queries_with_step", diff --git a/docs/sources/mimir/configure/configuration-parameters/index.md b/docs/sources/mimir/configure/configuration-parameters/index.md index 1f4e80b99b..fd481fc24a 100644 --- a/docs/sources/mimir/configure/configuration-parameters/index.md +++ b/docs/sources/mimir/configure/configuration-parameters/index.md @@ -3578,6 +3578,9 @@ The `limits` block configures default and per-tenant limits imposed by component # (experimental) List of queries to block. [blocked_queries: | default = ] +# (experimental) List of http requests to block. +[blocked_requests: | default = ] + # Mutate incoming queries to align their start and end with their step to # improve result caching. # CLI flag: -query-frontend.align-queries-with-step diff --git a/docs/sources/mimir/manage/mimir-runbooks/_index.md b/docs/sources/mimir/manage/mimir-runbooks/_index.md index 28d462e835..8adfc31d10 100644 --- a/docs/sources/mimir/manage/mimir-runbooks/_index.md +++ b/docs/sources/mimir/manage/mimir-runbooks/_index.md @@ -2550,6 +2550,19 @@ How to **fix** it: This error only occurs when an administrator has explicitly define a blocked list for a given tenant. After assessing whether or not the reason for blocking one or multiple queries you can update the tenant's limits and remove the pattern. +### err-mimir-request-blocked + +This error occurs when a query-frontend blocks a HTTP request because the request matches at least one of the rules defined in the limits. + +How it **works**: + +- The query-frontend implements a checker responsible for assessing whether the request is blocked or not. +- To configure the limit, set the block `blocked_requests` in the `limits`. + +How to **fix** it: + +This error only occurs when an administrator has explicitly define a blocked list for a given tenant. After assessing whether or not the reason for blocking one or multiple requests you can update the tenant's limits and remove the configuration. + ### err-mimir-alertmanager-max-grafana-config-size This non-critical error occurs when the Alertmanager receives a Grafana Alertmanager configuration larger than the configured size limit. diff --git a/pkg/frontend/querymiddleware/limits.go b/pkg/frontend/querymiddleware/limits.go index 7e76fdc32e..26e156e9c8 100644 --- a/pkg/frontend/querymiddleware/limits.go +++ b/pkg/frontend/querymiddleware/limits.go @@ -105,6 +105,9 @@ type Limits interface { // BlockedQueries returns the blocked queries. BlockedQueries(userID string) []*validation.BlockedQuery + // BlockedRequests returns the blocked http requests. + BlockedRequests(userID string) []*validation.BlockedRequest + // AlignQueriesWithStep returns if queries should be adjusted to be step-aligned AlignQueriesWithStep(userID string) bool diff --git a/pkg/frontend/querymiddleware/limits_test.go b/pkg/frontend/querymiddleware/limits_test.go index ac2b361576..67830d295c 100644 --- a/pkg/frontend/querymiddleware/limits_test.go +++ b/pkg/frontend/querymiddleware/limits_test.go @@ -601,6 +601,10 @@ func (m multiTenantMockLimits) IngestStorageReadConsistency(userID string) strin return m.byTenant[userID].ingestStorageReadConsistency } +func (m multiTenantMockLimits) BlockedRequests(userID string) []*validation.BlockedRequest { + return m.byTenant[userID].blockedRequests +} + type mockLimits struct { maxQueryLookback time.Duration maxQueryLength time.Duration @@ -626,6 +630,7 @@ type mockLimits struct { enabledPromQLExperimentalFunctions []string prom2RangeCompat bool blockedQueries []*validation.BlockedQuery + blockedRequests []*validation.BlockedRequest alignQueriesWithStep bool queryIngestersWithin time.Duration ingestStorageReadConsistency string @@ -741,6 +746,10 @@ func (m mockLimits) IngestStorageReadConsistency(string) string { return m.ingestStorageReadConsistency } +func (m mockLimits) BlockedRequests(string) []*validation.BlockedRequest { + return m.blockedRequests +} + type mockHandler struct { mock.Mock } diff --git a/pkg/frontend/querymiddleware/blocker.go b/pkg/frontend/querymiddleware/query_blocker.go similarity index 100% rename from pkg/frontend/querymiddleware/blocker.go rename to pkg/frontend/querymiddleware/query_blocker.go diff --git a/pkg/frontend/querymiddleware/blocker_test.go b/pkg/frontend/querymiddleware/query_blocker_test.go similarity index 100% rename from pkg/frontend/querymiddleware/blocker_test.go rename to pkg/frontend/querymiddleware/query_blocker_test.go diff --git a/pkg/frontend/querymiddleware/request_blocker.go b/pkg/frontend/querymiddleware/request_blocker.go new file mode 100644 index 0000000000..a1adfcdc1a --- /dev/null +++ b/pkg/frontend/querymiddleware/request_blocker.go @@ -0,0 +1,84 @@ +// SPDX-License-Identifier: AGPL-3.0-only + +package querymiddleware + +import ( + "net/http" + + "github.com/go-kit/log" + "github.com/go-kit/log/level" + "github.com/grafana/dskit/tenant" + "github.com/prometheus/client_golang/prometheus" + "github.com/prometheus/client_golang/prometheus/promauto" + + apierror "github.com/grafana/mimir/pkg/api/error" + "github.com/grafana/mimir/pkg/util/globalerror" +) + +func newRequestBlockedError() error { + return apierror.New(apierror.TypeBadData, globalerror.RequestBlocked.Message("the request has been blocked by the cluster administrator")) +} + +type requestBlocker struct { + limits Limits + logger log.Logger + blockedRequestsCounter *prometheus.CounterVec +} + +func newRequestBlocker( + limits Limits, + logger log.Logger, + registerer prometheus.Registerer, +) *requestBlocker { + blockedRequestsCounter := promauto.With(registerer).NewCounterVec(prometheus.CounterOpts{ + Name: "cortex_query_frontend_rejected_requests_total", + Help: "Number of HTTP requests that were rejected by the cluster administrator.", + }, []string{"user"}) + return &requestBlocker{ + limits: limits, + logger: logger, + blockedRequestsCounter: blockedRequestsCounter, + } +} + +func (rb *requestBlocker) isBlocked(r *http.Request) error { + tenants, err := tenant.TenantIDs(r.Context()) + if err != nil { + return nil + } + + for _, tenant := range tenants { + blockedRequests := rb.limits.BlockedRequests(tenant) + + for _, blockedRequest := range blockedRequests { + if blockedPath := blockedRequest.Path; blockedPath != "" && blockedPath != r.URL.Path { + continue + } + + if blockedMethod := blockedRequest.Method; blockedMethod != "" && blockedMethod != r.Method { + continue + } + + if blockedParams := blockedRequest.QueryParams; len(blockedParams) > 0 { + query := r.URL.Query() + blockedByParams := false + for key, blockedValue := range blockedParams { + if query.Get(key) == blockedValue { + blockedByParams = true + break + } + } + + if !blockedByParams { + continue + } + } + + level.Info(rb.logger).Log("msg", "request blocked", "user", tenant, "url", r.URL.String(), "method", r.Method) + rb.blockedRequestsCounter.WithLabelValues(tenant).Inc() + return newRequestBlockedError() + } + } + return nil + +} diff --git a/pkg/frontend/querymiddleware/request_blocker_test.go b/pkg/frontend/querymiddleware/request_blocker_test.go new file mode 100644 index 0000000000..22a4506ea3 --- /dev/null +++ b/pkg/frontend/querymiddleware/request_blocker_test.go @@ -0,0 +1,126 @@ +// SPDX-License-Identifier: AGPL-3.0-only + +package querymiddleware + +import ( + "context" + "net/http" + "testing" + + "github.com/go-kit/log" + "github.com/grafana/dskit/user" + "github.com/prometheus/client_golang/prometheus" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + + "github.com/grafana/mimir/pkg/util/validation" +) + +func TestRequestBlocker_IsBlocked(t *testing.T) { + const userID = "user-1" + // Mock the limits. + limits := multiTenantMockLimits{ + byTenant: map[string]mockLimits{ + userID: { + blockedRequests: []*validation.BlockedRequest{ + {Path: "/blocked-by-path"}, + {Method: "POST"}, + {QueryParams: map[string]string{"foo": "bar"}}, + {Path: "/blocked-by-path2", Method: "GET", QueryParams: map[string]string{"foo": "bar2"}}, + }, + }, + }, + } + + tests := []struct { + name string + request func() *http.Request + expected error + }{ + { + name: "request is not blocked", + request: func() *http.Request { + req, err := http.NewRequest(http.MethodGet, "/not-blocked", nil) + require.NoError(t, err) + return req + }, + expected: nil, + }, + { + name: "request is blocked by path", + request: func() *http.Request { + req, err := http.NewRequest(http.MethodGet, "/blocked-by-path", nil) + require.NoError(t, err) + return req + }, + expected: newRequestBlockedError(), + }, + { + name: "request is blocked by method", + request: func() *http.Request { + req, err := http.NewRequest(http.MethodPost, "/not-blocked", nil) + require.NoError(t, err) + return req + }, + expected: newRequestBlockedError(), + }, + { + name: "request is blocked by query params", + request: func() *http.Request { + req, err := http.NewRequest(http.MethodGet, "/not-blocked?foo=bar", nil) + require.NoError(t, err) + return req + }, + expected: newRequestBlockedError(), + }, + { + name: "request is blocked by path, method and query params", + request: func() *http.Request { + req, err := http.NewRequest(http.MethodGet, "/blocked-by-path2?foo=bar2", nil) + require.NoError(t, err) + return req + }, + expected: newRequestBlockedError(), + }, + { + name: "request does not fully match blocked request (different method)", + request: func() *http.Request { + req, err := http.NewRequest(http.MethodDelete, "/blocked-by-path2?foo=bar2", nil) + require.NoError(t, err) + return req + }, + expected: nil, + }, + { + name: "request does not fully match blocked request (different query params)", + request: func() *http.Request { + req, err := http.NewRequest(http.MethodGet, "/blocked-by-path2?foo=bar3", nil) + require.NoError(t, err) + return req + }, + expected: nil, + }, + { + name: "request does not fully match blocked request (different path)", + request: func() *http.Request { + req, err := http.NewRequest(http.MethodGet, "/blocked-by-path3?foo=bar2", nil) + require.NoError(t, err) + return req + }, + expected: nil, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + blocker := newRequestBlocker(limits, log.NewNopLogger(), prometheus.NewRegistry()) + + req := tt.request() + ctx := user.InjectOrgID(context.Background(), userID) + req = req.WithContext(ctx) + + err := blocker.isBlocked(req) + assert.Equal(t, err, tt.expected) + }) + } +} diff --git a/pkg/frontend/querymiddleware/roundtrip.go b/pkg/frontend/querymiddleware/roundtrip.go index 3ac38c4e57..a67e1f5d43 100644 --- a/pkg/frontend/querymiddleware/roundtrip.go +++ b/pkg/frontend/querymiddleware/roundtrip.go @@ -257,6 +257,7 @@ func newQueryTripperware( } queryRangeMiddleware, queryInstantMiddleware, remoteReadMiddleware := newQueryMiddlewares(cfg, log, limits, codec, c, cacheKeyGenerator, cacheExtractor, engine, registerer) + requestBlocker := newRequestBlocker(limits, log, registerer) return func(next http.RoundTripper) http.RoundTripper { // IMPORTANT: roundtrippers are executed in *reverse* order because they are wrappers. @@ -311,6 +312,10 @@ func newQueryTripperware( cardinality = NewCardinalityQueryRequestValidationRoundTripper(cardinality) return RoundTripFunc(func(r *http.Request) (*http.Response, error) { + if err := requestBlocker.isBlocked(r); err != nil { + return nil, err + } + switch { case IsRangeQuery(r.URL.Path): return queryrange.RoundTrip(r) diff --git a/pkg/util/globalerror/user.go b/pkg/util/globalerror/user.go index 68dd3ef104..66a978f1e4 100644 --- a/pkg/util/globalerror/user.go +++ b/pkg/util/globalerror/user.go @@ -65,6 +65,7 @@ const ( IngestionRateLimited ID = "tenant-max-ingestion-rate" TooManyHAClusters ID = "tenant-too-many-ha-clusters" QueryBlocked ID = "query-blocked" + RequestBlocked ID = "request-blocked" SampleTimestampTooOld ID = "sample-timestamp-too-old" SampleOutOfOrder ID = "sample-out-of-order" diff --git a/pkg/util/validation/blocked_request.go b/pkg/util/validation/blocked_request.go new file mode 100644 index 0000000000..4ffe9a7c5a --- /dev/null +++ b/pkg/util/validation/blocked_request.go @@ -0,0 +1,9 @@ +// SPDX-License-Identifier: AGPL-3.0-only + +package validation + +type BlockedRequest struct { + Path string `yaml:"path,omitempty"` + Method string `yaml:"method,omitempty"` + QueryParams map[string]string `yaml:"query_params,omitempty"` +} diff --git a/pkg/util/validation/limits.go b/pkg/util/validation/limits.go index 18a3e938f9..4e771bfb99 100644 --- a/pkg/util/validation/limits.go +++ b/pkg/util/validation/limits.go @@ -187,6 +187,7 @@ type Limits struct { ResultsCacheForUnalignedQueryEnabled bool `yaml:"cache_unaligned_requests" json:"cache_unaligned_requests" category:"advanced"` MaxQueryExpressionSizeBytes int `yaml:"max_query_expression_size_bytes" json:"max_query_expression_size_bytes"` BlockedQueries []*BlockedQuery `yaml:"blocked_queries,omitempty" json:"blocked_queries,omitempty" doc:"nocli|description=List of queries to block." category:"experimental"` + BlockedRequests []*BlockedRequest `yaml:"blocked_requests,omitempty" json:"blocked_requests,omitempty" doc:"nocli|description=List of http requests to block." category:"experimental"` AlignQueriesWithStep bool `yaml:"align_queries_with_step" json:"align_queries_with_step"` EnabledPromQLExperimentalFunctions flagext.StringSliceCSV `yaml:"enabled_promql_experimental_functions" json:"enabled_promql_experimental_functions" category:"experimental"` Prom2RangeCompat bool `yaml:"prom2_range_compat" json:"prom2_range_compat" category:"experimental"` @@ -735,6 +736,11 @@ func (o *Overrides) BlockedQueries(userID string) []*BlockedQuery { return o.getOverridesForUser(userID).BlockedQueries } +// BlockedRequests returns the blocked http requests. +func (o *Overrides) BlockedRequests(userID string) []*BlockedRequest { + return o.getOverridesForUser(userID).BlockedRequests +} + // MaxLabelsQueryLength returns the limit of the length (in time) of a label names or values request. func (o *Overrides) MaxLabelsQueryLength(userID string) time.Duration { return time.Duration(o.getOverridesForUser(userID).MaxLabelsQueryLength) diff --git a/tools/doc-generator/parse/parser.go b/tools/doc-generator/parse/parser.go index 8a49efb7fc..9892879459 100644 --- a/tools/doc-generator/parse/parser.go +++ b/tools/doc-generator/parse/parser.go @@ -367,6 +367,8 @@ func getFieldCustomType(t reflect.Type) (string, bool) { return "relabel_config...", true case reflect.TypeOf([]*validation.BlockedQuery{}).String(): return "blocked_queries_config...", true + case reflect.TypeOf([]*validation.BlockedRequest{}).String(): + return "blocked_requests_config...", true case reflect.TypeOf(asmodel.CustomTrackersConfig{}).String(): return "map of tracker name (string) to matcher (string)", true default: @@ -455,6 +457,8 @@ func getCustomFieldType(t reflect.Type) (string, bool) { return "relabel_config...", true case reflect.TypeOf([]*validation.BlockedQuery{}).String(): return "blocked_queries_config...", true + case reflect.TypeOf([]*validation.BlockedRequest{}).String(): + return "blocked_requests_config...", true case reflect.TypeOf(asmodel.CustomTrackersConfig{}).String(): return "map of tracker name (string) to matcher (string)", true default: @@ -488,6 +492,8 @@ func ReflectType(typ string) reflect.Type { return reflect.TypeOf([]*relabel.Config{}) case "blocked_queries_config...": return reflect.TypeOf([]*validation.BlockedQuery{}) + case "blocked_requests_config...": + return reflect.TypeOf([]*validation.BlockedRequest{}) case "map of string to float64": return reflect.TypeOf(validation.LimitsMap[float64]{}) case "map of string to int":