Disallowing configure AM with the v1 api (prometheus#13883)

* Stop supporting Alertmanager v1

* Disallowing configure AM with the v1 api

Signed-off-by: alanprot <[email protected]>

* Update config/config_test.go

Co-authored-by: Ayoub Mrini <[email protected]>
Signed-off-by: Alan Protasio <[email protected]>

* Update config/config.go

Co-authored-by: Ayoub Mrini <[email protected]>
Signed-off-by: Alan Protasio <[email protected]>

* Addressing coments

Signed-off-by: alanprot <[email protected]>

* Update notifier/notifier.go

Co-authored-by: Ayoub Mrini <[email protected]>
Signed-off-by: Alan Protasio <[email protected]>

* Update config/config_test.go

Co-authored-by: Jan Fajerski <[email protected]>
Signed-off-by: Alan Protasio <[email protected]>

---------

Signed-off-by: alanprot <[email protected]>
Signed-off-by: Alan Protasio <[email protected]>
Co-authored-by: Ayoub Mrini <[email protected]>
Co-authored-by: Jan Fajerski <[email protected]>

Author: 3 people

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

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.

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)

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

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,37 +576,24 @@ 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()
 						return false
 					}
 				}
 
-				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 {

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))
 	}
 }