diff --git a/pkg/services/dashboards/dashboard.go b/pkg/services/dashboards/dashboard.go index b904e3c2e07..3f14c6f00b0 100644 --- a/pkg/services/dashboards/dashboard.go +++ b/pkg/services/dashboards/dashboard.go @@ -23,14 +23,14 @@ type DashboardService interface { // To fetch a dashboard under root by title should set the folder UID to point to an empty string // eg. util.Pointer("") GetDashboard(ctx context.Context, query *GetDashboardQuery) (*Dashboard, error) - GetDashboards(ctx context.Context, query *GetDashboardsQuery) ([]*Dashboard, error) + GetDashboards(ctx context.Context, query *GetDashboardsQuery) ([]*Dashboard, error) // use sparely only if you truly need dashboard.Data GetDashboardTags(ctx context.Context, query *GetDashboardTagsQuery) ([]*DashboardTagCloudItem, error) GetDashboardUIDByID(ctx context.Context, query *GetDashboardRefByIDQuery) (*DashboardRef, error) ImportDashboard(ctx context.Context, dto *SaveDashboardDTO) (*Dashboard, error) SaveDashboard(ctx context.Context, dto *SaveDashboardDTO, allowUiUpdate bool) (*Dashboard, error) SearchDashboards(ctx context.Context, query *FindPersistedDashboardsQuery) (model.HitList, error) CountInFolders(ctx context.Context, orgID int64, folderUIDs []string, user identity.Requester) (int64, error) - GetDashboardsSharedWithUser(ctx context.Context, user identity.Requester) ([]*Dashboard, error) + GetDashboardsSharedWithUser(ctx context.Context, user identity.Requester) ([]*DashboardRef, error) GetAllDashboards(ctx context.Context) ([]*Dashboard, error) GetAllDashboardsByOrgId(ctx context.Context, orgID int64) ([]*Dashboard, error) SoftDeleteDashboard(ctx context.Context, orgID int64, dashboardUid string) error diff --git a/pkg/services/dashboards/dashboard_service_mock.go b/pkg/services/dashboards/dashboard_service_mock.go index ad526998488..ac9bbbb0bb6 100644 --- a/pkg/services/dashboards/dashboard_service_mock.go +++ b/pkg/services/dashboards/dashboard_service_mock.go @@ -377,23 +377,23 @@ func (_m *FakeDashboardService) GetDashboards(ctx context.Context, query *GetDas } // GetDashboardsSharedWithUser provides a mock function with given fields: ctx, user -func (_m *FakeDashboardService) GetDashboardsSharedWithUser(ctx context.Context, user identity.Requester) ([]*Dashboard, error) { +func (_m *FakeDashboardService) GetDashboardsSharedWithUser(ctx context.Context, user identity.Requester) ([]*DashboardRef, error) { ret := _m.Called(ctx, user) if len(ret) == 0 { panic("no return value specified for GetDashboardsSharedWithUser") } - var r0 []*Dashboard + var r0 []*DashboardRef var r1 error - if rf, ok := ret.Get(0).(func(context.Context, identity.Requester) ([]*Dashboard, error)); ok { + if rf, ok := ret.Get(0).(func(context.Context, identity.Requester) ([]*DashboardRef, error)); ok { return rf(ctx, user) } - if rf, ok := ret.Get(0).(func(context.Context, identity.Requester) []*Dashboard); ok { + if rf, ok := ret.Get(0).(func(context.Context, identity.Requester) []*DashboardRef); ok { r0 = rf(ctx, user) } else { if ret.Get(0) != nil { - r0 = ret.Get(0).([]*Dashboard) + r0 = ret.Get(0).([]*DashboardRef) } } diff --git a/pkg/services/dashboards/database/database.go b/pkg/services/dashboards/database/database.go index 032d574b07f..1ada23143d9 100644 --- a/pkg/services/dashboards/database/database.go +++ b/pkg/services/dashboards/database/database.go @@ -906,7 +906,7 @@ func (d *dashboardStore) GetDashboardUIDByID(ctx context.Context, query *dashboa us := &dashboards.DashboardRef{} err := d.store.WithDbSession(ctx, func(sess *db.Session) error { - var rawSQL = `SELECT uid, slug from dashboard WHERE Id=?` + var rawSQL = `SELECT uid, slug, folder_uid from dashboard WHERE Id=?` exists, err := sess.SQL(rawSQL, query.ID).Get(us) if err != nil { return err diff --git a/pkg/services/dashboards/database/database_test.go b/pkg/services/dashboards/database/database_test.go index 7d30eca738c..6078a30e9b6 100644 --- a/pkg/services/dashboards/database/database_test.go +++ b/pkg/services/dashboards/database/database_test.go @@ -188,6 +188,7 @@ func TestIntegrationDashboardDataAccess(t *testing.T) { queryResult, err := dashboardStore.GetDashboardUIDByID(context.Background(), &query) require.NoError(t, err) require.Equal(t, queryResult.UID, savedDash.UID) + require.Equal(t, queryResult.FolderUID, savedFolder.UID) }) t.Run("Shouldn't be able to get a dashboard with just an OrgID", func(t *testing.T) { diff --git a/pkg/services/dashboards/models.go b/pkg/services/dashboards/models.go index fd074aba343..a071c5bd298 100644 --- a/pkg/services/dashboards/models.go +++ b/pkg/services/dashboards/models.go @@ -288,8 +288,9 @@ type GetDashboardsByPluginIDQuery struct { } type DashboardRef struct { - UID string `xorm:"uid"` - Slug string + UID string `xorm:"uid"` + Slug string + FolderUID string `xorm:"folder_uid"` } type GetDashboardRefByIDQuery struct { diff --git a/pkg/services/dashboards/service/dashboard_service.go b/pkg/services/dashboards/service/dashboard_service.go index 77d508b9aac..24381908e5a 100644 --- a/pkg/services/dashboards/service/dashboard_service.go +++ b/pkg/services/dashboards/service/dashboard_service.go @@ -1345,12 +1345,13 @@ func (dr *DashboardServiceImpl) GetDashboardUIDByID(ctx context.Context, query * return nil, fmt.Errorf("unexpected number of dashboards found: %d. desired: 1", len(result)) } - return &dashboards.DashboardRef{UID: result[0].UID, Slug: result[0].Slug}, nil + return &dashboards.DashboardRef{UID: result[0].UID, Slug: result[0].Slug, FolderUID: result[0].FolderUID}, nil } return dr.dashboardStore.GetDashboardUIDByID(ctx, query) } +// expensive query in new flow !! use sparingly - only if you truly need dashboard.Data func (dr *DashboardServiceImpl) GetDashboards(ctx context.Context, query *dashboards.GetDashboardsQuery) ([]*dashboards.Dashboard, error) { if dr.features.IsEnabledGlobally(featuremgmt.FlagKubernetesClientDashboardsFolders) { if query.OrgID == 0 { @@ -1386,17 +1387,16 @@ func (dr *DashboardServiceImpl) GetDashboards(ctx context.Context, query *dashbo return dr.dashboardStore.GetDashboards(ctx, query) } -func (dr *DashboardServiceImpl) GetDashboardsSharedWithUser(ctx context.Context, user identity.Requester) ([]*dashboards.Dashboard, error) { +func (dr *DashboardServiceImpl) GetDashboardsSharedWithUser(ctx context.Context, user identity.Requester) ([]*dashboards.DashboardRef, error) { return dr.getDashboardsSharedWithUser(ctx, user) } -func (dr *DashboardServiceImpl) getDashboardsSharedWithUser(ctx context.Context, user identity.Requester) ([]*dashboards.Dashboard, error) { +func (dr *DashboardServiceImpl) getDashboardsSharedWithUser(ctx context.Context, user identity.Requester) ([]*dashboards.DashboardRef, error) { ctx, span := tracer.Start(ctx, "dashboards.service.getDashboardsSharedWithUser") defer span.End() permissions := user.GetPermissions() dashboardPermissions := permissions[dashboards.ActionDashboardsRead] - sharedDashboards := make([]*dashboards.Dashboard, 0) dashboardUids := make([]string, 0) for _, p := range dashboardPermissions { if dashboardUid, found := strings.CutPrefix(p, dashboards.ScopeDashboardsPrefix); found { @@ -1407,27 +1407,42 @@ func (dr *DashboardServiceImpl) getDashboardsSharedWithUser(ctx context.Context, } if len(dashboardUids) == 0 { - return sharedDashboards, nil + return []*dashboards.DashboardRef{}, nil } dashboardsQuery := &dashboards.GetDashboardsQuery{ DashboardUIDs: dashboardUids, OrgID: user.GetOrgID(), } - sharedDashboards, err := dr.GetDashboards(ctx, dashboardsQuery) + + var err error + var dashs []*dashboards.Dashboard + if dr.features.IsEnabledGlobally(featuremgmt.FlagKubernetesClientDashboardsFolders) { + dashs, err = dr.searchDashboardsThroughK8s(ctx, &dashboards.FindPersistedDashboardsQuery{ + DashboardUIDs: dashboardUids, + OrgId: user.GetOrgID(), + }) + } else { + dashs, err = dr.dashboardStore.GetDashboards(ctx, dashboardsQuery) + } if err != nil { return nil, err } + + sharedDashboards := make([]*dashboards.DashboardRef, len(dashs)) + for i, d := range dashs { + sharedDashboards[i] = &dashboards.DashboardRef{UID: d.UID, Slug: d.Slug, FolderUID: d.FolderUID} + } + return dr.filterUserSharedDashboards(ctx, user, sharedDashboards) } // filterUserSharedDashboards filter dashboards directly assigned to user, but not located in folders with view permissions -func (dr *DashboardServiceImpl) filterUserSharedDashboards(ctx context.Context, user identity.Requester, userDashboards []*dashboards.Dashboard) ([]*dashboards.Dashboard, error) { +func (dr *DashboardServiceImpl) filterUserSharedDashboards(ctx context.Context, user identity.Requester, userDashboards []*dashboards.DashboardRef) ([]*dashboards.DashboardRef, error) { ctx, span := tracer.Start(ctx, "dashboards.service.filterUserSharedDashboards") defer span.End() - filteredDashboards := make([]*dashboards.Dashboard, 0) - + filteredDashboards := make([]*dashboards.DashboardRef, 0) folderUIDs := make([]string, 0) for _, dashboard := range userDashboards { folderUIDs = append(folderUIDs, dashboard.FolderUID) diff --git a/pkg/services/dashboards/service/dashboard_service_test.go b/pkg/services/dashboards/service/dashboard_service_test.go index 198c1e0b296..57101bf5be3 100644 --- a/pkg/services/dashboards/service/dashboard_service_test.go +++ b/pkg/services/dashboards/service/dashboard_service_test.go @@ -7,6 +7,7 @@ import ( "testing" "time" + "github.com/prometheus/client_golang/prometheus" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/mock" "github.com/stretchr/testify/require" @@ -40,6 +41,7 @@ import ( "github.com/grafana/grafana/pkg/services/user" "github.com/grafana/grafana/pkg/setting" "github.com/grafana/grafana/pkg/storage/unified/resource" + "golang.org/x/exp/slices" ) func TestDashboardService(t *testing.T) { @@ -1558,6 +1560,7 @@ func TestSearchDashboards(t *testing.T) { cfg: setting.NewCfg(), dashboardStore: &fakeStore, folderService: fakeFolders, + metrics: newDashboardsMetrics(prometheus.NewRegistry()), } expectedResult := model.HitList{ @@ -1676,6 +1679,109 @@ func TestSearchDashboards(t *testing.T) { require.Equal(t, expectedResult, result) k8sCliMock.AssertExpectations(t) }) + + t.Run("Should handle Shared with me folder correctly", func(t *testing.T) { + ctx, k8sCliMock := setupK8sDashboardTests(service) + service.features = featuremgmt.WithFeatures(featuremgmt.FlagNestedFolders, featuremgmt.FlagKubernetesClientDashboardsFolders) + k8sCliMock.On("GetNamespace", mock.Anything, mock.Anything).Return("default") + k8sCliMock.On("Search", mock.Anything, int64(1), mock.MatchedBy(func(req *resource.ResourceSearchRequest) bool { + if len(req.Options.Fields) == 0 { + return false + } + // make sure the search request includes the shared folders + for _, field := range req.Options.Fields { + if field.Key == resource.SEARCH_FIELD_NAME { + return slices.Equal(field.Values, []string{"shared-uid1", "shared-uid2"}) + } + } + return false + })).Return(&resource.ResourceSearchResponse{ + Results: &resource.ResourceTable{ + Columns: []*resource.ResourceTableColumnDefinition{ + { + Name: "title", + Type: resource.ResourceTableColumnDefinition_STRING, + }, + { + Name: "folder", + Type: resource.ResourceTableColumnDefinition_STRING, + }, + { + Name: "tags", + Type: resource.ResourceTableColumnDefinition_STRING, + }, + }, + Rows: []*resource.ResourceTableRow{ + { + Key: &resource.ResourceKey{ + Name: "shared-uid1", + Resource: "dashboard", + }, + Cells: [][]byte{ + []byte("Shared Dashboard 1"), + []byte("f1"), + []byte("[\"shared\"]"), + }, + }, + }, + }, + TotalHits: 1, + }, nil).Once() + k8sCliMock.On("Search", mock.Anything, int64(1), mock.MatchedBy(func(req *resource.ResourceSearchRequest) bool { + if len(req.Options.Fields) == 0 { + return false + } + for _, field := range req.Options.Fields { + if field.Key == resource.SEARCH_FIELD_NAME { + return slices.Equal(field.Values, []string{"shared-uid1"}) + } + } + return false + })).Return(&resource.ResourceSearchResponse{ + Results: &resource.ResourceTable{ + Columns: []*resource.ResourceTableColumnDefinition{ + { + Name: "title", + Type: resource.ResourceTableColumnDefinition_STRING, + }, + { + Name: "folder", + Type: resource.ResourceTableColumnDefinition_STRING, + }, + { + Name: "tags", + Type: resource.ResourceTableColumnDefinition_STRING, + }, + }, + Rows: []*resource.ResourceTableRow{ + { + Key: &resource.ResourceKey{ + Name: "shared-uid1", + Resource: "dashboard", + }, + Cells: [][]byte{ + []byte("Shared Dashboard 1"), + []byte("f1"), + []byte("[\"shared\"]"), + }, + }, + }, + }, + TotalHits: 1, + }, nil) + fakeFolders.ExpectedFolders = []*folder.Folder{} + query := dashboards.FindPersistedDashboardsQuery{ + FolderUIDs: []string{folder.SharedWithMeFolderUID}, + SignedInUser: &user.SignedInUser{OrgID: 1, Permissions: map[int64]map[string][]string{1: {dashboards.ActionDashboardsRead: {dashboards.ScopeDashboardsPrefix + "shared-uid1", dashboards.ScopeDashboardsPrefix + "shared-uid2"}}}}, + } + + result, err := service.SearchDashboards(ctx, &query) + require.NoError(t, err) + require.Len(t, result, 1) + require.Equal(t, "shared-uid1", result[0].UID) + require.Equal(t, "Shared Dashboard 1", result[0].Title) + k8sCliMock.AssertExpectations(t) + }) } func TestGetDashboards(t *testing.T) { @@ -1815,8 +1921,9 @@ func TestGetDashboardUIDByID(t *testing.T) { } expectedResult := &dashboards.DashboardRef{ - UID: "uid1", - Slug: "dashboard-1", + UID: "uid1", + Slug: "dashboard-1", + FolderUID: "folder1", } query := &dashboards.GetDashboardRefByIDQuery{ ID: 1, @@ -1977,6 +2084,119 @@ func TestGetDashboardTags(t *testing.T) { }) } +func TestGetDashboardsSharedWithUser(t *testing.T) { + fakeStore := dashboards.FakeDashboardStore{} + defer fakeStore.AssertExpectations(t) + service := &DashboardServiceImpl{ + cfg: setting.NewCfg(), + dashboardStore: &fakeStore, + folderService: &foldertest.FakeService{}, + } + + user := &user.SignedInUser{ + OrgID: 1, + Permissions: map[int64]map[string][]string{ + 1: { + dashboards.ActionDashboardsRead: { + dashboards.ScopeDashboardsPrefix + "dashboard1", + dashboards.ScopeDashboardsPrefix + "dashboard2", + }, + }, + }, + } + + expectedDashboards := []*dashboards.Dashboard{ + { + UID: "dashboard1", + Slug: "dashboard-1", + FolderUID: "folder1", + }, + { + UID: "dashboard2", + Slug: "dashboard-2", + FolderUID: "folder2", + }, + } + + expectedFolderRefs := []*dashboards.DashboardRef{ + { + UID: "dashboard1", + Slug: "dashboard-1", + FolderUID: "folder1", + }, + { + UID: "dashboard2", + Slug: "dashboard-2", + FolderUID: "folder2", + }, + } + + t.Run("Should fallback to dashboard store if Kubernetes feature flags are not enabled", func(t *testing.T) { + service.features = featuremgmt.WithFeatures() + fakeStore.On("GetDashboards", mock.Anything, &dashboards.GetDashboardsQuery{ + DashboardUIDs: []string{"dashboard1", "dashboard2"}, + OrgID: 1, + }).Return(expectedDashboards, nil).Once() + + result, err := service.GetDashboardsSharedWithUser(context.Background(), user) + require.NoError(t, err) + require.Equal(t, expectedFolderRefs, result) + fakeStore.AssertExpectations(t) + }) + + t.Run("Should use Kubernetes client if feature flags are enabled", func(t *testing.T) { + ctx, k8sCliMock := setupK8sDashboardTests(service) + service.features = featuremgmt.WithFeatures(featuremgmt.FlagKubernetesClientDashboardsFolders) + + k8sCliMock.On("GetNamespace", mock.Anything, mock.Anything).Return("default") + k8sCliMock.On("Search", mock.Anything, int64(1), mock.MatchedBy(func(req *resource.ResourceSearchRequest) bool { + return req.Options.Fields[0].Key == "name" && + slices.Equal(req.Options.Fields[0].Values, []string{"dashboard1", "dashboard2"}) + })).Return(&resource.ResourceSearchResponse{ + Results: &resource.ResourceTable{ + Columns: []*resource.ResourceTableColumnDefinition{ + { + Name: "title", + Type: resource.ResourceTableColumnDefinition_STRING, + }, + { + Name: "folder", + Type: resource.ResourceTableColumnDefinition_STRING, + }, + }, + Rows: []*resource.ResourceTableRow{ + { + Key: &resource.ResourceKey{ + Name: "dashboard1", + Resource: "dashboard", + }, + Cells: [][]byte{ + []byte("Dashboard 1"), + []byte("folder1"), + }, + }, + { + Key: &resource.ResourceKey{ + Name: "dashboard2", + Resource: "dashboard", + }, + Cells: [][]byte{ + []byte("Dashboard 2"), + []byte("folder2"), + }, + }, + }, + }, + TotalHits: 2, + }, nil) + + result, err := service.GetDashboardsSharedWithUser(ctx, user) + require.NoError(t, err) + require.Equal(t, expectedFolderRefs, result) + k8sCliMock.AssertExpectations(t) + }) +} + func TestQuotaCount(t *testing.T) { fakeStore := dashboards.FakeDashboardStore{} defer fakeStore.AssertExpectations(t)