Bug: Fix loading behavior with FlagExternalCorePlugins (#78388)

pkg/cmd/grafana-cli/services/services.go

@@ -15,6 +15,7 @@ import (
 	"github.com/grafana/grafana/pkg/plugins"
 	"github.com/grafana/grafana/pkg/plugins/manager/loader/finder"
 	"github.com/grafana/grafana/pkg/plugins/manager/sources"
+	"github.com/grafana/grafana/pkg/services/featuremgmt"
 )
 
 var (
@@ -77,7 +78,7 @@ func GetLocalPlugin(pluginDir, pluginID string) (plugins.FoundPlugin, error) {
 }
 
 func GetLocalPlugins(pluginDir string) []*plugins.FoundBundle {
-	f := finder.NewLocalFinder(true)
+	f := finder.NewLocalFinder(true, featuremgmt.WithFeatures())
 
 	res, err := f.Find(context.Background(), sources.NewLocalSource(plugins.ClassExternal, []string{pluginDir}))
 	if err != nil {

pkg/plugins/manager/loader/finder/local.go

@@ -14,6 +14,7 @@ import (
 	"github.com/grafana/grafana/pkg/plugins"
 	"github.com/grafana/grafana/pkg/plugins/config"
 	"github.com/grafana/grafana/pkg/plugins/log"
+	"github.com/grafana/grafana/pkg/services/featuremgmt"
 	"github.com/grafana/grafana/pkg/util"
 )
 
@@ -26,17 +27,19 @@ var (
 type Local struct {
 	log        log.Logger
 	production bool
+	features   plugins.FeatureToggles
 }
 
-func NewLocalFinder(devMode bool) *Local {
+func NewLocalFinder(devMode bool, features plugins.FeatureToggles) *Local {
 	return &Local{
 		production: !devMode,
 		log:        log.New("local.finder"),
+		features:   features,
 	}
 }
 
 func ProvideLocalFinder(cfg *config.Cfg) *Local {
-	return NewLocalFinder(cfg.DevMode)
+	return NewLocalFinder(cfg.DevMode, cfg.Features)
 }
 
 func (l *Local) Find(ctx context.Context, src plugins.PluginSource) ([]*plugins.FoundBundle, error) {
@@ -58,7 +61,8 @@ func (l *Local) Find(ctx context.Context, src plugins.PluginSource) ([]*plugins.
 		}
 
 		followDistFolder := true
-		if src.PluginClass(ctx) == plugins.ClassCore {
+		if src.PluginClass(ctx) == plugins.ClassCore &&
+			!l.features.IsEnabledGlobally(featuremgmt.FlagExternalCorePlugins) {
 			followDistFolder = false
 		}
 		paths, err := l.getAbsPluginJSONPaths(path, followDistFolder)

pkg/plugins/manager/loader/finder/local_test.go

@@ -13,6 +13,7 @@ import (
 
 	"github.com/grafana/grafana/pkg/plugins"
 	"github.com/grafana/grafana/pkg/plugins/manager/fakes"
+	"github.com/grafana/grafana/pkg/services/featuremgmt"
 	"github.com/grafana/grafana/pkg/util"
 )
 
@@ -25,6 +26,7 @@ func TestFinder_Find(t *testing.T) {
 	testCases := []struct {
 		name            string
 		pluginDirs      []string
+		pluginClass     plugins.Class
 		expectedBundles []*plugins.FoundBundle
 		err             error
 	}{
@@ -245,11 +247,46 @@ func TestFinder_Find(t *testing.T) {
 				},
 			},
 		},
+		{
+			name:        "Plugin with dist folder (core class)",
+			pluginDirs:  []string{filepath.Join(testData, "plugin-with-dist")},
+			pluginClass: plugins.ClassCore,
+			expectedBundles: []*plugins.FoundBundle{
+				{
+					Primary: plugins.FoundPlugin{
+						JSONData: plugins.JSONData{
+							ID:   "test-datasource",
+							Type: plugins.TypeDataSource,
+							Name: "Test",
+							Info: plugins.Info{
+								Author: plugins.InfoLink{
+									Name: "Will Browne",
+									URL:  "https://willbrowne.com",
+								},
+								Description: "Test",
+								Version:     "1.0.0",
+							},
+							Dependencies: plugins.Dependencies{
+								GrafanaVersion: "*",
+								Plugins:        []plugins.Dependency{},
+							},
+							State:      plugins.ReleaseStateAlpha,
+							Backend:    true,
+							Executable: "test",
+						},
+						FS: mustNewStaticFSForTests(t, filepath.Join(testData, "plugin-with-dist/plugin/dist")),
+					},
+				},
+			},
+		},
 	}
 	for _, tc := range testCases {
 		t.Run(tc.name, func(t *testing.T) {
-			f := NewLocalFinder(false)
+			f := NewLocalFinder(false, featuremgmt.WithFeatures(featuremgmt.FlagExternalCorePlugins))
 			pluginBundles, err := f.Find(context.Background(), &fakes.FakePluginSource{
+				PluginClassFunc: func(ctx context.Context) plugins.Class {
+					return tc.pluginClass
+				},
 				PluginURIsFunc: func(ctx context.Context) []string {
 					return tc.pluginDirs
 				},
@@ -281,7 +318,7 @@ func TestFinder_getAbsPluginJSONPaths(t *testing.T) {
 			walk = origWalk
 		})
 
-		finder := NewLocalFinder(false)
+		finder := NewLocalFinder(false, featuremgmt.WithFeatures())
 		paths, err := finder.getAbsPluginJSONPaths("test", true)
 		require.NoError(t, err)
 		require.Empty(t, paths)
@@ -296,7 +333,7 @@ func TestFinder_getAbsPluginJSONPaths(t *testing.T) {
 			walk = origWalk
 		})
 
-		finder := NewLocalFinder(false)
+		finder := NewLocalFinder(false, featuremgmt.WithFeatures())
 		paths, err := finder.getAbsPluginJSONPaths("test", true)
 		require.NoError(t, err)
 		require.Empty(t, paths)
@@ -311,7 +348,7 @@ func TestFinder_getAbsPluginJSONPaths(t *testing.T) {
 			walk = origWalk
 		})
 
-		finder := NewLocalFinder(false)
+		finder := NewLocalFinder(false, featuremgmt.WithFeatures())
 		paths, err := finder.getAbsPluginJSONPaths("test", true)
 		require.Error(t, err)
 		require.Empty(t, paths)
@@ -329,7 +366,7 @@ func TestFinder_getAbsPluginJSONPaths(t *testing.T) {
 			walk = origWalk
 		})
 
-		finder := NewLocalFinder(false)
+		finder := NewLocalFinder(false, featuremgmt.WithFeatures())
 		paths, err := finder.getAbsPluginJSONPaths("test", false)
 		require.ErrorIs(t, err, filepath.SkipDir)
 		require.Empty(t, paths)
@@ -366,7 +403,7 @@ func TestFinder_getAbsPluginJSONPaths_PluginClass(t *testing.T) {
 			},
 		}
 		for _, tc := range tcs {
-			pluginBundles, err := NewLocalFinder(false).getAbsPluginJSONPaths(dir, tc.followDist)
+			pluginBundles, err := NewLocalFinder(false, featuremgmt.WithFeatures()).getAbsPluginJSONPaths(dir, tc.followDist)
 			require.NoError(t, err)
 
 			sort.Strings(pluginBundles)

pkg/plugins/manager/pipeline/discovery/steps.go

@@ -13,7 +13,7 @@ import (
 // DefaultFindFunc is the default function used for the Find step of the Discovery stage. It will scan the local
 // filesystem for plugins.
 func DefaultFindFunc(cfg *config.Cfg) FindFunc {
-	return finder.NewLocalFinder(cfg.DevMode).Find
+	return finder.NewLocalFinder(cfg.DevMode, cfg.Features).Find
 }
 
 // DuplicatePluginValidation is a filter step that will filter out any plugins that are already registered with the

pkg/plugins/manager/testdata/plugin-with-dist/plugin/dist/MANIFEST.txt

@@ -0,0 +1,28 @@
+
+-----BEGIN PGP SIGNED MESSAGE-----
+Hash: SHA512
+
+{
+  "manifestVersion": "2.0.0",
+  "signatureType": "grafana",
+  "signedByOrg": "grafana",
+  "signedByOrgName": "Grafana Labs",
+  "plugin": "test-datasource",
+  "version": "1.0.0",
+  "time": 1661171059101,
+  "keyId": "7e4d0c6a708866e7",
+  "files": {
+    "plugin.json": "203ef4a613c5693c437a665cd67f95e2756a0f71b336b2ffb265db7c180d0b19"
+  }
+}
+-----BEGIN PGP SIGNATURE-----
+Version: OpenPGP.js v4.10.10
+Comment: https://openpgpjs.org
+
+wrgEARMKAAYFAmMDdXMAIQkQfk0ManCIZucWIQTzOyW2kQdOhGNlcPN+TQxq
+cIhm54zLAgdfVimeut6Gw9MrIACBZUSH0ht9p9j+iG6MDjpmEFIpqVJrem6f
+8wBv0/kmYU3LV9MWyPuUeRfBdccjQKSjEXlfEAIJAVmut9LcSKIykhWuQA+7
+VMVvJPXzlPkeoYsGYvzAlxh8i2UomCU15UChe62Gzq5V5HgGYkX5layIb5XX
+y2Pio0lc
+=/TR0
+-----END PGP SIGNATURE-----

pkg/plugins/manager/testdata/plugin-with-dist/plugin/dist/plugin.json

@@ -0,0 +1,16 @@
+{
+  "type": "datasource",
+  "name": "Test",
+  "id": "test-datasource",
+  "backend": true,
+  "executable": "test",
+  "state": "alpha",
+  "info": {
+    "version": "1.0.0",
+    "description": "Test",
+    "author": {
+      "name": "Will Browne",
+      "url": "https://willbrowne.com"
+    }
+  }
+}

pkg/plugins/manager/testdata/plugin-with-dist/plugin/plugin.json

@@ -0,0 +1,16 @@
+{
+  "type": "datasource",
+  "name": "Test",
+  "id": "test-datasource",
+  "backend": true,
+  "executable": "test",
+  "state": "alpha",
+  "info": {
+    "version": "2.0.0",
+    "description": "Test",
+    "author": {
+      "name": "Will Browne",
+      "url": "https://willbrowne.com"
+    }
+  }
+}

pkg/plugins/pfs/pfs_test.go

@@ -107,6 +107,10 @@ func TestParsePluginTestdata(t *testing.T) {
 			rootid:  "test-datasource",
 			subpath: "plugin",
 		},
+		"plugin-with-dist": {
+			rootid:  "test-datasource",
+			subpath: "plugin",
+		},
 		"no-rootfile": {
 			err: ErrNoRootFile,
 		},

pkg/services/pluginsintegration/loader/loader_test.go

@@ -1623,7 +1623,8 @@ func newLoader(t *testing.T, cfg *config.Cfg, reg registry.Service, proc process
 	terminate, err := pipeline.ProvideTerminationStage(cfg, reg, proc)
 	require.NoError(t, err)
 
-	return ProvideService(pipeline.ProvideDiscoveryStage(cfg, finder.NewLocalFinder(false), reg),
+	return ProvideService(pipeline.ProvideDiscoveryStage(cfg,
+		finder.NewLocalFinder(false, featuremgmt.WithFeatures()), reg),
 		pipeline.ProvideBootstrapStage(cfg, signature.DefaultCalculator(cfg), assets),
 		pipeline.ProvideValidationStage(cfg, signature.NewValidator(signature.NewUnsignedAuthorizer(cfg)), angularInspector, sigErrTracker),
 		pipeline.ProvideInitializationStage(cfg, reg, lic, backendFactory, proc, &fakes.FakeAuthService{}, fakes.NewFakeRoleRegistry()),
@@ -1655,7 +1656,8 @@ func newLoaderWithOpts(t *testing.T, cfg *config.Cfg, opts loaderDepOpts) *Loade
 		backendFactoryProvider = fakes.NewFakeBackendProcessProvider()
 	}
 
-	return ProvideService(pipeline.ProvideDiscoveryStage(cfg, finder.NewLocalFinder(false), reg),
+	return ProvideService(pipeline.ProvideDiscoveryStage(cfg,
+		finder.NewLocalFinder(false, featuremgmt.WithFeatures()), reg),
 		pipeline.ProvideBootstrapStage(cfg, signature.DefaultCalculator(cfg), assets),
 		pipeline.ProvideValidationStage(cfg, signature.NewValidator(signature.NewUnsignedAuthorizer(cfg)), angularInspector, sigErrTracker),
 		pipeline.ProvideInitializationStage(cfg, reg, lic, backendFactoryProvider, proc, authServiceRegistry, fakes.NewFakeRoleRegistry()),

pkg/services/pluginsintegration/test_helper.go

@@ -51,7 +51,7 @@ func CreateIntegrationTestCtx(t *testing.T, cfg *setting.Cfg, coreRegistry *core
 	proc := process.ProvideService()
 	errTracker := pluginerrs.ProvideSignatureErrorTracker()
 
-	disc := pipeline.ProvideDiscoveryStage(pCfg, finder.NewLocalFinder(true), reg)
+	disc := pipeline.ProvideDiscoveryStage(pCfg, finder.NewLocalFinder(true, pCfg.Features), reg)
 	boot := pipeline.ProvideBootstrapStage(pCfg, signature.ProvideService(pCfg, statickey.New()), assetpath.ProvideService(pCfg, cdn))
 	valid := pipeline.ProvideValidationStage(pCfg, signature.NewValidator(signature.NewUnsignedAuthorizer(pCfg)), angularInspector, errTracker)
 	init := pipeline.ProvideInitializationStage(pCfg, reg, fakes.NewFakeLicensingService(), provider.ProvideService(coreRegistry), proc, &fakes.FakeAuthService{}, fakes.NewFakeRoleRegistry())
@@ -86,7 +86,7 @@ type LoaderOpts struct {
 
 func CreateTestLoader(t *testing.T, cfg *pluginsCfg.Cfg, opts LoaderOpts) *loader.Loader {
 	if opts.Discoverer == nil {
-		opts.Discoverer = pipeline.ProvideDiscoveryStage(cfg, finder.NewLocalFinder(cfg.DevMode), registry.ProvideService())
+		opts.Discoverer = pipeline.ProvideDiscoveryStage(cfg, finder.NewLocalFinder(cfg.DevMode, cfg.Features), registry.ProvideService())
 	}
 
 	if opts.Bootstrapper == nil {