Skip to content

Commit

Permalink
Fix UTF-8 not supported in group_by (#3619)
Browse files Browse the repository at this point in the history
* 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 <george.robinson@grafana.com>
---------

Signed-off-by: George Robinson <george.robinson@grafana.com>
  • Loading branch information
grobinson-grafana authored Nov 24, 2023
1 parent 70bd5da commit 4494abf
Show file tree
Hide file tree
Showing 4 changed files with 93 additions and 3 deletions.
2 changes: 1 addition & 1 deletion config/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
29 changes: 27 additions & 2 deletions matchers/compat/parse.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,20 +16,28 @@ 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"
"github.com/prometheus/alertmanager/pkg/labels"
)

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)
Expand All @@ -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)
}
Expand Down Expand Up @@ -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))
}
}
63 changes: 63 additions & 0 deletions matchers/compat/parse_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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))
})
}
}
2 changes: 2 additions & 0 deletions test/with_api_v2/acceptance/utf8_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -280,6 +280,8 @@ route:
- receiver: webhook
matchers:
- foo🙂=bar
group_by:
- foo🙂
group_wait: 1s
receivers:
- name: default
Expand Down

0 comments on commit 4494abf

Please sign in to comment.