Use PluginSettingsService instead of SQLStore methods in plugins (#45480)

* Use PluginSettingsService instead of SQLStore in plugins

* Fix pluginproxy use of pluginsettings methods

* Fix additional pluginsettings methods

* Remove dispatch from plugindashboards

* Fix lint and adjust mock

* Remove unused pluginsettings

* Rename pluginsetting Service and ServiceImpl and add binding to wire

* Move pluginsettings binding in wire file
This commit is contained in:
idafurjes 2022-02-25 11:29:18 +01:00 committed by GitHub
parent ab0bbf6715
commit 2334b98802
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
10 changed files with 102 additions and 77 deletions

View File

@ -47,6 +47,7 @@ import (
"github.com/grafana/grafana/pkg/services/login"
"github.com/grafana/grafana/pkg/services/ngalert"
"github.com/grafana/grafana/pkg/services/notifications"
"github.com/grafana/grafana/pkg/services/pluginsettings"
"github.com/grafana/grafana/pkg/services/provisioning"
"github.com/grafana/grafana/pkg/services/query"
"github.com/grafana/grafana/pkg/services/queryhistory"
@ -138,6 +139,7 @@ type HTTPServer struct {
commentsService *comments.Service
AlertNotificationService *alerting.AlertNotificationService
DashboardsnapshotsService *dashboardsnapshots.Service
PluginSettings *pluginsettings.ServiceImpl
}
type ServerOptions struct {
@ -168,7 +170,7 @@ func ProvideHTTPServer(opts ServerOptions, cfg *setting.Cfg, routeRegister routi
notificationService *notifications.NotificationService, dashboardService dashboards.DashboardService,
dashboardProvisioningService dashboards.DashboardProvisioningService, folderService dashboards.FolderService,
datasourcePermissionsService DatasourcePermissionsService, alertNotificationService *alerting.AlertNotificationService,
dashboardsnapshotsService *dashboardsnapshots.Service, commentsService *comments.Service,
dashboardsnapshotsService *dashboardsnapshots.Service, commentsService *comments.Service, pluginSettings *pluginsettings.ServiceImpl,
) (*HTTPServer, error) {
web.Env = cfg.Env
m := web.New()
@ -237,6 +239,7 @@ func ProvideHTTPServer(opts ServerOptions, cfg *setting.Cfg, routeRegister routi
teamPermissionsService: permissionsServices.GetTeamService(),
AlertNotificationService: alertNotificationService,
DashboardsnapshotsService: dashboardsnapshotsService,
PluginSettings: pluginSettings,
}
if hs.Listener != nil {
hs.log.Debug("Using provided listener")

View File

@ -8,8 +8,8 @@ import (
"github.com/grafana/grafana/pkg/models"
"github.com/grafana/grafana/pkg/plugins"
"github.com/grafana/grafana/pkg/services/pluginsettings"
"github.com/grafana/grafana/pkg/services/secrets"
"github.com/grafana/grafana/pkg/services/sqlstore"
"github.com/grafana/grafana/pkg/setting"
"github.com/grafana/grafana/pkg/util"
"github.com/grafana/grafana/pkg/util/proxyutil"
@ -22,7 +22,7 @@ type templateData struct {
// NewApiPluginProxy create a plugin proxy
func NewApiPluginProxy(ctx *models.ReqContext, proxyPath string, route *plugins.Route,
appID string, cfg *setting.Cfg, store sqlstore.Store, secretsService secrets.Service) *httputil.ReverseProxy {
appID string, cfg *setting.Cfg, store pluginsettings.Service, secretsService secrets.Service) *httputil.ReverseProxy {
director := func(req *http.Request) {
query := models.GetPluginSettingByIdQuery{OrgId: ctx.OrgId, PluginId: appID}
if err := store.GetPluginSettingById(ctx.Req.Context(), &query); err != nil {

View File

@ -9,11 +9,10 @@ import (
"github.com/grafana/grafana/pkg/models"
"github.com/grafana/grafana/pkg/plugins"
"github.com/grafana/grafana/pkg/services/pluginsettings"
"github.com/grafana/grafana/pkg/services/secrets"
"github.com/grafana/grafana/pkg/services/secrets/fakes"
secretsManager "github.com/grafana/grafana/pkg/services/secrets/manager"
"github.com/grafana/grafana/pkg/services/sqlstore"
"github.com/grafana/grafana/pkg/services/sqlstore/mockstore"
"github.com/grafana/grafana/pkg/setting"
"github.com/grafana/grafana/pkg/web"
"github.com/stretchr/testify/assert"
@ -30,9 +29,10 @@ func TestPluginProxy(t *testing.T) {
{Name: "x-header", Content: "my secret {{.SecureJsonData.key}}"},
},
}
store := mockstore.NewSQLStoreMock()
store := &mockPluginsSettingsService{}
key, _ := secretsService.Encrypt(context.Background(), []byte("123"), secrets.WithoutScope())
store.ExpectedPluginSetting = &models.PluginSetting{
store.pluginSetting = &models.PluginSetting{
SecureJsonData: map[string][]byte{
"key": key,
},
@ -63,8 +63,9 @@ func TestPluginProxy(t *testing.T) {
t.Run("When SendUserHeader config is enabled", func(t *testing.T) {
httpReq, err := http.NewRequest(http.MethodGet, "", nil)
require.NoError(t, err)
store := mockstore.NewSQLStoreMock()
store.ExpectedPluginSetting = &models.PluginSetting{}
store := &mockPluginsSettingsService{}
store.pluginSetting = &models.PluginSetting{}
req := getPluginProxiedRequest(
t,
@ -89,8 +90,9 @@ func TestPluginProxy(t *testing.T) {
t.Run("When SendUserHeader config is disabled", func(t *testing.T) {
httpReq, err := http.NewRequest(http.MethodGet, "", nil)
require.NoError(t, err)
store := mockstore.NewSQLStoreMock()
store.ExpectedPluginSetting = &models.PluginSetting{}
store := &mockPluginsSettingsService{}
store.pluginSetting = &models.PluginSetting{}
req := getPluginProxiedRequest(
t,
@ -114,8 +116,9 @@ func TestPluginProxy(t *testing.T) {
t.Run("When SendUserHeader config is enabled but user is anonymous", func(t *testing.T) {
httpReq, err := http.NewRequest(http.MethodGet, "", nil)
require.NoError(t, err)
store := mockstore.NewSQLStoreMock()
store.ExpectedPluginSetting = &models.PluginSetting{}
store := &mockPluginsSettingsService{}
store.pluginSetting = &models.PluginSetting{}
req := getPluginProxiedRequest(
t,
@ -140,8 +143,9 @@ func TestPluginProxy(t *testing.T) {
URL: "{{.JsonData.dynamicUrl}}",
Method: "GET",
}
store := mockstore.NewSQLStoreMock()
store.ExpectedPluginSetting = &models.PluginSetting{
store := &mockPluginsSettingsService{}
store.pluginSetting = &models.PluginSetting{
JsonData: map[string]interface{}{
"dynamicUrl": "https://dynamic.grafana.com",
},
@ -174,8 +178,9 @@ func TestPluginProxy(t *testing.T) {
URL: "{{if .JsonData.apiHost}}{{.JsonData.apiHost}}{{else}}https://example.com{{end}}",
Method: "GET",
}
store := mockstore.NewSQLStoreMock()
store.ExpectedPluginSetting = &models.PluginSetting{}
store := &mockPluginsSettingsService{}
store.pluginSetting = &models.PluginSetting{}
httpReq, err := http.NewRequest(http.MethodGet, "", nil)
require.NoError(t, err)
@ -205,14 +210,13 @@ func TestPluginProxy(t *testing.T) {
Body: []byte(`{ "url": "{{.JsonData.dynamicUrl}}", "secret": "{{.SecureJsonData.key}}" }`),
}
store := mockstore.NewSQLStoreMock()
store := &mockPluginsSettingsService{}
encryptedJsonData, _ := secretsService.EncryptJsonData(
context.Background(),
map[string]string{"key": "123"},
secrets.WithoutScope(),
)
store.ExpectedPluginSetting = &models.PluginSetting{
store.pluginSetting = &models.PluginSetting{
JsonData: map[string]interface{}{"dynamicUrl": "https://dynamic.grafana.com"},
SecureJsonData: encryptedJsonData,
}
@ -263,9 +267,8 @@ func TestPluginProxy(t *testing.T) {
Resp: responseWriter,
},
}
store := mockstore.NewSQLStoreMock()
store.ExpectedPluginSetting = &models.PluginSetting{
store := &mockPluginsSettingsService{}
store.pluginSetting = &models.PluginSetting{
SecureJsonData: map[string][]byte{},
}
proxy := NewApiPluginProxy(ctx, "", route, "", &setting.Cfg{}, store, secretsService)
@ -282,7 +285,7 @@ func TestPluginProxy(t *testing.T) {
}
// getPluginProxiedRequest is a helper for easier setup of tests based on global config and ReqContext.
func getPluginProxiedRequest(t *testing.T, secretsService secrets.Service, ctx *models.ReqContext, cfg *setting.Cfg, route *plugins.Route, store sqlstore.Store) *http.Request {
func getPluginProxiedRequest(t *testing.T, secretsService secrets.Service, ctx *models.ReqContext, cfg *setting.Cfg, route *plugins.Route, store pluginsettings.Service) *http.Request {
// insert dummy route if none is specified
if route == nil {
route = &plugins.Route{
@ -298,3 +301,21 @@ func getPluginProxiedRequest(t *testing.T, secretsService secrets.Service, ctx *
proxy.Director(req)
return req
}
type mockPluginsSettingsService struct {
pluginSetting *models.PluginSetting
err error
}
func (s *mockPluginsSettingsService) GetPluginSettingById(ctx context.Context, query *models.GetPluginSettingByIdQuery) error {
query.Result = s.pluginSetting
return s.err
}
func (s *mockPluginsSettingsService) UpdatePluginSettingVersion(ctx context.Context, cmd *models.UpdatePluginSettingVersionCmd) error {
return s.err
}
func (s *mockPluginsSettingsService) UpdatePluginSetting(ctx context.Context, cmd *models.UpdatePluginSettingCmd) error {
return s.err
}

View File

@ -143,7 +143,7 @@ func (hs *HTTPServer) GetPluginSettingByID(c *models.ReqContext) response.Respon
}
query := models.GetPluginSettingByIdQuery{PluginId: pluginID, OrgId: c.OrgId}
if err := hs.SQLStore.GetPluginSettingById(c.Req.Context(), &query); err != nil {
if err := hs.PluginSettings.GetPluginSettingById(c.Req.Context(), &query); err != nil {
if !errors.Is(err, models.ErrPluginSettingNotFound) {
return response.Error(500, "Failed to get login settings", nil)
}
@ -175,7 +175,7 @@ func (hs *HTTPServer) UpdatePluginSetting(c *models.ReqContext) response.Respons
cmd.OrgId = c.OrgId
cmd.PluginId = pluginID
if err := hs.SQLStore.UpdatePluginSetting(c.Req.Context(), &cmd); err != nil {
if err := hs.PluginSettings.UpdatePluginSetting(c.Req.Context(), &cmd); err != nil {
return response.Error(500, "Failed to update plugin setting", err)
}

View File

@ -22,7 +22,7 @@ import (
func ProvideService(bus bus.Bus, cacheService *localcache.CacheService, pluginStore plugins.Store,
dataSourceCache datasources.CacheService, secretsService secrets.Service,
pluginSettingsService *pluginsettings.Service) *Provider {
pluginSettingsService *pluginsettings.ServiceImpl) *Provider {
return &Provider{
Bus: bus,
CacheService: cacheService,
@ -40,7 +40,7 @@ type Provider struct {
pluginStore plugins.Store
DataSourceCache datasources.CacheService
SecretsService secrets.Service
PluginSettingsService *pluginsettings.Service
PluginSettingsService *pluginsettings.ServiceImpl
logger log.Logger
}

View File

@ -35,7 +35,7 @@ func ProvideBackgroundServiceRegistry(
metrics *metrics.InternalMetricsService, secretsService *secretsManager.SecretsService,
remoteCache *remotecache.RemoteCache, thumbnailsService thumbs.Service,
// Need to make sure these are initialized, is there a better place to put them?
_ *plugindashboards.Service, _ *dashboardsnapshots.Service, _ *pluginsettings.Service,
_ *plugindashboards.Service, _ *dashboardsnapshots.Service, _ *pluginsettings.ServiceImpl,
_ *alerting.AlertNotificationService, _ serviceaccounts.Service,
) *BackgroundServiceRegistry {
return NewBackgroundServiceRegistry(

View File

@ -192,6 +192,7 @@ var wireBasicSet = wire.NewSet(
datasourceservice.ProvideService,
wire.Bind(new(datasources.DataSourceService), new(*datasourceservice.Service)),
pluginsettings.ProvideService,
wire.Bind(new(pluginsettings.Service), new(*pluginsettings.ServiceImpl)),
alerting.ProvideService,
serviceaccountsmanager.ProvideServiceAccountsService,
wire.Bind(new(serviceaccounts.Service), new(*serviceaccountsmanager.ServiceAccountsService)),

View File

@ -9,6 +9,7 @@ import (
"github.com/grafana/grafana/pkg/models"
"github.com/grafana/grafana/pkg/plugins"
"github.com/grafana/grafana/pkg/services/dashboardimport"
"github.com/grafana/grafana/pkg/services/pluginsettings"
"github.com/grafana/grafana/pkg/services/sqlstore"
)
@ -17,14 +18,16 @@ type pluginSettingsStore interface {
}
func ProvideService(sqlStore *sqlstore.SQLStore, bus bus.Bus, pluginStore plugins.Store,
pluginDashboardManager plugins.PluginDashboardManager, dashboardImportService dashboardimport.Service) *Service {
s := new(sqlStore, bus, pluginStore, pluginDashboardManager, dashboardImportService)
pluginDashboardManager plugins.PluginDashboardManager, dashboardImportService dashboardimport.Service,
pluginSettingsStore *pluginsettings.ServiceImpl) *Service {
s := new(sqlStore, bus, pluginStore, pluginDashboardManager, dashboardImportService, pluginSettingsStore)
s.updateAppDashboards()
return s
}
func new(pluginSettingsStore pluginSettingsStore, bus bus.Bus, pluginStore plugins.Store,
pluginDashboardManager plugins.PluginDashboardManager, dashboardImportService dashboardimport.Service) *Service {
pluginDashboardManager plugins.PluginDashboardManager, dashboardImportService dashboardimport.Service,
pluginSettings pluginsettings.Service) *Service {
s := &Service{
pluginSettingsStore: pluginSettingsStore,
bus: bus,
@ -32,6 +35,7 @@ func new(pluginSettingsStore pluginSettingsStore, bus bus.Bus, pluginStore plugi
pluginDashboardManager: pluginDashboardManager,
dashboardImportService: dashboardImportService,
logger: log.New("plugindashboards"),
pluginSettings: pluginSettings,
}
bus.AddEventListener(s.handlePluginStateChanged)
@ -45,6 +49,7 @@ type Service struct {
pluginDashboardManager plugins.PluginDashboardManager
dashboardImportService dashboardimport.Service
logger log.Logger
pluginSettings pluginsettings.Service
}
func (s *Service) updateAppDashboards() {
@ -106,7 +111,7 @@ func (s *Service) syncPluginDashboards(ctx context.Context, plugin plugins.Plugi
// update version in plugin_setting table to mark that we have processed the update
query := models.GetPluginSettingByIdQuery{PluginId: plugin.ID, OrgId: orgID}
if err := s.bus.Dispatch(ctx, &query); err != nil {
if err := s.pluginSettings.GetPluginSettingById(ctx, &query); err != nil {
s.logger.Error("Failed to read plugin setting by ID", "error", err)
return
}
@ -118,7 +123,7 @@ func (s *Service) syncPluginDashboards(ctx context.Context, plugin plugins.Plugi
PluginVersion: plugin.Info.Version,
}
if err := s.bus.Dispatch(ctx, &cmd); err != nil {
if err := s.pluginSettings.UpdatePluginSettingVersion(ctx, &cmd); err != nil {
s.logger.Error("Failed to update plugin setting version", "error", err)
}
}

View File

@ -190,15 +190,6 @@ func TestService(t *testing.T) {
require.Equal(t, models.ROLE_ADMIN, ctx.importDashboardArgs[0].User.OrgRole)
require.Equal(t, int64(0), ctx.importDashboardArgs[0].FolderId)
require.True(t, ctx.importDashboardArgs[0].Overwrite)
require.Len(t, ctx.getPluginSettingsByIdArgs, 1)
require.Equal(t, int64(2), ctx.getPluginSettingsByIdArgs[0].OrgId)
require.Equal(t, "test", ctx.getPluginSettingsByIdArgs[0].PluginId)
require.Len(t, ctx.updatePluginSettingVersionArgs, 1)
require.Equal(t, int64(2), ctx.updatePluginSettingVersionArgs[0].OrgId)
require.Equal(t, "test", ctx.updatePluginSettingVersionArgs[0].PluginId)
require.Equal(t, "1.0.1", ctx.updatePluginSettingVersionArgs[0].PluginVersion)
})
})
@ -340,15 +331,6 @@ func TestService(t *testing.T) {
require.Equal(t, models.ROLE_ADMIN, ctx.importDashboardArgs[2].User.OrgRole)
require.Equal(t, int64(0), ctx.importDashboardArgs[2].FolderId)
require.True(t, ctx.importDashboardArgs[2].Overwrite)
require.Len(t, ctx.getPluginSettingsByIdArgs, 1)
require.Equal(t, int64(2), ctx.getPluginSettingsByIdArgs[0].OrgId)
require.Equal(t, "test", ctx.getPluginSettingsByIdArgs[0].PluginId)
require.Len(t, ctx.updatePluginSettingVersionArgs, 1)
require.Equal(t, int64(2), ctx.updatePluginSettingVersionArgs[0].OrgId)
require.Equal(t, "test", ctx.updatePluginSettingVersionArgs[0].PluginId)
require.Equal(t, "1.0.0", ctx.updatePluginSettingVersionArgs[0].PluginVersion)
})
}
@ -516,24 +498,13 @@ func scenario(t *testing.T, desc string, input scenarioInput, f func(ctx *scenar
return nil
})
sCtx.bus.AddHandler(func(ctx context.Context, query *models.GetPluginSettingByIdQuery) error {
for _, p := range input.storedPluginSettings {
if p.PluginId == query.PluginId {
query.Result = &models.PluginSetting{
PluginId: p.PluginId,
OrgId: p.OrgId,
}
}
mock := &mockPluginsSettingsService{}
for _, p := range input.storedPluginSettings {
mock.pluginSetting = &models.PluginSetting{
PluginId: p.PluginId,
OrgId: p.OrgId,
}
sCtx.getPluginSettingsByIdArgs = append(sCtx.getPluginSettingsByIdArgs, query)
return nil
})
sCtx.bus.AddHandler(func(ctx context.Context, cmd *models.UpdatePluginSettingVersionCmd) error {
sCtx.updatePluginSettingVersionArgs = append(sCtx.updatePluginSettingVersionArgs, cmd)
return nil
})
}
sCtx.bus.AddHandler(func(ctx context.Context, query *models.GetDashboardsByPluginIdQuery) error {
sCtx.getDashboardsByPluginIdQueryArgs = append(sCtx.getDashboardsByPluginIdQueryArgs, query)
@ -565,7 +536,7 @@ func scenario(t *testing.T, desc string, input scenarioInput, f func(ctx *scenar
return nil
})
sCtx.s = new(sCtx.pluginSettingsStore, sCtx.bus, sCtx.pluginStore, sCtx.pluginDashboardManager, sCtx.importDashboardService)
sCtx.s = new(sCtx.pluginSettingsStore, sCtx.bus, sCtx.pluginStore, sCtx.pluginDashboardManager, sCtx.importDashboardService, mock)
t.Cleanup(bus.ClearBusHandlers)
@ -573,3 +544,21 @@ func scenario(t *testing.T, desc string, input scenarioInput, f func(ctx *scenar
f(sCtx)
})
}
type mockPluginsSettingsService struct {
pluginSetting *models.PluginSetting
err error
}
func (s *mockPluginsSettingsService) GetPluginSettingById(ctx context.Context, query *models.GetPluginSettingByIdQuery) error {
query.Result = s.pluginSetting
return s.err
}
func (s *mockPluginsSettingsService) UpdatePluginSettingVersion(ctx context.Context, cmd *models.UpdatePluginSettingVersionCmd) error {
return s.err
}
func (s *mockPluginsSettingsService) UpdatePluginSetting(ctx context.Context, cmd *models.UpdatePluginSettingCmd) error {
return s.err
}

View File

@ -12,7 +12,7 @@ import (
"github.com/grafana/grafana/pkg/services/sqlstore"
)
type Service struct {
type ServiceImpl struct {
Bus bus.Bus
SQLStore *sqlstore.SQLStore
SecretsService secrets.Service
@ -21,6 +21,12 @@ type Service struct {
pluginSettingDecryptionCache secureJSONDecryptionCache
}
type Service interface {
GetPluginSettingById(ctx context.Context, query *models.GetPluginSettingByIdQuery) error
UpdatePluginSetting(ctx context.Context, cmd *models.UpdatePluginSettingCmd) error
UpdatePluginSettingVersion(ctx context.Context, cmd *models.UpdatePluginSettingVersionCmd) error
}
type cachedDecryptedJSON struct {
updated time.Time
json map[string]string
@ -31,8 +37,8 @@ type secureJSONDecryptionCache struct {
sync.Mutex
}
func ProvideService(bus bus.Bus, store *sqlstore.SQLStore, secretsService secrets.Service) *Service {
s := &Service{
func ProvideService(bus bus.Bus, store *sqlstore.SQLStore, secretsService secrets.Service) *ServiceImpl {
s := &ServiceImpl{
Bus: bus,
SQLStore: store,
SecretsService: secretsService,
@ -49,11 +55,11 @@ func ProvideService(bus bus.Bus, store *sqlstore.SQLStore, secretsService secret
return s
}
func (s *Service) GetPluginSettingById(ctx context.Context, query *models.GetPluginSettingByIdQuery) error {
func (s *ServiceImpl) GetPluginSettingById(ctx context.Context, query *models.GetPluginSettingByIdQuery) error {
return s.SQLStore.GetPluginSettingById(ctx, query)
}
func (s *Service) UpdatePluginSetting(ctx context.Context, cmd *models.UpdatePluginSettingCmd) error {
func (s *ServiceImpl) UpdatePluginSetting(ctx context.Context, cmd *models.UpdatePluginSettingCmd) error {
var err error
cmd.EncryptedSecureJsonData, err = s.SecretsService.EncryptJsonData(ctx, cmd.SecureJsonData, secrets.WithoutScope())
if err != nil {
@ -63,11 +69,11 @@ func (s *Service) UpdatePluginSetting(ctx context.Context, cmd *models.UpdatePlu
return s.SQLStore.UpdatePluginSetting(ctx, cmd)
}
func (s *Service) UpdatePluginSettingVersion(ctx context.Context, cmd *models.UpdatePluginSettingVersionCmd) error {
func (s *ServiceImpl) UpdatePluginSettingVersion(ctx context.Context, cmd *models.UpdatePluginSettingVersionCmd) error {
return s.SQLStore.UpdatePluginSettingVersion(ctx, cmd)
}
func (s *Service) DecryptedValues(ps *models.PluginSetting) map[string]string {
func (s *ServiceImpl) DecryptedValues(ps *models.PluginSetting) map[string]string {
s.pluginSettingDecryptionCache.Lock()
defer s.pluginSettingDecryptionCache.Unlock()