Revert "Plugins: Forbid loading Angular plugins when Angular is disabled (#69225)" (#69657)

This reverts commit ff34279ff4.

pkg/api/plugin_resource_test.go

@@ -22,7 +22,6 @@ import (
 	pluginClient "github.com/grafana/grafana/pkg/plugins/manager/client"
 	"github.com/grafana/grafana/pkg/plugins/manager/fakes"
 	"github.com/grafana/grafana/pkg/plugins/manager/loader"
-	"github.com/grafana/grafana/pkg/plugins/manager/loader/angulardetector"
 	"github.com/grafana/grafana/pkg/plugins/manager/loader/assetpath"
 	"github.com/grafana/grafana/pkg/plugins/manager/loader/finder"
 	"github.com/grafana/grafana/pkg/plugins/manager/registry"
@@ -68,8 +67,7 @@ func TestCallResource(t *testing.T) {
 	reg := registry.ProvideService()
 	l := loader.ProvideService(pCfg, fakes.NewFakeLicensingService(), signature.NewUnsignedAuthorizer(pCfg),
 		reg, provider.ProvideService(coreRegistry), finder.NewLocalFinder(pCfg), fakes.NewFakeRoleRegistry(),
-		assetpath.ProvideService(pluginscdn.ProvideService(pCfg)), signature.ProvideService(pCfg, statickey.New()),
+		assetpath.ProvideService(pluginscdn.ProvideService(pCfg)), signature.ProvideService(pCfg, statickey.New()))
-		angulardetector.NewDefaultPatternsListInspector())
 	srcs := sources.ProvideService(cfg, pCfg)
 	ps, err := store.ProvideService(reg, srcs, l)
 	require.NoError(t, err)

pkg/plugins/config/config.go

@@ -39,13 +39,11 @@ type Cfg struct {
 	GrafanaComURL string
 
 	Features plugins.FeatureToggles
-
-	AngularSupportEnabled bool
 }
 
 func NewCfg(devMode bool, pluginsPath string, pluginSettings setting.PluginSettings, pluginsAllowUnsigned []string,
 	awsAllowedAuthProviders []string, awsAssumeRoleEnabled bool, azure *azsettings.AzureSettings, secureSocksDSProxy setting.SecureSocksDSProxySettings,
-	grafanaVersion string, logDatasourceRequests bool, pluginsCDNURLTemplate string, tracing Tracing, features plugins.FeatureToggles, angularSupportEnabled bool) *Cfg {
+	grafanaVersion string, logDatasourceRequests bool, pluginsCDNURLTemplate string, tracing Tracing, features plugins.FeatureToggles) *Cfg {
 	return &Cfg{
 		log:                     log.New("plugin.cfg"),
 		PluginsPath:             pluginsPath,
@@ -62,6 +60,5 @@ func NewCfg(devMode bool, pluginsPath string, pluginSettings setting.PluginSetti
 		Tracing:                 tracing,
 		GrafanaComURL:           "https://grafana.com",
 		Features:                features,
-		AngularSupportEnabled:   angularSupportEnabled,
 	}
 }

pkg/plugins/manager/loader/angulardetector/angulardetector.go

@@ -2,6 +2,8 @@ package angulardetector
 
 import (
 	"bytes"
+	"fmt"
+	"io"
 	"regexp"
 
 	"github.com/grafana/grafana/pkg/plugins"
@@ -38,9 +40,43 @@ func (d *regexDetector) Detect(moduleJs []byte) bool {
 	return d.regex.Match(moduleJs)
 }
 
-// Inspector can inspect a module.js and determine if it's an Angular plugin or not.
+// angularDetectors contains all the detectors to detect Angular plugins.
-type Inspector interface {
+// They are executed in the specified order.
-	// Inspect open module.js and checks if the plugin is using Angular by matching against its source code.
+var angularDetectors = []detector{
-	// It returns true if module.js matches against any of the detectors in angularDetectors.
+	&containsBytesDetector{pattern: []byte("PanelCtrl")},
-	Inspect(p *plugins.Plugin) (bool, error)
+	&containsBytesDetector{pattern: []byte("QueryCtrl")},
+	&containsBytesDetector{pattern: []byte("app/plugins/sdk")},
+	&containsBytesDetector{pattern: []byte("angular.isNumber(")},
+	&containsBytesDetector{pattern: []byte("editor.html")},
+	&containsBytesDetector{pattern: []byte("ctrl.annotation")},
+	&containsBytesDetector{pattern: []byte("getLegacyAngularInjector")},
+
+	&regexDetector{regex: regexp.MustCompile(`['"](app/core/utils/promiseToDigest)|(app/plugins/.*?)|(app/core/core_module)['"]`)},
+	&regexDetector{regex: regexp.MustCompile(`from\s+['"]grafana\/app\/`)},
+	&regexDetector{regex: regexp.MustCompile(`System\.register\(`)},
+}
+
+// Inspect open module.js and checks if the plugin is using Angular by matching against its source code.
+// It returns true if module.js matches against any of the detectors in angularDetectors.
+func Inspect(p *plugins.Plugin) (isAngular bool, err error) {
+	f, err := p.FS.Open("module.js")
+	if err != nil {
+		return false, fmt.Errorf("open module.js: %w", err)
+	}
+	defer func() {
+		if closeErr := f.Close(); closeErr != nil && err == nil {
+			err = fmt.Errorf("close module.js: %w", closeErr)
+		}
+	}()
+	b, err := io.ReadAll(f)
+	if err != nil {
+		return false, fmt.Errorf("module.js readall: %w", err)
+	}
+	for _, d := range angularDetectors {
+		if d.Detect(b) {
+			isAngular = true
+			break
+		}
+	}
+	return
 }

pkg/plugins/manager/loader/angulardetector/angulardetector_test.go

@@ -44,10 +44,9 @@ func TestAngularDetector_Inspect(t *testing.T) {
 		},
 		exp: false,
 	})
-	inspector := NewDefaultPatternsListInspector()
 	for _, tc := range tcs {
 		t.Run(tc.name, func(t *testing.T) {
-			isAngular, err := inspector.Inspect(tc.plugin)
+			isAngular, err := Inspect(tc.plugin)
 			require.NoError(t, err)
 			require.Equal(t, tc.exp, isAngular)
 		})
@@ -55,33 +54,7 @@ func TestAngularDetector_Inspect(t *testing.T) {
 
 	t.Run("no module.js", func(t *testing.T) {
 		p := &plugins.Plugin{FS: plugins.NewInMemoryFS(map[string][]byte{})}
-		_, err := inspector.Inspect(p)
+		_, err := Inspect(p)
 		require.ErrorIs(t, err, plugins.ErrFileNotExist)
 	})
 }
-
-func TestFakeInspector(t *testing.T) {
-	t.Run("FakeInspector", func(t *testing.T) {
-		var called bool
-		inspector := FakeInspector{InspectFunc: func(p *plugins.Plugin) (bool, error) {
-			called = true
-			return false, nil
-		}}
-		r, err := inspector.Inspect(&plugins.Plugin{})
-		require.True(t, called)
-		require.NoError(t, err)
-		require.False(t, r)
-	})
-
-	t.Run("AlwaysAngularFakeInspector", func(t *testing.T) {
-		r, err := AlwaysAngularFakeInspector.Inspect(&plugins.Plugin{})
-		require.NoError(t, err)
-		require.True(t, r)
-	})
-
-	t.Run("NeverAngularFakeInspector", func(t *testing.T) {
-		r, err := NeverAngularFakeInspector.Inspect(&plugins.Plugin{})
-		require.NoError(t, err)
-		require.False(t, r)
-	})
-}

pkg/plugins/manager/loader/angulardetector/fakes.go

@@ -1,29 +0,0 @@
-package angulardetector
-
-import "github.com/grafana/grafana/pkg/plugins"
-
-// FakeInspector is an inspector whose Inspect function can be set to any function.
-type FakeInspector struct {
-	// InspectFunc is the function called when calling Inspect()
-	InspectFunc func(p *plugins.Plugin) (bool, error)
-}
-
-func (i *FakeInspector) Inspect(p *plugins.Plugin) (bool, error) {
-	return i.InspectFunc(p)
-}
-
-var (
-	// AlwaysAngularFakeInspector is an inspector that always returns `true, nil`
-	AlwaysAngularFakeInspector = &FakeInspector{
-		InspectFunc: func(p *plugins.Plugin) (bool, error) {
-			return true, nil
-		},
-	}
-
-	// NeverAngularFakeInspector is an inspector that always returns `false, nil`
-	NeverAngularFakeInspector = &FakeInspector{
-		InspectFunc: func(p *plugins.Plugin) (bool, error) {
-			return false, nil
-		},
-	}
-)

pkg/plugins/manager/loader/angulardetector/service.go

@@ -1,62 +0,0 @@
-package angulardetector
-
-import (
-	"fmt"
-	"io"
-	"regexp"
-
-	"github.com/grafana/grafana/pkg/plugins"
-)
-
-// defaultDetectors contains all the detectors to detect Angular plugins.
-// They are executed in the specified order.
-var defaultDetectors = []detector{
-	&containsBytesDetector{pattern: []byte("PanelCtrl")},
-	&containsBytesDetector{pattern: []byte("QueryCtrl")},
-	&containsBytesDetector{pattern: []byte("app/plugins/sdk")},
-	&containsBytesDetector{pattern: []byte("angular.isNumber(")},
-	&containsBytesDetector{pattern: []byte("editor.html")},
-	&containsBytesDetector{pattern: []byte("ctrl.annotation")},
-	&containsBytesDetector{pattern: []byte("getLegacyAngularInjector")},
-
-	&regexDetector{regex: regexp.MustCompile(`['"](app/core/utils/promiseToDigest)|(app/plugins/.*?)|(app/core/core_module)['"]`)},
-	&regexDetector{regex: regexp.MustCompile(`from\s+['"]grafana\/app\/`)},
-	&regexDetector{regex: regexp.MustCompile(`System\.register\(`)},
-}
-
-// PatternsListInspector matches module.js against all the specified patterns, in sequence.
-type PatternsListInspector struct {
-	detectors []detector
-}
-
-// NewDefaultPatternsListInspector returns a new *PatternsListInspector using defaultDetectors as detectors.
-func NewDefaultPatternsListInspector() *PatternsListInspector {
-	return &PatternsListInspector{detectors: defaultDetectors}
-}
-
-func ProvideService() Inspector {
-	return NewDefaultPatternsListInspector()
-}
-
-func (i *PatternsListInspector) Inspect(p *plugins.Plugin) (isAngular bool, err error) {
-	f, err := p.FS.Open("module.js")
-	if err != nil {
-		return false, fmt.Errorf("open module.js: %w", err)
-	}
-	defer func() {
-		if closeErr := f.Close(); closeErr != nil && err == nil {
-			err = fmt.Errorf("close module.js: %w", closeErr)
-		}
-	}()
-	b, err := io.ReadAll(f)
-	if err != nil {
-		return false, fmt.Errorf("module.js readall: %w", err)
-	}
-	for _, d := range i.detectors {
-		if d.Detect(b) {
-			isAngular = true
-			break
-		}
-	}
-	return
-}

pkg/plugins/manager/loader/loader.go

@@ -36,24 +36,20 @@ type Loader struct {
 	log                 log.Logger
 	cfg                 *config.Cfg
 
-	angularInspector angulardetector.Inspector
-
 	errs map[string]*plugins.SignatureError
 }
 
 func ProvideService(cfg *config.Cfg, license plugins.Licensing, authorizer plugins.PluginLoaderAuthorizer,
 	pluginRegistry registry.Service, backendProvider plugins.BackendFactoryProvider, pluginFinder finder.Finder,
-	roleRegistry plugins.RoleRegistry, assetPath *assetpath.Service, signatureCalculator plugins.SignatureCalculator,
+	roleRegistry plugins.RoleRegistry, assetPath *assetpath.Service, signatureCalculator plugins.SignatureCalculator) *Loader {
-	angularInspector angulardetector.Inspector) *Loader {
 	return New(cfg, license, authorizer, pluginRegistry, backendProvider, process.NewManager(pluginRegistry),
-		roleRegistry, assetPath, pluginFinder, signatureCalculator, angularInspector)
+		roleRegistry, assetPath, pluginFinder, signatureCalculator)
 }
 
 func New(cfg *config.Cfg, license plugins.Licensing, authorizer plugins.PluginLoaderAuthorizer,
 	pluginRegistry registry.Service, backendProvider plugins.BackendFactoryProvider,
 	processManager process.Service, roleRegistry plugins.RoleRegistry,
-	assetPath *assetpath.Service, pluginFinder finder.Finder, signatureCalculator plugins.SignatureCalculator,
+	assetPath *assetpath.Service, pluginFinder finder.Finder, signatureCalculator plugins.SignatureCalculator) *Loader {
-	angularInspector angulardetector.Inspector) *Loader {
 	return &Loader{
 		pluginFinder:        pluginFinder,
 		pluginRegistry:      pluginRegistry,
@@ -66,7 +62,6 @@ func New(cfg *config.Cfg, license plugins.Licensing, authorizer plugins.PluginLo
 		roleRegistry:        roleRegistry,
 		cfg:                 cfg,
 		assetPath:           assetPath,
-		angularInspector:    angularInspector,
 	}
 }
 
@@ -167,6 +162,15 @@ func (l *Loader) loadPlugins(ctx context.Context, src plugins.PluginSource, foun
 			}
 		}
 
+		// Detect angular for external plugins
+		if plugin.IsExternalPlugin() {
+			var err error
+			plugin.AngularDetected, err = angulardetector.Inspect(plugin)
+			if err != nil {
+				l.log.Warn("could not inspect plugin for angular", "pluginID", plugin.ID, "err", err)
+			}
+		}
+
 		if plugin.IsApp() {
 			setDefaultNavURL(plugin)
 		}
@@ -181,21 +185,6 @@ func (l *Loader) loadPlugins(ctx context.Context, src plugins.PluginSource, foun
 	// initialize plugins
 	initializedPlugins := make([]*plugins.Plugin, 0)
 	for _, p := range verifiedPlugins {
-		// Detect angular for external plugins
-		if p.IsExternalPlugin() {
-			var err error
-			p.AngularDetected, err = l.angularInspector.Inspect(p)
-			if err != nil {
-				l.log.Warn("could not inspect plugin for angular", "pluginID", p.ID, "err", err)
-			}
-
-			// Do not initialize plugins if they're using Angular and Angular support is disabled
-			if p.AngularDetected && !l.cfg.AngularSupportEnabled {
-				l.log.Error("Refusing to initialize plugin because it's using Angular, which has been disabled", "pluginID", p.ID)
-				continue
-			}
-		}
-
 		err := l.pluginInitializer.Initialize(ctx, p)
 		if err != nil {
 			l.log.Error("Could not initialize plugin", "pluginId", p.ID, "err", err)

pkg/plugins/manager/loader/loader_test.go

@@ -16,7 +16,6 @@ import (
 	"github.com/grafana/grafana/pkg/plugins/config"
 	"github.com/grafana/grafana/pkg/plugins/log"
 	"github.com/grafana/grafana/pkg/plugins/manager/fakes"
-	"github.com/grafana/grafana/pkg/plugins/manager/loader/angulardetector"
 	"github.com/grafana/grafana/pkg/plugins/manager/loader/assetpath"
 	"github.com/grafana/grafana/pkg/plugins/manager/loader/finder"
 	"github.com/grafana/grafana/pkg/plugins/manager/loader/initializer"
@@ -1077,58 +1076,6 @@ func TestLoader_Load_SkipUninitializedPlugins(t *testing.T) {
 	})
 }
 
-func TestLoader_Load_Angular(t *testing.T) {
-	fakePluginSource := &fakes.FakePluginSource{
-		PluginClassFunc: func(ctx context.Context) plugins.Class {
-			return plugins.External
-		},
-		PluginURIsFunc: func(ctx context.Context) []string {
-			return []string{"../testdata/valid-v2-signature"}
-		},
-	}
-	for _, cfgTc := range []struct {
-		name string
-		cfg  *config.Cfg
-	}{
-		{name: "angular support enabled", cfg: &config.Cfg{AngularSupportEnabled: true}},
-		{name: "angular support disabled", cfg: &config.Cfg{AngularSupportEnabled: false}},
-	} {
-		t.Run(cfgTc.name, func(t *testing.T) {
-			for _, tc := range []struct {
-				name             string
-				angularInspector angulardetector.Inspector
-				shouldLoad       bool
-			}{
-				{
-					name:             "angular plugin",
-					angularInspector: angulardetector.AlwaysAngularFakeInspector,
-					// angular plugins should load only if allowed by the cfg
-					shouldLoad: cfgTc.cfg.AngularSupportEnabled,
-				},
-				{
-					name:             "non angular plugin",
-					angularInspector: angulardetector.NeverAngularFakeInspector,
-					// non-angular plugins should always load
-					shouldLoad: true,
-				},
-			} {
-				t.Run(tc.name, func(t *testing.T) {
-					l := newLoader(cfgTc.cfg, func(l *Loader) {
-						l.angularInspector = tc.angularInspector
-					})
-					p, err := l.Load(context.Background(), fakePluginSource)
-					require.NoError(t, err)
-					if tc.shouldLoad {
-						require.Len(t, p, 1, "plugin should have been loaded")
-					} else {
-						require.Empty(t, p, "plugin shouldn't have been loaded")
-					}
-				})
-			}
-		})
-	}
-}
-
 func TestLoader_Load_NestedPlugins(t *testing.T) {
 	rootDir, err := filepath.Abs("../")
 	if err != nil {
@@ -1440,7 +1387,7 @@ func newLoader(cfg *config.Cfg, cbs ...func(loader *Loader)) *Loader {
 	l := New(cfg, &fakes.FakeLicensingService{}, signature.NewUnsignedAuthorizer(cfg), fakes.NewFakePluginRegistry(),
 		fakes.NewFakeBackendProcessProvider(), fakes.NewFakeProcessManager(), fakes.NewFakeRoleRegistry(),
 		assetpath.ProvideService(pluginscdn.ProvideService(cfg)), finder.NewLocalFinder(cfg),
-		signature.ProvideService(cfg, statickey.New()), angulardetector.NewDefaultPatternsListInspector())
+		signature.ProvideService(cfg, statickey.New()))
 
 	for _, cb := range cbs {
 		cb(l)

pkg/plugins/manager/manager_integration_test.go

@@ -10,6 +10,7 @@ import (
 	"github.com/grafana/grafana-azure-sdk-go/azsettings"
 	"github.com/grafana/grafana-plugin-sdk-go/backend"
 	"github.com/grafana/grafana-plugin-sdk-go/backend/httpclient"
+	"github.com/grafana/grafana/pkg/services/accesscontrol/acimpl"
 	"github.com/stretchr/testify/require"
 	"gopkg.in/ini.v1"
 
@@ -21,7 +22,6 @@ import (
 	"github.com/grafana/grafana/pkg/plugins/manager/client"
 	"github.com/grafana/grafana/pkg/plugins/manager/fakes"
 	"github.com/grafana/grafana/pkg/plugins/manager/loader"
-	"github.com/grafana/grafana/pkg/plugins/manager/loader/angulardetector"
 	"github.com/grafana/grafana/pkg/plugins/manager/loader/assetpath"
 	"github.com/grafana/grafana/pkg/plugins/manager/loader/finder"
 	"github.com/grafana/grafana/pkg/plugins/manager/registry"
@@ -30,7 +30,6 @@ import (
 	"github.com/grafana/grafana/pkg/plugins/manager/sources"
 	"github.com/grafana/grafana/pkg/plugins/manager/store"
 	"github.com/grafana/grafana/pkg/plugins/pluginscdn"
-	"github.com/grafana/grafana/pkg/services/accesscontrol/acimpl"
 	"github.com/grafana/grafana/pkg/services/featuremgmt"
 	"github.com/grafana/grafana/pkg/services/licensing"
 	"github.com/grafana/grafana/pkg/services/pluginsintegration/config"
@@ -120,8 +119,7 @@ func TestIntegrationPluginManager(t *testing.T) {
 	lic := plicensing.ProvideLicensing(cfg, &licensing.OSSLicensingService{Cfg: cfg})
 	l := loader.ProvideService(pCfg, lic, signature.NewUnsignedAuthorizer(pCfg),
 		reg, provider.ProvideService(coreRegistry), finder.NewLocalFinder(pCfg), fakes.NewFakeRoleRegistry(),
-		assetpath.ProvideService(pluginscdn.ProvideService(pCfg)), signature.ProvideService(pCfg, statickey.New()),
+		assetpath.ProvideService(pluginscdn.ProvideService(pCfg)), signature.ProvideService(pCfg, statickey.New()))
-		angulardetector.NewDefaultPatternsListInspector())
 	srcs := sources.ProvideService(cfg, pCfg)
 	ps, err := store.ProvideService(reg, srcs, l)
 	require.NoError(t, err)

pkg/services/pluginsintegration/config/config.go

@@ -41,7 +41,6 @@ func ProvideConfig(settingProvider setting.Provider, grafanaCfg *setting.Cfg, fe
 		grafanaCfg.PluginsCDNURLTemplate,
 		tracingCfg,
 		featuremgmt.ProvideToggles(features),
-		grafanaCfg.AngularSupportEnabled,
 	), nil
 }
 

pkg/services/pluginsintegration/pluginsintegration.go

@@ -12,7 +12,6 @@ import (
 	"github.com/grafana/grafana/pkg/plugins/manager/client"
 	"github.com/grafana/grafana/pkg/plugins/manager/filestore"
 	"github.com/grafana/grafana/pkg/plugins/manager/loader"
-	"github.com/grafana/grafana/pkg/plugins/manager/loader/angulardetector"
 	"github.com/grafana/grafana/pkg/plugins/manager/loader/assetpath"
 	"github.com/grafana/grafana/pkg/plugins/manager/loader/finder"
 	"github.com/grafana/grafana/pkg/plugins/manager/process"
@@ -52,7 +51,6 @@ var WireSet = wire.NewSet(
 	coreplugin.ProvideCoreRegistry,
 	pluginscdn.ProvideService,
 	assetpath.ProvideService,
-	angulardetector.ProvideService,
 	loader.ProvideService,
 	wire.Bind(new(loader.Service), new(*loader.Loader)),
 	wire.Bind(new(plugins.ErrorResolver), new(*loader.Loader)),