diff --git a/pkg/services/folder/folderimpl/unifiedstore.go b/pkg/services/folder/folderimpl/unifiedstore.go index 7d893355903..dc61d1ea2bf 100644 --- a/pkg/services/folder/folderimpl/unifiedstore.go +++ b/pkg/services/folder/folderimpl/unifiedstore.go @@ -2,9 +2,7 @@ package folderimpl import ( "context" - "errors" "fmt" - "net/http" "strings" apierrors "k8s.io/apimachinery/pkg/api/errors" @@ -153,13 +151,11 @@ func (ss *FolderUnifiedStoreImpl) Get(ctx context.Context, q folder.GetFolderQue func (ss *FolderUnifiedStoreImpl) GetParents(ctx context.Context, q folder.GetParentsQuery) ([]*folder.Folder, error) { hits := []*folder.Folder{} - parentUid := q.UID - - for parentUid != "" { - folder, err := ss.Get(ctx, folder.GetFolderQuery{UID: &parentUid, OrgID: q.OrgID}) + parentUID := q.UID + for parentUID != "" { + folder, err := ss.Get(ctx, folder.GetFolderQuery{UID: &parentUID, OrgID: q.OrgID}) if err != nil { - var statusError *apierrors.StatusError - if errors.As(err, &statusError) && statusError.ErrStatus.Code == http.StatusForbidden { + if apierrors.IsForbidden(err) { // If we get a Forbidden error when requesting the parent folder, it means the user does not have access // to it, nor its parents. So we can stop looping break @@ -167,15 +163,15 @@ func (ss *FolderUnifiedStoreImpl) GetParents(ctx context.Context, q folder.GetPa return nil, err } - parentUid = folder.ParentUID + parentUID = folder.ParentUID hits = append(hits, folder) } - if len(hits) > 0 { - return util.Reverse(hits[1:]), nil + if len(hits) == 0 { + return hits, nil } - return hits, nil + return util.Reverse(hits[1:]), nil } func (ss *FolderUnifiedStoreImpl) GetChildren(ctx context.Context, q folder.GetChildrenQuery) ([]*folder.FolderReference, error) { @@ -340,46 +336,37 @@ func (ss *FolderUnifiedStoreImpl) GetFolders(ctx context.Context, q folder.GetFo return nil, err } - m := map[string]*folder.Folder{} + filters := make(map[string]struct{}, len(q.UIDs)) + for _, uid := range q.UIDs { + filters[uid] = struct{}{} + } + + folderMap := make(map[string]*folder.Folder) + relations := make(map[string]string) + if q.WithFullpath || q.WithFullpathUIDs { + for _, folder := range folders { + folderMap[folder.UID] = folder + relations[folder.UID] = folder.ParentUID + } + } + + hits := make([]*folder.Folder, 0, len(folders)) for _, f := range folders { + if shouldSkipFolder(f, filters) { + continue + } + if (q.WithFullpath || q.WithFullpathUIDs) && f.Fullpath == "" { - parents, err := ss.GetParents(ctx, folder.GetParentsQuery{UID: f.UID, OrgID: q.OrgID}) - if err != nil { - return nil, fmt.Errorf("failed to get parents for folder %s: %w", f.UID, err) - } - // If we don't have a parent, we just return the current folder as the full path - f.Fullpath, f.FullpathUIDs = computeFullPath(append(parents, f)) + buildFolderFullPaths(f, relations, folderMap) } - m[f.UID] = f - } - - hits := []*folder.Folder{} - - if len(q.UIDs) > 0 { - //return only the specified q.UIDs - for _, uid := range q.UIDs { - f, ok := m[uid] - if ok { - hits = append(hits, f) - } - } - - return hits, nil - } - - /* - if len(q.AncestorUIDs) > 0 { - // TODO - //return all nodes under those ancestors, requires building a tree - } - */ - - //return everything - for _, f := range m { hits = append(hits, f) } + // TODO: return all nodes under those ancestors, requires building a tree + // if len(q.AncestorUIDs) > 0 { + // } + return hits, nil } @@ -490,3 +477,42 @@ func computeFullPath(parents []*folder.Folder) (string, string) { } return strings.Join(fullpath, "/"), strings.Join(fullpathUIDs, "/") } + +func buildFolderFullPaths(f *folder.Folder, relations map[string]string, folderMap map[string]*folder.Folder) { + titles := make([]string, 0) + uids := make([]string, 0) + + titles = append(titles, f.Title) + uids = append(uids, f.UID) + + currentUID := f.UID + for currentUID != "" { + parentUID, exists := relations[currentUID] + if !exists { + break + } + + if parentUID == "" { + break + } + + parentFolder, exists := folderMap[parentUID] + if !exists { + break + } + titles = append(titles, parentFolder.Title) + uids = append(uids, parentFolder.UID) + currentUID = parentFolder.UID + } + + f.Fullpath = strings.Join(util.Reverse(titles), "/") + f.FullpathUIDs = strings.Join(util.Reverse(uids), "/") +} + +func shouldSkipFolder(f *folder.Folder, filterUIDs map[string]struct{}) bool { + if len(filterUIDs) == 0 { + return false + } + _, exists := filterUIDs[f.UID] + return !exists +} diff --git a/pkg/services/folder/folderimpl/unifiedstore_test.go b/pkg/services/folder/folderimpl/unifiedstore_test.go index f17b954078b..76751583bd5 100644 --- a/pkg/services/folder/folderimpl/unifiedstore_test.go +++ b/pkg/services/folder/folderimpl/unifiedstore_test.go @@ -174,6 +174,7 @@ func TestGetParents(t *testing.T) { require.Len(t, result, 1) require.Equal(t, "parenttwo", result[0].UID) }) + t.Run("should stop if parent folder is not found", func(t *testing.T) { mockCli.On("Get", mock.Anything, "parentone", orgID, mock.Anything, mock.Anything).Return(nil, apierrors.NewNotFound(schema.GroupResource{Group: "folders.folder.grafana.app", Resource: "folder"}, "parentone")).Once() @@ -439,3 +440,443 @@ func TestGetChildren(t *testing.T) { require.Equal(t, "folder3", result[1].UID) }) } + +func TestGetFolders(t *testing.T) { + type args struct { + ctx context.Context + q folder.GetFoldersFromStoreQuery + } + tests := []struct { + name string + args args + mock func(mockCli *client.MockK8sHandler) + want []*folder.Folder + wantErr bool + }{ + { + name: "should return all folders from k8s", + args: args{ + ctx: context.Background(), + q: folder.GetFoldersFromStoreQuery{ + GetFoldersQuery: folder.GetFoldersQuery{ + OrgID: orgID, + }, + }, + }, + mock: func(mockCli *client.MockK8sHandler) { + mockCli.On("List", mock.Anything, orgID, metav1.ListOptions{ + Limit: 100000, + TypeMeta: metav1.TypeMeta{}, + }).Return(&unstructured.UnstructuredList{ + Items: []unstructured.Unstructured{ + { + Object: map[string]interface{}{ + "metadata": map[string]interface{}{ + "name": "folder1", + "uid": "folder1", + }, + "spec": map[string]interface{}{ + "title": "folder1", + }, + }, + }, + { + Object: map[string]interface{}{ + "metadata": map[string]interface{}{ + "name": "folder2", + "uid": "folder2", + }, + "spec": map[string]interface{}{ + "title": "folder2", + }, + }, + }, + }, + }, nil).Once() + }, + want: []*folder.Folder{ + { + UID: "folder1", + Title: "folder1", + OrgID: orgID, + }, + { + UID: "folder2", + Title: "folder2", + OrgID: orgID, + }, + }, + wantErr: false, + }, + { + name: "should return folders from k8s by uid", + args: args{ + ctx: context.Background(), + q: folder.GetFoldersFromStoreQuery{ + GetFoldersQuery: folder.GetFoldersQuery{ + OrgID: orgID, + UIDs: []string{"folder1", "folder2"}, + }, + }, + }, + mock: func(mockCli *client.MockK8sHandler) { + mockCli.On("List", mock.Anything, orgID, metav1.ListOptions{ + Limit: 100000, + TypeMeta: metav1.TypeMeta{}, + }).Return(&unstructured.UnstructuredList{ + Items: []unstructured.Unstructured{ + { + Object: map[string]interface{}{ + "metadata": map[string]interface{}{ + "name": "folder1", + "uid": "folder1", + }, + "spec": map[string]interface{}{ + "title": "folder1", + }, + }, + }, + { + Object: map[string]interface{}{ + "metadata": map[string]interface{}{ + "name": "folder2", + "uid": "folder2", + }, + "spec": map[string]interface{}{ + "title": "folder2", + }, + }, + }, + { + Object: map[string]interface{}{ + "metadata": map[string]interface{}{ + "name": "folder3", + "uid": "folder3", + }, + "spec": map[string]interface{}{ + "title": "folder3", + }, + }, + }, + }, + }, nil).Once() + }, + want: []*folder.Folder{ + { + UID: "folder1", + Title: "folder1", + OrgID: orgID, + }, + { + UID: "folder2", + Title: "folder2", + OrgID: orgID, + }, + }, + wantErr: false, + }, + { + name: "should return all folders from k8s with fullpath enabled", + args: args{ + ctx: context.Background(), + q: folder.GetFoldersFromStoreQuery{ + GetFoldersQuery: folder.GetFoldersQuery{ + OrgID: orgID, + WithFullpath: true, + }, + }, + }, + mock: func(mockCli *client.MockK8sHandler) { + mockCli.On("List", mock.Anything, orgID, metav1.ListOptions{ + Limit: 100000, + TypeMeta: metav1.TypeMeta{}, + LabelSelector: "grafana.app/fullpath=true", + }).Return(&unstructured.UnstructuredList{ + Items: []unstructured.Unstructured{ + { + Object: map[string]interface{}{ + "metadata": map[string]interface{}{ + "name": "root1", + "uid": "root1", + }, + "spec": map[string]interface{}{ + "title": "root1", + }, + }, + }, + { + Object: map[string]interface{}{ + "metadata": map[string]interface{}{ + "name": "root2", + "uid": "root2", + }, + "spec": map[string]interface{}{ + "title": "root2", + }, + }, + }, + }, + }, nil).Once() + }, + want: []*folder.Folder{ + { + UID: "root1", + Title: "root1", + OrgID: orgID, + Fullpath: "root1", + FullpathUIDs: "root1", + }, + { + UID: "root2", + Title: "root2", + OrgID: orgID, + Fullpath: "root2", + FullpathUIDs: "root2", + }, + }, + wantErr: false, + }, + { + name: "should return folders from k8s by uid with fullpath enabled", + args: args{ + ctx: context.Background(), + q: folder.GetFoldersFromStoreQuery{ + GetFoldersQuery: folder.GetFoldersQuery{ + OrgID: orgID, + UIDs: []string{"folder1", "folder2"}, + WithFullpath: true, + }, + }, + }, + mock: func(mockCli *client.MockK8sHandler) { + mockCli.On("List", mock.Anything, orgID, metav1.ListOptions{ + Limit: 100000, + TypeMeta: metav1.TypeMeta{}, + LabelSelector: "grafana.app/fullpath=true", + }).Return(&unstructured.UnstructuredList{ + Items: []unstructured.Unstructured{ + { + Object: map[string]interface{}{ + "metadata": map[string]interface{}{ + "name": "parentcommon", + "uid": "parentcommon", + }, + "spec": map[string]interface{}{ + "title": "parentcommon", + }, + }, + }, + { + Object: map[string]interface{}{ + "metadata": map[string]interface{}{ + "name": "folder1", + "uid": "folder1", + "annotations": map[string]interface{}{"grafana.app/folder": "parentcommon"}, + }, + "spec": map[string]interface{}{ + "title": "folder1", + }, + }, + }, + { + Object: map[string]interface{}{ + "metadata": map[string]interface{}{ + "name": "folder2", + "uid": "folder2", + "annotations": map[string]interface{}{"grafana.app/folder": "parentcommon"}, + }, + "spec": map[string]interface{}{ + "title": "folder2", + }, + }, + }, + }, + }, nil).Once() + }, + want: []*folder.Folder{ + { + UID: "folder1", + Title: "folder1", + OrgID: orgID, + Fullpath: "parentcommon/folder1", + FullpathUIDs: "parentcommon/folder1", + }, + { + UID: "folder2", + Title: "folder2", + OrgID: orgID, + Fullpath: "parentcommon/folder2", + FullpathUIDs: "parentcommon/folder2", + }, + }, + wantErr: false, + }, + { + name: "should return error if k8s returns error", + args: args{ + ctx: context.Background(), + q: folder.GetFoldersFromStoreQuery{ + GetFoldersQuery: folder.GetFoldersQuery{ + OrgID: orgID, + UIDs: []string{"folder1", "folder2"}, + }, + }, + }, + mock: func(mockCli *client.MockK8sHandler) { + mockCli.On("List", mock.Anything, orgID, metav1.ListOptions{ + Limit: 100000, + TypeMeta: metav1.TypeMeta{}, + }).Return(nil, apierrors.NewNotFound(schema.GroupResource{Group: "folders.folder.grafana.app", Resource: "folder"}, "folder1")).Once() + }, + want: nil, + wantErr: true, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + mockCLI := new(client.MockK8sHandler) + tt.mock(mockCLI) + ss := &FolderUnifiedStoreImpl{ + k8sclient: mockCLI, + userService: usertest.NewUserServiceFake(), + } + got, err := ss.GetFolders(tt.args.ctx, tt.args.q) + require.Equal(t, tt.wantErr, err != nil, "GetFolders() error = %v, wantErr %v", err, tt.wantErr) + if !tt.wantErr { + require.Len(t, got, len(tt.want), "GetFolders() = %v, want %v", got, tt.want) + for i, folder := range got { + require.Equal(t, tt.want[i].UID, folder.UID, "GetFolders() = %v, want %v", got, tt.want) + require.Equal(t, tt.want[i].OrgID, folder.OrgID, "GetFolders() = %v, want %v", got, tt.want) + require.Equal(t, tt.want[i].Fullpath, folder.Fullpath, "GetFolders() = %v, want %v", got, tt.want) + require.Equal(t, tt.want[i].FullpathUIDs, folder.FullpathUIDs, "GetFolders() = %v, want %v", got, tt.want) + } + } + }) + } +} + +func TestBuildFolderFullPaths(t *testing.T) { + type args struct { + f *folder.Folder + relations map[string]string + folderMap map[string]*folder.Folder + } + tests := []struct { + name string + args args + want *folder.Folder + }{ + { + name: "should build full path for a folder with no parents", + args: args{ + f: &folder.Folder{ + Title: "Root", + UID: "root-uid", + }, + relations: map[string]string{}, + folderMap: map[string]*folder.Folder{}, + }, + want: &folder.Folder{ + Title: "Root", + UID: "root-uid", + Fullpath: "Root", + FullpathUIDs: "root-uid", + }, + }, + { + name: "should build full path for a folder with one parent", + args: args{ + f: &folder.Folder{ + Title: "Child", + UID: "child-uid", + ParentUID: "parent-uid", + }, + relations: map[string]string{ + "child-uid": "parent-uid", + }, + folderMap: map[string]*folder.Folder{ + "parent-uid": { + Title: "Parent", + UID: "parent-uid", + }, + }, + }, + want: &folder.Folder{ + Title: "Child", + UID: "child-uid", + ParentUID: "parent-uid", + Fullpath: "Parent/Child", + FullpathUIDs: "parent-uid/child-uid", + }, + }, + { + name: "should build full path for a folder with multiple parents", + args: args{ + f: &folder.Folder{ + Title: "Child", + UID: "child-uid", + ParentUID: "parent-uid", + }, + relations: map[string]string{ + "child-uid": "parent-uid", + "parent-uid": "grandparent-uid", + "grandparent-uid": "", + }, + folderMap: map[string]*folder.Folder{ + "child-uid": { + Title: "Child", + UID: "child-uid", + ParentUID: "parent-uid", + }, + "parent-uid": { + Title: "Parent", + UID: "parent-uid", + ParentUID: "grandparent-uid", + }, + "grandparent-uid": { + Title: "Grandparent", + UID: "grandparent-uid", + }, + }, + }, + want: &folder.Folder{ + Title: "Child", + UID: "child-uid", + ParentUID: "parent-uid", + Fullpath: "Grandparent/Parent/Child", + FullpathUIDs: "grandparent-uid/parent-uid/child-uid", + }, + }, + { + name: "should build full path for a folder with no parents in the map", + args: args{ + f: &folder.Folder{ + Title: "Child", + UID: "child-uid", + ParentUID: "parent-uid", + }, + relations: map[string]string{ + "child-uid": "parent-uid", + }, + folderMap: map[string]*folder.Folder{}, + }, + want: &folder.Folder{ + Title: "Child", + UID: "child-uid", + ParentUID: "parent-uid", + Fullpath: "Child", + FullpathUIDs: "child-uid", + }, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + buildFolderFullPaths(tt.args.f, tt.args.relations, tt.args.folderMap) + require.Equal(t, tt.want.Fullpath, tt.args.f.Fullpath, "BuildFolderFullPaths() = %v, want %v", tt.args.f.Fullpath, tt.want.Fullpath) + require.Equal(t, tt.want.FullpathUIDs, tt.args.f.FullpathUIDs, "BuildFolderFullPaths() = %v, want %v", tt.args.f.FullpathUIDs, tt.want.FullpathUIDs) + require.Equal(t, tt.want.Title, tt.args.f.Title, "BuildFolderFullPaths() = %v, want %v", tt.args.f.Title, tt.want.Title) + require.Equal(t, tt.want.UID, tt.args.f.UID, "BuildFolderFullPaths() = %v, want %v", tt.args.f.UID, tt.want.UID) + require.Equal(t, tt.want.ParentUID, tt.args.f.ParentUID, "BuildFolderFullPaths() = %v, want %v", tt.args.f.ParentUID, tt.want.ParentUID) + }) + } +}