Chore: use org service methods (#55768)

* Chore: use org service methods

* fix tests

* fix errors

* adjust func signatures for getbyname

* 💩

* Use the same fake service to get the user in AC and in HS

* Fix middleware test

* Fix more middleware test

* Fix api tests

Co-authored-by: gamab <gabi.mabs@gmail.com>
Co-authored-by: Ida Furjesova <ida.furjesova@grafana.com>
This commit is contained in:
Kat Yang
2022-10-04 14:48:02 -04:00
committed by GitHub
parent 169f1ab974
commit 7715672fb3
14 changed files with 108 additions and 67 deletions

View File

@@ -7,6 +7,7 @@ import (
"github.com/grafana/grafana/pkg/models"
"github.com/grafana/grafana/pkg/services/encryption"
"github.com/grafana/grafana/pkg/services/notifications"
"github.com/grafana/grafana/pkg/services/org"
)
type Manager interface {
@@ -29,8 +30,8 @@ type SQLStore interface {
}
// Provision alert notifiers
func Provision(ctx context.Context, configDirectory string, alertingService Manager, sqlstore SQLStore, encryptionService encryption.Internal, notificationService *notifications.NotificationService) error {
dc := newNotificationProvisioner(sqlstore, alertingService, encryptionService, notificationService, log.New("provisioning.notifiers"))
func Provision(ctx context.Context, configDirectory string, alertingService Manager, orgService org.Service, sqlstore SQLStore, encryptionService encryption.Internal, notificationService *notifications.NotificationService) error {
dc := newNotificationProvisioner(orgService, sqlstore, alertingService, encryptionService, notificationService, log.New("provisioning.notifiers"))
return dc.applyChanges(ctx, configDirectory)
}
@@ -40,9 +41,10 @@ type NotificationProvisioner struct {
cfgProvider *configReader
alertingManager Manager
sqlstore SQLStore
orgService org.Service
}
func newNotificationProvisioner(store SQLStore, alertingManager Manager, encryptionService encryption.Internal, notifiationService *notifications.NotificationService, log log.Logger) NotificationProvisioner {
func newNotificationProvisioner(orgService org.Service, store SQLStore, alertingManager Manager, encryptionService encryption.Internal, notifiationService *notifications.NotificationService, log log.Logger) NotificationProvisioner {
return NotificationProvisioner{
log: log,
alertingManager: alertingManager,
@@ -52,7 +54,8 @@ func newNotificationProvisioner(store SQLStore, alertingManager Manager, encrypt
log: log,
orgStore: store,
},
sqlstore: store,
sqlstore: store,
orgService: orgService,
}
}
@@ -73,11 +76,12 @@ func (dc *NotificationProvisioner) deleteNotifications(ctx context.Context, noti
dc.log.Info("Deleting alert notification", "name", notification.Name, "uid", notification.UID)
if notification.OrgID == 0 && notification.OrgName != "" {
getOrg := &models.GetOrgByNameQuery{Name: notification.OrgName}
if err := dc.sqlstore.GetOrgByNameHandler(ctx, getOrg); err != nil {
getOrg := org.GetOrgByNameQuery{Name: notification.OrgName}
res, err := dc.orgService.GetByName(ctx, &getOrg)
if err != nil {
return err
}
notification.OrgID = getOrg.Result.Id
notification.OrgID = res.ID
} else if notification.OrgID < 0 {
notification.OrgID = 1
}
@@ -102,11 +106,12 @@ func (dc *NotificationProvisioner) deleteNotifications(ctx context.Context, noti
func (dc *NotificationProvisioner) mergeNotifications(ctx context.Context, notificationToMerge []*notificationFromConfig) error {
for _, notification := range notificationToMerge {
if notification.OrgID == 0 && notification.OrgName != "" {
getOrg := &models.GetOrgByNameQuery{Name: notification.OrgName}
if err := dc.sqlstore.GetOrgByNameHandler(ctx, getOrg); err != nil {
getOrg := org.GetOrgByNameQuery{Name: notification.OrgName}
res, err := dc.orgService.GetByName(ctx, &getOrg)
if err != nil {
return err
}
notification.OrgID = getOrg.Result.Id
notification.OrgID = res.ID
} else if notification.OrgID < 0 {
notification.OrgID = 1
}

View File

@@ -12,6 +12,8 @@ import (
"github.com/grafana/grafana/pkg/services/alerting/notifiers"
encryptionservice "github.com/grafana/grafana/pkg/services/encryption/service"
"github.com/grafana/grafana/pkg/services/notifications"
"github.com/grafana/grafana/pkg/services/org"
"github.com/grafana/grafana/pkg/services/org/orgtest"
"github.com/grafana/grafana/pkg/services/sqlstore"
"github.com/stretchr/testify/require"
@@ -34,6 +36,8 @@ func TestNotificationAsConfig(t *testing.T) {
var sqlStore *sqlstore.SQLStore
var ns *alerting.AlertNotificationService
logger := log.New("fake.log")
orgService := orgtest.NewOrgServiceFake()
orgService.ExpectedOrg = &org.Org{}
encryptionService := encryptionservice.SetupTestService(t)
@@ -146,7 +150,7 @@ func TestNotificationAsConfig(t *testing.T) {
setup()
fakeAlertNotification := &fakeAlertNotification{}
fakeAlertNotification.ExpectedAlertNotification = &models.AlertNotification{OrgId: 1}
dc := newNotificationProvisioner(sqlStore, fakeAlertNotification, encryptionService, nil, logger)
dc := newNotificationProvisioner(orgService, sqlStore, fakeAlertNotification, encryptionService, nil, logger)
err := dc.applyChanges(context.Background(), twoNotificationsConfig)
if err != nil {
@@ -172,7 +176,7 @@ func TestNotificationAsConfig(t *testing.T) {
require.Equal(t, len(notificationsQuery.Result), 1)
t.Run("should update one notification", func(t *testing.T) {
dc := newNotificationProvisioner(sqlStore, &fakeAlertNotification{}, encryptionService, nil, logger)
dc := newNotificationProvisioner(orgService, sqlStore, &fakeAlertNotification{}, encryptionService, nil, logger)
err = dc.applyChanges(context.Background(), twoNotificationsConfig)
if err != nil {
t.Fatalf("applyChanges return an error %v", err)
@@ -182,7 +186,7 @@ func TestNotificationAsConfig(t *testing.T) {
t.Run("Two notifications with is_default", func(t *testing.T) {
setup()
dc := newNotificationProvisioner(sqlStore, &fakeAlertNotification{}, encryptionService, nil, logger)
dc := newNotificationProvisioner(orgService, sqlStore, &fakeAlertNotification{}, encryptionService, nil, logger)
err := dc.applyChanges(context.Background(), doubleNotificationsConfig)
t.Run("should both be inserted", func(t *testing.T) {
require.NoError(t, err)
@@ -217,7 +221,7 @@ func TestNotificationAsConfig(t *testing.T) {
require.Equal(t, len(notificationsQuery.Result), 2)
t.Run("should have two new notifications", func(t *testing.T) {
dc := newNotificationProvisioner(sqlStore, &fakeAlertNotification{}, encryptionService, nil, logger)
dc := newNotificationProvisioner(orgService, sqlStore, &fakeAlertNotification{}, encryptionService, nil, logger)
err := dc.applyChanges(context.Background(), twoNotificationsConfig)
if err != nil {
t.Fatalf("applyChanges return an error %v", err)
@@ -247,7 +251,7 @@ func TestNotificationAsConfig(t *testing.T) {
err = ns.SQLStore.CreateAlertNotificationCommand(context.Background(), &existingNotificationCmd)
require.NoError(t, err)
dc := newNotificationProvisioner(sqlStore, &fakeAlertNotification{}, encryptionService, nil, logger)
dc := newNotificationProvisioner(orgService, sqlStore, &fakeAlertNotification{}, encryptionService, nil, logger)
err = dc.applyChanges(context.Background(), correctPropertiesWithOrgName)
if err != nil {
t.Fatalf("applyChanges return an error %v", err)
@@ -256,7 +260,7 @@ func TestNotificationAsConfig(t *testing.T) {
t.Run("Config doesn't contain required field", func(t *testing.T) {
setup()
dc := newNotificationProvisioner(sqlStore, &fakeAlertNotification{}, encryptionService, nil, logger)
dc := newNotificationProvisioner(orgService, sqlStore, &fakeAlertNotification{}, encryptionService, nil, logger)
err := dc.applyChanges(context.Background(), noRequiredFields)
require.NotNil(t, err)
@@ -270,7 +274,7 @@ func TestNotificationAsConfig(t *testing.T) {
t.Run("Empty yaml file", func(t *testing.T) {
t.Run("should have not changed repo", func(t *testing.T) {
setup()
dc := newNotificationProvisioner(sqlStore, &fakeAlertNotification{}, encryptionService, nil, logger)
dc := newNotificationProvisioner(orgService, sqlStore, &fakeAlertNotification{}, encryptionService, nil, logger)
err := dc.applyChanges(context.Background(), emptyFile)
if err != nil {
t.Fatalf("applyChanges return an error %v", err)

View File

@@ -7,22 +7,19 @@ import (
"github.com/grafana/grafana/pkg/infra/log"
"github.com/grafana/grafana/pkg/models"
"github.com/grafana/grafana/pkg/plugins"
"github.com/grafana/grafana/pkg/services/org"
"github.com/grafana/grafana/pkg/services/pluginsettings"
)
type Store interface {
GetOrgByNameHandler(ctx context.Context, query *models.GetOrgByNameQuery) error
}
// Provision scans a directory for provisioning config files
// and provisions the app in those files.
func Provision(ctx context.Context, configDirectory string, store Store, pluginStore plugins.Store, pluginSettings pluginsettings.Service) error {
func Provision(ctx context.Context, configDirectory string, pluginStore plugins.Store, pluginSettings pluginsettings.Service, orgService org.Service) error {
logger := log.New("provisioning.plugins")
ap := PluginProvisioner{
log: logger,
cfgProvider: newConfigReader(logger, pluginStore),
store: store,
pluginSettings: pluginSettings,
orgService: orgService,
}
return ap.applyChanges(ctx, configDirectory)
}
@@ -32,18 +29,19 @@ func Provision(ctx context.Context, configDirectory string, store Store, pluginS
type PluginProvisioner struct {
log log.Logger
cfgProvider configReader
store Store
pluginSettings pluginsettings.Service
orgService org.Service
}
func (ap *PluginProvisioner) apply(ctx context.Context, cfg *pluginsAsConfig) error {
for _, app := range cfg.Apps {
if app.OrgID == 0 && app.OrgName != "" {
getOrgQuery := &models.GetOrgByNameQuery{Name: app.OrgName}
if err := ap.store.GetOrgByNameHandler(ctx, getOrgQuery); err != nil {
getOrgQuery := &org.GetOrgByNameQuery{Name: app.OrgName}
res, err := ap.orgService.GetByName(ctx, getOrgQuery)
if err != nil {
return err
}
app.OrgID = getOrgQuery.Result.Id
app.OrgID = res.ID
} else if app.OrgID < 0 {
app.OrgID = 1
}

View File

@@ -5,6 +5,8 @@ import (
"errors"
"testing"
"github.com/grafana/grafana/pkg/services/org"
"github.com/grafana/grafana/pkg/services/org/orgtest"
"github.com/grafana/grafana/pkg/services/pluginsettings"
"github.com/grafana/grafana/pkg/infra/log"
@@ -34,7 +36,9 @@ func TestPluginProvisioner(t *testing.T) {
}
reader := &testConfigReader{result: cfg}
store := &mockStore{}
ap := PluginProvisioner{log: log.New("test"), cfgProvider: reader, store: store, pluginSettings: store}
orgMock := orgtest.NewOrgServiceFake()
orgMock.ExpectedOrg = &org.Org{ID: 4}
ap := PluginProvisioner{log: log.New("test"), cfgProvider: reader, pluginSettings: store, orgService: orgMock}
err := ap.applyChanges(context.Background(), "")
require.NoError(t, err)

View File

@@ -18,6 +18,7 @@ import (
"github.com/grafana/grafana/pkg/services/ngalert/provisioning"
"github.com/grafana/grafana/pkg/services/ngalert/store"
"github.com/grafana/grafana/pkg/services/notifications"
"github.com/grafana/grafana/pkg/services/org"
"github.com/grafana/grafana/pkg/services/pluginsettings"
prov_alerting "github.com/grafana/grafana/pkg/services/provisioning/alerting"
"github.com/grafana/grafana/pkg/services/provisioning/dashboards"
@@ -49,6 +50,7 @@ func ProvideService(
searchService searchV2.SearchService,
quotaService quota.Service,
secrectService secrets.Service,
orgService org.Service,
) (*ProvisioningServiceImpl, error) {
s := &ProvisioningServiceImpl{
Cfg: cfg,
@@ -72,6 +74,7 @@ func ProvideService(
quotaService: quotaService,
secretService: secrectService,
log: log.New("provisioning"),
orgService: orgService,
}
return s, nil
}
@@ -103,9 +106,9 @@ func NewProvisioningServiceImpl() *ProvisioningServiceImpl {
// Used for testing purposes
func newProvisioningServiceImpl(
newDashboardProvisioner dashboards.DashboardProvisionerFactory,
provisionNotifiers func(context.Context, string, notifiers.Manager, notifiers.SQLStore, encryption.Internal, *notifications.NotificationService) error,
provisionNotifiers func(context.Context, string, notifiers.Manager, org.Service, notifiers.SQLStore, encryption.Internal, *notifications.NotificationService) error,
provisionDatasources func(context.Context, string, datasources.Store, datasources.CorrelationsStore, utils.OrgStore) error,
provisionPlugins func(context.Context, string, plugins.Store, plugifaces.Store, pluginsettings.Service) error,
provisionPlugins func(context.Context, string, plugifaces.Store, pluginsettings.Service, org.Service) error,
) *ProvisioningServiceImpl {
return &ProvisioningServiceImpl{
log: log.New("provisioning"),
@@ -119,6 +122,7 @@ func newProvisioningServiceImpl(
type ProvisioningServiceImpl struct {
Cfg *setting.Cfg
SQLStore *sqlstore.SQLStore
orgService org.Service
ac accesscontrol.AccessControl
pluginStore plugifaces.Store
EncryptionService encryption.Internal
@@ -127,9 +131,9 @@ type ProvisioningServiceImpl struct {
pollingCtxCancel context.CancelFunc
newDashboardProvisioner dashboards.DashboardProvisionerFactory
dashboardProvisioner dashboards.DashboardProvisioner
provisionNotifiers func(context.Context, string, notifiers.Manager, notifiers.SQLStore, encryption.Internal, *notifications.NotificationService) error
provisionNotifiers func(context.Context, string, notifiers.Manager, org.Service, notifiers.SQLStore, encryption.Internal, *notifications.NotificationService) error
provisionDatasources func(context.Context, string, datasources.Store, datasources.CorrelationsStore, utils.OrgStore) error
provisionPlugins func(context.Context, string, plugins.Store, plugifaces.Store, pluginsettings.Service) error
provisionPlugins func(context.Context, string, plugifaces.Store, pluginsettings.Service, org.Service) error
provisionAlerting func(context.Context, prov_alerting.ProvisionerConfig) error
mutex sync.Mutex
dashboardProvisioningService dashboardservice.DashboardProvisioningService
@@ -211,7 +215,7 @@ func (ps *ProvisioningServiceImpl) ProvisionDatasources(ctx context.Context) err
func (ps *ProvisioningServiceImpl) ProvisionPlugins(ctx context.Context) error {
appPath := filepath.Join(ps.Cfg.ProvisioningPath, "plugins")
if err := ps.provisionPlugins(ctx, appPath, ps.SQLStore, ps.pluginStore, ps.pluginsSettings); err != nil {
if err := ps.provisionPlugins(ctx, appPath, ps.pluginStore, ps.pluginsSettings, ps.orgService); err != nil {
err = fmt.Errorf("%v: %w", "app provisioning error", err)
ps.log.Error("Failed to provision plugins", "error", err)
return err
@@ -221,7 +225,7 @@ func (ps *ProvisioningServiceImpl) ProvisionPlugins(ctx context.Context) error {
func (ps *ProvisioningServiceImpl) ProvisionNotifications(ctx context.Context) error {
alertNotificationsPath := filepath.Join(ps.Cfg.ProvisioningPath, "notifiers")
if err := ps.provisionNotifiers(ctx, alertNotificationsPath, ps.alertingService, ps.SQLStore, ps.EncryptionService, ps.NotificationService); err != nil {
if err := ps.provisionNotifiers(ctx, alertNotificationsPath, ps.alertingService, ps.orgService, ps.SQLStore, ps.EncryptionService, ps.NotificationService); err != nil {
err = fmt.Errorf("%v: %w", "Alert notification provisioning error", err)
ps.log.Error("Failed to provision alert notifications", "error", err)
return err