Folders: Fix folder pagination for cloud instances with many folders (#90008)

* filter the k6 folder out in the SQL queries rather than during post processing to ensure that the correct number of results is always returned

* linting
This commit is contained in:
Ieva
2024-07-05 11:19:03 +01:00
committed by GitHub
parent 4f06568f8a
commit e9ebb6eaa4
7 changed files with 95 additions and 24 deletions

View File

@@ -64,13 +64,7 @@ func (hs *HTTPServer) GetFolders(c *contextmodel.ReqContext) response.Response {
}
hits := make([]dtos.FolderSearchHit, 0)
requesterIsSvcAccount := c.SignedInUser.GetID().Namespace() == identity.NamespaceServiceAccount
for _, f := range folders {
// only list k6 folders when requested by a service account - prevents showing k6 folders in the UI for users
if (f.UID == accesscontrol.K6FolderUID || f.ParentUID == accesscontrol.K6FolderUID) && !requesterIsSvcAccount {
continue
}
hits = append(hits, dtos.FolderSearchHit{
ID: f.ID, // nolint:staticcheck
UID: f.UID,

View File

@@ -7,6 +7,7 @@ import (
"strings"
"time"
"github.com/grafana/grafana/pkg/apimachinery/identity"
"github.com/grafana/grafana/pkg/infra/db"
"github.com/grafana/grafana/pkg/infra/log"
"github.com/grafana/grafana/pkg/infra/metrics"
@@ -876,6 +877,11 @@ func (d *dashboardStore) FindDashboards(ctx context.Context, query *dashboards.F
})
}
// only list k6 folders when requested by a service account - prevents showing k6 folders in the UI for users
if query.SignedInUser == nil || query.SignedInUser.GetID().Namespace() != identity.NamespaceServiceAccount {
filters = append(filters, searchstore.K6FolderFilter{})
}
filters = append(filters, permissions.NewAccessControlDashboardPermissionFilter(query.SignedInUser, query.Permission, query.Type, d.features, recursiveQueriesAreSupported))
filters = append(filters, searchstore.DeletedFilter{Deleted: query.IsDeleted})

View File

@@ -700,16 +700,9 @@ func makeQueryResult(query *dashboards.FindPersistedDashboardsQuery, res []dashb
hitList := make([]*model.Hit, 0)
hits := make(map[int64]*model.Hit)
requesterIsSvcAccount := query.SignedInUser.GetID().Namespace() == identity.NamespaceServiceAccount
for _, item := range res {
hit, exists := hits[item.ID]
if !exists {
// Don't list k6 items for users, we don't want users to interact with k6 folders directly through folder UI
if (item.UID == accesscontrol.K6FolderUID || item.FolderUID == accesscontrol.K6FolderUID) && !requesterIsSvcAccount {
continue
}
metrics.MFolderIDsServiceCount.WithLabelValues(metrics.Dashboard).Inc()
hit = &model.Hit{
ID: item.ID,

View File

@@ -180,17 +180,7 @@ func (s *Service) GetFolders(ctx context.Context, q folder.GetFoldersQuery) ([]*
}
}
// only list k6 folders when requested by a service account - prevents showing k6 folders in the UI for users
result := make([]*folder.Folder, 0, len(dashFolders))
requesterIsSvcAccount := qry.SignedInUser.GetID().Namespace() == identity.NamespaceServiceAccount
for _, folder := range dashFolders {
if (folder.UID == accesscontrol.K6FolderUID || folder.ParentUID == accesscontrol.K6FolderUID) && !requesterIsSvcAccount {
continue
}
result = append(result, folder)
}
return result, nil
return dashFolders, nil
}
func (s *Service) Get(ctx context.Context, q *folder.GetFolderQuery) (*folder.Folder, error) {

View File

@@ -9,9 +9,11 @@ import (
"github.com/grafana/dskit/concurrency"
"github.com/grafana/grafana/pkg/apimachinery/identity"
"github.com/grafana/grafana/pkg/infra/db"
"github.com/grafana/grafana/pkg/infra/log"
"github.com/grafana/grafana/pkg/infra/metrics"
"github.com/grafana/grafana/pkg/services/accesscontrol"
"github.com/grafana/grafana/pkg/services/dashboards"
"github.com/grafana/grafana/pkg/services/folder"
"github.com/grafana/grafana/pkg/services/sqlstore/migrator"
@@ -319,6 +321,13 @@ func (ss *sqlStore) GetChildren(ctx context.Context, q folder.GetChildrenQuery)
}
sql.WriteString(")")
}
// only list k6 folders when requested by a service account - prevents showing k6 folders in the UI for users
if q.SignedInUser == nil || q.SignedInUser.GetID().Namespace() != identity.NamespaceServiceAccount {
sql.WriteString(" AND uid != ?")
args = append(args, accesscontrol.K6FolderUID)
}
sql.WriteString(" ORDER BY title ASC")
if q.Limit != 0 {
@@ -474,6 +483,12 @@ func (ss *sqlStore) GetFolders(ctx context.Context, q getFoldersQuery) ([]*folde
}
}
// only list k6 folders when requested by a service account - prevents showing k6 folders in the UI for users
if q.SignedInUser == nil || q.SignedInUser.GetID().Namespace() != identity.NamespaceServiceAccount {
s.WriteString(" AND f0.uid != ? AND (f0.parent_uid != ? OR f0.parent_uid IS NULL)")
args = append(args, accesscontrol.K6FolderUID, accesscontrol.K6FolderUID)
}
if len(q.ancestorUIDs) == 0 {
if q.OrderByTitle {
s.WriteString(` ORDER BY f0.title ASC`)

View File

@@ -14,11 +14,13 @@ import (
"github.com/stretchr/testify/require"
"github.com/grafana/grafana/pkg/infra/db"
"github.com/grafana/grafana/pkg/services/accesscontrol"
"github.com/grafana/grafana/pkg/services/folder"
"github.com/grafana/grafana/pkg/services/org"
"github.com/grafana/grafana/pkg/services/org/orgimpl"
"github.com/grafana/grafana/pkg/services/quota/quotatest"
"github.com/grafana/grafana/pkg/services/sqlstore"
"github.com/grafana/grafana/pkg/services/user"
"github.com/grafana/grafana/pkg/setting"
"github.com/grafana/grafana/pkg/util"
)
@@ -731,6 +733,68 @@ func TestIntegrationGetChildren(t *testing.T) {
t.Errorf("Result mismatch (-want +got):\n%s", diff)
}
})
t.Run("should hide k6-app folder for users but not for service accounts", func(t *testing.T) {
_, err = folderStore.Create(context.Background(), folder.CreateFolderCommand{
Title: "k6-app-folder",
OrgID: orgID,
UID: accesscontrol.K6FolderUID,
})
require.NoError(t, err)
children, err := folderStore.GetChildren(context.Background(), folder.GetChildrenQuery{
OrgID: orgID,
SignedInUser: usr,
})
require.NoError(t, err)
require.Equal(t, 1, len(children))
assert.Equal(t, parent.UID, children[0].UID)
// Service account should be able to list k6 folder
children, err = folderStore.GetChildren(context.Background(), folder.GetChildrenQuery{
OrgID: orgID,
SignedInUser: &user.SignedInUser{UserID: 2, OrgID: orgID, IsServiceAccount: true},
})
require.NoError(t, err)
require.Equal(t, 2, len(children))
childrenUIDs := make([]string, 0, len(children))
for _, child := range children {
childrenUIDs = append(childrenUIDs, child.UID)
}
assert.EqualValues(t, []string{parent.UID, accesscontrol.K6FolderUID}, childrenUIDs)
})
t.Run("pagination works if k6-app folder is hidden", func(t *testing.T) {
for i := 0; i < 4; i++ {
_, err = folderStore.Create(context.Background(), folder.CreateFolderCommand{
Title: fmt.Sprintf("root-%d", i),
OrgID: orgID,
UID: fmt.Sprintf("root-%d", i),
})
require.NoError(t, err)
}
// Should skip k6-app folder but get parent folder and two more folders
children, err := folderStore.GetChildren(context.Background(), folder.GetChildrenQuery{
OrgID: orgID,
SignedInUser: usr,
Limit: 3,
})
require.NoError(t, err)
require.Equal(t, 3, len(children))
assert.EqualValues(t, []string{parent.UID, "root-0", "root-1"}, []string{children[0].UID, children[1].UID, children[2].UID})
// Should get the two remaining folders
children, err = folderStore.GetChildren(context.Background(), folder.GetChildrenQuery{
OrgID: orgID,
SignedInUser: usr,
Page: 2,
Limit: 3,
})
require.NoError(t, err)
require.Equal(t, 2, len(children))
assert.EqualValues(t, []string{"root-2", "root-3"}, []string{children[0].UID, children[1].UID})
})
}
func TestIntegrationGetHeight(t *testing.T) {

View File

@@ -4,6 +4,7 @@ import (
"fmt"
"strings"
"github.com/grafana/grafana/pkg/services/accesscontrol"
"github.com/grafana/grafana/pkg/services/folder"
"github.com/grafana/grafana/pkg/services/search/model"
"github.com/grafana/grafana/pkg/services/sqlstore/migrator"
@@ -127,6 +128,14 @@ func (f DashboardFilter) Where() (string, []any) {
return sqlUIDin("dashboard.uid", f.UIDs)
}
type K6FolderFilter struct{}
func (f K6FolderFilter) Where() (string, []any) {
filter := "dashboard.uid != ? AND (dashboard.folder_uid != ? OR dashboard.folder_uid IS NULL)"
params := []any{accesscontrol.K6FolderUID, accesscontrol.K6FolderUID}
return filter, params
}
type TagsFilter struct {
Tags []string
}