From 0003efa2853af398bcdf9efaabd5600ab93df26c Mon Sep 17 00:00:00 2001 From: Leonor Oliveira <9090754+leonorfmartins@users.noreply.github.com> Date: Mon, 27 Jan 2025 10:51:41 +0100 Subject: [PATCH] Library Panels: Remove dashboards table dependency when getting all library panels (#99160) * Remove dashboards table dependency when getting all library panels * Filter library elements a user can see using the folder service * Stop using folder name as UID in get all elements tests * Set actual folder name not UID when getting all elements * Stop selecting folder name in the get all elements sql query * Introduce a library elements param selector without where clause * Include empty string as general folder UID when getting all library elements --------- Co-authored-by: suntala --- pkg/services/libraryelements/database.go | 79 +++++++++++++------ .../libraryelements_create_test.go | 6 +- .../libraryelements_patch_test.go | 4 +- .../libraryelements_permissions_test.go | 12 +-- .../libraryelements/libraryelements_test.go | 4 +- pkg/services/libraryelements/model/model.go | 1 + pkg/services/libraryelements/writers.go | 4 +- 7 files changed, 73 insertions(+), 37 deletions(-) diff --git a/pkg/services/libraryelements/database.go b/pkg/services/libraryelements/database.go index f093a8dbe54..6bf532a34d4 100644 --- a/pkg/services/libraryelements/database.go +++ b/pkg/services/libraryelements/database.go @@ -15,7 +15,6 @@ import ( "github.com/grafana/grafana/pkg/kinds/librarypanel" ac "github.com/grafana/grafana/pkg/services/accesscontrol" "github.com/grafana/grafana/pkg/services/dashboards" - "github.com/grafana/grafana/pkg/services/dashboards/dashboardaccess" "github.com/grafana/grafana/pkg/services/featuremgmt" "github.com/grafana/grafana/pkg/services/folder" "github.com/grafana/grafana/pkg/services/libraryelements/model" @@ -277,6 +276,9 @@ func (l *LibraryElementService) deleteLibraryElement(c context.Context, signedIn // getLibraryElements gets a Library Element where param == value func (l *LibraryElementService) getLibraryElements(c context.Context, store db.DB, cfg *setting.Cfg, signedInUser identity.Requester, params []Pair, features featuremgmt.FeatureToggles, cmd model.GetLibraryElementCommand) ([]model.LibraryElementDTO, error) { + if len(params) < 1 { + return nil, fmt.Errorf("expected at least one parameter pair") + } libraryElements := make([]model.LibraryElementWithMeta, 0) recursiveQueriesAreSupported, err := store.RecursiveQueriesAreSupported() @@ -287,18 +289,16 @@ func (l *LibraryElementService) getLibraryElements(c context.Context, store db.D err = store.WithDbSession(c, func(session *db.Session) error { builder := db.NewSqlBuilder(cfg, features, store.GetDialect(), recursiveQueriesAreSupported) builder.Write(selectLibraryElementDTOWithMeta) - builder.Write(", ? as folder_name ", cmd.FolderName) builder.Write(getFromLibraryElementDTOWithMeta(store.GetDialect())) metrics.MFolderIDsServiceCount.WithLabelValues(metrics.LibraryElements).Inc() + + builder.Write(" WHERE ") // nolint:staticcheck writeParamSelectorSQL(&builder, append(params, Pair{"folder_id", cmd.FolderID})...) - builder.Write(" UNION ") - builder.Write(selectLibraryElementDTOWithMeta) - builder.Write(", dashboard.title as folder_name ") - builder.Write(getFromLibraryElementDTOWithMeta(store.GetDialect())) - builder.Write(" INNER JOIN dashboard AS dashboard on le.folder_id = dashboard.id AND le.folder_id <> 0") + + builder.Write(" OR le.folder_id <> 0 AND ") writeParamSelectorSQL(&builder, params...) - builder.Write(` OR dashboard.id=0`) + if err := session.SQL(builder.GetSQLString(), builder.GetParams()...).Find(&libraryElements); err != nil { return err } @@ -385,6 +385,7 @@ func (l *LibraryElementService) getLibraryElementsByName(c context.Context, sign return l.getLibraryElements(c, l.SQLStore, l.Cfg, signedInUser, []Pair{{"org_id", signedInUser.GetOrgID()}, {"name", name}}, l.features, model.GetLibraryElementCommand{ FolderName: dashboards.RootFolderName, + Name: name, }) } @@ -416,7 +417,6 @@ func (l *LibraryElementService) getAllLibraryElements(c context.Context, signedI builder := db.NewSqlBuilder(l.Cfg, l.features, l.SQLStore.GetDialect(), recursiveQueriesAreSupported) if folderFilter.includeGeneralFolder { builder.Write(selectLibraryElementDTOWithMeta) - builder.Write(", 'General' as folder_name ") builder.Write(", '' as folder_uid ") builder.Write(getFromLibraryElementDTOWithMeta(l.SQLStore.GetDialect())) builder.Write(` WHERE le.org_id=? AND le.folder_id=0`, signedInUser.GetOrgID()) @@ -427,11 +427,9 @@ func (l *LibraryElementService) getAllLibraryElements(c context.Context, signedI builder.Write(" UNION ") } builder.Write(selectLibraryElementDTOWithMeta) - builder.Write(", dashboard.title as folder_name ") - builder.Write(", dashboard.uid as folder_uid ") + builder.Write(", le.folder_uid as folder_uid ") builder.Write(getFromLibraryElementDTOWithMeta(l.SQLStore.GetDialect())) - builder.Write(" INNER JOIN dashboard AS dashboard on le.folder_id = dashboard.id AND le.folder_id<>0") - builder.Write(` WHERE le.org_id=?`, signedInUser.GetOrgID()) + builder.Write(` WHERE le.org_id=? AND le.folder_id<>0`, signedInUser.GetOrgID()) writeKindSQL(query, &builder) writeSearchStringSQL(query, l.SQLStore, &builder) writeExcludeSQL(query, &builder) @@ -439,9 +437,6 @@ func (l *LibraryElementService) getAllLibraryElements(c context.Context, signedI if err := folderFilter.writeFolderFilterSQL(false, &builder); err != nil { return err } - if !signedInUser.HasRole(org.RoleAdmin) { - builder.WriteDashboardPermissionFilter(signedInUser, dashboardaccess.PERMISSION_VIEW, "") - } if query.SortDirection == search.SortAlphaDesc.Name { builder.Write(" ORDER BY 1 DESC") } else { @@ -454,7 +449,28 @@ func (l *LibraryElementService) getAllLibraryElements(c context.Context, signedI metrics.MFolderIDsServiceCount.WithLabelValues(metrics.LibraryElements).Inc() retDTOs := make([]model.LibraryElementDTO, 0) + // getting all folders a user can see + fs, err := l.folderService.GetFolders(c, folder.GetFoldersQuery{OrgID: signedInUser.GetOrgID(), SignedInUser: signedInUser}) + if err != nil { + return err + } + // Every signed in user can see the general folder. The general folder might have "general" or the empty string as its UID. + var folderUIDS = []string{"general", ""} + folderMap := map[string]string{} + for _, f := range fs { + folderUIDS = append(folderUIDS, f.UID) + folderMap[f.UID] = f.Title + } + // if the user is not an admin, we need to filter out elements that are not in folders the user can see for _, element := range elements { + if !signedInUser.HasRole(org.RoleAdmin) { + if !contains(folderUIDS, element.FolderUID) { + continue + } + } + if folderMap[element.FolderUID] == "" { + folderMap[element.FolderUID] = dashboards.RootFolderName + } retDTOs = append(retDTOs, model.LibraryElementDTO{ ID: element.ID, OrgID: element.OrgID, @@ -468,7 +484,7 @@ func (l *LibraryElementService) getAllLibraryElements(c context.Context, signedI Model: element.Model, Version: element.Version, Meta: model.LibraryElementDTOMeta{ - FolderName: element.FolderName, + FolderName: folderMap[element.FolderUID], FolderUID: element.FolderUID, ConnectedDashboards: element.ConnectedDashboards, Created: element.Created, @@ -501,8 +517,7 @@ func (l *LibraryElementService) getAllLibraryElements(c context.Context, signedI } countBuilder.Write(selectLibraryElementDTOWithMeta) countBuilder.Write(getFromLibraryElementDTOWithMeta(l.SQLStore.GetDialect())) - countBuilder.Write(" INNER JOIN dashboard AS dashboard on le.folder_id = dashboard.id and le.folder_id<>0") - countBuilder.Write(` WHERE le.org_id=?`, signedInUser.GetOrgID()) + countBuilder.Write(` WHERE le.org_id=? AND le.folder_id<>0`, signedInUser.GetOrgID()) writeKindSQL(query, &countBuilder) writeSearchStringSQL(query, l.SQLStore, &countBuilder) writeExcludeSQL(query, &countBuilder) @@ -685,14 +700,25 @@ func (l *LibraryElementService) getConnections(c context.Context, signedInUser i builder.Write(" LEFT JOIN " + l.SQLStore.GetDialect().Quote("user") + " AS u1 ON lec.created_by = u1.id") builder.Write(" INNER JOIN dashboard AS dashboard on lec.connection_id = dashboard.id") builder.Write(` WHERE lec.element_id=?`, element.ID) - if signedInUser.GetOrgRole() != org.RoleAdmin { - builder.WriteDashboardPermissionFilter(signedInUser, dashboardaccess.PERMISSION_VIEW, "") - } if err := session.SQL(builder.GetSQLString(), builder.GetParams()...).Find(&libraryElementConnections); err != nil { return err } + fs, err := l.folderService.GetFolders(c, folder.GetFoldersQuery{OrgID: signedInUser.GetOrgID(), SignedInUser: signedInUser}) + if err != nil { + return err + } + // Every signed in user can see the general folder. The general folder might have "general" or the empty string as its UID. + var folderUIDS = []string{"general", ""} + for _, f := range fs { + folderUIDS = append(folderUIDS, f.UID) + } for _, connection := range libraryElementConnections { + if !signedInUser.HasRole(org.RoleAdmin) { + if !contains(folderUIDS, element.FolderUID) { + continue + } + } connections = append(connections, model.LibraryElementConnectionDTO{ ID: connection.ID, Kind: connection.Kind, @@ -871,3 +897,12 @@ func (l *LibraryElementService) deleteLibraryElementsInFolderUID(c context.Conte return nil }) } + +func contains(slice []string, element string) bool { + for _, item := range slice { + if item == element { + return true + } + } + return false +} diff --git a/pkg/services/libraryelements/libraryelements_create_test.go b/pkg/services/libraryelements/libraryelements_create_test.go index 7adca02ebd1..45e7a797b5b 100644 --- a/pkg/services/libraryelements/libraryelements_create_test.go +++ b/pkg/services/libraryelements/libraryelements_create_test.go @@ -44,7 +44,7 @@ func TestCreateLibraryElement(t *testing.T) { Version: 1, Meta: model.LibraryElementDTOMeta{ FolderName: "ScenarioFolder", - FolderUID: "ScenarioFolder", + FolderUID: "uid_for_ScenarioFolder", ConnectedDashboards: 0, Created: sc.initialResult.Result.Meta.Created, Updated: sc.initialResult.Result.Meta.Updated, @@ -95,7 +95,7 @@ func TestCreateLibraryElement(t *testing.T) { Version: 1, Meta: model.LibraryElementDTOMeta{ FolderName: "ScenarioFolder", - FolderUID: "ScenarioFolder", + FolderUID: "uid_for_ScenarioFolder", ConnectedDashboards: 0, Created: result.Result.Meta.Created, Updated: result.Result.Meta.Updated, @@ -174,7 +174,7 @@ func TestCreateLibraryElement(t *testing.T) { Version: 1, Meta: model.LibraryElementDTOMeta{ FolderName: "ScenarioFolder", - FolderUID: "ScenarioFolder", + FolderUID: "uid_for_ScenarioFolder", ConnectedDashboards: 0, Created: result.Result.Meta.Created, Updated: result.Result.Meta.Updated, diff --git a/pkg/services/libraryelements/libraryelements_patch_test.go b/pkg/services/libraryelements/libraryelements_patch_test.go index 8e7dce686ea..19cdb8c248a 100644 --- a/pkg/services/libraryelements/libraryelements_patch_test.go +++ b/pkg/services/libraryelements/libraryelements_patch_test.go @@ -67,7 +67,7 @@ func TestPatchLibraryElement(t *testing.T) { Version: 2, Meta: model.LibraryElementDTOMeta{ FolderName: "NewFolder", - FolderUID: "NewFolder", + FolderUID: "uid_for_NewFolder", ConnectedDashboards: 0, Created: sc.initialResult.Result.Meta.Created, Updated: result.Result.Meta.Updated, @@ -111,7 +111,7 @@ func TestPatchLibraryElement(t *testing.T) { sc.initialResult.Result.Meta.Updated = result.Result.Meta.Updated sc.initialResult.Result.Version = 2 sc.initialResult.Result.Meta.FolderName = "NewFolder" - sc.initialResult.Result.Meta.FolderUID = "NewFolder" + sc.initialResult.Result.Meta.FolderUID = "uid_for_NewFolder" if diff := cmp.Diff(sc.initialResult.Result, result.Result, getCompareOptions()...); diff != "" { t.Fatalf("Result mismatch (-want +got):\n%s", diff) } diff --git a/pkg/services/libraryelements/libraryelements_permissions_test.go b/pkg/services/libraryelements/libraryelements_permissions_test.go index f9af4ed15ad..393810a0644 100644 --- a/pkg/services/libraryelements/libraryelements_permissions_test.go +++ b/pkg/services/libraryelements/libraryelements_permissions_test.go @@ -145,7 +145,7 @@ func TestLibraryElementCreatePermissions(t *testing.T) { { desc: "can create library elements when granted write access to the correct folder", permissions: map[string][]string{ - dashboards.ActionFoldersWrite: {dashboards.ScopeFoldersProvider.GetResourceScopeUID("Folder")}, + dashboards.ActionFoldersWrite: {dashboards.ScopeFoldersProvider.GetResourceScopeUID("uid_for_Folder")}, dashboards.ActionFoldersRead: {dashboards.ScopeFoldersProvider.GetResourceAllScope()}, }, status: http.StatusOK, @@ -161,7 +161,7 @@ func TestLibraryElementCreatePermissions(t *testing.T) { { desc: "can't create library elements when granted write access to the wrong folder", permissions: map[string][]string{ - dashboards.ActionFoldersWrite: {dashboards.ScopeFoldersProvider.GetResourceScopeUID("Other_folder")}, + dashboards.ActionFoldersWrite: {dashboards.ScopeFoldersProvider.GetResourceScopeUID("uid_for_Other_folder")}, dashboards.ActionFoldersRead: {dashboards.ScopeFoldersProvider.GetResourceAllScope()}, }, status: http.StatusForbidden, @@ -169,7 +169,7 @@ func TestLibraryElementCreatePermissions(t *testing.T) { { desc: "can't create library elements when granted read access to the right folder", permissions: map[string][]string{ - dashboards.ActionFoldersRead: {dashboards.ScopeFoldersProvider.GetResourceScopeUID("Folder")}, + dashboards.ActionFoldersRead: {dashboards.ScopeFoldersProvider.GetResourceScopeUID("uid_for_Folder")}, }, status: http.StatusForbidden, }, @@ -201,7 +201,7 @@ func TestLibraryElementPatchPermissions(t *testing.T) { { desc: "can move library elements when granted write access to the source and destination folders", permissions: map[string][]string{ - dashboards.ActionFoldersWrite: {dashboards.ScopeFoldersProvider.GetResourceScopeUID("FromFolder"), dashboards.ScopeFoldersProvider.GetResourceScopeUID("ToFolder")}, + dashboards.ActionFoldersWrite: {dashboards.ScopeFoldersProvider.GetResourceScopeUID("uid_for_FromFolder"), dashboards.ScopeFoldersProvider.GetResourceScopeUID("uid_for_ToFolder")}, dashboards.ActionFoldersRead: {dashboards.ScopeFoldersProvider.GetResourceAllScope()}, }, status: http.StatusOK, @@ -267,8 +267,8 @@ func TestLibraryElementDeletePermissions(t *testing.T) { { desc: "can delete library elements when granted write access to the correct folder", permissions: map[string][]string{ - dashboards.ActionFoldersWrite: {dashboards.ScopeFoldersProvider.GetResourceScopeUID("Folder")}, - dashboards.ActionFoldersRead: {dashboards.ScopeFoldersProvider.GetResourceScopeUID("Folder")}, + dashboards.ActionFoldersWrite: {dashboards.ScopeFoldersProvider.GetResourceScopeUID("uid_for_Folder")}, + dashboards.ActionFoldersRead: {dashboards.ScopeFoldersProvider.GetResourceScopeUID("uid_for_Folder")}, }, status: http.StatusOK, }, diff --git a/pkg/services/libraryelements/libraryelements_test.go b/pkg/services/libraryelements/libraryelements_test.go index 8f2a379cc80..f67a3e64ec6 100644 --- a/pkg/services/libraryelements/libraryelements_test.go +++ b/pkg/services/libraryelements/libraryelements_test.go @@ -343,11 +343,11 @@ func createFolder(t *testing.T, sc scenarioContext, title string, folderSvc fold store := folderimpl.ProvideStore(sc.sqlStore) folderSvc = folderimpl.ProvideService(store, ac, bus.ProvideBus(tracing.InitializeTracerForTest()), dashboardStore, folderStore, sc.sqlStore, features, supportbundlestest.NewFakeBundleService(), nil, cfg, nil, tracing.InitializeTracerForTest()) - t.Logf("Creating folder with title and UID %q", title) + t.Logf("Creating folder with title %q and UID uid_for_%s", title, title) } ctx := identity.WithRequester(context.Background(), &sc.user) folder, err := folderSvc.Create(ctx, &folder.CreateFolderCommand{ - OrgID: sc.user.OrgID, Title: title, UID: title, SignedInUser: &sc.user, + OrgID: sc.user.OrgID, Title: title, UID: "uid_for_" + title, SignedInUser: &sc.user, }) require.NoError(t, err) diff --git a/pkg/services/libraryelements/model/model.go b/pkg/services/libraryelements/model/model.go index e53ef13816f..0828e9578f6 100644 --- a/pkg/services/libraryelements/model/model.go +++ b/pkg/services/libraryelements/model/model.go @@ -214,6 +214,7 @@ type GetLibraryElementCommand struct { // Deprecated: use FolderUID instead FolderID int64 UID string + Name string } // SearchLibraryElementsQuery is the query used for searching for Elements diff --git a/pkg/services/libraryelements/writers.go b/pkg/services/libraryelements/writers.go index 18781637fac..9629b96b66a 100644 --- a/pkg/services/libraryelements/writers.go +++ b/pkg/services/libraryelements/writers.go @@ -23,7 +23,7 @@ func selectLibraryElementByParam(params []Pair) (string, []any) { conditions = append(conditions, "le."+p.key+"=?") values = append(values, p.value) } - return ` WHERE ` + strings.Join(conditions, " AND "), values + return strings.Join(conditions, " AND "), values } func writeParamSelectorSQL(builder *db.SQLBuilder, params ...Pair) { @@ -162,7 +162,7 @@ func (f *FolderFilter) writeFolderFilterSQL(includeGeneral bool, builder *db.SQL paramsUIDs = append(paramsUIDs, folderUID) } if len(paramsUIDs) > 0 { - sql.WriteString(` AND dashboard.uid IN (?` + strings.Repeat(",?", len(paramsUIDs)-1) + ")") + sql.WriteString(` AND le.folder_uid IN (?` + strings.Repeat(",?", len(paramsUIDs)-1) + ")") builder.Write(sql.String(), paramsUIDs...) }