Skip to content

Fix UTF-8 not supported in group_by #3619

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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