Alerting: Don't stop the migration when alert rule tags are invalid (#51253)

* Alerting: Don't stop the migration when alert rule tags are invalid

As we migrate we expect the `alertRuleTags` on a dashboard alert to be a JSON object. However, it seems this is not really validated by Grafana and an user can change the format to something else that the JSON parser is not able to marshal into a `map[string]string`.

Let's do a bit better by "attempting" to parse the tags and if we can't we'll simple return an empty map. The data is still there so if the user wishes they can go back, fix the data and attemp the migration again.

pkg/services/ngalert/CHANGELOG.md

@@ -51,6 +51,7 @@ Scopes must have an order to ensure consistency and ease of search, this helps u
 - [BUGFIX] State manager to use tick time to determine stale states #50991
 - [ENHANCEMENT] Scheduler: Drop ticks if rule evaluation is too slow and adds a metric grafana_alerting_schedule_rule_evaluations_missed_total to track missed evaluations per rule #48885
 - [ENHANCEMENT] Ticker to tick at predictable time #50197
+- [ENHANCEMENT] Migration: Don't stop the migration when failing to parse alert rule tags #51253
 
 ## 9.0.0
 

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

@@ -5,6 +5,7 @@ import (
 	"fmt"
 	"time"
 
+	"github.com/grafana/grafana/pkg/components/simplejson"
 	"github.com/grafana/grafana/pkg/expr"
 	legacymodels "github.com/grafana/grafana/pkg/models"
 	ngmodels "github.com/grafana/grafana/pkg/services/ngalert/models"
@@ -81,9 +82,11 @@ func (a *alertRule) makeVersion() *alertRuleVersion {
 }
 
 func addMigrationInfo(da *dashAlert) (map[string]string, map[string]string) {
-	lbls := da.ParsedSettings.AlertRuleTags
-	if lbls == nil {
-		lbls = make(map[string]string)
+	tagsMap := simplejson.NewFromAny(da.ParsedSettings.AlertRuleTags).MustMap()
+	lbls := make(map[string]string, len(tagsMap))
+
+	for k, v := range tagsMap {
+		lbls[k] = simplejson.NewFromAny(v).MustString()
 	}
 
 	annotations := make(map[string]string, 3)

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

@@ -1,6 +1,7 @@
 package ualert
 
 import (
+	"encoding/json"
 	"testing"
 
 	"github.com/grafana/grafana/pkg/components/simplejson"
@@ -50,3 +51,36 @@ func TestMigrateAlertRuleQueries(t *testing.T) {
 		})
 	}
 }
+
+func TestAddMigrationInfo(t *testing.T) {
+	tt := []struct {
+		name                string
+		tagsJSON            string
+		expectedLabels      map[string]string
+		expectedAnnotations map[string]string
+	}{
+		{
+			name:                "when alert rule tags are a JSON array, they're ignored.",
+			tagsJSON:            `{ "alertRuleTags": ["one", "two", "three", "four"] }`,
+			expectedLabels:      map[string]string{},
+			expectedAnnotations: map[string]string{"__alertId__": "0", "__dashboardUid__": "", "__panelId__": "0"},
+		},
+		{
+			name:                "when alert rule tags are a JSON object",
+			tagsJSON:            `{ "alertRuleTags": { "key": "value", "key2": "value2" } }`,
+			expectedLabels:      map[string]string{"key": "value", "key2": "value2"},
+			expectedAnnotations: map[string]string{"__alertId__": "0", "__dashboardUid__": "", "__panelId__": "0"},
+		},
+	}
+
+	for _, tc := range tt {
+		t.Run(tc.name, func(t *testing.T) {
+			var settings dashAlertSettings
+			require.NoError(t, json.Unmarshal([]byte(tc.tagsJSON), &settings))
+
+			labels, annotations := addMigrationInfo(&dashAlert{ParsedSettings: &settings})
+			require.Equal(t, tc.expectedLabels, labels)
+			require.Equal(t, tc.expectedAnnotations, annotations)
+		})
+	}
+}

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

@@ -40,7 +40,7 @@ WHERE org_id IN (SELECT id from org)
 	AND dashboard_id IN (SELECT id from dashboard)
 `
 
-// slurpDashAlerts loads all alerts from the alert database table into the
+// slurpDashAlerts loads all alerts from the alert database table into
 // the dashAlert type. If there are alerts that belong to either organization or dashboard that does not exist, those alerts will not be returned/
 // Additionally it unmarshals the json settings for the alert into the
 // ParsedSettings property of the dash alert.
@@ -68,7 +68,7 @@ type dashAlertSettings struct {
 	NoDataState         string               `json:"noDataState"`
 	ExecutionErrorState string               `json:"executionErrorState"`
 	Conditions          []dashAlertCondition `json:"conditions"`
-	AlertRuleTags       map[string]string    `json:"alertRuleTags"`
+	AlertRuleTags       interface{}          `json:"alertRuleTags"`
 	Notifications       []dashAlertNot       `json:"notifications"`
 }