Repository: prometheus/alertmanager
Pull request #4292 (2025-03-05 02:18:16)

Fix subtle bug in JSON/YAML encoding of inhibition rules
This commit fixes a subtle bug introduced in #4177 where external code that imports `config.InhibitRule` for the purpose of JSON/YAML encoding can encode an inhibition rule with missing equals labels. This bug will happen if someone updates their Go modules, writes labels to the `Equal` field, and does not also write the same labels to the `EqualStr` field.

Before #4177:

```go
b, err := json.Marshal(InhibitRule{
    Equals: model.LabelNames{"foo"},
})
```
```json
{
    "equals": ["foo"]
}
```

After #4177:

```go
b, err := json.Marshal(InhibitRule{
    Equals: model.LabelNames{"foo"},
})
```
```json
{}
```


To fix this, I propose removing `EqualStr` and changing `Equal` from `model.LabelNames` to `[]string`. This will cause an intentional breaking change, and require the maintainer of downstream code to use `[]string` instead of `model.LabelNames`. I consider this a better option to the subtle bug explained earlier.

The reason we change `model.LabelNames` to `[]string` is `prometheus/common` cannot support two modes at the same time (UTF-8 enabled and UTF-8 disabled). This is a blocking issue for projects like Mimir where UTF-8 development is at different stages in the Alertmanager and metrics codebases, and so Mimir depends on a global variable in `prometheus/common` due to a diamond dependency problem. This change avoids that dependency problem.

This still keeps the fix for #4177, but does so with an explicit breaking change to downstream users.

grobinson-grafana: @SuperQ as reviewer of #4177, would you also be able to review and merge this?

Patch:
diff --git a/config/config.go b/config/config.go
index b8e48413..6f54c523 100644
--- a/config/config.go
+++ b/config/config.go
@@ -968,11 +968,7 @@ 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:"-" 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"`
+	Equal []string `yaml:"equal,omitempty" json:"equal,omitempty"`
 }
 
 // UnmarshalYAML implements the yaml.Unmarshaler interface for InhibitRule.
@@ -994,12 +990,11 @@ func (r *InhibitRule) UnmarshalYAML(unmarshal func(interface{}) error) error {
 		}
 	}
 
-	for _, l := range r.EqualStr {
+	for _, l := range r.Equal {
 		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/inhibit/inhibit.go b/inhibit/inhibit.go
index eb1f7752..458367cb 100644
--- a/inhibit/inhibit.go
+++ b/inhibit/inhibit.go
@@ -214,7 +214,7 @@ func NewInhibitRule(cr config.InhibitRule) *InhibitRule {
 
 	equal := map[model.LabelName]struct{}{}
 	for _, ln := range cr.Equal {
-		equal[ln] = struct{}{}
+		equal[model.LabelName(ln)] = struct{}{}
 	}
 
 	return &InhibitRule{


Test patch:
diff --git a/config/config_test.go b/config/config_test.go
index 99984827..728d7670 100644
--- a/config/config_test.go
+++ b/config/config_test.go
@@ -1461,7 +1461,7 @@ func TestInhibitRuleEqual(t *testing.T) {
 	// The inhibition rule should have the expected equal labels.
 	require.Len(t, c.InhibitRules, 1)
 	r := c.InhibitRules[0]
-	require.Equal(t, model.LabelNames{"qux", "corge"}, r.Equal)
+	require.Equal(t, []string{"qux", "corge"}, r.Equal)
 
 	// Should not be able to load configuration with UTF-8 in equals list.
 	_, err = LoadFile("testdata/conf.inhibit-equal-utf8.yml")
@@ -1484,7 +1484,7 @@ func TestInhibitRuleEqual(t *testing.T) {
 	// The inhibition rule should have the expected equal labels.
 	require.Len(t, c.InhibitRules, 1)
 	r = c.InhibitRules[0]
-	require.Equal(t, model.LabelNames{"qux", "corge"}, r.Equal)
+	require.Equal(t, []string{"qux", "corge"}, r.Equal)
 
 	// Should also be able to load configuration with UTF-8 in equals list.
 	c, err = LoadFile("testdata/conf.inhibit-equal-utf8.yml")
@@ -1493,5 +1493,5 @@ func TestInhibitRuleEqual(t *testing.T) {
 	// The inhibition rule should have the expected equal labels.
 	require.Len(t, c.InhibitRules, 1)
 	r = c.InhibitRules[0]
-	require.Equal(t, model.LabelNames{"qux🙂", "corge"}, r.Equal)
+	require.Equal(t, []string{"qux🙂", "corge"}, r.Equal)
 }
diff --git a/inhibit/inhibit_test.go b/inhibit/inhibit_test.go
index 1f6f4ef2..59ff52b1 100644
--- a/inhibit/inhibit_test.go
+++ b/inhibit/inhibit_test.go
@@ -144,12 +144,12 @@ func TestInhibitRuleMatches(t *testing.T) {
 	rule1 := config.InhibitRule{
 		SourceMatch: map[string]string{"s1": "1"},
 		TargetMatch: map[string]string{"t1": "1"},
-		Equal:       model.LabelNames{"e"},
+		Equal:       []string{"e"},
 	}
 	rule2 := config.InhibitRule{
 		SourceMatch: map[string]string{"s2": "1"},
 		TargetMatch: map[string]string{"t2": "1"},
-		Equal:       model.LabelNames{"e"},
+		Equal:       []string{"e"},
 	}
 
 	m := types.NewMarker(prometheus.NewRegistry())
@@ -240,12 +240,12 @@ func TestInhibitRuleMatchers(t *testing.T) {
 	rule1 := config.InhibitRule{
 		SourceMatchers: config.Matchers{&labels.Matcher{Type: labels.MatchEqual, Name: "s1", Value: "1"}},
 		TargetMatchers: config.Matchers{&labels.Matcher{Type: labels.MatchNotEqual, Name: "t1", Value: "1"}},
-		Equal:          model.LabelNames{"e"},
+		Equal:          []string{"e"},
 	}
 	rule2 := config.InhibitRule{
 		SourceMatchers: config.Matchers{&labels.Matcher{Type: labels.MatchEqual, Name: "s2", Value: "1"}},
 		TargetMatchers: config.Matchers{&labels.Matcher{Type: labels.MatchEqual, Name: "t2", Value: "1"}},
-		Equal:          model.LabelNames{"e"},
+		Equal:          []string{"e"},
 	}
 
 	m := types.NewMarker(prometheus.NewRegistry())
@@ -374,7 +374,7 @@ func TestInhibit(t *testing.T) {
 		return config.InhibitRule{
 			SourceMatch: map[string]string{"s": "1"},
 			TargetMatch: map[string]string{"t": "1"},
-			Equal:       model.LabelNames{"e"},
+			Equal:       []string{"e"},
 		}
 	}
 	// alertOne is muted by alertTwo when it is active.