Browse Source

Merge pull request #10996 from grafana/snapshot_permissions

Snapshot fixes and changes to snapshot list
Daniel Lee 7 years ago
parent
commit
3b836c9c5f

+ 0 - 3
conf/defaults.ini

@@ -187,9 +187,6 @@ external_snapshot_name = Publish to snapshot.raintank.io
 # remove expired snapshot
 # remove expired snapshot
 snapshot_remove_expired = true
 snapshot_remove_expired = true
 
 
-# remove snapshots after 90 days
-snapshot_TTL_days = 90
-
 #################################### Dashboards ##################
 #################################### Dashboards ##################
 
 
 [dashboards]
 [dashboards]

+ 0 - 3
conf/sample.ini

@@ -175,9 +175,6 @@ log_queries =
 # remove expired snapshot
 # remove expired snapshot
 ;snapshot_remove_expired = true
 ;snapshot_remove_expired = true
 
 
-# remove snapshots after 90 days
-;snapshot_TTL_days = 90
-
 #################################### Dashboards History ##################
 #################################### Dashboards History ##################
 [dashboards]
 [dashboards]
 # Number dashboard versions to keep (per dashboard). Default: 20, Minimum: 1
 # Number dashboard versions to keep (per dashboard). Default: 20, Minimum: 1

+ 1 - 4
docs/sources/installation/configuration.md

@@ -795,12 +795,9 @@ Set root url to a Grafana instance where you want to publish external snapshots
 ### external_snapshot_name
 ### external_snapshot_name
 Set name for external snapshot button. Defaults to `Publish to snapshot.raintank.io`
 Set name for external snapshot button. Defaults to `Publish to snapshot.raintank.io`
 
 
-### remove expired snapshot
+### snapshot_remove_expired
 Enabled to automatically remove expired snapshots
 Enabled to automatically remove expired snapshots
 
 
-### remove snapshots after 90 days
-Time to live for snapshots.
-
 ## [external_image_storage]
 ## [external_image_storage]
 These options control how images should be made public so they can be shared on services like slack.
 These options control how images should be made public so they can be shared on services like slack.
 
 

+ 1 - 1
pkg/api/api.go

@@ -106,7 +106,7 @@ func (hs *HttpServer) registerRoutes() {
 	r.Post("/api/snapshots/", bind(m.CreateDashboardSnapshotCommand{}), CreateDashboardSnapshot)
 	r.Post("/api/snapshots/", bind(m.CreateDashboardSnapshotCommand{}), CreateDashboardSnapshot)
 	r.Get("/api/snapshot/shared-options/", GetSharingOptions)
 	r.Get("/api/snapshot/shared-options/", GetSharingOptions)
 	r.Get("/api/snapshots/:key", GetDashboardSnapshot)
 	r.Get("/api/snapshots/:key", GetDashboardSnapshot)
-	r.Get("/api/snapshots-delete/:key", reqEditorRole, DeleteDashboardSnapshot)
+	r.Get("/api/snapshots-delete/:key", reqEditorRole, wrap(DeleteDashboardSnapshot))
 
 
 	// api renew session based on remember cookie
 	// api renew session based on remember cookie
 	r.Get("/api/login/ping", quota("session"), LoginApiPing)
 	r.Get("/api/login/ping", quota("session"), LoginApiPing)

+ 35 - 7
pkg/api/dashboard_snapshot.go

@@ -8,6 +8,7 @@ import (
 	"github.com/grafana/grafana/pkg/metrics"
 	"github.com/grafana/grafana/pkg/metrics"
 	"github.com/grafana/grafana/pkg/middleware"
 	"github.com/grafana/grafana/pkg/middleware"
 	m "github.com/grafana/grafana/pkg/models"
 	m "github.com/grafana/grafana/pkg/models"
+	"github.com/grafana/grafana/pkg/services/guardian"
 	"github.com/grafana/grafana/pkg/setting"
 	"github.com/grafana/grafana/pkg/setting"
 	"github.com/grafana/grafana/pkg/util"
 	"github.com/grafana/grafana/pkg/util"
 )
 )
@@ -56,6 +57,7 @@ func CreateDashboardSnapshot(c *middleware.Context, cmd m.CreateDashboardSnapsho
 	})
 	})
 }
 }
 
 
+// GET /api/snapshots/:key
 func GetDashboardSnapshot(c *middleware.Context) {
 func GetDashboardSnapshot(c *middleware.Context) {
 	key := c.Params(":key")
 	key := c.Params(":key")
 	query := &m.GetDashboardSnapshotQuery{Key: key}
 	query := &m.GetDashboardSnapshotQuery{Key: key}
@@ -90,18 +92,43 @@ func GetDashboardSnapshot(c *middleware.Context) {
 	c.JSON(200, dto)
 	c.JSON(200, dto)
 }
 }
 
 
-func DeleteDashboardSnapshot(c *middleware.Context) {
+// GET /api/snapshots-delete/:key
+func DeleteDashboardSnapshot(c *middleware.Context) Response {
 	key := c.Params(":key")
 	key := c.Params(":key")
+
+	query := &m.GetDashboardSnapshotQuery{DeleteKey: key}
+
+	err := bus.Dispatch(query)
+	if err != nil {
+		return ApiError(500, "Failed to get dashboard snapshot", err)
+	}
+
+	if query.Result == nil {
+		return ApiError(404, "Failed to get dashboard snapshot", nil)
+	}
+	dashboard := query.Result.Dashboard
+	dashboardId := dashboard.Get("id").MustInt64()
+
+	guardian := guardian.New(dashboardId, c.OrgId, c.SignedInUser)
+	canEdit, err := guardian.CanEdit()
+	if err != nil {
+		return ApiError(500, "Error while checking permissions for snapshot", err)
+	}
+
+	if !canEdit && query.Result.UserId != c.SignedInUser.UserId {
+		return ApiError(403, "Access denied to this snapshot", nil)
+	}
+
 	cmd := &m.DeleteDashboardSnapshotCommand{DeleteKey: key}
 	cmd := &m.DeleteDashboardSnapshotCommand{DeleteKey: key}
 
 
 	if err := bus.Dispatch(cmd); err != nil {
 	if err := bus.Dispatch(cmd); err != nil {
-		c.JsonApiErr(500, "Failed to delete dashboard snapshot", err)
-		return
+		return ApiError(500, "Failed to delete dashboard snapshot", err)
 	}
 	}
 
 
-	c.JSON(200, util.DynMap{"message": "Snapshot deleted. It might take an hour before it's cleared from a CDN cache."})
+	return Json(200, util.DynMap{"message": "Snapshot deleted. It might take an hour before it's cleared from a CDN cache."})
 }
 }
 
 
+// GET /api/dashboard/snapshots
 func SearchDashboardSnapshots(c *middleware.Context) Response {
 func SearchDashboardSnapshots(c *middleware.Context) Response {
 	query := c.Query("query")
 	query := c.Query("query")
 	limit := c.QueryInt("limit")
 	limit := c.QueryInt("limit")
@@ -111,9 +138,10 @@ func SearchDashboardSnapshots(c *middleware.Context) Response {
 	}
 	}
 
 
 	searchQuery := m.GetDashboardSnapshotsQuery{
 	searchQuery := m.GetDashboardSnapshotsQuery{
-		Name:  query,
-		Limit: limit,
-		OrgId: c.OrgId,
+		Name:         query,
+		Limit:        limit,
+		OrgId:        c.OrgId,
+		SignedInUser: c.SignedInUser,
 	}
 	}
 
 
 	err := bus.Dispatch(&searchQuery)
 	err := bus.Dispatch(&searchQuery)

+ 97 - 0
pkg/api/dashboard_snapshot_test.go

@@ -0,0 +1,97 @@
+package api
+
+import (
+	"testing"
+	"time"
+
+	"github.com/grafana/grafana/pkg/bus"
+	"github.com/grafana/grafana/pkg/components/simplejson"
+	m "github.com/grafana/grafana/pkg/models"
+
+	. "github.com/smartystreets/goconvey/convey"
+)
+
+func TestDashboardSnapshotApiEndpoint(t *testing.T) {
+	Convey("Given a single snapshot", t, func() {
+		jsonModel, _ := simplejson.NewJson([]byte(`{"id":100}`))
+
+		mockSnapshotResult := &m.DashboardSnapshot{
+			Id:        1,
+			Dashboard: jsonModel,
+			Expires:   time.Now().Add(time.Duration(1000) * time.Second),
+			UserId:    999999,
+		}
+
+		bus.AddHandler("test", func(query *m.GetDashboardSnapshotQuery) error {
+			query.Result = mockSnapshotResult
+			return nil
+		})
+
+		bus.AddHandler("test", func(cmd *m.DeleteDashboardSnapshotCommand) error {
+			return nil
+		})
+
+		viewerRole := m.ROLE_VIEWER
+		editorRole := m.ROLE_EDITOR
+		aclMockResp := []*m.DashboardAclInfoDTO{}
+		bus.AddHandler("test", func(query *m.GetDashboardAclInfoListQuery) error {
+			query.Result = aclMockResp
+			return nil
+		})
+
+		teamResp := []*m.Team{}
+		bus.AddHandler("test", func(query *m.GetTeamsByUserQuery) error {
+			query.Result = teamResp
+			return nil
+		})
+
+		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 GET on", "GET", "/api/snapshots-delete/12345", "/api/snapshots-delete/:key", m.ROLE_EDITOR, func(sc *scenarioContext) {
+					sc.handlerFunc = DeleteDashboardSnapshot
+					sc.fakeReqWithParams("GET", sc.url, map[string]string{"key": "12345"}).exec()
+
+					So(sc.resp.Code, ShouldEqual, 403)
+				})
+			})
+		})
+
+		Convey("When user is editor and dashboard has default ACL", func() {
+			aclMockResp = []*m.DashboardAclInfoDTO{
+				{Role: &viewerRole, Permission: m.PERMISSION_VIEW},
+				{Role: &editorRole, Permission: m.PERMISSION_EDIT},
+			}
+
+			Convey("Should be able to delete a snapshot", func() {
+				loggedInUserScenarioWithRole("When calling GET on", "GET", "/api/snapshots-delete/12345", "/api/snapshots-delete/:key", m.ROLE_EDITOR, func(sc *scenarioContext) {
+					sc.handlerFunc = DeleteDashboardSnapshot
+					sc.fakeReqWithParams("GET", sc.url, map[string]string{"key": "12345"}).exec()
+
+					So(sc.resp.Code, ShouldEqual, 200)
+					respJSON, err := simplejson.NewJson(sc.resp.Body.Bytes())
+					So(err, ShouldBeNil)
+
+					So(respJSON.Get("message").MustString(), ShouldStartWith, "Snapshot deleted")
+				})
+			})
+		})
+
+		Convey("When user is editor and is the creator of the snapshot", func() {
+			aclMockResp = []*m.DashboardAclInfoDTO{}
+			mockSnapshotResult.UserId = TestUserID
+
+			Convey("Should be able to delete a snapshot", func() {
+				loggedInUserScenarioWithRole("When calling GET on", "GET", "/api/snapshots-delete/12345", "/api/snapshots-delete/:key", m.ROLE_EDITOR, func(sc *scenarioContext) {
+					sc.handlerFunc = DeleteDashboardSnapshot
+					sc.fakeReqWithParams("GET", sc.url, map[string]string{"key": "12345"}).exec()
+
+					So(sc.resp.Code, ShouldEqual, 200)
+					respJSON, err := simplejson.NewJson(sc.resp.Body.Bytes())
+					So(err, ShouldBeNil)
+
+					So(respJSON.Get("message").MustString(), ShouldStartWith, "Snapshot deleted")
+				})
+			})
+		})
+	})
+}

+ 7 - 4
pkg/models/dashboard_snapshot.go

@@ -64,10 +64,12 @@ type DeleteDashboardSnapshotCommand struct {
 }
 }
 
 
 type DeleteExpiredSnapshotsCommand struct {
 type DeleteExpiredSnapshotsCommand struct {
+	DeletedRows int64
 }
 }
 
 
 type GetDashboardSnapshotQuery struct {
 type GetDashboardSnapshotQuery struct {
-	Key string
+	Key       string
+	DeleteKey string
 
 
 	Result *DashboardSnapshot
 	Result *DashboardSnapshot
 }
 }
@@ -76,9 +78,10 @@ type DashboardSnapshots []*DashboardSnapshot
 type DashboardSnapshotsList []*DashboardSnapshotDTO
 type DashboardSnapshotsList []*DashboardSnapshotDTO
 
 
 type GetDashboardSnapshotsQuery struct {
 type GetDashboardSnapshotsQuery struct {
-	Name  string
-	Limit int
-	OrgId int64
+	Name         string
+	Limit        int
+	OrgId        int64
+	SignedInUser *SignedInUser
 
 
 	Result DashboardSnapshotsList
 	Result DashboardSnapshotsList
 }
 }

+ 1 - 0
pkg/models/dashboard_version.go

@@ -75,4 +75,5 @@ type GetDashboardVersionsQuery struct {
 //
 //
 
 
 type DeleteExpiredVersionsCommand struct {
 type DeleteExpiredVersionsCommand struct {
+	DeletedRows int64
 }
 }

+ 12 - 2
pkg/services/cleanup/cleanup.go

@@ -83,11 +83,21 @@ func (service *CleanUpService) cleanUpTmpFiles() {
 }
 }
 
 
 func (service *CleanUpService) deleteExpiredSnapshots() {
 func (service *CleanUpService) deleteExpiredSnapshots() {
-	bus.Dispatch(&m.DeleteExpiredSnapshotsCommand{})
+	cmd := m.DeleteExpiredSnapshotsCommand{}
+	if err := bus.Dispatch(&cmd); err != nil {
+		service.log.Error("Failed to delete expired snapshots", "error", err.Error())
+	} else {
+		service.log.Debug("Deleted expired snapshots", "rows affected", cmd.DeletedRows)
+	}
 }
 }
 
 
 func (service *CleanUpService) deleteExpiredDashboardVersions() {
 func (service *CleanUpService) deleteExpiredDashboardVersions() {
-	bus.Dispatch(&m.DeleteExpiredVersionsCommand{})
+	cmd := m.DeleteExpiredVersionsCommand{}
+	if err := bus.Dispatch(&cmd); err != nil {
+		service.log.Error("Failed to delete expired dashboard versions", "error", err.Error())
+	} else {
+		service.log.Debug("Deleted old/expired dashboard versions", "rows affected", cmd.DeletedRows)
+	}
 }
 }
 
 
 func (service *CleanUpService) deleteOldLoginAttempts() {
 func (service *CleanUpService) deleteOldLoginAttempts() {

+ 26 - 12
pkg/services/sqlstore/dashboard_snapshot.go

@@ -16,20 +16,23 @@ func init() {
 	bus.AddHandler("sql", DeleteExpiredSnapshots)
 	bus.AddHandler("sql", DeleteExpiredSnapshots)
 }
 }
 
 
+// DeleteExpiredSnapshots removes snapshots with old expiry dates.
+// SnapShotRemoveExpired is deprecated and should be removed in the future.
+// Snapshot expiry is decided by the user when they share the snapshot.
 func DeleteExpiredSnapshots(cmd *m.DeleteExpiredSnapshotsCommand) error {
 func DeleteExpiredSnapshots(cmd *m.DeleteExpiredSnapshotsCommand) error {
 	return inTransaction(func(sess *DBSession) error {
 	return inTransaction(func(sess *DBSession) error {
-		var expiredCount int64 = 0
-
-		if setting.SnapShotRemoveExpired {
-			deleteExpiredSql := "DELETE FROM dashboard_snapshot WHERE expires < ?"
-			expiredResponse, err := x.Exec(deleteExpiredSql, time.Now)
-			if err != nil {
-				return err
-			}
-			expiredCount, _ = expiredResponse.RowsAffected()
+		if !setting.SnapShotRemoveExpired {
+			sqlog.Warn("[Deprecated] The snapshot_remove_expired setting is outdated. Please remove from your config.")
+			return nil
 		}
 		}
 
 
-		sqlog.Debug("Deleted old/expired snaphots", "expired", expiredCount)
+		deleteExpiredSql := "DELETE FROM dashboard_snapshot WHERE expires < ?"
+		expiredResponse, err := sess.Exec(deleteExpiredSql, time.Now())
+		if err != nil {
+			return err
+		}
+		cmd.DeletedRows, _ = expiredResponse.RowsAffected()
+
 		return nil
 		return nil
 	})
 	})
 }
 }
@@ -72,7 +75,7 @@ func DeleteDashboardSnapshot(cmd *m.DeleteDashboardSnapshotCommand) error {
 }
 }
 
 
 func GetDashboardSnapshot(query *m.GetDashboardSnapshotQuery) error {
 func GetDashboardSnapshot(query *m.GetDashboardSnapshotQuery) error {
-	snapshot := m.DashboardSnapshot{Key: query.Key}
+	snapshot := m.DashboardSnapshot{Key: query.Key, DeleteKey: query.DeleteKey}
 	has, err := x.Get(&snapshot)
 	has, err := x.Get(&snapshot)
 
 
 	if err != nil {
 	if err != nil {
@@ -85,6 +88,8 @@ func GetDashboardSnapshot(query *m.GetDashboardSnapshotQuery) error {
 	return nil
 	return nil
 }
 }
 
 
+// SearchDashboardSnapshots returns a list of all snapshots for admins
+// for other roles, it returns snapshots created by the user
 func SearchDashboardSnapshots(query *m.GetDashboardSnapshotsQuery) error {
 func SearchDashboardSnapshots(query *m.GetDashboardSnapshotsQuery) error {
 	var snapshots = make(m.DashboardSnapshotsList, 0)
 	var snapshots = make(m.DashboardSnapshotsList, 0)
 
 
@@ -95,7 +100,16 @@ func SearchDashboardSnapshots(query *m.GetDashboardSnapshotsQuery) error {
 		sess.Where("name LIKE ?", query.Name)
 		sess.Where("name LIKE ?", query.Name)
 	}
 	}
 
 
-	sess.Where("org_id = ?", query.OrgId)
+	// admins can see all snapshots, everyone else can only see their own snapshots
+	if query.SignedInUser.OrgRole == m.ROLE_ADMIN {
+		sess.Where("org_id = ?", query.OrgId)
+	} else if !query.SignedInUser.IsAnonymous {
+		sess.Where("org_id = ? AND user_id = ?", query.OrgId, query.SignedInUser.UserId)
+	} else {
+		query.Result = snapshots
+		return nil
+	}
+
 	err := sess.Find(&snapshots)
 	err := sess.Find(&snapshots)
 	query.Result = snapshots
 	query.Result = snapshots
 	return err
 	return err

+ 136 - 2
pkg/services/sqlstore/dashboard_snapshot_test.go

@@ -2,11 +2,14 @@ package sqlstore
 
 
 import (
 import (
 	"testing"
 	"testing"
+	"time"
 
 
+	"github.com/go-xorm/xorm"
 	. "github.com/smartystreets/goconvey/convey"
 	. "github.com/smartystreets/goconvey/convey"
 
 
 	"github.com/grafana/grafana/pkg/components/simplejson"
 	"github.com/grafana/grafana/pkg/components/simplejson"
 	m "github.com/grafana/grafana/pkg/models"
 	m "github.com/grafana/grafana/pkg/models"
+	"github.com/grafana/grafana/pkg/setting"
 )
 )
 
 
 func TestDashboardSnapshotDBAccess(t *testing.T) {
 func TestDashboardSnapshotDBAccess(t *testing.T) {
@@ -14,17 +17,19 @@ func TestDashboardSnapshotDBAccess(t *testing.T) {
 	Convey("Testing DashboardSnapshot data access", t, func() {
 	Convey("Testing DashboardSnapshot data access", t, func() {
 		InitTestDB(t)
 		InitTestDB(t)
 
 
-		Convey("Given saved snaphot", func() {
+		Convey("Given saved snapshot", func() {
 			cmd := m.CreateDashboardSnapshotCommand{
 			cmd := m.CreateDashboardSnapshotCommand{
 				Key: "hej",
 				Key: "hej",
 				Dashboard: simplejson.NewFromAny(map[string]interface{}{
 				Dashboard: simplejson.NewFromAny(map[string]interface{}{
 					"hello": "mupp",
 					"hello": "mupp",
 				}),
 				}),
+				UserId: 1000,
+				OrgId:  1,
 			}
 			}
 			err := CreateDashboardSnapshot(&cmd)
 			err := CreateDashboardSnapshot(&cmd)
 			So(err, ShouldBeNil)
 			So(err, ShouldBeNil)
 
 
-			Convey("Should be able to get snaphot by key", func() {
+			Convey("Should be able to get snapshot by key", func() {
 				query := m.GetDashboardSnapshotQuery{Key: "hej"}
 				query := m.GetDashboardSnapshotQuery{Key: "hej"}
 				err = GetDashboardSnapshot(&query)
 				err = GetDashboardSnapshot(&query)
 				So(err, ShouldBeNil)
 				So(err, ShouldBeNil)
@@ -33,6 +38,135 @@ func TestDashboardSnapshotDBAccess(t *testing.T) {
 				So(query.Result.Dashboard.Get("hello").MustString(), ShouldEqual, "mupp")
 				So(query.Result.Dashboard.Get("hello").MustString(), ShouldEqual, "mupp")
 			})
 			})
 
 
+			Convey("And the user has the admin role", func() {
+				Convey("Should return all the snapshots", func() {
+					query := m.GetDashboardSnapshotsQuery{
+						OrgId:        1,
+						SignedInUser: &m.SignedInUser{OrgRole: m.ROLE_ADMIN},
+					}
+					err := SearchDashboardSnapshots(&query)
+					So(err, ShouldBeNil)
+
+					So(query.Result, ShouldNotBeNil)
+					So(len(query.Result), ShouldEqual, 1)
+				})
+			})
+
+			Convey("And the user has the editor role and has created a snapshot", func() {
+				Convey("Should return all the snapshots", func() {
+					query := m.GetDashboardSnapshotsQuery{
+						OrgId:        1,
+						SignedInUser: &m.SignedInUser{OrgRole: m.ROLE_EDITOR, UserId: 1000},
+					}
+					err := SearchDashboardSnapshots(&query)
+					So(err, ShouldBeNil)
+
+					So(query.Result, ShouldNotBeNil)
+					So(len(query.Result), ShouldEqual, 1)
+				})
+			})
+
+			Convey("And the user has the editor role and has not created any snapshot", func() {
+				Convey("Should not return any snapshots", func() {
+					query := m.GetDashboardSnapshotsQuery{
+						OrgId:        1,
+						SignedInUser: &m.SignedInUser{OrgRole: m.ROLE_EDITOR, UserId: 2},
+					}
+					err := SearchDashboardSnapshots(&query)
+					So(err, ShouldBeNil)
+
+					So(query.Result, ShouldNotBeNil)
+					So(len(query.Result), ShouldEqual, 0)
+				})
+			})
+
+			Convey("And the user is anonymous", func() {
+				cmd := m.CreateDashboardSnapshotCommand{
+					Key:       "strangesnapshotwithuserid0",
+					DeleteKey: "adeletekey",
+					Dashboard: simplejson.NewFromAny(map[string]interface{}{
+						"hello": "mupp",
+					}),
+					UserId: 0,
+					OrgId:  1,
+				}
+				err := CreateDashboardSnapshot(&cmd)
+				So(err, ShouldBeNil)
+
+				Convey("Should not return any snapshots", func() {
+					query := m.GetDashboardSnapshotsQuery{
+						OrgId:        1,
+						SignedInUser: &m.SignedInUser{OrgRole: m.ROLE_EDITOR, IsAnonymous: true, UserId: 0},
+					}
+					err := SearchDashboardSnapshots(&query)
+					So(err, ShouldBeNil)
+
+					So(query.Result, ShouldNotBeNil)
+					So(len(query.Result), ShouldEqual, 0)
+				})
+			})
 		})
 		})
 	})
 	})
 }
 }
+
+func TestDeleteExpiredSnapshots(t *testing.T) {
+	Convey("Testing dashboard snapshots clean up", t, func() {
+		x := InitTestDB(t)
+
+		setting.SnapShotRemoveExpired = true
+
+		notExpiredsnapshot := createTestSnapshot(x, "key1", 1000)
+		createTestSnapshot(x, "key2", -1000)
+		createTestSnapshot(x, "key3", -1000)
+
+		Convey("Clean up old dashboard snapshots", func() {
+			err := DeleteExpiredSnapshots(&m.DeleteExpiredSnapshotsCommand{})
+			So(err, ShouldBeNil)
+
+			query := m.GetDashboardSnapshotsQuery{
+				OrgId:        1,
+				SignedInUser: &m.SignedInUser{OrgRole: m.ROLE_ADMIN},
+			}
+			err = SearchDashboardSnapshots(&query)
+			So(err, ShouldBeNil)
+
+			So(len(query.Result), ShouldEqual, 1)
+			So(query.Result[0].Key, ShouldEqual, notExpiredsnapshot.Key)
+		})
+
+		Convey("Don't delete anything if there are no expired snapshots", func() {
+			err := DeleteExpiredSnapshots(&m.DeleteExpiredSnapshotsCommand{})
+			So(err, ShouldBeNil)
+
+			query := m.GetDashboardSnapshotsQuery{
+				OrgId:        1,
+				SignedInUser: &m.SignedInUser{OrgRole: m.ROLE_ADMIN},
+			}
+			SearchDashboardSnapshots(&query)
+
+			So(len(query.Result), ShouldEqual, 1)
+		})
+	})
+}
+
+func createTestSnapshot(x *xorm.Engine, key string, expires int64) *m.DashboardSnapshot {
+	cmd := m.CreateDashboardSnapshotCommand{
+		Key:       key,
+		DeleteKey: "delete" + key,
+		Dashboard: simplejson.NewFromAny(map[string]interface{}{
+			"hello": "mupp",
+		}),
+		UserId:  1000,
+		OrgId:   1,
+		Expires: expires,
+	}
+	err := CreateDashboardSnapshot(&cmd)
+	So(err, ShouldBeNil)
+
+	// Set expiry date manually - to be able to create expired snapshots
+	expireDate := time.Now().Add(time.Second * time.Duration(expires))
+	_, err = x.Exec("update dashboard_snapshot set expires = ? where "+dialect.Quote("key")+" = ?", expireDate, key)
+	So(err, ShouldBeNil)
+
+	return cmd.Result
+}

+ 1 - 3
pkg/services/sqlstore/dashboard_version.go

@@ -69,7 +69,6 @@ func GetDashboardVersions(query *m.GetDashboardVersionsQuery) error {
 
 
 func DeleteExpiredVersions(cmd *m.DeleteExpiredVersionsCommand) error {
 func DeleteExpiredVersions(cmd *m.DeleteExpiredVersionsCommand) error {
 	return inTransaction(func(sess *DBSession) error {
 	return inTransaction(func(sess *DBSession) error {
-		expiredCount := int64(0)
 		versions := []DashboardVersionExp{}
 		versions := []DashboardVersionExp{}
 		versionsToKeep := setting.DashboardVersionsToKeep
 		versionsToKeep := setting.DashboardVersionsToKeep
 
 
@@ -98,8 +97,7 @@ func DeleteExpiredVersions(cmd *m.DeleteExpiredVersionsCommand) error {
 			if err != nil {
 			if err != nil {
 				return err
 				return err
 			}
 			}
-			expiredCount, _ = expiredResponse.RowsAffected()
-			sqlog.Debug("Deleted old/expired dashboard versions", "expired", expiredCount)
+			cmd.DeletedRows, _ = expiredResponse.RowsAffected()
 		}
 		}
 
 
 		return nil
 		return nil

+ 0 - 2
pkg/setting/setting.go

@@ -88,7 +88,6 @@ var (
 	ExternalSnapshotUrl   string
 	ExternalSnapshotUrl   string
 	ExternalSnapshotName  string
 	ExternalSnapshotName  string
 	ExternalEnabled       bool
 	ExternalEnabled       bool
-	SnapShotTTLDays       int
 	SnapShotRemoveExpired bool
 	SnapShotRemoveExpired bool
 
 
 	// Dashboard history
 	// Dashboard history
@@ -523,7 +522,6 @@ func NewConfigContext(args *CommandLineArgs) error {
 	ExternalSnapshotName = snapshots.Key("external_snapshot_name").String()
 	ExternalSnapshotName = snapshots.Key("external_snapshot_name").String()
 	ExternalEnabled = snapshots.Key("external_enabled").MustBool(true)
 	ExternalEnabled = snapshots.Key("external_enabled").MustBool(true)
 	SnapShotRemoveExpired = snapshots.Key("snapshot_remove_expired").MustBool(true)
 	SnapShotRemoveExpired = snapshots.Key("snapshot_remove_expired").MustBool(true)
-	SnapShotTTLDays = snapshots.Key("snapshot_TTL_days").MustInt(90)
 
 
 	// read dashboard settings
 	// read dashboard settings
 	dashboards := Cfg.Section("dashboards")
 	dashboards := Cfg.Section("dashboards")