PluginManager: Make remaining plugin state non-global (#32094)

* PluginDashboards: Use plugin manager interface

Signed-off-by: Arve Knudsen <arve.knudsen@gmail.com>

* PluginManager: Make panels non-global

Signed-off-by: Arve Knudsen <arve.knudsen@gmail.com>

* PluginManager: Make apps non-global

Signed-off-by: Arve Knudsen <arve.knudsen@gmail.com>

* PluginManager: Make static routes non-global

Signed-off-by: Arve Knudsen <arve.knudsen@gmail.com>

* PluginManager: Make pluginTypes non-global

Signed-off-by: Arve Knudsen <arve.knudsen@gmail.com>

pkg/api/api.go

@@ -275,7 +275,7 @@ func (hs *HTTPServer) registerRoutes() {
 
 		apiRoute.Group("/plugins", func(pluginRoute routing.RouteRegister) {
 			pluginRoute.Get("/:pluginId/dashboards/", routing.Wrap(hs.GetPluginDashboards))
-			pluginRoute.Post("/:pluginId/settings", bind(models.UpdatePluginSettingCmd{}), routing.Wrap(UpdatePluginSetting))
+			pluginRoute.Post("/:pluginId/settings", bind(models.UpdatePluginSettingCmd{}), routing.Wrap(hs.UpdatePluginSetting))
 			pluginRoute.Get("/:pluginId/metrics", routing.Wrap(hs.CollectPluginMetrics))
 		}, reqOrgAdmin)
 
@@ -447,6 +447,7 @@ func (hs *HTTPServer) registerRoutes() {
 	r.Delete("/api/snapshots/:key", reqEditorRole, routing.Wrap(DeleteDashboardSnapshot))
 
 	// Frontend logs
-	sourceMapStore := frontendlogging.NewSourceMapStore(hs.Cfg, frontendlogging.ReadSourceMapFromFS)
-	r.Post("/log", middleware.RateLimit(hs.Cfg.Sentry.EndpointRPS, hs.Cfg.Sentry.EndpointBurst, time.Now), bind(frontendlogging.FrontendSentryEvent{}), routing.Wrap(NewFrontendLogMessageHandler(sourceMapStore)))
+	sourceMapStore := frontendlogging.NewSourceMapStore(hs.Cfg, hs.PluginManager, frontendlogging.ReadSourceMapFromFS)
+	r.Post("/log", middleware.RateLimit(hs.Cfg.Sentry.EndpointRPS, hs.Cfg.Sentry.EndpointBurst, time.Now),
+		bind(frontendlogging.FrontendSentryEvent{}), routing.Wrap(NewFrontendLogMessageHandler(sourceMapStore)))
 }

pkg/api/app_routes.go

@@ -11,7 +11,6 @@ import (
 	"github.com/grafana/grafana/pkg/middleware"
 	"github.com/grafana/grafana/pkg/models"
 	"github.com/grafana/grafana/pkg/plugins"
-	"github.com/grafana/grafana/pkg/plugins/manager"
 	"github.com/grafana/grafana/pkg/util"
 	macaron "gopkg.in/macaron.v1"
 )
@@ -32,7 +31,7 @@ func (hs *HTTPServer) initAppPluginRoutes(r *macaron.Macaron) {
 		TLSHandshakeTimeout: 10 * time.Second,
 	}
 
-	for _, plugin := range manager.Apps {
+	for _, plugin := range hs.PluginManager.Apps() {
 		for _, route := range plugin.Routes {
 			url := util.JoinURLFragments("/api/plugin-proxy/"+plugin.Id, route.Path)
 			handlers := make([]macaron.Handler, 0)

pkg/api/fakes.go

@@ -4,6 +4,8 @@ import "github.com/grafana/grafana/pkg/plugins"
 
 type fakePluginManager struct {
 	plugins.Manager
+
+	staticRoutes []*plugins.PluginStaticRoute
 }
 
 func (pm *fakePluginManager) GetPlugin(id string) *plugins.PluginBase {
@@ -17,3 +19,7 @@ func (pm *fakePluginManager) GetDataSource(id string) *plugins.DataSourcePlugin
 func (pm *fakePluginManager) Renderer() *plugins.RendererPlugin {
 	return nil
 }
+
+func (pm *fakePluginManager) StaticRoutes() []*plugins.PluginStaticRoute {
+	return pm.staticRoutes
+}

pkg/api/frontend_logging_test.go

@@ -16,7 +16,6 @@ import (
 	"github.com/grafana/grafana/pkg/api/routing"
 	"github.com/grafana/grafana/pkg/models"
 	"github.com/grafana/grafana/pkg/plugins"
-	"github.com/grafana/grafana/pkg/plugins/manager"
 	"github.com/grafana/grafana/pkg/setting"
 	log "github.com/inconshreveable/log15"
 
@@ -71,7 +70,17 @@ func logSentryEventScenario(t *testing.T, desc string, event frontendlogging.Fro
 			return nil, os.ErrNotExist
 		}
 
-		sourceMapStore := frontendlogging.NewSourceMapStore(cfg, readSourceMap)
+		// fake plugin route so we will try to find a source map there
+		pm := fakePluginManager{
+			staticRoutes: []*plugins.PluginStaticRoute{
+				{
+					Directory: "/usr/local/telepathic-panel",
+					PluginId:  "telepathic",
+				},
+			},
+		}
+
+		sourceMapStore := frontendlogging.NewSourceMapStore(cfg, &pm, readSourceMap)
 
 		loggingHandler := NewFrontendLogMessageHandler(sourceMapStore)
 
@@ -90,12 +99,6 @@ func TestFrontendLoggingEndpoint(t *testing.T) {
 	ts, err := time.Parse("2006-01-02T15:04:05.000Z", "2020-10-22T06:29:29.078Z")
 	require.NoError(t, err)
 
-	// fake plugin route so we will try to find a source map there. I can't believe I can do this
-	manager.StaticRoutes = append(manager.StaticRoutes, &plugins.PluginStaticRoute{
-		Directory: "/usr/local/telepathic-panel",
-		PluginId:  "telepathic",
-	})
-
 	t.Run("FrontendLoggingEndpoint", func(t *testing.T) {
 		request := sentry.Request{
 			URL: "http://localhost:3000/",
@@ -144,19 +147,20 @@ func TestFrontendLoggingEndpoint(t *testing.T) {
 			},
 		}
 
-		logSentryEventScenario(t, "Should log received error event", errorEvent, func(sc *scenarioContext, logs []*log.Record, sourceMapReads []SourceMapReadRecord) {
-			assert.Equal(t, 200, sc.resp.Code)
-			assert.Len(t, logs, 1)
-			assertContextContains(t, logs[0], "logger", "frontend")
-			assertContextContains(t, logs[0], "url", errorEvent.Request.URL)
-			assertContextContains(t, logs[0], "user_agent", errorEvent.Request.Headers["User-Agent"])
-			assertContextContains(t, logs[0], "event_id", errorEvent.EventID)
-			assertContextContains(t, logs[0], "original_timestamp", errorEvent.Timestamp)
-			assertContextContains(t, logs[0], "stacktrace", `UserError: Please replace user and try again
+		logSentryEventScenario(t, "Should log received error event", errorEvent,
+			func(sc *scenarioContext, logs []*log.Record, sourceMapReads []SourceMapReadRecord) {
+				assert.Equal(t, 200, sc.resp.Code)
+				assert.Len(t, logs, 1)
+				assertContextContains(t, logs[0], "logger", "frontend")
+				assertContextContains(t, logs[0], "url", errorEvent.Request.URL)
+				assertContextContains(t, logs[0], "user_agent", errorEvent.Request.Headers["User-Agent"])
+				assertContextContains(t, logs[0], "event_id", errorEvent.EventID)
+				assertContextContains(t, logs[0], "original_timestamp", errorEvent.Timestamp)
+				assertContextContains(t, logs[0], "stacktrace", `UserError: Please replace user and try again
   at foofn (foo.js:123:23)
   at barfn (bar.js:113:231)`)
-			assert.NotContains(t, logs[0].Ctx, "context")
-		})
+				assert.NotContains(t, logs[0].Ctx, "context")
+			})
 
 		messageEvent := frontendlogging.FrontendSentryEvent{
 			Event: &sentry.Event{
@@ -170,21 +174,22 @@ func TestFrontendLoggingEndpoint(t *testing.T) {
 			Exception: nil,
 		}
 
-		logSentryEventScenario(t, "Should log received message event", messageEvent, func(sc *scenarioContext, logs []*log.Record, sourceMapReads []SourceMapReadRecord) {
-			assert.Equal(t, 200, sc.resp.Code)
-			assert.Len(t, logs, 1)
-			assert.Equal(t, "hello world", logs[0].Msg)
-			assert.Equal(t, log.LvlInfo, logs[0].Lvl)
-			assertContextContains(t, logs[0], "logger", "frontend")
-			assertContextContains(t, logs[0], "url", messageEvent.Request.URL)
-			assertContextContains(t, logs[0], "user_agent", messageEvent.Request.Headers["User-Agent"])
-			assertContextContains(t, logs[0], "event_id", messageEvent.EventID)
-			assertContextContains(t, logs[0], "original_timestamp", messageEvent.Timestamp)
-			assert.NotContains(t, logs[0].Ctx, "stacktrace")
-			assert.NotContains(t, logs[0].Ctx, "context")
-			assertContextContains(t, logs[0], "user_email", user.Email)
-			assertContextContains(t, logs[0], "user_id", user.ID)
-		})
+		logSentryEventScenario(t, "Should log received message event", messageEvent,
+			func(sc *scenarioContext, logs []*log.Record, sourceMapReads []SourceMapReadRecord) {
+				assert.Equal(t, 200, sc.resp.Code)
+				assert.Len(t, logs, 1)
+				assert.Equal(t, "hello world", logs[0].Msg)
+				assert.Equal(t, log.LvlInfo, logs[0].Lvl)
+				assertContextContains(t, logs[0], "logger", "frontend")
+				assertContextContains(t, logs[0], "url", messageEvent.Request.URL)
+				assertContextContains(t, logs[0], "user_agent", messageEvent.Request.Headers["User-Agent"])
+				assertContextContains(t, logs[0], "event_id", messageEvent.EventID)
+				assertContextContains(t, logs[0], "original_timestamp", messageEvent.Timestamp)
+				assert.NotContains(t, logs[0].Ctx, "stacktrace")
+				assert.NotContains(t, logs[0].Ctx, "context")
+				assertContextContains(t, logs[0], "user_email", user.Email)
+				assertContextContains(t, logs[0], "user_id", user.ID)
+			})
 
 		eventWithContext := frontendlogging.FrontendSentryEvent{
 			Event: &sentry.Event{
@@ -205,13 +210,14 @@ func TestFrontendLoggingEndpoint(t *testing.T) {
 			Exception: nil,
 		}
 
-		logSentryEventScenario(t, "Should log event context", eventWithContext, func(sc *scenarioContext, logs []*log.Record, sourceMapReads []SourceMapReadRecord) {
-			assert.Equal(t, 200, sc.resp.Code)
-			assert.Len(t, logs, 1)
-			assertContextContains(t, logs[0], "context_foo_one", "two")
-			assertContextContains(t, logs[0], "context_foo_three", "4")
-			assertContextContains(t, logs[0], "context_bar", "baz")
-		})
+		logSentryEventScenario(t, "Should log event context", eventWithContext,
+			func(sc *scenarioContext, logs []*log.Record, sourceMapReads []SourceMapReadRecord) {
+				assert.Equal(t, 200, sc.resp.Code)
+				assert.Len(t, logs, 1)
+				assertContextContains(t, logs[0], "context_foo_one", "two")
+				assertContextContains(t, logs[0], "context_foo_three", "4")
+				assertContextContains(t, logs[0], "context_bar", "baz")
+			})
 
 		errorEventForSourceMapping := frontendlogging.FrontendSentryEvent{
 			Event: &event,
@@ -271,10 +277,11 @@ func TestFrontendLoggingEndpoint(t *testing.T) {
 			},
 		}
 
-		logSentryEventScenario(t, "Should load sourcemap and transform stacktrace line when possible", errorEventForSourceMapping, func(sc *scenarioContext, logs []*log.Record, sourceMapReads []SourceMapReadRecord) {
-			assert.Equal(t, 200, sc.resp.Code)
-			assert.Len(t, logs, 1)
-			assertContextContains(t, logs[0], "stacktrace", `UserError: Please replace user and try again
+		logSentryEventScenario(t, "Should load sourcemap and transform stacktrace line when possible",
+			errorEventForSourceMapping, func(sc *scenarioContext, logs []*log.Record, sourceMapReads []SourceMapReadRecord) {
+				assert.Equal(t, 200, sc.resp.Code)
+				assert.Len(t, logs, 1)
+				assertContextContains(t, logs[0], "stacktrace", `UserError: Please replace user and try again
   at ? (core|webpack:///./some_source.ts:2:2)
   at ? (telepathic|webpack:///./some_source.ts:3:2)
   at explode (http://localhost:3000/public/build/error.js:3:10)
@@ -282,20 +289,20 @@ func TestFrontendLoggingEndpoint(t *testing.T) {
   at nope (http://localhost:3000/baz.js:3:10)
   at fake (http://localhost:3000/public/build/../../secrets.txt:3:10)
   at ? (core|webpack:///./some_source.ts:3:2)`)
-			assert.Len(t, sourceMapReads, 6)
-			assert.Equal(t, "/staticroot", sourceMapReads[0].dir)
-			assert.Equal(t, "build/moo/foo.js.map", sourceMapReads[0].path)
-			assert.Equal(t, "/usr/local/telepathic-panel", sourceMapReads[1].dir)
-			assert.Equal(t, "/foo.js.map", sourceMapReads[1].path)
-			assert.Equal(t, "/staticroot", sourceMapReads[2].dir)
-			assert.Equal(t, "build/error.js.map", sourceMapReads[2].path)
-			assert.Equal(t, "/staticroot", sourceMapReads[3].dir)
-			assert.Equal(t, "build/bar.js.map", sourceMapReads[3].path)
-			assert.Equal(t, "/staticroot", sourceMapReads[4].dir)
-			assert.Equal(t, "secrets.txt.map", sourceMapReads[4].path)
-			assert.Equal(t, "/staticroot", sourceMapReads[5].dir)
-			assert.Equal(t, "build/foo.js.map", sourceMapReads[5].path)
-		})
+				assert.Len(t, sourceMapReads, 6)
+				assert.Equal(t, "/staticroot", sourceMapReads[0].dir)
+				assert.Equal(t, "build/moo/foo.js.map", sourceMapReads[0].path)
+				assert.Equal(t, "/usr/local/telepathic-panel", sourceMapReads[1].dir)
+				assert.Equal(t, "/foo.js.map", sourceMapReads[1].path)
+				assert.Equal(t, "/staticroot", sourceMapReads[2].dir)
+				assert.Equal(t, "build/error.js.map", sourceMapReads[2].path)
+				assert.Equal(t, "/staticroot", sourceMapReads[3].dir)
+				assert.Equal(t, "build/bar.js.map", sourceMapReads[3].path)
+				assert.Equal(t, "/staticroot", sourceMapReads[4].dir)
+				assert.Equal(t, "secrets.txt.map", sourceMapReads[4].path)
+				assert.Equal(t, "/staticroot", sourceMapReads[5].dir)
+				assert.Equal(t, "build/foo.js.map", sourceMapReads[5].path)
+			})
 	})
 }
 

pkg/api/frontendlogging/source_maps.go

@@ -12,7 +12,7 @@ import (
 	sourcemap "github.com/go-sourcemap/sourcemap"
 
 	"github.com/getsentry/sentry-go"
-	"github.com/grafana/grafana/pkg/plugins/manager"
+	"github.com/grafana/grafana/pkg/plugins"
 	"github.com/grafana/grafana/pkg/setting"
 )
 
@@ -47,12 +47,14 @@ type SourceMapStore struct {
 	cache         map[string]*sourceMap
 	cfg           *setting.Cfg
 	readSourceMap ReadSourceMapFn
+	pluginManager plugins.Manager
 }
 
-func NewSourceMapStore(cfg *setting.Cfg, readSourceMap ReadSourceMapFn) *SourceMapStore {
+func NewSourceMapStore(cfg *setting.Cfg, pluginManager plugins.Manager, readSourceMap ReadSourceMapFn) *SourceMapStore {
 	return &SourceMapStore{
 		cache:         make(map[string]*sourceMap),
 		cfg:           cfg,
+		pluginManager: pluginManager,
 		readSourceMap: readSourceMap,
 	}
 }
@@ -69,7 +71,8 @@ func (store *SourceMapStore) guessSourceMapLocation(sourceURL string) (*sourceMa
 	}
 
 	// determine if source comes from grafana core, locally or CDN, look in public build dir on fs
-	if strings.HasPrefix(u.Path, "/public/build/") || (store.cfg.CDNRootURL != nil && strings.HasPrefix(sourceURL, store.cfg.CDNRootURL.String()) && strings.Contains(u.Path, "/public/build/")) {
+	if strings.HasPrefix(u.Path, "/public/build/") || (store.cfg.CDNRootURL != nil &&
+		strings.HasPrefix(sourceURL, store.cfg.CDNRootURL.String()) && strings.Contains(u.Path, "/public/build/")) {
 		pathParts := strings.SplitN(u.Path, "/public/build/", 2)
 		if len(pathParts) == 2 {
 			return &sourceMapLocation{
@@ -80,7 +83,7 @@ func (store *SourceMapStore) guessSourceMapLocation(sourceURL string) (*sourceMa
 		}
 		// if source comes from a plugin, look in plugin dir
 	} else if strings.HasPrefix(u.Path, "/public/plugins/") {
-		for _, route := range manager.StaticRoutes {
+		for _, route := range store.pluginManager.StaticRoutes() {
 			pluginPrefix := filepath.Join("/public/plugins/", route.PluginId)
 			if strings.HasPrefix(u.Path, pluginPrefix) {
 				return &sourceMapLocation{

pkg/api/http_server.go

@@ -32,7 +32,6 @@ import (
 	"github.com/grafana/grafana/pkg/models"
 	"github.com/grafana/grafana/pkg/plugins/backendplugin"
 	_ "github.com/grafana/grafana/pkg/plugins/backendplugin/manager"
-	"github.com/grafana/grafana/pkg/plugins/manager"
 	"github.com/grafana/grafana/pkg/plugins/plugindashboards"
 	"github.com/grafana/grafana/pkg/registry"
 	"github.com/grafana/grafana/pkg/services/contexthandler"
@@ -321,7 +320,7 @@ func (hs *HTTPServer) addMiddlewaresAndStaticRoutes() {
 
 	m.Use(middleware.Recovery(hs.Cfg))
 
-	for _, route := range manager.StaticRoutes {
+	for _, route := range hs.PluginManager.StaticRoutes() {
 		pluginRoute := path.Join("/public/plugins/", route.PluginId)
 		hs.log.Debug("Plugins: Adding route", "route", pluginRoute, "dir", route.Directory)
 		hs.mapStatic(m, route.Directory, "", pluginRoute)

pkg/api/plugins.go

@@ -16,7 +16,6 @@ import (
 	"github.com/grafana/grafana/pkg/plugins"
 	"github.com/grafana/grafana/pkg/plugins/adapters"
 	"github.com/grafana/grafana/pkg/plugins/backendplugin"
-	"github.com/grafana/grafana/pkg/plugins/manager"
 	"github.com/grafana/grafana/pkg/util/errutil"
 )
 
@@ -168,7 +167,7 @@ func (hs *HTTPServer) GetPluginSettingByID(c *models.ReqContext) response.Respon
 		SignatureOrg:  def.SignatureOrg,
 	}
 
-	if app, ok := manager.Apps[def.Id]; ok {
+	if app := hs.PluginManager.GetApp(def.Id); app != nil {
 		dto.Enabled = app.AutoEnabled
 		dto.Pinned = app.AutoEnabled
 	}
@@ -187,16 +186,15 @@ func (hs *HTTPServer) GetPluginSettingByID(c *models.ReqContext) response.Respon
 	return response.JSON(200, dto)
 }
 
-func UpdatePluginSetting(c *models.ReqContext, cmd models.UpdatePluginSettingCmd) response.Response {
+func (hs *HTTPServer) UpdatePluginSetting(c *models.ReqContext, cmd models.UpdatePluginSettingCmd) response.Response {
 	pluginID := c.Params(":pluginId")
 
+	if app := hs.PluginManager.GetApp(pluginID); app == nil {
+		return response.Error(404, "Plugin not installed", nil)
+	}
+
 	cmd.OrgId = c.OrgId
 	cmd.PluginId = pluginID
-
-	if _, ok := manager.Apps[cmd.PluginId]; !ok {
-		return response.Error(404, "Plugin not installed.", nil)
-	}
-
 	if err := bus.Dispatch(&cmd); err != nil {
 		return response.Error(500, "Failed to update plugin setting", err)
 	}