From c78d5b94af29fe2a7ed7e265629b91403c9b5619 Mon Sep 17 00:00:00 2001 From: Alan Protasio Date: Fri, 18 Oct 2024 06:23:14 -0700 Subject: [PATCH] Disallowing configure AM with the v1 api (#13883) * Stop supporting Alertmanager v1 * Disallowing configure AM with the v1 api Signed-off-by: alanprot * Update config/config_test.go Co-authored-by: Ayoub Mrini Signed-off-by: Alan Protasio * Update config/config.go Co-authored-by: Ayoub Mrini Signed-off-by: Alan Protasio * Addressing coments Signed-off-by: alanprot * Update notifier/notifier.go Co-authored-by: Ayoub Mrini Signed-off-by: Alan Protasio * Update config/config_test.go Co-authored-by: Jan Fajerski Signed-off-by: Alan Protasio --------- Signed-off-by: alanprot Signed-off-by: Alan Protasio Co-authored-by: Ayoub Mrini Co-authored-by: Jan Fajerski --- .../config_with_service_discovery_files.yml | 2 +- config/config.go | 3 +- config/config_test.go | 5 ++++ .../config_with_deprecated_am_api_config.yml | 7 +++++ notifier/notifier.go | 29 +++++-------------- notifier/notifier_test.go | 12 ++++---- 6 files changed, 29 insertions(+), 29 deletions(-) create mode 100644 config/testdata/config_with_deprecated_am_api_config.yml diff --git a/cmd/promtool/testdata/config_with_service_discovery_files.yml b/cmd/promtool/testdata/config_with_service_discovery_files.yml index 13b6d7faf..6a550a840 100644 --- a/cmd/promtool/testdata/config_with_service_discovery_files.yml +++ b/cmd/promtool/testdata/config_with_service_discovery_files.yml @@ -6,7 +6,7 @@ scrape_configs: alerting: alertmanagers: - scheme: http - api_version: v1 + api_version: v2 file_sd_configs: - files: - nonexistent_file.yml diff --git a/config/config.go b/config/config.go index a88b0d32f..17405309b 100644 --- a/config/config.go +++ b/config/config.go @@ -955,6 +955,7 @@ func (a AlertmanagerConfigs) ToMap() map[string]*AlertmanagerConfig { // AlertmanagerAPIVersion represents a version of the // github.com/prometheus/alertmanager/api, e.g. 'v1' or 'v2'. +// 'v1' is no longer supported. type AlertmanagerAPIVersion string // UnmarshalYAML implements the yaml.Unmarshaler interface. @@ -984,7 +985,7 @@ const ( ) var SupportedAlertmanagerAPIVersions = []AlertmanagerAPIVersion{ - AlertmanagerAPIVersionV1, AlertmanagerAPIVersionV2, + AlertmanagerAPIVersionV2, } // AlertmanagerConfig configures how Alertmanagers can be discovered and communicated with. diff --git a/config/config_test.go b/config/config_test.go index 547070dac..9734847f3 100644 --- a/config/config_test.go +++ b/config/config_test.go @@ -1500,6 +1500,11 @@ var expectedConf = &Config{ }, } +func TestYAMLNotLongerSupportedAMApi(t *testing.T) { + _, err := LoadFile("testdata/config_with_no_longer_supported_am_api_config.yml", false, promslog.NewNopLogger()) + require.Error(t, err) +} + func TestYAMLRoundtrip(t *testing.T) { want, err := LoadFile("testdata/roundtrip.good.yml", false, promslog.NewNopLogger()) require.NoError(t, err) diff --git a/config/testdata/config_with_deprecated_am_api_config.yml b/config/testdata/config_with_deprecated_am_api_config.yml new file mode 100644 index 000000000..ac89537ff --- /dev/null +++ b/config/testdata/config_with_deprecated_am_api_config.yml @@ -0,0 +1,7 @@ +alerting: + alertmanagers: + - scheme: http + api_version: v1 + file_sd_configs: + - files: + - nonexistent_file.yml diff --git a/notifier/notifier.go b/notifier/notifier.go index 482d2fdaa..e970b67e6 100644 --- a/notifier/notifier.go +++ b/notifier/notifier.go @@ -542,10 +542,10 @@ func (n *Manager) sendAll(alerts ...*Alert) bool { begin := time.Now() - // v1Payload and v2Payload represent 'alerts' marshaled for Alertmanager API - // v1 or v2. Marshaling happens below. Reference here is for caching between + // cachedPayload represent 'alerts' marshaled for Alertmanager API v2. + // Marshaling happens below. Reference here is for caching between // for loop iterations. - var v1Payload, v2Payload []byte + var cachedPayload []byte n.mtx.RLock() amSets := n.alertmanagers @@ -576,29 +576,16 @@ func (n *Manager) sendAll(alerts ...*Alert) bool { continue } // We can't use the cached values from previous iteration. - v1Payload, v2Payload = nil, nil + cachedPayload = nil } switch ams.cfg.APIVersion { - case config.AlertmanagerAPIVersionV1: - { - if v1Payload == nil { - v1Payload, err = json.Marshal(amAlerts) - if err != nil { - n.logger.Error("Encoding alerts for Alertmanager API v1 failed", "err", err) - ams.mtx.RUnlock() - return false - } - } - - payload = v1Payload - } case config.AlertmanagerAPIVersionV2: { - if v2Payload == nil { + if cachedPayload == nil { openAPIAlerts := alertsToOpenAPIAlerts(amAlerts) - v2Payload, err = json.Marshal(openAPIAlerts) + cachedPayload, err = json.Marshal(openAPIAlerts) if err != nil { n.logger.Error("Encoding alerts for Alertmanager API v2 failed", "err", err) ams.mtx.RUnlock() @@ -606,7 +593,7 @@ func (n *Manager) sendAll(alerts ...*Alert) bool { } } - payload = v2Payload + payload = cachedPayload } default: { @@ -621,7 +608,7 @@ func (n *Manager) sendAll(alerts ...*Alert) bool { if len(ams.cfg.AlertRelabelConfigs) > 0 { // We can't use the cached values on the next iteration. - v1Payload, v2Payload = nil, nil + cachedPayload = nil } for _, am := range ams.ams { diff --git a/notifier/notifier_test.go b/notifier/notifier_test.go index 83eaf8168..97b0274f2 100644 --- a/notifier/notifier_test.go +++ b/notifier/notifier_test.go @@ -50,27 +50,27 @@ func TestPostPath(t *testing.T) { }{ { in: "", - out: "/api/v1/alerts", + out: "/api/v2/alerts", }, { in: "/", - out: "/api/v1/alerts", + out: "/api/v2/alerts", }, { in: "/prefix", - out: "/prefix/api/v1/alerts", + out: "/prefix/api/v2/alerts", }, { in: "/prefix//", - out: "/prefix/api/v1/alerts", + out: "/prefix/api/v2/alerts", }, { in: "prefix//", - out: "/prefix/api/v1/alerts", + out: "/prefix/api/v2/alerts", }, } for _, c := range cases { - require.Equal(t, c.out, postPath(c.in, config.AlertmanagerAPIVersionV1)) + require.Equal(t, c.out, postPath(c.in, config.AlertmanagerAPIVersionV2)) } }