diff --git a/pkg/api/common_test.go b/pkg/api/common_test.go index 22e9546a806..79abfa8845a 100644 --- a/pkg/api/common_test.go +++ b/pkg/api/common_test.go @@ -378,7 +378,7 @@ func setupHTTPServerWithCfgDb(t *testing.T, useFakeAccessControl, enableAccessCo RouteRegister: routeRegister, SQLStore: store, searchUsersService: searchusers.ProvideUsersService(db, filters.ProvideOSSSearchUserFilter()), - dashboardService: dashboardservice.ProvideDashboardService(dashboardsStore, nil), + dashboardService: dashboardservice.ProvideDashboardService(cfg, dashboardsStore, nil, features, accesscontrolmock.NewPermissionsServicesMock()), } // Defining the accesscontrol service has to be done before registering routes diff --git a/pkg/api/dashboard.go b/pkg/api/dashboard.go index 95354c42889..17e1e4b49c9 100644 --- a/pkg/api/dashboard.go +++ b/pkg/api/dashboard.go @@ -17,10 +17,8 @@ import ( "github.com/grafana/grafana/pkg/components/simplejson" "github.com/grafana/grafana/pkg/infra/metrics" "github.com/grafana/grafana/pkg/models" - "github.com/grafana/grafana/pkg/services/accesscontrol" "github.com/grafana/grafana/pkg/services/alerting" "github.com/grafana/grafana/pkg/services/dashboards" - "github.com/grafana/grafana/pkg/services/featuremgmt" "github.com/grafana/grafana/pkg/services/guardian" "github.com/grafana/grafana/pkg/util" "github.com/grafana/grafana/pkg/web" @@ -360,12 +358,6 @@ func (hs *HTTPServer) postDashboard(c *models.ReqContext, cmd models.SaveDashboa return apierrors.ToDashboardErrorResponse(ctx, hs.pluginStore, err) } - if newDashboard { - if err := hs.setDashboardPermissions(c, cmd, dashboard); err != nil { - hs.log.Error("Could not make user admin", "dashboard", dashboard.Title, "user", c.SignedInUser.UserId, "error", err) - } - } - // connect library panels for this dashboard after the dashboard is stored and has an ID err = hs.LibraryPanelService.ConnectLibraryPanelsForDashboard(ctx, c.SignedInUser, dashboard) if err != nil { @@ -383,35 +375,6 @@ func (hs *HTTPServer) postDashboard(c *models.ReqContext, cmd models.SaveDashboa }) } -func (hs *HTTPServer) setDashboardPermissions(c *models.ReqContext, cmd models.SaveDashboardCommand, dash *models.Dashboard) error { - inFolder := dash.FolderId > 0 - if hs.Features.IsEnabled(featuremgmt.FlagAccesscontrol) { - resourceID := strconv.FormatInt(dash.Id, 10) - svc := hs.permissionServices.GetDashboardService() - - permissions := []accesscontrol.SetResourcePermissionCommand{ - {UserID: c.UserId, Permission: models.PERMISSION_ADMIN.String()}, - } - - if !inFolder { - permissions = append(permissions, []accesscontrol.SetResourcePermissionCommand{ - {BuiltinRole: string(models.ROLE_EDITOR), Permission: models.PERMISSION_EDIT.String()}, - {BuiltinRole: string(models.ROLE_VIEWER), Permission: models.PERMISSION_VIEW.String()}, - }...) - } - _, err := svc.SetPermissions(c.Req.Context(), c.OrgId, resourceID, permissions...) - if err != nil { - return err - } - } else if hs.Cfg.EditorsCanAdmin { - if err := hs.dashboardService.MakeUserAdmin(c.Req.Context(), cmd.OrgId, cmd.UserId, dash.Id, !inFolder); err != nil { - return err - } - } - - return nil -} - // GetHomeDashboard returns the home dashboard. func (hs *HTTPServer) GetHomeDashboard(c *models.ReqContext) response.Response { prefsQuery := models.GetPreferencesWithDefaultsQuery{User: c.SignedInUser} diff --git a/pkg/api/dashboard_permission_test.go b/pkg/api/dashboard_permission_test.go index 7c17b7def6b..cd2b68e373a 100644 --- a/pkg/api/dashboard_permission_test.go +++ b/pkg/api/dashboard_permission_test.go @@ -9,6 +9,7 @@ import ( "github.com/grafana/grafana/pkg/api/response" "github.com/grafana/grafana/pkg/api/routing" "github.com/grafana/grafana/pkg/models" + accesscontrolmock "github.com/grafana/grafana/pkg/services/accesscontrol/mock" "github.com/grafana/grafana/pkg/services/dashboards/database" dashboardservice "github.com/grafana/grafana/pkg/services/dashboards/manager" "github.com/grafana/grafana/pkg/services/featuremgmt" @@ -26,13 +27,16 @@ func TestDashboardPermissionAPIEndpoint(t *testing.T) { dashboardStore := &database.FakeDashboardStore{} defer dashboardStore.AssertExpectations(t) + features := featuremgmt.WithFeatures() mockSQLStore := mockstore.NewSQLStoreMock() hs := &HTTPServer{ - Cfg: settings, - dashboardService: dashboardservice.ProvideDashboardService(dashboardStore, nil), - SQLStore: mockSQLStore, - Features: featuremgmt.WithFeatures(), + Cfg: settings, + SQLStore: mockSQLStore, + Features: features, + dashboardService: dashboardservice.ProvideDashboardService( + settings, dashboardStore, nil, features, accesscontrolmock.NewPermissionsServicesMock(), + ), } t.Run("Given user has no admin permissions", func(t *testing.T) { diff --git a/pkg/api/dashboard_test.go b/pkg/api/dashboard_test.go index 7344604a315..f38d8048ba2 100644 --- a/pkg/api/dashboard_test.go +++ b/pkg/api/dashboard_test.go @@ -16,6 +16,7 @@ import ( "github.com/grafana/grafana/pkg/components/simplejson" "github.com/grafana/grafana/pkg/infra/usagestats" "github.com/grafana/grafana/pkg/models" + accesscontrolmock "github.com/grafana/grafana/pkg/services/accesscontrol/mock" "github.com/grafana/grafana/pkg/services/alerting" "github.com/grafana/grafana/pkg/services/dashboards" "github.com/grafana/grafana/pkg/services/dashboards/database" @@ -214,14 +215,18 @@ func TestDashboardAPIEndpoint(t *testing.T) { mockSQLStore := mockstore.NewSQLStoreMock() mockSQLStore.ExpectedDashboard = fakeDash + cfg := setting.NewCfg() + features := featuremgmt.WithFeatures() dashboardStore := database.ProvideDashboardStore(sqlstore.InitTestDB(t)) hs := &HTTPServer{ - Cfg: setting.NewCfg(), + Cfg: cfg, Live: newTestLive(t), LibraryPanelService: &mockLibraryPanelService{}, LibraryElementService: &mockLibraryElementService{}, - dashboardService: service.ProvideDashboardService(dashboardStore, nil), SQLStore: mockSQLStore, + dashboardService: service.ProvideDashboardService( + cfg, dashboardStore, nil, features, accesscontrolmock.NewPermissionsServicesMock(), + ), } hs.SQLStore = mockSQLStore @@ -934,14 +939,18 @@ func getDashboardShouldReturn200WithConfig(t *testing.T, sc *scenarioContext, pr libraryPanelsService := mockLibraryPanelService{} libraryElementsService := mockLibraryElementService{} + cfg := setting.NewCfg() + features := featuremgmt.WithFeatures() hs := &HTTPServer{ - Cfg: setting.NewCfg(), - LibraryPanelService: &libraryPanelsService, - LibraryElementService: &libraryElementsService, - ProvisioningService: provisioningService, - dashboardProvisioningService: service.ProvideDashboardService(dashboardStore, nil), - SQLStore: sc.sqlStore, + Cfg: cfg, + LibraryPanelService: &libraryPanelsService, + LibraryElementService: &libraryElementsService, + SQLStore: sc.sqlStore, + ProvisioningService: provisioningService, + dashboardProvisioningService: service.ProvideDashboardService( + cfg, dashboardStore, nil, features, accesscontrolmock.NewPermissionsServicesMock(), + ), } hs.callGetDashboard(sc) diff --git a/pkg/api/folder.go b/pkg/api/folder.go index 8ab9d95390a..faee784f8a8 100644 --- a/pkg/api/folder.go +++ b/pkg/api/folder.go @@ -11,8 +11,6 @@ import ( "github.com/grafana/grafana/pkg/api/dtos" "github.com/grafana/grafana/pkg/api/response" "github.com/grafana/grafana/pkg/models" - "github.com/grafana/grafana/pkg/services/accesscontrol" - "github.com/grafana/grafana/pkg/services/featuremgmt" "github.com/grafana/grafana/pkg/services/guardian" "github.com/grafana/grafana/pkg/services/libraryelements" "github.com/grafana/grafana/pkg/util" @@ -73,36 +71,10 @@ func (hs *HTTPServer) CreateFolder(c *models.ReqContext) response.Response { return apierrors.ToFolderErrorResponse(err) } - if err := hs.setFolderPermission(c, folder.Id); err != nil { - hs.log.Error("Could not make user admin", "folder", folder.Title, "user", - c.SignedInUser.UserId, "error", err) - } - g := guardian.New(c.Req.Context(), folder.Id, c.OrgId, c.SignedInUser) return response.JSON(200, hs.toFolderDto(c.Req.Context(), g, folder)) } -func (hs *HTTPServer) setFolderPermission(c *models.ReqContext, folderID int64) error { - if hs.Features.IsEnabled(featuremgmt.FlagAccesscontrol) { - resourceID := strconv.FormatInt(folderID, 10) - svc := hs.permissionServices.GetFolderService() - - _, err := svc.SetPermissions(c.Req.Context(), c.OrgId, resourceID, []accesscontrol.SetResourcePermissionCommand{ - {UserID: c.UserId, Permission: models.PERMISSION_ADMIN.String()}, - {BuiltinRole: string(models.ROLE_EDITOR), Permission: models.PERMISSION_EDIT.String()}, - {BuiltinRole: string(models.ROLE_VIEWER), Permission: models.PERMISSION_VIEW.String()}, - }...) - if err != nil { - return err - } - } else if hs.Cfg.EditorsCanAdmin { - if err := hs.folderService.MakeUserAdmin(c.Req.Context(), c.OrgId, c.UserId, folderID, true); err != nil { - return err - } - } - return nil -} - func (hs *HTTPServer) UpdateFolder(c *models.ReqContext) response.Response { cmd := models.UpdateFolderCommand{} if err := web.Bind(c.Req, &cmd); err != nil { diff --git a/pkg/api/folder_permission_test.go b/pkg/api/folder_permission_test.go index 5538bc8c693..921c751b8ba 100644 --- a/pkg/api/folder_permission_test.go +++ b/pkg/api/folder_permission_test.go @@ -5,6 +5,8 @@ import ( "fmt" "testing" + accesscontrolmock "github.com/grafana/grafana/pkg/services/accesscontrol/mock" + "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" @@ -32,7 +34,18 @@ func TestFolderPermissionAPIEndpoint(t *testing.T) { dashboardStore := &database.FakeDashboardStore{} defer dashboardStore.AssertExpectations(t) - hs := &HTTPServer{Cfg: settings, folderService: folderService, dashboardService: service.ProvideDashboardService(dashboardStore, nil), Features: featuremgmt.WithFeatures()} + features := featuremgmt.WithFeatures() + permissionsServices := accesscontrolmock.NewPermissionsServicesMock() + + hs := &HTTPServer{ + Cfg: settings, + Features: features, + folderService: folderService, + permissionServices: permissionsServices, + dashboardService: service.ProvideDashboardService( + settings, dashboardStore, nil, features, permissionsServices, + ), + } t.Run("Given folder not exists", func(t *testing.T) { folderService.On("GetFolderByUID", mock.Anything, mock.Anything, mock.Anything, mock.Anything).Return(nil, models.ErrFolderNotFound).Twice() diff --git a/pkg/services/dashboardimport/service/service.go b/pkg/services/dashboardimport/service/service.go index b1c10d73fd1..cceaaef84a0 100644 --- a/pkg/services/dashboardimport/service/service.go +++ b/pkg/services/dashboardimport/service/service.go @@ -2,7 +2,6 @@ package service import ( "context" - "strconv" "github.com/grafana/grafana/pkg/api/routing" "github.com/grafana/grafana/pkg/models" @@ -12,7 +11,6 @@ import ( "github.com/grafana/grafana/pkg/services/dashboardimport/api" "github.com/grafana/grafana/pkg/services/dashboardimport/utils" "github.com/grafana/grafana/pkg/services/dashboards" - "github.com/grafana/grafana/pkg/services/featuremgmt" "github.com/grafana/grafana/pkg/services/librarypanels" "github.com/grafana/grafana/pkg/services/quota" "github.com/grafana/grafana/pkg/services/schemaloader" @@ -22,14 +20,12 @@ func ProvideService(routeRegister routing.RouteRegister, quotaService *quota.QuotaService, schemaLoaderService *schemaloader.SchemaLoaderService, pluginDashboardManager plugins.PluginDashboardManager, pluginStore plugins.Store, libraryPanelService librarypanels.Service, dashboardService dashboards.DashboardService, - ac accesscontrol.AccessControl, permissionsServices accesscontrol.PermissionsServices, features featuremgmt.FeatureToggles, + ac accesscontrol.AccessControl, ) *ImportDashboardService { s := &ImportDashboardService{ - features: features, - pluginDashboardManager: pluginDashboardManager, - dashboardService: dashboardService, - libraryPanelService: libraryPanelService, - dashboardPermissionsService: permissionsServices.GetDashboardService(), + pluginDashboardManager: pluginDashboardManager, + dashboardService: dashboardService, + libraryPanelService: libraryPanelService, } dashboardImportAPI := api.New(s, quotaService, schemaLoaderService, pluginStore, ac) @@ -39,11 +35,9 @@ func ProvideService(routeRegister routing.RouteRegister, } type ImportDashboardService struct { - features featuremgmt.FeatureToggles - pluginDashboardManager plugins.PluginDashboardManager - dashboardService dashboards.DashboardService - libraryPanelService librarypanels.Service - dashboardPermissionsService accesscontrol.PermissionsService + pluginDashboardManager plugins.PluginDashboardManager + dashboardService dashboards.DashboardService + libraryPanelService librarypanels.Service } func (s *ImportDashboardService) ImportDashboard(ctx context.Context, req *dashboardimport.ImportDashboardRequest) (*dashboardimport.ImportDashboardResponse, error) { @@ -94,12 +88,6 @@ func (s *ImportDashboardService) ImportDashboard(ctx context.Context, req *dashb return nil, err } - if s.features.IsEnabled(featuremgmt.FlagAccesscontrol) { - if err := s.setDashboardPermissions(ctx, req.User, savedDash); err != nil { - return nil, err - } - } - return &dashboardimport.ImportDashboardResponse{ UID: savedDash.Uid, PluginId: req.PluginId, @@ -115,24 +103,3 @@ func (s *ImportDashboardService) ImportDashboard(ctx context.Context, req *dashb Slug: savedDash.Slug, }, nil } - -func (s *ImportDashboardService) setDashboardPermissions(ctx context.Context, user *models.SignedInUser, dashboard *models.Dashboard) error { - resourceID := strconv.FormatInt(dashboard.Id, 10) - - permissions := []accesscontrol.SetResourcePermissionCommand{ - {UserID: user.UserId, Permission: models.PERMISSION_ADMIN.String()}, - } - - if dashboard.FolderId == 0 { - permissions = append(permissions, []accesscontrol.SetResourcePermissionCommand{ - {BuiltinRole: string(models.ROLE_EDITOR), Permission: models.PERMISSION_EDIT.String()}, - {BuiltinRole: string(models.ROLE_VIEWER), Permission: models.PERMISSION_VIEW.String()}, - }...) - } - _, err := s.dashboardPermissionsService.SetPermissions(ctx, user.OrgId, resourceID, permissions...) - if err != nil { - return err - } - - return nil -} diff --git a/pkg/services/dashboardimport/service/service_test.go b/pkg/services/dashboardimport/service/service_test.go index e168c1d1fe3..e72a80e682a 100644 --- a/pkg/services/dashboardimport/service/service_test.go +++ b/pkg/services/dashboardimport/service/service_test.go @@ -11,7 +11,6 @@ import ( "github.com/grafana/grafana/pkg/plugins" "github.com/grafana/grafana/pkg/services/dashboardimport" "github.com/grafana/grafana/pkg/services/dashboards" - "github.com/grafana/grafana/pkg/services/featuremgmt" "github.com/grafana/grafana/pkg/services/librarypanels" "github.com/stretchr/testify/require" ) @@ -56,7 +55,6 @@ func TestImportDashboardService(t *testing.T) { pluginDashboardManager: pluginDashboardManager, dashboardService: dashboardService, libraryPanelService: libraryPanelService, - features: featuremgmt.WithFeatures(), } req := &dashboardimport.ImportDashboardRequest{ @@ -106,7 +104,6 @@ func TestImportDashboardService(t *testing.T) { } libraryPanelService := &libraryPanelServiceMock{} s := &ImportDashboardService{ - features: featuremgmt.WithFeatures(), dashboardService: dashboardService, libraryPanelService: libraryPanelService, } diff --git a/pkg/services/dashboards/manager/dashboard_service.go b/pkg/services/dashboards/manager/dashboard_service.go index e4a3d380250..8ae02b0e728 100644 --- a/pkg/services/dashboards/manager/dashboard_service.go +++ b/pkg/services/dashboards/manager/dashboard_service.go @@ -3,6 +3,7 @@ package service import ( "context" "fmt" + "strconv" "strings" "time" @@ -14,6 +15,7 @@ import ( "github.com/grafana/grafana/pkg/services/accesscontrol" "github.com/grafana/grafana/pkg/services/alerting" m "github.com/grafana/grafana/pkg/services/dashboards" + "github.com/grafana/grafana/pkg/services/featuremgmt" "github.com/grafana/grafana/pkg/services/guardian" "github.com/grafana/grafana/pkg/setting" "github.com/grafana/grafana/pkg/util" @@ -30,16 +32,27 @@ var ( ) type DashboardServiceImpl struct { - dashboardStore m.Store - dashAlertExtractor alerting.DashAlertExtractor - log log.Logger + cfg *setting.Cfg + log log.Logger + dashboardStore m.Store + dashAlertExtractor alerting.DashAlertExtractor + features featuremgmt.FeatureToggles + folderPermissions accesscontrol.PermissionsService + dashboardPermissions accesscontrol.PermissionsService } -func ProvideDashboardService(store m.Store, dashAlertExtractor alerting.DashAlertExtractor) *DashboardServiceImpl { +func ProvideDashboardService( + cfg *setting.Cfg, store m.Store, dashAlertExtractor alerting.DashAlertExtractor, + features featuremgmt.FeatureToggles, permissionsServices accesscontrol.PermissionsServices, +) *DashboardServiceImpl { return &DashboardServiceImpl{ - dashboardStore: store, - dashAlertExtractor: dashAlertExtractor, - log: log.New("dashboard-service"), + cfg: cfg, + log: log.New("dashboard-service"), + dashboardStore: store, + dashAlertExtractor: dashAlertExtractor, + features: features, + folderPermissions: permissionsServices.GetFolderService(), + dashboardPermissions: permissionsServices.GetDashboardService(), } } @@ -234,6 +247,12 @@ func (dr *DashboardServiceImpl) SaveProvisionedDashboard(ctx context.Context, dt return nil, err } + if dto.Dashboard.Id == 0 { + if err := dr.setDefaultPermissions(ctx, dto, dash, true); err != nil { + dr.log.Error("Could not make user admin", "dashboard", dash.Title, "user", dto.User.UserId, "error", err) + } + } + return dash, nil } @@ -269,6 +288,12 @@ func (dr *DashboardServiceImpl) SaveFolderForProvisionedDashboards(ctx context.C return nil, err } + if dto.Dashboard.Id == 0 { + if err := dr.setDefaultPermissions(ctx, dto, dash, true); err != nil { + dr.log.Error("Could not make user admin", "dashboard", dash.Title, "user", dto.User.UserId, "error", err) + } + } + return dash, nil } @@ -307,6 +332,13 @@ func (dr *DashboardServiceImpl) SaveDashboard(ctx context.Context, dto *m.SaveDa return nil, err } + // new dashboard created + if dto.Dashboard.Id == 0 { + if err := dr.setDefaultPermissions(ctx, dto, dash, false); err != nil { + dr.log.Error("Could not make user admin", "dashboard", dash.Title, "user", dto.User.UserId, "error", err) + } + } + return dash, nil } @@ -398,6 +430,10 @@ func (dr *DashboardServiceImpl) ImportDashboard(ctx context.Context, dto *m.Save return nil, err } + if err := dr.setDefaultPermissions(ctx, dto, dash, false); err != nil { + dr.log.Error("Could not make user admin", "dashboard", dash.Title, "user", dto.User.UserId, "error", err) + } + return dash, nil } @@ -406,3 +442,39 @@ func (dr *DashboardServiceImpl) ImportDashboard(ctx context.Context, dto *m.Save func (dr *DashboardServiceImpl) UnprovisionDashboard(ctx context.Context, dashboardId int64) error { return dr.dashboardStore.UnprovisionDashboard(ctx, dashboardId) } + +func (dr *DashboardServiceImpl) setDefaultPermissions(ctx context.Context, dto *m.SaveDashboardDTO, dash *models.Dashboard, provisioned bool) error { + inFolder := dash.FolderId > 0 + if dr.features.IsEnabled(featuremgmt.FlagAccesscontrol) { + resourceID := strconv.FormatInt(dash.Id, 10) + var permissions []accesscontrol.SetResourcePermissionCommand + if !provisioned { + permissions = append(permissions, accesscontrol.SetResourcePermissionCommand{ + UserID: dto.User.UserId, Permission: models.PERMISSION_ADMIN.String(), + }) + } + + if !inFolder { + permissions = append(permissions, []accesscontrol.SetResourcePermissionCommand{ + {BuiltinRole: string(models.ROLE_EDITOR), Permission: models.PERMISSION_EDIT.String()}, + {BuiltinRole: string(models.ROLE_VIEWER), Permission: models.PERMISSION_VIEW.String()}, + }...) + } + + svc := dr.dashboardPermissions + if dash.IsFolder { + svc = dr.folderPermissions + } + + _, err := svc.SetPermissions(ctx, dto.OrgId, resourceID, permissions...) + if err != nil { + return err + } + } else if dr.cfg.EditorsCanAdmin && !provisioned { + if err := dr.MakeUserAdmin(ctx, dto.OrgId, dto.User.UserId, dash.Id, !inFolder); err != nil { + return err + } + } + + return nil +} diff --git a/pkg/services/dashboards/manager/dashboard_service_integration_test.go b/pkg/services/dashboards/manager/dashboard_service_integration_test.go index 713b56aedd5..4bee3f2a03a 100644 --- a/pkg/services/dashboards/manager/dashboard_service_integration_test.go +++ b/pkg/services/dashboards/manager/dashboard_service_integration_test.go @@ -5,15 +5,18 @@ package service import ( "context" - "github.com/grafana/grafana/pkg/services/alerting" "testing" "github.com/grafana/grafana/pkg/components/simplejson" "github.com/grafana/grafana/pkg/models" + accesscontrolmock "github.com/grafana/grafana/pkg/services/accesscontrol/mock" + "github.com/grafana/grafana/pkg/services/alerting" dashbboardservice "github.com/grafana/grafana/pkg/services/dashboards" "github.com/grafana/grafana/pkg/services/dashboards/database" + "github.com/grafana/grafana/pkg/services/featuremgmt" "github.com/grafana/grafana/pkg/services/guardian" "github.com/grafana/grafana/pkg/services/sqlstore" + "github.com/grafana/grafana/pkg/setting" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" ) @@ -854,7 +857,11 @@ func callSaveWithResult(t *testing.T, cmd models.SaveDashboardCommand, sqlStore dto := toSaveDashboardDto(cmd) dashboardStore := database.ProvideDashboardStore(sqlStore) - res, err := ProvideDashboardService(dashboardStore, &dummyDashAlertExtractor{}).SaveDashboard(context.Background(), &dto, false) + service := ProvideDashboardService( + setting.NewCfg(), dashboardStore, &dummyDashAlertExtractor{}, + featuremgmt.WithFeatures(), accesscontrolmock.NewPermissionsServicesMock(), + ) + res, err := service.SaveDashboard(context.Background(), &dto, false) require.NoError(t, err) return res @@ -863,7 +870,11 @@ func callSaveWithResult(t *testing.T, cmd models.SaveDashboardCommand, sqlStore func callSaveWithError(cmd models.SaveDashboardCommand, sqlStore *sqlstore.SQLStore) error { dto := toSaveDashboardDto(cmd) dashboardStore := database.ProvideDashboardStore(sqlStore) - _, err := ProvideDashboardService(dashboardStore, &dummyDashAlertExtractor{}).SaveDashboard(context.Background(), &dto, false) + service := ProvideDashboardService( + setting.NewCfg(), dashboardStore, &dummyDashAlertExtractor{}, + featuremgmt.WithFeatures(), accesscontrolmock.NewPermissionsServicesMock(), + ) + _, err := service.SaveDashboard(context.Background(), &dto, false) return err } @@ -890,7 +901,11 @@ func saveTestDashboard(t *testing.T, title string, orgID, folderID int64, sqlSto } dashboardStore := database.ProvideDashboardStore(sqlStore) - res, err := ProvideDashboardService(dashboardStore, &dummyDashAlertExtractor{}).SaveDashboard(context.Background(), &dto, false) + service := ProvideDashboardService( + setting.NewCfg(), dashboardStore, &dummyDashAlertExtractor{}, + featuremgmt.WithFeatures(), accesscontrolmock.NewPermissionsServicesMock(), + ) + res, err := service.SaveDashboard(context.Background(), &dto, false) require.NoError(t, err) return res @@ -918,7 +933,11 @@ func saveTestFolder(t *testing.T, title string, orgID int64, sqlStore *sqlstore. } dashboardStore := database.ProvideDashboardStore(sqlStore) - res, err := ProvideDashboardService(dashboardStore, &dummyDashAlertExtractor{}).SaveDashboard(context.Background(), &dto, false) + service := ProvideDashboardService( + setting.NewCfg(), dashboardStore, &dummyDashAlertExtractor{}, + featuremgmt.WithFeatures(), accesscontrolmock.NewPermissionsServicesMock(), + ) + res, err := service.SaveDashboard(context.Background(), &dto, false) require.NoError(t, err) return res diff --git a/pkg/services/dashboards/manager/folder_service.go b/pkg/services/dashboards/manager/folder_service.go index b1424176336..87683f11345 100644 --- a/pkg/services/dashboards/manager/folder_service.go +++ b/pkg/services/dashboards/manager/folder_service.go @@ -3,29 +3,43 @@ package service import ( "context" "errors" + "strconv" "strings" + "github.com/grafana/grafana/pkg/services/featuremgmt" + "github.com/grafana/grafana/pkg/bus" "github.com/grafana/grafana/pkg/infra/log" "github.com/grafana/grafana/pkg/models" + "github.com/grafana/grafana/pkg/services/accesscontrol" "github.com/grafana/grafana/pkg/services/dashboards" "github.com/grafana/grafana/pkg/services/guardian" "github.com/grafana/grafana/pkg/services/search" + "github.com/grafana/grafana/pkg/setting" ) type FolderServiceImpl struct { + log log.Logger + cfg *setting.Cfg dashboardService dashboards.DashboardService dashboardStore dashboards.Store searchService *search.SearchService - log log.Logger + features featuremgmt.FeatureToggles + permissions accesscontrol.PermissionsService } -func ProvideFolderService(dashboardService dashboards.DashboardService, dashboardStore dashboards.Store, searchService *search.SearchService) *FolderServiceImpl { +func ProvideFolderService( + cfg *setting.Cfg, dashboardService dashboards.DashboardService, dashboardStore dashboards.Store, + searchService *search.SearchService, features featuremgmt.FeatureToggles, permissionsServices accesscontrol.PermissionsServices, +) *FolderServiceImpl { return &FolderServiceImpl{ + cfg: cfg, + log: log.New("folder-service"), dashboardService: dashboardService, dashboardStore: dashboardStore, searchService: searchService, - log: log.New("folder-service"), + features: features, + permissions: permissionsServices.GetFolderService(), } } @@ -149,6 +163,22 @@ func (f *FolderServiceImpl) CreateFolder(ctx context.Context, user *models.Signe return nil, toFolderError(err) } + var permissionErr error + if f.features.IsEnabled(featuremgmt.FlagAccesscontrol) { + resourceID := strconv.FormatInt(dashFolder.Id, 10) + _, permissionErr = f.permissions.SetPermissions(ctx, orgID, resourceID, []accesscontrol.SetResourcePermissionCommand{ + {UserID: userID, Permission: models.PERMISSION_ADMIN.String()}, + {BuiltinRole: string(models.ROLE_EDITOR), Permission: models.PERMISSION_EDIT.String()}, + {BuiltinRole: string(models.ROLE_VIEWER), Permission: models.PERMISSION_VIEW.String()}, + }...) + } else if f.cfg.EditorsCanAdmin { + permissionErr = f.MakeUserAdmin(ctx, orgID, userID, dashFolder.Id, true) + } + + if permissionErr != nil { + f.log.Error("Could not make user admin", "folder", dashFolder.Title, "user", userID, "error", permissionErr) + } + return dashToFolder(dashFolder), nil } diff --git a/pkg/services/dashboards/manager/folder_service_test.go b/pkg/services/dashboards/manager/folder_service_test.go index 9b199959694..d462fad6889 100644 --- a/pkg/services/dashboards/manager/folder_service_test.go +++ b/pkg/services/dashboards/manager/folder_service_test.go @@ -9,9 +9,12 @@ import ( "github.com/grafana/grafana/pkg/bus" "github.com/grafana/grafana/pkg/models" + acmock "github.com/grafana/grafana/pkg/services/accesscontrol/mock" "github.com/grafana/grafana/pkg/services/dashboards" "github.com/grafana/grafana/pkg/services/dashboards/database" + "github.com/grafana/grafana/pkg/services/featuremgmt" "github.com/grafana/grafana/pkg/services/guardian" + "github.com/grafana/grafana/pkg/setting" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/mock" "github.com/stretchr/testify/require" @@ -24,10 +27,13 @@ func TestFolderService(t *testing.T) { t.Run("Folder service tests", func(t *testing.T) { store := &database.FakeDashboardStore{} defer store.AssertExpectations(t) + cfg := setting.NewCfg() + features := featuremgmt.WithFeatures() + permissionsServices := acmock.NewPermissionsServicesMock() + dashboardService := ProvideDashboardService(cfg, store, nil, features, permissionsServices) service := ProvideFolderService( - &dashboards.FakeDashboardService{DashboardService: ProvideDashboardService(store, nil)}, - store, - nil, + cfg, &dashboards.FakeDashboardService{DashboardService: dashboardService}, + store, nil, features, permissionsServices, ) t.Run("Given user has no permissions", func(t *testing.T) { diff --git a/pkg/services/libraryelements/libraryelements_test.go b/pkg/services/libraryelements/libraryelements_test.go index 273b90ad2d8..d13c0df3ece 100644 --- a/pkg/services/libraryelements/libraryelements_test.go +++ b/pkg/services/libraryelements/libraryelements_test.go @@ -13,10 +13,12 @@ import ( "github.com/grafana/grafana/pkg/api/response" "github.com/grafana/grafana/pkg/components/simplejson" "github.com/grafana/grafana/pkg/models" + acmock "github.com/grafana/grafana/pkg/services/accesscontrol/mock" "github.com/grafana/grafana/pkg/services/alerting" "github.com/grafana/grafana/pkg/services/dashboards" "github.com/grafana/grafana/pkg/services/dashboards/database" dashboardservice "github.com/grafana/grafana/pkg/services/dashboards/manager" + "github.com/grafana/grafana/pkg/services/featuremgmt" "github.com/grafana/grafana/pkg/services/sqlstore" "github.com/grafana/grafana/pkg/setting" "github.com/grafana/grafana/pkg/web" @@ -197,7 +199,11 @@ func createDashboard(t *testing.T, sqlStore *sqlstore.SQLStore, user models.Sign dashboardStore := database.ProvideDashboardStore(sqlStore) dashAlertExtractor := alerting.ProvideDashAlertExtractorService(nil, nil) - dashboard, err := dashboardservice.ProvideDashboardService(dashboardStore, dashAlertExtractor).SaveDashboard(context.Background(), dashItem, true) + service := dashboardservice.ProvideDashboardService( + setting.NewCfg(), dashboardStore, dashAlertExtractor, + featuremgmt.WithFeatures(), acmock.NewPermissionsServicesMock(), + ) + dashboard, err := service.SaveDashboard(context.Background(), dashItem, true) require.NoError(t, err) return dashboard @@ -207,9 +213,19 @@ func createFolderWithACL(t *testing.T, sqlStore *sqlstore.SQLStore, title string items []folderACLItem) *models.Folder { t.Helper() + cfg := setting.NewCfg() + features := featuremgmt.WithFeatures() + permissionsServices := acmock.NewPermissionsServicesMock() dashboardStore := database.ProvideDashboardStore(sqlStore) - d := dashboardservice.ProvideDashboardService(dashboardStore, nil) - s := dashboardservice.ProvideFolderService(d, dashboardStore, nil) + + d := dashboardservice.ProvideDashboardService( + cfg, dashboardStore, nil, + features, permissionsServices, + ) + s := dashboardservice.ProvideFolderService( + cfg, d, dashboardStore, nil, + features, permissionsServices, + ) t.Logf("Creating folder with title and UID %q", title) folder, err := s.CreateFolder(context.Background(), &user, user.OrgId, title, title) require.NoError(t, err) @@ -294,11 +310,17 @@ func testScenario(t *testing.T, desc string, fn func(t *testing.T, sc scenarioCo role := models.ROLE_ADMIN sqlStore := sqlstore.InitTestDB(t) dashboardStore := database.ProvideDashboardStore(sqlStore) - dashboardService := dashboardservice.ProvideDashboardService(dashboardStore, &alerting.DashAlertExtractorService{}) + dashboardService := dashboardservice.ProvideDashboardService( + setting.NewCfg(), dashboardStore, nil, + featuremgmt.WithFeatures(), acmock.NewPermissionsServicesMock(), + ) service := LibraryElementService{ - Cfg: setting.NewCfg(), - SQLStore: sqlStore, - folderService: dashboardservice.ProvideFolderService(dashboardService, dashboardStore, nil), + Cfg: setting.NewCfg(), + SQLStore: sqlStore, + folderService: dashboardservice.ProvideFolderService( + setting.NewCfg(), dashboardService, dashboardStore, nil, + featuremgmt.WithFeatures(), acmock.NewPermissionsServicesMock(), + ), } user := models.SignedInUser{ diff --git a/pkg/services/librarypanels/librarypanels_test.go b/pkg/services/librarypanels/librarypanels_test.go index 51c5e7e957c..a41de4c9b38 100644 --- a/pkg/services/librarypanels/librarypanels_test.go +++ b/pkg/services/librarypanels/librarypanels_test.go @@ -11,10 +11,12 @@ import ( "github.com/grafana/grafana/pkg/api/routing" "github.com/grafana/grafana/pkg/components/simplejson" "github.com/grafana/grafana/pkg/models" + acmock "github.com/grafana/grafana/pkg/services/accesscontrol/mock" "github.com/grafana/grafana/pkg/services/alerting" "github.com/grafana/grafana/pkg/services/dashboards" "github.com/grafana/grafana/pkg/services/dashboards/database" dashboardservice "github.com/grafana/grafana/pkg/services/dashboards/manager" + "github.com/grafana/grafana/pkg/services/featuremgmt" "github.com/grafana/grafana/pkg/services/libraryelements" "github.com/grafana/grafana/pkg/services/sqlstore" "github.com/grafana/grafana/pkg/setting" @@ -1416,9 +1418,13 @@ func createDashboard(t *testing.T, sqlStore *sqlstore.SQLStore, user *models.Sig Overwrite: false, } - dashboadStore := database.ProvideDashboardStore(sqlStore) + dashboardStore := database.ProvideDashboardStore(sqlStore) dashAlertService := alerting.ProvideDashAlertExtractorService(nil, nil) - dashboard, err := dashboardservice.ProvideDashboardService(dashboadStore, dashAlertService).SaveDashboard(context.Background(), dashItem, true) + service := dashboardservice.ProvideDashboardService( + setting.NewCfg(), dashboardStore, dashAlertService, + featuremgmt.WithFeatures(), acmock.NewPermissionsServicesMock(), + ) + dashboard, err := service.SaveDashboard(context.Background(), dashItem, true) require.NoError(t, err) return dashboard @@ -1428,9 +1434,13 @@ func createFolderWithACL(t *testing.T, sqlStore *sqlstore.SQLStore, title string items []folderACLItem) *models.Folder { t.Helper() + cfg := setting.NewCfg() + features := featuremgmt.WithFeatures() + permissionsServices := acmock.NewPermissionsServicesMock() dashboardStore := database.ProvideDashboardStore(sqlStore) - d := dashboardservice.ProvideDashboardService(dashboardStore, nil) - s := dashboardservice.ProvideFolderService(d, dashboardStore, nil) + d := dashboardservice.ProvideDashboardService(cfg, dashboardStore, nil, features, permissionsServices) + s := dashboardservice.ProvideFolderService(cfg, d, dashboardStore, nil, features, permissionsServices) + t.Logf("Creating folder with title and UID %q", title) folder, err := s.CreateFolder(context.Background(), user, user.OrgId, title, title) require.NoError(t, err) @@ -1518,7 +1528,18 @@ func testScenario(t *testing.T, desc string, fn func(t *testing.T, sc scenarioCo role := models.ROLE_ADMIN sqlStore := sqlstore.InitTestDB(t) dashboardStore := database.ProvideDashboardStore(sqlStore) - folderService := dashboardservice.ProvideFolderService(dashboardservice.ProvideDashboardService(dashboardStore, &alerting.DashAlertExtractorService{}), dashboardStore, nil) + + features := featuremgmt.WithFeatures() + permissionsServices := acmock.NewPermissionsServicesMock() + + dashboardService := dashboardservice.ProvideDashboardService( + cfg, dashboardStore, &alerting.DashAlertExtractorService{}, + features, permissionsServices, + ) + folderService := dashboardservice.ProvideFolderService( + cfg, dashboardService, dashboardStore, nil, + features, permissionsServices, + ) elementService := libraryelements.ProvideService(cfg, sqlStore, routing.NewRouteRegister(), folderService) service := LibraryPanelService{ diff --git a/pkg/services/ngalert/tests/util.go b/pkg/services/ngalert/tests/util.go index f481c1d7548..4ab5e027433 100644 --- a/pkg/services/ngalert/tests/util.go +++ b/pkg/services/ngalert/tests/util.go @@ -9,9 +9,10 @@ import ( "github.com/grafana/grafana/pkg/api/routing" "github.com/grafana/grafana/pkg/infra/log" - "github.com/grafana/grafana/pkg/services/accesscontrol/mock" + acmock "github.com/grafana/grafana/pkg/services/accesscontrol/mock" databasestore "github.com/grafana/grafana/pkg/services/dashboards/database" dashboardservice "github.com/grafana/grafana/pkg/services/dashboards/manager" + "github.com/grafana/grafana/pkg/services/featuremgmt" "github.com/grafana/grafana/pkg/services/ngalert" "github.com/grafana/grafana/pkg/services/ngalert/metrics" "github.com/grafana/grafana/pkg/services/ngalert/models" @@ -42,8 +43,20 @@ func SetupTestEnv(t *testing.T, baseInterval time.Duration) (*ngalert.AlertNG, * sqlStore := sqlstore.InitTestDB(t) secretsService := secretsManager.SetupTestService(t, database.ProvideSecretsStore(sqlStore)) dashboardStore := databasestore.ProvideDashboardStore(sqlStore) - folderService := dashboardservice.ProvideFolderService(dashboardservice.ProvideDashboardService(dashboardStore, nil), dashboardStore, nil) - ac := mock.New() + + ac := acmock.New() + features := featuremgmt.WithFeatures() + permissionsServices := acmock.NewPermissionsServicesMock() + + dashboardService := dashboardservice.ProvideDashboardService( + cfg, dashboardStore, nil, + features, permissionsServices, + ) + folderService := dashboardservice.ProvideFolderService( + cfg, dashboardService, dashboardStore, nil, + features, permissionsServices, + ) + ng, err := ngalert.ProvideService( cfg, nil, routing.NewRouteRegister(), sqlStore, nil, nil, nil, nil, secretsService, nil, m, folderService, ac, diff --git a/pkg/services/provisioning/dashboards/dashboard.go b/pkg/services/provisioning/dashboards/dashboard.go index 07143d26452..60752f263ac 100644 --- a/pkg/services/provisioning/dashboards/dashboard.go +++ b/pkg/services/provisioning/dashboards/dashboard.go @@ -7,9 +7,7 @@ import ( "github.com/grafana/grafana/pkg/infra/log" "github.com/grafana/grafana/pkg/models" - "github.com/grafana/grafana/pkg/services/accesscontrol" "github.com/grafana/grafana/pkg/services/dashboards" - "github.com/grafana/grafana/pkg/services/featuremgmt" "github.com/grafana/grafana/pkg/services/provisioning/utils" "github.com/grafana/grafana/pkg/util/errutil" ) @@ -25,10 +23,7 @@ type DashboardProvisioner interface { } // DashboardProvisionerFactory creates DashboardProvisioners based on input -type DashboardProvisionerFactory func( - context.Context, string, dashboards.DashboardProvisioningService, utils.OrgStore, - featuremgmt.FeatureToggles, accesscontrol.PermissionsServices, -) (DashboardProvisioner, error) +type DashboardProvisionerFactory func(context.Context, string, dashboards.DashboardProvisioningService, utils.OrgStore) (DashboardProvisioner, error) // Provisioner is responsible for syncing dashboard from disk to Grafana's database. type Provisioner struct { @@ -40,10 +35,7 @@ type Provisioner struct { } // New returns a new DashboardProvisioner -func New( - ctx context.Context, configDirectory string, provisioner dashboards.DashboardProvisioningService, orgStore utils.OrgStore, - features featuremgmt.FeatureToggles, permissionsServices accesscontrol.PermissionsServices, -) (DashboardProvisioner, error) { +func New(ctx context.Context, configDirectory string, provisioner dashboards.DashboardProvisioningService, orgStore utils.OrgStore) (DashboardProvisioner, error) { logger := log.New("provisioning.dashboard") cfgReader := &configReader{path: configDirectory, log: logger, orgStore: orgStore} configs, err := cfgReader.readConfig(ctx) @@ -51,7 +43,7 @@ func New( return nil, errutil.Wrap("Failed to read dashboards config", err) } - fileReaders, err := getFileReaders(configs, logger, provisioner, features, permissionsServices) + fileReaders, err := getFileReaders(configs, logger, provisioner) if err != nil { return nil, errutil.Wrap("Failed to initialize file readers", err) } @@ -132,14 +124,13 @@ func (provider *Provisioner) GetAllowUIUpdatesFromConfig(name string) bool { func getFileReaders( configs []*config, logger log.Logger, service dashboards.DashboardProvisioningService, - features featuremgmt.FeatureToggles, permissionsServices accesscontrol.PermissionsServices, ) ([]*FileReader, error) { var readers []*FileReader for _, config := range configs { switch config.Type { case "file": - fileReader, err := NewDashboardFileReader(config, logger.New("type", config.Type, "name", config.Name), service, features, permissionsServices) + fileReader, err := NewDashboardFileReader(config, logger.New("type", config.Type, "name", config.Name), service) if err != nil { return nil, errutil.Wrapf(err, "Failed to create file reader for config %v", config.Name) } diff --git a/pkg/services/provisioning/dashboards/file_reader.go b/pkg/services/provisioning/dashboards/file_reader.go index 12bae81eb77..61c8b83e9b9 100644 --- a/pkg/services/provisioning/dashboards/file_reader.go +++ b/pkg/services/provisioning/dashboards/file_reader.go @@ -7,7 +7,6 @@ import ( "io/ioutil" "os" "path/filepath" - "strconv" "strings" "sync" "time" @@ -16,9 +15,7 @@ import ( "github.com/grafana/grafana/pkg/components/simplejson" "github.com/grafana/grafana/pkg/infra/log" "github.com/grafana/grafana/pkg/models" - "github.com/grafana/grafana/pkg/services/accesscontrol" "github.com/grafana/grafana/pkg/services/dashboards" - "github.com/grafana/grafana/pkg/services/featuremgmt" "github.com/grafana/grafana/pkg/util" ) @@ -40,15 +37,10 @@ type FileReader struct { mux sync.RWMutex usageTracker *usageTracker dbWriteAccessRestricted bool - permissionsServices accesscontrol.PermissionsServices - features featuremgmt.FeatureToggles } // NewDashboardFileReader returns a new filereader based on `config` -func NewDashboardFileReader( - cfg *config, log log.Logger, service dashboards.DashboardProvisioningService, - features featuremgmt.FeatureToggles, permissionsServices accesscontrol.PermissionsServices, -) (*FileReader, error) { +func NewDashboardFileReader(cfg *config, log log.Logger, service dashboards.DashboardProvisioningService) (*FileReader, error) { var path string path, ok := cfg.Options["path"].(string) if !ok { @@ -72,8 +64,6 @@ func NewDashboardFileReader( dashboardProvisioningService: service, FoldersFromFilesStructure: foldersFromFilesStructure, usageTracker: newUsageTracker(), - features: features, - permissionsServices: permissionsServices, }, nil } @@ -274,24 +264,10 @@ func (fr *FileReader) saveDashboard(ctx context.Context, path string, folderID i Updated: resolvedFileInfo.ModTime().Unix(), CheckSum: jsonFile.checkSum, } - savedDash, err := fr.dashboardProvisioningService.SaveProvisionedDashboard(ctx, dash, dp) + _, err := fr.dashboardProvisioningService.SaveProvisionedDashboard(ctx, dash, dp) if err != nil { return provisioningMetadata, err } - - if !alreadyProvisioned && fr.features.IsEnabled(featuremgmt.FlagAccesscontrol) { - svc := fr.permissionsServices.GetDashboardService() - _, err := svc.SetPermissions(ctx, savedDash.OrgId, strconv.FormatInt(savedDash.Id, 10), accesscontrol.SetResourcePermissionCommand{ - BuiltinRole: "Viewer", - Permission: "View", - }, accesscontrol.SetResourcePermissionCommand{ - BuiltinRole: "Editor", - Permission: "Edit", - }) - if err != nil { - fr.log.Warn("failed to set permissions for provisioned dashboard", "dashboardId", savedDash.Id, "err", err) - } - } } else { fr.log.Warn("Not saving new dashboard due to restricted database access", "provisioner", fr.Cfg.Name, "file", path, "folderId", dash.Dashboard.FolderId) @@ -341,19 +317,6 @@ func (fr *FileReader) getOrCreateFolderID(ctx context.Context, cfg *config, serv return 0, err } - if fr.features.IsEnabled(featuremgmt.FlagAccesscontrol) { - _, err = fr.permissionsServices.GetFolderService().SetPermissions(ctx, dbDash.OrgId, strconv.FormatInt(dbDash.Id, 10), accesscontrol.SetResourcePermissionCommand{ - BuiltinRole: "Viewer", - Permission: "View", - }, accesscontrol.SetResourcePermissionCommand{ - BuiltinRole: "Editor", - Permission: "Edit", - }) - if err != nil { - return 0, err - } - } - return dbDash.Id, nil } diff --git a/pkg/services/provisioning/dashboards/file_reader_linux_test.go b/pkg/services/provisioning/dashboards/file_reader_linux_test.go index 5050eefaaa1..c8f288d52f4 100644 --- a/pkg/services/provisioning/dashboards/file_reader_linux_test.go +++ b/pkg/services/provisioning/dashboards/file_reader_linux_test.go @@ -8,7 +8,6 @@ import ( "testing" "github.com/grafana/grafana/pkg/infra/log" - "github.com/grafana/grafana/pkg/services/featuremgmt" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" ) @@ -26,7 +25,7 @@ func TestProvisionedSymlinkedFolder(t *testing.T) { Options: map[string]interface{}{"path": symlinkedFolder}, } - reader, err := NewDashboardFileReader(cfg, log.New("test-logger"), nil, featuremgmt.WithFeatures(), nil) + reader, err := NewDashboardFileReader(cfg, log.New("test-logger"), nil) if err != nil { t.Error("expected err to be nil") } diff --git a/pkg/services/provisioning/dashboards/file_reader_test.go b/pkg/services/provisioning/dashboards/file_reader_test.go index eb5cb2c4e56..80124825fa6 100644 --- a/pkg/services/provisioning/dashboards/file_reader_test.go +++ b/pkg/services/provisioning/dashboards/file_reader_test.go @@ -12,7 +12,6 @@ import ( "github.com/grafana/grafana/pkg/infra/log" "github.com/grafana/grafana/pkg/models" "github.com/grafana/grafana/pkg/services/dashboards" - "github.com/grafana/grafana/pkg/services/featuremgmt" "github.com/grafana/grafana/pkg/util" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/mock" @@ -43,7 +42,7 @@ func TestCreatingNewDashboardFileReader(t *testing.T) { t.Run("using path parameter", func(t *testing.T) { cfg := setup() cfg.Options["path"] = defaultDashboards - reader, err := NewDashboardFileReader(cfg, log.New("test-logger"), nil, featuremgmt.WithFeatures(), nil) + reader, err := NewDashboardFileReader(cfg, log.New("test-logger"), nil) require.NoError(t, err) require.NotEqual(t, reader.Path, "") }) @@ -51,7 +50,7 @@ func TestCreatingNewDashboardFileReader(t *testing.T) { t.Run("using folder as options", func(t *testing.T) { cfg := setup() cfg.Options["folder"] = defaultDashboards - reader, err := NewDashboardFileReader(cfg, log.New("test-logger"), nil, featuremgmt.WithFeatures(), nil) + reader, err := NewDashboardFileReader(cfg, log.New("test-logger"), nil) require.NoError(t, err) require.NotEqual(t, reader.Path, "") }) @@ -60,7 +59,7 @@ func TestCreatingNewDashboardFileReader(t *testing.T) { cfg := setup() cfg.Options["path"] = foldersFromFilesStructure cfg.Options["foldersFromFilesStructure"] = true - reader, err := NewDashboardFileReader(cfg, log.New("test-logger"), nil, featuremgmt.WithFeatures(), nil) + reader, err := NewDashboardFileReader(cfg, log.New("test-logger"), nil) require.NoError(t, err) require.NotEqual(t, reader.Path, "") }) @@ -73,7 +72,7 @@ func TestCreatingNewDashboardFileReader(t *testing.T) { } cfg.Options["folder"] = fullPath - reader, err := NewDashboardFileReader(cfg, log.New("test-logger"), nil, featuremgmt.WithFeatures(), nil) + reader, err := NewDashboardFileReader(cfg, log.New("test-logger"), nil) require.NoError(t, err) require.Equal(t, reader.Path, fullPath) @@ -83,7 +82,7 @@ func TestCreatingNewDashboardFileReader(t *testing.T) { t.Run("using relative path", func(t *testing.T) { cfg := setup() cfg.Options["folder"] = defaultDashboards - reader, err := NewDashboardFileReader(cfg, log.New("test-logger"), nil, featuremgmt.WithFeatures(), nil) + reader, err := NewDashboardFileReader(cfg, log.New("test-logger"), nil) require.NoError(t, err) resolvedPath := reader.resolvedPath() @@ -119,7 +118,7 @@ func TestDashboardFileReader(t *testing.T) { fakeService.On("SaveFolderForProvisionedDashboards", mock.Anything, mock.Anything).Return(&models.Dashboard{Id: 1}, nil).Once() fakeService.On("SaveProvisionedDashboard", mock.Anything, mock.Anything, mock.Anything).Return(&models.Dashboard{Id: 2}, nil).Times(2) - reader, err := NewDashboardFileReader(cfg, logger, nil, featuremgmt.WithFeatures(), nil) + reader, err := NewDashboardFileReader(cfg, logger, nil) reader.dashboardProvisioningService = fakeService require.NoError(t, err) @@ -139,7 +138,7 @@ func TestDashboardFileReader(t *testing.T) { inserted++ }) - reader, err := NewDashboardFileReader(cfg, logger, nil, featuremgmt.WithFeatures(), nil) + reader, err := NewDashboardFileReader(cfg, logger, nil) reader.dashboardProvisioningService = fakeService require.NoError(t, err) @@ -176,7 +175,7 @@ func TestDashboardFileReader(t *testing.T) { fakeService.On("GetProvisionedDashboardData", configName).Return(provisionedDashboard, nil).Once() - reader, err := NewDashboardFileReader(cfg, logger, nil, featuremgmt.WithFeatures(), nil) + reader, err := NewDashboardFileReader(cfg, logger, nil) reader.dashboardProvisioningService = fakeService require.NoError(t, err) @@ -204,7 +203,7 @@ func TestDashboardFileReader(t *testing.T) { fakeService.On("GetProvisionedDashboardData", configName).Return(provisionedDashboard, nil).Once() fakeService.On("SaveProvisionedDashboard", mock.Anything, mock.Anything, mock.Anything).Return(&models.Dashboard{}, nil).Once() - reader, err := NewDashboardFileReader(cfg, logger, nil, featuremgmt.WithFeatures(), nil) + reader, err := NewDashboardFileReader(cfg, logger, nil) reader.dashboardProvisioningService = fakeService require.NoError(t, err) @@ -239,7 +238,7 @@ func TestDashboardFileReader(t *testing.T) { fakeService.On("GetProvisionedDashboardData", configName).Return(provisionedDashboard, nil).Once() - reader, err := NewDashboardFileReader(cfg, logger, nil, featuremgmt.WithFeatures(), nil) + reader, err := NewDashboardFileReader(cfg, logger, nil) reader.dashboardProvisioningService = fakeService require.NoError(t, err) @@ -267,7 +266,7 @@ func TestDashboardFileReader(t *testing.T) { fakeService.On("GetProvisionedDashboardData", configName).Return(provisionedDashboard, nil).Once() fakeService.On("SaveProvisionedDashboard", mock.Anything, mock.Anything, mock.Anything).Return(&models.Dashboard{}, nil).Once() - reader, err := NewDashboardFileReader(cfg, logger, nil, featuremgmt.WithFeatures(), nil) + reader, err := NewDashboardFileReader(cfg, logger, nil) reader.dashboardProvisioningService = fakeService require.NoError(t, err) @@ -282,7 +281,7 @@ func TestDashboardFileReader(t *testing.T) { fakeService.On("GetProvisionedDashboardData", configName).Return(nil, nil).Once() fakeService.On("SaveProvisionedDashboard", mock.Anything, mock.Anything, mock.Anything).Return(&models.Dashboard{}, nil).Once() - reader, err := NewDashboardFileReader(cfg, logger, nil, featuremgmt.WithFeatures(), nil) + reader, err := NewDashboardFileReader(cfg, logger, nil) reader.dashboardProvisioningService = fakeService require.NoError(t, err) @@ -299,7 +298,7 @@ func TestDashboardFileReader(t *testing.T) { fakeService.On("SaveFolderForProvisionedDashboards", mock.Anything, mock.Anything).Return(&models.Dashboard{}, nil).Times(2) fakeService.On("SaveProvisionedDashboard", mock.Anything, mock.Anything, mock.Anything).Return(&models.Dashboard{}, nil).Times(3) - reader, err := NewDashboardFileReader(cfg, logger, nil, featuremgmt.WithFeatures(), nil) + reader, err := NewDashboardFileReader(cfg, logger, nil) reader.dashboardProvisioningService = fakeService require.NoError(t, err) @@ -316,7 +315,7 @@ func TestDashboardFileReader(t *testing.T) { Folder: "", } - _, err := NewDashboardFileReader(cfg, logger, nil, featuremgmt.WithFeatures(), nil) + _, err := NewDashboardFileReader(cfg, logger, nil) require.NotNil(t, err) }) @@ -324,7 +323,7 @@ func TestDashboardFileReader(t *testing.T) { setup() cfg.Options["path"] = brokenDashboards - _, err := NewDashboardFileReader(cfg, logger, nil, featuremgmt.WithFeatures(), nil) + _, err := NewDashboardFileReader(cfg, logger, nil) require.NoError(t, err) }) @@ -337,14 +336,14 @@ func TestDashboardFileReader(t *testing.T) { fakeService.On("SaveFolderForProvisionedDashboards", mock.Anything, mock.Anything).Return(&models.Dashboard{}, nil).Times(2) fakeService.On("SaveProvisionedDashboard", mock.Anything, mock.Anything, mock.Anything).Return(&models.Dashboard{}, nil).Times(2) - reader1, err := NewDashboardFileReader(cfg1, logger, nil, featuremgmt.WithFeatures(), nil) + reader1, err := NewDashboardFileReader(cfg1, logger, nil) reader1.dashboardProvisioningService = fakeService require.NoError(t, err) err = reader1.walkDisk(context.Background()) require.NoError(t, err) - reader2, err := NewDashboardFileReader(cfg2, logger, nil, featuremgmt.WithFeatures(), nil) + reader2, err := NewDashboardFileReader(cfg2, logger, nil) reader2.dashboardProvisioningService = fakeService require.NoError(t, err) @@ -364,7 +363,7 @@ func TestDashboardFileReader(t *testing.T) { "folder": defaultDashboards, }, } - r, err := NewDashboardFileReader(cfg, logger, nil, featuremgmt.WithFeatures(), nil) + r, err := NewDashboardFileReader(cfg, logger, nil) require.NoError(t, err) _, err = r.getOrCreateFolderID(context.Background(), cfg, fakeService, cfg.Folder) @@ -384,7 +383,7 @@ func TestDashboardFileReader(t *testing.T) { } fakeService.On("SaveFolderForProvisionedDashboards", mock.Anything, mock.Anything).Return(&models.Dashboard{Id: 1}, nil).Once() - r, err := NewDashboardFileReader(cfg, logger, nil, featuremgmt.WithFeatures(), nil) + r, err := NewDashboardFileReader(cfg, logger, nil) require.NoError(t, err) _, err = r.getOrCreateFolderID(context.Background(), cfg, fakeService, cfg.Folder) @@ -439,7 +438,7 @@ func TestDashboardFileReader(t *testing.T) { cfg.DisableDeletion = true - reader, err := NewDashboardFileReader(cfg, logger, nil, featuremgmt.WithFeatures(), nil) + reader, err := NewDashboardFileReader(cfg, logger, nil) reader.dashboardProvisioningService = fakeService require.NoError(t, err) @@ -454,7 +453,7 @@ func TestDashboardFileReader(t *testing.T) { fakeService.On("SaveProvisionedDashboard", mock.Anything, mock.Anything, mock.Anything).Return(&models.Dashboard{}, nil).Once() fakeService.On("DeleteProvisionedDashboard", mock.Anything, mock.Anything, mock.Anything).Return(nil).Once() - reader, err := NewDashboardFileReader(cfg, logger, nil, featuremgmt.WithFeatures(), nil) + reader, err := NewDashboardFileReader(cfg, logger, nil) reader.dashboardProvisioningService = fakeService require.NoError(t, err) diff --git a/pkg/services/provisioning/dashboards/validator_test.go b/pkg/services/provisioning/dashboards/validator_test.go index eea41e13fe1..1af1e7c05ff 100644 --- a/pkg/services/provisioning/dashboards/validator_test.go +++ b/pkg/services/provisioning/dashboards/validator_test.go @@ -9,7 +9,6 @@ import ( "github.com/grafana/grafana/pkg/infra/log" "github.com/grafana/grafana/pkg/models" "github.com/grafana/grafana/pkg/services/dashboards" - "github.com/grafana/grafana/pkg/services/featuremgmt" "github.com/stretchr/testify/mock" "github.com/stretchr/testify/require" ) @@ -37,7 +36,7 @@ func TestDuplicatesValidator(t *testing.T) { t.Run("Duplicates validator should collect info about duplicate UIDs and titles within folders", func(t *testing.T) { const folderName = "duplicates-validator-folder" - r, err := NewDashboardFileReader(cfg, logger, nil, featuremgmt.WithFeatures(), nil) + r, err := NewDashboardFileReader(cfg, logger, nil) require.NoError(t, err) fakeService.On("SaveFolderForProvisionedDashboards", mock.Anything, mock.Anything).Return(&models.Dashboard{}, nil).Times(6) fakeService.On("GetProvisionedDashboardData", mock.Anything).Return([]*models.DashboardProvisioning{}, nil).Times(4) @@ -56,11 +55,11 @@ func TestDuplicatesValidator(t *testing.T) { Options: map[string]interface{}{"path": dashboardContainingUID}, } - reader1, err := NewDashboardFileReader(cfg1, logger, nil, featuremgmt.WithFeatures(), nil) + reader1, err := NewDashboardFileReader(cfg1, logger, nil) reader1.dashboardProvisioningService = fakeService require.NoError(t, err) - reader2, err := NewDashboardFileReader(cfg2, logger, nil, featuremgmt.WithFeatures(), nil) + reader2, err := NewDashboardFileReader(cfg2, logger, nil) reader2.dashboardProvisioningService = fakeService require.NoError(t, err) @@ -92,7 +91,7 @@ func TestDuplicatesValidator(t *testing.T) { t.Run("Duplicates validator should not collect info about duplicate UIDs and titles within folders for different orgs", func(t *testing.T) { const folderName = "duplicates-validator-folder" - r, err := NewDashboardFileReader(cfg, logger, nil, featuremgmt.WithFeatures(), nil) + r, err := NewDashboardFileReader(cfg, logger, nil) require.NoError(t, err) folderID, err := r.getOrCreateFolderID(context.Background(), cfg, fakeService, folderName) require.NoError(t, err) @@ -108,11 +107,11 @@ func TestDuplicatesValidator(t *testing.T) { Options: map[string]interface{}{"path": dashboardContainingUID}, } - reader1, err := NewDashboardFileReader(cfg1, logger, nil, featuremgmt.WithFeatures(), nil) + reader1, err := NewDashboardFileReader(cfg1, logger, nil) reader1.dashboardProvisioningService = fakeService require.NoError(t, err) - reader2, err := NewDashboardFileReader(cfg2, logger, nil, featuremgmt.WithFeatures(), nil) + reader2, err := NewDashboardFileReader(cfg2, logger, nil) reader2.dashboardProvisioningService = fakeService require.NoError(t, err) @@ -168,15 +167,15 @@ func TestDuplicatesValidator(t *testing.T) { Name: "third", Type: "file", OrgID: 2, Folder: "duplicates-validator-folder", Options: map[string]interface{}{"path": twoDashboardsWithUID}, } - reader1, err := NewDashboardFileReader(cfg1, logger, nil, featuremgmt.WithFeatures(), nil) + reader1, err := NewDashboardFileReader(cfg1, logger, nil) reader1.dashboardProvisioningService = fakeService require.NoError(t, err) - reader2, err := NewDashboardFileReader(cfg2, logger, nil, featuremgmt.WithFeatures(), nil) + reader2, err := NewDashboardFileReader(cfg2, logger, nil) reader2.dashboardProvisioningService = fakeService require.NoError(t, err) - reader3, err := NewDashboardFileReader(cfg3, logger, nil, featuremgmt.WithFeatures(), nil) + reader3, err := NewDashboardFileReader(cfg3, logger, nil) reader3.dashboardProvisioningService = fakeService require.NoError(t, err) @@ -193,7 +192,7 @@ func TestDuplicatesValidator(t *testing.T) { duplicates := duplicateValidator.getDuplicates() - r, err := NewDashboardFileReader(cfg, logger, nil, featuremgmt.WithFeatures(), nil) + r, err := NewDashboardFileReader(cfg, logger, nil) require.NoError(t, err) folderID, err := r.getOrCreateFolderID(context.Background(), cfg, fakeService, cfg1.Folder) require.NoError(t, err) @@ -210,7 +209,7 @@ func TestDuplicatesValidator(t *testing.T) { sort.Strings(titleUsageReaders) require.Equal(t, []string{"first"}, titleUsageReaders) - r, err = NewDashboardFileReader(cfg3, logger, nil, featuremgmt.WithFeatures(), nil) + r, err = NewDashboardFileReader(cfg3, logger, nil) require.NoError(t, err) folderID, err = r.getOrCreateFolderID(context.Background(), cfg3, fakeService, cfg3.Folder) require.NoError(t, err) diff --git a/pkg/services/provisioning/provisioning.go b/pkg/services/provisioning/provisioning.go index 6fa9e322b5f..4fb9202524f 100644 --- a/pkg/services/provisioning/provisioning.go +++ b/pkg/services/provisioning/provisioning.go @@ -8,12 +8,10 @@ import ( "github.com/grafana/grafana/pkg/infra/log" plugifaces "github.com/grafana/grafana/pkg/plugins" "github.com/grafana/grafana/pkg/registry" - "github.com/grafana/grafana/pkg/services/accesscontrol" "github.com/grafana/grafana/pkg/services/alerting" dashboardservice "github.com/grafana/grafana/pkg/services/dashboards" datasourceservice "github.com/grafana/grafana/pkg/services/datasources" "github.com/grafana/grafana/pkg/services/encryption" - "github.com/grafana/grafana/pkg/services/featuremgmt" "github.com/grafana/grafana/pkg/services/notifications" "github.com/grafana/grafana/pkg/services/pluginsettings" "github.com/grafana/grafana/pkg/services/provisioning/dashboards" @@ -31,7 +29,6 @@ func ProvideService(cfg *setting.Cfg, sqlStore *sqlstore.SQLStore, pluginStore p dashboardService dashboardservice.DashboardProvisioningService, datasourceService datasourceservice.DataSourceService, alertingService *alerting.AlertNotificationService, pluginSettings pluginsettings.Service, - features featuremgmt.FeatureToggles, permissionsServices accesscontrol.PermissionsServices, ) (*ProvisioningServiceImpl, error) { s := &ProvisioningServiceImpl{ Cfg: cfg, @@ -48,8 +45,6 @@ func ProvideService(cfg *setting.Cfg, sqlStore *sqlstore.SQLStore, pluginStore p datasourceService: datasourceService, alertingService: alertingService, pluginsSettings: pluginSettings, - features: features, - permissionsServices: permissionsServices, } return s, nil } @@ -110,8 +105,6 @@ type ProvisioningServiceImpl struct { datasourceService datasourceservice.DataSourceService alertingService *alerting.AlertNotificationService pluginsSettings pluginsettings.Service - features featuremgmt.FeatureToggles - permissionsServices accesscontrol.PermissionsServices } func (ps *ProvisioningServiceImpl) RunInitProvisioners(ctx context.Context) error { @@ -194,7 +187,7 @@ func (ps *ProvisioningServiceImpl) ProvisionNotifications(ctx context.Context) e func (ps *ProvisioningServiceImpl) ProvisionDashboards(ctx context.Context) error { dashboardPath := filepath.Join(ps.Cfg.ProvisioningPath, "dashboards") - dashProvisioner, err := ps.newDashboardProvisioner(ctx, dashboardPath, ps.dashboardService, ps.SQLStore, ps.features, ps.permissionsServices) + dashProvisioner, err := ps.newDashboardProvisioner(ctx, dashboardPath, ps.dashboardService, ps.SQLStore) if err != nil { return errutil.Wrap("Failed to create provisioner", err) } diff --git a/pkg/services/provisioning/provisioning_test.go b/pkg/services/provisioning/provisioning_test.go index 9bac4f2f7b7..4493d6569dd 100644 --- a/pkg/services/provisioning/provisioning_test.go +++ b/pkg/services/provisioning/provisioning_test.go @@ -6,9 +6,7 @@ import ( "testing" "time" - "github.com/grafana/grafana/pkg/services/accesscontrol" dashboardstore "github.com/grafana/grafana/pkg/services/dashboards" - "github.com/grafana/grafana/pkg/services/featuremgmt" "github.com/grafana/grafana/pkg/services/provisioning/dashboards" "github.com/grafana/grafana/pkg/services/provisioning/utils" "github.com/grafana/grafana/pkg/setting" @@ -95,7 +93,7 @@ func setup() *serviceTestStruct { } serviceTest.service = newProvisioningServiceImpl( - func(context.Context, string, dashboardstore.DashboardProvisioningService, utils.OrgStore, featuremgmt.FeatureToggles, accesscontrol.PermissionsServices) (dashboards.DashboardProvisioner, error) { + func(context.Context, string, dashboardstore.DashboardProvisioningService, utils.OrgStore) (dashboards.DashboardProvisioner, error) { return serviceTest.mock, nil }, nil,