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 <arati.rana@grafana.com>
This commit is contained in:
Leonor Oliveira
2025-01-27 10:51:41 +01:00
committed by GitHub
parent 25bb210ca3
commit 0003efa285
7 changed files with 73 additions and 37 deletions

View File

@ -15,7 +15,6 @@ import (
"github.com/grafana/grafana/pkg/kinds/librarypanel" "github.com/grafana/grafana/pkg/kinds/librarypanel"
ac "github.com/grafana/grafana/pkg/services/accesscontrol" ac "github.com/grafana/grafana/pkg/services/accesscontrol"
"github.com/grafana/grafana/pkg/services/dashboards" "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/featuremgmt"
"github.com/grafana/grafana/pkg/services/folder" "github.com/grafana/grafana/pkg/services/folder"
"github.com/grafana/grafana/pkg/services/libraryelements/model" "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 // 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) { 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) libraryElements := make([]model.LibraryElementWithMeta, 0)
recursiveQueriesAreSupported, err := store.RecursiveQueriesAreSupported() 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 { err = store.WithDbSession(c, func(session *db.Session) error {
builder := db.NewSqlBuilder(cfg, features, store.GetDialect(), recursiveQueriesAreSupported) builder := db.NewSqlBuilder(cfg, features, store.GetDialect(), recursiveQueriesAreSupported)
builder.Write(selectLibraryElementDTOWithMeta) builder.Write(selectLibraryElementDTOWithMeta)
builder.Write(", ? as folder_name ", cmd.FolderName)
builder.Write(getFromLibraryElementDTOWithMeta(store.GetDialect())) builder.Write(getFromLibraryElementDTOWithMeta(store.GetDialect()))
metrics.MFolderIDsServiceCount.WithLabelValues(metrics.LibraryElements).Inc() metrics.MFolderIDsServiceCount.WithLabelValues(metrics.LibraryElements).Inc()
builder.Write(" WHERE ")
// nolint:staticcheck // nolint:staticcheck
writeParamSelectorSQL(&builder, append(params, Pair{"folder_id", cmd.FolderID})...) writeParamSelectorSQL(&builder, append(params, Pair{"folder_id", cmd.FolderID})...)
builder.Write(" UNION ")
builder.Write(selectLibraryElementDTOWithMeta) builder.Write(" OR le.folder_id <> 0 AND ")
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")
writeParamSelectorSQL(&builder, params...) writeParamSelectorSQL(&builder, params...)
builder.Write(` OR dashboard.id=0`)
if err := session.SQL(builder.GetSQLString(), builder.GetParams()...).Find(&libraryElements); err != nil { if err := session.SQL(builder.GetSQLString(), builder.GetParams()...).Find(&libraryElements); err != nil {
return err 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, return l.getLibraryElements(c, l.SQLStore, l.Cfg, signedInUser, []Pair{{"org_id", signedInUser.GetOrgID()}, {"name", name}}, l.features,
model.GetLibraryElementCommand{ model.GetLibraryElementCommand{
FolderName: dashboards.RootFolderName, 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) builder := db.NewSqlBuilder(l.Cfg, l.features, l.SQLStore.GetDialect(), recursiveQueriesAreSupported)
if folderFilter.includeGeneralFolder { if folderFilter.includeGeneralFolder {
builder.Write(selectLibraryElementDTOWithMeta) builder.Write(selectLibraryElementDTOWithMeta)
builder.Write(", 'General' as folder_name ")
builder.Write(", '' as folder_uid ") builder.Write(", '' as folder_uid ")
builder.Write(getFromLibraryElementDTOWithMeta(l.SQLStore.GetDialect())) builder.Write(getFromLibraryElementDTOWithMeta(l.SQLStore.GetDialect()))
builder.Write(` WHERE le.org_id=? AND le.folder_id=0`, signedInUser.GetOrgID()) 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(" UNION ")
} }
builder.Write(selectLibraryElementDTOWithMeta) builder.Write(selectLibraryElementDTOWithMeta)
builder.Write(", dashboard.title as folder_name ") builder.Write(", le.folder_uid as folder_uid ")
builder.Write(", dashboard.uid as folder_uid ")
builder.Write(getFromLibraryElementDTOWithMeta(l.SQLStore.GetDialect())) 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=? AND le.folder_id<>0`, signedInUser.GetOrgID())
builder.Write(` WHERE le.org_id=?`, signedInUser.GetOrgID())
writeKindSQL(query, &builder) writeKindSQL(query, &builder)
writeSearchStringSQL(query, l.SQLStore, &builder) writeSearchStringSQL(query, l.SQLStore, &builder)
writeExcludeSQL(query, &builder) writeExcludeSQL(query, &builder)
@ -439,9 +437,6 @@ func (l *LibraryElementService) getAllLibraryElements(c context.Context, signedI
if err := folderFilter.writeFolderFilterSQL(false, &builder); err != nil { if err := folderFilter.writeFolderFilterSQL(false, &builder); err != nil {
return err return err
} }
if !signedInUser.HasRole(org.RoleAdmin) {
builder.WriteDashboardPermissionFilter(signedInUser, dashboardaccess.PERMISSION_VIEW, "")
}
if query.SortDirection == search.SortAlphaDesc.Name { if query.SortDirection == search.SortAlphaDesc.Name {
builder.Write(" ORDER BY 1 DESC") builder.Write(" ORDER BY 1 DESC")
} else { } else {
@ -454,7 +449,28 @@ func (l *LibraryElementService) getAllLibraryElements(c context.Context, signedI
metrics.MFolderIDsServiceCount.WithLabelValues(metrics.LibraryElements).Inc() metrics.MFolderIDsServiceCount.WithLabelValues(metrics.LibraryElements).Inc()
retDTOs := make([]model.LibraryElementDTO, 0) 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 { 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{ retDTOs = append(retDTOs, model.LibraryElementDTO{
ID: element.ID, ID: element.ID,
OrgID: element.OrgID, OrgID: element.OrgID,
@ -468,7 +484,7 @@ func (l *LibraryElementService) getAllLibraryElements(c context.Context, signedI
Model: element.Model, Model: element.Model,
Version: element.Version, Version: element.Version,
Meta: model.LibraryElementDTOMeta{ Meta: model.LibraryElementDTOMeta{
FolderName: element.FolderName, FolderName: folderMap[element.FolderUID],
FolderUID: element.FolderUID, FolderUID: element.FolderUID,
ConnectedDashboards: element.ConnectedDashboards, ConnectedDashboards: element.ConnectedDashboards,
Created: element.Created, Created: element.Created,
@ -501,8 +517,7 @@ func (l *LibraryElementService) getAllLibraryElements(c context.Context, signedI
} }
countBuilder.Write(selectLibraryElementDTOWithMeta) countBuilder.Write(selectLibraryElementDTOWithMeta)
countBuilder.Write(getFromLibraryElementDTOWithMeta(l.SQLStore.GetDialect())) 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=? AND le.folder_id<>0`, signedInUser.GetOrgID())
countBuilder.Write(` WHERE le.org_id=?`, signedInUser.GetOrgID())
writeKindSQL(query, &countBuilder) writeKindSQL(query, &countBuilder)
writeSearchStringSQL(query, l.SQLStore, &countBuilder) writeSearchStringSQL(query, l.SQLStore, &countBuilder)
writeExcludeSQL(query, &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(" 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(" INNER JOIN dashboard AS dashboard on lec.connection_id = dashboard.id")
builder.Write(` WHERE lec.element_id=?`, element.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 { if err := session.SQL(builder.GetSQLString(), builder.GetParams()...).Find(&libraryElementConnections); err != nil {
return err 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 { for _, connection := range libraryElementConnections {
if !signedInUser.HasRole(org.RoleAdmin) {
if !contains(folderUIDS, element.FolderUID) {
continue
}
}
connections = append(connections, model.LibraryElementConnectionDTO{ connections = append(connections, model.LibraryElementConnectionDTO{
ID: connection.ID, ID: connection.ID,
Kind: connection.Kind, Kind: connection.Kind,
@ -871,3 +897,12 @@ func (l *LibraryElementService) deleteLibraryElementsInFolderUID(c context.Conte
return nil return nil
}) })
} }
func contains(slice []string, element string) bool {
for _, item := range slice {
if item == element {
return true
}
}
return false
}

View File

@ -44,7 +44,7 @@ func TestCreateLibraryElement(t *testing.T) {
Version: 1, Version: 1,
Meta: model.LibraryElementDTOMeta{ Meta: model.LibraryElementDTOMeta{
FolderName: "ScenarioFolder", FolderName: "ScenarioFolder",
FolderUID: "ScenarioFolder", FolderUID: "uid_for_ScenarioFolder",
ConnectedDashboards: 0, ConnectedDashboards: 0,
Created: sc.initialResult.Result.Meta.Created, Created: sc.initialResult.Result.Meta.Created,
Updated: sc.initialResult.Result.Meta.Updated, Updated: sc.initialResult.Result.Meta.Updated,
@ -95,7 +95,7 @@ func TestCreateLibraryElement(t *testing.T) {
Version: 1, Version: 1,
Meta: model.LibraryElementDTOMeta{ Meta: model.LibraryElementDTOMeta{
FolderName: "ScenarioFolder", FolderName: "ScenarioFolder",
FolderUID: "ScenarioFolder", FolderUID: "uid_for_ScenarioFolder",
ConnectedDashboards: 0, ConnectedDashboards: 0,
Created: result.Result.Meta.Created, Created: result.Result.Meta.Created,
Updated: result.Result.Meta.Updated, Updated: result.Result.Meta.Updated,
@ -174,7 +174,7 @@ func TestCreateLibraryElement(t *testing.T) {
Version: 1, Version: 1,
Meta: model.LibraryElementDTOMeta{ Meta: model.LibraryElementDTOMeta{
FolderName: "ScenarioFolder", FolderName: "ScenarioFolder",
FolderUID: "ScenarioFolder", FolderUID: "uid_for_ScenarioFolder",
ConnectedDashboards: 0, ConnectedDashboards: 0,
Created: result.Result.Meta.Created, Created: result.Result.Meta.Created,
Updated: result.Result.Meta.Updated, Updated: result.Result.Meta.Updated,

View File

@ -67,7 +67,7 @@ func TestPatchLibraryElement(t *testing.T) {
Version: 2, Version: 2,
Meta: model.LibraryElementDTOMeta{ Meta: model.LibraryElementDTOMeta{
FolderName: "NewFolder", FolderName: "NewFolder",
FolderUID: "NewFolder", FolderUID: "uid_for_NewFolder",
ConnectedDashboards: 0, ConnectedDashboards: 0,
Created: sc.initialResult.Result.Meta.Created, Created: sc.initialResult.Result.Meta.Created,
Updated: result.Result.Meta.Updated, 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.Meta.Updated = result.Result.Meta.Updated
sc.initialResult.Result.Version = 2 sc.initialResult.Result.Version = 2
sc.initialResult.Result.Meta.FolderName = "NewFolder" 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 != "" { if diff := cmp.Diff(sc.initialResult.Result, result.Result, getCompareOptions()...); diff != "" {
t.Fatalf("Result mismatch (-want +got):\n%s", diff) t.Fatalf("Result mismatch (-want +got):\n%s", diff)
} }

View File

@ -145,7 +145,7 @@ func TestLibraryElementCreatePermissions(t *testing.T) {
{ {
desc: "can create library elements when granted write access to the correct folder", desc: "can create library elements when granted write access to the correct folder",
permissions: map[string][]string{ permissions: map[string][]string{
dashboards.ActionFoldersWrite: {dashboards.ScopeFoldersProvider.GetResourceScopeUID("Folder")}, dashboards.ActionFoldersWrite: {dashboards.ScopeFoldersProvider.GetResourceScopeUID("uid_for_Folder")},
dashboards.ActionFoldersRead: {dashboards.ScopeFoldersProvider.GetResourceAllScope()}, dashboards.ActionFoldersRead: {dashboards.ScopeFoldersProvider.GetResourceAllScope()},
}, },
status: http.StatusOK, 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", desc: "can't create library elements when granted write access to the wrong folder",
permissions: map[string][]string{ 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()}, dashboards.ActionFoldersRead: {dashboards.ScopeFoldersProvider.GetResourceAllScope()},
}, },
status: http.StatusForbidden, 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", desc: "can't create library elements when granted read access to the right folder",
permissions: map[string][]string{ permissions: map[string][]string{
dashboards.ActionFoldersRead: {dashboards.ScopeFoldersProvider.GetResourceScopeUID("Folder")}, dashboards.ActionFoldersRead: {dashboards.ScopeFoldersProvider.GetResourceScopeUID("uid_for_Folder")},
}, },
status: http.StatusForbidden, 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", desc: "can move library elements when granted write access to the source and destination folders",
permissions: map[string][]string{ 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()}, dashboards.ActionFoldersRead: {dashboards.ScopeFoldersProvider.GetResourceAllScope()},
}, },
status: http.StatusOK, 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", desc: "can delete library elements when granted write access to the correct folder",
permissions: map[string][]string{ permissions: map[string][]string{
dashboards.ActionFoldersWrite: {dashboards.ScopeFoldersProvider.GetResourceScopeUID("Folder")}, dashboards.ActionFoldersWrite: {dashboards.ScopeFoldersProvider.GetResourceScopeUID("uid_for_Folder")},
dashboards.ActionFoldersRead: {dashboards.ScopeFoldersProvider.GetResourceScopeUID("Folder")}, dashboards.ActionFoldersRead: {dashboards.ScopeFoldersProvider.GetResourceScopeUID("uid_for_Folder")},
}, },
status: http.StatusOK, status: http.StatusOK,
}, },

View File

@ -343,11 +343,11 @@ func createFolder(t *testing.T, sc scenarioContext, title string, folderSvc fold
store := folderimpl.ProvideStore(sc.sqlStore) store := folderimpl.ProvideStore(sc.sqlStore)
folderSvc = folderimpl.ProvideService(store, ac, bus.ProvideBus(tracing.InitializeTracerForTest()), dashboardStore, folderStore, sc.sqlStore, folderSvc = folderimpl.ProvideService(store, ac, bus.ProvideBus(tracing.InitializeTracerForTest()), dashboardStore, folderStore, sc.sqlStore,
features, supportbundlestest.NewFakeBundleService(), nil, cfg, nil, tracing.InitializeTracerForTest()) 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) ctx := identity.WithRequester(context.Background(), &sc.user)
folder, err := folderSvc.Create(ctx, &folder.CreateFolderCommand{ 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) require.NoError(t, err)

View File

@ -214,6 +214,7 @@ type GetLibraryElementCommand struct {
// Deprecated: use FolderUID instead // Deprecated: use FolderUID instead
FolderID int64 FolderID int64
UID string UID string
Name string
} }
// SearchLibraryElementsQuery is the query used for searching for Elements // SearchLibraryElementsQuery is the query used for searching for Elements

View File

@ -23,7 +23,7 @@ func selectLibraryElementByParam(params []Pair) (string, []any) {
conditions = append(conditions, "le."+p.key+"=?") conditions = append(conditions, "le."+p.key+"=?")
values = append(values, p.value) 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) { func writeParamSelectorSQL(builder *db.SQLBuilder, params ...Pair) {
@ -162,7 +162,7 @@ func (f *FolderFilter) writeFolderFilterSQL(includeGeneral bool, builder *db.SQL
paramsUIDs = append(paramsUIDs, folderUID) paramsUIDs = append(paramsUIDs, folderUID)
} }
if len(paramsUIDs) > 0 { 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...) builder.Write(sql.String(), paramsUIDs...)
} }