From 95d8aa93a9198a3fff068a23f6510a934bb46823 Mon Sep 17 00:00:00 2001 From: Yuri Tseretyan Date: Mon, 10 Apr 2023 12:32:08 -0400 Subject: [PATCH 01/12] introduce labels.IsValidName that returns true to strings that contain any UTF-8 characters except all whitespaces Signed-off-by: Yuri Tseretyan --- pkg/labels/labels.go | 48 ++++++++++++++++++++++++++ pkg/labels/labels_test.go | 71 +++++++++++++++++++++++++++++++++++++++ 2 files changed, 119 insertions(+) create mode 100644 pkg/labels/labels.go create mode 100644 pkg/labels/labels_test.go diff --git a/pkg/labels/labels.go b/pkg/labels/labels.go new file mode 100644 index 0000000000..401b008e21 --- /dev/null +++ b/pkg/labels/labels.go @@ -0,0 +1,48 @@ +// Copyright 2017 The Prometheus Authors +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package labels + +import ( + "fmt" + "unicode" + "unicode/utf8" + + "github.com/prometheus/common/model" +) + +// IsValidName validates that model.LabelName is not a whitespace string and contains only valid UTF-8 symbols +func IsValidName(ln model.LabelName) bool { + lns := string(ln) + allSpaces := true + for _, i := range lns { + if !unicode.IsSpace(i) { + allSpaces = false + break + } + } + return !allSpaces && utf8.ValidString(lns) +} + +// IsValidSet validates that model.LabelSet keys and values are are valid +func IsValidSet(ls model.LabelSet) error { + for ln, lv := range ls { + if !IsValidName(ln) { + return fmt.Errorf("invalid name %q", ln) + } + if !lv.IsValid() { + return fmt.Errorf("invalid value %q", lv) + } + } + return nil +} diff --git a/pkg/labels/labels_test.go b/pkg/labels/labels_test.go new file mode 100644 index 0000000000..cc2bb431f1 --- /dev/null +++ b/pkg/labels/labels_test.go @@ -0,0 +1,71 @@ +// Copyright 2017 The Prometheus Authors +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package labels + +import ( + "strings" + "testing" + "unicode" + + "github.com/prometheus/common/model" + "github.com/stretchr/testify/require" +) + +func TestIsValidName(t *testing.T) { + testCases := []struct { + name string + labelName model.LabelName + valid bool + }{ + { + name: "invalid: empty string", + labelName: "", + valid: false, + }, + { + name: "invalid: all spaces", + labelName: " ", + valid: false, + }, + { + name: "invalid: only whitespaces", + labelName: func() model.LabelName { + whiteSpaceBuilder := strings.Builder{} + for _, r16 := range unicode.White_Space.R16 { + for sym := r16.Lo; sym <= r16.Hi; sym += r16.Stride { + whiteSpaceBuilder.WriteRune(rune(sym)) + } + } + return model.LabelName(whiteSpaceBuilder.String()) + }(), + valid: false, + }, + { + name: "valid: Prometheus label", + labelName: "TEST_label_name_12345", + valid: true, + }, + { + name: "valid: any UTF-8 character", + labelName: "\nlabel:test data.爱!🙂\t", + valid: true, + }, + } + + for _, testCase := range testCases { + t.Run(testCase.name, func(t *testing.T) { + require.Equal(t, testCase.valid, IsValidName(testCase.labelName)) + }) + } +} From 56dc89dfc188c29fe3a38bd61653c78cae241dc5 Mon Sep 17 00:00:00 2001 From: Yuri Tseretyan Date: Mon, 10 Apr 2023 12:48:14 -0400 Subject: [PATCH 02/12] implement validate for types.Alert code copied from model.Alert and changed label validation Signed-off-by: Yuri Tseretyan --- types/types.go | 21 ++++++ types/types_test.go | 155 ++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 176 insertions(+) diff --git a/types/types.go b/types/types.go index f3101f8234..688f9d09b5 100644 --- a/types/types.go +++ b/types/types.go @@ -14,6 +14,7 @@ package types import ( + "fmt" "strings" "sync" "time" @@ -302,6 +303,26 @@ type Alert struct { Timeout bool } +// Validate checks whether the alert data is inconsistent. Overrides the method for model.Alert with custom validation of Labels and Annotations +func (a *Alert) Validate() error { + if a.StartsAt.IsZero() { + return fmt.Errorf("start time missing") + } + if !a.EndsAt.IsZero() && a.EndsAt.Before(a.StartsAt) { + return fmt.Errorf("start time must be before end time") + } + if err := labels.IsValidSet(a.Labels); err != nil { + return fmt.Errorf("invalid label set: %s", err) + } + if len(a.Labels) == 0 { + return fmt.Errorf("at least one label pair required") + } + if err := labels.IsValidSet(a.Annotations); err != nil { + return fmt.Errorf("invalid annotations: %s", err) + } + return nil +} + // AlertSlice is a sortable slice of Alerts. type AlertSlice []*Alert diff --git a/types/types_test.go b/types/types_test.go index 04ed3e19e1..bb251a6b0d 100644 --- a/types/types_test.go +++ b/types/types_test.go @@ -17,6 +17,7 @@ import ( "reflect" "sort" "strconv" + "strings" "testing" "time" @@ -443,3 +444,157 @@ func TestNewMarkerRegistersMetrics(t *testing.T) { t.Error("expected NewMarker to register metrics on the given registerer") } } + +func TestAlertValidate(t *testing.T) { + ts := time.Now() + cases := []struct { + name string + alert *Alert + err string + }{ + { + alert: &Alert{ + Alert: model.Alert{ + Labels: model.LabelSet{"a": "b"}, + StartsAt: ts, + }, + }, + }, + { + alert: &Alert{ + Alert: model.Alert{ + Labels: model.LabelSet{"a": "b"}, + }, + }, + err: "start time missing", + }, + { + alert: &Alert{ + Alert: model.Alert{ + Labels: model.LabelSet{"a": "b"}, + StartsAt: ts, + EndsAt: ts, + }, + }, + }, + { + alert: &Alert{ + Alert: model.Alert{ + Labels: model.LabelSet{"a": "b"}, + StartsAt: ts, + EndsAt: ts.Add(1 * time.Minute), + }, + }, + }, + { + alert: &Alert{ + Alert: model.Alert{ + Labels: model.LabelSet{"a": "b"}, + StartsAt: ts, + EndsAt: ts.Add(-1 * time.Minute), + }, + }, + err: "start time must be before end time", + }, + { + alert: &Alert{ + Alert: model.Alert{ + StartsAt: ts, + }, + }, + err: "at least one label pair required", + }, + { + alert: &Alert{ + Alert: model.Alert{ + Labels: model.LabelSet{"!any-utf8.象征.or.‼️": "label"}, + StartsAt: ts, + }, + }, + }, + { + alert: &Alert{ + Alert: model.Alert{ + Labels: model.LabelSet{" \n\t ": "label"}, + StartsAt: ts, + }, + }, + err: "invalid label set: invalid name", + }, + { + alert: &Alert{ + Alert: model.Alert{ + Labels: model.LabelSet{"\xff.name": "label"}, + StartsAt: ts, + }, + }, + err: "invalid label set: invalid name", + }, + { + alert: &Alert{ + Alert: model.Alert{ + Labels: model.LabelSet{"a": "b", "bad": "\xfflabel"}, + StartsAt: ts, + }, + }, + err: "invalid label set: invalid value", + }, + { + alert: &Alert{ + Alert: model.Alert{ + Labels: model.LabelSet{"a": "b"}, + Annotations: model.LabelSet{"!any-utf8.象征.or.‼️": "label"}, + StartsAt: ts, + }, + }, + }, + { + alert: &Alert{ + Alert: model.Alert{ + Labels: model.LabelSet{"a": "b"}, + Annotations: model.LabelSet{" \n\t ": "label"}, + StartsAt: ts, + }, + }, + err: "invalid annotations: invalid name", + }, + { + alert: &Alert{ + Alert: model.Alert{ + Labels: model.LabelSet{"a": "b"}, + Annotations: model.LabelSet{"\xff.name": "label"}, + StartsAt: ts, + }, + }, + err: "invalid annotations: invalid name", + }, + { + alert: &Alert{ + Alert: model.Alert{ + Labels: model.LabelSet{"a": "b"}, + Annotations: model.LabelSet{"bad": "\xfflabel"}, + StartsAt: ts, + }, + }, + err: "invalid annotations: invalid value", + }, + } + + for i, c := range cases { + err := c.alert.Validate() + if err == nil { + if c.err == "" { + continue + } + t.Errorf("%d. Expected error %q but got none", i, c.err) + continue + } + if c.err == "" { + t.Errorf("%d. Expected no error but got %q", i, err) + continue + } + if !strings.Contains(err.Error(), c.err) { + t.Errorf("%d. Expected error to contain %q but got %q", i, c.err, err) + } + } +} From 29e070358f8e8d9acb20c23963bf2afd294586d3 Mon Sep 17 00:00:00 2001 From: Yuri Tseretyan Date: Mon, 10 Apr 2023 12:51:04 -0400 Subject: [PATCH 03/12] update silence matcher validate to use labels.IsValidName Signed-off-by: Yuri Tseretyan --- silence/silence.go | 2 +- silence/silence_test.go | 27 ++++++++++++++++++++++++--- 2 files changed, 25 insertions(+), 4 deletions(-) diff --git a/silence/silence.go b/silence/silence.go index 68844d38b0..52ad8adb29 100644 --- a/silence/silence.go +++ b/silence/silence.go @@ -469,7 +469,7 @@ func (s *Silences) GC() (int, error) { // ValidateMatcher runs validation on the matcher name, type, and pattern. var ValidateMatcher = func(m *pb.Matcher) error { - if !model.LabelName(m.Name).IsValid() { + if !labels.IsValidName(model.LabelName(m.Name)) { return fmt.Errorf("invalid label name %q", m.Name) } switch m.Type { diff --git a/silence/silence_test.go b/silence/silence_test.go index a9c6f634a2..ecc491c015 100644 --- a/silence/silence_test.go +++ b/silence/silence_test.go @@ -1147,13 +1147,21 @@ func TestValidateMatcher(t *testing.T) { Type: pb.Matcher_NOT_REGEXP, }, err: "", - }, { + }, + { m: &pb.Matcher{ - Name: "00", + Name: "\t\n ", Pattern: "a", Type: pb.Matcher_EQUAL, }, err: "invalid label name", + }, + { + m: &pb.Matcher{ + Name: "0.some.label👍", + Pattern: "a", + Type: pb.Matcher_EQUAL, + }, }, { m: &pb.Matcher{ Name: "a", @@ -1239,7 +1247,7 @@ func TestValidateSilence(t *testing.T) { Id: "some_id", Matchers: []*pb.Matcher{ {Name: "a", Pattern: "b"}, - {Name: "00", Pattern: "b"}, + {Name: " ", Pattern: "b"}, }, StartsAt: validTimestamp, EndsAt: validTimestamp, @@ -1247,6 +1255,19 @@ func TestValidateSilence(t *testing.T) { }, err: "invalid label matcher", }, + { + s: &pb.Silence{ + Id: "some_id", + Matchers: []*pb.Matcher{ + {Name: "a", Pattern: "b"}, + {Name: "==some\r\n.label==ʤ", Pattern: "b"}, + }, + StartsAt: validTimestamp, + EndsAt: validTimestamp, + UpdatedAt: validTimestamp, + }, + err: "", + }, { s: &pb.Silence{ Id: "some_id", From 4c01b12ccea32af1d6dcec9921f56b866b73efac Mon Sep 17 00:00:00 2001 From: Yuri Tseretyan Date: Mon, 10 Apr 2023 14:24:13 -0400 Subject: [PATCH 04/12] lint silence_test.go Signed-off-by: Yuri Tseretyan --- silence/silence_test.go | 21 ++++++++++++++------- 1 file changed, 14 insertions(+), 7 deletions(-) diff --git a/silence/silence_test.go b/silence/silence_test.go index ecc491c015..0a327d1036 100644 --- a/silence/silence_test.go +++ b/silence/silence_test.go @@ -1126,21 +1126,24 @@ func TestValidateMatcher(t *testing.T) { Type: pb.Matcher_EQUAL, }, err: "", - }, { + }, + { m: &pb.Matcher{ Name: "a", Pattern: "b", Type: pb.Matcher_NOT_EQUAL, }, err: "", - }, { + }, + { m: &pb.Matcher{ Name: "a", Pattern: "b", Type: pb.Matcher_REGEXP, }, err: "", - }, { + }, + { m: &pb.Matcher{ Name: "a", Pattern: "b", @@ -1162,28 +1165,32 @@ func TestValidateMatcher(t *testing.T) { Pattern: "a", Type: pb.Matcher_EQUAL, }, - }, { + }, + { m: &pb.Matcher{ Name: "a", Pattern: "((", Type: pb.Matcher_REGEXP, }, err: "invalid regular expression", - }, { + }, + { m: &pb.Matcher{ Name: "a", Pattern: "))", Type: pb.Matcher_NOT_REGEXP, }, err: "invalid regular expression", - }, { + }, + { m: &pb.Matcher{ Name: "a", Pattern: "\xff", Type: pb.Matcher_EQUAL, }, err: "invalid label value", - }, { + }, + { m: &pb.Matcher{ Name: "a", Pattern: "b", From 7ae44f17c89da0f2b5891f8d0556b9e903922325 Mon Sep 17 00:00:00 2001 From: Yuri Tseretyan Date: Mon, 10 Apr 2023 13:03:32 -0400 Subject: [PATCH 05/12] update validation of Route.GroupBy to use labels.IsValidName Signed-off-by: Yuri Tseretyan --- config/config.go | 2 +- config/config_test.go | 17 +++++++++++++++-- 2 files changed, 16 insertions(+), 3 deletions(-) diff --git a/config/config.go b/config/config.go index d85dbe14ad..95c3df1f71 100644 --- a/config/config.go +++ b/config/config.go @@ -802,7 +802,7 @@ func (r *Route) UnmarshalYAML(unmarshal func(interface{}) error) error { r.GroupByAll = true } else { labelName := model.LabelName(l) - if !labelName.IsValid() { + if !labels.IsValidName(labelName) { return fmt.Errorf("invalid label name %q in group_by list", l) } r.GroupBy = append(r.GroupBy, labelName) diff --git a/config/config_test.go b/config/config_test.go index 7631d37cb7..f5742e4ae5 100644 --- a/config/config_test.go +++ b/config/config_test.go @@ -309,14 +309,14 @@ receivers: func TestGroupByInvalidLabel(t *testing.T) { in := ` route: - group_by: ['-invalid-'] + group_by: [' '] receiver: team-X-mails receivers: - name: 'team-X-mails' ` _, err := Load(in) - expected := "invalid label name \"-invalid-\" in group_by list" + expected := `invalid label name " " in group_by list` if err == nil { t.Fatalf("no error returned, expected:\n%q", expected) @@ -326,6 +326,19 @@ receivers: } } +func TestGroupByUtf8Label(t *testing.T) { + in := ` +route: + group_by: ['👍.any.label'] + receiver: team-X-mails +receivers: +- name: 'team-X-mails' +` + _, err := Load(in) + + require.NoError(t, err) +} + func TestRootRouteExists(t *testing.T) { in := ` receivers: From ecaf75e71a7e638b9a142333434aee52e8cfa690 Mon Sep 17 00:00:00 2001 From: Yuri Tseretyan Date: Mon, 10 Apr 2023 13:30:47 -0400 Subject: [PATCH 06/12] refactor ParseMatcher. extract unescapeMatcherString Signed-off-by: Yuri Tseretyan --- pkg/labels/parse.go | 39 +++++++++++++++++++++++++-------------- 1 file changed, 25 insertions(+), 14 deletions(-) diff --git a/pkg/labels/parse.go b/pkg/labels/parse.go index a125d59d8a..6f984bb9a9 100644 --- a/pkg/labels/parse.go +++ b/pkg/labels/parse.go @@ -14,6 +14,7 @@ package labels import ( + "fmt" "regexp" "strings" "unicode/utf8" @@ -122,21 +123,33 @@ func ParseMatcher(s string) (_ *Matcher, err error) { } var ( - rawValue = ms[3] - value strings.Builder - escaped bool - expectTrailingQuote bool + rawValue = ms[3] ) - if strings.HasPrefix(rawValue, "\"") { - rawValue = strings.TrimPrefix(rawValue, "\"") - expectTrailingQuote = true + if !utf8.ValidString(rawValue) { + return nil, errors.Errorf("matcher value not valid UTF-8: %s", rawValue) } - if !utf8.ValidString(rawValue) { - return nil, errors.Errorf("matcher value not valid UTF-8: %s", ms[3]) + value, err := unescapeMatcherString(rawValue) + if err != nil { + return nil, fmt.Errorf("matcher value %w", err) } + return NewMatcher(typeMap[ms[2]], ms[1], value) +} + +// unescapeMatcherString unescapes sequences that are escaped by openMetricsEscape: \n, \\, \", and removes leading and trailing double quotes. +func unescapeMatcherString(raw string) (string, error) { + var ( + escaped bool + value strings.Builder + rawValue = raw + expectTrailingQuote = false + ) + if strings.HasPrefix(rawValue, `"`) { + rawValue = rawValue[1:] + expectTrailingQuote = true + } // Unescape the rawValue: for i, r := range rawValue { if escaped { @@ -163,17 +176,15 @@ func ParseMatcher(s string) (_ *Matcher, err error) { value.WriteByte('\\') case '"': if !expectTrailingQuote || i < len(rawValue)-1 { - return nil, errors.Errorf("matcher value contains unescaped double quote: %s", ms[3]) + return "", errors.Errorf("contains unescaped double quote: %s", raw) } expectTrailingQuote = false default: value.WriteRune(r) } } - if expectTrailingQuote { - return nil, errors.Errorf("matcher value contains unescaped double quote: %s", ms[3]) + return "", errors.Errorf("contains unescaped double quote: %s", raw) } - - return NewMatcher(typeMap[ms[2]], ms[1], value.String()) + return value.String(), nil } From d19c81abe3de8a93aa719a2b5d140ad0342b82be Mon Sep 17 00:00:00 2001 From: Yuri Tseretyan Date: Mon, 10 Apr 2023 13:34:07 -0400 Subject: [PATCH 07/12] add validation of matcher type Signed-off-by: Yuri Tseretyan --- pkg/labels/parse.go | 33 +++++++++++++++++++++++++---- pkg/labels/parse_test.go | 45 ++++++++++++++++++++++++++++++++++++++++ 2 files changed, 74 insertions(+), 4 deletions(-) diff --git a/pkg/labels/parse.go b/pkg/labels/parse.go index 6f984bb9a9..9ab0ccb222 100644 --- a/pkg/labels/parse.go +++ b/pkg/labels/parse.go @@ -20,12 +20,14 @@ import ( "unicode/utf8" "github.com/pkg/errors" + "github.com/prometheus/common/model" ) var ( // '=~' has to come before '=' because otherwise only the '=' // will be consumed, and the '~' will be part of the 3rd token. - re = regexp.MustCompile(`^\s*([a-zA-Z_:][a-zA-Z0-9_:]*)\s*(=~|=|!=|!~)\s*((?s).*?)\s*$`) + // only prometheus-compatible labels are allowed to be without wrapping double quotes. + re = regexp.MustCompile(`^\s*("[^"]+"|[a-zA-Z_:][a-zA-Z0-9_:]*)\s*(=~|=|!=|!~)\s*((?s).*?)\s*$`) typeMap = map[string]MatchType{ "=": MatchEqual, "!=": MatchNotEqual, @@ -105,8 +107,9 @@ func ParseMatchers(s string) ([]*Matcher, error) { // UIs and config files. To support the interactive nature of the use cases, the // parser is in various aspects fairly tolerant. // -// The syntax of a matcher consists of three tokens: (1) A valid Prometheus -// label name. (2) One of '=', '!=', '=~', or '!~', with the same meaning as +// The syntax of a matcher consists of three tokens: (1) Either a UTF-8 string +// enclosed in double quotes or a valid Prometheus label name. +// (2) One of '=', '!=', '=~', or '!~', with the same meaning as // known from PromQL selectors. (3) A UTF-8 string, which may be enclosed in // double quotes. Before or after each token, there may be any amount of // whitespace, which will be discarded. The 3rd token may be the empty @@ -123,9 +126,18 @@ func ParseMatcher(s string) (_ *Matcher, err error) { } var ( + rawName = ms[1] rawValue = ms[3] ) + matcherType, ok := typeMap[ms[2]] + if !ok { + return nil, errors.Errorf("matcher type is unknown: %s", ms[2]) + } + + if !utf8.ValidString(rawName) { + return nil, errors.Errorf("matcher label not valid UTF-8: %s", rawName) + } if !utf8.ValidString(rawValue) { return nil, errors.Errorf("matcher value not valid UTF-8: %s", rawValue) } @@ -135,7 +147,20 @@ func ParseMatcher(s string) (_ *Matcher, err error) { return nil, fmt.Errorf("matcher value %w", err) } - return NewMatcher(typeMap[ms[2]], ms[1], value) + name := rawName + // if name is quoted, then it can contain any UTF-8 character. Unescape some escape sequences. + if strings.HasPrefix(rawName, `"`) { + name, err = unescapeMatcherString(rawName) + if err != nil { + return nil, fmt.Errorf("matcher label name %w", err) + } + } + + if !IsValidName(model.LabelName(name)) { + return nil, errors.Errorf("invalid matcher label name: \"%s\"", name) + } + + return NewMatcher(matcherType, name, value) } // unescapeMatcherString unescapes sequences that are escaped by openMetricsEscape: \n, \\, \", and removes leading and trailing double quotes. diff --git a/pkg/labels/parse_test.go b/pkg/labels/parse_test.go index e1203f4a1a..58d0446269 100644 --- a/pkg/labels/parse_test.go +++ b/pkg/labels/parse_test.go @@ -196,6 +196,51 @@ func TestMatchers(t *testing.T) { return []*Matcher{m} }(), }, + { + input: `{"foo"="bar", instance=~"some-api.*"}`, + want: func() []*Matcher { + ms := []*Matcher{} + m, _ := NewMatcher(MatchEqual, "foo", "bar") + m2, _ := NewMatcher(MatchRegexp, "instance", "some-api.*") + return append(ms, m, m2) + }(), + }, + { + input: `{"foo.bar"="bar"}`, + want: func() []*Matcher { + ms := []*Matcher{} + m, _ := NewMatcher(MatchEqual, "foo.bar", "bar") + return append(ms, m) + }(), + }, + { + input: `{"foo=bar~="="bar"}`, + want: func() []*Matcher { + ms := []*Matcher{} + m, _ := NewMatcher(MatchEqual, "foo=bar~=", "bar") + return append(ms, m) + }(), + }, + { + input: `{"foo\\=bar~=\n爱"="bar"}`, + want: func() []*Matcher { + ms := []*Matcher{} + m, _ := NewMatcher(MatchEqual, "foo\\=bar~=\n爱", "bar") + return append(ms, m) + }(), + }, + { + input: "{\"foo\t\r\n\\=bar~=\n爱\"=bar}", + want: func() []*Matcher { + ms := []*Matcher{} + m, _ := NewMatcher(MatchEqual, "foo\t\r\n\\=bar~=\n爱", "bar") + return append(ms, m) + }(), + }, + { + input: `{"\n "="bar"}`, + err: "invalid matcher label name: \"\n \"", + }, { input: `job="value`, err: `matcher value contains unescaped double quote: "value`, From 2bc49b5467afe7fb720fdf53941181199b8d74ec Mon Sep 17 00:00:00 2001 From: Yuriy Tseretyan Date: Mon, 17 Apr 2023 12:17:10 -0400 Subject: [PATCH 08/12] fix support for double quotes --- pkg/labels/parse.go | 2 +- pkg/labels/parse_test.go | 8 ++++++++ 2 files changed, 9 insertions(+), 1 deletion(-) diff --git a/pkg/labels/parse.go b/pkg/labels/parse.go index 9ab0ccb222..8d2026541e 100644 --- a/pkg/labels/parse.go +++ b/pkg/labels/parse.go @@ -27,7 +27,7 @@ var ( // '=~' has to come before '=' because otherwise only the '=' // will be consumed, and the '~' will be part of the 3rd token. // only prometheus-compatible labels are allowed to be without wrapping double quotes. - re = regexp.MustCompile(`^\s*("[^"]+"|[a-zA-Z_:][a-zA-Z0-9_:]*)\s*(=~|=|!=|!~)\s*((?s).*?)\s*$`) + re = regexp.MustCompile(`^\s*((?s)".+"|[a-zA-Z_:][a-zA-Z0-9_:]*)\s*(=~|=|!=|!~)\s*((?s).*?)\s*$`) typeMap = map[string]MatchType{ "=": MatchEqual, "!=": MatchNotEqual, diff --git a/pkg/labels/parse_test.go b/pkg/labels/parse_test.go index 58d0446269..25b97d1c6f 100644 --- a/pkg/labels/parse_test.go +++ b/pkg/labels/parse_test.go @@ -237,6 +237,14 @@ func TestMatchers(t *testing.T) { return append(ms, m) }(), }, + { + input: `{"\""="\""}`, + want: func() []*Matcher { + ms := []*Matcher{} + m, _ := NewMatcher(MatchEqual, `"`, `"`) + return append(ms, m) + }(), + }, { input: `{"\n "="bar"}`, err: "invalid matcher label name: \"\n \"", From ccc3af9ebcf65db1111dcc7de2f38a9b837d1207 Mon Sep 17 00:00:00 2001 From: Yuri Tseretyan Date: Mon, 10 Apr 2023 13:36:10 -0400 Subject: [PATCH 09/12] update Matcher.String to return utf-8 matcher label wrapped in double quotes Signed-off-by: Yuri Tseretyan --- pkg/labels/matcher.go | 3 +++ pkg/labels/matcher_test.go | 6 ++++++ 2 files changed, 9 insertions(+) diff --git a/pkg/labels/matcher.go b/pkg/labels/matcher.go index 445c905651..63429beee6 100644 --- a/pkg/labels/matcher.go +++ b/pkg/labels/matcher.go @@ -74,6 +74,9 @@ func NewMatcher(t MatchType, n, v string) (*Matcher, error) { } func (m *Matcher) String() string { + if !model.LabelName(m.Name).IsValid() { + return fmt.Sprintf(`"%s"%s"%s"`, openMetricsEscape(m.Name), m.Type, openMetricsEscape(m.Value)) + } return fmt.Sprintf(`%s%s"%s"`, m.Name, m.Type, openMetricsEscape(m.Value)) } diff --git a/pkg/labels/matcher_test.go b/pkg/labels/matcher_test.go index 21d1b7f089..ea95b48319 100644 --- a/pkg/labels/matcher_test.go +++ b/pkg/labels/matcher_test.go @@ -182,6 +182,12 @@ line`, value: `tab stop`, want: `foo="tab stop"`, }, + { + name: `utf-8.name\😉`, + op: MatchEqual, + value: `tab stop`, + want: `"utf-8.name\\😉"="tab stop"`, + }, } for _, test := range tests { From 51b5f25cf3e67461b4e143cdb7fd1a0185b7987f Mon Sep 17 00:00:00 2001 From: Yuriy Tseretyan Date: Wed, 12 Apr 2023 15:04:32 -0400 Subject: [PATCH 10/12] add acceptance tests for v2 API Signed-off-by: Yuri Tseretyan --- test/with_api_v2/acceptance/api_test.go | 67 +++++++++++++++++++++ test/with_api_v2/acceptance/silence_test.go | 55 +++++++++++++++++ 2 files changed, 122 insertions(+) diff --git a/test/with_api_v2/acceptance/api_test.go b/test/with_api_v2/acceptance/api_test.go index b2d5133d46..7036a26011 100644 --- a/test/with_api_v2/acceptance/api_test.go +++ b/test/with_api_v2/acceptance/api_test.go @@ -232,3 +232,70 @@ receivers: require.Equal(t, models.AlertStatusStateActive, *al.Status.State) } } + +func TestFilterAlertRequestWithUtf8Labels(t *testing.T) { + t.Parallel() + + conf := ` +route: + receiver: "default" + group_by: [] + group_wait: 1s + group_interval: 10m + repeat_interval: 1h + +receivers: +- name: "default" + webhook_configs: + - url: 'http://%s' +` + + at := a.NewAcceptanceTest(t, &a.AcceptanceOpts{ + Tolerance: 1 * time.Second, + }) + co := at.Collector("webhook") + wh := a.NewWebhook(t, co) + + amc := at.AlertmanagerCluster(fmt.Sprintf(conf, wh.Address()), 1) + require.NoError(t, amc.Start()) + defer amc.Terminate() + + am := amc.Members()[0] + + now := time.Now() + startsAt := strfmt.DateTime(now) + endsAt := strfmt.DateTime(now.Add(5 * time.Minute)) + + labels := models.LabelSet(map[string]string{"alertname": "test1", "🎶": "jazz"}) + pa1 := &models.PostableAlert{ + StartsAt: startsAt, + EndsAt: endsAt, + Alert: models.Alert{Labels: labels}, + } + labels = models.LabelSet(map[string]string{"system": "foo", " some\r\n\tspaces ": "utf-8"}) + pa2 := &models.PostableAlert{ + StartsAt: startsAt, + EndsAt: endsAt, + Alert: models.Alert{Labels: labels}, + } + alertParams := alert.NewPostAlertsParams() + alertParams.Alerts = models.PostableAlerts{pa1, pa2} + _, err := am.Client().Alert.PostAlerts(alertParams) + require.NoError(t, err) + + filter := []string{`"🎶"=jazz`} + resp, err := am.Client().Alert.GetAlerts(alert.NewGetAlertsParams().WithFilter(filter)) + require.NoError(t, err) + require.Equal(t, 1, len(resp.Payload)) + for _, al := range resp.Payload { + require.Equal(t, models.AlertStatusStateActive, *al.Status.State) + } + + filter = []string{"\" some\r\n\tspaces \"=utf-8"} + resp, err = am.Client().Alert.GetAlerts(alert.NewGetAlertsParams().WithFilter(filter)) + require.NoError(t, err) + require.Equal(t, 1, len(resp.Payload)) + for _, al := range resp.Payload { + require.Equal(t, models.AlertStatusStateActive, *al.Status.State) + } +} diff --git a/test/with_api_v2/acceptance/silence_test.go b/test/with_api_v2/acceptance/silence_test.go index 228df97733..79456d47e1 100644 --- a/test/with_api_v2/acceptance/silence_test.go +++ b/test/with_api_v2/acceptance/silence_test.go @@ -122,3 +122,58 @@ receivers: t.Log(co.Check()) } + +func TestSilencingUtf8Labels(t *testing.T) { + t.Parallel() + + conf := ` +route: + receiver: "default" + group_by: [] + group_wait: 1s + group_interval: 1s + repeat_interval: 1ms + +receivers: +- name: "default" + webhook_configs: + - url: 'http://%s' +` + + at := NewAcceptanceTest(t, &AcceptanceOpts{ + Tolerance: 150 * time.Millisecond, + }) + + co := at.Collector("webhook") + wh := NewWebhook(t, co) + + amc := at.AlertmanagerCluster(fmt.Sprintf(conf, wh.Address()), 1) + + // No repeat interval is configured. Thus, we receive an alert + // notification every second. + amc.Push(At(1), Alert("alertname", "test1", "label.with-\"-and-⚠️", "test").Active(1)) + amc.Push(At(1), Alert("alertname", "test2", " label\nwith spaces ", "test").Active(1)) + + co.Want(Between(2, 2.5), + Alert("alertname", "test1", "label.with-\"-and-⚠️", "test").Active(1), + Alert("alertname", "test2", " label\nwith spaces ", "test").Active(1), + ) + + // Add a silence that affects alerts + amc.SetSilence(At(2.3), Silence(2.5, 4.5).Match("label.with-\"-and-⚠️", "test")) + amc.SetSilence(At(2.3), Silence(2.5, 4.5).MatchRE(" label\nwith spaces ", ".+")) + + co.Want(Between(3, 3.5)) + co.Want(Between(4, 4.5)) + + // Silence should be over now and we receive both alerts again. + + co.Want(Between(5, 5.5), + Alert("alertname", "test1", "label.with-\"-and-⚠️", "test").Active(1), + Alert("alertname", "test2", " label\nwith spaces ", "test").Active(1), + ) + + at.Run() + + t.Log(co.Check()) +} From 48c7d4eed02285da1e231791256d22ce8c223497 Mon Sep 17 00:00:00 2001 From: Yuriy Tseretyan Date: Wed, 12 Apr 2023 15:03:59 -0400 Subject: [PATCH 11/12] update help in amtool to mention double quotes Signed-off-by: Yuri Tseretyan --- cli/alert_add.go | 4 ++++ cli/alert_query.go | 7 ++++++- cli/silence_add.go | 4 ++++ cli/silence_query.go | 4 ++++ 4 files changed, 18 insertions(+), 1 deletion(-) diff --git a/cli/alert_add.go b/cli/alert_add.go index e27d1522b3..80ea5e8cfb 100644 --- a/cli/alert_add.go +++ b/cli/alert_add.go @@ -46,6 +46,10 @@ be assumed to be the value of the alertname pair. amtool alert add foo node=bar +To specify a label name that contains any UTF-8 character, enclose it in double quotes: + + amtool alert add foo '"==label.name=="=bar' + One or more annotations can be added using the --annotation flag: amtool alert add foo node=bar \ diff --git a/cli/alert_query.go b/cli/alert_query.go index ff06b439b7..f4a0260a96 100644 --- a/cli/alert_query.go +++ b/cli/alert_query.go @@ -45,6 +45,10 @@ amtool alert query alertname=foo node=bar amtool alert query foo node=bar + If label name contains any UTF-8 symbols, enclose it in double quotes + +amtool alert query '"==label.name=="=bar' + If alertname is omitted and the first argument does not contain a '=' or a '=~' then it will be assumed to be the value of the alertname pair. @@ -52,7 +56,8 @@ amtool alert query 'alertname=~foo.*' As well as direct equality, regex matching is also supported. The '=~' syntax (similar to prometheus) is used to represent a regex match. Regex matching - can be used in combination with a direct match. + can be used in combination with a direct match. + Amtool supports several flags for filtering the returned alerts by state (inhibited, silenced, active, unprocessed). If none of these flags is given, diff --git a/cli/silence_add.go b/cli/silence_add.go index 3125c512f8..162febe8d6 100644 --- a/cli/silence_add.go +++ b/cli/silence_add.go @@ -68,6 +68,10 @@ const silenceAddHelp = `Add a new alertmanager silence As well as direct equality, regex matching is also supported. The '=~' syntax (similar to Prometheus) is used to represent a regex match. Regex matching can be used in combination with a direct match. + + amtool silence add '"path.to.label"=~foo + + To use any UTF-8 character in label name enclose it in double quotes. ` func configureSilenceAddCmd(cc *kingpin.CmdClause) { diff --git a/cli/silence_query.go b/cli/silence_query.go index 89a2722be0..684bc645b6 100644 --- a/cli/silence_query.go +++ b/cli/silence_query.go @@ -50,6 +50,10 @@ amtool silence query alertname=foo node=bar amtool silence query foo node=bar + If label name contains any UTF-8 symbols, enclose it in double quotes + +amtool silence query '"==label.name=="=bar' + If alertname is omitted and the first argument does not contain a '=' or a '=~' then it will be assumed to be the value of the alertname pair. From 024a9f08d932cffc358fed0913db618f452a2bcb Mon Sep 17 00:00:00 2001 From: Yuri Tseretyan Date: Wed, 12 Apr 2023 15:35:57 -0400 Subject: [PATCH 12/12] update docs Signed-off-by: Yuri Tseretyan --- docs/configuration.md | 24 ++++++++++++++---------- 1 file changed, 14 insertions(+), 10 deletions(-) diff --git a/docs/configuration.md b/docs/configuration.md index b5bc69f532..ba59b4fbd5 100644 --- a/docs/configuration.md +++ b/docs/configuration.md @@ -38,7 +38,8 @@ value is set to the specified default. Generic placeholders are defined as follows: * ``: a duration matching the regular expression `((([0-9]+)y)?(([0-9]+)w)?(([0-9]+)d)?(([0-9]+)h)?(([0-9]+)m)?(([0-9]+)s)?(([0-9]+)ms)?|0)`, e.g. `1d`, `1h30m`, `5m`, `10s` -* ``: a string matching the regular expression `[a-zA-Z_][a-zA-Z0-9_]*` +* ``: a string matching the regular expression `[a-zA-Z_][a-zA-Z0-9_]*` +* ``: a string of unicode characters * ``: a string of unicode characters * ``: a valid path in the current working directory * ``: a boolean that can take the values `true` or `false` @@ -171,12 +172,12 @@ current node. # DEPRECATED: Use matchers below. # A set of equality matchers an alert has to fulfill to match the node. match: - [ : , ... ] + [ : , ... ] # DEPRECATED: Use matchers below. # A set of regex-matchers an alert has to fulfill to match the node. match_re: - [ : , ... ] + [ : , ... ] # A list of matchers that an alert has to fulfill to match the node. matchers: @@ -382,10 +383,10 @@ to reason about and does not trigger this special case. # DEPRECATED: Use target_matchers below. # Matchers that have to be fulfilled in the alerts to be muted. target_match: - [ : , ... ] + [ : , ... ] # DEPRECATED: Use target_matchers below. target_match_re: - [ : , ... ] + [ : , ... ] # A list of matchers that have to be fulfilled by the target # alerts to be muted. @@ -396,10 +397,10 @@ target_matchers: # Matchers for which one or more alerts have to exist for the # inhibition to take effect. source_match: - [ : , ... ] + [ : , ... ] # DEPRECATED: Use source_matchers below. source_match_re: - [ : , ... ] + [ : , ... ] # A list of matchers for which one or more alerts have # to exist for the inhibition to take effect. @@ -420,13 +421,15 @@ Label matchers are used both in routes and inhibition rules to match certain ale A matcher is a string with a syntax inspired by PromQL and OpenMetrics. The syntax of a matcher consists of three tokens: -- A valid Prometheus label name. +- A UTF-8 string enclosed in double quotes that contains at least one non-whitespace character. If string is a valid Prometheus label name then double quotes can be omitted. - One of `=`, `!=`, `=~`, or `!~`. `=` means equals, `!=` means that the strings are not equal, `=~` is used for equality of regex expressions and `!~` is used for un-equality of regex expressions. They have the same meaning as known from PromQL selectors. -- A UTF-8 string, which may be enclosed in double quotes. Before or after each token, there may be any amount of whitespace. +- A UTF-8 string, which may be enclosed in double quotes. Can be an empty string. -The 3rd token may be the empty string. Within the 3rd token, OpenMetrics escaping rules apply: `\"` for a double-quote, `\n` for a line feed, `\\` for a literal backslash. Unescaped `"` must not occur inside the 3rd token (only as the 1st or last character). However, literal line feed characters are tolerated, as are single `\` characters not followed by `\`, `n`, or `"`. They act as a literal backslash in that case. +Before or after each token, there may be any amount of whitespace. + +Within the 1st and 3rd token, OpenMetrics escaping rules apply: `\"` for a double-quote, `\n` for a line feed, `\\` for a literal backslash. Unescaped `"` must not occur inside the token (only as the 1st or last character). However, literal line feed characters are tolerated, as are single `\` characters not followed by `\`, `n`, or `"`. They act as a literal backslash in that case. Matchers are ANDed together, meaning that all matchers must evaluate to "true" when tested against the labels on a given alert. For example, an alert with these labels: @@ -452,6 +455,7 @@ Here are some examples of valid string matchers: matchers: - foo = bar - dings !=bums + - "utf-8.string" = value ``` 2. Similar to example 1, shown below are two equality matchers combined in a short form YAML list.