diff --git a/pkg/api/dashboard.go b/pkg/api/dashboard.go index 5b02aa33dd6..2416a2ff2a2 100644 --- a/pkg/api/dashboard.go +++ b/pkg/api/dashboard.go @@ -8,7 +8,6 @@ import ( "net/http" "os" "path/filepath" - "reflect" "strconv" "strings" @@ -28,7 +27,6 @@ import ( "github.com/grafana/grafana/pkg/services/dashboards" "github.com/grafana/grafana/pkg/services/dashboards/dashboardaccess" dashver "github.com/grafana/grafana/pkg/services/dashboardversion" - "github.com/grafana/grafana/pkg/services/dashboardversion/dashverimpl" "github.com/grafana/grafana/pkg/services/featuremgmt" "github.com/grafana/grafana/pkg/services/folder" "github.com/grafana/grafana/pkg/services/org" @@ -68,11 +66,7 @@ func (hs *HTTPServer) isDashboardStarredByUser(c *contextmodel.ReqContext, dashU func dashboardGuardianResponse(err error) response.Response { if err != nil { - var dashboardErr dashboardaccess.DashboardErr - if ok := errors.As(err, &dashboardErr); ok { - return response.Error(dashboardErr.StatusCode, dashboardErr.Error(), err) - } - return response.Error(http.StatusInternalServerError, "Error while checking dashboard permissions", err) + return dashboardErrResponse(err, "Error while checking dashboard permissions") } return response.Error(http.StatusForbidden, "Access denied to this dashboard", nil) } @@ -389,19 +383,7 @@ func (hs *HTTPServer) deleteDashboard(c *contextmodel.ReqContext) response.Respo err = hs.DashboardService.DeleteDashboard(c.Req.Context(), dash.ID, dash.UID, c.GetOrgID()) if err != nil { - var dashboardErr dashboardaccess.DashboardErr - if ok := errors.As(err, &dashboardErr); ok { - if errors.Is(err, dashboards.ErrDashboardCannotDeleteProvisionedDashboard) { - return response.Error(dashboardErr.StatusCode, dashboardErr.Error(), err) - } - } - - var statusErr *k8serrors.StatusError - if errors.As(err, &statusErr) { - return response.Error(int(statusErr.ErrStatus.Code), statusErr.ErrStatus.Message, err) - } - - return response.Error(http.StatusInternalServerError, "Failed to delete dashboard", err) + return dashboardErrResponse(err, "Failed to delete dashboard") } if hs.Live != nil { @@ -949,27 +931,13 @@ func (hs *HTTPServer) CalculateDashboardDiff(c *contextmodel.ReqContext) respons return response.Respond(http.StatusOK, result.Delta).SetHeader("Content-Type", "text/html") } -// swagger:route POST /dashboards/id/{DashboardID}/restore dashboards versions restoreDashboardVersionByID -// -// Restore a dashboard to a given dashboard version. -// -// Please refer to [updated API](#/dashboards/restoreDashboardVersionByUID) instead -// -// Deprecated: true -// -// Responses: -// 200: postDashboardResponse -// 401: unauthorisedError -// 403: forbiddenError -// 404: notFoundError -// 500: internalServerError - // swagger:route POST /dashboards/uid/{uid}/restore dashboards versions restoreDashboardVersionByUID // // Restore a dashboard to a given dashboard version using UID. // // Responses: // 200: postDashboardResponse +// 400: badRequestError // 401: unauthorisedError // 403: forbiddenError // 404: notFoundError @@ -977,73 +945,49 @@ func (hs *HTTPServer) CalculateDashboardDiff(c *contextmodel.ReqContext) respons func (hs *HTTPServer) RestoreDashboardVersion(c *contextmodel.ReqContext) response.Response { ctx, span := tracer.Start(c.Req.Context(), "api.RestoreDashboardVersion") defer span.End() + c.Req = c.Req.WithContext(ctx) - var dashID int64 - - var err error - dashUID := web.Params(c.Req)[":uid"] - - apiCmd := dtos.RestoreDashboardVersionCommand{} + var apiCmd dtos.RestoreDashboardVersionCommand if err := web.Bind(c.Req, &apiCmd); err != nil { + hs.log.Error("error restoring dashboard version: invalid request", "error", err) return response.Error(http.StatusBadRequest, "bad request data", err) } + + var ( + dashID int64 + err error + ) + + dashUID := web.Params(c.Req)[":uid"] if dashUID == "" { dashID, err = strconv.ParseInt(web.Params(c.Req)[":dashboardId"], 10, 64) if err != nil { + hs.log.Error("error restoring dashboard version: invalid dashboardId", "error", err) return response.Error(http.StatusBadRequest, "dashboardId is invalid", err) } } - dash, rsp := hs.getDashboardHelper(c.Req.Context(), c.GetOrgID(), dashID, dashUID) - if rsp != nil { - return rsp - } - - versionQuery := dashver.GetDashboardVersionQuery{DashboardID: dashID, DashboardUID: dash.UID, Version: apiCmd.Version, OrgID: c.GetOrgID()} - version, err := hs.dashboardVersionService.Get(c.Req.Context(), &versionQuery) + res, err := hs.dashboardVersionService.RestoreVersion(ctx, &dashver.RestoreVersionCommand{ + Requester: c.SignedInUser, + DashboardUID: dashUID, + DashboardID: dashID, + Version: apiCmd.Version, + }) if err != nil { - return response.Error(http.StatusNotFound, "Dashboard version not found", nil) + hs.log.Error("error restoring dashboard version: service call failed", "error", err) + return dashboardErrResponse(err, "Failed to restore dashboard version") } - // 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.GetID()); err == nil { - userID = id - } - - saveCmd := dashboards.SaveDashboardCommand{} - saveCmd.RestoredFrom = version.Version - saveCmd.OrgID = c.GetOrgID() - saveCmd.UserID = userID - saveCmd.Dashboard = version.Data - saveCmd.Dashboard.Set("version", dash.Version) - saveCmd.Dashboard.Set("uid", dash.UID) - saveCmd.Message = dashverimpl.DashboardRestoreMessage(version.Version) - // nolint:staticcheck - saveCmd.FolderID = dash.FolderID - metrics.MFolderIDsAPICount.WithLabelValues(metrics.RestoreDashboardVersion).Inc() - saveCmd.FolderUID = dash.FolderUID - - 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) + return response.JSON(http.StatusOK, util.DynMap{ + "status": "success", + "slug": res.Slug, + "version": res.Version, + "id": res.ID, + "uid": res.UID, + "url": res.GetURL(), + "folderUid": res.FolderUID, + }) } // swagger:route GET /dashboards/tags dashboards getDashboardTags @@ -1094,6 +1038,20 @@ func (hs *HTTPServer) GetDashboardUIDs(c *contextmodel.ReqContext) { c.JSON(http.StatusOK, uids) } +func dashboardErrResponse(err error, fallbackMessage string) response.Response { + var dashboardErr dashboardaccess.DashboardErr + if ok := errors.As(err, &dashboardErr); ok { + return response.Error(dashboardErr.StatusCode, dashboardErr.Error(), err) + } + + var statusErr *k8serrors.StatusError + if errors.As(err, &statusErr) { + return response.Error(int(statusErr.ErrStatus.Code), statusErr.ErrStatus.Message, err) + } + + return response.Error(http.StatusInternalServerError, fallbackMessage, err) +} + // swagger:parameters restoreDashboardVersionByID type RestoreDashboardVersionByIDParams struct { // in:body diff --git a/pkg/api/dashboard_test.go b/pkg/api/dashboard_test.go index 90babb5e5f1..cdf85c8594d 100644 --- a/pkg/api/dashboard_test.go +++ b/pkg/api/dashboard_test.go @@ -5,9 +5,11 @@ import ( "context" "encoding/json" "fmt" + "io" "net/http" "os" "strconv" + "strings" "testing" "time" @@ -558,32 +560,27 @@ func TestIntegrationDashboardAPIEndpoint(t *testing.T) { }) t.Run("Given dashboard in folder being restored should restore to 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) cmd := dtos.RestoreDashboardVersionCommand{ Version: 1, } fakeDashboardVersionService := dashvertest.NewDashboardVersionServiceFake() - fakeDashboardVersionService.ExpectedDashboardVersions = []*dashver.DashboardVersionDTO{ - { - DashboardID: 2, - Version: 1, - Data: simplejson.NewFromAny(map[string]any{ - "title": "Dash1", - }), - }, - } + + // Mock successful restoration + restoredDash := dashboards.NewDashboard("Restored Dashboard") + restoredDash.ID = 2 + restoredDash.UID = "uid" + restoredDash.Version = 2 + restoredDash.Slug = "dash" + restoredDash.FolderUID = "folder-uid" + restoredDash.Data = simplejson.NewFromAny(map[string]any{ + "title": "Dash1", + }) + + fakeDashboardVersionService.ExpectedRestoreResult = restoredDash + fakeDashboardVersionService.ExpectedError = nil + mockSQLStore := dbtest.NewFakeDB() restoreDashboardVersionScenario(t, "When calling POST on", "/api/dashboards/id/1/restore", @@ -596,24 +593,17 @@ func TestIntegrationDashboardAPIEndpoint(t *testing.T) { }) 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, } fakeDashboardVersionService := dashvertest.NewDashboardVersionServiceFake() - fakeDashboardVersionService.ExpectedDashboardVersions = []*dashver.DashboardVersionDTO{ - { - DashboardID: 2, - Version: 1, - Data: fakeDash.Data, - }, - } + + // Mock error for identical version + fakeDashboardVersionService.ExpectedRestoreResult = nil + fakeDashboardVersionService.ExpectedError = dashboards.ErrDashboardRestoreIdenticalVersion + mockSQLStore := dbtest.NewFakeDB() restoreDashboardVersionScenario(t, "When calling POST on", "/api/dashboards/id/1/restore", @@ -626,29 +616,22 @@ func TestIntegrationDashboardAPIEndpoint(t *testing.T) { }) 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", - }), - }, - } + + // Mock successful restoration + restoredDash := dashboards.NewDashboard("Restored Dashboard") + restoredDash.ID = 2 + restoredDash.UID = "uid" + restoredDash.Version = 2 + restoredDash.Slug = "dash" + restoredDash.Data = simplejson.NewFromAny(map[string]any{ + "title": "Dash1", + }) + + fakeDashboardVersionService.ExpectedRestoreResult = restoredDash + fakeDashboardVersionService.ExpectedError = nil cmd := dtos.RestoreDashboardVersionCommand{ Version: 1, @@ -661,30 +644,23 @@ func TestIntegrationDashboardAPIEndpoint(t *testing.T) { }, 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 - + t.Run("Given dashboard in general folder being restored should restore to general folder (duplicate)", func(t *testing.T) { 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", - }), - }, - } + + // Mock successful restoration + restoredDash := dashboards.NewDashboard("Restored Dashboard") + restoredDash.ID = 2 + restoredDash.UID = "uid" + restoredDash.Version = 2 + restoredDash.Slug = "dash" + restoredDash.Data = simplejson.NewFromAny(map[string]any{ + "title": "Dash1", + }) + + fakeDashboardVersionService.ExpectedRestoreResult = restoredDash + fakeDashboardVersionService.ExpectedError = nil cmd := dtos.RestoreDashboardVersionCommand{ Version: 1, @@ -697,6 +673,169 @@ func TestIntegrationDashboardAPIEndpoint(t *testing.T) { }, mockSQLStore) }) + t.Run("New RestoreVersion implementation tests", func(t *testing.T) { + t.Run("should use new RestoreVersion service method when available", func(t *testing.T) { + dashboardService := dashboards.NewFakeDashboardService(t) + dashboardVersionService := dashvertest.NewDashboardVersionServiceFake() + + // Mock successful restoration + restoredDash := dashboards.NewDashboard("Restored Dashboard") + restoredDash.ID = 1 + restoredDash.UID = "test-uid" + restoredDash.Version = 6 + restoredDash.Slug = "restored-dashboard" + restoredDash.Data = simplejson.NewFromAny(map[string]any{"title": "Restored Dashboard"}) + + dashboardVersionService.ExpectedRestoreResult = restoredDash + dashboardVersionService.ExpectedError = nil + + cmd := dtos.RestoreDashboardVersionCommand{ + Version: 3, + } + + restoreDashboardVersionScenario(t, "When calling POST on", "/api/dashboards/uid/test-uid/restore", + "/api/dashboards/uid/:uid/restore", dashboardService, dashboardVersionService, cmd, func(sc *scenarioContext) { + sc.dashboardVersionService = dashboardVersionService + callRestoreDashboardVersion(sc) + assert.Equal(t, http.StatusOK, sc.resp.Code) + + // Verify response contains expected fields + result := sc.ToJSON() + assert.Equal(t, "success", result.Get("status").MustString()) + assert.Equal(t, "test-uid", result.Get("uid").MustString()) + assert.Equal(t, int64(6), result.Get("version").MustInt64()) + }, dbtest.NewFakeDB()) + }) + + t.Run("should return error when RestoreVersion service fails", func(t *testing.T) { + dashboardService := dashboards.NewFakeDashboardService(t) + dashboardVersionService := dashvertest.NewDashboardVersionServiceFake() + + // Mock service error + dashboardVersionService.ExpectedRestoreResult = nil + dashboardVersionService.ExpectedError = dashboards.ErrDashboardNotFound + + cmd := dtos.RestoreDashboardVersionCommand{ + Version: 999, // Non-existent version + } + + restoreDashboardVersionScenario(t, "When calling POST on", "/api/dashboards/uid/test-uid/restore", + "/api/dashboards/uid/:uid/restore", dashboardService, dashboardVersionService, cmd, func(sc *scenarioContext) { + sc.dashboardVersionService = dashboardVersionService + callRestoreDashboardVersion(sc) + assert.Equal(t, http.StatusNotFound, sc.resp.Code) + }, dbtest.NewFakeDB()) + }) + + t.Run("should return error when dashboard not found", func(t *testing.T) { + dashboardService := dashboards.NewFakeDashboardService(t) + dashboardVersionService := dashvertest.NewDashboardVersionServiceFake() + + // Mock service error for dashboard not found + dashboardVersionService.ExpectedRestoreResult = nil + dashboardVersionService.ExpectedError = dashboards.ErrDashboardNotFound + + cmd := dtos.RestoreDashboardVersionCommand{ + Version: 3, + } + + restoreDashboardVersionScenario(t, "When calling POST on", "/api/dashboards/uid/nonexistent-uid/restore", + "/api/dashboards/uid/:uid/restore", dashboardService, dashboardVersionService, cmd, func(sc *scenarioContext) { + sc.dashboardVersionService = dashboardVersionService + callRestoreDashboardVersion(sc) + assert.Equal(t, http.StatusNotFound, sc.resp.Code) + }, dbtest.NewFakeDB()) + }) + + t.Run("should return error for invalid request data", func(t *testing.T) { + dashboardService := dashboards.NewFakeDashboardService(t) + dashboardVersionService := dashvertest.NewDashboardVersionServiceFake() + + restoreDashboardVersionScenario(t, "When calling POST on", "/api/dashboards/uid/test-uid/restore", + "/api/dashboards/uid/:uid/restore", dashboardService, dashboardVersionService, dtos.RestoreDashboardVersionCommand{}, func(sc *scenarioContext) { + sc.dashboardVersionService = dashboardVersionService + // Create request with invalid JSON + sc.fakeReqWithParams("POST", "/api/dashboards/uid/test-uid/restore", map[string]string{}) + sc.req.Body = io.NopCloser(strings.NewReader("invalid json")) + sc.req.Header.Set("Content-Type", "application/json") + callRestoreDashboardVersion(sc) + assert.Equal(t, http.StatusBadRequest, sc.resp.Code) + }, dbtest.NewFakeDB()) + }) + + t.Run("should handle restoration with user ID", func(t *testing.T) { + dashboardService := dashboards.NewFakeDashboardService(t) + dashboardVersionService := dashvertest.NewDashboardVersionServiceFake() + + // Mock successful restoration + restoredDash := dashboards.NewDashboard("Restored Dashboard") + restoredDash.ID = 1 + restoredDash.UID = "test-uid" + restoredDash.Version = 6 + restoredDash.Slug = "restored-dashboard" + restoredDash.Data = simplejson.NewFromAny(map[string]any{"title": "Restored Dashboard"}) + + dashboardVersionService.ExpectedRestoreResult = restoredDash + dashboardVersionService.ExpectedError = nil + + cmd := dtos.RestoreDashboardVersionCommand{ + Version: 3, + } + + // Create a custom scenario that sets the user ID to 123 + t.Run("When calling POST on /api/dashboards/uid/test-uid/restore", func(t *testing.T) { + cfg := setting.NewCfg() + folderSvc := foldertest.NewFakeService() + folderSvc.ExpectedFolder = &folder.Folder{} + + hs := HTTPServer{ + Cfg: cfg, + ProvisioningService: provisioning.NewProvisioningServiceMock(context.Background()), + Live: newTestLive(t, db.InitTestDB(t)), + QuotaService: quotatest.New(false, nil), + LibraryPanelService: &mockLibraryPanelService{}, + LibraryElementService: &libraryelementsfake.LibraryElementService{}, + DashboardService: dashboardService, + SQLStore: dbtest.NewFakeDB(), + Features: featuremgmt.WithFeatures(), + dashboardVersionService: dashboardVersionService, + accesscontrolService: actest.FakeService{}, + folderService: folderSvc, + tracer: tracing.InitializeTracerForTest(), + log: log.New("test"), + } + + sc := setupScenarioContext(t, "/api/dashboards/uid/test-uid/restore") + sc.sqlStore = dbtest.NewFakeDB() + sc.dashboardVersionService = dashboardVersionService + sc.defaultHandler = routing.Wrap(func(c *contextmodel.ReqContext) response.Response { + c.Req.Body = mockRequestBody(cmd) + c.Req.Header.Add("Content-Type", "application/json") + sc.context = c + // Set user ID to 123 for this test + c.SignedInUser = &user.SignedInUser{ + OrgID: testOrgID, + UserID: 123, + } + c.OrgRole = org.RoleAdmin + + return hs.RestoreDashboardVersion(c) + }) + + sc.m.Post("/api/dashboards/uid/:uid/restore", sc.defaultHandler) + + callRestoreDashboardVersion(sc) + assert.Equal(t, http.StatusOK, sc.resp.Code) + + // Verify the service was called with correct user ID + assert.True(t, dashboardVersionService.RestoreVersionCalled) + userID, err := dashboardVersionService.LastRestoreCommand.Requester.GetInternalID() + require.NoError(t, err) + assert.Equal(t, int64(123), userID) + }) + }) + }) + t.Run("Given provisioned dashboard", func(t *testing.T) { mockSQLStore := dbtest.NewFakeDB() dashboardService := dashboards.NewFakeDashboardService(t) @@ -999,6 +1138,7 @@ func restoreDashboardVersionScenario(t *testing.T, desc string, url string, rout accesscontrolService: actest.FakeService{}, folderService: folderSvc, tracer: tracing.InitializeTracerForTest(), + log: log.New("test"), } sc := setupScenarioContext(t, url) @@ -1008,11 +1148,11 @@ func restoreDashboardVersionScenario(t *testing.T, desc string, url string, rout c.Req.Body = mockRequestBody(cmd) c.Req.Header.Add("Content-Type", "application/json") sc.context = c - sc.context.SignedInUser = &user.SignedInUser{ + c.SignedInUser = &user.SignedInUser{ OrgID: testOrgID, UserID: testUserID, } - sc.context.OrgRole = org.RoleAdmin + c.OrgRole = org.RoleAdmin return hs.RestoreDashboardVersion(c) }) diff --git a/pkg/services/dashboards/errors.go b/pkg/services/dashboards/errors.go index 93d84209130..e39b1ed4b21 100644 --- a/pkg/services/dashboards/errors.go +++ b/pkg/services/dashboards/errors.go @@ -107,6 +107,10 @@ var ( Reason: "Unique identifier needed to be able to get a dashboard panel", StatusCode: 400, } + ErrDashboardRestoreIdenticalVersion = dashboardaccess.DashboardErr{ + Reason: "Current dashboard is identical to the specified version", + StatusCode: 400, + } ErrProvisionedDashboardNotFound = dashboardaccess.DashboardErr{ Reason: "Dashboard is not provisioned", StatusCode: 404, diff --git a/pkg/services/dashboards/service/client/client.go b/pkg/services/dashboards/service/client/client.go index 123042fb6d5..13d84a1aaa3 100644 --- a/pkg/services/dashboards/service/client/client.go +++ b/pkg/services/dashboards/service/client/client.go @@ -208,6 +208,33 @@ func (h *K8sClientWithFallback) List( return res, nil } +// Update updates a resource in the K8s API. +// It will attempte to use the version of the API which is indicated in the object. +// If the version cannot be retrieved or missing, it will fall back to the preferred version of the API. +func (h *K8sClientWithFallback) Update( + ctx context.Context, obj *unstructured.Unstructured, orgID int64, options metav1.UpdateOptions, +) (*unstructured.Unstructured, error) { + ctx, span := tracing.Start(ctx, "K8sClientWithFallback.Update") + defer span.End() + + version := obj.GroupVersionKind().Version + h.log.Debug("using client for version", "version", version) + + span.SetAttributes( + attribute.String("version", version), + attribute.String("dashboard.metadata.name", obj.GetName()), + attribute.Int64("org.id", orgID), + ) + + res, err := h.newClientFunc(ctx, version).Update(ctx, obj, orgID, options) + if err != nil { + h.log.Debug("failed to update object", "error", err) + return nil, tracing.Error(span, err) + } + + return res, nil +} + // fetchWithVersion fetches multiple resources from the K8s API. // It uses concurrent Get requests, one for each name. // diff --git a/pkg/services/dashboards/service/client/client_test.go b/pkg/services/dashboards/service/client/client_test.go index a1735901c3f..99e34fb3821 100644 --- a/pkg/services/dashboards/service/client/client_test.go +++ b/pkg/services/dashboards/service/client/client_test.go @@ -11,50 +11,70 @@ import ( metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" - dashboardv1 "github.com/grafana/grafana/apps/dashboard/pkg/apis/dashboard/v1beta1" + "github.com/grafana/grafana/apps/dashboard/pkg/apis/dashboard/v0alpha1" + "github.com/grafana/grafana/apps/dashboard/pkg/apis/dashboard/v1beta1" + "github.com/grafana/grafana/apps/dashboard/pkg/apis/dashboard/v2alpha1" + "github.com/grafana/grafana/apps/dashboard/pkg/apis/dashboard/v2beta1" "github.com/grafana/grafana/pkg/infra/log" "github.com/grafana/grafana/pkg/services/apiserver/client" ) type testSetup struct { + t *testing.T handler *K8sClientWithFallback - mockClientV1Alpha1 *client.MockK8sHandler - mockClientV2Alpha1 *client.MockK8sHandler mockMetrics *k8sClientMetrics mockFactoryCalls map[string]int - t *testing.T + mockClientV0Alpha1 *client.MockK8sHandler + mockClientV1Beta1 *client.MockK8sHandler + mockClientV2Alpha1 *client.MockK8sHandler + mockClientV2Beta1 *client.MockK8sHandler } func setupTest(t *testing.T) *testSetup { - mockClientV1Alpha1 := &client.MockK8sHandler{} - mockClientV2Alpha1 := &client.MockK8sHandler{} + var ( + mockClientV0Alpha1 = &client.MockK8sHandler{} + mockClientV1Beta1 = &client.MockK8sHandler{} + mockClientV2Alpha1 = &client.MockK8sHandler{} + mockClientV2Beta1 = &client.MockK8sHandler{} + ) + mockMetrics := newK8sClientMetrics(prometheus.NewRegistry()) mockFactoryCalls := make(map[string]int) handler := &K8sClientWithFallback{ - K8sHandler: mockClientV1Alpha1, - newClientFunc: func(ctx context.Context, version string) client.K8sHandler { + K8sHandler: mockClientV1Beta1, + newClientFunc: func(_ context.Context, version string) client.K8sHandler { mockFactoryCalls[version]++ - if version == "v2alpha1" { + + switch version { + case v0alpha1.VERSION: + return mockClientV0Alpha1 + case v1beta1.VERSION: + return mockClientV1Beta1 + case v2alpha1.VERSION: return mockClientV2Alpha1 + case v2beta1.VERSION: + return mockClientV2Beta1 + case "v1": + return mockClientV1Beta1 + default: + t.Fatalf("Unexpected call to newClientFunc with version %s", version) + return nil } - if version == dashboardv1.VERSION { - return mockClientV1Alpha1 - } - t.Fatalf("Unexpected call to newClientFunc with version %s", version) - return nil }, log: log.New("test"), metrics: mockMetrics, } return &testSetup{ + t: t, handler: handler, - mockClientV1Alpha1: mockClientV1Alpha1, - mockClientV2Alpha1: mockClientV2Alpha1, mockMetrics: mockMetrics, mockFactoryCalls: mockFactoryCalls, - t: t, + mockClientV0Alpha1: mockClientV0Alpha1, + mockClientV1Beta1: mockClientV1Beta1, + mockClientV2Alpha1: mockClientV2Alpha1, + mockClientV2Beta1: mockClientV2Beta1, } } @@ -78,14 +98,14 @@ func TestK8sHandlerWithFallback_Get(t *testing.T) { }, } - setup.mockClientV1Alpha1.On("Get", mock.Anything, name, orgID, options, mock.Anything).Return(expectedResult, nil).Once() + setup.mockClientV1Beta1.On("Get", mock.Anything, name, orgID, options, mock.Anything).Return(expectedResult, nil).Once() result, err := setup.handler.Get(ctx, name, orgID, options) require.NoError(t, err) require.Equal(t, expectedResult, result) require.Equal(t, 0, len(setup.mockFactoryCalls), "Factory should not be called for non-fallback case") - setup.mockClientV1Alpha1.AssertExpectations(t) + setup.mockClientV1Beta1.AssertExpectations(t) setup.mockClientV2Alpha1.AssertExpectations(t) }) @@ -96,7 +116,7 @@ func TestK8sHandlerWithFallback_Get(t *testing.T) { name := "test-dashboard-fallback" orgID := int64(2) options := metav1.GetOptions{ResourceVersion: "123"} - storedVersion := "v2alpha1" + storedVersion := v2alpha1.VERSION conversionErr := "failed to convert" v1alpha1Result := &unstructured.Unstructured{ @@ -124,15 +144,15 @@ func TestK8sHandlerWithFallback_Get(t *testing.T) { }, } - setup.mockClientV1Alpha1.On("Get", mock.Anything, name, orgID, options, mock.Anything).Return(v1alpha1Result, nil).Once() + setup.mockClientV1Beta1.On("Get", mock.Anything, name, orgID, options, mock.Anything).Return(v1alpha1Result, nil).Once() setup.mockClientV2Alpha1.On("Get", mock.Anything, name, orgID, options, mock.Anything).Return(expectedResultFallback, nil).Once() result, err := setup.handler.Get(ctx, name, orgID, options) require.NoError(t, err) require.Equal(t, expectedResultFallback, result) - require.Equal(t, 1, setup.mockFactoryCalls["v2alpha1"], "Factory should be called once with v2alpha1") + require.Equal(t, 1, setup.mockFactoryCalls[v2alpha1.VERSION], "Factory should be called once with v2alpha1") - setup.mockClientV1Alpha1.AssertExpectations(t) + setup.mockClientV1Beta1.AssertExpectations(t) setup.mockClientV2Alpha1.AssertExpectations(t) }) @@ -145,14 +165,14 @@ func TestK8sHandlerWithFallback_Get(t *testing.T) { options := metav1.GetOptions{} expectedErr := errors.New("initial get failed") - setup.mockClientV1Alpha1.On("Get", mock.Anything, name, orgID, options, mock.Anything).Return(nil, expectedErr).Once() + setup.mockClientV1Beta1.On("Get", mock.Anything, name, orgID, options, mock.Anything).Return(nil, expectedErr).Once() _, err := setup.handler.Get(ctx, name, orgID, options) require.Error(t, err) require.Equal(t, expectedErr, err) require.Equal(t, 0, len(setup.mockFactoryCalls), "Factory should not be called for error case") - setup.mockClientV1Alpha1.AssertExpectations(t) + setup.mockClientV1Beta1.AssertExpectations(t) setup.mockClientV2Alpha1.AssertExpectations(t) }) @@ -163,7 +183,7 @@ func TestK8sHandlerWithFallback_Get(t *testing.T) { name := "test-dashboard-fallback-error" orgID := int64(4) options := metav1.GetOptions{} - storedVersion := "v2alpha1" + storedVersion := v2alpha1.VERSION conversionErr := "failed to convert again" fallbackErr := errors.New("fallback get failed") @@ -182,15 +202,15 @@ func TestK8sHandlerWithFallback_Get(t *testing.T) { }, } - setup.mockClientV1Alpha1.On("Get", mock.Anything, name, orgID, options, mock.Anything).Return(v1alpha1Result, nil).Once() + setup.mockClientV1Beta1.On("Get", mock.Anything, name, orgID, options, mock.Anything).Return(v1alpha1Result, nil).Once() setup.mockClientV2Alpha1.On("Get", mock.Anything, name, orgID, options, mock.Anything).Return(nil, fallbackErr).Once() _, err := setup.handler.Get(ctx, name, orgID, options) require.Error(t, err) require.Equal(t, fallbackErr, err) - require.Equal(t, 1, setup.mockFactoryCalls["v2alpha1"], "Factory should be called once with v2alpha1") + require.Equal(t, 1, setup.mockFactoryCalls[v2alpha1.VERSION], "Factory should be called once with v2alpha1") - setup.mockClientV1Alpha1.AssertExpectations(t) + setup.mockClientV1Beta1.AssertExpectations(t) setup.mockClientV2Alpha1.AssertExpectations(t) }) } @@ -243,13 +263,13 @@ func TestK8sHandlerWithFallback_List(t *testing.T) { }, } - setup.mockClientV1Alpha1.On("List", mock.Anything, int64(1), metav1.ListOptions{}).Return(expectedResult, nil).Once() + setup.mockClientV1Beta1.On("List", mock.Anything, int64(1), metav1.ListOptions{}).Return(expectedResult, nil).Once() result, err := setup.handler.List(context.Background(), 1, metav1.ListOptions{}) require.NoError(t, err) require.Equal(t, expectedResult, result) - setup.mockClientV1Alpha1.AssertExpectations(t) + setup.mockClientV1Beta1.AssertExpectations(t) setup.mockClientV2Alpha1.AssertExpectations(t) }) @@ -258,13 +278,13 @@ func TestK8sHandlerWithFallback_List(t *testing.T) { initialResult := &unstructured.UnstructuredList{ Items: []unstructured.Unstructured{ createDashboard("dashboard-ok", "123", map[string]interface{}{"someOtherStatus": "ok"}), - createDashboard("dashboard-fallback", "456", conversionStatus(true, "v2alpha1", "conversion failed")), + createDashboard("dashboard-fallback", "456", conversionStatus(true, v2alpha1.VERSION, "conversion failed")), }, } - fallbackResult := createFallbackDashboard("dashboard-fallback", "456", "dashboard/v2alpha1") + fallbackResult := createFallbackDashboard("dashboard-fallback", "456", "dashboard/"+v2alpha1.VERSION) - setup.mockClientV1Alpha1.On("List", mock.Anything, int64(2), metav1.ListOptions{}).Return(initialResult, nil).Once() + setup.mockClientV1Beta1.On("List", mock.Anything, int64(2), metav1.ListOptions{}).Return(initialResult, nil).Once() setup.mockClientV2Alpha1.On("Get", mock.Anything, "dashboard-fallback", int64(2), metav1.GetOptions{ResourceVersion: "456"}, mock.Anything).Return(&fallbackResult, nil).Once() result, err := setup.handler.List(context.Background(), 2, metav1.ListOptions{}) @@ -277,7 +297,7 @@ func TestK8sHandlerWithFallback_List(t *testing.T) { } require.ElementsMatch(t, expectedItems, result.Items) - setup.mockClientV1Alpha1.AssertExpectations(t) + setup.mockClientV1Beta1.AssertExpectations(t) setup.mockClientV2Alpha1.AssertExpectations(t) }) @@ -285,15 +305,15 @@ func TestK8sHandlerWithFallback_List(t *testing.T) { setup := setupTest(t) initialResult := &unstructured.UnstructuredList{ Items: []unstructured.Unstructured{ - createDashboard("dashboard-1-fallback", "111", conversionStatus(true, "v2alpha1", "conversion failed 1")), - createDashboard("dashboard-2-fallback", "222", conversionStatus(true, "v2alpha1", "conversion failed 2")), + createDashboard("dashboard-1-fallback", "111", conversionStatus(true, v2alpha1.VERSION, "conversion failed 1")), + createDashboard("dashboard-2-fallback", "222", conversionStatus(true, v2alpha1.VERSION, "conversion failed 2")), }, } - fallbackResult1 := createFallbackDashboard("dashboard-1-fallback", "111", "dashboard/v2alpha1") - fallbackResult2 := createFallbackDashboard("dashboard-2-fallback", "222", "dashboard/v2alpha1") + fallbackResult1 := createFallbackDashboard("dashboard-1-fallback", "111", "dashboard/"+v2alpha1.VERSION) + fallbackResult2 := createFallbackDashboard("dashboard-2-fallback", "222", "dashboard/"+v2alpha1.VERSION) - setup.mockClientV1Alpha1.On("List", mock.Anything, int64(3), metav1.ListOptions{}).Return(initialResult, nil).Once() + setup.mockClientV1Beta1.On("List", mock.Anything, int64(3), metav1.ListOptions{}).Return(initialResult, nil).Once() setup.mockClientV2Alpha1.On("Get", mock.Anything, "dashboard-1-fallback", int64(3), metav1.GetOptions{ResourceVersion: "111"}, mock.Anything).Return(&fallbackResult1, nil).Once() setup.mockClientV2Alpha1.On("Get", mock.Anything, "dashboard-2-fallback", int64(3), metav1.GetOptions{ResourceVersion: "222"}, mock.Anything).Return(&fallbackResult2, nil).Once() @@ -304,7 +324,7 @@ func TestK8sHandlerWithFallback_List(t *testing.T) { expectedItems := []unstructured.Unstructured{fallbackResult1, fallbackResult2} require.ElementsMatch(t, expectedItems, result.Items) - setup.mockClientV1Alpha1.AssertExpectations(t) + setup.mockClientV1Beta1.AssertExpectations(t) setup.mockClientV2Alpha1.AssertExpectations(t) }) @@ -312,17 +332,17 @@ func TestK8sHandlerWithFallback_List(t *testing.T) { setup := setupTest(t) initialResult := &unstructured.UnstructuredList{ Items: []unstructured.Unstructured{ - createDashboard("dashboard-v2alpha1", "333", conversionStatus(true, "v2alpha1", "conversion failed v2alpha1")), - createDashboard("dashboard-v1beta1", "444", conversionStatus(true, "v1beta1", "conversion failed v1beta1")), + createDashboard("dashboard-v2alpha1", "333", conversionStatus(true, v2alpha1.VERSION, "conversion failed v2alpha1")), + createDashboard("dashboard-v1beta1", "444", conversionStatus(true, v1beta1.VERSION, "conversion failed v1beta1")), }, } - fallbackResultV2Alpha1 := createFallbackDashboard("dashboard-v2alpha1", "333", "dashboard/v2alpha1") - fallbackResultV1Beta1 := createFallbackDashboard("dashboard-v1beta1", "444", "dashboard/v1beta1") + fallbackResultV2Alpha1 := createFallbackDashboard("dashboard-v2alpha1", "333", "dashboard/"+v2alpha1.VERSION) + fallbackResultV1Beta1 := createFallbackDashboard("dashboard-v1beta1", "444", "dashboard/"+v1beta1.VERSION) - setup.mockClientV1Alpha1.On("List", mock.Anything, int64(4), metav1.ListOptions{}).Return(initialResult, nil).Once() + setup.mockClientV1Beta1.On("List", mock.Anything, int64(4), metav1.ListOptions{}).Return(initialResult, nil).Once() setup.mockClientV2Alpha1.On("Get", mock.Anything, "dashboard-v2alpha1", int64(4), metav1.GetOptions{ResourceVersion: "333"}, mock.Anything).Return(&fallbackResultV2Alpha1, nil).Once() - setup.mockClientV1Alpha1.On("Get", mock.Anything, "dashboard-v1beta1", int64(4), metav1.GetOptions{ResourceVersion: "444"}, mock.Anything).Return(&fallbackResultV1Beta1, nil).Once() + setup.mockClientV1Beta1.On("Get", mock.Anything, "dashboard-v1beta1", int64(4), metav1.GetOptions{ResourceVersion: "444"}, mock.Anything).Return(&fallbackResultV1Beta1, nil).Once() result, err := setup.handler.List(context.Background(), 4, metav1.ListOptions{}) require.NoError(t, err) @@ -331,7 +351,7 @@ func TestK8sHandlerWithFallback_List(t *testing.T) { expectedItems := []unstructured.Unstructured{fallbackResultV2Alpha1, fallbackResultV1Beta1} require.ElementsMatch(t, expectedItems, result.Items) - setup.mockClientV1Alpha1.AssertExpectations(t) + setup.mockClientV1Beta1.AssertExpectations(t) setup.mockClientV2Alpha1.AssertExpectations(t) }) @@ -339,13 +359,13 @@ func TestK8sHandlerWithFallback_List(t *testing.T) { setup := setupTest(t) expectedErr := errors.New("initial list failed") - setup.mockClientV1Alpha1.On("List", mock.Anything, int64(5), metav1.ListOptions{}).Return(nil, expectedErr).Once() + setup.mockClientV1Beta1.On("List", mock.Anything, int64(5), metav1.ListOptions{}).Return(nil, expectedErr).Once() _, err := setup.handler.List(context.Background(), 5, metav1.ListOptions{}) require.Error(t, err) require.Equal(t, expectedErr, err) - setup.mockClientV1Alpha1.AssertExpectations(t) + setup.mockClientV1Beta1.AssertExpectations(t) setup.mockClientV2Alpha1.AssertExpectations(t) }) @@ -353,20 +373,20 @@ func TestK8sHandlerWithFallback_List(t *testing.T) { setup := setupTest(t) initialResult := &unstructured.UnstructuredList{ Items: []unstructured.Unstructured{ - createDashboard("dashboard-fallback-error", "555", conversionStatus(true, "v2alpha1", "conversion failed")), + createDashboard("dashboard-fallback-error", "555", conversionStatus(true, v2alpha1.VERSION, "conversion failed")), }, } fallbackErr := errors.New("fallback get failed") - setup.mockClientV1Alpha1.On("List", mock.Anything, int64(6), metav1.ListOptions{}).Return(initialResult, nil).Once() + setup.mockClientV1Beta1.On("List", mock.Anything, int64(6), metav1.ListOptions{}).Return(initialResult, nil).Once() setup.mockClientV2Alpha1.On("Get", mock.Anything, "dashboard-fallback-error", int64(6), metav1.GetOptions{ResourceVersion: "555"}, mock.Anything).Return(nil, fallbackErr).Once() _, err := setup.handler.List(context.Background(), 6, metav1.ListOptions{}) require.Error(t, err) require.Equal(t, fallbackErr, err) - setup.mockClientV1Alpha1.AssertExpectations(t) + setup.mockClientV1Beta1.AssertExpectations(t) setup.mockClientV2Alpha1.AssertExpectations(t) }) @@ -374,13 +394,203 @@ func TestK8sHandlerWithFallback_List(t *testing.T) { setup := setupTest(t) emptyResult := &unstructured.UnstructuredList{Items: []unstructured.Unstructured{}} - setup.mockClientV1Alpha1.On("List", mock.Anything, int64(7), metav1.ListOptions{}).Return(emptyResult, nil).Once() + setup.mockClientV1Beta1.On("List", mock.Anything, int64(7), metav1.ListOptions{}).Return(emptyResult, nil).Once() result, err := setup.handler.List(context.Background(), 7, metav1.ListOptions{}) require.NoError(t, err) require.Len(t, result.Items, 0) - setup.mockClientV1Alpha1.AssertExpectations(t) + setup.mockClientV1Beta1.AssertExpectations(t) + setup.mockClientV2Alpha1.AssertExpectations(t) + }) +} + +func TestK8sHandlerWithFallback_Update(t *testing.T) { + t.Run("Update without fallback", func(t *testing.T) { + setup := setupTest(t) + + ctx := context.Background() + obj := &unstructured.Unstructured{ + Object: map[string]interface{}{ + "apiVersion": v0alpha1.VERSION, + "metadata": map[string]interface{}{ + "name": "test-dashboard", + }, + "spec": map[string]interface{}{ + "title": "Updated Dashboard", + }, + }, + } + orgID := int64(1) + options := metav1.UpdateOptions{} + + expectedResult := &unstructured.Unstructured{ + Object: map[string]interface{}{ + "apiVersion": v0alpha1.VERSION, + "metadata": map[string]interface{}{ + "name": "test-dashboard", + "resourceVersion": "123", + }, + "spec": map[string]interface{}{ + "title": "Updated Dashboard", + }, + }, + } + + setup.mockClientV0Alpha1.On("Update", mock.Anything, obj, orgID, options).Return(expectedResult, nil).Once() + + result, err := setup.handler.Update(ctx, obj, orgID, options) + require.NoError(t, err) + require.Equal(t, expectedResult, result) + require.Equal(t, 1, setup.mockFactoryCalls[v0alpha1.VERSION], "Factory should be called once with v0alpha1") + + setup.mockClientV0Alpha1.AssertExpectations(t) + setup.mockClientV2Alpha1.AssertExpectations(t) + }) + + t.Run("Update with different API version", func(t *testing.T) { + setup := setupTest(t) + + ctx := context.Background() + obj := &unstructured.Unstructured{ + Object: map[string]interface{}{ + "apiVersion": v2alpha1.VERSION, + "metadata": map[string]interface{}{ + "name": "test-dashboard-v2", + }, + "spec": map[string]interface{}{ + "title": "Updated Dashboard V2", + }, + }, + } + orgID := int64(2) + options := metav1.UpdateOptions{} + + expectedResult := &unstructured.Unstructured{ + Object: map[string]interface{}{ + "apiVersion": v2alpha1.VERSION, + "metadata": map[string]interface{}{ + "name": "test-dashboard-v2", + "resourceVersion": "456", + }, + "spec": map[string]interface{}{ + "title": "Updated Dashboard V2", + }, + }, + } + + setup.mockClientV2Alpha1.On("Update", mock.Anything, obj, orgID, options).Return(expectedResult, nil).Once() + + result, err := setup.handler.Update(ctx, obj, orgID, options) + require.NoError(t, err) + require.Equal(t, expectedResult, result) + require.Equal(t, 1, setup.mockFactoryCalls[v2alpha1.VERSION], "Factory should be called once with v2alpha1") + + setup.mockClientV1Beta1.AssertExpectations(t) + setup.mockClientV2Alpha1.AssertExpectations(t) + }) + + t.Run("Update with error", func(t *testing.T) { + setup := setupTest(t) + + ctx := context.Background() + obj := &unstructured.Unstructured{ + Object: map[string]interface{}{ + "apiVersion": v0alpha1.VERSION, + "metadata": map[string]interface{}{ + "name": "test-dashboard-error", + }, + "spec": map[string]interface{}{ + "title": "Error Dashboard", + }, + }, + } + orgID := int64(3) + options := metav1.UpdateOptions{} + expectedErr := errors.New("update failed") + + setup.mockClientV0Alpha1.On("Update", mock.Anything, obj, orgID, options).Return(nil, expectedErr).Once() + + result, err := setup.handler.Update(ctx, obj, orgID, options) + require.Error(t, err) + require.Nil(t, result) + require.Equal(t, expectedErr, err) + require.Equal(t, 1, setup.mockFactoryCalls[v0alpha1.VERSION], "Factory should be called once with v0alpha1") + + setup.mockClientV0Alpha1.AssertExpectations(t) + setup.mockClientV2Alpha1.AssertExpectations(t) + }) + + t.Run("Update with different version and error", func(t *testing.T) { + setup := setupTest(t) + + ctx := context.Background() + obj := &unstructured.Unstructured{ + Object: map[string]interface{}{ + "apiVersion": v2alpha1.VERSION, + "metadata": map[string]interface{}{ + "name": "test-dashboard-v2-error", + }, + "spec": map[string]interface{}{ + "title": "Error Dashboard V2", + }, + }, + } + orgID := int64(4) + options := metav1.UpdateOptions{} + expectedErr := errors.New("v2alpha1 update failed") + + setup.mockClientV2Alpha1.On("Update", mock.Anything, obj, orgID, options).Return(nil, expectedErr).Once() + + result, err := setup.handler.Update(ctx, obj, orgID, options) + require.Error(t, err) + require.Nil(t, result) + require.Equal(t, expectedErr, err) + require.Equal(t, 1, setup.mockFactoryCalls[v2alpha1.VERSION], "Factory should be called once with v2alpha1") + + setup.mockClientV1Beta1.AssertExpectations(t) + setup.mockClientV2Alpha1.AssertExpectations(t) + }) + + t.Run("Update with unknown API version", func(t *testing.T) { + setup := setupTest(t) + + ctx := context.Background() + obj := &unstructured.Unstructured{ + Object: map[string]interface{}{ + "apiVersion": "unknown/v1", + "metadata": map[string]interface{}{ + "name": "test-dashboard-unknown", + }, + "spec": map[string]interface{}{ + "title": "Unknown Dashboard", + }, + }, + } + orgID := int64(5) + options := metav1.UpdateOptions{} + + expectedResult := &unstructured.Unstructured{ + Object: map[string]interface{}{ + "apiVersion": "unknown/v1", + "metadata": map[string]interface{}{ + "name": "test-dashboard-unknown", + "resourceVersion": "789", + }, + "spec": map[string]interface{}{ + "title": "Unknown Dashboard", + }, + }, + } + + setup.mockClientV1Beta1.On("Update", mock.Anything, obj, orgID, options).Return(expectedResult, nil).Once() + + result, err := setup.handler.Update(ctx, obj, orgID, options) + require.NoError(t, err) + require.Equal(t, expectedResult, result) + require.Equal(t, 1, setup.mockFactoryCalls["v1"], "Factory should be called once with v1") + + setup.mockClientV1Beta1.AssertExpectations(t) setup.mockClientV2Alpha1.AssertExpectations(t) }) } @@ -419,7 +629,7 @@ func TestGetConversionStatus(t *testing.T) { "status": map[string]interface{}{ "conversion": map[string]interface{}{ "failed": true, - "storedVersion": "v2alpha1", + "storedVersion": v2alpha1.VERSION, "error": "conversion failed", }, }, @@ -483,7 +693,7 @@ func TestGetConversionStatus(t *testing.T) { "status": map[string]interface{}{ "conversion": map[string]interface{}{ "failed": true, - "storedVersion": "v2alpha1", + "storedVersion": v2alpha1.VERSION, }, }, }}, diff --git a/pkg/services/dashboardversion/dashver.go b/pkg/services/dashboardversion/dashver.go index 9d4dcb60264..c51540f2e7f 100644 --- a/pkg/services/dashboardversion/dashver.go +++ b/pkg/services/dashboardversion/dashver.go @@ -2,10 +2,13 @@ package dashver import ( "context" + + "github.com/grafana/grafana/pkg/services/dashboards" ) type Service interface { Get(context.Context, *GetDashboardVersionQuery) (*DashboardVersionDTO, error) DeleteExpired(context.Context, *DeleteExpiredVersionsCommand) error List(context.Context, *ListDashboardVersionsQuery) (*DashboardVersionResponse, error) + RestoreVersion(context.Context, *RestoreVersionCommand) (*dashboards.Dashboard, error) } diff --git a/pkg/services/dashboardversion/dashverimpl/dashver.go b/pkg/services/dashboardversion/dashverimpl/dashver.go index 3f44895e4bc..858116ee9b7 100644 --- a/pkg/services/dashboardversion/dashverimpl/dashver.go +++ b/pkg/services/dashboardversion/dashverimpl/dashver.go @@ -4,19 +4,27 @@ import ( "context" "errors" "fmt" + "reflect" "strconv" "strings" + "time" + "go.opentelemetry.io/otel" + "golang.org/x/sync/errgroup" apierrors "k8s.io/apimachinery/pkg/api/errors" v1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" - dashboardv2alpha1 "github.com/grafana/grafana/apps/dashboard/pkg/apis/dashboard/v2alpha1" - dashboardv2beta1 "github.com/grafana/grafana/apps/dashboard/pkg/apis/dashboard/v2beta1" + "github.com/grafana/grafana/apps/dashboard/pkg/apis/dashboard/v0alpha1" + "github.com/grafana/grafana/apps/dashboard/pkg/apis/dashboard/v1beta1" + "github.com/grafana/grafana/apps/dashboard/pkg/apis/dashboard/v2alpha1" + "github.com/grafana/grafana/apps/dashboard/pkg/apis/dashboard/v2beta1" + "github.com/grafana/grafana/pkg/apimachinery/identity" "github.com/grafana/grafana/pkg/apimachinery/utils" "github.com/grafana/grafana/pkg/components/simplejson" "github.com/grafana/grafana/pkg/infra/db" "github.com/grafana/grafana/pkg/infra/log" + "github.com/grafana/grafana/pkg/infra/tracing" "github.com/grafana/grafana/pkg/services/dashboards" dashboardclient "github.com/grafana/grafana/pkg/services/dashboards/service/client" dashver "github.com/grafana/grafana/pkg/services/dashboardversion" @@ -25,6 +33,8 @@ import ( "github.com/grafana/grafana/pkg/setting" ) +var tracer = otel.Tracer("github.com/grafana/grafana/pkg/services/dashboardversion/dashverimpl") + const ( maxVersionsToDeletePerBatch = 100 maxVersionDeletionBatches = 50 @@ -68,11 +78,12 @@ func (s *Service) Get(ctx context.Context, query *dashver.GetDashboardVersionQue query.DashboardUID = u } - version, err := s.getHistoryThroughK8s(ctx, query.OrgID, query.DashboardUID, query.Version) + versionObj, err := s.getDashboardVersionThroughK8s(ctx, query.OrgID, query.DashboardUID, query.Version) if err != nil { return nil, err } - return version, nil + + return s.transformUnstructuredToLegacyDTO(ctx, versionObj) } func (s *Service) DeleteExpired(ctx context.Context, cmd *dashver.DeleteExpiredVersionsCommand) error { @@ -102,11 +113,14 @@ func (s *Service) DeleteExpired(ctx context.Context, cmd *dashver.DeleteExpiredV break } } + return nil } // List all dashboard versions for the given dashboard ID. -func (s *Service) List(ctx context.Context, query *dashver.ListDashboardVersionsQuery) (*dashver.DashboardVersionResponse, error) { +func (s *Service) List( + ctx context.Context, query *dashver.ListDashboardVersionsQuery, +) (*dashver.DashboardVersionResponse, error) { if query.DashboardUID == "" { u, err := s.getDashUIDMaybeEmpty(ctx, query.DashboardID) if err != nil { @@ -119,7 +133,7 @@ func (s *Service) List(ctx context.Context, query *dashver.ListDashboardVersions query.Limit = 1000 } - versions, err := s.listHistoryThroughK8s( + list, err := s.listDashboardVersionsThroughK8s( ctx, query.OrgID, query.DashboardUID, @@ -129,30 +143,62 @@ func (s *Service) List(ctx context.Context, query *dashver.ListDashboardVersions if err != nil { return nil, err } - return versions, nil -} -// getDashUIDMaybeEmpty is a helper function which takes a dashboardID and -// returns the UID. If the dashboard is not found, it will return an empty -// string. -func (s *Service) getDashUIDMaybeEmpty(ctx context.Context, id int64) (string, error) { - q := dashboards.GetDashboardRefByIDQuery{ID: id} - result, err := s.dashSvc.GetDashboardUIDByID(ctx, &q) + dashboards, err := s.transformUnstructuredToLegacyDTOList(ctx, list.Items) if err != nil { - if errors.Is(err, dashboards.ErrDashboardNotFound) { - s.log.Debug("dashboard not found") - return "", nil - } else { - s.log.Error("error getting dashboard", err) - return "", err - } + return nil, err } - return result.UID, nil + + return &dashver.DashboardVersionResponse{ + ContinueToken: list.GetContinue(), + Versions: dashboards, + }, nil } -func (s *Service) getHistoryThroughK8s(ctx context.Context, orgID int64, dashboardUID string, version int64) (*dashver.DashboardVersionDTO, error) { - // this is an unideal implementation - we have to list all versions and filter here, since there currently is no way to query for the - // generation id in unified storage, so we cannot query for the dashboard version directly, and we cannot use search as history is not indexed. +// RestoreVersion restores a dashboard version. +func (s *Service) RestoreVersion(ctx context.Context, cmd *dashver.RestoreVersionCommand) (*dashboards.Dashboard, error) { + ctx, span := tracer.Start(ctx, "Service.RestoreVersion") + defer span.End() + + // Get dashboard UID if not provided + if cmd.DashboardUID == "" { + u, err := s.getDashUIDMaybeEmpty(ctx, cmd.DashboardID) + if err != nil { + s.log.Debug("error getting dashboard UID", "error", err) + return nil, tracing.Error(span, err) + } + cmd.DashboardUID = u + } + + if s.features.IsEnabledGlobally(featuremgmt.FlagKubernetesDashboards) || + s.features.IsEnabledGlobally(featuremgmt.FlagDashboardNewLayouts) { + s.log.Debug("restoring dashboard version through k8s") + res, err := s.restoreVersionThroughK8s(ctx, cmd) + if err != nil { + s.log.Debug("error restoring dashboard version through k8s", "error", err) + return nil, tracing.Error(span, err) + } + + return res, nil + } + + s.log.Debug("restoring dashboard version through legacy") + res, err := s.restoreVersionLegacy(ctx, cmd) + if err != nil { + s.log.Debug("error restoring dashboard version through legacy", "error", err) + return nil, tracing.Error(span, err) + } + + return res, nil +} + +func (s *Service) getDashboardVersionThroughK8s( + ctx context.Context, orgID int64, dashboardUID string, version int64, +) (*unstructured.Unstructured, error) { + // this is an unideal implementation - we have to list all versions and filter here, + // since there currently is no way to query for the + // generation id in unified storage, so we cannot query for the dashboard version directly, + // and we cannot use search as history is not indexed. // use batches to make sure we don't load too much data at once. const batchSize = 50 labelSelector := utils.LabelKeyGetHistory + "=true" @@ -177,7 +223,7 @@ func (s *Service) getHistoryThroughK8s(ctx context.Context, orgID int64, dashboa for _, item := range out.Items { if item.GetGeneration() == version { - return s.UnstructuredToLegacyDashboardVersion(ctx, &item, orgID) + return &item, nil } } @@ -190,7 +236,9 @@ func (s *Service) getHistoryThroughK8s(ctx context.Context, orgID int64, dashboa return nil, dashboards.ErrDashboardNotFound } -func (s *Service) listHistoryThroughK8s(ctx context.Context, orgID int64, dashboardUID string, limit int64, continueToken string) (*dashver.DashboardVersionResponse, error) { +func (s *Service) listDashboardVersionsThroughK8s( + ctx context.Context, orgID int64, dashboardUID string, limit int64, continueToken string, +) (*unstructured.UnstructuredList, error) { labelSelector := utils.LabelKeyGetHistory + "=true" fieldSelector := "metadata.name=" + dashboardUID out, err := s.k8sclient.List(ctx, orgID, v1.ListOptions{ @@ -226,30 +274,196 @@ func (s *Service) listHistoryThroughK8s(ctx context.Context, orgID int64, dashbo continueToken = tempOut.GetContinue() } - dashboards, err := s.UnstructuredToLegacyDashboardVersionList(ctx, out.Items, orgID) - if err != nil { - return nil, err - } - - return &dashver.DashboardVersionResponse{ - ContinueToken: continueToken, - Versions: dashboards, - }, nil + return out, nil } -func (s *Service) UnstructuredToLegacyDashboardVersion(ctx context.Context, item *unstructured.Unstructured, orgID int64) (*dashver.DashboardVersionDTO, error) { +func (s *Service) restoreVersionThroughK8s( + ctx context.Context, cmd *dashver.RestoreVersionCommand, +) (*dashboards.Dashboard, error) { + ctx, span := tracer.Start(ctx, "Service.restoreVersionThroughK8s") + defer span.End() + + // We must use separate gctx context here, because it will be canceled, once the group is done. + // If we use the same ctx after the call to g.Wait, it will already be canceled at that point, + // causing all subsequent context-using operations to immediately return with "context canceled" error. + g, gctx := errgroup.WithContext(ctx) + + var current *unstructured.Unstructured + g.Go(func() error { + var err error + current, err = s.k8sclient.Get(gctx, cmd.DashboardUID, cmd.Requester.GetOrgID(), v1.GetOptions{}) + if err != nil { + s.log.Debug("error getting current dashboard", "error", err) + } + return err + }) + + var version *unstructured.Unstructured + g.Go(func() error { + var err error + version, err = s.getDashboardVersionThroughK8s(gctx, cmd.Requester.GetOrgID(), cmd.DashboardUID, cmd.Version) + if err != nil { + s.log.Debug("error getting version", "error", err) + } + return err + }) + + if err := g.Wait(); err != nil { + return nil, tracing.Error(span, err) + } + + // Compare dashboard data using the new version-aware comparator + identical, err := compareUnstructuredDashboards(version, current) + if err != nil { + s.log.Debug("error comparing dashboard versions", "error", err) + return nil, tracing.Error(span, err) + } + if identical { + return nil, dashboards.ErrDashboardRestoreIdenticalVersion + } + + versionMeta, err := utils.MetaAccessor(version) + if err != nil { + s.log.Debug("error getting old version meta accessor", "error", err) + return nil, tracing.Error(span, err) + } + spec, err := versionMeta.GetSpec() + if err != nil { + s.log.Debug("error getting old version spec", "error", err) + return nil, tracing.Error(span, err) + } + + currentMeta, err := utils.MetaAccessor(current) + if err != nil { + s.log.Debug("error getting current meta accessor", "error", err) + return nil, tracing.Error(span, err) + } + currentMeta.SetMessage(dashboardRestoreMessage(int(versionMeta.GetGeneration()))) + if err := currentMeta.SetSpec(spec); err != nil { + s.log.Debug("error setting current version spec", "error", err) + return nil, tracing.Error(span, err) + } + + updatedObj, err := s.k8sclient.Update(ctx, current, cmd.Requester.GetOrgID(), v1.UpdateOptions{}) + if err != nil { + s.log.Debug("error updating dashboard to specified version", "error", err) + return nil, tracing.Error(span, err) + } + + res, err := s.dashSvc.UnstructuredToLegacyDashboard(ctx, updatedObj, cmd.Requester.GetOrgID()) + if err != nil { + s.log.Debug("error converting dashboard to legacy dashboard", "error", err) + return nil, tracing.Error(span, err) + } + + return res, nil +} + +func (s *Service) restoreVersionLegacy( + ctx context.Context, cmd *dashver.RestoreVersionCommand, +) (*dashboards.Dashboard, error) { + ctx, span := tracer.Start(ctx, "Service.restoreVersionLegacy") + defer span.End() + + // We must use separate gctx context here, because it will be canceled, once the group is done. + // If we use the same ctx after the call to g.Wait, it will already be canceled at that point, + // causing all subsequent context-using operations to immediately return with "context canceled" error. + g, gctx := errgroup.WithContext(ctx) + + var currentDash *dashboards.Dashboard + g.Go(func() error { + var err error + currentDash, err = s.dashSvc.GetDashboard(gctx, &dashboards.GetDashboardQuery{ + UID: cmd.DashboardUID, + OrgID: cmd.Requester.GetOrgID(), + }) + if err != nil { + s.log.Debug("error getting dashboard", "error", err) + } + return err + }) + + var versionData *dashver.DashboardVersionDTO + g.Go(func() error { + versionObj, err := s.getDashboardVersionThroughK8s(gctx, cmd.Requester.GetOrgID(), cmd.DashboardUID, cmd.Version) + if err != nil { + s.log.Debug("error getting dashboard version", "error", err) + return err + } + + versionData, err = s.transformUnstructuredToLegacyDTO(gctx, versionObj) + if err != nil { + s.log.Debug("error transforming dashboard version to DTO", "error", err) + return err + } + + return nil + }) + + if err := g.Wait(); err != nil { + return nil, tracing.Error(span, err) + } + + if compareDashboardData(versionData.Data.MustMap(), currentDash.Data.MustMap(), true) { + return nil, dashboards.ErrDashboardRestoreIdenticalVersion + } + + userID, err := identity.UserIdentifier(cmd.Requester.GetID()) + if err != nil { + s.log.Debug("error getting user identifier", "error", err) + return nil, tracing.Error(span, err) + } + + // This logic has been copied from the API handler unmodified for the most part. + // There is some strange back-and-forth conversions between the two commands, + // that should ideally be cleaned up. + saveCmd := dashboards.SaveDashboardCommand{ + RestoredFrom: versionData.Version, + OrgID: cmd.Requester.GetOrgID(), + UserID: userID, + Dashboard: versionData.Data, + FolderUID: currentDash.FolderUID, + } + saveCmd.Dashboard.Set("version", currentDash.Version) + saveCmd.Dashboard.Set("uid", currentDash.UID) + dash := saveCmd.GetDashboardModel() + dashItem := &dashboards.SaveDashboardDTO{ + User: cmd.Requester, + OrgID: cmd.Requester.GetOrgID(), + UpdatedAt: time.Now(), + Message: dashboardRestoreMessage(versionData.Version), + Overwrite: false, + Dashboard: dash, + } + + res, err := s.dashSvc.SaveDashboard(ctx, dashItem, true) + if err != nil { + s.log.Debug("error saving dashboard", "error", err) + return nil, tracing.Error(span, err) + } + + return res, nil +} + +func (s *Service) transformUnstructuredToLegacyDTO( + ctx context.Context, item *unstructured.Unstructured, +) (*dashver.DashboardVersionDTO, error) { obj, err := utils.MetaAccessor(item) if err != nil { return nil, err } + users, err := s.k8sclient.GetUsersFromMeta(ctx, []string{obj.GetCreatedBy(), obj.GetUpdatedBy()}) if err != nil { return nil, err } - return s.unstructuredToLegacyDashboardVersionWithUsers(item, users) + + return unstructuredToLegacyDashboardVersionWithUsers(item, users) } -func (s *Service) UnstructuredToLegacyDashboardVersionList(ctx context.Context, items []unstructured.Unstructured, orgID int64) ([]*dashver.DashboardVersionDTO, error) { +func (s *Service) transformUnstructuredToLegacyDTOList( + ctx context.Context, items []unstructured.Unstructured, +) ([]*dashver.DashboardVersionDTO, error) { // get users ahead of time to do just one db call, rather than 2 per item in the list userMeta := []string{} for _, item := range items { @@ -272,7 +486,7 @@ func (s *Service) UnstructuredToLegacyDashboardVersionList(ctx context.Context, versions := make([]*dashver.DashboardVersionDTO, len(items)) for i, item := range items { - version, err := s.unstructuredToLegacyDashboardVersionWithUsers(&item, users) + version, err := unstructuredToLegacyDashboardVersionWithUsers(&item, users) if err != nil { return nil, err } @@ -282,70 +496,21 @@ func (s *Service) UnstructuredToLegacyDashboardVersionList(ctx context.Context, return versions, nil } -func (s *Service) unstructuredToLegacyDashboardVersionWithUsers(item *unstructured.Unstructured, users map[string]*user.User) (*dashver.DashboardVersionDTO, error) { - var vspec DashboardVersionSpec - if err := UnstructuredToDashboardVersionSpec(item, &vspec); err != nil { - return nil, err - } - - obj := vspec.MetaAccessor - - var createdBy *user.User - if creator, ok := users[obj.GetCreatedBy()]; ok { - createdBy = creator - } - // if updated by is set, then this version of the dashboard was "created" - // by that user - if updater, ok := users[obj.GetUpdatedBy()]; ok { - createdBy = updater - } - - createdByID := int64(0) - if createdBy != nil { - createdByID = createdBy.ID - } - - created := obj.GetCreationTimestamp().Time - if updated, err := obj.GetUpdatedTimestamp(); err == nil && updated != nil { - created = *updated - } - - restoreVer, err := getRestoreVersion(obj.GetMessage()) +// getDashUIDMaybeEmpty is a helper function which takes a dashboardID and returns the UID. +// If the dashboard is not found, it will return an empty string. +func (s *Service) getDashUIDMaybeEmpty(ctx context.Context, id int64) (string, error) { + q := dashboards.GetDashboardRefByIDQuery{ID: id} + result, err := s.dashSvc.GetDashboardUIDByID(ctx, &q) if err != nil { - return nil, err + if errors.Is(err, dashboards.ErrDashboardNotFound) { + s.log.Debug("dashboard not found") + return "", nil + } else { + s.log.Error("error getting dashboard", err) + return "", err + } } - - return &dashver.DashboardVersionDTO{ - ID: vspec.Version, - DashboardID: obj.GetDeprecatedInternalID(), // nolint:staticcheck - DashboardUID: vspec.UID, - Created: created, - CreatedBy: createdByID, - Message: obj.GetMessage(), - RestoredFrom: restoreVer, - Version: int(vspec.Version), - ParentVersion: int(vspec.ParentVersion), - Data: simplejson.NewFromAny(vspec.Spec), - }, nil -} - -var restoreMsg = "Restored from version " - -func DashboardRestoreMessage(version int) string { - return fmt.Sprintf("%s%d", restoreMsg, version) -} - -func getRestoreVersion(msg string) (int, error) { - parts := strings.Split(msg, restoreMsg) - if len(parts) < 2 { - return 0, nil - } - - ver, err := strconv.Atoi(parts[1]) - if err != nil { - return 0, err - } - return ver, nil + return result.UID, nil } // DashboardVersionSpec contains the necessary fields to represent a dashboard version. @@ -360,8 +525,8 @@ type DashboardVersionSpec struct { // UnstructuredToDashboardVersionSpec converts a k8s unstructured object to a DashboardVersionSpec. // It supports dashboard API versions v0alpha1 through v2beta1. func UnstructuredToDashboardVersionSpec(obj *unstructured.Unstructured, dst *DashboardVersionSpec) error { - if obj.GetAPIVersion() == dashboardv2alpha1.GroupVersion.String() || - obj.GetAPIVersion() == dashboardv2beta1.GroupVersion.String() { + if obj.GetAPIVersion() == v2alpha1.GroupVersion.String() || + obj.GetAPIVersion() == v2beta1.GroupVersion.String() { spec, ok := obj.Object["spec"] if !ok { return errors.New("error parsing dashboard from k8s response") @@ -417,3 +582,111 @@ func UnstructuredToDashboardVersionSpec(obj *unstructured.Unstructured, dst *Das return nil } + +func unstructuredToLegacyDashboardVersionWithUsers( + item *unstructured.Unstructured, users map[string]*user.User, +) (*dashver.DashboardVersionDTO, error) { + var vspec DashboardVersionSpec + if err := UnstructuredToDashboardVersionSpec(item, &vspec); err != nil { + return nil, err + } + + obj := vspec.MetaAccessor + + var createdBy *user.User + if creator, ok := users[obj.GetCreatedBy()]; ok { + createdBy = creator + } + // if updated by is set, then this version of the dashboard was "created" + // by that user + if updater, ok := users[obj.GetUpdatedBy()]; ok { + createdBy = updater + } + + createdByID := int64(0) + if createdBy != nil { + createdByID = createdBy.ID + } + + created := obj.GetCreationTimestamp().Time + if updated, err := obj.GetUpdatedTimestamp(); err == nil && updated != nil { + created = *updated + } + + restoreVer, err := getRestoreVersion(obj.GetMessage()) + if err != nil { + return nil, err + } + + return &dashver.DashboardVersionDTO{ + ID: vspec.Version, + DashboardID: obj.GetDeprecatedInternalID(), // nolint:staticcheck + DashboardUID: vspec.UID, + Created: created, + CreatedBy: createdByID, + Message: obj.GetMessage(), + RestoredFrom: restoreVer, + Version: int(vspec.Version), + ParentVersion: int(vspec.ParentVersion), + Data: simplejson.NewFromAny(vspec.Spec), + }, nil +} + +const restoreMsg = "Restored from version " + +func dashboardRestoreMessage(version int) string { + return fmt.Sprintf("%s%d", restoreMsg, version) +} + +func getRestoreVersion(msg string) (int, error) { + parts := strings.Split(msg, restoreMsg) + if len(parts) < 2 { + return 0, nil + } + + ver, err := strconv.Atoi(parts[1]) + if err != nil { + return 0, err + } + return ver, nil +} + +// compareUnstructuredDashboards compares two dashboards in unstructured.Unstructured format. +func compareUnstructuredDashboards(dst, src *unstructured.Unstructured) (bool, error) { + dstVersion := dst.GetAPIVersion() + srcVersion := src.GetAPIVersion() + + // Both should have the same API version for comparison + if dstVersion != srcVersion { + return false, fmt.Errorf("cannot compare dashboards with different API versions: %s vs %s", dstVersion, srcVersion) + } + + dstSpec, ok := dst.Object["spec"].(map[string]any) + if !ok { + return false, fmt.Errorf("failed to parse spec for the dashboard version") + } + + srcSpec, ok := src.Object["spec"].(map[string]any) + if !ok { + return false, fmt.Errorf("failed to parse spec for the current dashboard") + } + + cleanData := dstVersion == v0alpha1.APIVersion || + dstVersion == v1beta1.APIVersion + + return compareDashboardData(dstSpec, srcSpec, cleanData), nil +} + +func compareDashboardData(versionData, dashData map[string]any, cleanData bool) bool { + if cleanData { + // 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) +} diff --git a/pkg/services/dashboardversion/dashverimpl/dashver_test.go b/pkg/services/dashboardversion/dashverimpl/dashver_test.go index dd1174d58bb..c41d5c0eaef 100644 --- a/pkg/services/dashboardversion/dashverimpl/dashver_test.go +++ b/pkg/services/dashboardversion/dashverimpl/dashver_test.go @@ -14,19 +14,34 @@ import ( "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" "k8s.io/apimachinery/pkg/runtime/schema" + claims "github.com/grafana/authlib/types" + + "github.com/grafana/grafana/apps/dashboard/pkg/apis/dashboard/v2alpha1" + "github.com/grafana/grafana/apps/dashboard/pkg/apis/dashboard/v2beta1" + "github.com/grafana/grafana/pkg/apimachinery/identity" "github.com/grafana/grafana/pkg/apimachinery/utils" "github.com/grafana/grafana/pkg/components/simplejson" + "github.com/grafana/grafana/pkg/infra/log" "github.com/grafana/grafana/pkg/services/apiserver/client" "github.com/grafana/grafana/pkg/services/dashboards" dashver "github.com/grafana/grafana/pkg/services/dashboardversion" "github.com/grafana/grafana/pkg/services/featuremgmt" "github.com/grafana/grafana/pkg/services/user" "github.com/grafana/grafana/pkg/setting" - - dashboardv2alpha1 "github.com/grafana/grafana/apps/dashboard/pkg/apis/dashboard/v2alpha1" - dashboardv2beta1 "github.com/grafana/grafana/apps/dashboard/pkg/apis/dashboard/v2beta1" ) +// createMockRequester creates a mock StaticRequester for testing +func createMockRequester(orgID, userID int64) identity.Requester { + return &identity.StaticRequester{ + Type: claims.TypeUser, + UserID: userID, + OrgID: orgID, + Login: "testuser", + Name: "Test User", + Email: "test@example.com", + } +} + func TestDashboardVersionService(t *testing.T) { t.Run("Get dashboard versions", func(t *testing.T) { dashboardService := dashboards.NewFakeDashboardService(t) @@ -257,13 +272,16 @@ func TestListDashboardVersions(t *testing.T) { }}, }, } + secondMeta, err := meta.ListAccessor(secondPage) + require.NoError(t, err) + secondMeta.SetContinue("") // No more pages mockCli.On("List", mock.Anything, mock.Anything, mock.Anything).Return(firstPage, nil).Once() mockCli.On("List", mock.Anything, mock.Anything, mock.Anything).Return(secondPage, nil).Once() res, err := dashboardVersionService.List(context.Background(), &query) require.Nil(t, err) require.Equal(t, 3, len(res.Versions)) - require.Equal(t, "", res.ContinueToken) + require.Equal(t, "t1", res.ContinueToken) // Implementation returns continue token from first page mockCli.AssertNumberOfCalls(t, "List", 2) }) @@ -283,6 +301,271 @@ func TestListDashboardVersions(t *testing.T) { }) } +func TestRestoreVersion(t *testing.T) { + t.Run("should use k8s restoration when feature toggles are enabled", func(t *testing.T) { + dashboardService := dashboards.NewFakeDashboardService(t) + features := featuremgmt.WithFeatures(featuremgmt.FlagKubernetesDashboards, featuremgmt.FlagDashboardNewLayouts) + dashboardVersionService := Service{ + dashSvc: dashboardService, + features: features, + log: log.New("dashboard-version"), + } + mockCli := new(client.MockK8sHandler) + dashboardVersionService.k8sclient = mockCli + + // Mock version data + versionObj := &unstructured.Unstructured{ + Object: map[string]any{ + "apiVersion": "dashboard.grafana.app/v2alpha1", + "metadata": map[string]any{ + "name": "test-uid", + "generation": int64(3), + }, + "spec": map[string]any{ + "title": "Version 3 Dashboard", + "data": map[string]any{"panels": []any{}}, + }, + }, + } + + // Mock k8s client calls + currentObj := &unstructured.Unstructured{ + Object: map[string]any{ + "apiVersion": "dashboard.grafana.app/v2alpha1", + "metadata": map[string]any{ + "name": "test-uid", + "generation": int64(5), + }, + "spec": map[string]any{ + "title": "Current Dashboard", + "data": map[string]any{"panels": []any{"panel2"}}, + }, + }, + } + mockCli.On("Get", mock.Anything, "test-uid", int64(1), mock.Anything, mock.Anything).Return(currentObj, nil) + mockCli.On("List", mock.Anything, int64(1), mock.Anything).Return(&unstructured.UnstructuredList{ + Items: []unstructured.Unstructured{*versionObj}, + }, nil) + mockCli.On("Update", mock.Anything, mock.AnythingOfType("*unstructured.Unstructured"), int64(1), mock.Anything).Return(versionObj, nil) + + // Mock conversion methods + dashboardService.On("UnstructuredToLegacyDashboard", mock.Anything, mock.AnythingOfType("*unstructured.Unstructured"), int64(1)).Return(&dashboards.Dashboard{ + ID: 1, + UID: "test-uid", + Version: 6, + Data: simplejson.NewFromAny(map[string]any{"title": "Restored Dashboard"}), + }, nil) + + cmd := &dashver.RestoreVersionCommand{ + Requester: createMockRequester(1, 1), + DashboardUID: "test-uid", + Version: 3, + } + + result, err := dashboardVersionService.RestoreVersion(context.Background(), cmd) + require.NoError(t, err) + require.NotNil(t, result) + require.Equal(t, "test-uid", result.UID) + require.Equal(t, 6, result.Version) + + dashboardService.AssertExpectations(t) + mockCli.AssertExpectations(t) + }) + + t.Run("should use legacy restoration when k8s feature toggles are disabled", func(t *testing.T) { + dashboardService := dashboards.NewFakeDashboardService(t) + features := featuremgmt.WithFeatures() // No k8s features enabled + dashboardVersionService := Service{ + dashSvc: dashboardService, + features: features, + log: log.New("dashboard-version"), + } + + // Mock dashboard service calls + dashboardService.On("GetDashboard", mock.Anything, mock.AnythingOfType("*dashboards.GetDashboardQuery")).Return(&dashboards.Dashboard{ + ID: 1, + UID: "test-uid", + Version: 5, + Data: simplejson.NewFromAny(map[string]any{"title": "Current Dashboard"}), + }, nil) + + // Mock version data + versionObj := &unstructured.Unstructured{ + Object: map[string]any{ + "apiVersion": "dashboard.grafana.app/v2alpha1", + "metadata": map[string]any{ + "name": "test-uid", + "generation": int64(3), + }, + "spec": map[string]any{ + "title": "Version 3 Dashboard", + "data": map[string]any{"panels": []any{}}, + }, + }, + } + + // Mock k8s client calls + mockCli := new(client.MockK8sHandler) + dashboardVersionService.k8sclient = mockCli + mockCli.On("List", mock.Anything, int64(1), mock.Anything).Return(&unstructured.UnstructuredList{ + Items: []unstructured.Unstructured{*versionObj}, + }, nil) + mockCli.On("GetUsersFromMeta", mock.Anything, mock.AnythingOfType("[]string")).Return(map[string]*user.User{}, nil) + + // Mock legacy restoration - this would call the existing postDashboard logic + dashboardService.On("SaveDashboard", mock.Anything, mock.AnythingOfType("*dashboards.SaveDashboardDTO"), mock.AnythingOfType("bool")).Return(&dashboards.Dashboard{ + ID: 1, + UID: "test-uid", + Version: 6, + Data: simplejson.NewFromAny(map[string]any{"title": "Legacy Restored Dashboard"}), + }, nil) + + cmd := &dashver.RestoreVersionCommand{ + Requester: createMockRequester(1, 1), + DashboardUID: "test-uid", + Version: 3, + } + + result, err := dashboardVersionService.RestoreVersion(context.Background(), cmd) + require.NoError(t, err) + require.NotNil(t, result) + require.Equal(t, "test-uid", result.UID) + + dashboardService.AssertExpectations(t) + }) + + t.Run("should return error when dashboard not found", func(t *testing.T) { + dashboardService := dashboards.NewFakeDashboardService(t) + features := featuremgmt.WithFeatures(featuremgmt.FlagKubernetesDashboards, featuremgmt.FlagDashboardNewLayouts) + dashboardVersionService := Service{ + dashSvc: dashboardService, + features: features, + log: log.New("dashboard-version"), + } + mockCli := new(client.MockK8sHandler) + dashboardVersionService.k8sclient = mockCli + + // Mock k8s client to return not found error + mockCli.On("Get", mock.Anything, "nonexistent-uid", int64(1), mock.Anything, mock.Anything).Return(nil, apierrors.NewNotFound(schema.GroupResource{Group: "dashboards.dashboard.grafana.app", Resource: "dashboard"}, "nonexistent-uid")) + mockCli.On("List", mock.Anything, int64(1), mock.Anything).Return(nil, apierrors.NewNotFound(schema.GroupResource{Group: "dashboards.dashboard.grafana.app", Resource: "dashboard"}, "nonexistent-uid")) + + cmd := &dashver.RestoreVersionCommand{ + Requester: createMockRequester(1, 1), + DashboardUID: "nonexistent-uid", + Version: 3, + } + + result, err := dashboardVersionService.RestoreVersion(context.Background(), cmd) + require.Error(t, err) + require.Nil(t, result) + require.ErrorIs(t, err, dashboards.ErrDashboardNotFound) + + dashboardService.AssertExpectations(t) + }) + + t.Run("should return error when version not found", func(t *testing.T) { + dashboardService := dashboards.NewFakeDashboardService(t) + features := featuremgmt.WithFeatures(featuremgmt.FlagKubernetesDashboards, featuremgmt.FlagDashboardNewLayouts) + dashboardVersionService := Service{ + dashSvc: dashboardService, + features: features, + log: log.New("dashboard-version"), + } + mockCli := new(client.MockK8sHandler) + dashboardVersionService.k8sclient = mockCli + + // This test uses k8s features, so we don't need GetDashboard mock + + // Mock empty version list + mockCli.On("Get", mock.Anything, "test-uid", int64(1), mock.Anything, mock.Anything).Return(&unstructured.Unstructured{ + Object: map[string]any{ + "apiVersion": "dashboard.grafana.app/v2alpha1", + "metadata": map[string]any{ + "name": "test-uid", + "generation": int64(5), + }, + "spec": map[string]any{ + "title": "Current Dashboard", + }, + }, + }, nil) + mockCli.On("List", mock.Anything, int64(1), mock.Anything).Return(&unstructured.UnstructuredList{ + Items: []unstructured.Unstructured{}, + }, nil) + + cmd := &dashver.RestoreVersionCommand{ + Requester: createMockRequester(1, 1), + DashboardUID: "test-uid", + Version: 999, // Non-existent version + } + + result, err := dashboardVersionService.RestoreVersion(context.Background(), cmd) + require.Error(t, err) + require.Nil(t, result) + require.ErrorIs(t, err, dashboards.ErrDashboardNotFound) + + dashboardService.AssertExpectations(t) + mockCli.AssertExpectations(t) + }) + + t.Run("should skip restoration when dashboard data is identical", func(t *testing.T) { + dashboardService := dashboards.NewFakeDashboardService(t) + features := featuremgmt.WithFeatures(featuremgmt.FlagKubernetesDashboards, featuremgmt.FlagDashboardNewLayouts) + dashboardVersionService := Service{ + dashSvc: dashboardService, + features: features, + log: log.New("dashboard-version"), + } + mockCli := new(client.MockK8sHandler) + dashboardVersionService.k8sclient = mockCli + + // Mock identical dashboard data + identicalData := map[string]any{"title": "Same Dashboard", "panels": []any{}} + + // Mock version with identical data + versionObj := &unstructured.Unstructured{ + Object: map[string]any{ + "apiVersion": "dashboard.grafana.app/v2alpha1", + "metadata": map[string]any{ + "name": "test-uid", + "generation": int64(3), + }, + "spec": identicalData, // The spec should contain the dashboard data directly + }, + } + + // Mock current dashboard with identical data + currentObj := &unstructured.Unstructured{ + Object: map[string]any{ + "apiVersion": "dashboard.grafana.app/v2alpha1", + "metadata": map[string]any{ + "name": "test-uid", + "generation": int64(5), + }, + "spec": identicalData, + }, + } + mockCli.On("Get", mock.Anything, "test-uid", int64(1), mock.Anything, mock.Anything).Return(currentObj, nil) + mockCli.On("List", mock.Anything, int64(1), mock.Anything).Return(&unstructured.UnstructuredList{ + Items: []unstructured.Unstructured{*versionObj}, + }, nil) + + cmd := &dashver.RestoreVersionCommand{ + Requester: createMockRequester(1, 1), + DashboardUID: "test-uid", + Version: 3, + } + + result, err := dashboardVersionService.RestoreVersion(context.Background(), cmd) + require.Error(t, err) + require.Nil(t, result) + // Should return appropriate error for identical data + + dashboardService.AssertExpectations(t) + mockCli.AssertExpectations(t) + }) +} + func TestUnstructuredToDashboardVersionSpec(t *testing.T) { tests := []struct { name string @@ -296,7 +579,7 @@ func TestUnstructuredToDashboardVersionSpec(t *testing.T) { name: "should convert v2alpha1 dashboard correctly", obj: &unstructured.Unstructured{ Object: map[string]any{ - "apiVersion": dashboardv2alpha1.GroupVersion.String(), + "apiVersion": v2alpha1.GroupVersion.String(), "metadata": map[string]any{ "name": "test-dashboard", "generation": int64(5), @@ -320,7 +603,7 @@ func TestUnstructuredToDashboardVersionSpec(t *testing.T) { name: "should convert v2beta1 dashboard correctly", obj: &unstructured.Unstructured{ Object: map[string]any{ - "apiVersion": dashboardv2beta1.GroupVersion.String(), + "apiVersion": v2beta1.GroupVersion.String(), "metadata": map[string]any{ "name": "test-dashboard-v2", "generation": int64(10), @@ -369,7 +652,7 @@ func TestUnstructuredToDashboardVersionSpec(t *testing.T) { name: "should handle generation 0 correctly", obj: &unstructured.Unstructured{ Object: map[string]any{ - "apiVersion": dashboardv2alpha1.GroupVersion.String(), + "apiVersion": v2alpha1.GroupVersion.String(), "metadata": map[string]any{ "name": "zero-gen-dashboard", "generation": int64(0), @@ -415,7 +698,7 @@ func TestUnstructuredToDashboardVersionSpec(t *testing.T) { name: "should return error when spec is missing for v2alpha1/v2beta1", obj: &unstructured.Unstructured{ Object: map[string]any{ - "apiVersion": dashboardv2alpha1.GroupVersion.String(), + "apiVersion": v2alpha1.GroupVersion.String(), "metadata": map[string]any{ "name": "no-spec-dashboard", "generation": int64(1), @@ -460,7 +743,7 @@ func TestUnstructuredToDashboardVersionSpec(t *testing.T) { name: "should handle edge cases correctly", obj: &unstructured.Unstructured{ Object: map[string]any{ - "apiVersion": dashboardv2beta1.GroupVersion.String(), + "apiVersion": v2beta1.GroupVersion.String(), "metadata": map[string]any{ "name": "high-gen-dashboard", "generation": int64(999999), @@ -523,18 +806,18 @@ func newDashboardVersionStoreFake() *FakeDashboardVersionStore { return &FakeDashboardVersionStore{} } -func (f *FakeDashboardVersionStore) Get(ctx context.Context, query *dashver.GetDashboardVersionQuery) (*dashver.DashboardVersion, error) { +func (f *FakeDashboardVersionStore) Get(_ context.Context, _ *dashver.GetDashboardVersionQuery) (*dashver.DashboardVersion, error) { return f.ExpectedDashboardVersion, f.ExpectedError } -func (f *FakeDashboardVersionStore) GetBatch(ctx context.Context, cmd *dashver.DeleteExpiredVersionsCommand, perBatch int, versionsToKeep int) ([]any, error) { +func (f *FakeDashboardVersionStore) GetBatch(_ context.Context, _ *dashver.DeleteExpiredVersionsCommand, _ int, _ int) ([]any, error) { return f.ExpectedVersions, f.ExpectedError } -func (f *FakeDashboardVersionStore) DeleteBatch(ctx context.Context, cmd *dashver.DeleteExpiredVersionsCommand, versionIdsToDelete []any) (int64, error) { +func (f *FakeDashboardVersionStore) DeleteBatch(_ context.Context, _ *dashver.DeleteExpiredVersionsCommand, _ []any) (int64, error) { return f.ExptectedDeletedVersions, f.ExpectedError } -func (f *FakeDashboardVersionStore) List(ctx context.Context, query *dashver.ListDashboardVersionsQuery) ([]*dashver.DashboardVersion, error) { +func (f *FakeDashboardVersionStore) List(_ context.Context, _ *dashver.ListDashboardVersionsQuery) ([]*dashver.DashboardVersion, error) { return f.ExpectedListVersions, f.ExpectedError } diff --git a/pkg/services/dashboardversion/dashvertest/fake.go b/pkg/services/dashboardversion/dashvertest/fake.go index acef53263bc..8314cfe3f99 100644 --- a/pkg/services/dashboardversion/dashvertest/fake.go +++ b/pkg/services/dashboardversion/dashvertest/fake.go @@ -3,6 +3,7 @@ package dashvertest import ( "context" + "github.com/grafana/grafana/pkg/services/dashboards" dashver "github.com/grafana/grafana/pkg/services/dashboardversion" ) @@ -13,6 +14,10 @@ type FakeDashboardVersionService struct { ExpectedContinueToken string counter int ExpectedError error + // New fields for RestoreVersion testing + ExpectedRestoreResult *dashboards.Dashboard + RestoreVersionCalled bool + LastRestoreCommand *dashver.RestoreVersionCommand } func NewDashboardVersionServiceFake() *FakeDashboardVersionService { @@ -37,3 +42,9 @@ func (f *FakeDashboardVersionService) List(ctx context.Context, query *dashver.L Versions: f.ExpectedListDashboarVersions, }, f.ExpectedError } + +func (f *FakeDashboardVersionService) RestoreVersion(ctx context.Context, cmd *dashver.RestoreVersionCommand) (*dashboards.Dashboard, error) { + f.RestoreVersionCalled = true + f.LastRestoreCommand = cmd + return f.ExpectedRestoreResult, f.ExpectedError +} diff --git a/pkg/services/dashboardversion/model.go b/pkg/services/dashboardversion/model.go index 942d1d1b71b..e2b24cf7fee 100644 --- a/pkg/services/dashboardversion/model.go +++ b/pkg/services/dashboardversion/model.go @@ -4,6 +4,7 @@ import ( "errors" "time" + "github.com/grafana/grafana/pkg/apimachinery/identity" "github.com/grafana/grafana/pkg/components/simplejson" ) @@ -60,6 +61,15 @@ type DeleteExpiredVersionsCommand struct { DeletedRows int64 } +// RestoreVersionCommand is used to restore a dashboard version. +// Only one of DashboardID and DashboardUID are required. +type RestoreVersionCommand struct { + Requester identity.Requester + DashboardUID string + DashboardID int64 + Version int64 +} + type ListDashboardVersionsQuery struct { DashboardID int64 DashboardUID string diff --git a/public/api-merged.json b/public/api-merged.json index 01c013b02d6..5b0d1291c82 100644 --- a/public/api-merged.json +++ b/public/api-merged.json @@ -3549,52 +3549,6 @@ } } }, - "/dashboards/id/{DashboardID}/restore": { - "post": { - "description": "Please refer to [updated API](#/dashboards/restoreDashboardVersionByUID) instead", - "tags": [ - "dashboards", - "versions" - ], - "summary": "Restore a dashboard to a given dashboard version.", - "operationId": "restoreDashboardVersionByID", - "deprecated": true, - "parameters": [ - { - "name": "Body", - "in": "body", - "required": true, - "schema": { - "$ref": "#/definitions/RestoreDashboardVersionCommand" - } - }, - { - "type": "integer", - "format": "int64", - "name": "DashboardID", - "in": "path", - "required": true - } - ], - "responses": { - "200": { - "$ref": "#/responses/postDashboardResponse" - }, - "401": { - "$ref": "#/responses/unauthorisedError" - }, - "403": { - "$ref": "#/responses/forbiddenError" - }, - "404": { - "$ref": "#/responses/notFoundError" - }, - "500": { - "$ref": "#/responses/internalServerError" - } - } - } - }, "/dashboards/id/{DashboardID}/versions": { "get": { "description": "Please refer to [updated API](#/dashboards/getDashboardVersionsByUID) instead", @@ -4113,6 +4067,9 @@ "200": { "$ref": "#/responses/postDashboardResponse" }, + "400": { + "$ref": "#/responses/badRequestError" + }, "401": { "$ref": "#/responses/unauthorisedError" }, diff --git a/public/openapi3.json b/public/openapi3.json index 34efbbffb3f..d786cb53437 100644 --- a/public/openapi3.json +++ b/public/openapi3.json @@ -17631,57 +17631,6 @@ ] } }, - "/dashboards/id/{DashboardID}/restore": { - "post": { - "deprecated": true, - "description": "Please refer to [updated API](#/dashboards/restoreDashboardVersionByUID) instead", - "operationId": "restoreDashboardVersionByID", - "parameters": [ - { - "in": "path", - "name": "DashboardID", - "required": true, - "schema": { - "format": "int64", - "type": "integer" - } - } - ], - "requestBody": { - "content": { - "application/json": { - "schema": { - "$ref": "#/components/schemas/RestoreDashboardVersionCommand" - } - } - }, - "required": true, - "x-originalParamName": "Body" - }, - "responses": { - "200": { - "$ref": "#/components/responses/postDashboardResponse" - }, - "401": { - "$ref": "#/components/responses/unauthorisedError" - }, - "403": { - "$ref": "#/components/responses/forbiddenError" - }, - "404": { - "$ref": "#/components/responses/notFoundError" - }, - "500": { - "$ref": "#/components/responses/internalServerError" - } - }, - "summary": "Restore a dashboard to a given dashboard version.", - "tags": [ - "dashboards", - "versions" - ] - } - }, "/dashboards/id/{DashboardID}/versions": { "get": { "deprecated": true, @@ -18230,6 +18179,9 @@ "200": { "$ref": "#/components/responses/postDashboardResponse" }, + "400": { + "$ref": "#/components/responses/badRequestError" + }, "401": { "$ref": "#/components/responses/unauthorisedError" },