From 84d6e67e6b7268cbbb2a035ccbe0a023317f161f Mon Sep 17 00:00:00 2001
From: Daniel Lee <dan.limerick@gmail.com>
Date: Wed, 7 Mar 2018 16:20:05 +0100
Subject: [PATCH] alerting: fixes validation error when saving alerts in dash

If a panelId in the dashboard json is set to zero then the validation
silently fails. Instead of returning an error, it just ignores alerts and
saves the dashboard.

(cherry picked from commit d96fbb486f6f7ea452a314398ad56eb0e8130177)
---
 pkg/services/alerting/extractor.go            |  9 ++-
 pkg/services/alerting/extractor_test.go       | 16 +++++
 .../alerting/test-data/panel-with-id-0.json   | 63 +++++++++++++++++++
 3 files changed, 86 insertions(+), 2 deletions(-)
 create mode 100644 pkg/services/alerting/test-data/panel-with-id-0.json

diff --git a/pkg/services/alerting/extractor.go b/pkg/services/alerting/extractor.go
index a609824cbc8..5206c81642e 100644
--- a/pkg/services/alerting/extractor.go
+++ b/pkg/services/alerting/extractor.go
@@ -143,10 +143,15 @@ func (e *DashAlertExtractor) GetAlertFromPanels(jsonWithPanels *simplejson.Json)
 
 		// validate
 		_, err = NewRuleFromDBAlert(alert)
-		if err == nil && alert.ValidToSave() {
+		if err != nil {
+			return nil, err
+		}
+
+		if alert.ValidToSave() {
 			alerts = append(alerts, alert)
 		} else {
-			return nil, err
+			e.log.Debug("Invalid Alert Data. Dashboard, Org or Panel ID is not correct", "alertName", alert.Name, "panelId", alert.PanelId)
+			return nil, m.ErrDashboardContainsInvalidAlertData
 		}
 	}
 
diff --git a/pkg/services/alerting/extractor_test.go b/pkg/services/alerting/extractor_test.go
index 71f3026025d..f8b678e66bd 100644
--- a/pkg/services/alerting/extractor_test.go
+++ b/pkg/services/alerting/extractor_test.go
@@ -150,6 +150,22 @@ func TestAlertRuleExtraction(t *testing.T) {
 			})
 		})
 
+		Convey("Panel with id set to zero should return error", func() {
+			panelWithIdZero, err := ioutil.ReadFile("./test-data/panel-with-id-0.json")
+			So(err, ShouldBeNil)
+
+			dashJson, err := simplejson.NewJson([]byte(panelWithIdZero))
+			So(err, ShouldBeNil)
+			dash := m.NewDashboardFromJson(dashJson)
+			extractor := NewDashAlertExtractor(dash, 1)
+
+			_, err = extractor.GetAlerts()
+
+			Convey("panel with id 0 should return error", func() {
+				So(err, ShouldNotBeNil)
+			})
+		})
+
 		Convey("Parse alerts from dashboard without rows", func() {
 			json, err := ioutil.ReadFile("./test-data/v5-dashboard.json")
 			So(err, ShouldBeNil)
diff --git a/pkg/services/alerting/test-data/panel-with-id-0.json b/pkg/services/alerting/test-data/panel-with-id-0.json
new file mode 100644
index 00000000000..d1f314a4f55
--- /dev/null
+++ b/pkg/services/alerting/test-data/panel-with-id-0.json
@@ -0,0 +1,63 @@
+{
+  "id": 57,
+  "title": "Graphite 4",
+  "originalTitle": "Graphite 4",
+  "tags": ["graphite"],
+  "rows": [
+  {
+    "panels": [
+    {
+      "title": "Active desktop users",
+      "id": 0,
+      "editable": true,
+      "type": "graph",
+      "targets": [
+      {
+        "refId": "A",
+        "target": "aliasByNode(statsd.fakesite.counters.session_start.desktop.count, 4)"
+      }
+      ],
+      "datasource": null,
+      "alert": {
+        "name": "name1",
+        "message": "desc1",
+        "handler": 1,
+        "frequency": "60s",
+        "conditions": [
+        {
+          "type": "query",
+          "query": {"params": ["A", "5m", "now"]},
+          "reducer": {"type": "avg", "params": []},
+          "evaluator": {"type": ">", "params": [100]}
+        }
+        ]
+      }
+    },
+    {
+      "title": "Active mobile users",
+      "id": 4,
+      "targets": [
+        {"refId": "A", "target": ""},
+        {"refId": "B", "target": "aliasByNode(statsd.fakesite.counters.session_start.mobile.count, 4)"}
+      ],
+      "datasource": "graphite2",
+      "alert": {
+        "name": "name2",
+        "message": "desc2",
+        "handler": 0,
+        "frequency": "60s",
+        "severity": "warning",
+        "conditions": [
+        {
+          "type": "query",
+          "query":  {"params": ["B", "5m", "now"]},
+          "reducer": {"type": "avg", "params": []},
+          "evaluator": {"type": ">", "params": [100]}
+        }
+        ]
+      }
+    }
+    ]
+  }
+]
+      }