From 12c5b81850cba649ca854dc8755d6f09cc25451f Mon Sep 17 00:00:00 2001 From: Stephanie Hingtgen Date: Thu, 27 Mar 2025 14:57:49 -0600 Subject: [PATCH] K8s: Dashboards: Reduce db calls to get users on list (#103020) --- .../dashboard/legacy/query_dashboards.sql | 2 +- ...--query_dashboards-export_with_history.sql | 2 +- ...uery_dashboards-history_uid_at_version.sql | 2 +- ...--query_dashboards-export_with_history.sql | 2 +- ...uery_dashboards-history_uid_at_version.sql | 2 +- ...--query_dashboards-export_with_history.sql | 2 +- ...uery_dashboards-history_uid_at_version.sql | 2 +- pkg/services/apiserver/client/client.go | 60 ++++++++++---- pkg/services/apiserver/client/client_mock.go | 6 +- pkg/services/apiserver/client/client_test.go | 35 +++++--- .../dashboards/service/dashboard_service.go | 60 +++++++++++--- .../service/dashboard_service_test.go | 32 +++---- .../dashboardversion/dashverimpl/dashver.go | 83 ++++++++++++++----- .../dashverimpl/dashver_test.go | 6 +- 14 files changed, 207 insertions(+), 89 deletions(-) diff --git a/pkg/registry/apis/dashboard/legacy/query_dashboards.sql b/pkg/registry/apis/dashboard/legacy/query_dashboards.sql index 5b045d3ef47..cd58e2d2571 100644 --- a/pkg/registry/apis/dashboard/legacy/query_dashboards.sql +++ b/pkg/registry/apis/dashboard/legacy/query_dashboards.sql @@ -22,7 +22,7 @@ LEFT OUTER JOIN {{ .Ident .VersionTable }} as dashboard_version ON dashboard.id {{ end }} LEFT OUTER JOIN {{ .Ident .ProvisioningTable }} as provisioning ON dashboard.id = provisioning.dashboard_id LEFT OUTER JOIN {{ .Ident .UserTable }} as created_user ON dashboard.created_by = created_user.id -LEFT OUTER JOIN {{ .Ident .UserTable }} as updated_user ON dashboard.updated_by = updated_user.id +LEFT OUTER JOIN {{ .Ident .UserTable }} as updated_user ON {{ if .Query.UseHistoryTable }}dashboard_version.created_by = updated_user.id{{ else }}dashboard.updated_by = updated_user.id{{ end }} WHERE dashboard.is_folder = {{ .Arg .Query.GetFolders }} AND dashboard.org_id = {{ .Arg .Query.OrgID }} {{ if .Query.UseHistoryTable }} diff --git a/pkg/registry/apis/dashboard/legacy/testdata/mysql--query_dashboards-export_with_history.sql b/pkg/registry/apis/dashboard/legacy/testdata/mysql--query_dashboards-export_with_history.sql index 056c5368d28..4a4d51f8763 100755 --- a/pkg/registry/apis/dashboard/legacy/testdata/mysql--query_dashboards-export_with_history.sql +++ b/pkg/registry/apis/dashboard/legacy/testdata/mysql--query_dashboards-export_with_history.sql @@ -14,7 +14,7 @@ FROM `grafana`.`dashboard` as dashboard LEFT OUTER JOIN `grafana`.`dashboard_version` as dashboard_version ON dashboard.id = dashboard_version.dashboard_id LEFT OUTER JOIN `grafana`.`dashboard_provisioning` as provisioning ON dashboard.id = provisioning.dashboard_id LEFT OUTER JOIN `grafana`.`user` as created_user ON dashboard.created_by = created_user.id -LEFT OUTER JOIN `grafana`.`user` as updated_user ON dashboard.updated_by = updated_user.id +LEFT OUTER JOIN `grafana`.`user` as updated_user ON dashboard_version.created_by = updated_user.id WHERE dashboard.is_folder = FALSE AND dashboard.org_id = 1 ORDER BY 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 926007760fc..f5ee5592cae 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 @@ -14,7 +14,7 @@ FROM `grafana`.`dashboard` as dashboard LEFT OUTER JOIN `grafana`.`dashboard_version` as dashboard_version ON dashboard.id = dashboard_version.dashboard_id LEFT OUTER JOIN `grafana`.`dashboard_provisioning` as provisioning ON dashboard.id = provisioning.dashboard_id LEFT OUTER JOIN `grafana`.`user` as created_user ON dashboard.created_by = created_user.id -LEFT OUTER JOIN `grafana`.`user` as updated_user ON dashboard.updated_by = updated_user.id +LEFT OUTER JOIN `grafana`.`user` as updated_user ON dashboard_version.created_by = updated_user.id WHERE dashboard.is_folder = FALSE AND dashboard.org_id = 2 AND dashboard.uid = 'UUU' diff --git a/pkg/registry/apis/dashboard/legacy/testdata/postgres--query_dashboards-export_with_history.sql b/pkg/registry/apis/dashboard/legacy/testdata/postgres--query_dashboards-export_with_history.sql index 46a1f03e48b..a6de5980952 100755 --- a/pkg/registry/apis/dashboard/legacy/testdata/postgres--query_dashboards-export_with_history.sql +++ b/pkg/registry/apis/dashboard/legacy/testdata/postgres--query_dashboards-export_with_history.sql @@ -14,7 +14,7 @@ FROM "grafana"."dashboard" as dashboard LEFT OUTER JOIN "grafana"."dashboard_version" as dashboard_version ON dashboard.id = dashboard_version.dashboard_id LEFT OUTER JOIN "grafana"."dashboard_provisioning" as provisioning ON dashboard.id = provisioning.dashboard_id LEFT OUTER JOIN "grafana"."user" as created_user ON dashboard.created_by = created_user.id -LEFT OUTER JOIN "grafana"."user" as updated_user ON dashboard.updated_by = updated_user.id +LEFT OUTER JOIN "grafana"."user" as updated_user ON dashboard_version.created_by = updated_user.id WHERE dashboard.is_folder = FALSE AND dashboard.org_id = 1 ORDER BY 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 9c55b7df230..a4be51a0737 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 @@ -14,7 +14,7 @@ FROM "grafana"."dashboard" as dashboard LEFT OUTER JOIN "grafana"."dashboard_version" as dashboard_version ON dashboard.id = dashboard_version.dashboard_id LEFT OUTER JOIN "grafana"."dashboard_provisioning" as provisioning ON dashboard.id = provisioning.dashboard_id LEFT OUTER JOIN "grafana"."user" as created_user ON dashboard.created_by = created_user.id -LEFT OUTER JOIN "grafana"."user" as updated_user ON dashboard.updated_by = updated_user.id +LEFT OUTER JOIN "grafana"."user" as updated_user ON dashboard_version.created_by = updated_user.id WHERE dashboard.is_folder = FALSE AND dashboard.org_id = 2 AND dashboard.uid = 'UUU' diff --git a/pkg/registry/apis/dashboard/legacy/testdata/sqlite--query_dashboards-export_with_history.sql b/pkg/registry/apis/dashboard/legacy/testdata/sqlite--query_dashboards-export_with_history.sql index 46a1f03e48b..a6de5980952 100755 --- a/pkg/registry/apis/dashboard/legacy/testdata/sqlite--query_dashboards-export_with_history.sql +++ b/pkg/registry/apis/dashboard/legacy/testdata/sqlite--query_dashboards-export_with_history.sql @@ -14,7 +14,7 @@ FROM "grafana"."dashboard" as dashboard LEFT OUTER JOIN "grafana"."dashboard_version" as dashboard_version ON dashboard.id = dashboard_version.dashboard_id LEFT OUTER JOIN "grafana"."dashboard_provisioning" as provisioning ON dashboard.id = provisioning.dashboard_id LEFT OUTER JOIN "grafana"."user" as created_user ON dashboard.created_by = created_user.id -LEFT OUTER JOIN "grafana"."user" as updated_user ON dashboard.updated_by = updated_user.id +LEFT OUTER JOIN "grafana"."user" as updated_user ON dashboard_version.created_by = updated_user.id WHERE dashboard.is_folder = FALSE AND dashboard.org_id = 1 ORDER BY 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 9c55b7df230..a4be51a0737 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 @@ -14,7 +14,7 @@ FROM "grafana"."dashboard" as dashboard LEFT OUTER JOIN "grafana"."dashboard_version" as dashboard_version ON dashboard.id = dashboard_version.dashboard_id LEFT OUTER JOIN "grafana"."dashboard_provisioning" as provisioning ON dashboard.id = provisioning.dashboard_id LEFT OUTER JOIN "grafana"."user" as created_user ON dashboard.created_by = created_user.id -LEFT OUTER JOIN "grafana"."user" as updated_user ON dashboard.updated_by = updated_user.id +LEFT OUTER JOIN "grafana"."user" as updated_user ON dashboard_version.created_by = updated_user.id WHERE dashboard.is_folder = FALSE AND dashboard.org_id = 2 AND dashboard.uid = 'UUU' diff --git a/pkg/services/apiserver/client/client.go b/pkg/services/apiserver/client/client.go index 2b312f309bd..2d42823693b 100644 --- a/pkg/services/apiserver/client/client.go +++ b/pkg/services/apiserver/client/client.go @@ -2,7 +2,6 @@ package client import ( "context" - "errors" "fmt" "strconv" "strings" @@ -37,7 +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) + GetUsersFromMeta(ctx context.Context, userMeta []string) (map[string]*user.User, error) } var _ K8sHandler = (*k8sHandler)(nil) @@ -203,26 +202,51 @@ 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] +// GetUsersFromMeta takes what meta accessor gives you from `GetCreatedBy` or `GetUpdatedBy` and returns the user(s), with the meta as the key +func (h *k8sHandler) GetUsersFromMeta(ctx context.Context, usersMeta []string) (map[string]*user.User, error) { + uids := []string{} + ids := []int64{} + metaToId := make(map[string]int64) + metaToUid := make(map[string]string) + userMap := make(map[string]*user.User) - 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}) + for _, userMeta := range usersMeta { + parts := strings.Split(userMeta, ":") + if len(parts) < 2 { + return userMap, nil + } + meta := parts[1] + + userId, err := strconv.ParseInt(meta, 10, 64) + if err == nil { + ids = append(ids, userId) + metaToId[userMeta] = userId + } else { + uids = append(uids, meta) + metaToUid[userMeta] = meta + } } - if err != nil && errors.Is(err, user.ErrUserNotFound) { - return &user.User{}, nil + users, err := h.userService.ListByIdOrUID(ctx, uids, ids) + if err != nil { + return userMap, nil } - return u, err + + for _, u := range users { + for meta, id := range metaToId { + if u.ID == id { + userMap[meta] = u + break + } + } + for meta, uid := range metaToUid { + if u.UID == uid { + userMap[meta] = u + break + } + } + } + return userMap, err } func (h *k8sHandler) getClient(ctx context.Context, orgID int64) (dynamic.ResourceInterface, error) { diff --git a/pkg/services/apiserver/client/client_mock.go b/pkg/services/apiserver/client/client_mock.go index 2c84a80e0be..f17c89daf67 100644 --- a/pkg/services/apiserver/client/client_mock.go +++ b/pkg/services/apiserver/client/client_mock.go @@ -82,12 +82,12 @@ 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) +func (m *MockK8sHandler) GetUsersFromMeta(ctx context.Context, usersMeta []string) (map[string]*user.User, error) { + args := m.Called(ctx, usersMeta) if args.Get(0) == nil { return nil, args.Error(1) } - return args.Get(0).(*user.User), args.Error(1) + return args.Get(0).(map[string]*user.User), args.Error(1) } type MockTestRestConfig struct { diff --git a/pkg/services/apiserver/client/client_test.go b/pkg/services/apiserver/client/client_test.go index c97c5f0a31d..ba48d38a5b2 100644 --- a/pkg/services/apiserver/client/client_test.go +++ b/pkg/services/apiserver/client/client_test.go @@ -9,26 +9,41 @@ import ( "github.com/stretchr/testify/require" ) -func TestGetUserFromMeta(t *testing.T) { +func TestGetUsersFromMeta(t *testing.T) { userSvcTest := usertest.NewUserServiceFake() - userSvcTest.ExpectedUser = &user.User{ - ID: 1, - UID: "uid-value", + userSvcTest.ExpectedListUsersByIdOrUid = []*user.User{ + { + ID: 1, + UID: "uid-value", + }, + { + ID: 2, + UID: "uid-value2", + }, } client := &k8sHandler{ userService: userSvcTest, } t.Run("returns user with valid UID", func(t *testing.T) { - result, err := client.GetUserFromMeta(context.Background(), "user:uid-value") + result, err := client.GetUsersFromMeta(context.Background(), []string{"user:uid-value"}) require.NoError(t, err) - require.Equal(t, "uid-value", result.UID) - require.Equal(t, int64(1), result.ID) + require.Equal(t, "uid-value", result["user:uid-value"].UID) + require.Equal(t, int64(1), result["user:uid-value"].ID) }) t.Run("returns user when id is passed in", func(t *testing.T) { - result, err := client.GetUserFromMeta(context.Background(), "user:1") + result, err := client.GetUsersFromMeta(context.Background(), []string{"user:1"}) require.NoError(t, err) - require.Equal(t, "uid-value", result.UID) - require.Equal(t, int64(1), result.ID) + require.Equal(t, "uid-value", result["user:1"].UID) + require.Equal(t, int64(1), result["user:1"].ID) + }) + + t.Run("returns users when id and uid are passed in", func(t *testing.T) { + result, err := client.GetUsersFromMeta(context.Background(), []string{"user:1", "user:uid-value2"}) + require.NoError(t, err) + require.Equal(t, "uid-value", result["user:1"].UID) + require.Equal(t, int64(1), result["user:1"].ID) + require.Equal(t, "uid-value2", result["user:uid-value2"].UID) + require.Equal(t, int64(2), result["user:uid-value2"].ID) }) } diff --git a/pkg/services/dashboards/service/dashboard_service.go b/pkg/services/dashboards/service/dashboard_service.go index 24381908e5a..cba4634650e 100644 --- a/pkg/services/dashboards/service/dashboard_service.go +++ b/pkg/services/dashboards/service/dashboard_service.go @@ -320,8 +320,14 @@ func (dr *DashboardServiceImpl) processDashboardBatch(ctx context.Context, orgID var errs []error itemsProcessed := 0 + // get users ahead of time to do just one db call, rather than 2 per item in the list + users, err := dr.getUsersForList(ctx, items, orgID) + if err != nil { + return 0, append(errs, err) + } + for _, item := range items { - dash, err := dr.UnstructuredToLegacyDashboard(ctx, &item, orgID) + dash, err := dr.unstructuredToLegacyDashboardWithUsers(&item, orgID, users) if err != nil { errs = append(errs, fmt.Errorf("failed to convert dashboard: %w", err)) continue @@ -1924,9 +1930,15 @@ func (dr *DashboardServiceImpl) listDashboardsThroughK8s(ctx context.Context, or return nil, dashboards.ErrDashboardNotFound } + // get users ahead of time to do just one db call, rather than 2 per item in the list + users, err := dr.getUsersForList(ctx, out.Items, orgID) + if err != nil { + return nil, err + } + dashboards := make([]*dashboards.Dashboard, 0) for _, item := range out.Items { - dash, err := dr.UnstructuredToLegacyDashboard(ctx, &item, orgID) + dash, err := dr.unstructuredToLegacyDashboardWithUsers(&item, orgID, users) if err != nil { return nil, err } @@ -2204,7 +2216,38 @@ func (dr *DashboardServiceImpl) searchDashboardsThroughK8s(ctx context.Context, return result, nil } +func (dr *DashboardServiceImpl) getUsersForList(ctx context.Context, items []unstructured.Unstructured, orgID int64) (map[string]*user.User, error) { + userMeta := []string{} + for _, item := range items { + obj, err := utils.MetaAccessor(&item) + if err != nil { + return nil, err + } + if obj.GetCreatedBy() != "" { + userMeta = append(userMeta, obj.GetCreatedBy()) + } + if obj.GetUpdatedBy() != "" { + userMeta = append(userMeta, obj.GetUpdatedBy()) + } + } + + return dr.k8sclient.GetUsersFromMeta(ctx, userMeta) +} + func (dr *DashboardServiceImpl) UnstructuredToLegacyDashboard(ctx context.Context, item *unstructured.Unstructured, orgID int64) (*dashboards.Dashboard, error) { + obj, err := utils.MetaAccessor(item) + if err != nil { + return nil, err + } + + users, err := dr.k8sclient.GetUsersFromMeta(ctx, []string{obj.GetCreatedBy(), obj.GetUpdatedBy()}) + if err != nil { + return nil, err + } + return dr.unstructuredToLegacyDashboardWithUsers(item, orgID, users) +} + +func (dr *DashboardServiceImpl) unstructuredToLegacyDashboardWithUsers(item *unstructured.Unstructured, orgID int64, users map[string]*user.User) (*dashboards.Dashboard, error) { spec, ok := item.Object["spec"].(map[string]any) if !ok { return nil, errors.New("error parsing dashboard from k8s response") @@ -2247,17 +2290,12 @@ func (dr *DashboardServiceImpl) UnstructuredToLegacyDashboard(ctx context.Contex out.PluginID = dashboard.GetPluginIDFromMeta(obj) - creator, err := dr.k8sclient.GetUserFromMeta(ctx, obj.GetCreatedBy()) - if err != nil { - return nil, err + if creator, ok := users[obj.GetCreatedBy()]; ok { + out.CreatedBy = creator.ID } - out.CreatedBy = creator.ID - - updater, err := dr.k8sclient.GetUserFromMeta(ctx, obj.GetUpdatedBy()) - if err != nil { - return nil, err + if updater, ok := users[obj.GetUpdatedBy()]; ok { + out.UpdatedBy = updater.ID } - out.UpdatedBy = updater.ID // any dashboards that have already been synced to unified storage will have the id in the spec // and not as a label. We will need to support this conversion until they have all been updated diff --git a/pkg/services/dashboards/service/dashboard_service_test.go b/pkg/services/dashboards/service/dashboard_service_test.go index 57101bf5be3..cf35a0b1777 100644 --- a/pkg/services/dashboards/service/dashboard_service_test.go +++ b/pkg/services/dashboards/service/dashboard_service_test.go @@ -320,7 +320,7 @@ func TestGetDashboard(t *testing.T) { 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, mock.Anything).Return(&dashboardUnstructured, nil).Once() - k8sCliMock.On("GetUserFromMeta", mock.Anything, mock.Anything).Return(&user.User{}, nil) + k8sCliMock.On("GetUsersFromMeta", mock.Anything, mock.Anything).Return(map[string]*user.User{}, nil) dashboard, err := service.GetDashboard(ctx, query) require.NoError(t, err) @@ -358,7 +358,7 @@ func TestGetDashboard(t *testing.T) { Data: simplejson.NewFromAny(map[string]any{"test": "test", "title": "testing slugify", "uid": "uid", "version": int64(2)}), } 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("GetUsersFromMeta", mock.Anything, mock.Anything).Return(map[string]*user.User{}, nil) k8sCliMock.On("Search", mock.Anything, mock.Anything, mock.Anything).Return(&resource.ResourceSearchResponse{ Results: &resource.ResourceTable{ Columns: []*resource.ResourceTableColumnDefinition{ @@ -457,7 +457,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("GetUsersFromMeta", mock.Anything, mock.Anything).Return(map[string]*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) @@ -509,7 +509,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("GetUsersFromMeta", mock.Anything, mock.Anything).Return(map[string]*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) @@ -1140,7 +1140,7 @@ func TestUnprovisionDashboard(t *testing.T) { // should update it to be without annotations k8sCliMock.On("Update", mock.Anything, dashWithoutAnnotations, 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("GetUsersFromMeta", mock.Anything, mock.Anything).Return(map[string]*user.User{}, nil) k8sCliMock.On("Search", mock.Anything, mock.Anything, mock.Anything).Return(&resource.ResourceSearchResponse{ Results: &resource.ResourceTable{ Columns: []*resource.ResourceTableColumnDefinition{ @@ -1207,7 +1207,7 @@ func TestGetDashboardsByPluginID(t *testing.T) { ctx, k8sCliMock := setupK8sDashboardTests(service) k8sCliMock.On("GetNamespace", mock.Anything, mock.Anything).Return("default") 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("GetUsersFromMeta", mock.Anything, mock.Anything).Return(map[string]*user.User{}, nil) k8sCliMock.On("Search", mock.Anything, mock.Anything, mock.MatchedBy(func(req *resource.ResourceSearchRequest) bool { return ( // gofmt comment helper req.Options.Fields[0].Key == "manager.kind" && req.Options.Fields[0].Values[0] == string(utils.ManagerKindPlugin) && @@ -1361,7 +1361,7 @@ 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(&dashboards.Dashboard{}, nil) - k8sCliMock.On("GetUserFromMeta", mock.Anything, mock.Anything).Return(&user.User{}, nil) + k8sCliMock.On("GetUsersFromMeta", mock.Anything, mock.Anything).Return(map[string]*user.User{}, nil) k8sCliMock.On("Update", mock.Anything, mock.Anything, mock.Anything, mock.Anything).Return(&dashboardUnstructured, nil) k8sCliMock.On("GetNamespace", mock.Anything).Return("default") @@ -1422,7 +1422,7 @@ 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("GetUserFromMeta", mock.Anything, mock.Anything).Return(&user.User{}, nil) + k8sCliMock.On("GetUsersFromMeta", mock.Anything, mock.Anything).Return(map[string]*user.User{}, nil) k8sCliMock.On("GetNamespace", mock.Anything).Return("default") k8sCliMock.On("Update", mock.Anything, mock.Anything, mock.Anything, mock.Anything).Return(&dashboardUnstructured, nil) @@ -1433,7 +1433,7 @@ 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("GetUserFromMeta", mock.Anything, mock.Anything).Return(&user.User{}, nil) + k8sCliMock.On("GetUsersFromMeta", mock.Anything, mock.Anything).Return(map[string]*user.User{}, nil) k8sCliMock.On("GetNamespace", mock.Anything).Return("default") k8sCliMock.On("Update", mock.Anything, mock.Anything, mock.Anything, mock.Anything).Return(&dashboardUnstructured, nil) @@ -1859,7 +1859,7 @@ func TestGetDashboards(t *testing.T) { k8sCliMock.On("GetNamespace", mock.Anything, mock.Anything).Return("default") 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("GetUsersFromMeta", mock.Anything, mock.Anything).Return(map[string]*user.User{}, nil) k8sCliMock.On("Search", mock.Anything, mock.Anything, mock.Anything).Return(&resource.ResourceSearchResponse{ Results: &resource.ResourceTable{ Columns: []*resource.ResourceTableColumnDefinition{ @@ -1977,7 +1977,7 @@ func TestGetDashboardUIDByID(t *testing.T) { func TestUnstructuredToLegacyDashboard(t *testing.T) { k8sCliMock := new(client.MockK8sHandler) - k8sCliMock.On("GetUserFromMeta", mock.Anything, mock.Anything).Return(&user.User{ID: 10, UID: "useruid"}, nil) + k8sCliMock.On("GetUsersFromMeta", mock.Anything, mock.Anything).Return(map[string]*user.User{"user:useruid": &user.User{ID: 10, UID: "useruid"}}, nil) dr := &DashboardServiceImpl{ k8sclient: k8sCliMock, } @@ -2022,7 +2022,7 @@ func TestUnstructuredToLegacyDashboard(t *testing.T) { item := &unstructured.Unstructured{ Object: map[string]interface{}{}, } - _, err := (&DashboardServiceImpl{}).UnstructuredToLegacyDashboard(context.Background(), item, int64(123)) + _, err := dr.UnstructuredToLegacyDashboard(context.Background(), item, int64(123)) assert.Error(t, err) assert.Equal(t, "error parsing dashboard from k8s response", err.Error()) }) @@ -2659,8 +2659,8 @@ func TestK8sDashboardCleanupJob(t *testing.T) { Items: []unstructured.Unstructured{dashboard2}, }, nil).Once() - // Mock GetUserFromMeta calls - k8sCliMock.On("GetUserFromMeta", mock.AnythingOfType("*context.valueCtx"), mock.Anything).Return(&user.User{}, nil).Times(4) + // should be called twice, one for each list call + k8sCliMock.On("GetUsersFromMeta", mock.AnythingOfType("*context.valueCtx"), mock.Anything).Return(map[string]*user.User{}, nil).Times(2) // Mock cleanup fakePublicDashboardService.On("DeleteByDashboardUIDs", mock.Anything, int64(1), []string{"dash1"}).Return(nil).Once() @@ -2737,8 +2737,8 @@ func TestK8sDashboardCleanupJob(t *testing.T) { Items: secondBatch, }, nil).Once() - // Mock GetUserFromMeta calls for each dashboard - k8sCliMock.On("GetUserFromMeta", mock.AnythingOfType("*context.valueCtx"), mock.Anything).Return(&user.User{}, nil).Times(10) + // should be called twice, one for each list call + k8sCliMock.On("GetUsersFromMeta", mock.AnythingOfType("*context.valueCtx"), mock.Anything).Return(map[string]*user.User{}, nil).Times(2) // Mock public dashboard deletion for each dashboard fakePublicDashboardService.On("DeleteByDashboardUIDs", mock.Anything, int64(1), []string{"dash1"}).Return(nil).Once() diff --git a/pkg/services/dashboardversion/dashverimpl/dashver.go b/pkg/services/dashboardversion/dashverimpl/dashver.go index 58c2d4f71df..bb314f9eea8 100644 --- a/pkg/services/dashboardversion/dashverimpl/dashver.go +++ b/pkg/services/dashboardversion/dashverimpl/dashver.go @@ -276,13 +276,9 @@ func (s *Service) listHistoryThroughK8s(ctx context.Context, orgID int64, dashbo 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 + dashboards, err := s.UnstructuredToLegacyDashboardVersionList(ctx, out.Items, orgID) + if err != nil { + return nil, err } return &dashver.DashboardVersionResponse{ @@ -292,6 +288,51 @@ func (s *Service) listHistoryThroughK8s(ctx context.Context, orgID int64, dashbo } func (s *Service) UnstructuredToLegacyDashboardVersion(ctx context.Context, item *unstructured.Unstructured, orgID int64) (*dashver.DashboardVersionDTO, error) { + obj, err := utils.MetaAccessor(item) + if err != nil { + return nil, err + } + users, err := s.k8sclient.GetUsersFromMeta(ctx, []string{obj.GetCreatedBy(), obj.GetUpdatedBy()}) + if err != nil { + return nil, err + } + return s.unstructuredToLegacyDashboardVersionWithUsers(item, users) +} + +func (s *Service) UnstructuredToLegacyDashboardVersionList(ctx context.Context, items []unstructured.Unstructured, orgID int64) ([]*dashver.DashboardVersionDTO, error) { + // get users ahead of time to do just one db call, rather than 2 per item in the list + userMeta := []string{} + for _, item := range items { + obj, err := utils.MetaAccessor(&item) + if err != nil { + return nil, err + } + if obj.GetCreatedBy() != "" { + userMeta = append(userMeta, obj.GetCreatedBy()) + } + if obj.GetUpdatedBy() != "" { + userMeta = append(userMeta, obj.GetUpdatedBy()) + } + } + + users, err := s.k8sclient.GetUsersFromMeta(ctx, userMeta) + if err != nil { + return nil, err + } + + versions := make([]*dashver.DashboardVersionDTO, len(items)) + for i, item := range items { + version, err := s.unstructuredToLegacyDashboardVersionWithUsers(&item, users) + if err != nil { + return nil, err + } + versions[i] = version + } + + return versions, nil +} + +func (s *Service) unstructuredToLegacyDashboardVersionWithUsers(item *unstructured.Unstructured, users map[string]*user.User) (*dashver.DashboardVersionDTO, error) { spec, ok := item.Object["spec"].(map[string]any) if !ok { return nil, errors.New("error parsing dashboard from k8s response") @@ -312,19 +353,21 @@ func (s *Service) UnstructuredToLegacyDashboardVersion(ctx context.Context, item spec["version"] = dashVersion } - createdBy, err := s.k8sclient.GetUserFromMeta(ctx, obj.GetCreatedBy()) - if err != nil { - return nil, err + var createdBy *user.User + if creator, ok := users[obj.GetCreatedBy()]; ok { + createdBy = creator } - // if updated by is set, then this version of the dashboard was "created" // by that user - if obj.GetUpdatedBy() != "" { - updatedBy, err := s.k8sclient.GetUserFromMeta(ctx, obj.GetUpdatedBy()) - if err == nil && updatedBy != nil { - createdBy = updatedBy - } + if updater, ok := users[obj.GetUpdatedBy()]; ok { + createdBy = updater } + + createdByID := int64(0) + if createdBy != nil { + createdByID = createdBy.ID + } + created := obj.GetCreationTimestamp().Time if updated, err := obj.GetUpdatedTimestamp(); err == nil && updated != nil { created = *updated @@ -335,20 +378,18 @@ func (s *Service) UnstructuredToLegacyDashboardVersion(ctx context.Context, item return nil, err } - out := dashver.DashboardVersionDTO{ + return &dashver.DashboardVersionDTO{ ID: dashVersion, DashboardID: obj.GetDeprecatedInternalID(), // nolint:staticcheck DashboardUID: uid, Created: created, - CreatedBy: createdBy.ID, + CreatedBy: createdByID, Message: obj.GetMessage(), RestoredFrom: restoreVer, Version: int(dashVersion), ParentVersion: int(parentVersion), Data: simplejson.NewFromAny(spec), - } - - return &out, nil + }, nil } var restoreMsg = "Restored from version " diff --git a/pkg/services/dashboardversion/dashverimpl/dashver_test.go b/pkg/services/dashboardversion/dashverimpl/dashver_test.go index 4f210ea906f..3511f03913e 100644 --- a/pkg/services/dashboardversion/dashverimpl/dashver_test.go +++ b/pkg/services/dashboardversion/dashverimpl/dashver_test.go @@ -73,7 +73,7 @@ func TestDashboardVersionService(t *testing.T) { obj, err := utils.MetaAccessor(dash) require.NoError(t, err) obj.SetUpdatedTimestamp(&updatedTimestamp) - mockCli.On("GetUserFromMeta", mock.Anything, "user:1").Return(&user.User{ID: 1}, nil) + mockCli.On("GetUsersFromMeta", mock.Anything, []string{"user:1", ""}).Return(map[string]*user.User{"user:1": &user.User{ID: 1}}, nil) mockCli.On("List", mock.Anything, int64(1), mock.Anything).Return(&unstructured.UnstructuredList{ Items: []unstructured.Unstructured{*dash}}, nil).Once() res, err := dashboardVersionService.Get(context.Background(), &dashver.GetDashboardVersionQuery{ @@ -93,7 +93,7 @@ func TestDashboardVersionService(t *testing.T) { Data: simplejson.NewFromAny(map[string]any{"uid": "uid", "version": int64(10), "hello": "world"}), }) - mockCli.On("GetUserFromMeta", mock.Anything, "user:2").Return(&user.User{ID: 2}, nil) + mockCli.On("GetUsersFromMeta", mock.Anything, []string{"user:1", "user:2"}).Return(map[string]*user.User{"user:1": {ID: 1}, "user:2": {ID: 2}}, nil) mockCli.On("List", mock.Anything, int64(1), mock.Anything).Return(&unstructured.UnstructuredList{ Items: []unstructured.Unstructured{{ Object: map[string]any{ @@ -269,7 +269,7 @@ func TestListDashboardVersions(t *testing.T) { Return(&dashboards.DashboardRef{UID: "uid"}, nil) query := dashver.ListDashboardVersionsQuery{DashboardID: 42} - mockCli.On("GetUserFromMeta", mock.Anything, mock.Anything).Return(&user.User{}, nil) + mockCli.On("GetUsersFromMeta", mock.Anything, mock.Anything).Return(map[string]*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{