Skip to content

Commit

Permalink
distributor: allow skipping label count validation (#9576)
Browse files Browse the repository at this point in the history
This PR introduces a new feature that enables users to bypass label count validation via the `X-Mimir-SkipLabelCountValidation` header.

A new configuration option, `-api.skip-label-count-validation-header-enabled` has been added to control this behavior. If enabled, users can set the header to true to skip label count validation.

In principle, this feature is not intended to be used by Mimir itself, but rather by projects that depend on it.

---------

Signed-off-by: Miguel Ángel Ortuño <[email protected]>
Co-authored-by: Mauro Stettler <[email protected]>
Co-authored-by: Taylor C <[email protected]>
  • Loading branch information
3 people authored Oct 11, 2024
1 parent 2c82657 commit 7ee1cf5
Show file tree
Hide file tree
Showing 16 changed files with 422 additions and 208 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@
* [FEATURE] Alertmanager: limit added for maximum size of the Grafana state (`-alertmanager.max-grafana-state-size-bytes`). #9475
* [FEATURE] Alertmanager: limit added for maximum size of the Grafana configuration (`-alertmanager.max-config-size-bytes`). #9402
* [FEATURE] Ingester: Experimental support for ingesting out-of-order native histograms. This is disabled by default and can be enabled by setting `-ingester.ooo-native-histograms-ingestion-enabled` to `true`. #7175
* [FEATURE] Distributor: Added `-api.skip-label-count-validation-header-enabled` option to allow skipping label count validation on the HTTP write path based on `X-Mimir-SkipLabelCountValidation` header being `true` or not. #9576
* [ENHANCEMENT] mimirtool: Adds bearer token support for mimirtool's analyze ruler/prometheus commands. #9587
* [ENHANCEMENT] Ruler: Support `exclude_alerts` parameter in `<prometheus-http-prefix>/api/v1/rules` endpoint. #9300
* [ENHANCEMENT] Distributor: add a metric to track tenants who are sending newlines in their label values called `cortex_distributor_label_values_with_newlines_total`. #9400
Expand Down
11 changes: 11 additions & 0 deletions cmd/mimir/config-descriptor.json
Original file line number Diff line number Diff line change
Expand Up @@ -85,6 +85,17 @@
"fieldType": "boolean",
"fieldCategory": "advanced"
},
{
"kind": "field",
"name": "skip_label_count_validation_header_enabled",
"required": false,
"desc": "Allows to disable enforcement of the label count limit \"max_label_names_per_series\" via X-Mimir-SkipLabelCountValidation header on the http write path. Allowing this for external clients allows any client to send invalid label counts. After enabling it, requests with a specific HTTP header set to true will not have label counts validated.",
"fieldValue": null,
"fieldDefaultValue": false,
"fieldFlag": "api.skip-label-count-validation-header-enabled",
"fieldType": "boolean",
"fieldCategory": "advanced"
},
{
"kind": "field",
"name": "get_request_for_ingester_shutdown_enabled",
Expand Down
2 changes: 2 additions & 0 deletions cmd/mimir/help-all.txt.tmpl
Original file line number Diff line number Diff line change
Expand Up @@ -313,6 +313,8 @@ Usage of ./cmd/mimir/mimir:
The URL under which Alertmanager is externally reachable (eg. could be different than -http.alertmanager-http-prefix in case Alertmanager is served via a reverse proxy). This setting is used both to configure the internal requests router and to generate links in alert templates. If the external URL has a path portion, it will be used to prefix all HTTP endpoints served by Alertmanager, both the UI and API. (default http://localhost:8080/alertmanager)
-api.get-request-for-ingester-shutdown-enabled
[deprecated] Enable GET requests to the /ingester/shutdown endpoint to trigger an ingester shutdown. This is a potentially dangerous operation and should only be enabled consciously.
-api.skip-label-count-validation-header-enabled
Allows to disable enforcement of the label count limit "max_label_names_per_series" via X-Mimir-SkipLabelCountValidation header on the http write path. Allowing this for external clients allows any client to send invalid label counts. After enabling it, requests with a specific HTTP header set to true will not have label counts validated.
-api.skip-label-name-validation-header-enabled
Allows to skip label name validation via X-Mimir-SkipLabelNameValidation header on the http write path. Use with caution as it breaks PromQL. Allowing this for external clients allows any client to send invalid label names. After enabling it, requests with a specific HTTP header set to true will not have label names validated.
-auth.multitenancy-enabled
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -141,6 +141,14 @@ api:
# CLI flag: -api.skip-label-name-validation-header-enabled
[skip_label_name_validation_header_enabled: <boolean> | default = false]

# (advanced) Allows to disable enforcement of the label count limit
# "max_label_names_per_series" via X-Mimir-SkipLabelCountValidation header on
# the http write path. Allowing this for external clients allows any client to
# send invalid label counts. After enabling it, requests with a specific HTTP
# header set to true will not have label counts validated.
# CLI flag: -api.skip-label-count-validation-header-enabled
[skip_label_count_validation_header_enabled: <boolean> | default = false]

# (deprecated) Enable GET requests to the /ingester/shutdown endpoint to
# trigger an ingester shutdown. This is a potentially dangerous operation and
# should only be enabled consciously.
Expand Down
6 changes: 4 additions & 2 deletions pkg/api/api.go
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,8 @@ import (
type ConfigHandler func(actualCfg interface{}, defaultCfg interface{}) http.HandlerFunc

type Config struct {
SkipLabelNameValidationHeader bool `yaml:"skip_label_name_validation_header_enabled" category:"advanced"`
SkipLabelNameValidationHeader bool `yaml:"skip_label_name_validation_header_enabled" category:"advanced"`
SkipLabelCountValidationHeader bool `yaml:"skip_label_count_validation_header_enabled" category:"advanced"`

// TODO: Remove option in Mimir 2.15.
GETRequestForIngesterShutdownEnabled bool `yaml:"get_request_for_ingester_shutdown_enabled" category:"deprecated"`
Expand All @@ -70,6 +71,7 @@ type Config struct {
// RegisterFlags adds the flags required to config this to the given FlagSet.
func (cfg *Config) RegisterFlags(f *flag.FlagSet) {
f.BoolVar(&cfg.SkipLabelNameValidationHeader, "api.skip-label-name-validation-header-enabled", false, "Allows to skip label name validation via X-Mimir-SkipLabelNameValidation header on the http write path. Use with caution as it breaks PromQL. Allowing this for external clients allows any client to send invalid label names. After enabling it, requests with a specific HTTP header set to true will not have label names validated.")
f.BoolVar(&cfg.SkipLabelCountValidationHeader, "api.skip-label-count-validation-header-enabled", false, "Allows to disable enforcement of the label count limit \"max_label_names_per_series\" via X-Mimir-SkipLabelCountValidation header on the http write path. Allowing this for external clients allows any client to send invalid label counts. After enabling it, requests with a specific HTTP header set to true will not have label counts validated.")
f.BoolVar(&cfg.GETRequestForIngesterShutdownEnabled, "api.get-request-for-ingester-shutdown-enabled", false, "Enable GET requests to the /ingester/shutdown endpoint to trigger an ingester shutdown. This is a potentially dangerous operation and should only be enabled consciously.")
cfg.RegisterFlagsWithPrefix("", f)
}
Expand Down Expand Up @@ -262,7 +264,7 @@ const OTLPPushEndpoint = "/otlp/v1/metrics"
func (a *API) RegisterDistributor(d *distributor.Distributor, pushConfig distributor.Config, reg prometheus.Registerer, limits *validation.Overrides) {
distributorpb.RegisterDistributorServer(a.server.GRPC, d)

a.RegisterRoute(PrometheusPushEndpoint, distributor.Handler(pushConfig.MaxRecvMsgSize, d.RequestBufferPool, a.sourceIPs, a.cfg.SkipLabelNameValidationHeader, limits, pushConfig.RetryConfig, d.PushWithMiddlewares, d.PushMetrics, a.logger), true, false, "POST")
a.RegisterRoute(PrometheusPushEndpoint, distributor.Handler(pushConfig.MaxRecvMsgSize, d.RequestBufferPool, a.sourceIPs, a.cfg.SkipLabelNameValidationHeader, a.cfg.SkipLabelCountValidationHeader, limits, pushConfig.RetryConfig, d.PushWithMiddlewares, d.PushMetrics, a.logger), true, false, "POST")
a.RegisterRoute(OTLPPushEndpoint, distributor.OTLPHandler(pushConfig.MaxOTLPRequestSize, d.RequestBufferPool, a.sourceIPs, limits, pushConfig.RetryConfig, d.PushWithMiddlewares, d.PushMetrics, reg, a.logger, pushConfig.DirectOTLPTranslationEnabled), true, false, "POST")

a.indexPage.AddLinks(defaultWeight, "Distributor", []IndexPageLink{
Expand Down
16 changes: 11 additions & 5 deletions pkg/distributor/distributor.go
Original file line number Diff line number Diff line change
Expand Up @@ -198,10 +198,14 @@ type Config struct {
// for testing and for extending the ingester by adding calls to the client
IngesterClientFactory ring_client.PoolFactory `yaml:"-"`

// when true the distributor does not validate the label name and value, Mimir doesn't directly use
// this (and should never use it) but this feature is used by other projects built on top of it
// When SkipLabelValidation is true the distributor does not validate the label name and value, Mimir doesn't directly use
// this (and should never use it) but this feature is used by other projects built on top of it.
SkipLabelValidation bool `yaml:"-"`

// When SkipLabelCountValidation is true the distributor does not validate the number of labels, Mimir doesn't directly use
// this (and should never use it) but this feature is used by other projects built on top of it.
SkipLabelCountValidation bool `yaml:"-"`

// This config is dynamically injected because it is defined in the querier config.
ShuffleShardingLookbackPeriod time.Duration `yaml:"-"`
StreamingChunksPerIngesterSeriesBufferSize uint64 `yaml:"-"`
Expand Down Expand Up @@ -713,8 +717,8 @@ func (d *Distributor) checkSample(ctx context.Context, userID, cluster, replica
// May alter timeseries data in-place.
// The returned error may retain the series labels.
// It uses the passed nowt time to observe the delay of sample timestamps.
func (d *Distributor) validateSeries(nowt time.Time, ts *mimirpb.PreallocTimeseries, userID, group string, skipLabelValidation bool, minExemplarTS, maxExemplarTS int64) error {
if err := validateLabels(d.sampleValidationMetrics, d.limits, userID, group, ts.Labels, skipLabelValidation); err != nil {
func (d *Distributor) validateSeries(nowt time.Time, ts *mimirpb.PreallocTimeseries, userID, group string, skipLabelValidation, skipLabelCountValidation bool, minExemplarTS, maxExemplarTS int64) error {
if err := validateLabels(d.sampleValidationMetrics, d.limits, userID, group, ts.Labels, skipLabelValidation, skipLabelCountValidation); err != nil {
return err
}

Expand Down Expand Up @@ -1051,8 +1055,10 @@ func (d *Distributor) prePushValidationMiddleware(next PushFunc) PushFunc {
d.labelsHistogram.Observe(float64(len(ts.Labels)))

skipLabelValidation := d.cfg.SkipLabelValidation || req.GetSkipLabelValidation()
skipLabelCountValidation := d.cfg.SkipLabelCountValidation || req.GetSkipLabelCountValidation()

// Note that validateSeries may drop some data in ts.
validationErr := d.validateSeries(now, &req.Timeseries[tsIdx], userID, group, skipLabelValidation, minExemplarTS, maxExemplarTS)
validationErr := d.validateSeries(now, &req.Timeseries[tsIdx], userID, group, skipLabelValidation, skipLabelCountValidation, minExemplarTS, maxExemplarTS)

// Errors in validation are considered non-fatal, as one series in a request may contain
// invalid data but all the remaining series could be perfectly valid.
Expand Down
2 changes: 1 addition & 1 deletion pkg/distributor/distributor_ingest_storage_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -301,7 +301,7 @@ func TestDistributor_Push_ShouldReturnErrorMappedTo4xxStatusCodeIfWriteRequestCo
sourceIPs, _ := middleware.NewSourceIPs("SomeField", "(.*)", false)

// Send write request through the HTTP handler.
h := Handler(maxRecvMsgSize, nil, sourceIPs, false, overrides, RetryConfig{}, distributors[0].PushWithMiddlewares, nil, log.NewNopLogger())
h := Handler(maxRecvMsgSize, nil, sourceIPs, false, false, overrides, RetryConfig{}, distributors[0].PushWithMiddlewares, nil, log.NewNopLogger())
h.ServeHTTP(resp, createRequest(t, marshalledReq))
assert.Equal(t, http.StatusBadRequest, resp.Code)
})
Expand Down
4 changes: 2 additions & 2 deletions pkg/distributor/distributor_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1661,7 +1661,7 @@ func TestDistributor_ExemplarValidation(t *testing.T) {
require.Len(t, regs, 1)

for _, ts := range tc.req.Timeseries {
err := ds[0].validateSeries(now, &ts, "user", "test-group", false, tc.minExemplarTS, tc.maxExemplarTS)
err := ds[0].validateSeries(now, &ts, "user", "test-group", false, false, tc.minExemplarTS, tc.maxExemplarTS)
assert.NoError(t, err)
}

Expand Down Expand Up @@ -1768,7 +1768,7 @@ func TestDistributor_HistogramReduction(t *testing.T) {
require.Len(t, regs, 1)

for _, ts := range tc.req.Timeseries {
err := ds[0].validateSeries(now, &ts, "user", "test-group", false, 0, 0)
err := ds[0].validateSeries(now, &ts, "user", "test-group", false, false, 0, 0)
if tc.expectedError != nil {
require.ErrorAs(t, err, &tc.expectedError)
} else {
Expand Down
16 changes: 13 additions & 3 deletions pkg/distributor/push.go
Original file line number Diff line number Diff line change
Expand Up @@ -44,8 +44,10 @@ var (
)

const (
SkipLabelNameValidationHeader = "X-Mimir-SkipLabelNameValidation"
statusClientClosedRequest = 499
SkipLabelNameValidationHeader = "X-Mimir-SkipLabelNameValidation"
SkipLabelCountValidationHeader = "X-Mimir-SkipLabelCountValidation"

statusClientClosedRequest = 499
)

type RetryConfig struct {
Expand Down Expand Up @@ -77,13 +79,14 @@ func Handler(
requestBufferPool util.Pool,
sourceIPs *middleware.SourceIPExtractor,
allowSkipLabelNameValidation bool,
allowSkipLabelCountValidation bool,
limits *validation.Overrides,
retryCfg RetryConfig,
push PushFunc,
pushMetrics *PushMetrics,
logger log.Logger,
) http.Handler {
return handler(maxRecvMsgSize, requestBufferPool, sourceIPs, allowSkipLabelNameValidation, limits, retryCfg, push, logger, func(ctx context.Context, r *http.Request, maxRecvMsgSize int, buffers *util.RequestBuffers, req *mimirpb.PreallocWriteRequest, _ log.Logger) error {
return handler(maxRecvMsgSize, requestBufferPool, sourceIPs, allowSkipLabelNameValidation, allowSkipLabelCountValidation, limits, retryCfg, push, logger, func(ctx context.Context, r *http.Request, maxRecvMsgSize int, buffers *util.RequestBuffers, req *mimirpb.PreallocWriteRequest, _ log.Logger) error {
protoBodySize, err := util.ParseProtoReader(ctx, r.Body, int(r.ContentLength), maxRecvMsgSize, buffers, req, util.RawSnappy)
if errors.Is(err, util.MsgSizeTooLargeErr{}) {
err = distributorMaxWriteMessageSizeErr{actual: int(r.ContentLength), limit: maxRecvMsgSize}
Expand Down Expand Up @@ -130,6 +133,7 @@ func handler(
requestBufferPool util.Pool,
sourceIPs *middleware.SourceIPExtractor,
allowSkipLabelNameValidation bool,
allowSkipLabelCountValidation bool,
limits *validation.Overrides,
retryCfg RetryConfig,
push PushFunc,
Expand Down Expand Up @@ -177,6 +181,12 @@ func handler(
req.SkipLabelValidation = false
}

if allowSkipLabelCountValidation {
req.SkipLabelCountValidation = req.SkipLabelCountValidation && r.Header.Get(SkipLabelCountValidationHeader) == "true"
} else {
req.SkipLabelCountValidation = false
}

cleanup := func() {
mimirpb.ReuseSlice(req.Timeseries)
rb.CleanUp()
Expand Down
Loading

0 comments on commit 7ee1cf5

Please sign in to comment.