Fixes the Alertmanager panicking when no external URL is provided (#3017)

* Validate alertmanager.web.external-url

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

* Improve http error messages when acessing an inactive/non-existent AM

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

* Update the changelog.

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

* Fix integration tests

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

* Update CHANGELOG.md

Co-authored-by: Marco Pracucci <[email protected]>
Signed-off-by: gotjosh <[email protected]>

* Update pkg/alertmanager/multitenant.go

Co-authored-by: Marco Pracucci <[email protected]>
Signed-off-by: gotjosh <[email protected]>

* Update pkg/alertmanager/multitenant.go

Co-authored-by: Marco Pracucci <[email protected]>
Signed-off-by: gotjosh <[email protected]>

* Address review comments

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

Co-authored-by: Marco Pracucci <[email protected]>

Author: gotjosh

CHANGELOG.md

@@ -22,6 +22,7 @@
 * [BUGFIX] Querier: query /series from ingesters regardless the `-querier.query-ingesters-within` setting. #3035
 * [BUGFIX] Experimental blocks storage: Ingester is less likely to hit gRPC message size limit when streaming data to queriers. #3015
 * [BUGFIX] Fix configuration for TLS server validation, TLS skip verify was hardcoded to true for all TLS configurations and prevented validation of server certificates. #3030
+* [BUGFIX] Fixes the Alertmanager panicking when no `-alertmanager.web.external-url` is provided. #3017
 
 ## 1.3.0 / 2020-08-21
 

integration/alertmanager_test.go

@@ -106,5 +106,5 @@ func TestAlertmanagerStoreAPI(t *testing.T) {
 	cfg, err = c.GetAlertmanagerConfig(context.Background())
 	require.Error(t, err)
 	require.Nil(t, cfg)
-	require.EqualError(t, err, e2ecortex.ErrNotFound.Error())
+	require.EqualError(t, err, "not found")
 }

integration/e2ecortex/client.go

@@ -212,6 +212,10 @@ func (c *Client) GetAlertmanagerConfig(ctx context.Context) (*alertConfig.Config
 		return nil, ErrNotFound
 	}
 
+	if resp.StatusCode/100 != 2 {
+		return nil, fmt.Errorf("getting config failed with status %d and error %v", resp.StatusCode, string(body))
+	}
+
 	var ss *ServerStatus
 	err = json.Unmarshal(body, &ss)
 	if err != nil {

pkg/alertmanager/multitenant.go

@@ -175,6 +175,10 @@ func NewMultitenantAlertmanager(cfg *MultitenantAlertmanagerConfig, logger log.L
 		return nil, fmt.Errorf("unable to create Alertmanager data directory %q: %s", cfg.DataDir, err)
 	}
 
+	if cfg.ExternalURL.URL == nil {
+		return nil, fmt.Errorf("unable to create Alertmanager because the external URL has not been configured")
+	}
+
 	var fallbackConfig []byte
 	if cfg.FallbackConfigFile != "" {
 		fallbackConfig, err = ioutil.ReadFile(cfg.FallbackConfigFile)
@@ -460,9 +464,10 @@ func (am *MultitenantAlertmanager) ServeHTTP(w http.ResponseWriter, req *http.Re
 	am.alertmanagersMtx.Unlock()
 
 	if !ok || !userAM.IsActive() {
-		http.Error(w, "no Alertmanager for this user ID", http.StatusNotFound)
+		http.Error(w, "the Alertmanager is not configured", http.StatusNotFound)
 		return
 	}
+
 	userAM.mux.ServeHTTP(w, req)
 }
 

pkg/alertmanager/multitenant_test.go

@@ -7,6 +7,7 @@ import (
 	"context"
 	"fmt"
 	"io/ioutil"
+	"net/http/httptest"
 	"os"
 	"testing"
 
@@ -15,6 +16,7 @@ import (
 	"github.com/prometheus/client_golang/prometheus/testutil"
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
+	"github.com/weaveworks/common/user"
 
 	"github.com/cortexproject/cortex/pkg/alertmanager/alerts"
 	"github.com/cortexproject/cortex/pkg/util/flagext"
@@ -177,3 +179,69 @@ func TestLoadAllConfigs(t *testing.T) {
 		cortex_alertmanager_config_invalid{user="user3"} 0
 	`), "cortex_alertmanager_config_invalid"))
 }
+
+func TestAlertmanager_NoExternalURL(t *testing.T) {
+	tempDir, err := ioutil.TempDir(os.TempDir(), "alertmanager")
+	require.NoError(t, err)
+	defer os.RemoveAll(tempDir)
+
+	// Create the Multitenant Alertmanager.
+	reg := prometheus.NewPedanticRegistry()
+	_, err = NewMultitenantAlertmanager(&MultitenantAlertmanagerConfig{
+		DataDir: tempDir,
+	}, log.NewNopLogger(), reg)
+
+	require.EqualError(t, err, "unable to create Alertmanager because the external URL has not been configured")
+}
+
+func TestAlertmanager_ServeHTTP(t *testing.T) {
+	mockStore := &mockAlertStore{
+		configs: map[string]alerts.AlertConfigDesc{},
+	}
+
+	externalURL := flagext.URLValue{}
+	err := externalURL.Set("http://localhost:8080/alertmanager")
+	require.NoError(t, err)
+
+	tempDir, err := ioutil.TempDir(os.TempDir(), "alertmanager")
+	require.NoError(t, err)
+	defer os.RemoveAll(tempDir)
+
+	// Create the Multitenant Alertmanager.
+	reg := prometheus.NewPedanticRegistry()
+	am := createMultitenantAlertmanager(&MultitenantAlertmanagerConfig{
+		ExternalURL: externalURL,
+		DataDir:     tempDir,
+	}, nil, nil, mockStore, log.NewNopLogger(), reg)
+
+	// Request when no user configuration is present.
+	req := httptest.NewRequest("GET", externalURL.String(), nil)
+	req.Header.Add(user.OrgIDHeaderName, "user1")
+	w := httptest.NewRecorder()
+
+	am.ServeHTTP(w, req)
+
+	resp := w.Result()
+	body, _ := ioutil.ReadAll(resp.Body)
+	require.Equal(t, "the Alertmanager is not configured\n", string(body))
+
+	// Create a configuration for the user in storage.
+	mockStore.configs["user1"] = alerts.AlertConfigDesc{
+		User:      "user1",
+		RawConfig: simpleConfigTwo,
+		Templates: []*alerts.TemplateDesc{},
+	}
+
+	// Make the alertmanager pick it up, then pause it.
+	err = am.updateConfigs()
+	require.NoError(t, err)
+	am.alertmanagers["user1"].Pause()
+
+	// Request when user configuration is paused.
+	w = httptest.NewRecorder()
+	am.ServeHTTP(w, req)
+
+	resp = w.Result()
+	body, _ = ioutil.ReadAll(resp.Body)
+	require.Equal(t, "the Alertmanager is not configured\n", string(body))
+}