From ed0e83201fa1f81824cd93763f75d2c560d10488 Mon Sep 17 00:00:00 2001 From: George Robinson Date: Fri, 20 Dec 2024 19:43:08 +0000 Subject: [PATCH] Fix UTF-8 not allowed in Equal field for inhibition rules 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 EqualStr. 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 --- config/config.go | 14 ++++++- config/config_test.go | 46 +++++++++++++++++++++ config/testdata/conf.inhibit-equal-utf8.yml | 10 +++++ config/testdata/conf.inhibit-equal.yml | 10 +++++ 4 files changed, 79 insertions(+), 1 deletion(-) create mode 100644 config/testdata/conf.inhibit-equal-utf8.yml create mode 100644 config/testdata/conf.inhibit-equal.yml diff --git a/config/config.go b/config/config.go index 890592ca90..7fe312278a 100644 --- a/config/config.go +++ b/config/config.go @@ -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 validation modes in recent versions of prometheus/common. + EqualStr []string `yaml:"equal,omitempty" json:"equal,omitempty"` } // UnmarshalYAML implements the yaml.Unmarshaler interface for InhibitRule. @@ -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 } diff --git a/config/config_test.go b/config/config_test.go index 15edfeed2d..b04cf976f3 100644 --- a/config/config_test.go +++ b/config/config_test.go @@ -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) { @@ -1387,3 +1391,45 @@ func TestNilRegexp(t *testing.T) { }) } } + +func TestInhibitRuleEqual(t *testing.T) { + c, err := LoadFile("testdata/conf.inhibit-equal.yml") + require.NoError(t, err) + + // Check that Equal has the expected labels. + require.Len(t, c.InhibitRules, 1) + r := c.InhibitRules[0] + require.Equal(t, model.LabelNames{"qux", "corge"}, r.Equal) + + // Should not be able to load configuration with UTF-8 in equals + _, err = LoadFile("testdata/conf.inhibit-equal-utf8.yml") + require.Error(t, err) + require.Equal(t, "invalid label name \"qux🙂\" in equal list", err.Error()) + + // 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) + + c, err = LoadFile("testdata/conf.inhibit-equal.yml") + require.NoError(t, err) + + // Check that Equal has the expected labels. + require.Len(t, c.InhibitRules, 1) + r = c.InhibitRules[0] + require.Equal(t, model.LabelNames{"qux", "corge"}, r.Equal) + + // Should also be able to load configuration with UTF-8 in equals + c, err = LoadFile("testdata/conf.inhibit-equal-utf8.yml") + require.NoError(t, err) + + // Check that Equal has the expected labels. + require.Len(t, c.InhibitRules, 1) + r = c.InhibitRules[0] + require.Equal(t, model.LabelNames{"qux🙂", "corge"}, r.Equal) +} diff --git a/config/testdata/conf.inhibit-equal-utf8.yml b/config/testdata/conf.inhibit-equal-utf8.yml new file mode 100644 index 0000000000..428d5b7eea --- /dev/null +++ b/config/testdata/conf.inhibit-equal-utf8.yml @@ -0,0 +1,10 @@ +route: + receiver: test +receivers: + - name: test +inhibit_rules: + - source_matchers: + - foo=bar + target_matchers: + - bar=baz + equal: ['qux🙂', 'corge'] diff --git a/config/testdata/conf.inhibit-equal.yml b/config/testdata/conf.inhibit-equal.yml new file mode 100644 index 0000000000..030268fa58 --- /dev/null +++ b/config/testdata/conf.inhibit-equal.yml @@ -0,0 +1,10 @@ +route: + receiver: test +receivers: + - name: test +inhibit_rules: + - source_matchers: + - foo=bar + target_matchers: + - bar=baz + equal: ['qux', 'corge']