Alerting: Modify configuration apply and save semantics - v2 (#34143)

* Save default configuration to the database and copy over secure settings

pkg/services/ngalert/api/api_alertmanager.go

@@ -1,7 +1,9 @@
 package api
 
 import (
+	"encoding/base64"
 	"errors"
+	"fmt"
 	"net/http"
 
 	"github.com/grafana/grafana/pkg/api/response"
@@ -11,6 +13,7 @@ import (
 	ngmodels "github.com/grafana/grafana/pkg/services/ngalert/models"
 	"github.com/grafana/grafana/pkg/services/ngalert/notifier"
 	"github.com/grafana/grafana/pkg/services/ngalert/store"
+	"github.com/grafana/grafana/pkg/setting"
 	"github.com/grafana/grafana/pkg/util"
 )
 
@@ -177,13 +180,68 @@ func (srv AlertmanagerSrv) RoutePostAlertingConfig(c *models.ReqContext, body ap
 	if !c.HasUserRole(models.ROLE_EDITOR) {
 		return response.Error(http.StatusForbidden, "Permission denied", nil)
 	}
-	err := body.EncryptSecureSettings()
+
+	// Get the last known working configuration
+	query := ngmodels.GetLatestAlertmanagerConfigurationQuery{}
+	if err := srv.store.GetLatestAlertmanagerConfiguration(&query); err != nil {
+		// If we don't have a configuration there's nothing for us to know and we should just continue saving the new one
+		if !errors.Is(err, store.ErrNoAlertmanagerConfiguration) {
+			return response.Error(http.StatusInternalServerError, "failed to get latest configuration", err)
+		}
+	}
+
+	currentConfig, err := notifier.Load([]byte(query.Result.AlertmanagerConfiguration))
 	if err != nil {
+		return response.Error(http.StatusInternalServerError, "failed to load lastest configuration", err)
+	}
+
+	// Copy the previously known secure settings
+	for i, r := range body.AlertmanagerConfig.Receivers {
+		for j, gr := range r.PostableGrafanaReceivers.GrafanaManagedReceivers {
+			if len(currentConfig.AlertmanagerConfig.Receivers) <= i { // this is a receiver we don't have any stored for - skip it.
+				continue
+			}
+			cr := currentConfig.AlertmanagerConfig.Receivers[i]
+
+			if len(cr.PostableGrafanaReceivers.GrafanaManagedReceivers) <= j { //  this is a receiver we don't have anything stored for - skip it.
+				continue
+			}
+			cgmr := cr.PostableGrafanaReceivers.GrafanaManagedReceivers[j]
+
+			//TODO: We use the name and type to match current stored receivers againt sent ones, but we should ideally use something unique e.g. UUID
+			if cgmr.Name == gr.Name && cgmr.Type == gr.Type {
+				// frontend sends only the secure settings that have to be updated
+				// therefore we have to copy from the last configuration only those secure settings not included in the request
+				for key, storedValue := range cgmr.SecureSettings {
+					_, ok := body.AlertmanagerConfig.Receivers[i].PostableGrafanaReceivers.GrafanaManagedReceivers[j].SecureSettings[key]
+					if !ok {
+						decodeValue, err := base64.StdEncoding.DecodeString(storedValue)
+						if err != nil {
+							return response.Error(http.StatusInternalServerError, fmt.Sprintf("failed to decode stored secure setting: %s", key), err)
+						}
+						decryptedValue, err := util.Decrypt(decodeValue, setting.SecretKey)
+						if err != nil {
+							return response.Error(http.StatusInternalServerError, fmt.Sprintf("failed to decrypt stored secure setting: %s", key), err)
+						}
+
+						if body.AlertmanagerConfig.Receivers[i].PostableGrafanaReceivers.GrafanaManagedReceivers[j].SecureSettings == nil {
+							body.AlertmanagerConfig.Receivers[i].PostableGrafanaReceivers.GrafanaManagedReceivers[j].SecureSettings = make(map[string]string, len(cgmr.SecureSettings))
+						}
+
+						body.AlertmanagerConfig.Receivers[i].PostableGrafanaReceivers.GrafanaManagedReceivers[j].SecureSettings[key] = string(decryptedValue)
+					}
+				}
+			}
+		}
+	}
+
+	if err := body.EncryptSecureSettings(); err != nil {
 		return response.Error(http.StatusInternalServerError, "failed to encrypt receiver secrets", err)
 	}
 
 	if err := srv.am.SaveAndApplyConfig(&body); err != nil {
-		return response.Error(http.StatusInternalServerError, "failed to save and apply Alertmanager configuration", err)
+		srv.log.Error("unable to save and apply alertmanager configuration", "err", err)
+		return response.Error(http.StatusBadRequest, "failed to save and apply Alertmanager configuration", err)
 	}
 
 	return response.JSON(http.StatusAccepted, util.DynMap{"message": "configuration created"})

pkg/services/ngalert/api/tooling/definitions/alertmanager.go

@@ -447,6 +447,21 @@ func (c *PostableApiAlertingConfig) validate() error {
 		return fmt.Errorf("cannot mix Alertmanager & Grafana receiver types")
 	}
 
+	if hasGrafReceivers {
+		// Taken from https://github.com/prometheus/alertmanager/blob/master/config/config.go#L170-L191
+		// Check if we have a root route. We cannot check for it in the
+		// UnmarshalYAML method because it won't be called if the input is empty
+		// (e.g. the config file is empty or only contains whitespace).
+		if c.Route == nil {
+			return fmt.Errorf("no route provided in config")
+		}
+
+		// Check if continue in root route.
+		if c.Route.Continue {
+			return fmt.Errorf("cannot have continue in root route")
+		}
+	}
+
 	for _, receiver := range AllReceivers(c.Route) {
 		_, ok := receivers[receiver]
 		if !ok {
@@ -475,9 +490,14 @@ func (c *PostableApiAlertingConfig) ReceiverType() ReceiverType {
 // AllReceivers will recursively walk a routing tree and return a list of all the
 // referenced receiver names.
 func AllReceivers(route *config.Route) (res []string) {
+	if route == nil {
+		return res
+	}
+
 	if route.Receiver != "" {
 		res = append(res, route.Receiver)
 	}
+
 	for _, subRoute := range route.Routes {
 		res = append(res, AllReceivers(subRoute)...)
 	}

pkg/services/ngalert/models/alertmanager.go

@@ -11,6 +11,7 @@ type AlertConfiguration struct {
 	AlertmanagerConfiguration string
 	ConfigurationVersion      string
 	CreatedAt                 time.Time `xorm:"created"`
+	Default                   bool
 }
 
 // GetLatestAlertmanagerConfigurationQuery is the query to get the latest alertmanager configuration.
@@ -22,6 +23,7 @@ type GetLatestAlertmanagerConfigurationQuery struct {
 type SaveAlertmanagerConfigurationCmd struct {
 	AlertmanagerConfiguration string
 	ConfigurationVersion      string
+	Default                   bool
 }
 
 type DeleteAlertmanagerConfigurationCmd struct {

pkg/services/ngalert/notifier/alertmanager.go

@@ -187,6 +187,8 @@ func (am *Alertmanager) StopAndWait() error {
 	return nil
 }
 
+// SaveAndApplyConfig saves the configuration the database and applies the configuration to the Alertmanager.
+// It rollbacks the save if we fail to apply the configuration.
 func (am *Alertmanager) SaveAndApplyConfig(cfg *apimodels.PostableUserConfig) error {
 	rawConfig, err := json.Marshal(&cfg)
 	if err != nil {
@@ -201,11 +203,14 @@ func (am *Alertmanager) SaveAndApplyConfig(cfg *apimodels.PostableUserConfig) er
 		ConfigurationVersion:      fmt.Sprintf("v%d", ngmodels.AlertConfigurationVersion),
 	}
 
-	if err := am.Store.SaveAlertmanagerConfiguration(cmd); err != nil {
-		return fmt.Errorf("failed to save Alertmanager configuration: %w", err)
-	}
-	if err := am.applyConfig(cfg, rawConfig); err != nil {
-		return fmt.Errorf("unable to reload configuration: %w", err)
+	err = am.Store.SaveAlertmanagerConfigurationWithCallback(cmd, func() error {
+		if err := am.applyConfig(cfg, rawConfig); err != nil {
+			return err
+		}
+		return nil
+	})
+	if err != nil {
+		return err
 	}
 
 	return nil
@@ -222,7 +227,18 @@ func (am *Alertmanager) SyncAndApplyConfigFromDatabase() error {
 	if err := am.Store.GetLatestAlertmanagerConfiguration(q); err != nil {
 		// If there's no configuration in the database, let's use the default configuration.
 		if errors.Is(err, store.ErrNoAlertmanagerConfiguration) {
-			q.Result = &ngmodels.AlertConfiguration{AlertmanagerConfiguration: alertmanagerDefaultConfiguration}
+			// First, let's save it to the database. We don't need to use a transaction here as we'll always succeed.
+			am.logger.Info("no Alertmanager configuration found, saving and applying a default")
+			savecmd := &ngmodels.SaveAlertmanagerConfigurationCmd{
+				AlertmanagerConfiguration: alertmanagerDefaultConfiguration,
+				Default:                   true,
+				ConfigurationVersion:      fmt.Sprintf("v%d", ngmodels.AlertConfigurationVersion),
+			}
+			if err := am.Store.SaveAlertmanagerConfiguration(savecmd); err != nil {
+				return err
+			}
+
+			q.Result = &ngmodels.AlertConfiguration{AlertmanagerConfiguration: alertmanagerDefaultConfiguration, Default: true}
 		} else {
 			return fmt.Errorf("unable to get Alertmanager configuration from the database: %w", err)
 		}

pkg/services/ngalert/store/alertmanager.go

@@ -42,15 +42,29 @@ func (st *DBstore) GetLatestAlertmanagerConfiguration(query *models.GetLatestAle
 }
 
 // SaveAlertmanagerConfiguration creates an alertmanager configuration.
-func (st *DBstore) SaveAlertmanagerConfiguration(cmd *models.SaveAlertmanagerConfigurationCmd) error {
-	return st.SQLStore.WithDbSession(context.Background(), func(sess *sqlstore.DBSession) error {
+func (st DBstore) SaveAlertmanagerConfiguration(cmd *models.SaveAlertmanagerConfigurationCmd) error {
+	return st.SaveAlertmanagerConfigurationWithCallback(cmd, func() error { return nil })
+}
+
+type SaveCallback func() error
+
+// SaveAlertmanagerConfigurationWithCallback creates an alertmanager configuration version and then executes a callback.
+// If the callback results in error in rollsback the transaction.
+func (st DBstore) SaveAlertmanagerConfigurationWithCallback(cmd *models.SaveAlertmanagerConfigurationCmd, callback SaveCallback) error {
+	return st.SQLStore.WithTransactionalDbSession(context.Background(), func(sess *sqlstore.DBSession) error {
 		config := models.AlertConfiguration{
 			AlertmanagerConfiguration: cmd.AlertmanagerConfiguration,
 			ConfigurationVersion:      cmd.ConfigurationVersion,
+			Default:                   cmd.Default,
 		}
 		if _, err := sess.Insert(config); err != nil {
 			return err
 		}
+
+		if err := callback(); err != nil {
+			return err
+		}
+
 		return nil
 	})
 }

pkg/services/ngalert/store/database.go

@@ -18,6 +18,7 @@ const AlertDefinitionMaxTitleLength = 190
 type AlertingStore interface {
 	GetLatestAlertmanagerConfiguration(*models.GetLatestAlertmanagerConfigurationQuery) error
 	SaveAlertmanagerConfiguration(*models.SaveAlertmanagerConfigurationCmd) error
+	SaveAlertmanagerConfigurationWithCallback(*models.SaveAlertmanagerConfigurationCmd, SaveCallback) error
 }
 
 // DBstore stores the alert definitions and instances in the database.

pkg/services/sqlstore/migrations/ualert/tables.go

@@ -251,4 +251,7 @@ func AddAlertmanagerConfigMigrations(mg *migrator.Migrator) {
 	}
 
 	mg.AddMigration("create_alert_configuration_table", migrator.NewAddTableMigration(alertConfiguration))
+	mg.AddMigration("Add column default in alert_configuration", migrator.NewAddColumnMigration(alertConfiguration, &migrator.Column{
+		Name: "default", Type: migrator.DB_Bool, Nullable: false, Default: "0",
+	}))
 }

pkg/tests/api/alerting/api_alertmanager_configuration_test.go

@@ -0,0 +1,178 @@
+package alerting
+
+import (
+	"fmt"
+	"net/http"
+	"testing"
+
+	"github.com/grafana/grafana/pkg/models"
+	"github.com/grafana/grafana/pkg/tests/testinfra"
+	"github.com/stretchr/testify/require"
+)
+
+func TestAlertmanagerConfigurationIsTransactional(t *testing.T) {
+	dir, path := testinfra.CreateGrafDir(t, testinfra.GrafanaOpts{
+		EnableFeatureToggles: []string{"ngalert"},
+		AnonymousUserRole:    models.ROLE_EDITOR,
+	})
+
+	store := testinfra.SetUpDatabase(t, dir)
+	grafanaListedAddr := testinfra.StartGrafana(t, dir, path, store)
+	alertConfigURL := fmt.Sprintf("http://%s/api/alertmanager/grafana/config/api/v1/alerts", grafanaListedAddr)
+
+	// On a blank start with no configuration, it saves and delivers the default configuration.
+	{
+		resp := getRequest(t, alertConfigURL, http.StatusOK) // nolint
+		require.JSONEq(t, defaultAlertmanagerConfigJSON, getBody(t, resp.Body))
+	}
+
+	// When creating new configuration, if it fails to apply - it does not save it.
+	{
+		payload := `
+{
+	"template_files": {},
+	"alertmanager_config": {
+		"route": {
+			"receiver": "slack.receiver"
+		},
+		"templates": null,
+		"receivers": [{
+			"name": "slack.receiver",
+			"grafana_managed_receiver_configs": [{
+				"settings": {
+					"iconEmoji": "",
+					"iconUrl": "",
+					"mentionGroups": "",
+					"mentionUsers": "",
+					"recipient": "#unified-alerting-test",
+					"username": ""
+				},
+				"secureSettings": {},
+				"type": "slack",
+				"sendReminder": true,
+				"name": "slack.receiver",
+				"disableResolveMessage": false,
+				"uid": ""
+			}]
+		}]
+	}
+}
+`
+		resp := postRequest(t, alertConfigURL, payload, http.StatusBadRequest) // nolint
+		require.JSONEq(t, "{\"error\":\"alert validation error: token must be specified when using the Slack chat API\", \"message\":\"failed to save and apply Alertmanager configuration\"}", getBody(t, resp.Body))
+
+		resp = getRequest(t, alertConfigURL, http.StatusOK) // nolint
+		require.JSONEq(t, defaultAlertmanagerConfigJSON, getBody(t, resp.Body))
+	}
+}
+
+func TestAlertmanagerConfigurationPersistSecrets(t *testing.T) {
+	dir, path := testinfra.CreateGrafDir(t, testinfra.GrafanaOpts{
+		EnableFeatureToggles: []string{"ngalert"},
+		AnonymousUserRole:    models.ROLE_EDITOR,
+	})
+
+	store := testinfra.SetUpDatabase(t, dir)
+	grafanaListedAddr := testinfra.StartGrafana(t, dir, path, store)
+	alertConfigURL := fmt.Sprintf("http://%s/api/alertmanager/grafana/config/api/v1/alerts", grafanaListedAddr)
+
+	// create a new configuration that has a secret
+	{
+		payload := `
+{
+	"template_files": {},
+	"alertmanager_config": {
+		"route": {
+			"receiver": "slack.receiver"
+		},
+		"templates": null,
+		"receivers": [{
+			"name": "slack.receiver",
+			"grafana_managed_receiver_configs": [{
+				"settings": {
+					"recipient": "#unified-alerting-test"
+				},
+				"secureSettings": {
+					"url": "http://averysecureurl.com/webhook"
+				},
+				"type": "slack",
+				"sendReminder": true,
+				"name": "slack.receiver",
+				"disableResolveMessage": false
+			}]
+		}]
+	}
+}
+`
+		resp := postRequest(t, alertConfigURL, payload, http.StatusAccepted) // nolint
+		require.JSONEq(t, `{"message":"configuration created"}`, getBody(t, resp.Body))
+	}
+	// Then, update the recipient
+	{
+		payload := `
+{
+	"template_files": {},
+	"alertmanager_config": {
+		"route": {
+			"receiver": "slack.receiver"
+		},
+		"templates": null,
+		"receivers": [{
+			"name": "slack.receiver",
+			"grafana_managed_receiver_configs": [{
+				"settings": {
+					"recipient": "#unified-alerting-test-but-updated"
+				},
+				"secureFields": {
+					"url": true
+				},
+				"type": "slack",
+				"sendReminder": true,
+				"name": "slack.receiver",
+				"disableResolveMessage": false
+			}]
+		}]
+	}
+}
+`
+		resp := postRequest(t, alertConfigURL, payload, http.StatusAccepted) // nolint
+		require.JSONEq(t, `{"message": "configuration created"}`, getBody(t, resp.Body))
+	}
+
+	// The secure settings must be present
+	{
+		resp := getRequest(t, alertConfigURL, http.StatusOK) // nolint
+		require.JSONEq(t, `
+{
+	"template_files": {},
+	"alertmanager_config": {
+		"route": {
+			"receiver": "slack.receiver"
+		},
+		"templates": null,
+		"receivers": [{
+			"name": "slack.receiver",
+			"grafana_managed_receiver_configs": [{
+				"id": 0,
+				"uid": "",
+				"name": "slack.receiver",
+				"type": "slack",
+				"isDefault": false,
+				"sendReminder": true,
+				"disableResolveMessage": false,
+				"frequency": "",
+				"created": "0001-01-01T00:00:00Z",
+				"updated": "0001-01-01T00:00:00Z",
+				"settings": {
+					"recipient": "#unified-alerting-test-but-updated"
+				},
+				"secureFields": {
+					"url": true
+				}
+			}]
+		}]
+	}
+}
+`, getBody(t, resp.Body))
+	}
+}

pkg/tests/api/alerting/api_notification_channel_test.go

@@ -1,10 +1,8 @@
 package alerting
 
 import (
-	"bytes"
 	"encoding/json"
 	"fmt"
-	"io"
 	"io/ioutil"
 	"net/http"
 	"regexp"
@@ -49,9 +47,12 @@ func TestNotificationChannels(t *testing.T) {
 	require.NoError(t, createUser(t, s, models.ROLE_EDITOR, "grafana", "password"))
 
 	{
-		// There are no notification channel config initially.
+		// There are no notification channel config initially - so it returns the default configuration.
 		alertsURL := fmt.Sprintf("http://grafana:password@%s/api/alertmanager/grafana/config/api/v1/alerts", grafanaListedAddr)
-		_ = getRequest(t, alertsURL, http.StatusNotFound) // nolint
+		resp := getRequest(t, alertsURL, http.StatusOK) // nolint
+		b, err := ioutil.ReadAll(resp.Body)
+		require.NoError(t, err)
+		require.JSONEq(t, defaultAlertmanagerConfigJSON, string(b))
 	}
 
 	{
@@ -60,7 +61,7 @@ func TestNotificationChannels(t *testing.T) {
 
 		// Post the alertmanager config.
 		u := fmt.Sprintf("http://grafana:password@%s/api/alertmanager/grafana/config/api/v1/alerts", grafanaListedAddr)
-		postRequest(t, u, amConfig, http.StatusAccepted)
+		_ = postRequest(t, u, amConfig, http.StatusAccepted) // nolint
 
 		// Verifying that all the receivers and routes have been registered.
 		alertsURL := fmt.Sprintf("http://grafana:password@%s/api/alertmanager/grafana/config/api/v1/alerts", grafanaListedAddr)
@@ -82,7 +83,7 @@ func TestNotificationChannels(t *testing.T) {
 
 		rulesConfig := getRulesConfig(t)
 		u := fmt.Sprintf("http://grafana:password@%s/api/ruler/grafana/api/v1/rules/default", grafanaListedAddr)
-		postRequest(t, u, rulesConfig, http.StatusAccepted)
+		_ = postRequest(t, u, rulesConfig, http.StatusAccepted) // nolint
 	}
 
 	// Eventually, we'll get all the desired alerts.
@@ -148,30 +149,6 @@ func getRulesConfig(t *testing.T) string {
 	return string(b)
 }
 
-func getRequest(t *testing.T, url string, expStatusCode int) *http.Response {
-	t.Helper()
-	// nolint:gosec
-	resp, err := http.Get(url)
-	t.Cleanup(func() {
-		require.NoError(t, resp.Body.Close())
-	})
-	require.NoError(t, err)
-	require.Equal(t, expStatusCode, resp.StatusCode)
-	return resp
-}
-
-func postRequest(t *testing.T, url string, body string, expStatusCode int) {
-	t.Helper()
-	buf := bytes.NewReader([]byte(body))
-	// nolint:gosec
-	resp, err := http.Post(url, "application/json", buf)
-	t.Cleanup(func() {
-		require.NoError(t, resp.Body.Close())
-	})
-	require.NoError(t, err)
-	require.Equal(t, expStatusCode, resp.StatusCode)
-}
-
 type mockNotificationChannel struct {
 	t      *testing.T
 	server *http.Server
@@ -214,13 +191,6 @@ func (nc *mockNotificationChannel) ServeHTTP(res http.ResponseWriter, req *http.
 	res.WriteHeader(http.StatusOK)
 }
 
-func getBody(t *testing.T, body io.ReadCloser) string {
-	t.Helper()
-	b, err := ioutil.ReadAll(body)
-	require.NoError(t, err)
-	return string(b)
-}
-
 func (nc *mockNotificationChannel) totalNotifications() int {
 	total := 0
 	nc.receivedNotificationsMtx.Lock()

pkg/tests/api/alerting/testing.go

@@ -0,0 +1,82 @@
+package alerting
+
+import (
+	"bytes"
+	"io"
+	"io/ioutil"
+	"net/http"
+	"testing"
+
+	"github.com/stretchr/testify/require"
+)
+
+const defaultAlertmanagerConfigJSON = `
+{
+	"template_files": null,
+	"alertmanager_config": {
+		"route": {
+			"receiver": "grafana-default-email"
+		},
+		"templates": null,
+		"receivers": [{
+			"name": "grafana-default-email",
+			"grafana_managed_receiver_configs": [{
+				"id": 0,
+				"uid": "",
+				"name": "email receiver",
+				"type": "email",
+				"isDefault": true,
+				"sendReminder": false,
+				"disableResolveMessage": false,
+				"frequency": "",
+				"created": "0001-01-01T00:00:00Z",
+				"updated": "0001-01-01T00:00:00Z",
+				"settings": {
+					"addresses": "\u003cexample@email.com\u003e"
+				},
+				"secureFields": {}
+			}]
+		}]
+	}
+}
+`
+
+func getRequest(t *testing.T, url string, expStatusCode int) *http.Response {
+	t.Helper()
+	// nolint:gosec
+	resp, err := http.Get(url)
+	t.Cleanup(func() {
+		require.NoError(t, resp.Body.Close())
+	})
+	require.NoError(t, err)
+	if expStatusCode != resp.StatusCode {
+		b, err := ioutil.ReadAll(resp.Body)
+		require.NoError(t, err)
+		t.Fatal(string(b))
+	}
+	return resp
+}
+
+func postRequest(t *testing.T, url string, body string, expStatusCode int) *http.Response {
+	t.Helper()
+	buf := bytes.NewReader([]byte(body))
+	// nolint:gosec
+	resp, err := http.Post(url, "application/json", buf)
+	t.Cleanup(func() {
+		require.NoError(t, resp.Body.Close())
+	})
+	require.NoError(t, err)
+	if expStatusCode != resp.StatusCode {
+		b, err := ioutil.ReadAll(resp.Body)
+		require.NoError(t, err)
+		t.Fatal(string(b))
+	}
+	return resp
+}
+
+func getBody(t *testing.T, body io.ReadCloser) string {
+	t.Helper()
+	b, err := ioutil.ReadAll(body)
+	require.NoError(t, err)
+	return string(b)
+}