Skip to content

Commit

Permalink
Fix UTF-8 not allowed in Equal field for inhibition rules
Browse files Browse the repository at this point in the history
This commit fixes a bug where UTF-8 characters are not allowed
in the Equal field for inhibition rules, even when UTF-8 strict
mode is enabled.

This bug occurred because we forgot to override the validation
in model.LabelName. I have copied the same logic used for
GroupBy with GroupByStr, adding EqualByStr.

We would like to upgrade prometheus/common in future and use
the validation there instead, but it presents challenges with
downstream projects like Mimir and Cortex where, at present,
UTF-8 can be enabled and disabled in separate components at the
same time, which is not supported in prometheus/common.

Signed-off-by: George Robinson <[email protected]>
  • Loading branch information
grobinson-grafana committed Dec 20, 2024
1 parent 3b61ae8 commit d50feee
Show file tree
Hide file tree
Showing 4 changed files with 73 additions and 1 deletion.
14 changes: 13 additions & 1 deletion config/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -942,7 +942,11 @@ type InhibitRule struct {
TargetMatchers Matchers `yaml:"target_matchers,omitempty" json:"target_matchers,omitempty"`
// A set of labels that must be equal between the source and target alert
// for them to be a match.
Equal model.LabelNames `yaml:"equal,omitempty" json:"equal,omitempty"`
Equal model.LabelNames `yaml:"-" json:"-"`
// EqualStr allows us to validate the label depending on whether UTF-8 is
// enabled or disabled. It should be removed when Alertmanager is updated
// to use the switchable validation in recent versions of prometheus/common.
EqualStr []string `yaml:"equal,omitempty" json:"equal,omitempty"`
}

// UnmarshalYAML implements the yaml.Unmarshaler interface for InhibitRule.
Expand All @@ -964,6 +968,14 @@ func (r *InhibitRule) UnmarshalYAML(unmarshal func(interface{}) error) error {
}
}

for _, l := range r.EqualStr {
labelName := model.LabelName(l)
if !compat.IsValidLabelName(labelName) {
return fmt.Errorf("invalid label name %q in equal list", l)
}
r.Equal = append(r.Equal, labelName)
}

return nil
}

Expand Down
40 changes: 40 additions & 0 deletions config/config_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -25,8 +25,12 @@ import (

commoncfg "github.com/prometheus/common/config"
"github.com/prometheus/common/model"
"github.com/prometheus/common/promslog"
"github.com/stretchr/testify/require"
"gopkg.in/yaml.v2"

"github.com/prometheus/alertmanager/featurecontrol"
"github.com/prometheus/alertmanager/matcher/compat"
)

func TestLoadEmptyString(t *testing.T) {
Expand Down Expand Up @@ -1387,3 +1391,39 @@ func TestNilRegexp(t *testing.T) {
})
}
}

func TestInhibitRuleEqual(t *testing.T) {
_, err := LoadFile("testdata/conf.inhibit-equal.yml")
if err != nil {
t.Errorf("Error parsing %s: %s", "testdata/conf.inhibit-equal.yml", err)
}
// Should not be able to load configuration with UTF-8 in equals
_, err = LoadFile("testdata/conf.inhibit-equal-utf8.yml")
if err == nil {
t.Errorf("Expected error parsing %s, got nil", "testdata/conf.inhibit-equal-utf8.yml")
}
expected := "invalid label name \"qux🙂\" in equal list"
if err.Error() != expected {
t.Errorf("Expected error \n%s, got \n%s", expected, err)
}

// Change the mode to UTF-8 mode.
ff, err := featurecontrol.NewFlags(promslog.NewNopLogger(), featurecontrol.FeatureUTF8StrictMode)
require.NoError(t, err)
compat.InitFromFlags(promslog.NewNopLogger(), ff)

// Restore the mode to classic at the end of the test.
ff, err = featurecontrol.NewFlags(promslog.NewNopLogger(), featurecontrol.FeatureClassicMode)
require.NoError(t, err)
defer compat.InitFromFlags(promslog.NewNopLogger(), ff)

_, err = LoadFile("testdata/conf.inhibit-equal.yml")
if err != nil {
t.Errorf("Error parsing %s: %s", "testdata/conf.inhibit-equal.yml", err)
}
// Should also be able to load configuration with UTF-8 in equals
_, err = LoadFile("testdata/conf.inhibit-equal-utf8.yml")
if err != nil {
t.Errorf("Error parsing %s: %s", "testdata/conf.inhibit-equal-utf8.yml", err)
}
}
10 changes: 10 additions & 0 deletions config/testdata/conf.inhibit-equal-utf8.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
route:
receiver: test
receivers:
- name: test
inhibit_rules:
- source_matchers:
- foo=bar

Check failure on line 7 in config/testdata/conf.inhibit-equal-utf8.yml

View workflow job for this annotation

GitHub Actions / Test

7:5 [indentation] wrong indentation: expected 6 but found 4
target_matchers:
- bar=baz

Check failure on line 9 in config/testdata/conf.inhibit-equal-utf8.yml

View workflow job for this annotation

GitHub Actions / Test

9:5 [indentation] wrong indentation: expected 6 but found 4
equal: ['qux🙂']

Check failure on line 10 in config/testdata/conf.inhibit-equal-utf8.yml

View workflow job for this annotation

GitHub Actions / Test

10:20 [new-line-at-end-of-file] no new line character at the end of file
10 changes: 10 additions & 0 deletions config/testdata/conf.inhibit-equal.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
route:
receiver: test
receivers:
- name: test
inhibit_rules:
- source_matchers:
- foo=bar

Check failure on line 7 in config/testdata/conf.inhibit-equal.yml

View workflow job for this annotation

GitHub Actions / Test

7:5 [indentation] wrong indentation: expected 6 but found 4
target_matchers:
- bar=baz

Check failure on line 9 in config/testdata/conf.inhibit-equal.yml

View workflow job for this annotation

GitHub Actions / Test

9:5 [indentation] wrong indentation: expected 6 but found 4
equal: ['qux']

Check failure on line 10 in config/testdata/conf.inhibit-equal.yml

View workflow job for this annotation

GitHub Actions / Test

10:19 [new-line-at-end-of-file] no new line character at the end of file

0 comments on commit d50feee

Please sign in to comment.