Skip to content

Commit

Permalink
Update Alertmanager to f69a508 (#7332)
Browse files Browse the repository at this point in the history 
* Update Alertmanager to f69a508

This commit updates Alertmanager to f69a508. The compat package
metrics are removed, and the tests are updated to check that
logs are emitted instead.

* Fix lint

* Fix lint again

* Use strings.Trim instead of subslice

* Remove integration test TestAlertmanagerMatchersMetrics

* Fix lint

* Prevent configurations with WebhookURLFile

This commit updates the validation rules to prevent Alertmanager
configurations for Discord and Microsoft Teams from using
WebhookURLFile.

* Add additional checks and tests for HTTPConfig

* Fix missing nil check

* Check for embedded HTTPConfig is not needed
  • Loading branch information
@grobinson-grafana
grobinson-grafana authored Feb 8, 2024
1 parent 7e7b220 commit 6ca9b40
Show file tree
Hide file tree
Showing 104 changed files with 5,947 additions and 2,324 deletions.
16 changes: 8 additions & 8 deletions go.mod
View file
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ require (
github.com/opentracing-contrib/go-stdlib v1.0.0
github.com/opentracing/opentracing-go v1.2.1-0.20220228012449-10b1cf09e00b
github.com/pkg/errors v0.9.1
github.com/prometheus/alertmanager v0.26.1-0.20240130102200-cab8ecbc95c4
github.com/prometheus/alertmanager v0.26.1-0.20240208095903-f69a5086657b
github.com/prometheus/client_golang v1.18.0
github.com/prometheus/client_model v0.5.0
github.com/prometheus/common v0.46.0
Expand Down Expand Up @@ -156,14 +156,14 @@ require (
github.com/go-logfmt/logfmt v0.6.0 // indirect
github.com/go-logr/logr v1.4.1 // indirect
github.com/go-logr/stdr v1.2.2 // indirect
github.com/go-openapi/analysis v0.21.4 // indirect
github.com/go-openapi/analysis v0.22.2 // indirect
github.com/go-openapi/errors v0.21.0 // indirect
github.com/go-openapi/jsonpointer v0.20.0 // indirect
github.com/go-openapi/jsonreference v0.20.2 // indirect
github.com/go-openapi/loads v0.21.2 // indirect
github.com/go-openapi/runtime v0.26.0 // indirect
github.com/go-openapi/spec v0.20.9 // indirect
github.com/go-openapi/validate v0.22.1 // indirect
github.com/go-openapi/jsonpointer v0.20.2 // indirect
github.com/go-openapi/jsonreference v0.20.4 // indirect
github.com/go-openapi/loads v0.21.5 // indirect
github.com/go-openapi/runtime v0.27.1 // indirect
github.com/go-openapi/spec v0.20.14 // indirect
github.com/go-openapi/validate v0.23.0 // indirect
github.com/go-redis/redis/v8 v8.11.5 // indirect
github.com/gofrs/uuid v4.4.0+incompatible // indirect
github.com/gogo/googleapis v1.4.1 // indirect
Expand Down
121 changes: 18 additions & 103 deletions go.sum
View file

Large diffs are not rendered by default.

82 changes: 0 additions & 82 deletions integration/alertmanager_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -283,88 +283,6 @@ func TestAlertmanagerUTF8StrictMode(t *testing.T) {
}))
}

// This test asserts that the correct metrics are incremented when configurations are uploaded,
// including configurations with disagreement, incompatible and invalid matchers. It can be deleted
// when the -alertmanager.utf8-strict-mode-enabled flag is removed.
func TestAlertmanagerMatchersMetrics(t *testing.T) {
s, err := e2e.NewScenario(networkName)
require.NoError(t, err)
defer s.Close()

consul := e2edb.NewConsul()
minio := e2edb.NewMinio(9000, alertsBucketName)
require.NoError(t, s.StartAndWaitReady(consul, minio))

// Upload the default configuration for two users.
require.NoError(t, uploadAlertmanagerConfig(minio, alertsBucketName, "user-1", mimirAlertmanagerUserConfigYaml))
require.NoError(t, uploadAlertmanagerConfig(minio, alertsBucketName, "user-2", mimirAlertmanagerUserConfigYaml))

alertmanager := e2emimir.NewAlertmanager(
"alertmanager",
mergeFlags(
AlertmanagerFlags(),
AlertmanagerS3Flags(),
AlertmanagerShardingFlags(consul.NetworkHTTPEndpoint(), 1),
),
)
require.NoError(t, s.StartAndWaitReady(alertmanager))
require.NoError(t, alertmanager.WaitSumMetrics(e2e.Equals(2), "cortex_alertmanager_config_last_reload_successful"))
require.NoError(t, alertmanager.WaitSumMetrics(e2e.Greater(0), "cortex_alertmanager_config_hash"))

c1, err := e2emimir.NewClient("", "", alertmanager.HTTPEndpoint(), "", "user-1")
require.NoError(t, err)
c2, err := e2emimir.NewClient("", "", alertmanager.HTTPEndpoint(), "", "user-2")
require.NoError(t, err)

// The metrics should all be zero as no configurations contain matchers.
metricNames := []string{
"alertmanager_matchers_parse_total",
"alertmanager_matchers_disagree_total",
"alertmanager_matchers_incompatible_total",
"alertmanager_matchers_invalid_total",
}
metrics, err := alertmanager.SumMetrics(metricNames, e2e.SkipMissingMetrics)
require.NoError(t, err)
require.Equal(t, []float64{0, 0, 0, 0}, metrics)

ctx, cancelFunc := context.WithTimeout(context.Background(), 15*time.Second)
defer cancelFunc()

// Upload a configuration for user1.
require.NoError(t, c1.SetAlertmanagerConfig(ctx, mimirAlertmanagerUserClassicConfigYaml, nil))
metrics, err = alertmanager.SumMetrics(metricNames, e2e.SkipMissingMetrics)
require.NoError(t, err)
// The sum for alertmanager_matchers_parse_total should be 4 as there are two matchers for origin=api
// and another two matchers for origin=config.
require.Equal(t, []float64{4, 0, 0, 0}, metrics)

// Upload a configuration for user2.
require.NoError(t, c2.SetAlertmanagerConfig(ctx, mimirAlertmanagerUserClassicConfigYaml, nil))
metrics, err = alertmanager.SumMetrics(metricNames, e2e.SkipMissingMetrics)
require.NoError(t, err)
// The sum for alertmanager_matchers_parse_total should be 8 as there are two matchers for origin=api
// and another two matchers for origin=config, and 4 from the previous sum.
require.Equal(t, []float64{8, 0, 0, 0}, metrics)

// Upload a configuration with disagreement.
require.NoError(t, c2.SetAlertmanagerConfig(ctx, mimirAlertmanagerDisagreementConfigYaml, nil))
metrics, err = alertmanager.SumMetrics(metricNames, e2e.SkipMissingMetrics)
require.NoError(t, err)
require.Equal(t, []float64{10, 1, 0, 0}, metrics)

// Upload a configuration with incompatible matchers.
require.NoError(t, c2.SetAlertmanagerConfig(ctx, mimirAlertmanagerIncompatibleConfigYaml, nil))
metrics, err = alertmanager.SumMetrics(metricNames, e2e.SkipMissingMetrics)
require.NoError(t, err)
require.Equal(t, []float64{12, 1, 1, 0}, metrics)

// Upload a configuration with invalid matchers.
require.EqualError(t, c2.SetAlertmanagerConfig(ctx, mimirAlertmanagerInvalidConfigYaml, nil), "setting config failed with status 400 and error error validating Alertmanager config: bad matcher format: \n")
metrics, err = alertmanager.SumMetrics(metricNames, e2e.SkipMissingMetrics)
require.NoError(t, err)
require.Equal(t, []float64{14, 1, 1, 2}, metrics)
}

func TestAlertmanagerV1Deprecated(t *testing.T) {
s, err := e2e.NewScenario(networkName)
require.NoError(t, err)
Expand Down
30 changes: 0 additions & 30 deletions integration/configs.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -77,36 +77,6 @@ receivers:
- name: test
`

mimirAlertmanagerDisagreementConfigYaml = `route:
receiver: test
group_by: [foo]
routes:
- matchers:
- foo="\xf0\x9f\x99\x82"
receivers:
- name: test
`

mimirAlertmanagerIncompatibleConfigYaml = `route:
receiver: test
group_by: [foo]
routes:
- matchers:
- foo=
receivers:
- name: test
`

mimirAlertmanagerInvalidConfigYaml = `route:
receiver: test
group_by: [foo]
routes:
- matchers:
- foo=,,
receivers:
- name: test
`

mimirRulerUserConfigYaml = `groups:
- name: rule
interval: 100s
Expand Down
4 changes: 2 additions & 2 deletions pkg/alertmanager/alertmanager_config.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -40,10 +40,10 @@ type matchersInhibitionRule struct {
// and disagreement. If no incompatible inputs or disagreement are found, then the Alertmanager
// can be switched to the UTF-8 strict mode. Otherwise, configurations should be fixed before
// enabling the mode.
func validateMatchersInConfigDesc(logger log.Logger, metrics *compat.Metrics, origin string, cfg alertspb.AlertConfigDesc) {
func validateMatchersInConfigDesc(logger log.Logger, origin string, cfg alertspb.AlertConfigDesc) {
// Do not add origin to the logger as it's added in the compat package.
logger = log.With(logger, "user", cfg.User)
parseFn := compat.FallbackMatchersParser(logger, metrics)
parseFn := compat.FallbackMatchersParser(logger)
matchersCfg := matchersConfig{}
if err := yaml.Unmarshal([]byte(cfg.RawConfig), &matchersCfg); err != nil {
level.Warn(logger).Log("msg", "Failed to load configuration in validateMatchersInConfigDesc", "origin", origin, "err", err)
Expand Down
83 changes: 51 additions & 32 deletions pkg/alertmanager/alertmanager_config_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3,28 +3,25 @@
package alertmanager

import (
"bytes"
"strings"
"testing"

"github.com/go-kit/log"
"github.com/prometheus/alertmanager/matchers/compat"
"github.com/prometheus/client_golang/prometheus"
"github.com/prometheus/client_golang/prometheus/testutil"
"github.com/stretchr/testify/require"

"github.com/grafana/mimir/pkg/alertmanager/alertspb"
)

func TestValidateMatchersInConfigDesc(t *testing.T) {
tests := []struct {
name string
config alertspb.AlertConfigDesc
total float64
disagreeTotal float64
incompatibleTotal float64
invalidTotal float64
name string
config alertspb.AlertConfigDesc
expected []string
}{{
name: "config is accepted",
config: alertspb.AlertConfigDesc{
User: "1",
RawConfig: `
route:
routes:
Expand All @@ -37,10 +34,15 @@ inhibit_rules:
- baz=qux
`,
},
total: 3,
expected: []string{
`level=debug user=1 msg="Parsing with UTF-8 matchers parser, with fallback to classic matchers parser" input="foo=bar" origin=test`,
`level=debug user=1 msg="Parsing with UTF-8 matchers parser, with fallback to classic matchers parser" input="bar=baz" origin=test`,
`level=debug user=1 msg="Parsing with UTF-8 matchers parser, with fallback to classic matchers parser" input="baz=qux" origin=test`,
},
}, {
name: "config contains invalid input",
config: alertspb.AlertConfigDesc{
User: "2",
RawConfig: `
route:
routes:
Expand All @@ -53,11 +55,18 @@ inhibit_rules:
- baz!qux
`,
},
total: 3,
invalidTotal: 3,
expected: []string{
`level=debug user=2 msg="Parsing with UTF-8 matchers parser, with fallback to classic matchers parser" input=foo!bar origin=test`,
`level=debug user=2 msg="Invalid matcher in route" input=foo!bar origin=test err="bad matcher format: foo!bar"`,
`level=debug user=2 msg="Parsing with UTF-8 matchers parser, with fallback to classic matchers parser" input=bar!baz origin=test`,
`level=debug user=2 msg="Invalid matcher in inhibition rule source matchers" input=bar!baz origin=test err="bad matcher format: bar!baz"`,
`level=debug user=2 msg="Parsing with UTF-8 matchers parser, with fallback to classic matchers parser" input=baz!qux origin=test`,
`level=debug user=2 msg="Invalid matcher in inhibition rule target matchers" input=baz!qux origin=test err="bad matcher format: baz!qux"`,
},
}, {
name: "config is accepted in matchers/parse but not pkg/labels",
config: alertspb.AlertConfigDesc{
User: "3",
RawConfig: `
route:
routes:
Expand All @@ -70,10 +79,15 @@ inhibit_rules:
- baz🙂=qux
`,
},
total: 3,
expected: []string{
`level=debug user=3 msg="Parsing with UTF-8 matchers parser, with fallback to classic matchers parser" input="foo🙂=bar" origin=test`,
`level=debug user=3 msg="Parsing with UTF-8 matchers parser, with fallback to classic matchers parser" input="bar🙂=baz" origin=test`,
`level=debug user=3 msg="Parsing with UTF-8 matchers parser, with fallback to classic matchers parser" input="baz🙂=qux" origin=test`,
},
}, {
name: "config is accepted in pkg/labels but not matchers/parse",
config: alertspb.AlertConfigDesc{
User: "4",
RawConfig: `
route:
routes:
Expand All @@ -86,11 +100,18 @@ inhibit_rules:
- baz=
`,
},
total: 3,
incompatibleTotal: 3,
expected: []string{
`level=debug user=4 msg="Parsing with UTF-8 matchers parser, with fallback to classic matchers parser" input="foo=" origin=test`,
`level=warn user=4 msg="Alertmanager is moving to a new parser for labels and matchers, and this input is incompatible. Alertmanager has instead parsed the input using the old matchers parser as a fallback. To make this input compatible with the new parser please make sure all regular expressions and values are double-quoted. If you are still seeing this message please open an issue." input="foo=" origin=test err="end of input: expected label value" suggestion="foo=\"\""`,
`level=debug user=4 msg="Parsing with UTF-8 matchers parser, with fallback to classic matchers parser" input="bar=" origin=test`,
`level=warn user=4 msg="Alertmanager is moving to a new parser for labels and matchers, and this input is incompatible. Alertmanager has instead parsed the input using the old matchers parser as a fallback. To make this input compatible with the new parser please make sure all regular expressions and values are double-quoted. If you are still seeing this message please open an issue." input="bar=" origin=test err="end of input: expected label value" suggestion="bar=\"\""`,
`level=debug user=4 msg="Parsing with UTF-8 matchers parser, with fallback to classic matchers parser" input="baz=" origin=test`,
`level=warn user=4 msg="Alertmanager is moving to a new parser for labels and matchers, and this input is incompatible. Alertmanager has instead parsed the input using the old matchers parser as a fallback. To make this input compatible with the new parser please make sure all regular expressions and values are double-quoted. If you are still seeing this message please open an issue." input="baz=" origin=test err="end of input: expected label value" suggestion="baz=\"\""`,
},
}, {
name: "config contains disagreement",
config: alertspb.AlertConfigDesc{
User: "5",
RawConfig: `
route:
routes:
Expand All @@ -103,27 +124,25 @@ inhibit_rules:
- baz="\xf0\x9f\x99\x82"
`,
},
total: 3,
disagreeTotal: 3,
expected: []string{
`level=debug user=5 msg="Parsing with UTF-8 matchers parser, with fallback to classic matchers parser" input="foo=\"\\xf0\\x9f\\x99\\x82\"" origin=test`,
`level=warn user=5 msg="Matchers input has disagreement" input="foo=\"\\xf0\\x9f\\x99\\x82\"" origin=test`,
`level=debug user=5 msg="Parsing with UTF-8 matchers parser, with fallback to classic matchers parser" input="bar=\"\\xf0\\x9f\\x99\\x82\"" origin=test`,
`level=warn user=5 msg="Matchers input has disagreement" input="bar=\"\\xf0\\x9f\\x99\\x82\"" origin=test`,
`level=debug user=5 msg="Parsing with UTF-8 matchers parser, with fallback to classic matchers parser" input="baz=\"\\xf0\\x9f\\x99\\x82\"" origin=test`,
`level=warn user=5 msg="Matchers input has disagreement" input="baz=\"\\xf0\\x9f\\x99\\x82\"" origin=test`,
},
}}

for _, test := range tests {
t.Run(test.name, func(t *testing.T) {
m := compat.NewMetrics(prometheus.NewRegistry())
validateMatchersInConfigDesc(log.NewNopLogger(), m, "test", test.config)
requireMetric(t, test.total, m.Total)
requireMetric(t, test.disagreeTotal, m.DisagreeTotal)
requireMetric(t, test.incompatibleTotal, m.IncompatibleTotal)
requireMetric(t, test.invalidTotal, m.InvalidTotal)
buf := bytes.Buffer{}
validateMatchersInConfigDesc(log.NewLogfmtLogger(&buf), "test", test.config)
lines := strings.Split(strings.Trim(buf.String(), "\n"), "\n")
require.Equal(t, len(test.expected), len(lines))
for i := range lines {
require.Equal(t, test.expected[i], lines[i])
}
})
}
}

func requireMetric(t *testing.T, expected float64, m *prometheus.CounterVec) {
if expected == 0 {
require.Equal(t, 0, testutil.CollectAndCount(m))
} else {
require.Equal(t, 1, testutil.CollectAndCount(m))
require.Equal(t, expected, testutil.ToFloat64(m))
}
}
Loading

0 comments on commit 6ca9b40

Please sign in to comment.