Skip to content

Commit 3251e86

Browse files
authored
Early Request Validation in Query Frontend (#10093)
* WIP sketch out validation roundtripper with existing metrics and labels query codecs * lint: license header * separate series request roundtrippers * introduce request validation roundtripper tests * lint import order * introduce & test cardinality query validation in frontend roundtripper; expand metrics query tests to cover instant queries * CHANGELOG
1 parent a26eee3 commit 3251e86

File tree

6 files changed

+387
-40
lines changed

6 files changed

+387
-40
lines changed

CHANGELOG.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -78,6 +78,7 @@
7878
* [ENHANCEMENT] Distributor: allow a different limit for info series (series ending in `_info`) label count, via `-validation.max-label-names-per-info-series`. #10028
7979
* [ENHANCEMENT] Ingester: do not reuse labels, samples and histograms slices in the write request if there are more entries than 10x the pre-allocated size. This should help to reduce the in-use memory in case of few requests with a very large number of labels, samples or histograms. #10040
8080
* [ENHANCEMENT] Query-Frontend: prune `<subquery> and on() (vector(x)==y)` style queries and stop pruning `<subquery> < -Inf`. Triggered by https://github.com/prometheus/prometheus/pull/15245. #10026
81+
* [ENHANCEMENT] Query-Frontend: perform request format validation before processing the request. #10093
8182
* [BUGFIX] Fix issue where functions such as `rate()` over native histograms could return incorrect values if a float stale marker was present in the selected range. #9508
8283
* [BUGFIX] Fix issue where negation of native histograms (eg. `-some_native_histogram_series`) did nothing. #9508
8384
* [BUGFIX] Fix issue where `metric might not be a counter, name does not end in _total/_sum/_count/_bucket` annotation would be emitted even if `rate` or `increase` did not have enough samples to compute a result. #9508

pkg/frontend/querymiddleware/codec.go

Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@ package querymiddleware
88
import (
99
"bytes"
1010
"context"
11+
"errors"
1112
"fmt"
1213
"io"
1314
"net/http"
@@ -32,6 +33,7 @@ import (
3233
"golang.org/x/exp/slices"
3334

3435
apierror "github.com/grafana/mimir/pkg/api/error"
36+
"github.com/grafana/mimir/pkg/cardinality"
3537
"github.com/grafana/mimir/pkg/mimirpb"
3638
"github.com/grafana/mimir/pkg/querier/api"
3739
"github.com/grafana/mimir/pkg/querier/stats"
@@ -582,6 +584,39 @@ func DecodeLabelsSeriesQueryTimeParams(reqValues *url.Values) (start, end int64,
582584
return start, end, err
583585
}
584586

587+
// DecodeCardinalityQueryParams strictly handles validation for cardinality API endpoint parameters.
588+
// The current decoding of the cardinality requests is handled in the cardinality package
589+
// which is not yet compatible with the codec's approach of using interfaces
590+
// and multiple concrete proto implementations to represent different query types.
591+
func DecodeCardinalityQueryParams(r *http.Request) (any, error) {
592+
var err error
593+
594+
reqValues, err := util.ParseRequestFormWithoutConsumingBody(r)
595+
if err != nil {
596+
return nil, apierror.New(apierror.TypeBadData, err.Error())
597+
}
598+
599+
var parsedReq any
600+
switch {
601+
case strings.HasSuffix(r.URL.Path, cardinalityLabelNamesPathSuffix):
602+
parsedReq, err = cardinality.DecodeLabelNamesRequestFromValues(reqValues)
603+
604+
case strings.HasSuffix(r.URL.Path, cardinalityLabelValuesPathSuffix):
605+
parsedReq, err = cardinality.DecodeLabelValuesRequestFromValues(reqValues)
606+
607+
case strings.HasSuffix(r.URL.Path, cardinalityActiveSeriesPathSuffix):
608+
parsedReq, err = cardinality.DecodeActiveSeriesRequestFromValues(reqValues)
609+
610+
default:
611+
return nil, errors.New("unknown cardinality API endpoint")
612+
}
613+
614+
if err != nil {
615+
return nil, apierror.New(apierror.TypeBadData, err.Error())
616+
}
617+
return parsedReq, nil
618+
}
619+
585620
func decodeQueryMinMaxTime(queryExpr parser.Expr, start, end, step int64, lookbackDelta time.Duration) (minTime, maxTime int64) {
586621
evalStmt := &parser.EvalStmt{
587622
Expr: queryExpr,
Lines changed: 92 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,92 @@
1+
// SPDX-License-Identifier: AGPL-3.0-only
2+
3+
package querymiddleware
4+
5+
import (
6+
"context"
7+
"net/http"
8+
9+
"github.com/grafana/dskit/cancellation"
10+
)
11+
12+
const requestValidationFailedFmt = "request validation failed for "
13+
14+
var errMetricsQueryRequestValidationFailed = cancellation.NewErrorf(
15+
requestValidationFailedFmt + "metrics query",
16+
)
17+
var errLabelsQueryRequestValidationFailed = cancellation.NewErrorf(
18+
requestValidationFailedFmt + "labels query",
19+
)
20+
var errCardinalityQueryRequestValidationFailed = cancellation.NewErrorf(
21+
requestValidationFailedFmt + "cardinality query",
22+
)
23+
24+
type MetricsQueryRequestValidationRoundTripper struct {
25+
codec Codec
26+
next http.RoundTripper
27+
}
28+
29+
func NewMetricsQueryRequestValidationRoundTripper(codec Codec, next http.RoundTripper) http.RoundTripper {
30+
return MetricsQueryRequestValidationRoundTripper{
31+
codec: codec,
32+
next: next,
33+
}
34+
}
35+
36+
func (rt MetricsQueryRequestValidationRoundTripper) RoundTrip(r *http.Request) (*http.Response, error) {
37+
ctx, cancel := context.WithCancelCause(r.Context())
38+
defer cancel(errMetricsQueryRequestValidationFailed)
39+
r = r.WithContext(ctx)
40+
41+
_, err := rt.codec.DecodeMetricsQueryRequest(ctx, r)
42+
if err != nil {
43+
return nil, err
44+
}
45+
return rt.next.RoundTrip(r)
46+
}
47+
48+
type LabelsQueryRequestValidationRoundTripper struct {
49+
codec Codec
50+
next http.RoundTripper
51+
}
52+
53+
func NewLabelsQueryRequestValidationRoundTripper(codec Codec, next http.RoundTripper) http.RoundTripper {
54+
return LabelsQueryRequestValidationRoundTripper{
55+
codec: codec,
56+
next: next,
57+
}
58+
}
59+
60+
func (rt LabelsQueryRequestValidationRoundTripper) RoundTrip(r *http.Request) (*http.Response, error) {
61+
ctx, cancel := context.WithCancelCause(r.Context())
62+
defer cancel(errLabelsQueryRequestValidationFailed)
63+
r = r.WithContext(ctx)
64+
65+
_, err := rt.codec.DecodeLabelsQueryRequest(ctx, r)
66+
if err != nil {
67+
return nil, err
68+
}
69+
return rt.next.RoundTrip(r)
70+
}
71+
72+
type CardinalityQueryRequestValidationRoundTripper struct {
73+
next http.RoundTripper
74+
}
75+
76+
func NewCardinalityQueryRequestValidationRoundTripper(next http.RoundTripper) http.RoundTripper {
77+
return CardinalityQueryRequestValidationRoundTripper{
78+
next: next,
79+
}
80+
}
81+
82+
func (rt CardinalityQueryRequestValidationRoundTripper) RoundTrip(r *http.Request) (*http.Response, error) {
83+
ctx, cancel := context.WithCancelCause(r.Context())
84+
defer cancel(errCardinalityQueryRequestValidationFailed)
85+
r = r.WithContext(ctx)
86+
87+
_, err := DecodeCardinalityQueryParams(r)
88+
if err != nil {
89+
return nil, err
90+
}
91+
return rt.next.RoundTrip(r)
92+
}

0 commit comments

Comments
 (0)