[search] folder name lookup performance (#100154)

[search] use search for folder name lookup

pkg/api/apierrors/folder.go

@@ -88,3 +88,7 @@ func ToFolderStatusError(err error) k8sErrors.StatusError {
 		},
 	}
 }
+
+func IsForbidden(err error) bool {
+	return k8sErrors.IsForbidden(err) || errors.Is(err, dashboards.ErrFolderAccessDenied)
+}

pkg/api/dashboard.go

@@ -201,6 +201,18 @@ func (hs *HTTPServer) GetDashboard(c *contextmodel.ReqContext) response.Response
 		if errors.Is(err, dashboards.ErrFolderNotFound) {
 			return response.Error(http.StatusNotFound, "Folder not found", err)
 		}
+		if apierrors.IsForbidden(err) {
+			// the dashboard is in a folder the user can't access, so return the dashboard without folder info
+			err = nil
+			queryResult = &folder.Folder{
+				UID: dash.FolderUID,
+			}
+		}
+		if err != nil {
+			hs.log.Error("Failed to get dashboard folder", "error", err)
+			return response.Error(http.StatusInternalServerError, "Dashboard folder could not be read", err)
+		}
+
 		meta.FolderUid = queryResult.UID
 		meta.FolderTitle = queryResult.Title
 		meta.FolderId = queryResult.ID // nolint:staticcheck

pkg/registry/apis/dashboard/search.go

@@ -351,7 +351,7 @@ func (s *SearchHandler) DoSearch(w http.ResponseWriter, r *http.Request) {
 		return
 	}
 
-	if parsedResults != nil && len(searchRequest.SortBy) == 0 {
+	if len(searchRequest.SortBy) == 0 {
 		// default sort by resource descending ( folders then dashboards ) then title
 		sort.Slice(parsedResults.Hits, func(i, j int) bool {
 			return parsedResults.Hits[i].Resource > parsedResults.Hits[j].Resource ||

pkg/services/dashboards/service/dashboard_service.go

@@ -14,6 +14,7 @@ import (
 	"github.com/prometheus/client_golang/prometheus"
 	"go.opentelemetry.io/otel"
 
+	"golang.org/x/exp/maps"
 	"golang.org/x/exp/slices"
 	"golang.org/x/sync/errgroup"
 	apierrors "k8s.io/apimachinery/pkg/api/errors"
@@ -1248,26 +1249,13 @@ func (dr *DashboardServiceImpl) FindDashboards(ctx context.Context, query *dashb
 			return nil, err
 		}
 
+		folderNames, err := dr.fetchFolderNames(ctx, query, response.Hits)
+		if err != nil {
+			return nil, err
+		}
+
 		finalResults := make([]dashboards.DashboardSearchProjection, len(response.Hits))
-		// Create a small runtime cache for folders to avoid extra calls to the folder service
-		foldersMap := make(map[string]*folder.Folder)
-		serviceCtx, serviceIdent := identity.WithServiceIdentity(ctx, query.OrgId)
 		for i, hit := range response.Hits {
-			f, ok := foldersMap[hit.Folder]
-			if !ok {
-				// We can get search result where user don't have access to parents. If that happens this thi
-				// will fail if we call it as the requesting user. To resolve this we call this as the service so we can
-				// garantuee that we can fetch the parent.
-				f, err = dr.folderService.Get(serviceCtx, &folder.GetFolderQuery{
-					UID:          &hit.Folder,
-					OrgID:        query.OrgId,
-					SignedInUser: serviceIdent,
-				})
-				if err != nil {
-					return nil, err
-				}
-				foldersMap[hit.Folder] = f
-			}
 			result := dashboards.DashboardSearchProjection{
 				ID:          hit.Field.GetNestedInt64(search.DASHBOARD_LEGACY_ID),
 				UID:         hit.Name,
@@ -1276,7 +1264,7 @@ func (dr *DashboardServiceImpl) FindDashboards(ctx context.Context, query *dashb
 				Slug:        slugify.Slugify(hit.Title),
 				IsFolder:    false,
 				FolderUID:   hit.Folder,
-				FolderTitle: f.Title,
+				FolderTitle: folderNames[hit.Folder],
 				Tags:        hit.Tags,
 			}
 
@@ -1293,6 +1281,28 @@ func (dr *DashboardServiceImpl) FindDashboards(ctx context.Context, query *dashb
 	return dr.dashboardStore.FindDashboards(ctx, query)
 }
 
+func (dr *DashboardServiceImpl) fetchFolderNames(ctx context.Context, query *dashboards.FindPersistedDashboardsQuery, hits []dashboardv0alpha1.DashboardHit) (map[string]string, error) {
+	// call this with elevated permissions so we can get folder names where user does not have access
+	// some dashboards are shared directly with user, but the folder is not accessible via the folder permissions
+	serviceCtx, serviceIdent := identity.WithServiceIdentity(ctx, query.OrgId)
+	search := folder.SearchFoldersQuery{
+		UIDs:         getFolderUIDs(hits),
+		OrgID:        query.OrgId,
+		SignedInUser: serviceIdent,
+	}
+
+	folders, err := dr.folderService.SearchFolders(serviceCtx, search)
+	if err != nil {
+		return nil, folder.ErrInternal.Errorf("failed to fetch parent folders: %w", err)
+	}
+
+	folderNames := make(map[string]string)
+	for _, f := range folders {
+		folderNames[f.UID] = f.Title
+	}
+	return folderNames, nil
+}
+
 func (dr *DashboardServiceImpl) SearchDashboards(ctx context.Context, query *dashboards.FindPersistedDashboardsQuery) (model.HitList, error) {
 	ctx, span := tracer.Start(ctx, "dashboards.service.SearchDashboards")
 	defer span.End()
@@ -1652,7 +1662,7 @@ func (dr *DashboardServiceImpl) listDashboardsThroughK8s(ctx context.Context, or
 	return dashboards, nil
 }
 
-func (dr *DashboardServiceImpl) searchDashboardsThroughK8sRaw(ctx context.Context, query *dashboards.FindPersistedDashboardsQuery) (*dashboardv0alpha1.SearchResults, error) {
+func (dr *DashboardServiceImpl) searchDashboardsThroughK8sRaw(ctx context.Context, query *dashboards.FindPersistedDashboardsQuery) (dashboardv0alpha1.SearchResults, error) {
 	request := &resource.ResourceSearchRequest{
 		Options: &resource.ListOptions{
 			Fields: []*resource.Requirement{},
@@ -1777,7 +1787,7 @@ func (dr *DashboardServiceImpl) searchDashboardsThroughK8sRaw(ctx context.Contex
 	}
 
 	if err != nil {
-		return nil, err
+		return dashboardv0alpha1.SearchResults{}, err
 	}
 
 	if federate != nil {
@@ -1794,7 +1804,7 @@ func (dr *DashboardServiceImpl) searchDashboardsThroughK8sRaw(ctx context.Contex
 
 	res, err := dr.k8sclient.Search(ctx, query.OrgId, request)
 	if err != nil {
-		return nil, err
+		return dashboardv0alpha1.SearchResults{}, err
 	}
 
 	return dashboardsearch.ParseResults(res, 0)
@@ -2083,3 +2093,13 @@ func LegacySaveCommandToUnstructured(cmd *dashboards.SaveDashboardCommand, names
 
 	return finalObj, nil
 }
+
+func getFolderUIDs(hits []dashboardv0alpha1.DashboardHit) []string {
+	folderSet := map[string]bool{}
+	for _, hit := range hits {
+		if hit.Folder != "" && !folderSet[hit.Folder] {
+			folderSet[hit.Folder] = true
+		}
+	}
+	return maps.Keys(folderSet)
+}

pkg/services/dashboards/service/dashboard_service_test.go

@@ -1338,7 +1338,9 @@ func TestSearchDashboards(t *testing.T) {
 	fakeFolders := foldertest.NewFakeService()
 	fakeFolders.ExpectedFolder = &folder.Folder{
 		Title: "testing-folder-1",
+		UID:   "f1",
 	}
+	fakeFolders.ExpectedFolders = []*folder.Folder{fakeFolders.ExpectedFolder}
 	defer fakeStore.AssertExpectations(t)
 	service := &DashboardServiceImpl{
 		cfg:            setting.NewCfg(),
@@ -1359,6 +1361,7 @@ func TestSearchDashboards(t *testing.T) {
 				"tag2",
 			},
 			FolderTitle: "testing-folder-1",
+			FolderUID:   "f1",
 		},
 		{
 			UID:         "uid2",
@@ -1369,6 +1372,7 @@ func TestSearchDashboards(t *testing.T) {
 			URL:         "/d/uid2/dashboard-2",
 			Tags:        []string{},
 			FolderTitle: "testing-folder-1",
+			FolderUID:   "f1",
 		},
 	}
 	query := dashboards.FindPersistedDashboardsQuery{
@@ -1384,6 +1388,7 @@ func TestSearchDashboards(t *testing.T) {
 				Title:       "Dashboard 1",
 				Tags:        []string{"tag1", "tag2"},
 				FolderTitle: "testing-folder-1",
+				FolderUID:   "f1",
 			},
 			{
 				UID:         "uid2",
@@ -1391,6 +1396,7 @@ func TestSearchDashboards(t *testing.T) {
 				OrgID:       1,
 				Title:       "Dashboard 2",
 				FolderTitle: "testing-folder-1",
+				FolderUID:   "f1",
 			},
 		}, nil).Once()
 		result, err := service.SearchDashboards(context.Background(), &query)
@@ -1401,6 +1407,13 @@ func TestSearchDashboards(t *testing.T) {
 
 	t.Run("Should use Kubernetes client if feature flags are enabled", func(t *testing.T) {
 		ctx, k8sCliMock := setupK8sDashboardTests(service)
+		expectedFolders := model.HitList{
+			{
+				UID:   "f1",
+				Title: "testing-folder-1",
+			},
+		}
+		fakeFolders.ExpectedHitList = expectedFolders
 		k8sCliMock.On("GetNamespace", mock.Anything, mock.Anything).Return("default")
 		k8sCliMock.On("Search", mock.Anything, mock.Anything, mock.Anything).Return(&resource.ResourceSearchResponse{
 			Results: &resource.ResourceTable{
@@ -1426,7 +1439,7 @@ func TestSearchDashboards(t *testing.T) {
 						},
 						Cells: [][]byte{
 							[]byte("Dashboard 1"),
-							[]byte(""),
+							[]byte("f1"),
 							[]byte("[\"tag1\", \"tag2\"]"),
 						},
 					},
@@ -1437,7 +1450,7 @@ func TestSearchDashboards(t *testing.T) {
 						},
 						Cells: [][]byte{
 							[]byte("Dashboard 2"),
-							[]byte(""),
+							[]byte("f1"),
 							[]byte(""),
 						},
 					},

pkg/services/dashboards/service/search/search.go

@@ -36,13 +36,13 @@ var (
 	}
 )
 
-func ParseResults(result *resource.ResourceSearchResponse, offset int64) (*v0alpha1.SearchResults, error) {
+func ParseResults(result *resource.ResourceSearchResponse, offset int64) (v0alpha1.SearchResults, error) {
 	if result == nil {
-		return nil, nil
+		return v0alpha1.SearchResults{}, nil
 	} else if result.Error != nil {
-		return nil, fmt.Errorf("%d error searching: %s: %s", result.Error.Code, result.Error.Message, result.Error.Details)
+		return v0alpha1.SearchResults{}, fmt.Errorf("%d error searching: %s: %s", result.Error.Code, result.Error.Message, result.Error.Details)
 	} else if result.Results == nil {
-		return nil, nil
+		return v0alpha1.SearchResults{}, nil
 	}
 
 	titleIDX := 0
@@ -66,7 +66,7 @@ func ParseResults(result *resource.ResourceSearchResponse, offset int64) (*v0alp
 		}
 	}
 
-	sr := &v0alpha1.SearchResults{
+	sr := v0alpha1.SearchResults{
 		Offset:    offset,
 		TotalHits: result.TotalHits,
 		QueryCost: result.QueryCost,
@@ -80,7 +80,7 @@ func ParseResults(result *resource.ResourceSearchResponse, offset int64) (*v0alp
 			if _, ok := excludedFields[col.Name]; !ok {
 				val, err := resource.DecodeCell(col, colIndex, row.Cells[colIndex])
 				if err != nil {
-					return nil, err
+					return v0alpha1.SearchResults{}, err
 				}
 				// Some of the dashboard fields come in as int32, but we need to convert them to int64 or else fields.Set() will panic
 				int32Val, ok := val.(int32)