fix(backend): simplify version probes

cmd/karma/proxy.go

@@ -97,10 +97,6 @@ func handlePostRequest(alertmanager *alertmanager.Alertmanager, h http.Handler)
 		}
 
 		ver := alertmanager.Version()
-		if ver == "" {
-			ver = "999.0"
-		}
-
 		m, err := mapper.GetSilenceMapper(ver)
 		if err != nil {
 			log.Error().Err(err).

cmd/karma/proxy_test.go

@@ -2,6 +2,7 @@ package main
 
 import (
 	"bytes"
+	"errors"
 	"fmt"
 	"io/ioutil"
 	"net/http"
@@ -378,15 +379,7 @@ func TestProxyUserRewrite(t *testing.T) {
   { "isRegex": false, "name": "alertname", "value": "Fake Alert" },
   { "isRegex": true, "name": "foo", "value": "(bar|baz)" }
 ]}`,
-			proxyRequestBody: `{
-"comment": "comment",
-"createdBy": "username",
-"startsAt": "2000-02-01T00:00:00.000Z",
-"endsAt": "2000-02-01T00:02:03.000Z",
-"matchers": [
-  { "isRegex": false, "name": "alertname", "value": "Fake Alert" },
-  { "isRegex": true, "name": "foo", "value": "(bar|baz)" }
-]}`,
+			proxyRequestBody: `{"comment":"comment","createdBy":"","endsAt":"2000-02-01T00:02:03.000Z","matchers":[{"isRegex":false,"name":"alertname","value":"Fake Alert"},{"isRegex":true,"name":"foo","value":"(bar|baz)"}],"startsAt":"2000-02-01T00:00:00.000Z"}`,
 		},
 		{
 			name:         "basicAuth, correct credentials, invalid JSON",
@@ -513,6 +506,7 @@ func TestProxyUserRewrite(t *testing.T) {
 				t.Logf("Testing alerts using mock files from Alertmanager %s", version)
 
 				config.Config.Listen.Prefix = "/"
+				config.Config.Authentication.Enabled = true
 				config.Config.Authentication.Header.Name = testCase.headerName
 				config.Config.Authentication.Header.ValueRegex = testCase.headerRe
 				config.Config.Authentication.BasicAuth.Users = testCase.basicAuthUsers
@@ -1215,3 +1209,106 @@ func TestProxySilenceACL(t *testing.T) {
 		})
 	}
 }
+
+type errReader int
+
+func (errReader) Read(p []byte) (n int, err error) {
+	return 0, errors.New("request read error")
+}
+
+func TestProxyRequestReadFailure(t *testing.T) {
+	for _, version := range mock.ListAllMocks() {
+		t.Logf("Testing alerts using mock files from Alertmanager %s", version)
+		config.Config.Listen.Prefix = "/"
+		config.Config.Authentication.Header.Name = ""
+		config.Config.Authentication.BasicAuth.Users = []config.AuthenticationUser{}
+
+		r := testRouter()
+		setupRouter(r)
+
+		am, err := alertmanager.NewAlertmanager(
+			"cluster",
+			"proxyRead",
+			"http://localhost",
+			alertmanager.WithRequestTimeout(time.Second*5),
+			alertmanager.WithProxy(true),
+		)
+		if err != nil {
+			t.Error(err)
+		}
+		setupRouterProxyHandlers(r, am)
+
+		req := httptest.NewRequest("POST", "/proxy/alertmanager/proxyRead/api/v2/silences", errReader(0))
+
+		resp := newCloseNotifyingRecorder()
+		r.ServeHTTP(resp, req)
+		if resp.Code != 500 {
+			t.Errorf("Got response code %d instead of 500", resp.Code)
+		}
+
+		gotBody, _ := ioutil.ReadAll(resp.Body)
+		if string(gotBody) != "request read error\n" {
+			t.Errorf("Body mismatch:\n%s", gotBody)
+		}
+	}
+}
+
+func TestProxyRequestToUnsupportedAlertmanager(t *testing.T) {
+	zerolog.SetGlobalLevel(zerolog.FatalLevel)
+
+	httpmock.Activate()
+	defer httpmock.DeactivateAndReset()
+
+	config.Config.Listen.Prefix = "/"
+	config.Config.Authentication.Header.Name = ""
+	config.Config.Authentication.BasicAuth.Users = []config.AuthenticationUser{}
+
+	r := testRouter()
+	setupRouter(r)
+
+	am, err := alertmanager.NewAlertmanager(
+		"cluster",
+		"proxyToUnsupported",
+		"http://localhost",
+		alertmanager.WithRequestTimeout(time.Second*5),
+		alertmanager.WithProxy(true),
+	)
+	if err != nil {
+		t.Error(err)
+	}
+	setupRouterProxyHandlers(r, am)
+
+	apiCache = cache.New(cache.NoExpiration, 10*time.Second)
+	httpmock.Reset()
+	httpmock.RegisterResponder("GET", "http://localhost/metrics", httpmock.NewStringResponder(200, `alertmanager_build_info{version="0.1.0"} 1
+	`))
+	httpmock.RegisterResponder("GET", "http://localhost/api/v2/status", httpmock.NewStringResponder(200, `{
+		"cluster": {
+			"name": "BBBBBBBBBBBBBBBBBBBBBBBBBB",
+			"peers": [],
+			"status": "ready"
+		}
+	}`))
+	httpmock.RegisterResponder("POST", "http://localhost/api/v2/silences", httpmock.NewStringResponder(200, "{}"))
+	httpmock.RegisterResponder("GET", "http://localhost/api/v2/silences", httpmock.NewStringResponder(200, "[]"))
+	httpmock.RegisterResponder("GET", "http://localhost/api/v2/alerts/groups", httpmock.NewStringResponder(200, "[]"))
+	_ = am.Pull()
+
+	if ver := am.Version(); ver != "0.1.0" {
+		t.Errorf("Got wrong version: %q", ver)
+		return
+	}
+
+	req := httptest.NewRequest("POST", "/proxy/alertmanager/proxyToUnsupported/api/v2/silences", ioutil.NopCloser(bytes.NewBufferString(`{}`)))
+
+	resp := newCloseNotifyingRecorder()
+	r.ServeHTTP(resp, req)
+	if resp.Code != 500 {
+		t.Errorf("Got response code %d instead of 500", resp.Code)
+	}
+
+	gotBody, _ := ioutil.ReadAll(resp.Body)
+	if string(gotBody) != "can't find silence mapper for Alertmanager 0.1.0\n" {
+		t.Errorf("Body mismatch:\n%s", gotBody)
+	}
+}

cmd/karma/views_test.go

@@ -1113,12 +1113,14 @@ func TestUpstreamStatus(t *testing.T) {
 				{
 					uri:  "http://ha1.example.com/metrics",
 					code: 200,
-					body: `alertmanager_build_info{version="0.20.0"} 1`,
+					body: `alertmanager_build_info{version="0.20.0"} 1
+					`,
 				},
 				{
 					uri:  "http://ha2.example.com/metrics",
 					code: 200,
-					body: `alertmanager_build_info{version="0.19.0"} 1`,
+					body: `alertmanager_build_info{version="0.19.0"} 1
+					`,
 				},
 				{
 					uri:  "http://ha1.example.com/api/v2/status",
@@ -1256,12 +1258,14 @@ func TestUpstreamStatus(t *testing.T) {
 				{
 					uri:  "http://ha1.example.com/metrics",
 					code: 200,
-					body: `alertmanager_build_info{version="0.20.0"} 1`,
+					body: `alertmanager_build_info{version="0.20.0"} 1
+					`,
 				},
 				{
 					uri:  "http://ha2.example.com/metrics",
 					code: 200,
-					body: `alertmanager_build_info{version="0.19.0"} 1`,
+					body: `alertmanager_build_info{version="0.19.0"} 1
+					`,
 				},
 				{
 					uri:  "http://ha1.example.com/api/v2/status",
@@ -1397,12 +1401,14 @@ func TestUpstreamStatus(t *testing.T) {
 				{
 					uri:  "http://ha1.example.com/metrics",
 					code: 200,
-					body: `alertmanager_build_info{version="0.20.0"} 1`,
+					body: `alertmanager_build_info{version="0.20.0"} 1
+					`,
 				},
 				{
 					uri:  "http://ha2.example.com/metrics",
 					code: 200,
-					body: `alertmanager_build_info{version="0.19.0"} 1`,
+					body: `alertmanager_build_info{version="0.19.0"} 1
+					`,
 				},
 				{
 					uri:  "http://ha1.example.com/api/v2/status",
@@ -1532,12 +1538,14 @@ func TestUpstreamStatus(t *testing.T) {
 				{
 					uri:  "http://ha1.example.com/metrics",
 					code: 200,
-					body: `alertmanager_build_info{version="0.20.0"} 1`,
+					body: `alertmanager_build_info{version="0.20.0"} 1
+					`,
 				},
 				{
 					uri:  "http://ha2.example.com/metrics",
 					code: 200,
-					body: `alertmanager_build_info{version="0.19.0"} 1`,
+					body: `alertmanager_build_info{version="0.19.0"} 1
+					`,
 				},
 				{
 					uri:  "http://ha1.example.com/api/v2/status",
@@ -1666,12 +1674,14 @@ func TestUpstreamStatus(t *testing.T) {
 				{
 					uri:  "http://broken1.example.com/metrics",
 					code: 200,
-					body: `alertmanager_build_info{version="0.20.0"} 1`,
+					body: `alertmanager_build_info{version="0.20.0"} 1
+					`,
 				},
 				{
 					uri:  "http://broken2.example.com/metrics",
 					code: 200,
-					body: `alertmanager_build_info{version="0.20.0"} 1`,
+					body: `alertmanager_build_info{version="0.20.0"} 1
+					`,
 				},
 				{
 					uri:  "http://broken1.example.com/api/v2/status",
@@ -1812,7 +1822,8 @@ func TestUpstreamStatus(t *testing.T) {
 				{
 					uri:  "http://ha2.example.com/metrics",
 					code: 200,
-					body: `alertmanager_build_info{version="0.21.0"} 1`,
+					body: `alertmanager_build_info{version="0.21.0"} 1
+					`,
 				},
 				{
 					uri:  "http://ha1.example.com/api/v2/status",
@@ -1922,7 +1933,7 @@ func TestUpstreamStatus(t *testing.T) {
 						Headers:         map[string]string{},
 						CORSCredentials: "omit",
 						Error:           "",
-						Version:         "0.20.0",
+						Version:         "",
 						Cluster:         "Errors",
 						ClusterMembers:  []string{"ha1", "ha2"},
 					},
@@ -1934,7 +1945,7 @@ func TestUpstreamStatus(t *testing.T) {
 						Headers:         map[string]string{},
 						CORSCredentials: "omit",
 						Error:           "json: cannot unmarshal array into Go value of type string",
-						Version:         "0.19.0",
+						Version:         "0.21.0",
 						Cluster:         "Errors",
 						ClusterMembers:  []string{"ha1", "ha2"},
 					},
@@ -1950,7 +1961,8 @@ func TestUpstreamStatus(t *testing.T) {
 				{
 					uri:  "http://ha1.example.com/metrics",
 					code: 200,
-					body: `alertmanager_build_info{version="0.20.0"} 1`,
+					body: `alertmanager_build_info{version="0.20.0"} 1
+					`,
 				},
 				{
 					uri:  "http://ha2.example.com/metrics",
@@ -1960,7 +1972,8 @@ func TestUpstreamStatus(t *testing.T) {
 				{
 					uri:  "http://single.example.com/metrics",
 					code: 200,
-					body: `alertmanager_build_info{version="0.21.0"} 1`,
+					body: `alertmanager_build_info{version="0.21.0"} 1
+					`,
 				},
 				{
 					uri:  "http://ha1.example.com/api/v2/status",
@@ -2124,24 +2137,31 @@ func TestUpstreamStatus(t *testing.T) {
 
 	for _, testCase := range testCases {
 		t.Run(testCase.Name, func(t *testing.T) {
+			zerolog.SetGlobalLevel(zerolog.DebugLevel)
+
 			httpmock.Activate()
 			defer httpmock.DeactivateAndReset()
-			for _, m := range testCase.mocks {
-				httpmock.RegisterResponder("GET", m.uri, httpmock.NewStringResponder(m.code, m.body))
-			}
 
+			config.Config.Listen.Prefix = "/"
+			config.Config.Authentication.Header.Name = ""
+			config.Config.Authentication.BasicAuth.Users = []config.AuthenticationUser{}
+
+			apiCache = cache.New(cache.NoExpiration, 10*time.Second)
 			alertmanager.UnregisterAll()
-			mockConfig()
 			config.Config.Alertmanager.Servers = testCase.upstreams
 			err := setupUpstreams()
 			if err != nil {
 				t.Error(err)
 			}
-			zerolog.SetGlobalLevel(zerolog.FatalLevel)
-			pullFromAlertmanager()
 			r := testRouter()
 			setupRouter(r)
 
+			httpmock.Reset()
+			for _, m := range testCase.mocks {
+				httpmock.RegisterResponder("GET", m.uri, httpmock.NewStringResponder(m.code, m.body))
+			}
+			pullFromAlertmanager()
+
 			req := httptest.NewRequest("GET", "/alerts.json?q=@receiver=by-cluster-service&q=alertname=HTTP_Probe_Failed&q=instance=web1", nil)
 			resp := httptest.NewRecorder()
 			r.ServeHTTP(resp, req)

internal/alertmanager/dedup_test.go

@@ -182,8 +182,10 @@ func TestStripReceivers(t *testing.T) {
 }
 
 func TestClearData(t *testing.T) {
-	zerolog.SetGlobalLevel(zerolog.PanicLevel)
+	zerolog.SetGlobalLevel(zerolog.ErrorLevel)
 	httpmock.Activate()
+	defer httpmock.DeactivateAndReset()
+
 	for _, version := range mock.ListAllMocks() {
 		name := fmt.Sprintf("clear-data-mock-%s", version)
 		uri := fmt.Sprintf("http://localhost/clear/%s", version)
@@ -191,8 +193,8 @@ func TestClearData(t *testing.T) {
 
 		mock.RegisterURL(fmt.Sprintf("%s/metrics", uri), version, "metrics")
 		_ = am.Pull()
-		if am.Version() != "" {
-			t.Errorf("[%s] Got non-empty version string: %s", am.Name, am.Version())
+		if am.Version() == "" {
+			t.Errorf("[%s] Got empty version string", am.Name)
 		}
 		if am.Error() == "" {
 			t.Errorf("[%s] Got empty error string", am.Name)

internal/alertmanager/models.go

@@ -50,14 +50,15 @@ type Alertmanager struct {
 	// lock protects data access while updating
 	lock sync.RWMutex
 	// fields for storing pulled data
-	alertGroups  []models.AlertGroup
-	silences     map[string]models.Silence
-	colors       models.LabelsColorMap
-	autocomplete []models.Autocomplete
-	knownLabels  []string
-	lastError    string
-	status       models.AlertmanagerStatus
-	clusterName  string
+	alertGroups      []models.AlertGroup
+	silences         map[string]models.Silence
+	colors           models.LabelsColorMap
+	autocomplete     []models.Autocomplete
+	knownLabels      []string
+	lastError        string
+	lastVersionProbe string
+	status           models.AlertmanagerStatus
+	clusterName      string
 	// metrics tracked per alertmanager instance
 	Metrics alertmanagerMetrics
 	// headers to send with each AlertManager request
@@ -67,15 +68,13 @@ type Alertmanager struct {
 }
 
 func (am *Alertmanager) probeVersion() string {
-	const fakeVersion = "999.0.0"
-
 	url, err := uri.JoinURL(am.URI, "metrics")
 	if err != nil {
 		log.Error().
 			Err(err).
 			Str("uri", am.SanitizedURI()).
 			Msg("Failed to join url with /metrics path")
-		return fakeVersion
+		return ""
 	}
 
 	source, err := am.reader.Read(url, am.HTTPHeaders)
@@ -85,13 +84,14 @@ func (am *Alertmanager) probeVersion() string {
 			Str("alertmanager", am.Name).
 			Str("uri", am.SanitizedURI()).
 			Msg("Request failed")
-		return fakeVersion
+		return ""
 	}
 	defer source.Close()
 
 	version, err := verprobe.Detect(source)
 	if err != nil {
-		return fakeVersion
+		log.Error().Err(err).Str("alertmanager", am.Name).Msg("Error while discovering version")
+		return ""
 	}
 	log.Info().
 		Str("version", version).
@@ -130,7 +130,6 @@ func (am *Alertmanager) clearData() {
 func (am *Alertmanager) clearStatus() {
 	am.lock.Lock()
 	am.status = models.AlertmanagerStatus{
-		Version: "",
 		ID:      "",
 		PeerIDs: []string{},
 	}
@@ -337,6 +336,11 @@ func (am *Alertmanager) Pull() error {
 	am.Metrics.Cycles++
 
 	version := am.probeVersion()
+	am.lock.Lock()
+	am.lastVersionProbe = version
+	am.lock.Unlock()
+
+	log.Debug().Str("alertmanager", am.Name).Str("version", version).Msg("Probed alertmanager version")
 
 	// verify that URI is correct
 	_, err := url.Parse(am.URI)
@@ -493,7 +497,7 @@ func (am *Alertmanager) Version() string {
 	am.lock.RLock()
 	defer am.lock.RUnlock()
 
-	return am.status.Version
+	return am.lastVersionProbe
 }
 
 // ClusterPeers returns a list of IDs of all peers this instance

internal/mapper/mapper.go

@@ -51,10 +51,18 @@ func fixSemVersion(version string) string {
 	return strings.SplitN(version, "-", 2)[0]
 }
 
+func latestIfEmpty(version string) string {
+	if version == "" {
+		return "999.0"
+	}
+	return version
+}
+
 // GetAlertMapper returns mapper for given version
 func GetAlertMapper(version string) (AlertMapper, error) {
+	ver := latestIfEmpty(version)
 	for _, m := range alertMappers {
-		if m.IsSupported(fixSemVersion(version)) {
+		if m.IsSupported(fixSemVersion(ver)) {
 			return m, nil
 		}
 	}
@@ -69,8 +77,9 @@ func RegisterSilenceMapper(m SilenceMapper) {
 
 // GetSilenceMapper returns mapper for given version
 func GetSilenceMapper(version string) (SilenceMapper, error) {
+	ver := latestIfEmpty(version)
 	for _, m := range silenceMappers {
-		if m.IsSupported(fixSemVersion(version)) {
+		if m.IsSupported(fixSemVersion(ver)) {
 			return m, nil
 		}
 	}
@@ -85,8 +94,9 @@ func RegisterStatusMapper(m StatusMapper) {
 
 // GetStatusMapper returns mapper for given version
 func GetStatusMapper(version string) (StatusMapper, error) {
+	ver := latestIfEmpty(version)
 	for _, m := range statusMappers {
-		if m.IsSupported(fixSemVersion(version)) {
+		if m.IsSupported(fixSemVersion(ver)) {
 			return m, nil
 		}
 	}

internal/mapper/mapper_test.go

@@ -16,6 +16,7 @@ type testCaseType struct {
 var testCases = []testCaseType{
 	{requestedVersion: "0.0", hadError: true},
 	{requestedVersion: "abc", hadError: true, hadPanic: true},
+	{requestedVersion: "0.1.0", hadError: true},
 	{requestedVersion: "0.4.0", hadError: true},
 	{requestedVersion: "0.4.0-rc-1", hadError: true},
 	{requestedVersion: "0.5.1-beta", hadError: true},
@@ -29,6 +30,8 @@ var testCases = []testCaseType{
 	{requestedVersion: "0.20-beta.1"},
 	{requestedVersion: "0.20"},
 	{requestedVersion: "0.20.1"},
+	{requestedVersion: "999.0"},
+	{requestedVersion: ""},
 }
 
 func TestGetAlertMapper(t *testing.T) {
@@ -46,13 +49,16 @@ func TestGetAlertMapper(t *testing.T) {
 				}
 			}()
 
-			_, err := mapper.GetAlertMapper(testCase.requestedVersion)
+			m, err := mapper.GetAlertMapper(testCase.requestedVersion)
 			if (err != nil) != testCase.hadError {
 				t.Errorf("[%s] expected error=%v, got %v", testCase.requestedVersion, testCase.hadError, err)
 			}
 			if hadPanic != testCase.hadPanic {
 				t.Errorf("[%s] expected panic=%v, got %v", testCase.requestedVersion, testCase.hadPanic, hadPanic)
 			}
+			if m == nil && !testCase.hadError && !testCase.hadPanic {
+				t.Errorf("[%s] got nil mapper", testCase.requestedVersion)
+			}
 		})
 	}
 }
@@ -72,13 +78,16 @@ func TestGetSilenceMapper(t *testing.T) {
 				}
 			}()
 
-			_, err := mapper.GetSilenceMapper(testCase.requestedVersion)
+			m, err := mapper.GetSilenceMapper(testCase.requestedVersion)
 			if (err != nil) != testCase.hadError {
 				t.Errorf("[%s] expected error=%v, got %v", testCase.requestedVersion, testCase.hadError, err)
 			}
 			if hadPanic != testCase.hadPanic {
 				t.Errorf("[%s] expected panic=%v, got %v", testCase.requestedVersion, testCase.hadPanic, hadPanic)
 			}
+			if m == nil && !testCase.hadError && !testCase.hadPanic {
+				t.Errorf("[%s] got nil mapper", testCase.requestedVersion)
+			}
 		})
 	}
 }
@@ -98,13 +107,16 @@ func TestGetStatusMapper(t *testing.T) {
 				}
 			}()
 
-			_, err := mapper.GetStatusMapper(testCase.requestedVersion)
+			m, err := mapper.GetStatusMapper(testCase.requestedVersion)
 			if (err != nil) != testCase.hadError {
 				t.Errorf("[%s] expected error=%v, got %v", testCase.requestedVersion, testCase.hadError, err)
 			}
 			if hadPanic != testCase.hadPanic {
 				t.Errorf("[%s] expected panic=%v, got %v", testCase.requestedVersion, testCase.hadPanic, hadPanic)
 			}
+			if m == nil && !testCase.hadError && !testCase.hadPanic {
+				t.Errorf("[%s] got nil mapper", testCase.requestedVersion)
+			}
 		})
 	}
 }

internal/mapper/v017/api.go

@@ -115,7 +115,6 @@ func status(c *client.AlertmanagerAPI, timeout time.Duration) (models.Alertmanag
 		return ret, err
 	}
 
-	ret.Version = *status.Payload.VersionInfo.Version
 	ret.ID = status.Payload.Cluster.Name
 	for _, p := range status.Payload.Cluster.Peers {
 		ret.PeerIDs = append(ret.PeerIDs, *p.Name)

internal/models/status.go

@@ -1,7 +1,6 @@
 package models
 
 type AlertmanagerStatus struct {
-	Version string
 	ID      string
 	PeerIDs []string
 }

internal/verprobe/verprobe_test.go

@@ -36,6 +36,11 @@ func TestDetect(t *testing.T) {
 			metrics: "alertmanager_build_info{version=\"1.2.3\"} 1\n",
 			version: "1.2.3",
 		},
+		{
+			metrics: "alertmanager_build_info{foo=\"bar\"} 1",
+			version: "",
+			isError: true,
+		},
 	}
 
 	for _, testCase := range testCases {