From 9532c2c70048c12201b29e86c0d20ee2ac2c84fe Mon Sep 17 00:00:00 2001
From: Luke Overend <luke@cloudflare.com>
Date: Tue, 12 Dec 2017 13:40:00 +0000
Subject: [PATCH] Pass ams to go routine when sending alerts (#3284)

Currently when sending alerts via the go routine within `sendAll`, the value
of `ams` is not passed to the routine, causing it to use the updated value of `ams`.

Example config:

```
alerting:
  alertmanagers:
    - basic_auth:
        username: 'prometheus'
        password: 'test123'
      static_configs:
      - targets:
        - localhost:9094
    - static_configs:
      - targets:
        - localhost:9095
```

In this example alerts sent to `localhost:9094` fail with:

```
level=error ts=2017-10-12T10:03:53.456819948Z caller=notifier.go:445
component=notifier alertmanager=http://localhost:9094/api/v1/alerts
count=1 msg="Error sending alert" err="bad response status 401
Unauthorized"
```

If you change the order to be:

```
alerting:
  alertmanagers:
    - static_configs:
      - targets:
        - localhost:9095
    - basic_auth:
        username: 'prometheus'
        password: 'test123'
      static_configs:
      - targets:
        - localhost:9094
```

It works as expected.

This commit changes the behavour so `ams` is passed to the go routine so
`n.sendOne` uses the appropriate `http.Client` details.
---
 notifier/notifier.go      |  4 ++--
 notifier/notifier_test.go | 27 +++++++++++++++++++++++++++
 2 files changed, 29 insertions(+), 2 deletions(-)

diff --git a/notifier/notifier.go b/notifier/notifier.go
index 9179c931e..36e63fd6e 100644
--- a/notifier/notifier.go
+++ b/notifier/notifier.go
@@ -437,7 +437,7 @@ func (n *Notifier) sendAll(alerts ...*Alert) bool {
 			ctx, cancel := context.WithTimeout(n.ctx, ams.cfg.Timeout)
 			defer cancel()
 
-			go func(am alertmanager) {
+			go func(ams *alertmanagerSet, am alertmanager) {
 				u := am.url().String()
 
 				if err := n.sendOne(ctx, ams.client, u, b); err != nil {
@@ -450,7 +450,7 @@ func (n *Notifier) sendAll(alerts ...*Alert) bool {
 				n.metrics.sent.WithLabelValues(u).Add(float64(len(alerts)))
 
 				wg.Done()
-			}(am)
+			}(ams, am)
 		}
 		ams.mtx.RUnlock()
 	}
diff --git a/notifier/notifier_test.go b/notifier/notifier_test.go
index 2911e5c19..a20452189 100644
--- a/notifier/notifier_test.go
+++ b/notifier/notifier_test.go
@@ -30,6 +30,7 @@ import (
 	"github.com/prometheus/common/model"
 	"github.com/prometheus/prometheus/config"
 	"github.com/prometheus/prometheus/pkg/labels"
+	"github.com/prometheus/prometheus/util/httputil"
 )
 
 func TestPostPath(t *testing.T) {
@@ -141,10 +142,20 @@ func TestHandlerSendAll(t *testing.T) {
 		}
 	}
 	server1 := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		user, pass, _ := r.BasicAuth()
+		if user != "prometheus" || pass != "testing_password" {
+			t.Fatalf("Incorrect auth details for an alertmanager")
+		}
+
 		f(w, r)
 		w.WriteHeader(status1)
 	}))
 	server2 := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		user, pass, _ := r.BasicAuth()
+		if user != "" || pass != "" {
+			t.Fatalf("Incorrectly received auth details for an alertmanager")
+		}
+
 		f(w, r)
 		w.WriteHeader(status2)
 	}))
@@ -153,11 +164,27 @@ func TestHandlerSendAll(t *testing.T) {
 	defer server2.Close()
 
 	h := New(&Options{}, nil)
+
+	authClient, _ := httputil.NewClientFromConfig(config.HTTPClientConfig{
+		BasicAuth: &config.BasicAuth{
+			Username: "prometheus",
+			Password: "testing_password",
+		},
+	}, "auth_alertmanager")
 	h.alertmanagers = append(h.alertmanagers, &alertmanagerSet{
 		ams: []alertmanager{
 			alertmanagerMock{
 				urlf: func() string { return server1.URL },
 			},
+		},
+		cfg: &config.AlertmanagerConfig{
+			Timeout: time.Second,
+		},
+		client: authClient,
+	})
+
+	h.alertmanagers = append(h.alertmanagers, &alertmanagerSet{
+		ams: []alertmanager{
 			alertmanagerMock{
 				urlf: func() string { return server2.URL },
 			},