From 4b868113bbcaeea0df2325dd591a8576d6e19fad Mon Sep 17 00:00:00 2001 From: Conor Broderick Date: Mon, 24 Jul 2017 13:49:20 +0100 Subject: [PATCH] Metric name validation (#2975) --- storage/remote/read.go | 25 ++++++++- storage/remote/read_test.go | 109 ++++++++++++++++++++++++++++++++++++ 2 files changed, 133 insertions(+), 1 deletion(-) diff --git a/storage/remote/read.go b/storage/remote/read.go index 0c1886f0e..02752bcab 100644 --- a/storage/remote/read.go +++ b/storage/remote/read.go @@ -14,6 +14,7 @@ package remote import ( + "fmt" "sync" "time" @@ -93,11 +94,33 @@ func (q *querier) read(ctx context.Context, from, through model.Time, matchers m if err != nil { return nil, err } - removeLabels(res, added) + err = validateLabelsAndMetricName(res) + if err != nil { + return nil, err + } + return res, err } +// validateLabelsAndMetricName validates the label names/values and metric names returned from remote read. +func validateLabelsAndMetricName(res model.Matrix) error { + for _, r := range res { + if !model.IsValidMetricName(r.Metric[model.MetricNameLabel]) { + return fmt.Errorf("Invalid metric name: %v", r.Metric[model.MetricNameLabel]) + } + for name, value := range r.Metric { + if !name.IsValid() { + return fmt.Errorf("Invalid label name: %v", name) + } + if !value.IsValid() { + return fmt.Errorf("Invalid label value: %v", value) + } + } + } + return nil +} + // addExternalLabels adds matchers for each external label. External labels // that already have a corresponding user-supplied matcher are skipped, as we // assume that the user explicitly wants to select a different value for them. diff --git a/storage/remote/read_test.go b/storage/remote/read_test.go index 9faa067e4..f6333bdfa 100644 --- a/storage/remote/read_test.go +++ b/storage/remote/read_test.go @@ -22,6 +22,115 @@ import ( "github.com/prometheus/prometheus/storage/metric" ) +func TestValidateLabelsAndMetricName(t *testing.T) { + tests := []struct { + result model.Matrix + expectedErr string + shouldPass bool + }{ + { + result: model.Matrix{ + &model.SampleStream{ + Metric: model.Metric{ + "__name__": "name", + "labelName": "labelValue", + }, + }, + }, + expectedErr: "", + shouldPass: true, + }, + { + result: model.Matrix{ + &model.SampleStream{ + Metric: model.Metric{ + "__name__": "name", + "_labelName": "labelValue", + }, + }, + }, + expectedErr: "", + shouldPass: true, + }, + { + result: model.Matrix{ + &model.SampleStream{ + Metric: model.Metric{ + "__name__": "name", + "@labelName": "labelValue", + }, + }, + }, + expectedErr: "Invalid label name: @labelName", + shouldPass: false, + }, + { + result: model.Matrix{ + &model.SampleStream{ + Metric: model.Metric{ + "__name__": "name", + "123labelName": "labelValue", + }, + }, + }, + expectedErr: "Invalid label name: 123labelName", + shouldPass: false, + }, + { + result: model.Matrix{ + &model.SampleStream{ + Metric: model.Metric{ + "__name__": "name", + "": "labelValue", + }, + }, + }, + expectedErr: "Invalid label name: ", + shouldPass: false, + }, + { + result: model.Matrix{ + &model.SampleStream{ + Metric: model.Metric{ + "__name__": "name", + "labelName": model.LabelValue([]byte{0xff}), + }, + }, + }, + expectedErr: "Invalid label value: " + string([]byte{0xff}), + shouldPass: false, + }, + { + result: model.Matrix{ + &model.SampleStream{ + Metric: model.Metric{ + "__name__": "@invalid_name", + }, + }, + }, + expectedErr: "Invalid metric name: @invalid_name", + shouldPass: false, + }, + } + + for _, test := range tests { + err := validateLabelsAndMetricName(test.result) + if test.shouldPass { + if err != nil { + t.Fatalf("Test should pass, got unexpected error: %v", err) + } + continue + } + if err != nil { + if err.Error() != test.expectedErr { + t.Fatalf("Unexpected error, got: %v, expected: %v", err, test.expectedErr) + } + } else { + t.Fatalf("Expected error, got none") + } + } +} + func mustNewLabelMatcher(mt metric.MatchType, name model.LabelName, val model.LabelValue) *metric.LabelMatcher { m, err := metric.NewLabelMatcher(mt, name, val) if err != nil {