Plugins: Add plugin settings DTO (#46283)

* add clearer service layer

* re-order frontend settings for clarity

* fix fetch fail

* fix API response

* fix mockstore

* in -> where

pkg/api/frontendsettings.go

@@ -8,129 +8,20 @@ import (
 	"github.com/grafana/grafana/pkg/plugins"
 	"github.com/grafana/grafana/pkg/services/accesscontrol"
 	"github.com/grafana/grafana/pkg/services/licensing"
+	"github.com/grafana/grafana/pkg/services/pluginsettings"
 	"github.com/grafana/grafana/pkg/setting"
 	"github.com/grafana/grafana/pkg/tsdb/grafanads"
 	"github.com/grafana/grafana/pkg/util"
 )
 
-func (hs *HTTPServer) getFSDataSources(c *models.ReqContext, enabledPlugins EnabledPlugins) (map[string]plugins.DataSourceDTO, error) {
-	orgDataSources := make([]*models.DataSource, 0)
-
-	if c.OrgId != 0 {
-		query := models.GetDataSourcesQuery{OrgId: c.OrgId, DataSourceLimit: hs.Cfg.DataSourceLimit}
-		err := hs.SQLStore.GetDataSources(c.Req.Context(), &query)
-
-		if err != nil {
-			return nil, err
-		}
-
-		filtered, err := hs.filterDatasourcesByQueryPermission(c.Req.Context(), c.SignedInUser, query.Result)
-		if err != nil {
-			return nil, err
-		}
-
-		orgDataSources = filtered
+func (hs *HTTPServer) GetFrontendSettings(c *models.ReqContext) {
+	settings, err := hs.getFrontendSettingsMap(c)
+	if err != nil {
+		c.JsonApiErr(400, "Failed to get frontend settings", err)
+		return
 	}
 
-	dataSources := make(map[string]plugins.DataSourceDTO)
-
-	for _, ds := range orgDataSources {
-		url := ds.Url
-
-		if ds.Access == models.DS_ACCESS_PROXY {
-			url = "/api/datasources/proxy/" + strconv.FormatInt(ds.Id, 10)
-		}
-
-		dsDTO := plugins.DataSourceDTO{
-			ID:        ds.Id,
-			UID:       ds.Uid,
-			Type:      ds.Type,
-			Name:      ds.Name,
-			URL:       url,
-			IsDefault: ds.IsDefault,
-			Access:    string(ds.Access),
-		}
-
-		plugin, exists := enabledPlugins.Get(plugins.DataSource, ds.Type)
-		if !exists {
-			c.Logger.Error("Could not find plugin definition for data source", "datasource_type", ds.Type)
-			continue
-		}
-		dsDTO.Preload = plugin.Preload
-		dsDTO.Module = plugin.Module
-		dsDTO.PluginMeta = &plugins.PluginMetaDTO{
-			JSONData:  plugin.JSONData,
-			Signature: plugin.Signature,
-			Module:    plugin.Module,
-			BaseURL:   plugin.BaseURL,
-		}
-
-		if ds.JsonData == nil {
-			dsDTO.JSONData = make(map[string]interface{})
-		} else {
-			dsDTO.JSONData = ds.JsonData.MustMap()
-		}
-
-		if ds.Access == models.DS_ACCESS_DIRECT {
-			if ds.BasicAuth {
-				dsDTO.BasicAuth = util.GetBasicAuthHeader(
-					ds.BasicAuthUser,
-					hs.DataSourcesService.DecryptedBasicAuthPassword(ds),
-				)
-			}
-			if ds.WithCredentials {
-				dsDTO.WithCredentials = ds.WithCredentials
-			}
-
-			if ds.Type == models.DS_INFLUXDB_08 {
-				dsDTO.Username = ds.User
-				dsDTO.Password = hs.DataSourcesService.DecryptedPassword(ds)
-				dsDTO.URL = url + "/db/" + ds.Database
-			}
-
-			if ds.Type == models.DS_INFLUXDB {
-				dsDTO.Username = ds.User
-				dsDTO.Password = hs.DataSourcesService.DecryptedPassword(ds)
-				dsDTO.URL = url
-			}
-		}
-
-		if (ds.Type == models.DS_INFLUXDB) || (ds.Type == models.DS_ES) {
-			dsDTO.Database = ds.Database
-		}
-
-		if ds.Type == models.DS_PROMETHEUS {
-			// add unproxied server URL for link to Prometheus web UI
-			ds.JsonData.Set("directUrl", ds.Url)
-		}
-
-		dataSources[ds.Name] = dsDTO
-	}
-
-	// add data sources that are built in (meaning they are not added via data sources page, nor have any entry in
-	// the datasource table)
-	for _, ds := range hs.pluginStore.Plugins(c.Req.Context(), plugins.DataSource) {
-		if ds.BuiltIn {
-			dto := plugins.DataSourceDTO{
-				Type:     string(ds.Type),
-				Name:     ds.Name,
-				JSONData: make(map[string]interface{}),
-				PluginMeta: &plugins.PluginMetaDTO{
-					JSONData:  ds.JSONData,
-					Signature: ds.Signature,
-					Module:    ds.Module,
-					BaseURL:   ds.BaseURL,
-				},
-			}
-			if ds.Name == grafanads.DatasourceName {
-				dto.ID = grafanads.DatasourceID
-				dto.UID = grafanads.DatasourceUID
-			}
-			dataSources[ds.Name] = dto
-		}
-	}
-
-	return dataSources, nil
+	c.JSON(200, settings)
 }
 
 // getFrontendSettingsMap returns a json object with all the settings needed for front end initialisation.
@@ -290,6 +181,126 @@ func (hs *HTTPServer) getFrontendSettingsMap(c *models.ReqContext) (map[string]i
 	return jsonObj, nil
 }
 
+func (hs *HTTPServer) getFSDataSources(c *models.ReqContext, enabledPlugins EnabledPlugins) (map[string]plugins.DataSourceDTO, error) {
+	orgDataSources := make([]*models.DataSource, 0)
+
+	if c.OrgId != 0 {
+		query := models.GetDataSourcesQuery{OrgId: c.OrgId, DataSourceLimit: hs.Cfg.DataSourceLimit}
+		err := hs.SQLStore.GetDataSources(c.Req.Context(), &query)
+
+		if err != nil {
+			return nil, err
+		}
+
+		filtered, err := hs.filterDatasourcesByQueryPermission(c.Req.Context(), c.SignedInUser, query.Result)
+		if err != nil {
+			return nil, err
+		}
+
+		orgDataSources = filtered
+	}
+
+	dataSources := make(map[string]plugins.DataSourceDTO)
+
+	for _, ds := range orgDataSources {
+		url := ds.Url
+
+		if ds.Access == models.DS_ACCESS_PROXY {
+			url = "/api/datasources/proxy/" + strconv.FormatInt(ds.Id, 10)
+		}
+
+		dsDTO := plugins.DataSourceDTO{
+			ID:        ds.Id,
+			UID:       ds.Uid,
+			Type:      ds.Type,
+			Name:      ds.Name,
+			URL:       url,
+			IsDefault: ds.IsDefault,
+			Access:    string(ds.Access),
+		}
+
+		plugin, exists := enabledPlugins.Get(plugins.DataSource, ds.Type)
+		if !exists {
+			c.Logger.Error("Could not find plugin definition for data source", "datasource_type", ds.Type)
+			continue
+		}
+		dsDTO.Preload = plugin.Preload
+		dsDTO.Module = plugin.Module
+		dsDTO.PluginMeta = &plugins.PluginMetaDTO{
+			JSONData:  plugin.JSONData,
+			Signature: plugin.Signature,
+			Module:    plugin.Module,
+			BaseURL:   plugin.BaseURL,
+		}
+
+		if ds.JsonData == nil {
+			dsDTO.JSONData = make(map[string]interface{})
+		} else {
+			dsDTO.JSONData = ds.JsonData.MustMap()
+		}
+
+		if ds.Access == models.DS_ACCESS_DIRECT {
+			if ds.BasicAuth {
+				dsDTO.BasicAuth = util.GetBasicAuthHeader(
+					ds.BasicAuthUser,
+					hs.DataSourcesService.DecryptedBasicAuthPassword(ds),
+				)
+			}
+			if ds.WithCredentials {
+				dsDTO.WithCredentials = ds.WithCredentials
+			}
+
+			if ds.Type == models.DS_INFLUXDB_08 {
+				dsDTO.Username = ds.User
+				dsDTO.Password = hs.DataSourcesService.DecryptedPassword(ds)
+				dsDTO.URL = url + "/db/" + ds.Database
+			}
+
+			if ds.Type == models.DS_INFLUXDB {
+				dsDTO.Username = ds.User
+				dsDTO.Password = hs.DataSourcesService.DecryptedPassword(ds)
+				dsDTO.URL = url
+			}
+		}
+
+		if (ds.Type == models.DS_INFLUXDB) || (ds.Type == models.DS_ES) {
+			dsDTO.Database = ds.Database
+		}
+
+		if ds.Type == models.DS_PROMETHEUS {
+			// add unproxied server URL for link to Prometheus web UI
+			ds.JsonData.Set("directUrl", ds.Url)
+		}
+
+		dataSources[ds.Name] = dsDTO
+	}
+
+	// add data sources that are built in (meaning they are not added via data sources page, nor have any entry in
+	// the datasource table)
+	for _, ds := range hs.pluginStore.Plugins(c.Req.Context(), plugins.DataSource) {
+		if ds.BuiltIn {
+			dto := plugins.DataSourceDTO{
+				Type:     string(ds.Type),
+				Name:     ds.Name,
+				JSONData: make(map[string]interface{}),
+				PluginMeta: &plugins.PluginMetaDTO{
+					JSONData:  ds.JSONData,
+					Signature: ds.Signature,
+					Module:    ds.Module,
+					BaseURL:   ds.BaseURL,
+				},
+			}
+			if ds.Name == grafanads.DatasourceName {
+				dto.ID = grafanads.DatasourceID
+				dto.UID = grafanads.DatasourceUID
+			}
+			dataSources[ds.Name] = dto
+		}
+	}
+
+	return dataSources, nil
+}
+
 func getPanelSort(id string) int {
 	sort := 100
 	switch id {
@@ -331,16 +342,6 @@ func getPanelSort(id string) int {
 	return sort
 }
 
-func (hs *HTTPServer) GetFrontendSettings(c *models.ReqContext) {
-	settings, err := hs.getFrontendSettingsMap(c)
-	if err != nil {
-		c.JsonApiErr(400, "Failed to get frontend settings", err)
-		return
-	}
-
-	c.JSON(200, settings)
-}
-
 // EnabledPlugins represents a mapping from plugin types (panel, data source, etc.) to plugin IDs to plugins
 // For example ["panel"] -> ["piechart"] -> {pie chart plugin DTO}
 type EnabledPlugins map[plugins.Type]map[string]plugins.PluginDTO
@@ -389,15 +390,15 @@ func (hs *HTTPServer) enabledPlugins(ctx context.Context, orgID int64) (EnabledP
 	return ep, nil
 }
 
-func (hs *HTTPServer) pluginSettings(ctx context.Context, orgID int64) (map[string]*models.PluginSettingInfoDTO, error) {
-	pluginSettings := make(map[string]*models.PluginSettingInfoDTO)
+func (hs *HTTPServer) pluginSettings(ctx context.Context, orgID int64) (map[string]*pluginsettings.DTO, error) {
+	pluginSettings := make(map[string]*pluginsettings.DTO)
 
 	// fill settings from database
-	if pss, err := hs.PluginSettings.GetPluginSettings(ctx, orgID); err != nil {
+	if pss, err := hs.PluginSettings.GetPluginSettings(ctx, &pluginsettings.GetArgs{OrgID: orgID}); err != nil {
 		return nil, err
 	} else {
 		for _, ps := range pss {
-			pluginSettings[ps.PluginId] = ps
+			pluginSettings[ps.PluginID] = ps
 		}
 	}
 
@@ -409,9 +410,9 @@ func (hs *HTTPServer) pluginSettings(ctx context.Context, orgID int64) (map[stri
 		}
 
 		// add new setting which is enabled depending on if AutoEnabled: true
-		pluginSetting := &models.PluginSettingInfoDTO{
-			PluginId: plugin.ID,
-			OrgId:    orgID,
+		pluginSetting := &pluginsettings.DTO{
+			PluginID: plugin.ID,
+			OrgID:    orgID,
 			Enabled:  plugin.AutoEnabled,
 			Pinned:   plugin.AutoEnabled,
 		}
@@ -427,9 +428,9 @@ func (hs *HTTPServer) pluginSettings(ctx context.Context, orgID int64) (map[stri
 		}
 
 		// add new setting which is enabled by default
-		pluginSetting := &models.PluginSettingInfoDTO{
-			PluginId: plugin.ID,
-			OrgId:    orgID,
+		pluginSetting := &pluginsettings.DTO{
+			PluginID: plugin.ID,
+			OrgID:    orgID,
 			Enabled:  true,
 		}
 

pkg/api/pluginproxy/pluginproxy.go

@@ -24,20 +24,21 @@ type templateData struct {
 func NewApiPluginProxy(ctx *models.ReqContext, proxyPath string, route *plugins.Route,
 	appID string, cfg *setting.Cfg, pluginSettingsService pluginsettings.Service, secretsService secrets.Service) *httputil.ReverseProxy {
 	director := func(req *http.Request) {
-		query := models.GetPluginSettingByIdQuery{OrgId: ctx.OrgId, PluginId: appID}
-		if err := pluginSettingsService.GetPluginSettingById(ctx.Req.Context(), &query); err != nil {
+		query := pluginsettings.GetByPluginIDArgs{OrgID: ctx.OrgId, PluginID: appID}
+		ps, err := pluginSettingsService.GetPluginSettingByPluginID(ctx.Req.Context(), &query)
+		if err != nil {
 			ctx.JsonApiErr(500, "Failed to fetch plugin settings", err)
 			return
 		}
 
-		secureJsonData, err := secretsService.DecryptJsonData(ctx.Req.Context(), query.Result.SecureJsonData)
+		secureJsonData, err := secretsService.DecryptJsonData(ctx.Req.Context(), ps.SecureJSONData)
 		if err != nil {
 			ctx.JsonApiErr(500, "Failed to decrypt plugin settings", err)
 			return
 		}
 
 		data := templateData{
-			JsonData:       query.Result.JsonData,
+			JsonData:       ps.JSONData,
 			SecureJsonData: secureJsonData,
 		}
 

pkg/api/pluginproxy/pluginproxy_test.go

@@ -32,8 +32,8 @@ func TestPluginProxy(t *testing.T) {
 
 		store := &mockPluginsSettingsService{}
 		key, _ := secretsService.Encrypt(context.Background(), []byte("123"), secrets.WithoutScope())
-		store.pluginSetting = &models.PluginSetting{
-			SecureJsonData: map[string][]byte{
+		store.pluginSetting = &pluginsettings.DTO{
+			SecureJSONData: map[string][]byte{
 				"key": key,
 			},
 		}
@@ -65,7 +65,7 @@ func TestPluginProxy(t *testing.T) {
 		require.NoError(t, err)
 
 		store := &mockPluginsSettingsService{}
-		store.pluginSetting = &models.PluginSetting{}
+		store.pluginSetting = &pluginsettings.DTO{}
 
 		req := getPluginProxiedRequest(
 			t,
@@ -92,7 +92,7 @@ func TestPluginProxy(t *testing.T) {
 		require.NoError(t, err)
 
 		store := &mockPluginsSettingsService{}
-		store.pluginSetting = &models.PluginSetting{}
+		store.pluginSetting = &pluginsettings.DTO{}
 
 		req := getPluginProxiedRequest(
 			t,
@@ -118,7 +118,7 @@ func TestPluginProxy(t *testing.T) {
 		require.NoError(t, err)
 
 		store := &mockPluginsSettingsService{}
-		store.pluginSetting = &models.PluginSetting{}
+		store.pluginSetting = &pluginsettings.DTO{}
 
 		req := getPluginProxiedRequest(
 			t,
@@ -145,8 +145,8 @@ func TestPluginProxy(t *testing.T) {
 		}
 
 		store := &mockPluginsSettingsService{}
-		store.pluginSetting = &models.PluginSetting{
-			JsonData: map[string]interface{}{
+		store.pluginSetting = &pluginsettings.DTO{
+			JSONData: map[string]interface{}{
 				"dynamicUrl": "https://dynamic.grafana.com",
 			},
 		}
@@ -180,7 +180,7 @@ func TestPluginProxy(t *testing.T) {
 		}
 
 		store := &mockPluginsSettingsService{}
-		store.pluginSetting = &models.PluginSetting{}
+		store.pluginSetting = &pluginsettings.DTO{}
 
 		httpReq, err := http.NewRequest(http.MethodGet, "", nil)
 		require.NoError(t, err)
@@ -216,9 +216,9 @@ func TestPluginProxy(t *testing.T) {
 			map[string]string{"key": "123"},
 			secrets.WithoutScope(),
 		)
-		store.pluginSetting = &models.PluginSetting{
-			JsonData:       map[string]interface{}{"dynamicUrl": "https://dynamic.grafana.com"},
-			SecureJsonData: encryptedJsonData,
+		store.pluginSetting = &pluginsettings.DTO{
+			JSONData:       map[string]interface{}{"dynamicUrl": "https://dynamic.grafana.com"},
+			SecureJSONData: encryptedJsonData,
 		}
 
 		httpReq, err := http.NewRequest(http.MethodGet, "", nil)
@@ -268,8 +268,8 @@ func TestPluginProxy(t *testing.T) {
 			},
 		}
 		pluginSettingsService := &mockPluginsSettingsService{
-			pluginSetting: &models.PluginSetting{
-				SecureJsonData: map[string][]byte{},
+			pluginSetting: &pluginsettings.DTO{
+				SecureJSONData: map[string][]byte{},
 			},
 		}
 		proxy := NewApiPluginProxy(ctx, "", route, "", &setting.Cfg{}, pluginSettingsService, secretsService)
@@ -304,27 +304,26 @@ func getPluginProxiedRequest(t *testing.T, secretsService secrets.Service, ctx *
 }
 
 type mockPluginsSettingsService struct {
-	pluginSetting *models.PluginSetting
+	pluginSetting *pluginsettings.DTO
 	err           error
 }
 
-func (s *mockPluginsSettingsService) GetPluginSettings(_ context.Context, _ int64) ([]*models.PluginSettingInfoDTO, error) {
+func (s *mockPluginsSettingsService) GetPluginSettings(_ context.Context, _ *pluginsettings.GetArgs) ([]*pluginsettings.DTO, error) {
 	return nil, s.err
 }
 
-func (s *mockPluginsSettingsService) GetPluginSettingById(_ context.Context, query *models.GetPluginSettingByIdQuery) error {
-	query.Result = s.pluginSetting
+func (s *mockPluginsSettingsService) GetPluginSettingByPluginID(_ context.Context, _ *pluginsettings.GetByPluginIDArgs) (*pluginsettings.DTO, error) {
+	return s.pluginSetting, s.err
+}
+
+func (s *mockPluginsSettingsService) UpdatePluginSettingPluginVersion(_ context.Context, _ *pluginsettings.UpdatePluginVersionArgs) error {
 	return s.err
 }
 
-func (s *mockPluginsSettingsService) UpdatePluginSettingVersion(_ context.Context, _ *models.UpdatePluginSettingVersionCmd) error {
+func (s *mockPluginsSettingsService) UpdatePluginSetting(_ context.Context, _ *pluginsettings.UpdateArgs) error {
 	return s.err
 }
 
-func (s *mockPluginsSettingsService) UpdatePluginSetting(_ context.Context, _ *models.UpdatePluginSettingCmd) error {
-	return s.err
-}
-
-func (s *mockPluginsSettingsService) DecryptedValues(_ *models.PluginSetting) map[string]string {
+func (s *mockPluginsSettingsService) DecryptedValues(_ *pluginsettings.DTO) map[string]string {
 	return nil
 }

pkg/api/plugins.go

@@ -24,6 +24,7 @@ import (
 	"github.com/grafana/grafana/pkg/plugins"
 	"github.com/grafana/grafana/pkg/plugins/backendplugin"
 	"github.com/grafana/grafana/pkg/plugins/manager/installer"
+	"github.com/grafana/grafana/pkg/services/pluginsettings"
 	"github.com/grafana/grafana/pkg/setting"
 	"github.com/grafana/grafana/pkg/util/errutil"
 	"github.com/grafana/grafana/pkg/util/proxyutil"
@@ -142,15 +143,18 @@ func (hs *HTTPServer) GetPluginSettingByID(c *models.ReqContext) response.Respon
 		dto.Pinned = plugin.AutoEnabled
 	}
 
-	query := models.GetPluginSettingByIdQuery{PluginId: pluginID, OrgId: c.OrgId}
-	if err := hs.PluginSettings.GetPluginSettingById(c.Req.Context(), &query); err != nil {
+	ps, err := hs.PluginSettings.GetPluginSettingByPluginID(c.Req.Context(), &pluginsettings.GetByPluginIDArgs{
+		PluginID: pluginID,
+		OrgID:    c.OrgId,
+	})
+	if err != nil {
 		if !errors.Is(err, models.ErrPluginSettingNotFound) {
-			return response.Error(500, "Failed to get login settings", nil)
+			return response.Error(http.StatusInternalServerError, "Failed to get plugin settings", nil)
 		}
 	} else {
-		dto.Enabled = query.Result.Enabled
-		dto.Pinned = query.Result.Pinned
-		dto.JsonData = query.Result.JsonData
+		dto.Enabled = ps.Enabled
+		dto.Pinned = ps.Pinned
+		dto.JsonData = ps.JSONData
 	}
 
 	update, exists := hs.pluginsUpdateChecker.HasUpdate(c.Req.Context(), plugin.ID)
@@ -175,7 +179,16 @@ func (hs *HTTPServer) UpdatePluginSetting(c *models.ReqContext) response.Respons
 
 	cmd.OrgId = c.OrgId
 	cmd.PluginId = pluginID
-	if err := hs.PluginSettings.UpdatePluginSetting(c.Req.Context(), &cmd); err != nil {
+	if err := hs.PluginSettings.UpdatePluginSetting(c.Req.Context(), &pluginsettings.UpdateArgs{
+		Enabled:                 cmd.Enabled,
+		Pinned:                  cmd.Pinned,
+		JSONData:                cmd.JsonData,
+		SecureJSONData:          cmd.SecureJsonData,
+		PluginVersion:           cmd.PluginVersion,
+		PluginID:                cmd.PluginId,
+		OrgID:                   cmd.OrgId,
+		EncryptedSecureJSONData: cmd.EncryptedSecureJsonData,
+	}); err != nil {
 		return response.Error(500, "Failed to update plugin setting", err)
 	}