Merge pull request #14441 from cinaglia/delete-external-snapshot

Add support for deleting external snapshots from Snapshots List

pkg/api/dashboard_snapshot.go

 package api
 
 import (
+	"bytes"
+	"encoding/json"
+	"fmt"
+	"net/http"
 	"time"
 
 	"github.com/grafana/grafana/pkg/api/dtos"
 	"github.com/grafana/grafana/pkg/bus"
+	"github.com/grafana/grafana/pkg/components/simplejson"
 	"github.com/grafana/grafana/pkg/metrics"
 	m "github.com/grafana/grafana/pkg/models"
 	"github.com/grafana/grafana/pkg/services/guardian"
@@ -12,6 +17,11 @@ import (
 	"github.com/grafana/grafana/pkg/util"
 )
 
+var client = &http.Client{
+	Timeout:   time.Second * 5,
+	Transport: &http.Transport{Proxy: http.ProxyFromEnvironment},
+}
+
 func GetSharingOptions(c *m.ReqContext) {
 	c.JSON(200, util.DynMap{
 		"externalSnapshotURL":  setting.ExternalSnapshotUrl,
@@ -20,26 +30,79 @@ func GetSharingOptions(c *m.ReqContext) {
 	})
 }
 
+type CreateExternalSnapshotResponse struct {
+	Key       string `json:"key"`
+	DeleteKey string `json:"deleteKey"`
+	Url       string `json:"url"`
+	DeleteUrl string `json:"deleteUrl"`
+}
+
+func createExternalDashboardSnapshot(cmd m.CreateDashboardSnapshotCommand) (*CreateExternalSnapshotResponse, error) {
+	var createSnapshotResponse CreateExternalSnapshotResponse
+	message := map[string]interface{}{
+		"name":      cmd.Name,
+		"expires":   cmd.Expires,
+		"dashboard": cmd.Dashboard,
+	}
+
+	messageBytes, err := simplejson.NewFromAny(message).Encode()
+	if err != nil {
+		return nil, err
+	}
+
+	response, err := client.Post(setting.ExternalSnapshotUrl+"/api/snapshots", "application/json", bytes.NewBuffer(messageBytes))
+	if err != nil {
+		return nil, err
+	}
+	defer response.Body.Close()
+
+	if response.StatusCode != 200 {
+		return nil, fmt.Errorf("Create external snapshot response status code %d", response.StatusCode)
+	}
+
+	if err := json.NewDecoder(response.Body).Decode(&createSnapshotResponse); err != nil {
+		return nil, err
+	}
+
+	return &createSnapshotResponse, nil
+}
+
+// POST /api/snapshots
 func CreateDashboardSnapshot(c *m.ReqContext, cmd m.CreateDashboardSnapshotCommand) {
 	if cmd.Name == "" {
 		cmd.Name = "Unnamed snapshot"
 	}
 
+	var url string
+	cmd.ExternalUrl = ""
+	cmd.OrgId = c.OrgId
+	cmd.UserId = c.UserId
+
 	if cmd.External {
-		// external snapshot ref requires key and delete key
-		if cmd.Key == "" || cmd.DeleteKey == "" {
-			c.JsonApiErr(400, "Missing key and delete key for external snapshot", nil)
+		if !setting.ExternalEnabled {
+			c.JsonApiErr(403, "External dashboard creation is disabled", nil)
+			return
+		}
+
+		response, err := createExternalDashboardSnapshot(cmd)
+		if err != nil {
+			c.JsonApiErr(500, "Failed to create external snaphost", err)
 			return
 		}
 
-		cmd.OrgId = -1
-		cmd.UserId = -1
+		url = response.Url
+		cmd.Key = response.Key
+		cmd.DeleteKey = response.DeleteKey
+		cmd.ExternalUrl = response.Url
+		cmd.ExternalDeleteUrl = response.DeleteUrl
+		cmd.Dashboard = simplejson.New()
+
 		metrics.M_Api_Dashboard_Snapshot_External.Inc()
 	} else {
 		cmd.Key = util.GetRandomString(32)
 		cmd.DeleteKey = util.GetRandomString(32)
-		cmd.OrgId = c.OrgId
-		cmd.UserId = c.UserId
+		url = setting.ToAbsUrl("dashboard/snapshot/" + cmd.Key)
+
 		metrics.M_Api_Dashboard_Snapshot_Create.Inc()
 	}
 
@@ -51,7 +114,7 @@ func CreateDashboardSnapshot(c *m.ReqContext, cmd m.CreateDashboardSnapshotComma
 	c.JSON(200, util.DynMap{
 		"key":       cmd.Key,
 		"deleteKey": cmd.DeleteKey,
-		"url":       setting.ToAbsUrl("dashboard/snapshot/" + cmd.Key),
+		"url":       url,
 		"deleteUrl": setting.ToAbsUrl("api/snapshots-delete/" + cmd.DeleteKey),
 	})
 }
@@ -91,6 +154,33 @@ func GetDashboardSnapshot(c *m.ReqContext) {
 	c.JSON(200, dto)
 }
 
+func deleteExternalDashboardSnapshot(externalUrl string) error {
+	response, err := client.Get(externalUrl)
+	if err != nil {
+		return err
+	}
+	defer response.Body.Close()
+
+	if response.StatusCode == 200 {
+		return nil
+	}
+
+	// Gracefully ignore "snapshot not found" errors as they could have already
+	// been removed either via the cleanup script or by request.
+	if response.StatusCode == 500 {
+		var respJson map[string]interface{}
+		if err := json.NewDecoder(response.Body).Decode(&respJson); err != nil {
+			return err
+		}
+
+		if respJson["message"] == "Failed to get dashboard snapshot" {
+			return nil
+		}
+	}
+
+	return fmt.Errorf("Unexpected response when deleting external snapshot. Status code: %d", response.StatusCode)
+}
+
 // GET /api/snapshots-delete/:deleteKey
 func DeleteDashboardSnapshotByDeleteKey(c *m.ReqContext) Response {
 	key := c.Params(":deleteKey")
@@ -102,6 +192,13 @@ func DeleteDashboardSnapshotByDeleteKey(c *m.ReqContext) Response {
 		return Error(500, "Failed to get dashboard snapshot", err)
 	}
 
+	if query.Result.External {
+		err := deleteExternalDashboardSnapshot(query.Result.ExternalDeleteUrl)
+		if err != nil {
+			return Error(500, "Failed to delete external dashboard", err)
+		}
+	}
+
 	cmd := &m.DeleteDashboardSnapshotCommand{DeleteKey: query.Result.DeleteKey}
 
 	if err := bus.Dispatch(cmd); err != nil {
@@ -138,6 +235,13 @@ func DeleteDashboardSnapshot(c *m.ReqContext) Response {
 		return Error(403, "Access denied to this snapshot", nil)
 	}
 
+	if query.Result.External {
+		err := deleteExternalDashboardSnapshot(query.Result.ExternalDeleteUrl)
+		if err != nil {
+			return Error(500, "Failed to delete external dashboard", err)
+		}
+	}
+
 	cmd := &m.DeleteDashboardSnapshotCommand{DeleteKey: query.Result.DeleteKey}
 
 	if err := bus.Dispatch(cmd); err != nil {

pkg/api/dashboard_snapshot_test.go

 package api
 
 import (
+	"fmt"
+	"net/http"
+	"net/http/httptest"
 	"testing"
 	"time"
 
@@ -13,13 +16,17 @@ import (
 
 func TestDashboardSnapshotApiEndpoint(t *testing.T) {
 	Convey("Given a single snapshot", t, func() {
+		var externalRequest *http.Request
 		jsonModel, _ := simplejson.NewJson([]byte(`{"id":100}`))
 
 		mockSnapshotResult := &m.DashboardSnapshot{
 			Id:        1,
+			Key:       "12345",
+			DeleteKey: "54321",
 			Dashboard: jsonModel,
 			Expires:   time.Now().Add(time.Duration(1000) * time.Second),
 			UserId:    999999,
+			External:  true,
 		}
 
 		bus.AddHandler("test", func(query *m.GetDashboardSnapshotQuery) error {
@@ -45,13 +52,25 @@ func TestDashboardSnapshotApiEndpoint(t *testing.T) {
 			return nil
 		})
 
+		setupRemoteServer := func(fn func(http.ResponseWriter, *http.Request)) *httptest.Server {
+			return httptest.NewServer(http.HandlerFunc(func(rw http.ResponseWriter, r *http.Request) {
+				fn(rw, r)
+			}))
+		}
+
 		Convey("When user has editor role and is not in the ACL", func() {
 			Convey("Should not be able to delete snapshot", func() {
 				loggedInUserScenarioWithRole("When calling DELETE on", "DELETE", "/api/snapshots/12345", "/api/snapshots/:key", m.ROLE_EDITOR, func(sc *scenarioContext) {
+					ts := setupRemoteServer(func(rw http.ResponseWriter, req *http.Request) {
+						externalRequest = req
+					})
+
+					mockSnapshotResult.ExternalDeleteUrl = ts.URL
 					sc.handlerFunc = DeleteDashboardSnapshot
 					sc.fakeReqWithParams("DELETE", sc.url, map[string]string{"key": "12345"}).exec()
 
 					So(sc.resp.Code, ShouldEqual, 403)
+					So(externalRequest, ShouldBeNil)
 				})
 			})
 		})
@@ -59,6 +78,12 @@ func TestDashboardSnapshotApiEndpoint(t *testing.T) {
 		Convey("When user is anonymous", func() {
 			Convey("Should be able to delete snapshot by deleteKey", func() {
 				anonymousUserScenario("When calling GET on", "GET", "/api/snapshots-delete/12345", "/api/snapshots-delete/:deleteKey", func(sc *scenarioContext) {
+					ts := setupRemoteServer(func(rw http.ResponseWriter, req *http.Request) {
+						rw.WriteHeader(200)
+						externalRequest = req
+					})
+
+					mockSnapshotResult.ExternalDeleteUrl = ts.URL
 					sc.handlerFunc = DeleteDashboardSnapshotByDeleteKey
 					sc.fakeReqWithParams("GET", sc.url, map[string]string{"deleteKey": "12345"}).exec()
 
@@ -67,6 +92,10 @@ func TestDashboardSnapshotApiEndpoint(t *testing.T) {
 					So(err, ShouldBeNil)
 
 					So(respJSON.Get("message").MustString(), ShouldStartWith, "Snapshot deleted")
+
+					So(externalRequest.Method, ShouldEqual, http.MethodGet)
+					So(fmt.Sprintf("http://%s", externalRequest.Host), ShouldEqual, ts.URL)
+					So(externalRequest.URL.EscapedPath(), ShouldEqual, "/")
 				})
 			})
 		})
@@ -79,6 +108,12 @@ func TestDashboardSnapshotApiEndpoint(t *testing.T) {
 
 			Convey("Should be able to delete a snapshot", func() {
 				loggedInUserScenarioWithRole("When calling DELETE on", "DELETE", "/api/snapshots/12345", "/api/snapshots/:key", m.ROLE_EDITOR, func(sc *scenarioContext) {
+					ts := setupRemoteServer(func(rw http.ResponseWriter, req *http.Request) {
+						rw.WriteHeader(200)
+						externalRequest = req
+					})
+
+					mockSnapshotResult.ExternalDeleteUrl = ts.URL
 					sc.handlerFunc = DeleteDashboardSnapshot
 					sc.fakeReqWithParams("DELETE", sc.url, map[string]string{"key": "12345"}).exec()
 
@@ -87,6 +122,8 @@ func TestDashboardSnapshotApiEndpoint(t *testing.T) {
 					So(err, ShouldBeNil)
 
 					So(respJSON.Get("message").MustString(), ShouldStartWith, "Snapshot deleted")
+					So(fmt.Sprintf("http://%s", externalRequest.Host), ShouldEqual, ts.URL)
+					So(externalRequest.URL.EscapedPath(), ShouldEqual, "/")
 				})
 			})
 		})
@@ -94,6 +131,7 @@ func TestDashboardSnapshotApiEndpoint(t *testing.T) {
 		Convey("When user is editor and is the creator of the snapshot", func() {
 			aclMockResp = []*m.DashboardAclInfoDTO{}
 			mockSnapshotResult.UserId = TestUserID
+			mockSnapshotResult.External = false
 
 			Convey("Should be able to delete a snapshot", func() {
 				loggedInUserScenarioWithRole("When calling DELETE on", "DELETE", "/api/snapshots/12345", "/api/snapshots/:key", m.ROLE_EDITOR, func(sc *scenarioContext) {
@@ -108,5 +146,54 @@ func TestDashboardSnapshotApiEndpoint(t *testing.T) {
 				})
 			})
 		})
+
+		Convey("When deleting an external snapshot", func() {
+			aclMockResp = []*m.DashboardAclInfoDTO{}
+			mockSnapshotResult.UserId = TestUserID
+
+			Convey("Should gracefully delete local snapshot when remote snapshot has already been removed", func() {
+				loggedInUserScenarioWithRole("When calling DELETE on", "DELETE", "/api/snapshots/12345", "/api/snapshots/:key", m.ROLE_EDITOR, func(sc *scenarioContext) {
+					ts := setupRemoteServer(func(rw http.ResponseWriter, req *http.Request) {
+						rw.Write([]byte(`{"message":"Failed to get dashboard snapshot"}`))
+						rw.WriteHeader(500)
+					})
+
+					mockSnapshotResult.ExternalDeleteUrl = ts.URL
+					sc.handlerFunc = DeleteDashboardSnapshot
+					sc.fakeReqWithParams("DELETE", sc.url, map[string]string{"key": "12345"}).exec()
+
+					So(sc.resp.Code, ShouldEqual, 200)
+				})
+			})
+
+			Convey("Should fail to delete local snapshot when an unexpected 500 error occurs", func() {
+				loggedInUserScenarioWithRole("When calling DELETE on", "DELETE", "/api/snapshots/12345", "/api/snapshots/:key", m.ROLE_EDITOR, func(sc *scenarioContext) {
+					ts := setupRemoteServer(func(rw http.ResponseWriter, req *http.Request) {
+						rw.WriteHeader(500)
+						rw.Write([]byte(`{"message":"Unexpected"}`))
+					})
+
+					mockSnapshotResult.ExternalDeleteUrl = ts.URL
+					sc.handlerFunc = DeleteDashboardSnapshot
+					sc.fakeReqWithParams("DELETE", sc.url, map[string]string{"key": "12345"}).exec()
+
+					So(sc.resp.Code, ShouldEqual, 500)
+				})
+			})
+
+			Convey("Should fail to delete local snapshot when an unexpected remote error occurs", func() {
+				loggedInUserScenarioWithRole("When calling DELETE on", "DELETE", "/api/snapshots/12345", "/api/snapshots/:key", m.ROLE_EDITOR, func(sc *scenarioContext) {
+					ts := setupRemoteServer(func(rw http.ResponseWriter, req *http.Request) {
+						rw.WriteHeader(404)
+					})
+
+					mockSnapshotResult.ExternalDeleteUrl = ts.URL
+					sc.handlerFunc = DeleteDashboardSnapshot
+					sc.fakeReqWithParams("DELETE", sc.url, map[string]string{"key": "12345"}).exec()
+
+					So(sc.resp.Code, ShouldEqual, 500)
+				})
+			})
+		})
 	})
 }

pkg/models/dashboard_snapshot.go

@@ -16,6 +16,7 @@ type DashboardSnapshot struct {
 	UserId            int64
 	External          bool
 	ExternalUrl       string
+	ExternalDeleteUrl string
 
 	Expires time.Time
 	Created time.Time
@@ -49,6 +50,9 @@ type CreateDashboardSnapshotCommand struct {
 
 	// these are passed when storing an external snapshot ref
 	External          bool   `json:"external"`
+	ExternalUrl       string `json:"-"`
+	ExternalDeleteUrl string `json:"-"`
+
 	Key       string `json:"key"`
 	DeleteKey string `json:"deleteKey"`
 

pkg/services/sqlstore/dashboard_snapshot.go

@@ -53,6 +53,8 @@ func CreateDashboardSnapshot(cmd *m.CreateDashboardSnapshotCommand) error {
 			OrgId:             cmd.OrgId,
 			UserId:            cmd.UserId,
 			External:          cmd.External,
+			ExternalUrl:       cmd.ExternalUrl,
+			ExternalDeleteUrl: cmd.ExternalDeleteUrl,
 			Dashboard:         cmd.Dashboard,
 			Expires:           expires,
 			Created:           time.Now(),

pkg/services/sqlstore/migrations/dashboard_snapshot_mig.go

@@ -60,4 +60,8 @@ func addDashboardSnapshotMigrations(mg *Migrator) {
 		{Name: "external_url", Type: DB_NVarchar, Length: 255, Nullable: false},
 		{Name: "dashboard", Type: DB_MediumText, Nullable: false},
 	}))
+
+	mg.AddMigration("Add column external_delete_url to dashboard_snapshots table", NewAddColumnMigration(snapshotV5, &Column{
+		Name: "external_delete_url", Type: DB_NVarchar, Length: 255, Nullable: true,
+	}))
 }

public/app/features/dashboard/share_snapshot_ctrl.ts

@@ -27,7 +27,6 @@ export class ShareSnapshotCtrl {
 
     $scope.init = () => {
       backendSrv.get('/api/snapshot/shared-options').then(options => {
-        $scope.externalUrl = options['externalSnapshotURL'];
         $scope.sharingButtonText = options['externalSnapshotName'];
         $scope.externalEnabled = options['externalEnabled'];
       });
@@ -61,30 +60,14 @@ export class ShareSnapshotCtrl {
         dashboard: dash,
         name: dash.title,
         expires: $scope.snapshot.expires,
+        external: external,
       };
 
-      const postUrl = external ? $scope.externalUrl + $scope.apiUrl : $scope.apiUrl;
-
-      backendSrv.post(postUrl, cmdData).then(
+      backendSrv.post($scope.apiUrl, cmdData).then(
         results => {
           $scope.loading = false;
-
-          if (external) {
           $scope.deleteUrl = results.deleteUrl;
           $scope.snapshotUrl = results.url;
-            $scope.saveExternalSnapshotRef(cmdData, results);
-          } else {
-            const url = $location.url();
-            let baseUrl = $location.absUrl();
-
-            if (url !== '/') {
-              baseUrl = baseUrl.replace(url, '') + '/';
-            }
-
-            $scope.snapshotUrl = baseUrl + 'dashboard/snapshot/' + results.key;
-            $scope.deleteUrl = baseUrl + 'api/snapshots-delete/' + results.deleteKey;
-          }
-
           $scope.step = 2;
         },
         () => {
@@ -161,14 +144,6 @@ export class ShareSnapshotCtrl {
         $scope.step = 3;
       });
     };
-
-    $scope.saveExternalSnapshotRef = (cmdData, results) => {
-      // save external in local instance as well
-      cmdData.external = true;
-      cmdData.key = results.key;
-      cmdData.deleteKey = results.deleteKey;
-      backendSrv.post('/api/snapshots/', cmdData);
-    };
   }
 }
 

public/app/features/manage-dashboards/SnapshotListCtrl.ts

@@ -5,10 +5,14 @@ export class SnapshotListCtrl {
   snapshots: any;
 
   /** @ngInject */
-  constructor(private $rootScope, private backendSrv, navModelSrv) {
+  constructor(private $rootScope, private backendSrv, navModelSrv, private $location) {
     this.navModel = navModelSrv.getNav('dashboards', 'snapshots', 0);
     this.backendSrv.get('/api/dashboard/snapshots').then(result => {
-      this.snapshots = result;
+      const baseUrl = this.$location.absUrl().replace($location.url(), '');
+      this.snapshots = result.map(snapshot => ({
+        ...snapshot,
+        url: snapshot.externalUrl || `${baseUrl}/dashboard/snapshot/${snapshot.key}`,
+      }));
     });
   }
 

public/app/features/manage-dashboards/partials/snapshot_list.html

@@ -6,17 +6,21 @@
       <th><strong>Name</strong></th>
       <th><strong>Snapshot url</strong></th>
       <th style="width: 70px"></th>
+      <th style="width: 30px"></th>
       <th style="width: 25px"></th>
 		</thead>
 		<tr ng-repeat="snapshot in ctrl.snapshots">
       <td>
-				<a href="dashboard/snapshot/{{snapshot.key}}">{{snapshot.name}}</a>
+        <a href="{{snapshot.url}}">{{snapshot.name}}</a>
       </td>
       <td >
-        <a href="dashboard/snapshot/{{snapshot.key}}">dashboard/snapshot/{{snapshot.key}}</a>
+        <a href="{{snapshot.url}}">{{snapshot.url}}</a>
+      </td>
+      <td>
+        <span class="query-keyword" ng-if="snapshot.external">External</span>
       </td>
       <td class="text-center">
-        <a href="dashboard/snapshot/{{snapshot.key}}" class="btn btn-inverse btn-mini">
+        <a href="{{snapshot.url}}" class="btn btn-inverse btn-mini">
           <i class="fa fa-eye"></i>
           View
         </a>