From 4494abfce419d1bbd3cb1a2c0b6584da88ac9b64 Mon Sep 17 00:00:00 2001 From: George Robinson Date: Fri, 24 Nov 2023 11:26:39 +0000 Subject: [PATCH] Fix UTF-8 not supported in group_by (#3619) * Fix UTF-8 not supported in group_by This commit fixes missing UTF-8 support in the group_by for routes. Signed-off-by: George Robinson --------- Signed-off-by: George Robinson --- config/config.go | 2 +- matchers/compat/parse.go | 29 ++++++++++- matchers/compat/parse_test.go | 63 ++++++++++++++++++++++++ test/with_api_v2/acceptance/utf8_test.go | 2 + 4 files changed, 93 insertions(+), 3 deletions(-) diff --git a/config/config.go b/config/config.go index d0dacd1a..5714f2d5 100644 --- a/config/config.go +++ b/config/config.go @@ -814,7 +814,7 @@ func (r *Route) UnmarshalYAML(unmarshal func(interface{}) error) error { r.GroupByAll = true } else { labelName := model.LabelName(l) - if !labelName.IsValid() { + if !compat.IsValidLabelName(labelName) { return fmt.Errorf("invalid label name %q in group_by list", l) } r.GroupBy = append(r.GroupBy, labelName) diff --git a/matchers/compat/parse.go b/matchers/compat/parse.go index 31cbe626..89b4204d 100644 --- a/matchers/compat/parse.go +++ b/matchers/compat/parse.go @@ -16,9 +16,11 @@ package compat import ( "fmt" "strings" + "unicode/utf8" "github.com/go-kit/log" "github.com/go-kit/log/level" + "github.com/prometheus/common/model" "github.com/prometheus/alertmanager/featurecontrol" "github.com/prometheus/alertmanager/matchers/parse" @@ -26,10 +28,16 @@ import ( ) var ( - parseMatcher = classicMatcherParser(log.NewNopLogger()) - parseMatchers = classicMatchersParser(log.NewNopLogger()) + isValidLabelName = isValidClassicLabelName(log.NewNopLogger()) + parseMatcher = classicMatcherParser(log.NewNopLogger()) + parseMatchers = classicMatchersParser(log.NewNopLogger()) ) +// IsValidLabelName returns true if the string is a valid label name. +func IsValidLabelName(name model.LabelName) bool { + return isValidLabelName(name) +} + type matcherParser func(s string) (*labels.Matcher, error) type matchersParser func(s string) (labels.Matchers, error) @@ -49,12 +57,15 @@ func Matchers(s string) (labels.Matchers, error) { // InitFromFlags initializes the compat package from the flagger. func InitFromFlags(l log.Logger, f featurecontrol.Flagger) { if f.ClassicMode() { + isValidLabelName = isValidClassicLabelName(l) parseMatcher = classicMatcherParser(l) parseMatchers = classicMatchersParser(l) } else if f.UTF8StrictMode() { + isValidLabelName = isValidUTF8LabelName(l) parseMatcher = utf8MatcherParser(l) parseMatchers = utf8MatchersParser(l) } else { + isValidLabelName = isValidUTF8LabelName(l) parseMatcher = fallbackMatcherParser(l) parseMatchers = fallbackMatchersParser(l) } @@ -166,3 +177,17 @@ func fallbackMatchersParser(l log.Logger) matchersParser { return m, nil } } + +// isValidClassicLabelName returns true if the string is a valid classic label name. +func isValidClassicLabelName(_ log.Logger) func(model.LabelName) bool { + return func(name model.LabelName) bool { + return name.IsValid() + } +} + +// isValidUTF8LabelName returns true if the string is a valid UTF-8 label name. +func isValidUTF8LabelName(_ log.Logger) func(model.LabelName) bool { + return func(name model.LabelName) bool { + return utf8.ValidString(string(name)) + } +} diff --git a/matchers/compat/parse_test.go b/matchers/compat/parse_test.go index d31e275b..ca50c14b 100644 --- a/matchers/compat/parse_test.go +++ b/matchers/compat/parse_test.go @@ -17,6 +17,7 @@ import ( "testing" "github.com/go-kit/log" + "github.com/prometheus/common/model" "github.com/stretchr/testify/require" "github.com/prometheus/alertmanager/pkg/labels" @@ -110,3 +111,65 @@ func mustNewMatcher(t *testing.T, op labels.MatchType, name, value string) *labe require.NoError(t, err) return m } + +func TestIsValidClassicLabelName(t *testing.T) { + tests := []struct { + name string + input model.LabelName + expected bool + }{{ + name: "is accepted", + input: "foo", + expected: true, + }, { + name: "is also accepted", + input: "_foo1", + expected: true, + }, { + name: "is not accepted", + input: "0foo", + expected: false, + }, { + name: "is also not accepted", + input: "foo🙂", + expected: false, + }} + + for _, test := range tests { + fn := isValidClassicLabelName(log.NewNopLogger()) + t.Run(test.name, func(t *testing.T) { + require.Equal(t, test.expected, fn(test.input)) + }) + } +} + +func TestIsValidUTF8LabelName(t *testing.T) { + tests := []struct { + name string + input model.LabelName + expected bool + }{{ + name: "is accepted", + input: "foo", + expected: true, + }, { + name: "is also accepted", + input: "_foo1", + expected: true, + }, { + name: "is accepted in UTF-8", + input: "0foo", + expected: true, + }, { + name: "is also accepted with UTF-8", + input: "foo🙂", + expected: true, + }} + + for _, test := range tests { + fn := isValidUTF8LabelName(log.NewNopLogger()) + t.Run(test.name, func(t *testing.T) { + require.Equal(t, test.expected, fn(test.input)) + }) + } +} diff --git a/test/with_api_v2/acceptance/utf8_test.go b/test/with_api_v2/acceptance/utf8_test.go index 75d4ecdb..79201847 100644 --- a/test/with_api_v2/acceptance/utf8_test.go +++ b/test/with_api_v2/acceptance/utf8_test.go @@ -280,6 +280,8 @@ route: - receiver: webhook matchers: - foo🙂=bar + group_by: + - foo🙂 group_wait: 1s receivers: - name: default