diff --git a/cmd/karma/proxy.go b/cmd/karma/proxy.go index da9a09806..72bfd00b0 100644 --- a/cmd/karma/proxy.go +++ b/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). diff --git a/cmd/karma/proxy_test.go b/cmd/karma/proxy_test.go index 000bdd8be..a78516087 100644 --- a/cmd/karma/proxy_test.go +++ b/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) + } +} diff --git a/cmd/karma/views_test.go b/cmd/karma/views_test.go index 4a75fc272..4b520bbf9 100644 --- a/cmd/karma/views_test.go +++ b/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) diff --git a/internal/alertmanager/dedup_test.go b/internal/alertmanager/dedup_test.go index ecebf05a4..087e5d4b5 100644 --- a/internal/alertmanager/dedup_test.go +++ b/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) diff --git a/internal/alertmanager/models.go b/internal/alertmanager/models.go index ef4378678..e0cb712fa 100644 --- a/internal/alertmanager/models.go +++ b/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 diff --git a/internal/mapper/mapper.go b/internal/mapper/mapper.go index 3cc6ec9ec..646916f0b 100644 --- a/internal/mapper/mapper.go +++ b/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 } } diff --git a/internal/mapper/mapper_test.go b/internal/mapper/mapper_test.go index 3b42df406..79fc8269c 100644 --- a/internal/mapper/mapper_test.go +++ b/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) + } }) } } diff --git a/internal/mapper/v017/api.go b/internal/mapper/v017/api.go index 50c196799..27825927c 100644 --- a/internal/mapper/v017/api.go +++ b/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) diff --git a/internal/models/status.go b/internal/models/status.go index 9b1fc20c3..b66e1a643 100644 --- a/internal/models/status.go +++ b/internal/models/status.go @@ -1,7 +1,6 @@ package models type AlertmanagerStatus struct { - Version string ID string PeerIDs []string } diff --git a/internal/verprobe/verprobe_test.go b/internal/verprobe/verprobe_test.go index 714db823f..bfc24a79c 100644 --- a/internal/verprobe/verprobe_test.go +++ b/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 {