diff --git a/model/relabel/relabel_test.go b/model/relabel/relabel_test.go index fbf8f05e07..c5064540f7 100644 --- a/model/relabel/relabel_test.go +++ b/model/relabel/relabel_test.go @@ -815,10 +815,8 @@ func TestTargetLabelValidity(t *testing.T) { require.Equal(t, test.valid, relabelTargetUTF8.Match([]byte(test.str)), "Expected %q to be %v", test.str, test.valid) }) - } }) - } func BenchmarkRelabel(b *testing.B) { diff --git a/model/rulefmt/rulefmt.go b/model/rulefmt/rulefmt.go index 76b12f0e5c..4769e6d5ba 100644 --- a/model/rulefmt/rulefmt.go +++ b/model/rulefmt/rulefmt.go @@ -233,6 +233,14 @@ func (r *RuleNode) Validate() (nodes []WrappedError) { node: &r.Record, }) } + // While record is a valid UTF-8 it's common mistake to put PromQL expression in the record name. + // Disallow "{}" chars. + if strings.Contains(r.Record.Value, "{") || strings.Contains(r.Record.Value, "}") { + nodes = append(nodes, WrappedError{ + err: fmt.Errorf("potential issue in the recording rule name; should it be in expr?: %s", r.Record.Value), + node: &r.Record, + }) + } } for k, v := range r.Labels { diff --git a/model/rulefmt/rulefmt_test.go b/model/rulefmt/rulefmt_test.go index 286e760a41..f24b02f7b3 100644 --- a/model/rulefmt/rulefmt_test.go +++ b/model/rulefmt/rulefmt_test.go @@ -26,10 +26,15 @@ import ( func TestParseFileSuccess(t *testing.T) { _, errs := ParseFile("testdata/test.yaml", false) require.Empty(t, errs, "unexpected errors parsing file") + + _, errs = ParseFile("testdata/utf-8_lname.good.yaml", false) + require.Empty(t, errs, "unexpected errors parsing file") + _, errs = ParseFile("testdata/utf-8_annotation.good.yaml", false) + require.Empty(t, errs, "unexpected errors parsing file") } func TestParseFileFailure(t *testing.T) { - table := []struct { + for _, c := range []struct { filename string errMsg string }{ @@ -53,17 +58,9 @@ func TestParseFileFailure(t *testing.T) { filename: "noexpr.bad.yaml", errMsg: "field 'expr' must be set in rule", }, - { - filename: "bad_lname.bad.yaml", - errMsg: "invalid label name", - }, - { - filename: "bad_annotation.bad.yaml", - errMsg: "invalid annotation name", - }, { filename: "invalid_record_name.bad.yaml", - errMsg: "invalid recording rule name", + errMsg: "potential issue in the recording rule name; should it be in expr?: strawberry{flavor=\"sweet\"}", }, { filename: "bad_field.bad.yaml", @@ -81,12 +78,12 @@ func TestParseFileFailure(t *testing.T) { filename: "record_and_keep_firing_for.bad.yaml", errMsg: "invalid field 'keep_firing_for' in recording rule", }, - } - - for _, c := range table { - _, errs := ParseFile(filepath.Join("testdata", c.filename), false) - require.NotEmpty(t, errs, "Expected error parsing %s but got none", c.filename) - require.ErrorContainsf(t, errs[0], c.errMsg, "Expected error for %s.", c.filename) + } { + t.Run(c.filename, func(t *testing.T) { + _, errs := ParseFile(filepath.Join("testdata", c.filename), false) + require.NotEmpty(t, errs, "Expected error parsing %s but got none", c.filename) + require.ErrorContainsf(t, errs[0], c.errMsg, "Expected error for %s.", c.filename) + }) } } diff --git a/model/rulefmt/testdata/bad_annotation.bad.yaml b/model/rulefmt/testdata/utf-8_annotation.good.yaml similarity index 100% rename from model/rulefmt/testdata/bad_annotation.bad.yaml rename to model/rulefmt/testdata/utf-8_annotation.good.yaml diff --git a/model/rulefmt/testdata/bad_lname.bad.yaml b/model/rulefmt/testdata/utf-8_lname.good.yaml similarity index 100% rename from model/rulefmt/testdata/bad_lname.bad.yaml rename to model/rulefmt/testdata/utf-8_lname.good.yaml