From 0cef2b9ae7cc2f2ac82cdd4dd4768c18f95160f6 Mon Sep 17 00:00:00 2001 From: Stephanie Hingtgen Date: Tue, 28 Jan 2025 07:17:52 -0700 Subject: [PATCH] Dashboard Versions: Make compatible with app platform (#99327) --- pkg/api/dashboard.go | 34 ++- pkg/api/dashboard_test.go | 12 +- pkg/api/dtos/dashboard.go | 4 +- pkg/components/dashdiffs/compare.go | 2 +- .../dashboard/legacy/query_dashboards.sql | 6 +- .../apis/dashboard/legacy/sql_dashboards.go | 26 +- pkg/registry/apis/dashboard/legacy/storage.go | 2 +- ...uery_dashboards-history_uid_at_version.sql | 4 +- ...uery_dashboards-history_uid_at_version.sql | 4 +- ...uery_dashboards-history_uid_at_version.sql | 4 +- pkg/registry/apis/dashboard/legacy/utils.go | 9 - .../apis/dashboard/legacy/utils_test.go | 13 - pkg/services/apiserver/client/client.go | 37 ++- pkg/services/apiserver/client/client_mock.go | 13 +- pkg/services/apiserver/client/client_test.go | 34 +++ .../dashboards/service/dashboard_service.go | 41 +-- .../service/dashboard_service_test.go | 67 +++-- pkg/services/dashboardversion/dashver.go | 2 +- .../dashboardversion/dashverimpl/dashver.go | 187 ++++++++++++- .../dashverimpl/dashver_test.go | 114 ++++++-- .../dashverimpl/store_test.go | 4 +- .../dashboardversion/dashvertest/fake.go | 8 +- pkg/services/dashboardversion/model.go | 24 +- pkg/storage/unified/resource/cdk_backend.go | 5 + pkg/storage/unified/resource/server.go | 10 + pkg/storage/unified/sql/backend.go | 4 + .../unified/sql/data/resource_history_get.sql | 2 +- .../unified/sql/test/integration_test.go | 77 ++++++ ...rce_history_get-read trash second page.sql | 2 +- ...rce_history_get-read trash second page.sql | 2 +- ...rce_history_get-read trash second page.sql | 2 +- .../dashboard-scene/scene/DashboardScene.tsx | 8 +- .../settings/VersionsEditView.test.tsx | 79 +++--- .../settings/VersionsEditView.tsx | 25 +- .../version-history/HistorySrv.test.ts | 4 +- .../settings/version-history/HistorySrv.ts | 1 + .../__mocks__/dashboardHistoryMocks.ts | 97 +++---- .../VersionsSettings.test.tsx | 41 ++- .../DashboardSettings/VersionsSettings.tsx | 25 +- .../DashboardSettings/__mocks__/versions.ts | 249 +++++++++--------- .../VersionHistory/RevertDashboardModal.tsx | 5 +- .../VersionHistoryComparison.tsx | 1 + .../VersionHistory/VersionHistoryTable.tsx | 1 + .../VersionHistory/useDashboardRestore.tsx | 9 +- 44 files changed, 887 insertions(+), 413 deletions(-) delete mode 100644 pkg/registry/apis/dashboard/legacy/utils.go delete mode 100644 pkg/registry/apis/dashboard/legacy/utils_test.go create mode 100644 pkg/services/apiserver/client/client_test.go diff --git a/pkg/api/dashboard.go b/pkg/api/dashboard.go index 486911d626c..5079d7d9877 100644 --- a/pkg/api/dashboard.go +++ b/pkg/api/dashboard.go @@ -24,6 +24,7 @@ import ( contextmodel "github.com/grafana/grafana/pkg/services/contexthandler/model" "github.com/grafana/grafana/pkg/services/dashboards" 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/guardian" @@ -828,21 +829,22 @@ func (hs *HTTPServer) GetDashboardVersions(c *contextmodel.ReqContext) response. } query := dashver.ListDashboardVersionsQuery{ - OrgID: c.SignedInUser.GetOrgID(), - DashboardID: dash.ID, - DashboardUID: dash.UID, - Limit: c.QueryInt("limit"), - Start: c.QueryInt("start"), + OrgID: c.SignedInUser.GetOrgID(), + DashboardID: dash.ID, + DashboardUID: dash.UID, + Limit: c.QueryInt("limit"), + Start: c.QueryInt("start"), + ContinueToken: c.Query("continueToken"), } - versions, err := hs.dashboardVersionService.List(c.Req.Context(), &query) + resp, err := hs.dashboardVersionService.List(c.Req.Context(), &query) if err != nil { return response.Error(http.StatusNotFound, fmt.Sprintf("No versions found for dashboardId %d", dash.ID), err) } - loginMem := make(map[int64]string, len(versions)) - res := make([]dashver.DashboardVersionMeta, 0, len(versions)) - for _, version := range versions { + loginMem := make(map[int64]string, len(resp.Versions)) + res := make([]dashver.DashboardVersionMeta, 0, len(resp.Versions)) + for _, version := range resp.Versions { msg := version.Message if version.RestoredFrom == version.Version { msg = "Initial save (created by migration)" @@ -883,7 +885,10 @@ func (hs *HTTPServer) GetDashboardVersions(c *contextmodel.ReqContext) response. }) } - return response.JSON(http.StatusOK, res) + return response.JSON(http.StatusOK, dashver.DashboardVersionResponseMeta{ + Versions: res, + ContinueToken: resp.ContinueToken, + }) } // swagger:route GET /dashboards/id/{DashboardID}/versions/{DashboardVersionID} dashboard_versions getDashboardVersionByID @@ -943,12 +948,15 @@ func (hs *HTTPServer) GetDashboardVersion(c *contextmodel.ReqContext) response.R return dashboardGuardianResponse(err) } - version, _ := strconv.ParseInt(web.Params(c.Req)[":id"], 10, 32) + version, err := strconv.ParseInt(web.Params(c.Req)[":id"], 10, 64) + if err != nil { + return response.Err(err) + } query := dashver.GetDashboardVersionQuery{ OrgID: c.SignedInUser.GetOrgID(), DashboardID: dash.ID, DashboardUID: dash.UID, - Version: int(version), + Version: version, } res, err := hs.dashboardVersionService.Get(c.Req.Context(), &query) @@ -1158,7 +1166,7 @@ func (hs *HTTPServer) RestoreDashboardVersion(c *contextmodel.ReqContext) respon saveCmd.Dashboard = version.Data saveCmd.Dashboard.Set("version", dash.Version) saveCmd.Dashboard.Set("uid", dash.UID) - saveCmd.Message = fmt.Sprintf("Restored from version %d", version.Version) + saveCmd.Message = dashverimpl.DashboardRestoreMessage(version.Version) // nolint:staticcheck saveCmd.FolderID = dash.FolderID metrics.MFolderIDsAPICount.WithLabelValues(metrics.RestoreDashboardVersion).Inc() diff --git a/pkg/api/dashboard_test.go b/pkg/api/dashboard_test.go index 34c6c2c4dbe..a08deefb4bb 100644 --- a/pkg/api/dashboard_test.go +++ b/pkg/api/dashboard_test.go @@ -743,10 +743,10 @@ func TestDashboardVersionsAPIEndpoint(t *testing.T) { }).callGetDashboardVersions(sc) assert.Equal(t, http.StatusOK, sc.resp.Code) - var versions []dashver.DashboardVersionMeta + var versions dashver.DashboardVersionResponseMeta err := json.NewDecoder(sc.resp.Body).Decode(&versions) require.NoError(t, err) - for _, v := range versions { + for _, v := range versions.Versions { assert.Equal(t, "test-user", v.CreatedBy) } }, mockSQLStore) @@ -769,10 +769,10 @@ func TestDashboardVersionsAPIEndpoint(t *testing.T) { }).callGetDashboardVersions(sc) assert.Equal(t, http.StatusOK, sc.resp.Code) - var versions []dashver.DashboardVersionMeta + var versions dashver.DashboardVersionResponseMeta err := json.NewDecoder(sc.resp.Body).Decode(&versions) require.NoError(t, err) - for _, v := range versions { + for _, v := range versions.Versions { assert.Equal(t, anonString, v.CreatedBy) } }, mockSQLStore) @@ -795,10 +795,10 @@ func TestDashboardVersionsAPIEndpoint(t *testing.T) { }).callGetDashboardVersions(sc) assert.Equal(t, http.StatusOK, sc.resp.Code) - var versions []dashver.DashboardVersionMeta + var versions dashver.DashboardVersionResponseMeta err := json.NewDecoder(sc.resp.Body).Decode(&versions) require.NoError(t, err) - for _, v := range versions { + for _, v := range versions.Versions { assert.Equal(t, anonString, v.CreatedBy) } }, mockSQLStore) diff --git a/pkg/api/dtos/dashboard.go b/pkg/api/dtos/dashboard.go index 9d8f96f8c7a..73c3d7cae6f 100644 --- a/pkg/api/dtos/dashboard.go +++ b/pkg/api/dtos/dashboard.go @@ -54,10 +54,10 @@ type CalculateDiffOptions struct { type CalculateDiffTarget struct { DashboardId int64 `json:"dashboardId"` - Version int `json:"version"` + Version int64 `json:"version"` UnsavedDashboard *simplejson.Json `json:"unsavedDashboard"` } type RestoreDashboardVersionCommand struct { - Version int `json:"version" binding:"Required"` + Version int64 `json:"version" binding:"Required"` } diff --git a/pkg/components/dashdiffs/compare.go b/pkg/components/dashdiffs/compare.go index e5c6e841901..7bb06a92ff2 100644 --- a/pkg/components/dashdiffs/compare.go +++ b/pkg/components/dashdiffs/compare.go @@ -34,7 +34,7 @@ type Options struct { type DiffTarget struct { DashboardId int64 - Version int + Version int64 UnsavedDashboard *simplejson.Json } diff --git a/pkg/registry/apis/dashboard/legacy/query_dashboards.sql b/pkg/registry/apis/dashboard/legacy/query_dashboards.sql index e5d532ffc87..c4901404f25 100644 --- a/pkg/registry/apis/dashboard/legacy/query_dashboards.sql +++ b/pkg/registry/apis/dashboard/legacy/query_dashboards.sql @@ -30,11 +30,11 @@ WHERE dashboard.is_folder = {{ .Arg .Query.GetFolders }} {{ if .Query.Version }} AND dashboard_version.version = {{ .Arg .Query.Version }} {{ else if .Query.LastID }} - AND dashboard_version.version < {{ .Arg .Query.LastID }} + AND dashboard_version.version <= {{ .Arg .Query.LastID }} {{ end }} ORDER BY - dashboard_version.created ASC, - dashboard_version.version ASC, + dashboard_version.created DESC, + dashboard_version.version DESC, dashboard.uid ASC {{ else }} {{ if .Query.UID }} diff --git a/pkg/registry/apis/dashboard/legacy/sql_dashboards.go b/pkg/registry/apis/dashboard/legacy/sql_dashboards.go index 999586b13a1..f3b86f8b639 100644 --- a/pkg/registry/apis/dashboard/legacy/sql_dashboards.go +++ b/pkg/registry/apis/dashboard/legacy/sql_dashboards.go @@ -115,8 +115,9 @@ func (a *dashboardSqlAccess) getRows(ctx context.Context, sql *legacysql.LegacyD rows = nil } return &rowsWrapper{ - rows: rows, - a: a, + rows: rows, + a: a, + history: query.GetHistory, // This looks up rules from the permissions on a user canReadDashboard: func(scopes ...string) bool { return true // ??? @@ -128,8 +129,9 @@ func (a *dashboardSqlAccess) getRows(ctx context.Context, sql *legacysql.LegacyD var _ resource.ListIterator = (*rowsWrapper)(nil) type rowsWrapper struct { - a *dashboardSqlAccess - rows *sql.Rows + a *dashboardSqlAccess + rows *sql.Rows + history bool canReadDashboard func(scopes ...string) bool @@ -157,7 +159,7 @@ func (r *rowsWrapper) Next() bool { // breaks after first readable value for r.rows.Next() { - r.row, err = r.a.scanRow(r.rows) + r.row, err = r.a.scanRow(r.rows, r.history) if err != nil { r.err = err return false @@ -187,6 +189,11 @@ func (r *rowsWrapper) ContinueToken() string { return r.row.token.String() } +// ContinueTokenWithCurrentRV implements resource.ListIterator. +func (r *rowsWrapper) ContinueTokenWithCurrentRV() string { + return r.row.token.String() +} + // Error implements resource.ListIterator. func (r *rowsWrapper) Error() error { return r.err @@ -218,7 +225,7 @@ func (r *rowsWrapper) Value() []byte { return b } -func (a *dashboardSqlAccess) scanRow(rows *sql.Rows) (*dashboardRow, error) { +func (a *dashboardSqlAccess) scanRow(rows *sql.Rows, history bool) (*dashboardRow, error) { dash := &dashboard.Dashboard{ TypeMeta: dashboard.DashboardResourceInfo.TypeMeta(), ObjectMeta: metav1.ObjectMeta{Annotations: make(map[string]string)}, @@ -255,8 +262,12 @@ func (a *dashboardSqlAccess) scanRow(rows *sql.Rows) (*dashboardRow, error) { ) row.token = &continueToken{orgId: orgId, id: dashboard_id} + // when listing from the history table, we want to use the version as the ID to continue from + if history { + row.token.id = version + } if err == nil { - row.RV = getResourceVersion(dashboard_id, version) + row.RV = version dash.ResourceVersion = fmt.Sprintf("%d", row.RV) dash.Namespace = a.namespacer(orgId) dash.UID = gapiutil.CalculateClusterWideUID(dash) @@ -405,6 +416,7 @@ func (a *dashboardSqlAccess) SaveDashboard(ctx context.Context, orgId int64, das } out, err := a.dashStore.SaveDashboard(ctx, dashboards.SaveDashboardCommand{ OrgID: orgId, + Message: meta.GetMessage(), PluginID: service.GetPluginIDFromMeta(meta), Dashboard: simplejson.NewFromAny(dash.Spec.UnstructuredContent()), FolderUID: meta.GetFolder(), diff --git a/pkg/registry/apis/dashboard/legacy/storage.go b/pkg/registry/apis/dashboard/legacy/storage.go index e5f51bf78f9..7064d1d9994 100644 --- a/pkg/registry/apis/dashboard/legacy/storage.go +++ b/pkg/registry/apis/dashboard/legacy/storage.go @@ -138,7 +138,7 @@ func (a *dashboardSqlAccess) ReadResource(ctx context.Context, req *resource.Rea } version := int64(0) if req.ResourceVersion > 0 { - version = getVersionFromRV(req.ResourceVersion) + version = req.ResourceVersion } dash, rv, err := a.GetDashboard(ctx, info.OrgID, req.Key.Name, version) diff --git a/pkg/registry/apis/dashboard/legacy/testdata/mysql--query_dashboards-history_uid_at_version.sql b/pkg/registry/apis/dashboard/legacy/testdata/mysql--query_dashboards-history_uid_at_version.sql index d85d3bbd8c8..728e9a8e72b 100755 --- a/pkg/registry/apis/dashboard/legacy/testdata/mysql--query_dashboards-history_uid_at_version.sql +++ b/pkg/registry/apis/dashboard/legacy/testdata/mysql--query_dashboards-history_uid_at_version.sql @@ -19,6 +19,6 @@ WHERE dashboard.is_folder = FALSE AND dashboard.uid = 'UUU' AND dashboard_version.version = 3 ORDER BY - dashboard_version.created ASC, - dashboard_version.version ASC, + dashboard_version.created DESC, + dashboard_version.version DESC, dashboard.uid ASC diff --git a/pkg/registry/apis/dashboard/legacy/testdata/postgres--query_dashboards-history_uid_at_version.sql b/pkg/registry/apis/dashboard/legacy/testdata/postgres--query_dashboards-history_uid_at_version.sql index 5fe3708643a..b149e393780 100755 --- a/pkg/registry/apis/dashboard/legacy/testdata/postgres--query_dashboards-history_uid_at_version.sql +++ b/pkg/registry/apis/dashboard/legacy/testdata/postgres--query_dashboards-history_uid_at_version.sql @@ -19,6 +19,6 @@ WHERE dashboard.is_folder = FALSE AND dashboard.uid = 'UUU' AND dashboard_version.version = 3 ORDER BY - dashboard_version.created ASC, - dashboard_version.version ASC, + dashboard_version.created DESC, + dashboard_version.version DESC, dashboard.uid ASC diff --git a/pkg/registry/apis/dashboard/legacy/testdata/sqlite--query_dashboards-history_uid_at_version.sql b/pkg/registry/apis/dashboard/legacy/testdata/sqlite--query_dashboards-history_uid_at_version.sql index 5fe3708643a..b149e393780 100755 --- a/pkg/registry/apis/dashboard/legacy/testdata/sqlite--query_dashboards-history_uid_at_version.sql +++ b/pkg/registry/apis/dashboard/legacy/testdata/sqlite--query_dashboards-history_uid_at_version.sql @@ -19,6 +19,6 @@ WHERE dashboard.is_folder = FALSE AND dashboard.uid = 'UUU' AND dashboard_version.version = 3 ORDER BY - dashboard_version.created ASC, - dashboard_version.version ASC, + dashboard_version.created DESC, + dashboard_version.version DESC, dashboard.uid ASC diff --git a/pkg/registry/apis/dashboard/legacy/utils.go b/pkg/registry/apis/dashboard/legacy/utils.go deleted file mode 100644 index c61db27788a..00000000000 --- a/pkg/registry/apis/dashboard/legacy/utils.go +++ /dev/null @@ -1,9 +0,0 @@ -package legacy - -func getResourceVersion(id int64, version int64) int64 { - return version + (id * 10000000) -} - -func getVersionFromRV(rv int64) int64 { - return rv % 10000000 -} diff --git a/pkg/registry/apis/dashboard/legacy/utils_test.go b/pkg/registry/apis/dashboard/legacy/utils_test.go deleted file mode 100644 index a058dae612a..00000000000 --- a/pkg/registry/apis/dashboard/legacy/utils_test.go +++ /dev/null @@ -1,13 +0,0 @@ -package legacy - -import ( - "testing" - - "github.com/stretchr/testify/require" -) - -func TestVersionHacks(t *testing.T) { - rv := getResourceVersion(123, 456) - require.Equal(t, int64(1230000456), rv) - require.Equal(t, int64(456), getVersionFromRV(rv)) -} diff --git a/pkg/services/apiserver/client/client.go b/pkg/services/apiserver/client/client.go index 2e25ca07ee2..2f45e426bc3 100644 --- a/pkg/services/apiserver/client/client.go +++ b/pkg/services/apiserver/client/client.go @@ -2,7 +2,10 @@ package client import ( "context" + "errors" "fmt" + "strconv" + "strings" "time" v1 "k8s.io/apimachinery/pkg/apis/meta/v1" @@ -16,6 +19,7 @@ import ( "github.com/grafana/grafana/pkg/services/apiserver" "github.com/grafana/grafana/pkg/services/apiserver/endpoints/request" "github.com/grafana/grafana/pkg/services/dashboards" + "github.com/grafana/grafana/pkg/services/user" "github.com/grafana/grafana/pkg/setting" "github.com/grafana/grafana/pkg/storage/unified/resource" k8sUser "k8s.io/apiserver/pkg/authentication/user" @@ -24,7 +28,7 @@ import ( type K8sHandler interface { GetNamespace(orgID int64) string - Get(ctx context.Context, name string, orgID int64, subresource ...string) (*unstructured.Unstructured, error) + Get(ctx context.Context, name string, orgID int64, options v1.GetOptions, subresource ...string) (*unstructured.Unstructured, error) Create(ctx context.Context, obj *unstructured.Unstructured, orgID int64) (*unstructured.Unstructured, error) Update(ctx context.Context, obj *unstructured.Unstructured, orgID int64) (*unstructured.Unstructured, error) Delete(ctx context.Context, name string, orgID int64, options v1.DeleteOptions) error @@ -32,6 +36,7 @@ type K8sHandler interface { List(ctx context.Context, orgID int64, options v1.ListOptions) (*unstructured.UnstructuredList, error) Search(ctx context.Context, orgID int64, in *resource.ResourceSearchRequest) (*resource.ResourceSearchResponse, error) GetStats(ctx context.Context, orgID int64) (*resource.ResourceStatsResponse, error) + GetUserFromMeta(ctx context.Context, userMeta string) (*user.User, error) } var _ K8sHandler = (*k8sHandler)(nil) @@ -41,9 +46,10 @@ type k8sHandler struct { gvr schema.GroupVersionResource restConfigProvider apiserver.RestConfigProvider searcher resource.ResourceIndexClient + userService user.Service } -func NewK8sHandler(cfg *setting.Cfg, namespacer request.NamespaceMapper, gvr schema.GroupVersionResource, restConfigProvider apiserver.RestConfigProvider, searcher resource.ResourceIndexClient, dashStore dashboards.Store) K8sHandler { +func NewK8sHandler(cfg *setting.Cfg, namespacer request.NamespaceMapper, gvr schema.GroupVersionResource, restConfigProvider apiserver.RestConfigProvider, searcher resource.ResourceIndexClient, dashStore dashboards.Store, userSvc user.Service) K8sHandler { legacySearcher := legacysearcher.NewDashboardSearchClient(dashStore) searchClient := resource.NewSearchClient(cfg, setting.UnifiedStorageConfigKeyDashboard, searcher, legacySearcher) return &k8sHandler{ @@ -51,6 +57,7 @@ func NewK8sHandler(cfg *setting.Cfg, namespacer request.NamespaceMapper, gvr sch gvr: gvr, restConfigProvider: restConfigProvider, searcher: searchClient, + userService: userSvc, } } @@ -58,7 +65,7 @@ func (h *k8sHandler) GetNamespace(orgID int64) string { return h.namespacer(orgID) } -func (h *k8sHandler) Get(ctx context.Context, name string, orgID int64, subresource ...string) (*unstructured.Unstructured, error) { +func (h *k8sHandler) Get(ctx context.Context, name string, orgID int64, options v1.GetOptions, subresource ...string) (*unstructured.Unstructured, error) { // create a new context - prevents issues when the request stems from the k8s api itself // otherwise the context goes through the handlers twice and causes issues newCtx, cancel, err := h.getK8sContext(ctx) @@ -73,7 +80,7 @@ func (h *k8sHandler) Get(ctx context.Context, name string, orgID int64, subresou return nil, nil } - return client.Get(newCtx, name, v1.GetOptions{}, subresource...) + return client.Get(newCtx, name, options, subresource...) } func (h *k8sHandler) Create(ctx context.Context, obj *unstructured.Unstructured, orgID int64) (*unstructured.Unstructured, error) { @@ -191,6 +198,28 @@ func (h *k8sHandler) GetStats(ctx context.Context, orgID int64) (*resource.Resou }) } +// GetUserFromMeta takes what meta accessor gives you from `GetCreatedBy` or `GetUpdatedBy` and returns the user +func (h *k8sHandler) GetUserFromMeta(ctx context.Context, userMeta string) (*user.User, error) { + parts := strings.Split(userMeta, ":") + if len(parts) < 2 { + return &user.User{}, nil + } + meta := parts[1] + + userId, err := strconv.ParseInt(meta, 10, 64) + var u *user.User + if err == nil { + u, err = h.userService.GetByID(ctx, &user.GetUserByIDQuery{ID: userId}) + } else { + u, err = h.userService.GetByUID(ctx, &user.GetUserByUIDQuery{UID: meta}) + } + + if err != nil && errors.Is(err, user.ErrUserNotFound) { + return &user.User{}, nil + } + return u, err +} + func (h *k8sHandler) getClient(ctx context.Context, orgID int64) (dynamic.ResourceInterface, bool) { cfg := h.restConfigProvider.GetRestConfig(ctx) if cfg == nil { diff --git a/pkg/services/apiserver/client/client_mock.go b/pkg/services/apiserver/client/client_mock.go index f3a4e347e2f..b5e2fc2eecb 100644 --- a/pkg/services/apiserver/client/client_mock.go +++ b/pkg/services/apiserver/client/client_mock.go @@ -5,6 +5,7 @@ import ( "github.com/stretchr/testify/mock" + "github.com/grafana/grafana/pkg/services/user" "github.com/grafana/grafana/pkg/storage/unified/resource" v1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" @@ -21,8 +22,8 @@ func (m *MockK8sHandler) GetNamespace(orgID int64) string { return args.String(0) } -func (m *MockK8sHandler) Get(ctx context.Context, name string, orgID int64, subresource ...string) (*unstructured.Unstructured, error) { - args := m.Called(ctx, name, orgID, subresource) +func (m *MockK8sHandler) Get(ctx context.Context, name string, orgID int64, options v1.GetOptions, subresource ...string) (*unstructured.Unstructured, error) { + args := m.Called(ctx, name, orgID, options, subresource) if args.Get(0) == nil { return nil, args.Error(1) } @@ -79,3 +80,11 @@ func (m *MockK8sHandler) GetStats(ctx context.Context, orgID int64) (*resource.R } return args.Get(0).(*resource.ResourceStatsResponse), args.Error(1) } + +func (m *MockK8sHandler) GetUserFromMeta(ctx context.Context, userMeta string) (*user.User, error) { + args := m.Called(ctx, userMeta) + if args.Get(0) == nil { + return nil, args.Error(1) + } + return args.Get(0).(*user.User), args.Error(1) +} diff --git a/pkg/services/apiserver/client/client_test.go b/pkg/services/apiserver/client/client_test.go new file mode 100644 index 00000000000..c97c5f0a31d --- /dev/null +++ b/pkg/services/apiserver/client/client_test.go @@ -0,0 +1,34 @@ +package client + +import ( + "context" + "testing" + + "github.com/grafana/grafana/pkg/services/user" + "github.com/grafana/grafana/pkg/services/user/usertest" + "github.com/stretchr/testify/require" +) + +func TestGetUserFromMeta(t *testing.T) { + userSvcTest := usertest.NewUserServiceFake() + userSvcTest.ExpectedUser = &user.User{ + ID: 1, + UID: "uid-value", + } + client := &k8sHandler{ + userService: userSvcTest, + } + t.Run("returns user with valid UID", func(t *testing.T) { + result, err := client.GetUserFromMeta(context.Background(), "user:uid-value") + require.NoError(t, err) + require.Equal(t, "uid-value", result.UID) + require.Equal(t, int64(1), result.ID) + }) + + t.Run("returns user when id is passed in", func(t *testing.T) { + result, err := client.GetUserFromMeta(context.Background(), "user:1") + require.NoError(t, err) + require.Equal(t, "uid-value", result.UID) + require.Equal(t, int64(1), result.ID) + }) +} diff --git a/pkg/services/dashboards/service/dashboard_service.go b/pkg/services/dashboards/service/dashboard_service.go index dd8d95348ad..ad78d14952f 100644 --- a/pkg/services/dashboards/service/dashboard_service.go +++ b/pkg/services/dashboards/service/dashboard_service.go @@ -68,7 +68,6 @@ type DashboardServiceImpl struct { dashboardStore dashboards.Store folderStore folder.FolderStore folderService folder.Service - userService user.Service orgService org.Service features featuremgmt.FeatureToggles folderPermissions accesscontrol.FolderPermissionsService @@ -91,7 +90,7 @@ func ProvideDashboardServiceImpl( restConfigProvider apiserver.RestConfigProvider, userService user.Service, unified resource.ResourceClient, quotaService quota.Service, orgService org.Service, publicDashboardService publicdashboards.ServiceWrapper, ) (*DashboardServiceImpl, error) { - k8sHandler := client.NewK8sHandler(cfg, request.GetNamespaceMapper(cfg), v0alpha1.DashboardResourceInfo.GroupVersionResource(), restConfigProvider, unified, dashboardStore) + k8sHandler := client.NewK8sHandler(cfg, request.GetNamespaceMapper(cfg), v0alpha1.DashboardResourceInfo.GroupVersionResource(), restConfigProvider, unified, dashboardStore, userService) dashSvc := &DashboardServiceImpl{ cfg: cfg, @@ -103,7 +102,6 @@ func ProvideDashboardServiceImpl( folderStore: folderStore, folderService: folderSvc, orgService: orgService, - userService: userService, k8sclient: k8sHandler, metrics: newDashboardsMetrics(r), dashboardPermissionsReady: make(chan struct{}), @@ -1489,7 +1487,7 @@ func (dr *DashboardServiceImpl) getDashboardThroughK8s(ctx context.Context, quer query.UID = result.UID } - out, err := dr.k8sclient.Get(ctx, query.UID, query.OrgID, subresource) + out, err := dr.k8sclient.Get(ctx, query.UID, query.OrgID, v1.GetOptions{}, subresource) if err != nil && !apierrors.IsNotFound(err) { return nil, err } else if err != nil || out == nil { @@ -1553,7 +1551,7 @@ func (dr *DashboardServiceImpl) saveDashboardThroughK8s(ctx context.Context, cmd func (dr *DashboardServiceImpl) createOrUpdateDash(ctx context.Context, obj unstructured.Unstructured, orgID int64) (*dashboards.Dashboard, error) { var out *unstructured.Unstructured - current, err := dr.k8sclient.Get(ctx, obj.GetName(), orgID) + current, err := dr.k8sclient.Get(ctx, obj.GetName(), orgID, v1.GetOptions{}) if current == nil || err != nil { out, err = dr.k8sclient.Create(ctx, &obj, orgID) if err != nil { @@ -1751,7 +1749,7 @@ func (dr *DashboardServiceImpl) searchProvisionedDashboardsThroughK8s(ctx contex for _, h := range searchResults.Hits { func(hit v0alpha1.DashboardHit) { g.Go(func() error { - out, err := dr.k8sclient.Get(ctx, hit.Name, query.OrgId) + out, err := dr.k8sclient.Get(ctx, hit.Name, query.OrgId, v1.GetOptions{}) if err != nil { return err } else if out == nil { @@ -1863,13 +1861,13 @@ func (dr *DashboardServiceImpl) UnstructuredToLegacyDashboard(ctx context.Contex out.PluginID = GetPluginIDFromMeta(obj) - creator, err := dr.getUserFromMeta(ctx, obj.GetCreatedBy()) + creator, err := dr.k8sclient.GetUserFromMeta(ctx, obj.GetCreatedBy()) if err != nil { return nil, err } out.CreatedBy = creator.ID - updater, err := dr.getUserFromMeta(ctx, obj.GetUpdatedBy()) + updater, err := dr.k8sclient.GetUserFromMeta(ctx, obj.GetUpdatedBy()) if err != nil { return nil, err } @@ -1906,25 +1904,6 @@ func (dr *DashboardServiceImpl) UnstructuredToLegacyDashboard(ctx context.Contex return &out, nil } -func (dr *DashboardServiceImpl) getUserFromMeta(ctx context.Context, userMeta string) (*user.User, error) { - if userMeta == "" || toUID(userMeta) == "" { - return &user.User{}, nil - } - usr, err := dr.getUser(ctx, toUID(userMeta)) - if err != nil && errors.Is(err, user.ErrUserNotFound) { - return &user.User{}, nil - } - return usr, err -} - -func (dr *DashboardServiceImpl) getUser(ctx context.Context, uid string) (*user.User, error) { - userId, err := strconv.ParseInt(uid, 10, 64) - if err == nil { - return dr.userService.GetByID(ctx, &user.GetUserByIDQuery{ID: userId}) - } - return dr.userService.GetByUID(ctx, &user.GetUserByUIDQuery{UID: uid}) -} - var pluginIDRepoName = "plugin" var fileProvisionedRepoPrefix = "file:" @@ -2010,11 +1989,3 @@ func LegacySaveCommandToUnstructured(cmd *dashboards.SaveDashboardCommand, names return finalObj, nil } - -func toUID(rawIdentifier string) string { - parts := strings.Split(rawIdentifier, ":") - if len(parts) < 2 { - return "" - } - return parts[1] -} diff --git a/pkg/services/dashboards/service/dashboard_service_test.go b/pkg/services/dashboards/service/dashboard_service_test.go index fbe99cf0fb4..d7adcd9ad9a 100644 --- a/pkg/services/dashboards/service/dashboard_service_test.go +++ b/pkg/services/dashboards/service/dashboard_service_test.go @@ -26,7 +26,6 @@ import ( "github.com/grafana/grafana/pkg/services/quota" "github.com/grafana/grafana/pkg/services/search/model" "github.com/grafana/grafana/pkg/services/user" - "github.com/grafana/grafana/pkg/services/user/usertest" "github.com/grafana/grafana/pkg/setting" "github.com/grafana/grafana/pkg/storage/unified/resource" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" @@ -315,7 +314,8 @@ func TestGetDashboard(t *testing.T) { Version: 1, Data: simplejson.NewFromAny(map[string]any{"test": "test", "title": "testing slugify", "uid": "uid", "version": int64(1)}), } - k8sCliMock.On("Get", mock.Anything, query.UID, mock.Anything, mock.Anything).Return(&dashboardUnstructured, nil).Once() + k8sCliMock.On("Get", mock.Anything, query.UID, mock.Anything, mock.Anything, mock.Anything).Return(&dashboardUnstructured, nil).Once() + k8sCliMock.On("GetUserFromMeta", mock.Anything, mock.Anything).Return(&user.User{}, nil) dashboard, err := service.GetDashboard(ctx, query) require.NoError(t, err) @@ -351,7 +351,8 @@ func TestGetDashboard(t *testing.T) { Version: 1, Data: simplejson.NewFromAny(map[string]any{"test": "test", "title": "testing slugify", "uid": "uid", "version": int64(1)}), } - k8sCliMock.On("Get", mock.Anything, mock.Anything, mock.Anything, mock.Anything).Return(&dashboardUnstructured, nil).Once() + k8sCliMock.On("Get", mock.Anything, mock.Anything, mock.Anything, mock.Anything, mock.Anything).Return(&dashboardUnstructured, nil).Once() + k8sCliMock.On("GetUserFromMeta", mock.Anything, mock.Anything).Return(&user.User{}, nil) k8sCliMock.On("Search", mock.Anything, mock.Anything, mock.Anything).Return(&resource.ResourceSearchResponse{ Results: &resource.ResourceTable{ Columns: []*resource.ResourceTableColumnDefinition{ @@ -391,7 +392,7 @@ func TestGetDashboard(t *testing.T) { t.Run("Should return error when Kubernetes client fails", func(t *testing.T) { ctx, k8sCliMock := setupK8sDashboardTests(service) - k8sCliMock.On("Get", mock.Anything, query.UID, mock.Anything, mock.Anything).Return(nil, assert.AnError).Once() + k8sCliMock.On("Get", mock.Anything, query.UID, mock.Anything, mock.Anything, mock.Anything).Return(nil, assert.AnError).Once() dashboard, err := service.GetDashboard(ctx, query) require.Error(t, err) @@ -401,7 +402,7 @@ func TestGetDashboard(t *testing.T) { t.Run("Should return dashboard not found if Kubernetes client returns nil", func(t *testing.T) { ctx, k8sCliMock := setupK8sDashboardTests(service) - k8sCliMock.On("Get", mock.Anything, query.UID, mock.Anything, mock.Anything).Return(nil, nil).Once() + k8sCliMock.On("Get", mock.Anything, query.UID, mock.Anything, mock.Anything, mock.Anything).Return(nil, nil).Once() dashboard, err := service.GetDashboard(ctx, query) require.Error(t, err) require.Equal(t, dashboards.ErrDashboardNotFound, err) @@ -450,6 +451,7 @@ func TestGetAllDashboards(t *testing.T) { Data: simplejson.NewFromAny(map[string]any{"test": "test", "title": "testing slugify", "uid": "uid", "version": int64(1)}), } + k8sCliMock.On("GetUserFromMeta", mock.Anything, mock.Anything).Return(&user.User{}, nil) k8sCliMock.On("List", mock.Anything, mock.Anything, mock.Anything).Return(&unstructured.UnstructuredList{Items: []unstructured.Unstructured{dashboardUnstructured}}, nil).Once() dashes, err := service.GetAllDashboards(ctx) @@ -501,6 +503,7 @@ func TestGetAllDashboardsByOrgId(t *testing.T) { Data: simplejson.NewFromAny(map[string]any{"test": "test", "title": "testing slugify", "uid": "uid", "version": int64(1)}), } + k8sCliMock.On("GetUserFromMeta", mock.Anything, mock.Anything).Return(&user.User{}, nil) k8sCliMock.On("List", mock.Anything, mock.Anything, mock.Anything).Return(&unstructured.UnstructuredList{Items: []unstructured.Unstructured{dashboardUnstructured}}, nil).Once() dashes, err := service.GetAllDashboardsByOrgId(ctx, 1) @@ -534,7 +537,7 @@ func TestGetProvisionedDashboardData(t *testing.T) { t.Run("Should use Kubernetes client if feature flags are enabled and get from relevant org", func(t *testing.T) { ctx, k8sCliMock := setupK8sDashboardTests(service) - k8sCliMock.On("Get", mock.Anything, mock.Anything, mock.Anything, mock.Anything).Return(&unstructured.Unstructured{Object: map[string]any{ + k8sCliMock.On("Get", mock.Anything, mock.Anything, mock.Anything, mock.Anything, mock.Anything).Return(&unstructured.Unstructured{Object: map[string]any{ "metadata": map[string]any{ "name": "uid", "labels": map[string]any{ @@ -632,7 +635,7 @@ func TestGetProvisionedDashboardDataByDashboardID(t *testing.T) { t.Run("Should use Kubernetes client if feature flags are enabled and get from whatever org it is in", func(t *testing.T) { ctx, k8sCliMock := setupK8sDashboardTests(service) - k8sCliMock.On("Get", mock.Anything, mock.Anything, mock.Anything, mock.Anything).Return(&unstructured.Unstructured{Object: map[string]any{ + k8sCliMock.On("Get", mock.Anything, mock.Anything, mock.Anything, mock.Anything, mock.Anything).Return(&unstructured.Unstructured{Object: map[string]any{ "metadata": map[string]any{ "name": "uid", "labels": map[string]any{ @@ -721,7 +724,7 @@ func TestGetProvisionedDashboardDataByDashboardUID(t *testing.T) { t.Run("Should use Kubernetes client if feature flags are enabled", func(t *testing.T) { ctx, k8sCliMock := setupK8sDashboardTests(service) - k8sCliMock.On("Get", mock.Anything, mock.Anything, mock.Anything, mock.Anything).Return(&unstructured.Unstructured{Object: map[string]any{ + k8sCliMock.On("Get", mock.Anything, mock.Anything, mock.Anything, mock.Anything, mock.Anything).Return(&unstructured.Unstructured{Object: map[string]any{ "metadata": map[string]any{ "name": "uid", "labels": map[string]any{ @@ -812,7 +815,7 @@ func TestDeleteOrphanedProvisionedDashboards(t *testing.T) { fakeStore.On("CleanupAfterDelete", mock.Anything, &dashboards.DeleteDashboardCommand{UID: "uid", OrgID: 1}).Return(nil).Once() fakeStore.On("CleanupAfterDelete", mock.Anything, &dashboards.DeleteDashboardCommand{UID: "uid3", OrgID: 2}).Return(nil).Once() fakePublicDashboardService.On("DeleteByDashboardUIDs", mock.Anything, mock.Anything, mock.Anything).Return(nil) - k8sCliMock.On("Get", mock.Anything, "uid", mock.Anything, mock.Anything).Return(&unstructured.Unstructured{Object: map[string]any{ + k8sCliMock.On("Get", mock.Anything, "uid", mock.Anything, mock.Anything, mock.Anything).Return(&unstructured.Unstructured{Object: map[string]any{ "metadata": map[string]any{ "name": "uid", "annotations": map[string]any{ @@ -825,7 +828,7 @@ func TestDeleteOrphanedProvisionedDashboards(t *testing.T) { "spec": map[string]any{}, }}, nil).Once() // should not delete this one, because it does not start with "file:" - k8sCliMock.On("Get", mock.Anything, "uid2", mock.Anything, mock.Anything).Return(&unstructured.Unstructured{Object: map[string]any{ + k8sCliMock.On("Get", mock.Anything, "uid2", mock.Anything, mock.Anything, mock.Anything).Return(&unstructured.Unstructured{Object: map[string]any{ "metadata": map[string]any{ "name": "uid2", "annotations": map[string]any{ @@ -836,7 +839,7 @@ func TestDeleteOrphanedProvisionedDashboards(t *testing.T) { "spec": map[string]any{}, }}, nil).Once() - k8sCliMock.On("Get", mock.Anything, "uid3", mock.Anything, mock.Anything).Return(&unstructured.Unstructured{Object: map[string]any{ + k8sCliMock.On("Get", mock.Anything, "uid3", mock.Anything, mock.Anything, mock.Anything).Return(&unstructured.Unstructured{Object: map[string]any{ "metadata": map[string]any{ "name": "uid3", "annotations": map[string]any{ @@ -958,7 +961,7 @@ func TestUnprovisionDashboard(t *testing.T) { }, "spec": map[string]any{}, }} - k8sCliMock.On("Get", mock.Anything, mock.Anything, mock.Anything, mock.Anything).Return(dash, nil) + k8sCliMock.On("Get", mock.Anything, mock.Anything, mock.Anything, mock.Anything, mock.Anything).Return(dash, nil) dashWithoutAnnotations := &unstructured.Unstructured{Object: map[string]any{ "apiVersion": "dashboard.grafana.app/v0alpha1", "kind": "Dashboard", @@ -975,6 +978,7 @@ func TestUnprovisionDashboard(t *testing.T) { // should update it to be without annotations k8sCliMock.On("Update", mock.Anything, dashWithoutAnnotations, mock.Anything, mock.Anything).Return(dashWithoutAnnotations, nil) k8sCliMock.On("GetNamespace", mock.Anything).Return("default") + k8sCliMock.On("GetUserFromMeta", mock.Anything, mock.Anything).Return(&user.User{}, nil) k8sCliMock.On("Search", mock.Anything, mock.Anything, mock.Anything).Return(&resource.ResourceSearchResponse{ Results: &resource.ResourceTable{ Columns: []*resource.ResourceTableColumnDefinition{ @@ -1039,7 +1043,8 @@ func TestGetDashboardsByPluginID(t *testing.T) { t.Run("Should use Kubernetes client if feature flags are enabled", func(t *testing.T) { ctx, k8sCliMock := setupK8sDashboardTests(service) - k8sCliMock.On("Get", mock.Anything, "uid", mock.Anything, mock.Anything).Return(uidUnstructured, nil) + k8sCliMock.On("Get", mock.Anything, "uid", mock.Anything, mock.Anything, mock.Anything).Return(uidUnstructured, nil) + k8sCliMock.On("GetUserFromMeta", mock.Anything, mock.Anything).Return(&user.User{}, nil) k8sCliMock.On("Search", mock.Anything, mock.Anything, mock.MatchedBy(func(req *resource.ResourceSearchRequest) bool { return req.Options.Fields[0].Key == "repo.name" && req.Options.Fields[0].Values[0] == "plugin" && req.Options.Fields[1].Key == "repo.path" && req.Options.Fields[1].Values[0] == "testing" @@ -1127,7 +1132,8 @@ func TestSaveProvisionedDashboard(t *testing.T) { t.Run("Should use Kubernetes create if feature flags are enabled", func(t *testing.T) { ctx, k8sCliMock := setupK8sDashboardTests(service) fakeStore.On("SaveProvisionedDashboard", mock.Anything, mock.Anything, mock.Anything).Return(nil) - k8sCliMock.On("Get", mock.Anything, mock.Anything, mock.Anything, mock.Anything).Return(nil, nil) + k8sCliMock.On("Get", mock.Anything, mock.Anything, mock.Anything, mock.Anything, mock.Anything).Return(nil, nil) + k8sCliMock.On("GetUserFromMeta", mock.Anything, mock.Anything).Return(&user.User{}, nil) k8sCliMock.On("Create", mock.Anything, mock.Anything, mock.Anything, mock.Anything).Return(&dashboardUnstructured, nil) k8sCliMock.On("GetNamespace", mock.Anything).Return("default") @@ -1188,7 +1194,8 @@ func TestSaveDashboard(t *testing.T) { t.Run("Should use Kubernetes create if feature flags are enabled and dashboard doesn't exist", func(t *testing.T) { ctx, k8sCliMock := setupK8sDashboardTests(service) - k8sCliMock.On("Get", mock.Anything, mock.Anything, mock.Anything, mock.Anything).Return(nil, nil) + k8sCliMock.On("Get", mock.Anything, mock.Anything, mock.Anything, mock.Anything, mock.Anything).Return(nil, nil) + k8sCliMock.On("GetUserFromMeta", mock.Anything, mock.Anything).Return(&user.User{}, nil) k8sCliMock.On("GetNamespace", mock.Anything).Return("default") k8sCliMock.On("Create", mock.Anything, mock.Anything, mock.Anything, mock.Anything).Return(&dashboardUnstructured, nil) @@ -1199,7 +1206,8 @@ func TestSaveDashboard(t *testing.T) { t.Run("Should use Kubernetes update if feature flags are enabled and dashboard exists", func(t *testing.T) { ctx, k8sCliMock := setupK8sDashboardTests(service) - k8sCliMock.On("Get", mock.Anything, mock.Anything, mock.Anything, mock.Anything).Return(&dashboardUnstructured, nil) + k8sCliMock.On("Get", mock.Anything, mock.Anything, mock.Anything, mock.Anything, mock.Anything).Return(&dashboardUnstructured, nil) + k8sCliMock.On("GetUserFromMeta", mock.Anything, mock.Anything).Return(&user.User{}, nil) k8sCliMock.On("GetNamespace", mock.Anything).Return("default") k8sCliMock.On("Update", mock.Anything, mock.Anything, mock.Anything, mock.Anything).Return(&dashboardUnstructured, nil) @@ -1210,7 +1218,7 @@ func TestSaveDashboard(t *testing.T) { t.Run("Should return an error if uid is invalid", func(t *testing.T) { ctx, k8sCliMock := setupK8sDashboardTests(service) - k8sCliMock.On("Get", mock.Anything, mock.Anything, mock.Anything, mock.Anything).Return(nil, nil) + k8sCliMock.On("Get", mock.Anything, mock.Anything, mock.Anything, mock.Anything, mock.Anything).Return(nil, nil) k8sCliMock.On("GetNamespace", mock.Anything).Return("default") k8sCliMock.On("Create", mock.Anything, mock.Anything, mock.Anything, mock.Anything).Return(&dashboardUnstructured, nil) @@ -1495,8 +1503,9 @@ func TestGetDashboards(t *testing.T) { t.Run("Should use Kubernetes client if feature flags are enabled", func(t *testing.T) { ctx, k8sCliMock := setupK8sDashboardTests(service) - k8sCliMock.On("Get", mock.Anything, "uid1", mock.Anything, mock.Anything).Return(uid1Unstructured, nil) - k8sCliMock.On("Get", mock.Anything, "uid2", mock.Anything, mock.Anything).Return(uid2Unstructured, nil) + k8sCliMock.On("Get", mock.Anything, "uid1", mock.Anything, mock.Anything, mock.Anything).Return(uid1Unstructured, nil) + k8sCliMock.On("Get", mock.Anything, "uid2", mock.Anything, mock.Anything, mock.Anything).Return(uid2Unstructured, nil) + k8sCliMock.On("GetUserFromMeta", mock.Anything, mock.Anything).Return(&user.User{}, nil) k8sCliMock.On("Search", mock.Anything, mock.Anything, mock.Anything).Return(&resource.ResourceSearchResponse{ Results: &resource.ResourceTable{ Columns: []*resource.ResourceTableColumnDefinition{ @@ -1611,10 +1620,10 @@ func TestGetDashboardUIDByID(t *testing.T) { } func TestUnstructuredToLegacyDashboard(t *testing.T) { - fake := usertest.NewUserServiceFake() - fake.ExpectedUser = &user.User{ID: 10, UID: "useruid"} + k8sCliMock := new(client.MockK8sHandler) + k8sCliMock.On("GetUserFromMeta", mock.Anything, mock.Anything).Return(&user.User{ID: 10, UID: "useruid"}, nil) dr := &DashboardServiceImpl{ - userService: fake, + k8sclient: k8sCliMock, } t.Run("successfully converts unstructured to legacy dashboard", func(t *testing.T) { uid := "36b7c825-79cc-435e-acf6-c78bd96a4510" @@ -1912,17 +1921,3 @@ func TestLegacySaveCommandToUnstructured(t *testing.T) { assert.Equal(t, result.GetAnnotations(), map[string]string(nil)) }) } - -func TestToUID(t *testing.T) { - t.Run("parses valid UID", func(t *testing.T) { - rawIdentifier := "user:uid-value" - result := toUID(rawIdentifier) - assert.Equal(t, "uid-value", result) - }) - - t.Run("returns empty string for invalid identifier", func(t *testing.T) { - rawIdentifier := "invalid-uid" - result := toUID(rawIdentifier) - assert.Equal(t, "", result) - }) -} diff --git a/pkg/services/dashboardversion/dashver.go b/pkg/services/dashboardversion/dashver.go index 8609e238d55..9d4dcb60264 100644 --- a/pkg/services/dashboardversion/dashver.go +++ b/pkg/services/dashboardversion/dashver.go @@ -7,5 +7,5 @@ import ( type Service interface { Get(context.Context, *GetDashboardVersionQuery) (*DashboardVersionDTO, error) DeleteExpired(context.Context, *DeleteExpiredVersionsCommand) error - List(context.Context, *ListDashboardVersionsQuery) ([]*DashboardVersionDTO, error) + List(context.Context, *ListDashboardVersionsQuery) (*DashboardVersionResponse, error) } diff --git a/pkg/services/dashboardversion/dashverimpl/dashver.go b/pkg/services/dashboardversion/dashverimpl/dashver.go index 04fed84c9bd..02bed95b96e 100644 --- a/pkg/services/dashboardversion/dashverimpl/dashver.go +++ b/pkg/services/dashboardversion/dashverimpl/dashver.go @@ -3,12 +3,26 @@ package dashverimpl import ( "context" "errors" + "fmt" + "strconv" + "strings" + "github.com/grafana/grafana/pkg/apimachinery/utils" + "github.com/grafana/grafana/pkg/apis/dashboard/v0alpha1" + "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/services/apiserver" + "github.com/grafana/grafana/pkg/services/apiserver/client" + "github.com/grafana/grafana/pkg/services/apiserver/endpoints/request" "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" + "github.com/grafana/grafana/pkg/storage/unified/resource" + v1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" ) const ( @@ -17,19 +31,32 @@ const ( ) type Service struct { - cfg *setting.Cfg - store store - dashSvc dashboards.DashboardService - log log.Logger + cfg *setting.Cfg + store store + dashSvc dashboards.DashboardService + k8sclient client.K8sHandler + features featuremgmt.FeatureToggles + log log.Logger } -func ProvideService(cfg *setting.Cfg, db db.DB, dashboardService dashboards.DashboardService) dashver.Service { +func ProvideService(cfg *setting.Cfg, db db.DB, dashboardService dashboards.DashboardService, dashboardStore dashboards.Store, features featuremgmt.FeatureToggles, + restConfigProvider apiserver.RestConfigProvider, userService user.Service, unified resource.ResourceClient) dashver.Service { return &Service{ cfg: cfg, store: &sqlStore{ db: db, dialect: db.GetDialect(), }, + features: features, + k8sclient: client.NewK8sHandler( + cfg, + request.GetNamespaceMapper(cfg), + v0alpha1.DashboardResourceInfo.GroupVersionResource(), + restConfigProvider, + unified, + dashboardStore, + userService, + ), dashSvc: dashboardService, log: log.New("dashboard-version"), } @@ -49,13 +76,21 @@ func (s *Service) Get(ctx context.Context, query *dashver.GetDashboardVersionQue // versions table, at time of this writing), so get the DashboardID if it // was not populated. if query.DashboardID == 0 { - id, err := s.getDashIDMaybeEmpty(ctx, query.DashboardUID) + id, err := s.getDashIDMaybeEmpty(ctx, query.DashboardUID, query.OrgID) if err != nil { return nil, err } query.DashboardID = id } + if s.features.IsEnabledGlobally(featuremgmt.FlagKubernetesCliDashboards) { + version, err := s.getHistoryThroughK8s(ctx, query.OrgID, query.DashboardUID, query.Version) + if err != nil { + return nil, err + } + return version, nil + } + version, err := s.store.Get(ctx, query) if err != nil { return nil, err @@ -95,7 +130,7 @@ func (s *Service) DeleteExpired(ctx context.Context, cmd *dashver.DeleteExpiredV } // List all dashboard versions for the given dashboard ID. -func (s *Service) List(ctx context.Context, query *dashver.ListDashboardVersionsQuery) ([]*dashver.DashboardVersionDTO, error) { +func (s *Service) List(ctx context.Context, query *dashver.ListDashboardVersionsQuery) (*dashver.DashboardVersionResponse, error) { // Get the DashboardUID if not populated if query.DashboardUID == "" { u, err := s.getDashUIDMaybeEmpty(ctx, query.DashboardID) @@ -109,7 +144,7 @@ func (s *Service) List(ctx context.Context, query *dashver.ListDashboardVersions // versions table, at time of this writing), so get the DashboardID if it // was not populated. if query.DashboardID == 0 { - id, err := s.getDashIDMaybeEmpty(ctx, query.DashboardUID) + id, err := s.getDashIDMaybeEmpty(ctx, query.DashboardUID, query.OrgID) if err != nil { return nil, err } @@ -118,6 +153,21 @@ func (s *Service) List(ctx context.Context, query *dashver.ListDashboardVersions if query.Limit == 0 { query.Limit = 1000 } + + if s.features.IsEnabledGlobally(featuremgmt.FlagKubernetesCliDashboards) { + versions, err := s.listHistoryThroughK8s( + ctx, + query.OrgID, + query.DashboardUID, + int64(query.Limit), + query.ContinueToken, + ) + if err != nil { + return nil, err + } + return versions, nil + } + dvs, err := s.store.List(ctx, query) if err != nil { return nil, err @@ -126,7 +176,9 @@ func (s *Service) List(ctx context.Context, query *dashver.ListDashboardVersions for i, v := range dvs { dtos[i] = v.ToDTO(query.DashboardUID) } - return dtos, nil + return &dashver.DashboardVersionResponse{ + Versions: dtos, + }, nil } // getDashUIDMaybeEmpty is a helper function which takes a dashboardID and @@ -149,8 +201,8 @@ func (s *Service) getDashUIDMaybeEmpty(ctx context.Context, id int64) (string, e // getDashIDMaybeEmpty is a helper function which takes a dashboardUID and // returns the ID. If the dashboard is not found, it will return -1. -func (s *Service) getDashIDMaybeEmpty(ctx context.Context, uid string) (int64, error) { - q := dashboards.GetDashboardQuery{UID: uid} +func (s *Service) getDashIDMaybeEmpty(ctx context.Context, uid string, orgID int64) (int64, error) { + q := dashboards.GetDashboardQuery{UID: uid, OrgID: orgID} result, err := s.dashSvc.GetDashboard(ctx, &q) if err != nil { if errors.Is(err, dashboards.ErrDashboardNotFound) { @@ -163,3 +215,116 @@ func (s *Service) getDashIDMaybeEmpty(ctx context.Context, uid string) (int64, e } return result.ID, nil } + +func (s *Service) getHistoryThroughK8s(ctx context.Context, orgID int64, dashboardUID string, rv int64) (*dashver.DashboardVersionDTO, error) { + out, err := s.k8sclient.Get(ctx, dashboardUID, orgID, v1.GetOptions{ResourceVersion: strconv.FormatInt(rv, 10)}) + if err != nil { + return nil, err + } else if out == nil { + return nil, dashboards.ErrDashboardNotFound + } + + dash, err := s.UnstructuredToLegacyDashboardVersion(ctx, out, orgID) + if err != nil { + return nil, err + } + + return dash, nil +} + +func (s *Service) listHistoryThroughK8s(ctx context.Context, orgID int64, dashboardUID string, limit int64, continueToken string) (*dashver.DashboardVersionResponse, error) { + out, err := s.k8sclient.List(ctx, orgID, v1.ListOptions{ + LabelSelector: utils.LabelKeyGetHistory + "=" + dashboardUID, + Limit: limit, + Continue: continueToken, + }) + if err != nil { + return nil, err + } else if out == nil { + return nil, dashboards.ErrDashboardNotFound + } + + dashboards := make([]*dashver.DashboardVersionDTO, len(out.Items)) + for i, item := range out.Items { + dash, err := s.UnstructuredToLegacyDashboardVersion(ctx, &item, orgID) + if err != nil { + return nil, err + } + dashboards[i] = dash + } + + return &dashver.DashboardVersionResponse{ + ContinueToken: out.GetContinue(), + Versions: dashboards, + }, nil +} + +func (s *Service) UnstructuredToLegacyDashboardVersion(ctx context.Context, item *unstructured.Unstructured, orgID int64) (*dashver.DashboardVersionDTO, error) { + spec, ok := item.Object["spec"].(map[string]any) + if !ok { + return nil, errors.New("error parsing dashboard from k8s response") + } + obj, err := utils.MetaAccessor(item) + if err != nil { + return nil, err + } + uid := obj.GetName() + spec["uid"] = uid + + dashVersion := 0 + parentVersion := 0 + if version, ok := spec["version"].(int64); ok { + dashVersion = int(version) + parentVersion = dashVersion - 1 + } + + createdBy, err := s.k8sclient.GetUserFromMeta(ctx, obj.GetCreatedBy()) + if err != nil { + return nil, err + } + + id, err := obj.GetResourceVersionInt64() + if err != nil { + return nil, err + } + + restoreVer, err := getRestoreVersion(obj.GetMessage()) + if err != nil { + return nil, err + } + + out := dashver.DashboardVersionDTO{ + ID: id, + DashboardID: obj.GetDeprecatedInternalID(), // nolint:staticcheck + DashboardUID: uid, + Created: obj.GetCreationTimestamp().Time, + CreatedBy: createdBy.ID, + Message: obj.GetMessage(), + RestoredFrom: restoreVer, + Version: dashVersion, + ParentVersion: parentVersion, + Data: simplejson.NewFromAny(spec), + } + + return &out, 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.ParseInt(parts[1], 10, 64) + if err != nil { + return 0, err + } + + return int(ver), nil +} diff --git a/pkg/services/dashboardversion/dashverimpl/dashver_test.go b/pkg/services/dashboardversion/dashverimpl/dashver_test.go index 7ec8fc4ef64..ed58c27bb7f 100644 --- a/pkg/services/dashboardversion/dashverimpl/dashver_test.go +++ b/pkg/services/dashboardversion/dashverimpl/dashver_test.go @@ -7,18 +7,24 @@ import ( "github.com/stretchr/testify/mock" "github.com/stretchr/testify/require" + v1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" + "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" ) func TestDashboardVersionService(t *testing.T) { dashboardVersionStore := newDashboardVersionStoreFake() dashboardService := dashboards.NewFakeDashboardService(t) - dashboardVersionService := Service{store: dashboardVersionStore, dashSvc: dashboardService} + dashboardVersionService := Service{store: dashboardVersionStore, dashSvc: dashboardService, features: featuremgmt.WithFeatures()} t.Run("Get dashboard version", func(t *testing.T) { dashboard := &dashver.DashboardVersion{ @@ -32,6 +38,44 @@ func TestDashboardVersionService(t *testing.T) { require.NoError(t, err) require.Equal(t, dashboard.ToDTO("uid"), dashboardVersion) }) + + t.Run("Get dashboard version through k8s", func(t *testing.T) { + dashboardService := dashboards.NewFakeDashboardService(t) + dashboardVersionService := Service{dashSvc: dashboardService, features: featuremgmt.WithFeatures()} + mockCli := new(client.MockK8sHandler) + dashboardVersionService.k8sclient = mockCli + dashboardVersionService.features = featuremgmt.WithFeatures(featuremgmt.FlagKubernetesCliDashboards) + dashboardService.On("GetDashboardUIDByID", mock.Anything, mock.AnythingOfType("*dashboards.GetDashboardRefByIDQuery")).Return(&dashboards.DashboardRef{UID: "uid"}, nil) + + mockCli.On("GetUserFromMeta", mock.Anything, mock.Anything).Return(&user.User{}, nil) + mockCli.On("Get", mock.Anything, "uid", int64(1), v1.GetOptions{ResourceVersion: "10"}, mock.Anything).Return(&unstructured.Unstructured{ + Object: map[string]any{ + "metadata": map[string]any{ + "name": "uid", + "resourceVersion": "12", + "labels": map[string]any{ + utils.LabelKeyDeprecatedInternalID: "42", // nolint:staticcheck + }, + }, + "spec": map[string]any{ + "version": int64(10), + }, + }}, nil).Once() + res, err := dashboardVersionService.Get(context.Background(), &dashver.GetDashboardVersionQuery{ + DashboardID: 42, + OrgID: 1, + Version: 10, + }) + require.Nil(t, err) + require.Equal(t, res, &dashver.DashboardVersionDTO{ + ID: 12, // RV should be used + Version: 10, + ParentVersion: 9, + DashboardID: 42, + DashboardUID: "uid", + Data: simplejson.NewFromAny(map[string]any{"uid": "uid", "version": int64(10)}), + }) + }) } func TestDeleteExpiredVersions(t *testing.T) { @@ -42,7 +86,7 @@ func TestDeleteExpiredVersions(t *testing.T) { dashboardVersionStore := newDashboardVersionStoreFake() dashboardService := dashboards.NewFakeDashboardService(t) dashboardVersionService := Service{ - cfg: cfg, store: dashboardVersionStore, dashSvc: dashboardService} + cfg: cfg, store: dashboardVersionStore, dashSvc: dashboardService, features: featuremgmt.WithFeatures()} t.Run("Don't delete anything if there are no expired versions", func(t *testing.T) { err := dashboardVersionService.DeleteExpired(context.Background(), &dashver.DeleteExpiredVersionsCommand{DeletedRows: 4}) @@ -67,7 +111,7 @@ func TestListDashboardVersions(t *testing.T) { t.Run("List all versions for a given Dashboard ID", func(t *testing.T) { dashboardVersionStore := newDashboardVersionStoreFake() dashboardService := dashboards.NewFakeDashboardService(t) - dashboardVersionService := Service{store: dashboardVersionStore, dashSvc: dashboardService} + dashboardVersionService := Service{store: dashboardVersionStore, dashSvc: dashboardService, features: featuremgmt.WithFeatures()} dashboardVersionStore.ExpectedListVersions = []*dashver.DashboardVersion{ {ID: 1, DashboardID: 42}, } @@ -78,15 +122,15 @@ func TestListDashboardVersions(t *testing.T) { query := dashver.ListDashboardVersionsQuery{DashboardID: 42} res, err := dashboardVersionService.List(context.Background(), &query) require.Nil(t, err) - require.Equal(t, 1, len(res)) + require.Equal(t, 1, len(res.Versions)) // validate that the UID was populated - require.EqualValues(t, []*dashver.DashboardVersionDTO{{ID: 1, DashboardID: 42, DashboardUID: "uid"}}, res) + require.EqualValues(t, &dashver.DashboardVersionResponse{Versions: []*dashver.DashboardVersionDTO{{ID: 1, DashboardID: 42, DashboardUID: "uid"}}}, res) }) t.Run("List all versions for a non-existent DashboardID", func(t *testing.T) { dashboardVersionStore := newDashboardVersionStoreFake() dashboardService := dashboards.NewFakeDashboardService(t) - dashboardVersionService := Service{store: dashboardVersionStore, dashSvc: dashboardService, log: log.NewNopLogger()} + dashboardVersionService := Service{store: dashboardVersionStore, dashSvc: dashboardService, log: log.NewNopLogger(), features: featuremgmt.WithFeatures()} dashboardVersionStore.ExpectedListVersions = []*dashver.DashboardVersion{ {ID: 1, DashboardID: 42}, } @@ -96,15 +140,15 @@ func TestListDashboardVersions(t *testing.T) { query := dashver.ListDashboardVersionsQuery{DashboardID: 42} res, err := dashboardVersionService.List(context.Background(), &query) require.Nil(t, err) - require.Equal(t, 1, len(res)) + require.Equal(t, 1, len(res.Versions)) // The DashboardID remains populated with the given value, even though the dash was not found - require.EqualValues(t, []*dashver.DashboardVersionDTO{{ID: 1, DashboardID: 42}}, res) + require.EqualValues(t, &dashver.DashboardVersionResponse{Versions: []*dashver.DashboardVersionDTO{{ID: 1, DashboardID: 42}}}, res) }) t.Run("List all versions for a given DashboardUID", func(t *testing.T) { dashboardVersionStore := newDashboardVersionStoreFake() dashboardService := dashboards.NewFakeDashboardService(t) - dashboardVersionService := Service{store: dashboardVersionStore, dashSvc: dashboardService, log: log.NewNopLogger()} + dashboardVersionService := Service{store: dashboardVersionStore, dashSvc: dashboardService, log: log.NewNopLogger(), features: featuremgmt.WithFeatures()} dashboardVersionStore.ExpectedListVersions = []*dashver.DashboardVersion{{DashboardID: 42, ID: 1}} dashboardService.On("GetDashboard", mock.Anything, mock.AnythingOfType("*dashboards.GetDashboardQuery")). Return(&dashboards.Dashboard{ID: 42}, nil) @@ -112,15 +156,15 @@ func TestListDashboardVersions(t *testing.T) { query := dashver.ListDashboardVersionsQuery{DashboardUID: "uid"} res, err := dashboardVersionService.List(context.Background(), &query) require.Nil(t, err) - require.Equal(t, 1, len(res)) + require.Equal(t, 1, len(res.Versions)) // validate that the dashboardID was populated from the GetDashboard method call. - require.EqualValues(t, []*dashver.DashboardVersionDTO{{ID: 1, DashboardID: 42, DashboardUID: "uid"}}, res) + require.EqualValues(t, &dashver.DashboardVersionResponse{Versions: []*dashver.DashboardVersionDTO{{ID: 1, DashboardID: 42, DashboardUID: "uid"}}}, res) }) t.Run("List all versions for a given non-existent DashboardUID", func(t *testing.T) { dashboardVersionStore := newDashboardVersionStoreFake() dashboardService := dashboards.NewFakeDashboardService(t) - dashboardVersionService := Service{store: dashboardVersionStore, dashSvc: dashboardService, log: log.NewNopLogger()} + dashboardVersionService := Service{store: dashboardVersionStore, dashSvc: dashboardService, log: log.NewNopLogger(), features: featuremgmt.WithFeatures()} dashboardVersionStore.ExpectedListVersions = []*dashver.DashboardVersion{{DashboardID: 42, ID: 1}} dashboardService.On("GetDashboard", mock.Anything, mock.AnythingOfType("*dashboards.GetDashboardQuery")). Return(nil, dashboards.ErrDashboardNotFound) @@ -128,15 +172,15 @@ func TestListDashboardVersions(t *testing.T) { query := dashver.ListDashboardVersionsQuery{DashboardUID: "uid"} res, err := dashboardVersionService.List(context.Background(), &query) require.Nil(t, err) - require.Equal(t, 1, len(res)) + require.Equal(t, 1, len(res.Versions)) // validate that the dashboardUID & ID are populated, even though the dash was not found - require.EqualValues(t, []*dashver.DashboardVersionDTO{{ID: 1, DashboardID: 42, DashboardUID: "uid"}}, res) + require.EqualValues(t, &dashver.DashboardVersionResponse{Versions: []*dashver.DashboardVersionDTO{{ID: 1, DashboardID: 42, DashboardUID: "uid"}}}, res) }) t.Run("List Dashboard versions - error from store", func(t *testing.T) { dashboardVersionStore := newDashboardVersionStoreFake() dashboardService := dashboards.NewFakeDashboardService(t) - dashboardVersionService := Service{store: dashboardVersionStore, dashSvc: dashboardService, log: log.NewNopLogger()} + dashboardVersionService := Service{store: dashboardVersionStore, dashSvc: dashboardService, log: log.NewNopLogger(), features: featuremgmt.WithFeatures()} dashboardVersionStore.ExpectedError = dashver.ErrDashboardVersionNotFound query := dashver.ListDashboardVersionsQuery{DashboardID: 42, DashboardUID: "42"} @@ -144,6 +188,46 @@ func TestListDashboardVersions(t *testing.T) { require.Nil(t, res) require.ErrorIs(t, err, dashver.ErrDashboardVersionNotFound) }) + + t.Run("List all versions for a given Dashboard ID through k8s", func(t *testing.T) { + dashboardService := dashboards.NewFakeDashboardService(t) + dashboardVersionService := Service{dashSvc: dashboardService, features: featuremgmt.WithFeatures()} + mockCli := new(client.MockK8sHandler) + dashboardVersionService.k8sclient = mockCli + dashboardVersionService.features = featuremgmt.WithFeatures(featuremgmt.FlagKubernetesCliDashboards) + + dashboardService.On("GetDashboardUIDByID", mock.Anything, + mock.AnythingOfType("*dashboards.GetDashboardRefByIDQuery")). + Return(&dashboards.DashboardRef{UID: "uid"}, nil) + + query := dashver.ListDashboardVersionsQuery{DashboardID: 42} + mockCli.On("GetUserFromMeta", mock.Anything, mock.Anything).Return(&user.User{}, nil) + mockCli.On("List", mock.Anything, mock.Anything, mock.Anything).Return(&unstructured.UnstructuredList{ + Items: []unstructured.Unstructured{{Object: map[string]any{ + "metadata": map[string]any{ + "name": "uid", + "resourceVersion": "12", + "labels": map[string]any{ + utils.LabelKeyDeprecatedInternalID: "42", // nolint:staticcheck + }, + }, + "spec": map[string]any{ + "version": int64(5), + }, + }}}}, nil).Once() + res, err := dashboardVersionService.List(context.Background(), &query) + require.Nil(t, err) + require.Equal(t, 1, len(res.Versions)) + require.EqualValues(t, &dashver.DashboardVersionResponse{ + Versions: []*dashver.DashboardVersionDTO{{ + ID: 12, // should take rv + DashboardID: 42, + ParentVersion: 4, + Version: 5, // should take from spec + DashboardUID: "uid", + Data: simplejson.NewFromAny(map[string]any{"uid": "uid", "version": int64(5)}), + }}}, res) + }) } type FakeDashboardVersionStore struct { diff --git a/pkg/services/dashboardversion/dashverimpl/store_test.go b/pkg/services/dashboardversion/dashverimpl/store_test.go index cccd1d220ce..a8c26b1d102 100644 --- a/pkg/services/dashboardversion/dashverimpl/store_test.go +++ b/pkg/services/dashboardversion/dashverimpl/store_test.go @@ -34,7 +34,7 @@ func testIntegrationGetDashboardVersion(t *testing.T, fn getStore) { query := dashver.GetDashboardVersionQuery{ DashboardID: savedDash.ID, - Version: savedDash.Version, + Version: int64(savedDash.Version), OrgID: 1, } @@ -60,7 +60,7 @@ func testIntegrationGetDashboardVersion(t *testing.T, fn getStore) { t.Run("Attempt to get a version that doesn't exist", func(t *testing.T) { query := dashver.GetDashboardVersionQuery{ DashboardID: int64(999), - Version: 123, + Version: int64(123), OrgID: 1, } diff --git a/pkg/services/dashboardversion/dashvertest/fake.go b/pkg/services/dashboardversion/dashvertest/fake.go index 1d45c63aa3c..acef53263bc 100644 --- a/pkg/services/dashboardversion/dashvertest/fake.go +++ b/pkg/services/dashboardversion/dashvertest/fake.go @@ -10,6 +10,7 @@ type FakeDashboardVersionService struct { ExpectedDashboardVersion *dashver.DashboardVersionDTO ExpectedDashboardVersions []*dashver.DashboardVersionDTO ExpectedListDashboarVersions []*dashver.DashboardVersionDTO + ExpectedContinueToken string counter int ExpectedError error } @@ -30,6 +31,9 @@ func (f *FakeDashboardVersionService) DeleteExpired(ctx context.Context, cmd *da return f.ExpectedError } -func (f *FakeDashboardVersionService) List(ctx context.Context, query *dashver.ListDashboardVersionsQuery) ([]*dashver.DashboardVersionDTO, error) { - return f.ExpectedListDashboarVersions, f.ExpectedError +func (f *FakeDashboardVersionService) List(ctx context.Context, query *dashver.ListDashboardVersionsQuery) (*dashver.DashboardVersionResponse, error) { + return &dashver.DashboardVersionResponse{ + ContinueToken: f.ExpectedContinueToken, + Versions: f.ExpectedListDashboarVersions, + }, f.ExpectedError } diff --git a/pkg/services/dashboardversion/model.go b/pkg/services/dashboardversion/model.go index 435fc794311..c4f585dc366 100644 --- a/pkg/services/dashboardversion/model.go +++ b/pkg/services/dashboardversion/model.go @@ -52,7 +52,7 @@ type GetDashboardVersionQuery struct { DashboardID int64 DashboardUID string OrgID int64 - Version int + Version int64 } type DeleteExpiredVersionsCommand struct { @@ -60,12 +60,19 @@ type DeleteExpiredVersionsCommand struct { } type ListDashboardVersionsQuery struct { - DashboardID int64 - DashboardUID string - OrgID int64 - Limit int - Start int + DashboardID int64 + DashboardUID string + OrgID int64 + Limit int + Start int + ContinueToken string } + +type DashboardVersionResponse struct { + ContinueToken string `json:"continueToken"` + Versions []*DashboardVersionDTO `json:"versions"` +} + type DashboardVersionDTO struct { ID int64 `json:"id"` DashboardID int64 `json:"dashboardId"` @@ -94,3 +101,8 @@ type DashboardVersionMeta struct { Data *simplejson.Json `json:"data"` CreatedBy string `json:"createdBy"` } + +type DashboardVersionResponseMeta struct { + ContinueToken string `json:"continueToken"` + Versions []DashboardVersionMeta `json:"versions"` +} diff --git a/pkg/storage/unified/resource/cdk_backend.go b/pkg/storage/unified/resource/cdk_backend.go index ded2d34d7aa..eefba09d78f 100644 --- a/pkg/storage/unified/resource/cdk_backend.go +++ b/pkg/storage/unified/resource/cdk_backend.go @@ -321,6 +321,11 @@ func (c *cdkListIterator) ContinueToken() string { return fmt.Sprintf("index:%d/key:%s", c.index, c.currentKey) } +// ContinueTokenWithCurrentRV implements ListIterator. +func (c *cdkListIterator) ContinueTokenWithCurrentRV() string { + return fmt.Sprintf("index:%d/key:%s", c.index, c.currentKey) +} + // Name implements ListIterator. func (c *cdkListIterator) Name() string { return c.currentKey // TODO (parse name from key) diff --git a/pkg/storage/unified/resource/server.go b/pkg/storage/unified/resource/server.go index fab1e585486..58d555bd429 100644 --- a/pkg/storage/unified/resource/server.go +++ b/pkg/storage/unified/resource/server.go @@ -41,6 +41,9 @@ type ListIterator interface { // The token that can be used to start iterating *after* this item ContinueToken() string + // The token that can be used to start iterating *before* this item + ContinueTokenWithCurrentRV() string + // ResourceVersion of the current item ResourceVersion() int64 @@ -756,9 +759,16 @@ func (s *server) List(ctx context.Context, req *ListRequest) (*ListResponse, err rsp.Items = append(rsp.Items, item) if len(rsp.Items) >= int(req.Limit) || pageBytes >= maxPageBytes { t := iter.ContinueToken() + if req.Source == ListRequest_HISTORY { + // history lists in desc order, so the continue token takes the + // final RV in the list, and then will start from there in the next page, + // rather than the lists first RV + t = iter.ContinueTokenWithCurrentRV() + } if iter.Next() { rsp.NextPageToken = t } + break } } diff --git a/pkg/storage/unified/sql/backend.go b/pkg/storage/unified/sql/backend.go index d56cd87b2e1..0401c77a25c 100644 --- a/pkg/storage/unified/sql/backend.go +++ b/pkg/storage/unified/sql/backend.go @@ -519,6 +519,10 @@ func (l *listIter) ContinueToken() string { return ContinueToken{ResourceVersion: l.listRV, StartOffset: l.offset}.String() } +func (l *listIter) ContinueTokenWithCurrentRV() string { + return ContinueToken{ResourceVersion: l.rv, StartOffset: l.offset}.String() +} + func (l *listIter) Error() error { return l.err } diff --git a/pkg/storage/unified/sql/data/resource_history_get.sql b/pkg/storage/unified/sql/data/resource_history_get.sql index cbb2786e534..5ab25eb2e0c 100644 --- a/pkg/storage/unified/sql/data/resource_history_get.sql +++ b/pkg/storage/unified/sql/data/resource_history_get.sql @@ -16,6 +16,6 @@ WHERE 1 = 1 AND {{ .Ident "action" }} = 3 {{ end }} {{ if (gt .StartRV 0) }} - AND {{ .Ident "resource_version" }} > {{ .Arg .StartRV }} + AND {{ .Ident "resource_version" }} < {{ .Arg .StartRV }} {{ end }} ORDER BY resource_version DESC diff --git a/pkg/storage/unified/sql/test/integration_test.go b/pkg/storage/unified/sql/test/integration_test.go index 5883610bd54..cc5cacc524b 100644 --- a/pkg/storage/unified/sql/test/integration_test.go +++ b/pkg/storage/unified/sql/test/integration_test.go @@ -351,6 +351,83 @@ func TestIntegrationBackendList(t *testing.T) { require.Equal(t, rv8, continueToken.ResourceVersion) require.Equal(t, int64(4), continueToken.StartOffset) }) + + // add 5 events for item1 - should be saved to history + rvHistory1, err := writeEvent(ctx, backend, "item1", resource.WatchEvent_MODIFIED) + require.NoError(t, err) + require.Greater(t, rvHistory1, rv1) + rvHistory2, err := writeEvent(ctx, backend, "item1", resource.WatchEvent_MODIFIED) + require.NoError(t, err) + require.Greater(t, rvHistory2, rvHistory1) + rvHistory3, err := writeEvent(ctx, backend, "item1", resource.WatchEvent_MODIFIED) + require.NoError(t, err) + require.Greater(t, rvHistory3, rvHistory2) + rvHistory4, err := writeEvent(ctx, backend, "item1", resource.WatchEvent_MODIFIED) + require.NoError(t, err) + require.Greater(t, rvHistory4, rvHistory3) + rvHistory5, err := writeEvent(ctx, backend, "item1", resource.WatchEvent_MODIFIED) + require.NoError(t, err) + require.Greater(t, rvHistory5, rvHistory4) + + t.Run("fetch first history page at revision with limit", func(t *testing.T) { + res, err := server.List(ctx, &resource.ListRequest{ + Limit: 3, + Source: resource.ListRequest_HISTORY, + Options: &resource.ListOptions{ + Key: &resource.ResourceKey{ + Namespace: "namespace", + Group: "group", + Resource: "resource", + Name: "item1", + }, + }, + }) + require.NoError(t, err) + require.NoError(t, err) + require.Nil(t, res.Error) + require.Len(t, res.Items, 3) + t.Log(res.Items) + // should be in desc order, so the newest RVs are returned first + require.Equal(t, "item1 MODIFIED", string(res.Items[0].Value)) + require.Equal(t, rvHistory5, res.Items[0].ResourceVersion) + require.Equal(t, "item1 MODIFIED", string(res.Items[1].Value)) + require.Equal(t, rvHistory4, res.Items[1].ResourceVersion) + require.Equal(t, "item1 MODIFIED", string(res.Items[2].Value)) + require.Equal(t, rvHistory3, res.Items[2].ResourceVersion) + + continueToken, err := sql.GetContinueToken(res.NextPageToken) + require.NoError(t, err) + // should return the furthest back RV as the next page token + require.Equal(t, rvHistory3, continueToken.ResourceVersion) + }) + + t.Run("fetch second page of history at revision", func(t *testing.T) { + continueToken := &sql.ContinueToken{ + ResourceVersion: rvHistory3, + StartOffset: 2, + } + res, err := server.List(ctx, &resource.ListRequest{ + NextPageToken: continueToken.String(), + Limit: 2, + Source: resource.ListRequest_HISTORY, + Options: &resource.ListOptions{ + Key: &resource.ResourceKey{ + Namespace: "namespace", + Group: "group", + Resource: "resource", + Name: "item1", + }, + }, + }) + require.NoError(t, err) + require.Nil(t, res.Error) + require.Len(t, res.Items, 2) + t.Log(res.Items) + require.Equal(t, "item1 MODIFIED", string(res.Items[0].Value)) + require.Equal(t, rvHistory2, res.Items[0].ResourceVersion) + require.Equal(t, "item1 MODIFIED", string(res.Items[1].Value)) + require.Equal(t, rvHistory1, res.Items[1].ResourceVersion) + }) } func TestIntegrationBlobSupport(t *testing.T) { diff --git a/pkg/storage/unified/sql/testdata/mysql--resource_history_get-read trash second page.sql b/pkg/storage/unified/sql/testdata/mysql--resource_history_get-read trash second page.sql index 16359886766..cd026c1dfef 100755 --- a/pkg/storage/unified/sql/testdata/mysql--resource_history_get-read trash second page.sql +++ b/pkg/storage/unified/sql/testdata/mysql--resource_history_get-read trash second page.sql @@ -10,5 +10,5 @@ WHERE 1 = 1 AND `group` = 'gg' AND `resource` = 'rr' AND `action` = 3 - AND `resource_version` > 123456 + AND `resource_version` < 123456 ORDER BY resource_version DESC diff --git a/pkg/storage/unified/sql/testdata/postgres--resource_history_get-read trash second page.sql b/pkg/storage/unified/sql/testdata/postgres--resource_history_get-read trash second page.sql index 0bf9c65a25a..b5df5de6315 100755 --- a/pkg/storage/unified/sql/testdata/postgres--resource_history_get-read trash second page.sql +++ b/pkg/storage/unified/sql/testdata/postgres--resource_history_get-read trash second page.sql @@ -10,5 +10,5 @@ WHERE 1 = 1 AND "group" = 'gg' AND "resource" = 'rr' AND "action" = 3 - AND "resource_version" > 123456 + AND "resource_version" < 123456 ORDER BY resource_version DESC diff --git a/pkg/storage/unified/sql/testdata/sqlite--resource_history_get-read trash second page.sql b/pkg/storage/unified/sql/testdata/sqlite--resource_history_get-read trash second page.sql index 0bf9c65a25a..b5df5de6315 100755 --- a/pkg/storage/unified/sql/testdata/sqlite--resource_history_get-read trash second page.sql +++ b/pkg/storage/unified/sql/testdata/sqlite--resource_history_get-read trash second page.sql @@ -10,5 +10,5 @@ WHERE 1 = 1 AND "group" = 'gg' AND "resource" = 'rr' AND "action" = 3 - AND "resource_version" > 123456 + AND "resource_version" < 123456 ORDER BY resource_version DESC diff --git a/public/app/features/dashboard-scene/scene/DashboardScene.tsx b/public/app/features/dashboard-scene/scene/DashboardScene.tsx index e7f9a6e7f07..3bef71caf21 100644 --- a/public/app/features/dashboard-scene/scene/DashboardScene.tsx +++ b/public/app/features/dashboard-scene/scene/DashboardScene.tsx @@ -374,7 +374,13 @@ export class DashboardScene extends SceneObjectBase { } public onRestore = async (version: DecoratedRevisionModel): Promise => { - const versionRsp = await historySrv.restoreDashboard(version.uid, version.version); + let versionRsp; + if (config.featureToggles.kubernetesCliDashboards) { + // the id here is the resource version in k8s, use this instead to get the specific version + versionRsp = await historySrv.restoreDashboard(version.uid, version.id); + } else { + versionRsp = await historySrv.restoreDashboard(version.uid, version.version); + } if (!Number.isInteger(versionRsp.version)) { return false; diff --git a/public/app/features/dashboard-scene/settings/VersionsEditView.test.tsx b/public/app/features/dashboard-scene/settings/VersionsEditView.test.tsx index 6d2b37219f6..e6449d0d3f6 100644 --- a/public/app/features/dashboard-scene/settings/VersionsEditView.test.tsx +++ b/public/app/features/dashboard-scene/settings/VersionsEditView.test.tsx @@ -112,44 +112,47 @@ describe('VersionsEditView', () => { }); function getVersions() { - return [ - { - id: 4, - dashboardId: 1, - dashboardUID: '_U4zObQMz', - parentVersion: 3, - restoredFrom: 0, - version: 4, - created: '2017-02-22T17:43:01-08:00', - createdBy: 'admin', - message: '', - checked: false, - }, - { - id: 3, - dashboardId: 1, - dashboardUID: '_U4zObQMz', - parentVersion: 1, - restoredFrom: 1, - version: 3, - created: '2017-02-22T17:43:01-08:00', - createdBy: 'admin', - message: '', - checked: false, - }, - { - id: 2, - dashboardId: 1, - dashboardUID: '_U4zObQMz', - parentVersion: 1, - restoredFrom: 1, - version: 2, - created: '2017-02-23T17:43:01-08:00', - createdBy: 'admin', - message: '', - checked: false, - }, - ]; + return { + continueToken: '', + versions: [ + { + id: 4, + dashboardId: 1, + dashboardUID: '_U4zObQMz', + parentVersion: 3, + restoredFrom: 0, + version: 4, + created: '2017-02-22T17:43:01-08:00', + createdBy: 'admin', + message: '', + checked: false, + }, + { + id: 3, + dashboardId: 1, + dashboardUID: '_U4zObQMz', + parentVersion: 1, + restoredFrom: 1, + version: 3, + created: '2017-02-22T17:43:01-08:00', + createdBy: 'admin', + message: '', + checked: false, + }, + { + id: 2, + dashboardId: 1, + dashboardUID: '_U4zObQMz', + parentVersion: 1, + restoredFrom: 1, + version: 2, + created: '2017-02-23T17:43:01-08:00', + createdBy: 'admin', + message: '', + checked: false, + }, + ], + }; } async function buildTestScene() { diff --git a/public/app/features/dashboard-scene/settings/VersionsEditView.tsx b/public/app/features/dashboard-scene/settings/VersionsEditView.tsx index c2acbb9b7af..17256382d0b 100644 --- a/public/app/features/dashboard-scene/settings/VersionsEditView.tsx +++ b/public/app/features/dashboard-scene/settings/VersionsEditView.tsx @@ -1,6 +1,7 @@ import * as React from 'react'; import { PageLayoutType, dateTimeFormat, dateTimeFormatTimeAgo } from '@grafana/data'; +import { config } from '@grafana/runtime'; import { SceneComponentProps, SceneObjectBase, sceneGraph } from '@grafana/scenes'; import { Spinner, Stack } from '@grafana/ui'; import { Page } from 'app/core/components/Page/Page'; @@ -41,6 +42,7 @@ export class VersionsEditView extends SceneObjectBase imp public static Component = VersionsEditorSettingsListView; private _limit: number = VERSIONS_FETCH_LIMIT; private _start = 0; + private _continueToken = ''; constructor(state: VersionsEditViewState) { super({ @@ -102,14 +104,20 @@ export class VersionsEditView extends SceneObjectBase imp this.setState({ isAppending: append }); + const requestOptions = this._continueToken + ? { limit: this._limit, start: this._start, continueToken: this._continueToken } + : { limit: this._limit, start: this._start }; + historySrv - .getHistoryList(uid, { limit: this._limit, start: this._start }) + .getHistoryList(uid, requestOptions) .then((result) => { this.setState({ isLoading: false, - versions: [...(this.state.versions ?? []), ...this.decorateVersions(result)], + versions: [...(this.state.versions ?? []), ...this.decorateVersions(result.versions)], }); this._start += this._limit; + // Update the continueToken for the next request, if available + this._continueToken = result.continueToken ?? ''; }) .catch((err) => console.log(err)) .finally(() => this.setState({ isAppending: false })); @@ -127,9 +135,15 @@ export class VersionsEditView extends SceneObjectBase imp if (!this._dashboard.state.uid) { return; } - - const lhs = await historySrv.getDashboardVersion(this._dashboard.state.uid, baseInfo.version); - const rhs = await historySrv.getDashboardVersion(this._dashboard.state.uid, newInfo.version); + let lhs, rhs; + if (config.featureToggles.kubernetesCliDashboards) { + // the id here is the resource version in k8s, use this instead to get the specific version + lhs = await historySrv.getDashboardVersion(this._dashboard.state.uid, baseInfo.id); + rhs = await historySrv.getDashboardVersion(this._dashboard.state.uid, newInfo.id); + } else { + lhs = await historySrv.getDashboardVersion(this._dashboard.state.uid, baseInfo.version); + rhs = await historySrv.getDashboardVersion(this._dashboard.state.uid, newInfo.version); + } this.setState({ baseInfo, @@ -145,6 +159,7 @@ export class VersionsEditView extends SceneObjectBase imp }; public reset = () => { + this._continueToken = ''; this.setState({ baseInfo: undefined, diffData: { diff --git a/public/app/features/dashboard-scene/settings/version-history/HistorySrv.test.ts b/public/app/features/dashboard-scene/settings/version-history/HistorySrv.test.ts index c2caee31a0e..84a737d5a1d 100644 --- a/public/app/features/dashboard-scene/settings/version-history/HistorySrv.test.ts +++ b/public/app/features/dashboard-scene/settings/version-history/HistorySrv.test.ts @@ -62,11 +62,11 @@ describe('historySrv', () => { describe('getDashboardVersion', () => { it('should return a version object for the given dashboard id and version', () => { - getMock.mockImplementation(() => Promise.resolve(versionsResponse[0])); + getMock.mockImplementation(() => Promise.resolve(versionsResponse.versions[0])); historySrv = new HistorySrv(); return historySrv.getDashboardVersion(dash.uid, 4).then((version) => { - expect(version).toEqual(versionsResponse[0]); + expect(version).toEqual(versionsResponse.versions[0]); }); }); diff --git a/public/app/features/dashboard-scene/settings/version-history/HistorySrv.ts b/public/app/features/dashboard-scene/settings/version-history/HistorySrv.ts index 61312b8ad45..95cd468ec63 100644 --- a/public/app/features/dashboard-scene/settings/version-history/HistorySrv.ts +++ b/public/app/features/dashboard-scene/settings/version-history/HistorySrv.ts @@ -4,6 +4,7 @@ import { Dashboard } from '@grafana/schema'; export interface HistoryListOpts { limit: number; start: number; + continueToken?: string; } export interface RevisionsModel { diff --git a/public/app/features/dashboard-scene/settings/version-history/__mocks__/dashboardHistoryMocks.ts b/public/app/features/dashboard-scene/settings/version-history/__mocks__/dashboardHistoryMocks.ts index 0de97c47a95..3fbefb31c92 100644 --- a/public/app/features/dashboard-scene/settings/version-history/__mocks__/dashboardHistoryMocks.ts +++ b/public/app/features/dashboard-scene/settings/version-history/__mocks__/dashboardHistoryMocks.ts @@ -1,51 +1,54 @@ export function versions() { - return [ - { - id: 4, - dashboardId: 1, - dashboardUID: '_U4zObQMz', - parentVersion: 3, - restoredFrom: 0, - version: 4, - created: '2017-02-22T17:43:01-08:00', - createdBy: 'admin', - message: '', - }, - { - id: 3, - dashboardId: 1, - dashboardUID: '_U4zObQMz', - parentVersion: 1, - restoredFrom: 1, - version: 3, - created: '2017-02-22T17:43:01-08:00', - createdBy: 'admin', - message: '', - }, - { - id: 2, - dashboardId: 1, - dashboardUID: '_U4zObQMz', - parentVersion: 0, - restoredFrom: -1, - version: 2, - created: '2017-02-22T17:29:52-08:00', - createdBy: 'admin', - message: '', - }, - { - id: 1, - dashboardId: 1, - dashboardUID: '_U4zObQMz', - parentVersion: 0, - restoredFrom: -1, - slug: 'history-dashboard', - version: 1, - created: '2017-02-22T17:06:37-08:00', - createdBy: 'admin', - message: '', - }, - ]; + return { + continueToken: '', + versions: [ + { + id: 4, + dashboardId: 1, + dashboardUID: '_U4zObQMz', + parentVersion: 3, + restoredFrom: 0, + version: 4, + created: '2017-02-22T17:43:01-08:00', + createdBy: 'admin', + message: '', + }, + { + id: 3, + dashboardId: 1, + dashboardUID: '_U4zObQMz', + parentVersion: 1, + restoredFrom: 1, + version: 3, + created: '2017-02-22T17:43:01-08:00', + createdBy: 'admin', + message: '', + }, + { + id: 2, + dashboardId: 1, + dashboardUID: '_U4zObQMz', + parentVersion: 0, + restoredFrom: -1, + version: 2, + created: '2017-02-22T17:29:52-08:00', + createdBy: 'admin', + message: '', + }, + { + id: 1, + dashboardId: 1, + dashboardUID: '_U4zObQMz', + parentVersion: 0, + restoredFrom: -1, + slug: 'history-dashboard', + version: 1, + created: '2017-02-22T17:06:37-08:00', + createdBy: 'admin', + message: '', + }, + ], + }; } export function restore(version: number, restoredFrom?: number) { diff --git a/public/app/features/dashboard/components/DashboardSettings/VersionsSettings.test.tsx b/public/app/features/dashboard/components/DashboardSettings/VersionsSettings.test.tsx index 4b0b91ef6e6..16f8d84f237 100644 --- a/public/app/features/dashboard/components/DashboardSettings/VersionsSettings.test.tsx +++ b/public/app/features/dashboard/components/DashboardSettings/VersionsSettings.test.tsx @@ -66,7 +66,7 @@ describe('VersionSettings', () => { await waitFor(() => expect(screen.getByRole('table')).toBeInTheDocument()); const tableBodyRows = within(screen.getAllByRole('rowgroup')[1]).getAllByRole('row'); - expect(tableBodyRows.length).toBe(versions.length); + expect(tableBodyRows.length).toBe(versions.versions.length); const firstRow = within(screen.getAllByRole('rowgroup')[1]).getAllByRole('row')[0]; @@ -76,7 +76,11 @@ describe('VersionSettings', () => { test('does not render buttons if versions === 1', async () => { // @ts-ignore - historySrv.getHistoryList.mockResolvedValue(versions.slice(0, 1)); + historySrv.getHistoryList.mockResolvedValue({ + continueToken: versions.continueToken, + versions: versions.versions.slice(0, 1), + }); + setup(); expect(screen.queryByRole('button', { name: /show more versions/i })).not.toBeInTheDocument(); @@ -90,7 +94,11 @@ describe('VersionSettings', () => { test('does not render show more button if versions < VERSIONS_FETCH_LIMIT', async () => { // @ts-ignore - historySrv.getHistoryList.mockResolvedValue(versions.slice(0, VERSIONS_FETCH_LIMIT - 5)); + historySrv.getHistoryList.mockResolvedValue({ + continueToken: versions.continueToken, + versions: versions.versions.slice(0, VERSIONS_FETCH_LIMIT - 5), + }); + setup(); expect(screen.queryByRole('button', { name: /show more versions/i })).not.toBeInTheDocument(); @@ -104,7 +112,11 @@ describe('VersionSettings', () => { test('renders buttons if versions >= VERSIONS_FETCH_LIMIT', async () => { // @ts-ignore - historySrv.getHistoryList.mockResolvedValue(versions.slice(0, VERSIONS_FETCH_LIMIT)); + historySrv.getHistoryList.mockResolvedValue({ + continueToken: versions.continueToken, + versions: versions.versions.slice(0, VERSIONS_FETCH_LIMIT), + }); + setup(); expect(screen.queryByRole('button', { name: /show more versions/i })).not.toBeInTheDocument(); @@ -124,9 +136,17 @@ describe('VersionSettings', () => { test('clicking show more appends results to the table', async () => { historySrv.getHistoryList // @ts-ignore - .mockImplementationOnce(() => Promise.resolve(versions.slice(0, VERSIONS_FETCH_LIMIT))) - .mockImplementationOnce( - () => new Promise((resolve) => setTimeout(() => resolve(versions.slice(VERSIONS_FETCH_LIMIT)), 1000)) + .mockImplementationOnce(() => + Promise.resolve({ + continueToken: versions.continueToken, + versions: versions.versions.slice(0, VERSIONS_FETCH_LIMIT), + }) + ) + .mockImplementationOnce(() => + Promise.resolve({ + continueToken: versions.continueToken, + versions: versions.versions.slice(VERSIONS_FETCH_LIMIT), + }) ); setup(); @@ -146,13 +166,16 @@ describe('VersionSettings', () => { await waitFor(() => { expect(screen.queryByText(/Fetching more entries/i)).not.toBeInTheDocument(); - expect(within(screen.getAllByRole('rowgroup')[1]).getAllByRole('row').length).toBe(versions.length); + expect(within(screen.getAllByRole('rowgroup')[1]).getAllByRole('row').length).toBe(versions.versions.length); }); }); test('selecting two versions and clicking compare button should render compare view', async () => { // @ts-ignore - historySrv.getHistoryList.mockResolvedValue(versions.slice(0, VERSIONS_FETCH_LIMIT)); + historySrv.getHistoryList.mockResolvedValue({ + continueToken: versions.continueToken, + versions: versions.versions.slice(0, VERSIONS_FETCH_LIMIT), + }); historySrv.getDashboardVersion // @ts-ignore .mockImplementationOnce(() => Promise.resolve(diffs.lhs)) diff --git a/public/app/features/dashboard/components/DashboardSettings/VersionsSettings.tsx b/public/app/features/dashboard/components/DashboardSettings/VersionsSettings.tsx index c22f31c9437..30ccbe39708 100644 --- a/public/app/features/dashboard/components/DashboardSettings/VersionsSettings.tsx +++ b/public/app/features/dashboard/components/DashboardSettings/VersionsSettings.tsx @@ -1,6 +1,7 @@ import { PureComponent } from 'react'; import * as React from 'react'; +import { config } from '@grafana/runtime'; import { Spinner, HorizontalGroup } from '@grafana/ui'; import { Page } from 'app/core/components/Page/Page'; import { @@ -38,11 +39,13 @@ export const VERSIONS_FETCH_LIMIT = 10; export class VersionsSettings extends PureComponent { limit: number; start: number; + continueToken: string; constructor(props: Props) { super(props); this.limit = VERSIONS_FETCH_LIMIT; this.start = 0; + this.continueToken = ''; this.state = { isAppending: true, isLoading: true, @@ -62,14 +65,20 @@ export class VersionsSettings extends PureComponent { getVersions = (append = false) => { this.setState({ isAppending: append }); + const requestOptions = this.continueToken + ? { limit: this.limit, start: this.start, continueToken: this.continueToken } + : { limit: this.limit, start: this.start }; + historySrv - .getHistoryList(this.props.dashboard.uid, { limit: this.limit, start: this.start }) + .getHistoryList(this.props.dashboard.uid, requestOptions) .then((res) => { this.setState({ isLoading: false, - versions: [...this.state.versions, ...this.decorateVersions(res)], + versions: [...(this.state.versions ?? []), ...this.decorateVersions(res.versions)], }); this.start += this.limit; + // Update the continueToken for the next request, if available + this.continueToken = res.continueToken ?? ''; }) .catch((err) => console.log(err)) .finally(() => this.setState({ isAppending: false })); @@ -84,8 +93,15 @@ export class VersionsSettings extends PureComponent { isLoading: true, }); - const lhs = await historySrv.getDashboardVersion(this.props.dashboard.uid, baseInfo.version); - const rhs = await historySrv.getDashboardVersion(this.props.dashboard.uid, newInfo.version); + let lhs, rhs; + if (config.featureToggles.kubernetesCliDashboards) { + // the id here is the resource version in k8s, use this instead to get the specific version + lhs = await historySrv.getDashboardVersion(this.props.dashboard.uid, baseInfo.id); + rhs = await historySrv.getDashboardVersion(this.props.dashboard.uid, newInfo.id); + } else { + lhs = await historySrv.getDashboardVersion(this.props.dashboard.uid, baseInfo.version); + rhs = await historySrv.getDashboardVersion(this.props.dashboard.uid, newInfo.version); + } this.setState({ baseInfo, @@ -121,6 +137,7 @@ export class VersionsSettings extends PureComponent { }; reset = () => { + this.continueToken = ''; this.setState({ baseInfo: undefined, diffData: { diff --git a/public/app/features/dashboard/components/DashboardSettings/__mocks__/versions.ts b/public/app/features/dashboard/components/DashboardSettings/__mocks__/versions.ts index 62f3cf4d876..3b3dc5629d8 100644 --- a/public/app/features/dashboard/components/DashboardSettings/__mocks__/versions.ts +++ b/public/app/features/dashboard/components/DashboardSettings/__mocks__/versions.ts @@ -1,126 +1,129 @@ -export const versions = [ - { - id: 249, - dashboardId: 74, - dashboardUID: '_U4zObQMz', - parentVersion: 10, - restoredFrom: 0, - version: 11, - created: '2021-01-15T14:44:44+01:00', - createdBy: 'admin', - message: 'testing changes...', - }, - { - id: 247, - dashboardId: 74, - dashboardUID: '_U4zObQMz', - parentVersion: 9, - restoredFrom: 0, - version: 10, - created: '2021-01-15T10:19:17+01:00', - createdBy: 'admin', - message: '', - }, - { - id: 246, - dashboardId: 74, - dashboardUID: '_U4zObQMz', - parentVersion: 8, - restoredFrom: 0, - version: 9, - created: '2021-01-15T10:18:12+01:00', - createdBy: 'admin', - message: '', - }, - { - id: 245, - dashboardId: 74, - dashboardUID: '_U4zObQMz', - parentVersion: 7, - restoredFrom: 0, - version: 8, - created: '2021-01-15T10:11:16+01:00', - createdBy: 'admin', - message: '', - }, - { - id: 239, - dashboardId: 74, - dashboardUID: '_U4zObQMz', - parentVersion: 6, - restoredFrom: 0, - version: 7, - created: '2021-01-14T15:14:25+01:00', - createdBy: 'admin', - message: '', - }, - { - id: 237, - dashboardId: 74, - dashboardUID: '_U4zObQMz', - parentVersion: 5, - restoredFrom: 0, - version: 6, - created: '2021-01-14T14:55:29+01:00', - createdBy: 'admin', - message: '', - }, - { - id: 236, - dashboardId: 74, - dashboardUID: '_U4zObQMz', - parentVersion: 4, - restoredFrom: 0, - version: 5, - created: '2021-01-14T14:28:01+01:00', - createdBy: 'admin', - message: '', - }, - { - id: 218, - dashboardId: 74, - dashboardUID: '_U4zObQMz', - parentVersion: 3, - restoredFrom: 0, - version: 4, - created: '2021-01-08T10:45:33+01:00', - createdBy: 'admin', - message: '', - }, - { - id: 217, - dashboardId: 74, - dashboardUID: '_U4zObQMz', - parentVersion: 2, - restoredFrom: 0, - version: 3, - created: '2021-01-05T15:41:33+01:00', - createdBy: 'admin', - message: '', - }, - { - id: 216, - dashboardId: 74, - dashboardUID: '_U4zObQMz', - parentVersion: 1, - restoredFrom: 0, - version: 2, - created: '2021-01-05T15:01:50+01:00', - createdBy: 'admin', - message: '', - }, - { - id: 215, - dashboardId: 74, - dashboardUID: '_U4zObQMz', - parentVersion: 1, - restoredFrom: 0, - version: 1, - created: '2021-01-05T14:59:15+01:00', - createdBy: 'admin', - message: '', - }, -]; +export const versions = { + continueToken: '', + versions: [ + { + id: 249, + dashboardId: 74, + dashboardUID: '_U4zObQMz', + parentVersion: 10, + restoredFrom: 0, + version: 11, + created: '2021-01-15T14:44:44+01:00', + createdBy: 'admin', + message: 'testing changes...', + }, + { + id: 247, + dashboardId: 74, + dashboardUID: '_U4zObQMz', + parentVersion: 9, + restoredFrom: 0, + version: 10, + created: '2021-01-15T10:19:17+01:00', + createdBy: 'admin', + message: '', + }, + { + id: 246, + dashboardId: 74, + dashboardUID: '_U4zObQMz', + parentVersion: 8, + restoredFrom: 0, + version: 9, + created: '2021-01-15T10:18:12+01:00', + createdBy: 'admin', + message: '', + }, + { + id: 245, + dashboardId: 74, + dashboardUID: '_U4zObQMz', + parentVersion: 7, + restoredFrom: 0, + version: 8, + created: '2021-01-15T10:11:16+01:00', + createdBy: 'admin', + message: '', + }, + { + id: 239, + dashboardId: 74, + dashboardUID: '_U4zObQMz', + parentVersion: 6, + restoredFrom: 0, + version: 7, + created: '2021-01-14T15:14:25+01:00', + createdBy: 'admin', + message: '', + }, + { + id: 237, + dashboardId: 74, + dashboardUID: '_U4zObQMz', + parentVersion: 5, + restoredFrom: 0, + version: 6, + created: '2021-01-14T14:55:29+01:00', + createdBy: 'admin', + message: '', + }, + { + id: 236, + dashboardId: 74, + dashboardUID: '_U4zObQMz', + parentVersion: 4, + restoredFrom: 0, + version: 5, + created: '2021-01-14T14:28:01+01:00', + createdBy: 'admin', + message: '', + }, + { + id: 218, + dashboardId: 74, + dashboardUID: '_U4zObQMz', + parentVersion: 3, + restoredFrom: 0, + version: 4, + created: '2021-01-08T10:45:33+01:00', + createdBy: 'admin', + message: '', + }, + { + id: 217, + dashboardId: 74, + dashboardUID: '_U4zObQMz', + parentVersion: 2, + restoredFrom: 0, + version: 3, + created: '2021-01-05T15:41:33+01:00', + createdBy: 'admin', + message: '', + }, + { + id: 216, + dashboardId: 74, + dashboardUID: '_U4zObQMz', + parentVersion: 1, + restoredFrom: 0, + version: 2, + created: '2021-01-05T15:01:50+01:00', + createdBy: 'admin', + message: '', + }, + { + id: 215, + dashboardId: 74, + dashboardUID: '_U4zObQMz', + parentVersion: 1, + restoredFrom: 0, + version: 1, + created: '2021-01-05T14:59:15+01:00', + createdBy: 'admin', + message: '', + }, + ], +}; export const diffs = { lhs: { diff --git a/public/app/features/dashboard/components/VersionHistory/RevertDashboardModal.tsx b/public/app/features/dashboard/components/VersionHistory/RevertDashboardModal.tsx index 5a4451b4596..955d45388c4 100644 --- a/public/app/features/dashboard/components/VersionHistory/RevertDashboardModal.tsx +++ b/public/app/features/dashboard/components/VersionHistory/RevertDashboardModal.tsx @@ -5,12 +5,13 @@ import { ConfirmModal } from '@grafana/ui'; import { useDashboardRestore } from './useDashboardRestore'; export interface RevertDashboardModalProps { hideModal: () => void; + id: number; version: number; } -export const RevertDashboardModal = ({ hideModal, version }: RevertDashboardModalProps) => { +export const RevertDashboardModal = ({ hideModal, id, version }: RevertDashboardModalProps) => { // TODO: how should state.error be handled? - const { state, onRestoreDashboard } = useDashboardRestore(version); + const { state, onRestoreDashboard } = useDashboardRestore(id, version); useEffect(() => { if (!state.loading && state.value) { diff --git a/public/app/features/dashboard/components/VersionHistory/VersionHistoryComparison.tsx b/public/app/features/dashboard/components/VersionHistory/VersionHistoryComparison.tsx index 2f23cd32e17..72294e80941 100644 --- a/public/app/features/dashboard/components/VersionHistory/VersionHistoryComparison.tsx +++ b/public/app/features/dashboard/components/VersionHistory/VersionHistoryComparison.tsx @@ -43,6 +43,7 @@ export const VersionHistoryComparison = ({ baseInfo, newInfo, diffData, isNewLat icon="history" onClick={() => { showModal(RevertDashboardModal, { + id: baseInfo.id, version: baseInfo.version, hideModal, }); diff --git a/public/app/features/dashboard/components/VersionHistory/VersionHistoryTable.tsx b/public/app/features/dashboard/components/VersionHistory/VersionHistoryTable.tsx index 2cd8514992a..5edd119e490 100644 --- a/public/app/features/dashboard/components/VersionHistory/VersionHistoryTable.tsx +++ b/public/app/features/dashboard/components/VersionHistory/VersionHistoryTable.tsx @@ -60,6 +60,7 @@ export const VersionHistoryTable = ({ versions, canCompare, onCheck }: VersionsT icon="history" onClick={() => { showModal(RevertDashboardModal, { + id: version.id, version: version.version, hideModal, }); diff --git a/public/app/features/dashboard/components/VersionHistory/useDashboardRestore.tsx b/public/app/features/dashboard/components/VersionHistory/useDashboardRestore.tsx index 4ee340a8110..91fd0964aca 100644 --- a/public/app/features/dashboard/components/VersionHistory/useDashboardRestore.tsx +++ b/public/app/features/dashboard/components/VersionHistory/useDashboardRestore.tsx @@ -2,7 +2,7 @@ import { useEffect } from 'react'; import { useAsyncFn } from 'react-use'; import { locationUtil } from '@grafana/data'; -import { locationService } from '@grafana/runtime'; +import { config, locationService } from '@grafana/runtime'; import { useAppNotification } from 'app/core/copy/appNotification'; import { historySrv } from 'app/features/dashboard-scene/settings/version-history'; import { useSelector } from 'app/types'; @@ -16,9 +16,12 @@ const restoreDashboard = async (version: number, dashboard: DashboardModel) => { return await historySrv.restoreDashboard(dashboard.uid, version); }; -export const useDashboardRestore = (version: number) => { +export const useDashboardRestore = (id: number, version: number) => { const dashboard = useSelector((state) => state.dashboard.getModel()); - const [state, onRestoreDashboard] = useAsyncFn(async () => await restoreDashboard(version, dashboard!), []); + const [state, onRestoreDashboard] = useAsyncFn( + async () => await restoreDashboard(config.featureToggles.kubernetesCliDashboards ? id : version, dashboard!), + [] + ); const notifyApp = useAppNotification(); useEffect(() => {