diff --git a/docs/sources/developers/http_api/dashboard_versions.md b/docs/sources/developers/http_api/dashboard_versions.md index 96960701098..ef25424bbc7 100644 --- a/docs/sources/developers/http_api/dashboard_versions.md +++ b/docs/sources/developers/http_api/dashboard_versions.md @@ -216,6 +216,7 @@ JSON response body schema: Status codes: - **200** - OK +- **400** - Bad request (specified version has the same content as the current dashboard) - **401** - Unauthorized - **404** - Not found (dashboard not found or dashboard version not found) - **500** - Internal server error (indicates issue retrieving dashboard tags from database) diff --git a/pkg/api/dashboard.go b/pkg/api/dashboard.go index 17d37830645..4406b5d8891 100644 --- a/pkg/api/dashboard.go +++ b/pkg/api/dashboard.go @@ -8,6 +8,7 @@ import ( "net/http" "os" "path/filepath" + "reflect" "strconv" "strings" @@ -1114,6 +1115,13 @@ func (hs *HTTPServer) RestoreDashboardVersion(c *contextmodel.ReqContext) respon return response.Error(http.StatusNotFound, "Dashboard version not found", nil) } + // do not allow restores if the json data is identical + // this is needed for the k8s flow, as the generation id will be used on the + // version table, and the generation id only increments when the actual spec is changed + if compareDashboardData(version.Data.MustMap(), dash.Data.MustMap()) { + return response.Error(http.StatusBadRequest, "Current dashboard is identical to the specified version", nil) + } + var userID int64 if id, err := identity.UserIdentifier(c.SignedInUser.GetID()); err == nil { userID = id @@ -1135,6 +1143,18 @@ func (hs *HTTPServer) RestoreDashboardVersion(c *contextmodel.ReqContext) respon return hs.postDashboard(c, saveCmd) } +func compareDashboardData(versionData, dashData map[string]any) bool { + // these can be different but the actual data is the same + delete(versionData, "version") + delete(dashData, "version") + delete(versionData, "id") + delete(dashData, "id") + delete(versionData, "uid") + delete(dashData, "uid") + + return reflect.DeepEqual(versionData, dashData) +} + // swagger:route GET /dashboards/tags dashboards getDashboardTags // // Get all dashboards tags of an organisation. diff --git a/pkg/api/dashboard_test.go b/pkg/api/dashboard_test.go index 0dfdcd43934..c32c3d1835c 100644 --- a/pkg/api/dashboard_test.go +++ b/pkg/api/dashboard_test.go @@ -585,6 +585,38 @@ func TestDashboardAPIEndpoint(t *testing.T) { } }).Return(nil, nil) + cmd := dtos.RestoreDashboardVersionCommand{ + Version: 1, + } + fakeDashboardVersionService := dashvertest.NewDashboardVersionServiceFake() + fakeDashboardVersionService.ExpectedDashboardVersions = []*dashver.DashboardVersionDTO{ + { + DashboardID: 2, + Version: 1, + Data: simplejson.NewFromAny(map[string]any{ + "title": "Dash1", + }), + }, + } + mockSQLStore := dbtest.NewFakeDB() + + restoreDashboardVersionScenario(t, "When calling POST on", "/api/dashboards/id/1/restore", + "/api/dashboards/id/:dashboardId/restore", dashboardService, fakeDashboardVersionService, cmd, func(sc *scenarioContext) { + sc.dashboardVersionService = fakeDashboardVersionService + + callRestoreDashboardVersion(sc) + assert.Equal(t, http.StatusOK, sc.resp.Code) + }, mockSQLStore) + }) + + t.Run("Should not be able to restore to the same data", func(t *testing.T) { + fakeDash := dashboards.NewDashboard("Child dash") + fakeDash.ID = 2 + fakeDash.HasACL = false + + dashboardService := dashboards.NewFakeDashboardService(t) + dashboardService.On("GetDashboard", mock.Anything, mock.AnythingOfType("*dashboards.GetDashboardQuery")).Return(fakeDash, nil) + cmd := dtos.RestoreDashboardVersionCommand{ Version: 1, } @@ -602,6 +634,42 @@ func TestDashboardAPIEndpoint(t *testing.T) { "/api/dashboards/id/:dashboardId/restore", dashboardService, fakeDashboardVersionService, cmd, func(sc *scenarioContext) { sc.dashboardVersionService = fakeDashboardVersionService + callRestoreDashboardVersion(sc) + assert.Equal(t, http.StatusBadRequest, sc.resp.Code) + }, mockSQLStore) + }) + + t.Run("Given dashboard in general folder being restored should restore to general folder", func(t *testing.T) { + fakeDash := dashboards.NewDashboard("Child dash") + fakeDash.ID = 2 + fakeDash.HasACL = false + + dashboardService := dashboards.NewFakeDashboardService(t) + dashboardService.On("GetDashboard", mock.Anything, mock.AnythingOfType("*dashboards.GetDashboardQuery")).Return(fakeDash, nil) + dashboardService.On("SaveDashboard", mock.Anything, mock.AnythingOfType("*dashboards.SaveDashboardDTO"), mock.AnythingOfType("bool")).Run(func(args mock.Arguments) { + cmd := args.Get(1).(*dashboards.SaveDashboardDTO) + cmd.Dashboard = &dashboards.Dashboard{ + ID: 2, UID: "uid", Title: "Dash", Slug: "dash", Version: 1, + } + }).Return(nil, nil) + + fakeDashboardVersionService := dashvertest.NewDashboardVersionServiceFake() + fakeDashboardVersionService.ExpectedDashboardVersions = []*dashver.DashboardVersionDTO{ + { + DashboardID: 2, + Version: 1, + Data: simplejson.NewFromAny(map[string]any{ + "title": "Dash1", + }), + }, + } + + cmd := dtos.RestoreDashboardVersionCommand{ + Version: 1, + } + mockSQLStore := dbtest.NewFakeDB() + restoreDashboardVersionScenario(t, "When calling POST on", "/api/dashboards/id/1/restore", + "/api/dashboards/id/:dashboardId/restore", dashboardService, fakeDashboardVersionService, cmd, func(sc *scenarioContext) { callRestoreDashboardVersion(sc) assert.Equal(t, http.StatusOK, sc.resp.Code) }, mockSQLStore) @@ -626,7 +694,9 @@ func TestDashboardAPIEndpoint(t *testing.T) { { DashboardID: 2, Version: 1, - Data: fakeDash.Data, + Data: simplejson.NewFromAny(map[string]any{ + "title": "Dash1", + }), }, }