Add validation of Alertmanager configs to API

CHANGELOG.md

@@ -17,6 +17,7 @@
 * [ENHANCEMENT] Query-tee: add support for doing a passthrough of requests to preferred backend for unregistered routes #3018
 * [ENHANCEMENT] Expose `storage.aws.dynamodb.backoff_config` configuration file field. #3026
 * [ENHANCEMENT] Added `cortex_request_message_bytes` and `cortex_response_message_bytes` histograms to track received and sent gRPC message and HTTP request/response sizes. Added `cortex_inflight_requests` gauge to track number of inflight gRPC and HTTP requests. #3064
+* [ENHANCEMENT] Add config validation to the experimental Alertmanager API. Invalid configs are no longer accepted. #3053
 * [BUGFIX] Query-frontend: Fixed rounding for incoming query timestamps, to be 100% Prometheus compatible. #2990
 * [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

go.sum

@@ -1087,6 +1087,7 @@ github.com/xdg/stringprep v1.0.0/go.mod h1:Jhud4/sHMO4oL310DaZAKk9ZaJ08SJfe+sJh0
 github.com/xiang90/probing v0.0.0-20190116061207-43a291ad63a2 h1:eY9dn8+vbi4tKz5Qo6v2eYzo7kUS51QINcR5jNpbZS8=
 github.com/xiang90/probing v0.0.0-20190116061207-43a291ad63a2/go.mod h1:UETIi67q53MR2AWcXfiuqkDkRtnGDLqkBTpCHuJHxtU=
 github.com/xlab/treeprint v0.0.0-20180616005107-d6fb6747feb6/go.mod h1:ce1O1j6UtZfjr22oyGxGLbauSBp2YVXpARAosm7dHBg=
+github.com/xlab/treeprint v1.0.0 h1:J0TkWtiuYgtdlrkkrDLISYBQ92M+X5m4LrIIMKrbDTs=
 github.com/xlab/treeprint v1.0.0/go.mod h1:IoImgRak9i3zJyuxOKUP1v4UZd1tMoKkq/Cimt1uhCg=
 github.com/yuin/goldmark v1.1.25/go.mod h1:3hX8gzYuyVAZsxl0MRgGTJEmQBFcNTphYh9decYSb74=
 github.com/yuin/goldmark v1.1.27/go.mod h1:3hX8gzYuyVAZsxl0MRgGTJEmQBFcNTphYh9decYSb74=

pkg/alertmanager/alerts/compat.go

@@ -7,7 +7,7 @@ var (
 )
 
 // ToProto transforms a yaml Alertmanager config and map of template files to an AlertConfigDesc
-func ToProto(cfg string, templates map[string]string, user string) (AlertConfigDesc, error) {
+func ToProto(cfg string, templates map[string]string, user string) AlertConfigDesc {
 	tmpls := []*TemplateDesc{}
 	for fn, body := range templates {
 		tmpls = append(tmpls, &TemplateDesc{
@@ -19,7 +19,7 @@ func ToProto(cfg string, templates map[string]string, user string) (AlertConfigD
 		User:      user,
 		RawConfig: cfg,
 		Templates: tmpls,
-	}, nil
+	}
 }
 
 // ParseTemplates returns a alertmanager config object

pkg/alertmanager/api.go

@@ -4,17 +4,22 @@ import (
 	"fmt"
 	"io/ioutil"
 	"net/http"
+	"os"
+	"path/filepath"
 
 	"github.com/cortexproject/cortex/pkg/alertmanager/alerts"
 	"github.com/cortexproject/cortex/pkg/util"
 
 	"github.com/go-kit/kit/log/level"
+	"github.com/prometheus/alertmanager/config"
+	"github.com/prometheus/alertmanager/template"
 	"github.com/weaveworks/common/user"
 	"gopkg.in/yaml.v2"
 )
 
 const (
 	errMarshallingYAML       = "error marshalling YAML Alertmanager config"
+	errValidatingConfig      = "error validating Alertmanager config"
 	errReadingConfiguration  = "unable to read the Alertmanager config"
 	errStoringConfiguration  = "unable to store the Alertmanager config"
 	errDeletingConfiguration = "unable to delete the Alertmanager config"
@@ -89,7 +94,13 @@ func (am *MultitenantAlertmanager) SetUserConfig(w http.ResponseWriter, r *http.
 		return
 	}
 
-	cfgDesc, _ := alerts.ToProto(cfg.AlertmanagerConfig, cfg.TemplateFiles, userID)
+	cfgDesc := alerts.ToProto(cfg.AlertmanagerConfig, cfg.TemplateFiles, userID)
+	if err := validateUserConfig(cfgDesc); err != nil {
+		level.Warn(logger).Log("msg", errValidatingConfig, "err", err.Error())
+		http.Error(w, fmt.Sprintf("%s: %s", errValidatingConfig, err.Error()), http.StatusBadRequest)
+		return
+	}
+
 	err = am.store.SetAlertConfig(r.Context(), cfgDesc)
 	if err != nil {
 		level.Error(logger).Log("msg", errStoringConfiguration, "err", err.Error())
@@ -118,3 +129,46 @@ func (am *MultitenantAlertmanager) DeleteUserConfig(w http.ResponseWriter, r *ht
 
 	w.WriteHeader(http.StatusOK)
 }
+
+func validateUserConfig(cfg alerts.AlertConfigDesc) error {
+	// Validation copied from: https://github.com/prometheus/alertmanager/blob/8e861c646bf67599a1704fc843c6a94d519ce312/cli/check_config.go#L65-L96
+	amCfg, err := config.Load(cfg.RawConfig)
+	if err != nil {
+		return err
+	}
+
+	// Create templates on disk in a temporary directory.
+	// Note: This means the validation will succeed if we can write to tmp but
+	// not to configured data dir, and on the flipside, it'll fail if we can't write
+	// to tmpDir. Ignoring both cases for now as they're ultra rare but will revisit if
+	// we see this in the wild.
+	tmpDir, err := ioutil.TempDir("", "validate-config")
+	if err != nil {
+		return err
+	}
+	defer os.RemoveAll(tmpDir)
+
+	for _, tmpl := range cfg.Templates {
+		_, err := createTemplateFile(tmpDir, cfg.User, tmpl.Filename, tmpl.Body)
+		if err != nil {
+			return err
+		}
+	}
+
+	templateFiles := make([]string, len(amCfg.Templates))
+	for i, t := range amCfg.Templates {
+		templateFiles[i] = filepath.Join(tmpDir, "templates", cfg.User, t)
+	}
+
+	_, err = template.FromGlobs(templateFiles...)
+	if err != nil {
+		return err
+	}
+
+	// Note: Not validating the MultitenantAlertmanager.transformConfig function as that
+	// that function shouldn't break configuration. Only way it can fail is if the base
+	// autoWebhookURL itself is broken. In that case, I would argue, we should accept the config
+	// not reject it.
+
+	return nil
+}

pkg/alertmanager/api_test.go

@@ -0,0 +1,115 @@
+package alertmanager
+
+import (
+	"bytes"
+	"context"
+	"io/ioutil"
+	"net/http"
+	"net/http/httptest"
+	"strings"
+	"testing"
+
+	"github.com/cortexproject/cortex/pkg/alertmanager/alerts"
+	"github.com/cortexproject/cortex/pkg/util"
+
+	"github.com/stretchr/testify/require"
+	"github.com/weaveworks/common/user"
+	"gopkg.in/yaml.v2"
+)
+
+func TestAMConfigValidationAPI(t *testing.T) {
+	testCases := []struct {
+		cfg    UserConfig
+		hasErr bool
+	}{
+		{
+			cfg: UserConfig{
+				AlertmanagerConfig: `
+route:
+  receiver: 'default-receiver'
+  group_wait: 30s
+  group_interval: 5m
+  repeat_interval: 4h
+  group_by: [cluster, alertname]
+`,
+			},
+			hasErr: true,
+		},
+		{
+			cfg: UserConfig{
+				AlertmanagerConfig: `
+route:
+  receiver: 'default-receiver'
+  group_wait: 30s
+  group_interval: 5m
+  repeat_interval: 4h
+  group_by: [cluster, alertname]
+receivers:
+  - name: default-receiver
+`,
+			},
+			hasErr: false,
+		},
+		{
+			cfg: UserConfig{
+				AlertmanagerConfig: `
+route:
+  receiver: 'default-receiver'
+  group_wait: 30s
+  group_interval: 5m
+  repeat_interval: 4h
+  group_by: [cluster, alertname]
+receivers:
+  - name: default-receiver
+`,
+				TemplateFiles: map[string]string{
+					"good.tpl":          "good-template",
+					"not/very/good.tpl": "bad-template", // Paths are not allowed in templates.
+				},
+			},
+			hasErr: true,
+		},
+	}
+
+	am := &MultitenantAlertmanager{
+		store:  noopAlertStore{},
+		logger: util.Logger,
+	}
+	for _, tc := range testCases {
+		payload, err := yaml.Marshal(&tc.cfg)
+		require.NoError(t, err)
+
+		req := httptest.NewRequest(http.MethodPost, "http://alertmanager/api/v1/alerts", bytes.NewReader(payload))
+		ctx := context.Background()
+		ctx = user.InjectOrgID(ctx, "testing")
+		require.NoError(t, user.InjectOrgIDIntoHTTPRequest(ctx, req))
+		w := httptest.NewRecorder()
+		am.SetUserConfig(w, req)
+
+		resp := w.Result()
+		if tc.hasErr {
+			require.Equal(t, 400, resp.StatusCode)
+			respBody, err := ioutil.ReadAll(resp.Body)
+			require.NoError(t, err)
+			require.True(t, strings.Contains(string(respBody), "error validating Alertmanager config"))
+		} else {
+			require.Equal(t, 201, resp.StatusCode)
+		}
+	}
+
+}
+
+type noopAlertStore struct{}
+
+func (noopAlertStore) ListAlertConfigs(ctx context.Context) (map[string]alerts.AlertConfigDesc, error) {
+	return nil, nil
+}
+func (noopAlertStore) GetAlertConfig(ctx context.Context, user string) (alerts.AlertConfigDesc, error) {
+	return alerts.AlertConfigDesc{}, nil
+}
+func (noopAlertStore) SetAlertConfig(ctx context.Context, cfg alerts.AlertConfigDesc) error {
+	return nil
+}
+func (noopAlertStore) DeleteAlertConfig(ctx context.Context, user string) error {
+	return nil
+}

pkg/alertmanager/multitenant.go

@@ -355,26 +355,6 @@ func (am *MultitenantAlertmanager) transformConfig(userID string, amConfig *amco
 	return amConfig, nil
 }
 
-func (am *MultitenantAlertmanager) createTemplatesFile(userID, fn, content string) (bool, error) {
-	dir := filepath.Join(am.cfg.DataDir, "templates", userID, filepath.Dir(fn))
-	err := os.MkdirAll(dir, 0755)
-	if err != nil {
-		return false, fmt.Errorf("unable to create Alertmanager templates directory %q: %s", dir, err)
-	}
-
-	file := filepath.Join(dir, fn)
-	// Check if the template file already exists and if it has changed
-	if tmpl, err := ioutil.ReadFile(file); err == nil && string(tmpl) == content {
-		return false, nil
-	}
-
-	if err := ioutil.WriteFile(file, []byte(content), 0644); err != nil {
-		return false, fmt.Errorf("unable to create Alertmanager template file %q: %s", file, err)
-	}
-
-	return true, nil
-}
-
 // setConfig applies the given configuration to the alertmanager for `userID`,
 // creating an alertmanager if it doesn't already exist.
 func (am *MultitenantAlertmanager) setConfig(cfg alerts.AlertConfigDesc) error {
@@ -386,7 +366,7 @@ func (am *MultitenantAlertmanager) setConfig(cfg alerts.AlertConfigDesc) error {
 	var hasTemplateChanges bool
 
 	for _, tmpl := range cfg.Templates {
-		hasChanged, err := am.createTemplatesFile(cfg.User, tmpl.Filename, tmpl.Body)
+		hasChanged, err := createTemplateFile(am.cfg.DataDir, cfg.User, tmpl.Filename, tmpl.Body)
 		if err != nil {
 			return err
 		}
@@ -506,3 +486,23 @@ func (s StatusHandler) ServeHTTP(w http.ResponseWriter, req *http.Request) {
 		http.Error(w, err.Error(), http.StatusInternalServerError)
 	}
 }
+
+func createTemplateFile(dataDir, userID, fn, content string) (bool, error) {
+	dir := filepath.Join(dataDir, "templates", userID, filepath.Dir(fn))
+	err := os.MkdirAll(dir, 0755)
+	if err != nil {
+		return false, fmt.Errorf("unable to create Alertmanager templates directory %q: %s", dir, err)
+	}
+
+	file := filepath.Join(dir, fn)
+	// Check if the template file already exists and if it has changed
+	if tmpl, err := ioutil.ReadFile(file); err == nil && string(tmpl) == content {
+		return false, nil
+	}
+
+	if err := ioutil.WriteFile(file, []byte(content), 0644); err != nil {
+		return false, fmt.Errorf("unable to create Alertmanager template file %q: %s", file, err)
+	}
+
+	return true, nil
+}