Skip to content

Commit ccad326

Browse files
juliusmhjuliusmh
committed
feat(querier): validate label/metric names with per-tenant naming scheme
1 parent 8ec6abb commit ccad326

22 files changed

+255
-107
lines changed

‎pkg/cardinality/request.go‎

Lines changed: 22 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@ import (
1313
"github.com/pkg/errors"
1414
"github.com/prometheus/common/model"
1515
"github.com/prometheus/prometheus/model/labels"
16+
"github.com/prometheus/prometheus/model/validation"
1617
"github.com/prometheus/prometheus/promql/parser"
1718
)
1819

@@ -145,27 +146,38 @@ func (r *LabelValuesRequest) String() string {
145146

146147
// DecodeLabelValuesRequest decodes the input http.Request into a LabelValuesRequest.
147148
// The input http.Request can either be a GET or POST with URL-encoded parameters.
148-
func DecodeLabelValuesRequest(r *http.Request, tenantMaxLimit int) (*LabelValuesRequest, error) {
149+
func DecodeLabelValuesRequest(
150+
r *http.Request,
151+
tenantMaxLimit int,
152+
tenantNameValidationScheme validation.NamingScheme,
153+
) (*LabelValuesRequest, error) {
149154
if err := r.ParseForm(); err != nil {
150155
return nil, err
151156
}
152-
153-
return DecodeLabelValuesRequestFromValuesWithTenantMaxLimit(r.Form, tenantMaxLimit)
157+
return DecodeLabelValuesRequestFromValuesWithTenantLimits(
158+
r.Form,
159+
tenantMaxLimit,
160+
tenantNameValidationScheme,
161+
)
154162
}
155163

156164
// DecodeLabelValuesRequestFromValues is like DecodeLabelValuesRequest but takes url.Values in input.
157165
func DecodeLabelValuesRequestFromValues(values url.Values) (*LabelValuesRequest, error) {
158-
return DecodeLabelValuesRequestFromValuesWithTenantMaxLimit(values, 0)
166+
return DecodeLabelValuesRequestFromValuesWithTenantLimits(values, 0, validation.LegacyNamingScheme)
159167
}
160168

161-
// DecodeLabelValuesRequestFromValuesWithTenantMaxLimit is like DecodeLabelValuesRequestFromValues but also accepts the tenantMaxLimit
162-
func DecodeLabelValuesRequestFromValuesWithTenantMaxLimit(values url.Values, tenantMaxLimit int) (*LabelValuesRequest, error) {
169+
// DecodeLabelValuesRequestFromValuesWithTenantLimits decodes label values from url.Values, respecting tenant limits.
170+
func DecodeLabelValuesRequestFromValuesWithTenantLimits(
171+
values url.Values,
172+
tenantMaxLimit int,
173+
tenantNameValidationScheme validation.NamingScheme,
174+
) (*LabelValuesRequest, error) {
163175
var (
164176
parsed = &LabelValuesRequest{}
165177
err error
166178
)
167179

168-
parsed.LabelNames, err = extractLabelNames(values)
180+
parsed.LabelNames, err = extractLabelNames(values, tenantNameValidationScheme)
169181
if err != nil {
170182
return nil, err
171183
}
@@ -254,18 +266,18 @@ func extractLimit(values url.Values, tenantMaxLimit int) (limit int, err error)
254266
}
255267

256268
// extractLabelNames parses and gets label_names query parameter containing an array of label values
257-
func extractLabelNames(values url.Values) ([]model.LabelName, error) {
269+
func extractLabelNames(values url.Values, nameValidationScheme validation.NamingScheme) ([]model.LabelName, error) {
258270
labelNamesParams := values["label_names[]"]
259271
if len(labelNamesParams) == 0 {
260272
return nil, fmt.Errorf("'label_names[]' param is required")
261273
}
262274

263275
labelNames := make([]model.LabelName, 0, len(labelNamesParams))
264276
for _, labelNameParam := range labelNamesParams {
265-
labelName := model.LabelName(labelNameParam)
266-
if !labelName.IsValid() {
277+
if !nameValidationScheme.IsValidLabelName(labelNameParam) {
267278
return nil, fmt.Errorf("invalid 'label_names' param '%v'", labelNameParam)
268279
}
280+
labelName := model.LabelName(labelNameParam)
269281
labelNames = append(labelNames, labelName)
270282
}
271283

‎pkg/cardinality/request_test.go‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@ import (
1010

1111
"github.com/prometheus/common/model"
1212
"github.com/prometheus/prometheus/model/labels"
13+
"github.com/prometheus/prometheus/model/validation"
1314
"github.com/stretchr/testify/assert"
1415
"github.com/stretchr/testify/require"
1516
)
@@ -98,7 +99,7 @@ func TestDecodeLabelValuesRequest(t *testing.T) {
9899
req, err := http.NewRequest("GET", "http://localhost?"+params.Encode(), nil)
99100
require.NoError(t, err)
100101

101-
actual, err := DecodeLabelValuesRequest(req, 0)
102+
actual, err := DecodeLabelValuesRequest(req, 0, validation.LegacyNamingScheme)
102103
require.NoError(t, err)
103104

104105
assert.Equal(t, expected, actual)
@@ -109,7 +110,7 @@ func TestDecodeLabelValuesRequest(t *testing.T) {
109110
require.NoError(t, err)
110111
req.Header.Set("Content-Type", "application/x-www-form-urlencoded")
111112

112-
actual, err := DecodeLabelValuesRequest(req, 0)
113+
actual, err := DecodeLabelValuesRequest(req, 0, validation.LegacyNamingScheme)
113114
require.NoError(t, err)
114115

115116
assert.Equal(t, expected, actual)

‎pkg/mimir/modules.go‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -827,6 +827,8 @@ func (t *Mimir) initQueryFrontendTripperware() (serv services.Service, err error
827827
panic(fmt.Sprintf("invalid config not caught by validation: unknown PromQL engine '%s'", t.Cfg.Querier.QueryEngine))
828828
}
829829

830+
eng = streamingpromqlcompat.NameValidatingEngine(eng, t.Overrides)
831+
830832
tripperware, err := querymiddleware.NewTripperware(
831833
t.Cfg.Frontend.QueryMiddleware,
832834
util_log.Logger,

‎pkg/querier/cardinality_analysis_handler.go‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -67,7 +67,8 @@ func LabelValuesCardinalityHandler(distributor Distributor, limits *validation.O
6767
return
6868
}
6969
tenantMaxLimit := limits.CardinalityAnalysisMaxResults(tenantID)
70-
cardinalityRequest, err := cardinality.DecodeLabelValuesRequest(r, tenantMaxLimit)
70+
tenantValidationScheme := limits.NameValidationScheme(tenantID)
71+
cardinalityRequest, err := cardinality.DecodeLabelValuesRequest(r, tenantMaxLimit, tenantValidationScheme)
7172
if err != nil {
7273
http.Error(w, err.Error(), http.StatusBadRequest)
7374
return

‎pkg/querier/engine/config.go‎

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -53,7 +53,12 @@ func (cfg *Config) RegisterFlags(f *flag.FlagSet) {
5353
}
5454

5555
// NewPromQLEngineOptions returns the PromQL engine options based on the provided config.
56-
func NewPromQLEngineOptions(cfg Config, activityTracker *activitytracker.ActivityTracker, logger log.Logger, reg prometheus.Registerer) (promql.EngineOpts, streamingpromql.EngineOpts) {
56+
func NewPromQLEngineOptions(
57+
cfg Config,
58+
activityTracker *activitytracker.ActivityTracker,
59+
logger log.Logger,
60+
reg prometheus.Registerer,
61+
) (promql.EngineOpts, streamingpromql.EngineOpts) {
5762
commonOpts := promql.EngineOpts{
5863
Logger: util_log.SlogFromGoKit(logger),
5964
Reg: reg,

‎pkg/querier/querier.go‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -197,6 +197,7 @@ func New(cfg Config, limits *validation.Overrides, distributor Distributor, quer
197197
panic(fmt.Sprintf("invalid config not caught by validation: unknown PromQL engine '%s'", cfg.QueryEngine))
198198
}
199199

200+
eng = compat.NameValidatingEngine(eng, limits)
200201
return NewSampleAndChunkQueryable(lazyQueryable), exemplarQueryable, eng, nil
201202
}
202203

‎pkg/querier/stats_renderer_test.go‎

Lines changed: 0 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,6 @@ import (
1313
"github.com/grafana/dskit/user"
1414
"github.com/grafana/regexp"
1515
"github.com/prometheus/client_golang/prometheus"
16-
"github.com/prometheus/common/model"
1716
"github.com/prometheus/common/promslog"
1817
"github.com/prometheus/common/route"
1918
"github.com/prometheus/prometheus/config"
@@ -26,12 +25,6 @@ import (
2625
mimir_stats "github.com/grafana/mimir/pkg/querier/stats"
2726
)
2827

29-
func init() {
30-
// Mimir doesn't support Prometheus' UTF-8 metric/label name scheme yet.
31-
// nolint:staticcheck
32-
model.NameValidationScheme = model.LegacyValidation
33-
}
34-
3528
func TestStatsRenderer(t *testing.T) {
3629

3730
testCases := map[string]struct {

‎pkg/streamingpromql/compat/name_validating_engine.go‎

Lines changed: 72 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,72 @@
1+
// SPDX-License-Identifier: AGPL-3.0-only
2+
3+
package compat
4+
5+
import (
6+
"context"
7+
"time"
8+
9+
"github.com/grafana/dskit/tenant"
10+
"github.com/grafana/mimir/pkg/util/validation"
11+
prom_validation "github.com/prometheus/prometheus/model/validation"
12+
"github.com/prometheus/prometheus/promql"
13+
"github.com/prometheus/prometheus/storage"
14+
)
15+
16+
type nameValidatingEngine struct {
17+
engine promql.QueryEngine
18+
limits *validation.Overrides
19+
}
20+
21+
// NameValidatingEngine creates a new promql.QueryEngine that wraps engine and overrides query options
22+
// with the tenants naming scheme from limits.
23+
func NameValidatingEngine(engine promql.QueryEngine, limits *validation.Overrides) promql.QueryEngine {
24+
return &nameValidatingEngine{engine: engine, limits: limits}
25+
}
26+
27+
type optsWithNamingScheme struct {
28+
promql.QueryOpts
29+
namingScheme prom_validation.NamingScheme
30+
}
31+
32+
func (o optsWithNamingScheme) EnablePerStepStats() bool {
33+
return o.QueryOpts.EnablePerStepStats()
34+
}
35+
36+
func (o optsWithNamingScheme) LookbackDelta() time.Duration {
37+
return o.QueryOpts.LookbackDelta()
38+
}
39+
40+
func (o optsWithNamingScheme) NameValidationScheme() prom_validation.NamingScheme {
41+
return o.namingScheme
42+
}
43+
44+
func (e nameValidatingEngine) NewInstantQuery(ctx context.Context, q storage.Queryable, opts promql.QueryOpts, qs string, ts time.Time) (promql.Query, error) {
45+
userID, err := tenant.TenantID(ctx)
46+
if err != nil {
47+
return nil, err
48+
}
49+
if opts == nil {
50+
opts = promql.NewPrometheusQueryOpts(false, 0, prom_validation.UTF8NamingScheme)
51+
}
52+
opts = &optsWithNamingScheme{
53+
QueryOpts: opts,
54+
namingScheme: e.limits.NameValidationScheme(userID),
55+
}
56+
return e.engine.NewInstantQuery(ctx, q, opts, qs, ts)
57+
}
58+
59+
func (e nameValidatingEngine) NewRangeQuery(ctx context.Context, q storage.Queryable, opts promql.QueryOpts, qs string, start, end time.Time, interval time.Duration) (promql.Query, error) {
60+
userID, err := tenant.TenantID(ctx)
61+
if err != nil {
62+
return nil, err
63+
}
64+
if opts == nil {
65+
opts = promql.NewPrometheusQueryOpts(false, 0, prom_validation.UTF8NamingScheme)
66+
}
67+
opts = &optsWithNamingScheme{
68+
QueryOpts: opts,
69+
namingScheme: e.limits.NameValidationScheme(userID),
70+
}
71+
return e.engine.NewRangeQuery(ctx, q, opts, qs, start, end, interval)
72+
}

‎pkg/streamingpromql/engine.go‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@ import (
1515
"github.com/go-kit/log"
1616
"github.com/prometheus/client_golang/prometheus"
1717
"github.com/prometheus/client_golang/prometheus/promauto"
18+
"github.com/prometheus/prometheus/model/validation"
1819
"github.com/prometheus/prometheus/promql"
1920
"github.com/prometheus/prometheus/storage"
2021
"go.opentelemetry.io/otel"
@@ -156,6 +157,7 @@ func NewStaticQueryLimitsProvider(maxEstimatedMemoryConsumptionPerQuery uint64)
156157

157158
type staticQueryLimitsProvider struct {
158159
maxEstimatedMemoryConsumptionPerQuery uint64
160+
nameValidationScheme validation.NamingScheme
159161
}
160162

161163
func (p staticQueryLimitsProvider) GetMaxEstimatedMemoryConsumptionPerQuery(_ context.Context) (uint64, error) {

‎pkg/streamingpromql/engine_test.go‎

Lines changed: 7 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,7 @@ import (
2424
"github.com/prometheus/prometheus/model/histogram"
2525
"github.com/prometheus/prometheus/model/labels"
2626
"github.com/prometheus/prometheus/model/timestamp"
27+
"github.com/prometheus/prometheus/model/validation"
2728
"github.com/prometheus/prometheus/promql"
2829
"github.com/prometheus/prometheus/promql/parser"
2930
"github.com/prometheus/prometheus/promql/parser/posrange"
@@ -3207,7 +3208,7 @@ func TestQueryStats(t *testing.T) {
32073208
runQueryAndGetSamplesStats := func(t *testing.T, engine promql.QueryEngine, expr string, isInstantQuery bool) *promstats.QuerySamples {
32083209
var q promql.Query
32093210
var err error
3210-
opts := promql.NewPrometheusQueryOpts(true, 0)
3211+
opts := promql.NewPrometheusQueryOpts(true, 0, validation.LegacyNamingScheme)
32113212
if isInstantQuery {
32123213
q, err = engine.NewInstantQuery(context.Background(), storage, opts, expr, end)
32133214
} else {
@@ -3501,7 +3502,7 @@ func TestQueryStatsUpstreamTestCases(t *testing.T) {
35013502
runQueryAndGetSamplesStats := func(t *testing.T, engine promql.QueryEngine, expr string, start, end time.Time, interval time.Duration) *promstats.QuerySamples {
35023503
var q promql.Query
35033504
var err error
3504-
opts := promql.NewPrometheusQueryOpts(true, 0)
3505+
opts := promql.NewPrometheusQueryOpts(true, 0, validation.LegacyNamingScheme)
35053506

35063507
if interval == 0 {
35073508
// Instant query
@@ -3882,7 +3883,7 @@ func TestQueryStatementLookbackDelta(t *testing.T) {
38823883
require.NoError(t, err)
38833884

38843885
t.Run("lookback delta not set in query options", func(t *testing.T) {
3885-
queryOpts := promql.NewPrometheusQueryOpts(false, 0)
3886+
queryOpts := promql.NewPrometheusQueryOpts(false, 0, validation.LegacyNamingScheme)
38863887
runTest(t, engine, queryOpts, defaultLookbackDelta)
38873888
})
38883889

@@ -3891,7 +3892,7 @@ func TestQueryStatementLookbackDelta(t *testing.T) {
38913892
})
38923893

38933894
t.Run("lookback delta set in query options", func(t *testing.T) {
3894-
queryOpts := promql.NewPrometheusQueryOpts(false, 14*time.Minute)
3895+
queryOpts := promql.NewPrometheusQueryOpts(false, 14*time.Minute, validation.LegacyNamingScheme)
38953896
runTest(t, engine, queryOpts, 14*time.Minute)
38963897
})
38973898
})
@@ -3903,7 +3904,7 @@ func TestQueryStatementLookbackDelta(t *testing.T) {
39033904
require.NoError(t, err)
39043905

39053906
t.Run("lookback delta not set in query options", func(t *testing.T) {
3906-
queryOpts := promql.NewPrometheusQueryOpts(false, 0)
3907+
queryOpts := promql.NewPrometheusQueryOpts(false, 0, validation.LegacyNamingScheme)
39073908
runTest(t, engine, queryOpts, 12*time.Minute)
39083909
})
39093910

@@ -3912,7 +3913,7 @@ func TestQueryStatementLookbackDelta(t *testing.T) {
39123913
})
39133914

39143915
t.Run("lookback delta set in query options", func(t *testing.T) {
3915-
queryOpts := promql.NewPrometheusQueryOpts(false, 14*time.Minute)
3916+
queryOpts := promql.NewPrometheusQueryOpts(false, 14*time.Minute, validation.LegacyNamingScheme)
39163917
runTest(t, engine, queryOpts, 14*time.Minute)
39173918
})
39183919
})

0 commit comments

Comments
 (0)