From 576ee4d3099131e12aa56f7562de416393ad86f4 Mon Sep 17 00:00:00 2001 From: Ganesh Vernekar Date: Thu, 13 Sep 2018 15:27:36 +0530 Subject: [PATCH] Label name check for 'count_values' (#4585) Signed-off-by: Ganesh Vernekar --- promql/engine.go | 4 ++++ promql/engine_test.go | 20 +++++++++++++++----- 2 files changed, 19 insertions(+), 5 deletions(-) diff --git a/promql/engine.go b/promql/engine.go index 721e0d255..2051774af 100644 --- a/promql/engine.go +++ b/promql/engine.go @@ -29,6 +29,7 @@ import ( "github.com/go-kit/kit/log/level" opentracing "github.com/opentracing/opentracing-go" "github.com/prometheus/client_golang/prometheus" + "github.com/prometheus/common/model" "github.com/prometheus/prometheus/pkg/labels" "github.com/prometheus/prometheus/pkg/timestamp" "github.com/prometheus/prometheus/pkg/value" @@ -1494,6 +1495,9 @@ func (ev *evaluator) aggregation(op ItemType, grouping []string, without bool, p var valueLabel string if op == itemCountValues { valueLabel = param.(string) + if !model.LabelName(valueLabel).IsValid() { + ev.errorf("invalid label name %q", valueLabel) + } if !without { grouping = append(grouping, valueLabel) } diff --git a/promql/engine_test.go b/promql/engine_test.go index 3bf40c2fc..6b9bf5665 100644 --- a/promql/engine_test.go +++ b/promql/engine_test.go @@ -23,6 +23,7 @@ import ( "github.com/go-kit/kit/log" "github.com/prometheus/prometheus/pkg/labels" "github.com/prometheus/prometheus/storage" + "github.com/prometheus/prometheus/util/testutil" ) func TestQueryConcurrency(t *testing.T) { @@ -267,11 +268,12 @@ load 10s } cases := []struct { - Query string - Result Value - Start time.Time - End time.Time - Interval time.Duration + Query string + Result Value + Start time.Time + End time.Time + Interval time.Duration + ShouldError bool }{ // Instant queries. { @@ -326,6 +328,10 @@ load 10s End: time.Unix(10, 0), Interval: 5 * time.Second, }, + { + Query: `count_values("wrong label!", metric)`, + ShouldError: true, + }, } for _, c := range cases { @@ -340,6 +346,10 @@ load 10s t.Fatalf("unexpected error creating query: %q", err) } res := qry.Exec(test.Context()) + if c.ShouldError { + testutil.NotOk(t, res.Err, "expected error for the query %q", c.Query) + continue + } if res.Err != nil { t.Fatalf("unexpected error running query: %q", res.Err) }