From ce8fd14f1fa2c2c5fc2c345c0c57671918dd7a69 Mon Sep 17 00:00:00 2001 From: Will Browne Date: Thu, 14 Dec 2023 17:33:29 +0100 Subject: [PATCH] Plugins: Make renderer service load renderer plugin (#77854) * rendering service loads renderer plugin * update naming * tidy * apply PR feedback * fix missing feature manager * fix step * set plugin --- pkg/api/fakes.go | 9 +- pkg/api/frontendsettings_test.go | 2 +- .../backendplugin/provider/provider.go | 4 +- pkg/plugins/ifaces.go | 5 - .../manager/pipeline/discovery/steps.go | 37 +++++ .../pluginsintegration/pipeline/pipeline.go | 7 +- .../pluginsintegration/pluginsintegration.go | 5 +- .../pluginsintegration/pluginstore/store.go | 9 -- .../pluginstore/store_test.go | 19 --- .../pluginsintegration/renderer/renderer.go | 128 ++++++++++++++++++ pkg/services/rendering/capabilities_test.go | 5 +- pkg/services/rendering/plugin_mode.go | 17 ++- pkg/services/rendering/rendering.go | 31 +++-- pkg/services/rendering/rendering_test.go | 7 +- pkg/services/rendering/svgSanitizer.go | 6 +- 15 files changed, 220 insertions(+), 71 deletions(-) create mode 100644 pkg/services/pluginsintegration/renderer/renderer.go diff --git a/pkg/api/fakes.go b/pkg/api/fakes.go index 629bf56eb6c..209a27f0304 100644 --- a/pkg/api/fakes.go +++ b/pkg/api/fakes.go @@ -4,6 +4,7 @@ import ( "context" "github.com/grafana/grafana/pkg/plugins" + "github.com/grafana/grafana/pkg/services/rendering" ) type fakePluginInstaller struct { @@ -34,12 +35,12 @@ func (pm *fakePluginInstaller) Remove(_ context.Context, pluginID string) error return nil } -type fakeRendererManager struct { - plugins.RendererManager +type fakeRendererPluginManager struct { + rendering.PluginManager } -func (ps *fakeRendererManager) Renderer(_ context.Context) *plugins.Plugin { - return nil +func (ps *fakeRendererPluginManager) Renderer(_ context.Context) (rendering.Plugin, bool) { + return nil, false } type fakePluginStaticRouteResolver struct { diff --git a/pkg/api/frontendsettings_test.go b/pkg/api/frontendsettings_test.go index 6d3726ada27..4e4e53e89a8 100644 --- a/pkg/api/frontendsettings_test.go +++ b/pkg/api/frontendsettings_test.go @@ -65,7 +65,7 @@ func setupTestEnvironment(t *testing.T, cfg *setting.Cfg, features *featuremgmt. License: &licensing.OSSLicensingService{Cfg: cfg}, RenderService: &rendering.RenderingService{ Cfg: cfg, - RendererPluginManager: &fakeRendererManager{}, + RendererPluginManager: &fakeRendererPluginManager{}, }, SQLStore: db.InitTestDB(t), SettingsProvider: setting.ProvideProvider(cfg), diff --git a/pkg/plugins/backendplugin/provider/provider.go b/pkg/plugins/backendplugin/provider/provider.go index 9c5cc2b5354..685c06a2df4 100644 --- a/pkg/plugins/backendplugin/provider/provider.go +++ b/pkg/plugins/backendplugin/provider/provider.go @@ -21,7 +21,7 @@ type Service struct { func New(providers ...PluginBackendProvider) *Service { if len(providers) == 0 { - return New(RendererProvider, SecretsManagerProvider, DefaultProvider) + return New(SecretsManagerProvider, DefaultProvider) } return &Service{ providerChain: providers, @@ -29,7 +29,7 @@ func New(providers ...PluginBackendProvider) *Service { } func ProvideService(coreRegistry *coreplugin.Registry) *Service { - return New(coreRegistry.BackendFactoryProvider(), RendererProvider, SecretsManagerProvider, DefaultProvider) + return New(coreRegistry.BackendFactoryProvider(), SecretsManagerProvider, DefaultProvider) } func (s *Service) BackendFactory(ctx context.Context, p *plugins.Plugin) backendplugin.PluginFactoryFunc { diff --git a/pkg/plugins/ifaces.go b/pkg/plugins/ifaces.go index 16ff4d5fd13..a81e18c409a 100644 --- a/pkg/plugins/ifaces.go +++ b/pkg/plugins/ifaces.go @@ -99,11 +99,6 @@ type BackendFactoryProvider interface { BackendFactory(ctx context.Context, p *Plugin) backendplugin.PluginFactoryFunc } -type RendererManager interface { - // Renderer returns a renderer plugin. - Renderer(ctx context.Context) *Plugin -} - type SecretsPluginManager interface { // SecretsManager returns a secretsmanager plugin SecretsManager(ctx context.Context) *Plugin diff --git a/pkg/plugins/manager/pipeline/discovery/steps.go b/pkg/plugins/manager/pipeline/discovery/steps.go index ceb36d5c331..179eca4c12a 100644 --- a/pkg/plugins/manager/pipeline/discovery/steps.go +++ b/pkg/plugins/manager/pipeline/discovery/steps.go @@ -2,6 +2,7 @@ package discovery import ( "context" + "slices" "github.com/grafana/grafana/pkg/plugins" "github.com/grafana/grafana/pkg/plugins/config" @@ -53,3 +54,39 @@ func (d *DuplicatePluginValidation) Filter(ctx context.Context, bundles []*plugi return res, nil } + +// PermittedPluginTypesFilter is a filter step that will filter out any plugins that are not of a permitted type. +type PermittedPluginTypesFilter struct { + permittedTypes []plugins.Type +} + +// NewPermittedPluginTypesFilterStep returns a new FindFilterFunc for filtering out any plugins that are not of a +// permitted type. This includes both the primary plugin and any child plugins. +func NewPermittedPluginTypesFilterStep(permittedTypes []plugins.Type) FindFilterFunc { + f := &PermittedPluginTypesFilter{ + permittedTypes: permittedTypes, + } + return f.Filter +} + +// Filter will filter out any plugins that are not of a permitted type. +func (n *PermittedPluginTypesFilter) Filter(_ context.Context, _ plugins.Class, bundles []*plugins.FoundBundle) ([]*plugins.FoundBundle, error) { + var r []*plugins.FoundBundle + for _, b := range bundles { + if !slices.Contains(n.permittedTypes, b.Primary.JSONData.Type) { + continue + } + + prohibitedType := false + for _, child := range b.Children { + if !slices.Contains(n.permittedTypes, child.JSONData.Type) { + prohibitedType = true + break + } + } + if !prohibitedType { + r = append(r, b) + } + } + return r, nil +} diff --git a/pkg/services/pluginsintegration/pipeline/pipeline.go b/pkg/services/pluginsintegration/pipeline/pipeline.go index 2ac5b3ac068..1c2c9598e61 100644 --- a/pkg/services/pluginsintegration/pipeline/pipeline.go +++ b/pkg/services/pluginsintegration/pipeline/pipeline.go @@ -23,10 +23,11 @@ import ( func ProvideDiscoveryStage(cfg *config.Cfg, pf finder.Finder, pr registry.Service) *discovery.Discovery { return discovery.New(cfg, discovery.Opts{ - FindFunc: func(ctx context.Context, src plugins.PluginSource) ([]*plugins.FoundBundle, error) { - return pf.Find(ctx, src) - }, + FindFunc: pf.Find, FindFilterFuncs: []discovery.FindFilterFunc{ + discovery.NewPermittedPluginTypesFilterStep([]plugins.Type{ + plugins.TypeDataSource, plugins.TypeApp, plugins.TypePanel, plugins.TypeSecretsManager, + }), func(ctx context.Context, _ plugins.Class, b []*plugins.FoundBundle) ([]*plugins.FoundBundle, error) { return discovery.NewDuplicatePluginFilterStep(pr).Filter(ctx, b) }, diff --git a/pkg/services/pluginsintegration/pluginsintegration.go b/pkg/services/pluginsintegration/pluginsintegration.go index eee838fda52..f1c24607614 100644 --- a/pkg/services/pluginsintegration/pluginsintegration.go +++ b/pkg/services/pluginsintegration/pluginsintegration.go @@ -47,7 +47,9 @@ import ( "github.com/grafana/grafana/pkg/services/pluginsintegration/pluginsettings" pluginSettings "github.com/grafana/grafana/pkg/services/pluginsintegration/pluginsettings/service" "github.com/grafana/grafana/pkg/services/pluginsintegration/pluginstore" + "github.com/grafana/grafana/pkg/services/pluginsintegration/renderer" "github.com/grafana/grafana/pkg/services/pluginsintegration/serviceregistration" + "github.com/grafana/grafana/pkg/services/rendering" "github.com/grafana/grafana/pkg/setting" ) @@ -56,7 +58,6 @@ var WireSet = wire.NewSet( config.ProvideConfig, pluginstore.ProvideService, wire.Bind(new(pluginstore.Store), new(*pluginstore.Service)), - wire.Bind(new(plugins.RendererManager), new(*pluginstore.Service)), wire.Bind(new(plugins.SecretsPluginManager), new(*pluginstore.Service)), wire.Bind(new(plugins.StaticRouteResolver), new(*pluginstore.Service)), process.ProvideService, @@ -111,6 +112,8 @@ var WireSet = wire.NewSet( dynamic.ProvideService, serviceregistration.ProvideService, wire.Bind(new(auth.ExternalServiceRegistry), new(*serviceregistration.Service)), + renderer.ProvideService, + wire.Bind(new(rendering.PluginManager), new(*renderer.Manager)), ) // WireExtensionSet provides a wire.ProviderSet of plugin providers that can be diff --git a/pkg/services/pluginsintegration/pluginstore/store.go b/pkg/services/pluginsintegration/pluginstore/store.go index 3fd4cadec59..164876d187b 100644 --- a/pkg/services/pluginsintegration/pluginstore/store.go +++ b/pkg/services/pluginsintegration/pluginstore/store.go @@ -93,15 +93,6 @@ func (s *Service) Plugins(ctx context.Context, pluginTypes ...plugins.Type) []Pl return pluginsList } -func (s *Service) Renderer(ctx context.Context) *plugins.Plugin { - for _, p := range s.availablePlugins(ctx) { - if p.IsRenderer() { - return p - } - } - return nil -} - func (s *Service) SecretsManager(ctx context.Context) *plugins.Plugin { for _, p := range s.availablePlugins(ctx) { if p.IsSecretsManager() { diff --git a/pkg/services/pluginsintegration/pluginstore/store_test.go b/pkg/services/pluginsintegration/pluginstore/store_test.go index 46890aa6fc5..f6817b13d00 100644 --- a/pkg/services/pluginsintegration/pluginstore/store_test.go +++ b/pkg/services/pluginsintegration/pluginstore/store_test.go @@ -146,25 +146,6 @@ func TestStore_Routes(t *testing.T) { }) } -func TestStore_Renderer(t *testing.T) { - t.Run("Renderer returns a single (non-decommissioned) renderer plugin", func(t *testing.T) { - p1 := &plugins.Plugin{JSONData: plugins.JSONData{ID: "test-renderer", Type: plugins.TypeRenderer}} - p2 := &plugins.Plugin{JSONData: plugins.JSONData{ID: "test-panel", Type: plugins.TypePanel}} - p3 := &plugins.Plugin{JSONData: plugins.JSONData{ID: "test-app", Type: plugins.TypeApp}} - - ps := New(&fakes.FakePluginRegistry{ - Store: map[string]*plugins.Plugin{ - p1.ID: p1, - p2.ID: p2, - p3.ID: p3, - }, - }, &fakes.FakeLoader{}) - - r := ps.Renderer(context.Background()) - require.Equal(t, p1, r) - }) -} - func TestStore_SecretsManager(t *testing.T) { t.Run("Renderer returns a single (non-decommissioned) secrets manager plugin", func(t *testing.T) { p1 := &plugins.Plugin{JSONData: plugins.JSONData{ID: "test-renderer", Type: plugins.TypeRenderer}} diff --git a/pkg/services/pluginsintegration/renderer/renderer.go b/pkg/services/pluginsintegration/renderer/renderer.go new file mode 100644 index 00000000000..102c5da7adf --- /dev/null +++ b/pkg/services/pluginsintegration/renderer/renderer.go @@ -0,0 +1,128 @@ +package renderer + +import ( + "context" + "errors" + + "github.com/grafana/grafana/pkg/infra/log" + "github.com/grafana/grafana/pkg/plugins" + "github.com/grafana/grafana/pkg/plugins/backendplugin/pluginextensionv2" + "github.com/grafana/grafana/pkg/plugins/backendplugin/provider" + "github.com/grafana/grafana/pkg/plugins/config" + "github.com/grafana/grafana/pkg/plugins/envvars" + "github.com/grafana/grafana/pkg/plugins/manager/loader" + "github.com/grafana/grafana/pkg/plugins/manager/pipeline/bootstrap" + "github.com/grafana/grafana/pkg/plugins/manager/pipeline/discovery" + "github.com/grafana/grafana/pkg/plugins/manager/pipeline/initialization" + "github.com/grafana/grafana/pkg/plugins/manager/pipeline/termination" + "github.com/grafana/grafana/pkg/plugins/manager/pipeline/validation" + "github.com/grafana/grafana/pkg/plugins/manager/registry" + "github.com/grafana/grafana/pkg/plugins/manager/signature" + "github.com/grafana/grafana/pkg/plugins/manager/sources" + "github.com/grafana/grafana/pkg/services/featuremgmt" + "github.com/grafana/grafana/pkg/services/rendering" +) + +func ProvideService(cfg *config.Cfg, registry registry.Service, licensing plugins.Licensing, + features featuremgmt.FeatureToggles) (*Manager, error) { + l, err := createLoader(cfg, registry, licensing, features) + if err != nil { + return nil, err + } + + return &Manager{ + cfg: cfg, + loader: l, + log: log.New("plugins.renderer"), + }, nil +} + +type Manager struct { + cfg *config.Cfg + loader loader.Service + log log.Logger + + renderer *Plugin +} + +type Plugin struct { + plugin *plugins.Plugin + + started bool +} + +func (p *Plugin) Client() (pluginextensionv2.RendererPlugin, error) { + if !p.started { + return nil, errors.New("renderer plugin not started") + } + + if p.plugin.Renderer == nil { + return nil, errors.New("renderer client not available") + } + + return p.plugin.Renderer, nil +} + +func (p *Plugin) Start(ctx context.Context) error { + p.started = true + return p.plugin.Start(ctx) +} + +func (p *Plugin) Version() string { + return p.plugin.JSONData.Info.Version +} + +func (m *Manager) Renderer(ctx context.Context) (rendering.Plugin, bool) { + if m.renderer != nil { + return m.renderer, true + } + + ps, err := m.loader.Load(ctx, sources.NewLocalSource(plugins.ClassExternal, []string{m.cfg.PluginsPath})) + if err != nil { + m.log.Error("Failed to load renderer plugin", "error", err) + return nil, false + } + + if len(ps) >= 1 { + m.renderer = &Plugin{plugin: ps[0]} + return m.renderer, true + } + + return nil, false +} + +func createLoader(cfg *config.Cfg, pr registry.Service, l plugins.Licensing, + features featuremgmt.FeatureToggles) (loader.Service, error) { + d := discovery.New(cfg, discovery.Opts{ + FindFilterFuncs: []discovery.FindFilterFunc{ + discovery.NewPermittedPluginTypesFilterStep([]plugins.Type{plugins.TypeRenderer}), + func(ctx context.Context, class plugins.Class, bundles []*plugins.FoundBundle) ([]*plugins.FoundBundle, error) { + return discovery.NewDuplicatePluginFilterStep(pr).Filter(ctx, bundles) + }, + }, + }) + b := bootstrap.New(cfg, bootstrap.Opts{ + DecorateFuncs: []bootstrap.DecorateFunc{}, // no decoration required + }) + v := validation.New(cfg, validation.Opts{ + ValidateFuncs: []validation.ValidateFunc{ + validation.SignatureValidationStep(signature.NewValidator(signature.NewUnsignedAuthorizer(cfg))), + }, + }) + i := initialization.New(cfg, initialization.Opts{ + InitializeFuncs: []initialization.InitializeFunc{ + initialization.BackendClientInitStep(envvars.NewProvider(cfg, l), provider.New(provider.RendererProvider)), + initialization.PluginRegistrationStep(pr), + }, + }) + t, err := termination.New(cfg, termination.Opts{ + TerminateFuncs: []termination.TerminateFunc{ + termination.DeregisterStep(pr), + }, + }) + if err != nil { + return nil, err + } + + return loader.New(d, b, v, i, t), nil +} diff --git a/pkg/services/rendering/capabilities_test.go b/pkg/services/rendering/capabilities_test.go index f0faab7c6a5..e9b4989dbbf 100644 --- a/pkg/services/rendering/capabilities_test.go +++ b/pkg/services/rendering/capabilities_test.go @@ -7,14 +7,13 @@ import ( "github.com/stretchr/testify/require" "github.com/grafana/grafana/pkg/infra/log" - "github.com/grafana/grafana/pkg/plugins" "github.com/grafana/grafana/pkg/setting" ) type dummyPluginManager struct{} -func (d *dummyPluginManager) Renderer(_ context.Context) *plugins.Plugin { - return nil +func (d *dummyPluginManager) Renderer(_ context.Context) (Plugin, bool) { + return nil, false } var dummyRendererUrl = "http://dummyurl.com" diff --git a/pkg/services/rendering/plugin_mode.go b/pkg/services/rendering/plugin_mode.go index 443dcd7df6d..5e4ff4897ff 100644 --- a/pkg/services/rendering/plugin_mode.go +++ b/pkg/services/rendering/plugin_mode.go @@ -8,10 +8,6 @@ import ( "github.com/grafana/grafana/pkg/plugins/backendplugin/pluginextensionv2" ) -func (rs *RenderingService) startPlugin(ctx context.Context) error { - return rs.pluginInfo.Start(ctx) -} - func (rs *RenderingService) renderViaPlugin(ctx context.Context, renderKey string, opts Opts) (*RenderResult, error) { // gives plugin some additional time to timeout and return possible errors. ctx, cancel := context.WithTimeout(ctx, getRequestTimeout(opts.TimeoutOpts)) @@ -45,7 +41,11 @@ func (rs *RenderingService) renderViaPlugin(ctx context.Context, renderKey strin } rs.log.Debug("Calling renderer plugin", "req", req) - rsp, err := rs.pluginInfo.Renderer.Render(ctx, req) + rc, err := rs.plugin.Client() + if err != nil { + return nil, err + } + rsp, err := rc.Render(ctx, req) if errors.Is(ctx.Err(), context.DeadlineExceeded) { rs.log.Info("Rendering timed out") return nil, ErrTimeout @@ -89,7 +89,12 @@ func (rs *RenderingService) renderCSVViaPlugin(ctx context.Context, renderKey st } rs.log.Debug("Calling renderer plugin", "req", req) - rsp, err := rs.pluginInfo.Renderer.RenderCSV(ctx, req) + rc, err := rs.plugin.Client() + if err != nil { + return nil, err + } + + rsp, err := rc.RenderCSV(ctx, req) if err != nil { if errors.Is(ctx.Err(), context.DeadlineExceeded) { rs.log.Info("Rendering timed out") diff --git a/pkg/services/rendering/rendering.go b/pkg/services/rendering/rendering.go index 45b38e6e83d..7f9e95815ed 100644 --- a/pkg/services/rendering/rendering.go +++ b/pkg/services/rendering/rendering.go @@ -18,7 +18,7 @@ import ( "github.com/grafana/grafana/pkg/infra/metrics" "github.com/grafana/grafana/pkg/infra/remotecache" "github.com/grafana/grafana/pkg/models" - "github.com/grafana/grafana/pkg/plugins" + "github.com/grafana/grafana/pkg/plugins/backendplugin/pluginextensionv2" "github.com/grafana/grafana/pkg/services/featuremgmt" "github.com/grafana/grafana/pkg/setting" "github.com/grafana/grafana/pkg/util" @@ -28,7 +28,7 @@ var _ Service = (*RenderingService)(nil) type RenderingService struct { log log.Logger - pluginInfo *plugins.Plugin + plugin Plugin renderAction renderFunc renderCSVAction renderCSVFunc sanitizeSVGAction sanitizeFunc @@ -43,10 +43,20 @@ type RenderingService struct { Cfg *setting.Cfg features *featuremgmt.FeatureManager RemoteCacheService *remotecache.RemoteCache - RendererPluginManager plugins.RendererManager + RendererPluginManager PluginManager } -func ProvideService(cfg *setting.Cfg, features *featuremgmt.FeatureManager, remoteCache *remotecache.RemoteCache, rm plugins.RendererManager) (*RenderingService, error) { +type PluginManager interface { + Renderer(ctx context.Context) (Plugin, bool) +} + +type Plugin interface { + Client() (pluginextensionv2.RendererPlugin, error) + Start(ctx context.Context) error + Version() string +} + +func ProvideService(cfg *setting.Cfg, features *featuremgmt.FeatureManager, remoteCache *remotecache.RemoteCache, rm PluginManager) (*RenderingService, error) { // ensure ImagesDir exists err := os.MkdirAll(cfg.ImagesDir, 0700) if err != nil { @@ -167,15 +177,13 @@ func (rs *RenderingService) Run(ctx context.Context) error { } } - if rs.pluginAvailable(ctx) { + if rp, exists := rs.RendererPluginManager.Renderer(ctx); exists { rs.log = rs.log.New("renderer", "plugin") - rs.pluginInfo = rs.RendererPluginManager.Renderer(ctx) - - if err := rs.startPlugin(ctx); err != nil { + rs.plugin = rp + if err := rs.plugin.Start(ctx); err != nil { return err } - - rs.version = rs.pluginInfo.Info.Version + rs.version = rp.Version() rs.renderAction = rs.renderViaPlugin rs.renderCSVAction = rs.renderCSVViaPlugin rs.sanitizeSVGAction = rs.sanitizeSVGViaPlugin @@ -193,7 +201,8 @@ func (rs *RenderingService) Run(ctx context.Context) error { } func (rs *RenderingService) pluginAvailable(ctx context.Context) bool { - return rs.RendererPluginManager.Renderer(ctx) != nil + _, exists := rs.RendererPluginManager.Renderer(ctx) + return exists } func (rs *RenderingService) remoteAvailable() bool { diff --git a/pkg/services/rendering/rendering_test.go b/pkg/services/rendering/rendering_test.go index 0cb8edd2a8e..d8b808aa170 100644 --- a/pkg/services/rendering/rendering_test.go +++ b/pkg/services/rendering/rendering_test.go @@ -14,7 +14,6 @@ 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/setting" ) @@ -104,15 +103,11 @@ func TestRenderErrorImage(t *testing.T) { }) } -type unavailableRendererManager struct{} - -func (m unavailableRendererManager) Renderer(_ context.Context) *plugins.Plugin { return nil } - func TestRenderUnavailableError(t *testing.T) { rs := RenderingService{ Cfg: &setting.Cfg{}, log: log.New("test"), - RendererPluginManager: unavailableRendererManager{}, + RendererPluginManager: &dummyPluginManager{}, } opts := Opts{ErrorOpts: ErrorOpts{ErrorRenderUnavailable: true}} result, err := rs.Render(context.Background(), opts, nil) diff --git a/pkg/services/rendering/svgSanitizer.go b/pkg/services/rendering/svgSanitizer.go index f4245f60347..9a87992e104 100644 --- a/pkg/services/rendering/svgSanitizer.go +++ b/pkg/services/rendering/svgSanitizer.go @@ -160,7 +160,11 @@ func (rs *RenderingService) sanitizeSVGViaPlugin(ctx context.Context, req *Sanit } rs.log.Debug("Sanitizer - plugin: calling", "filename", req.Filename, "contentLength", len(req.Content)) - rsp, err := rs.pluginInfo.Renderer.Sanitize(ctx, grpcReq) + rc, err := rs.plugin.Client() + if err != nil { + return nil, err + } + rsp, err := rc.Sanitize(ctx, grpcReq) if err != nil { if errors.Is(ctx.Err(), context.DeadlineExceeded) { rs.log.Info("Sanitizer - plugin: time out")