From 9d453d0dcc5b7e57de3238c370cd470d0b78cd4a Mon Sep 17 00:00:00 2001 From: Will Browne Date: Fri, 15 Mar 2024 10:58:51 +0100 Subject: [PATCH] Plugins: Remove direct featuremgmt.FeatureToggles dependency from plugins config (#84482) --- pkg/cmd/grafana-cli/services/services.go | 3 +- pkg/plugins/config/config.go | 11 +++-- pkg/plugins/manager/loader/finder/local.go | 7 +-- .../manager/loader/finder/local_test.go | 9 ++-- pkg/plugins/manager/loader/loader_test.go | 19 +++----- .../manager/pipeline/bootstrap/steps.go | 4 +- .../manager/pipeline/bootstrap/steps_test.go | 15 +++--- .../manager/pipeline/discovery/steps.go | 2 +- .../pluginsintegration/loader/loader_test.go | 46 +++++++------------ .../pluginsintegration/pipeline/steps.go | 3 +- .../pluginsintegration/pipeline/steps_test.go | 5 +- .../pluginsintegration/pluginconfig/config.go | 5 +- .../pluginsintegration/test_helper.go | 4 +- 13 files changed, 58 insertions(+), 75 deletions(-) diff --git a/pkg/cmd/grafana-cli/services/services.go b/pkg/cmd/grafana-cli/services/services.go index 53bebf8f552..00fffc487e2 100644 --- a/pkg/cmd/grafana-cli/services/services.go +++ b/pkg/cmd/grafana-cli/services/services.go @@ -15,7 +15,6 @@ 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 ( @@ -78,7 +77,7 @@ func GetLocalPlugin(pluginDir, pluginID string) (plugins.FoundPlugin, error) { } func GetLocalPlugins(pluginDir string) []*plugins.FoundBundle { - f := finder.NewLocalFinder(true, featuremgmt.WithFeatures()) + f := finder.NewLocalFinder(true) res, err := f.Find(context.Background(), sources.NewLocalSource(plugins.ClassExternal, []string{pluginDir})) if err != nil { diff --git a/pkg/plugins/config/config.go b/pkg/plugins/config/config.go index 88c4b8364c6..9fcf3bba71a 100644 --- a/pkg/plugins/config/config.go +++ b/pkg/plugins/config/config.go @@ -1,7 +1,6 @@ package config import ( - "github.com/grafana/grafana/pkg/services/featuremgmt" "github.com/grafana/grafana/pkg/setting" ) @@ -23,15 +22,21 @@ type PluginManagementCfg struct { GrafanaAppURL string - Features featuremgmt.FeatureToggles + Features Features AngularSupportEnabled bool HideAngularDeprecation []string } +// Features contains the feature toggles used for the plugin management system. +type Features struct { + ExternalCorePluginsEnabled bool + SkipHostEnvVarsEnabled bool +} + // NewPluginManagementCfg returns a new PluginManagementCfg. func NewPluginManagementCfg(devMode bool, pluginsPath string, pluginSettings setting.PluginSettings, pluginsAllowUnsigned []string, - pluginsCDNURLTemplate string, appURL string, features featuremgmt.FeatureToggles, angularSupportEnabled bool, + pluginsCDNURLTemplate string, appURL string, features Features, angularSupportEnabled bool, grafanaComURL string, disablePlugins []string, hideAngularDeprecation []string, forwardHostEnvVars []string, ) *PluginManagementCfg { return &PluginManagementCfg{ diff --git a/pkg/plugins/manager/loader/finder/local.go b/pkg/plugins/manager/loader/finder/local.go index 5c11a2cc651..29f3bacfed1 100644 --- a/pkg/plugins/manager/loader/finder/local.go +++ b/pkg/plugins/manager/loader/finder/local.go @@ -13,7 +13,6 @@ 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,19 +25,17 @@ var ( type Local struct { log log.Logger production bool - features featuremgmt.FeatureToggles } -func NewLocalFinder(devMode bool, features featuremgmt.FeatureToggles) *Local { +func NewLocalFinder(devMode bool) *Local { return &Local{ production: !devMode, log: log.New("local.finder"), - features: features, } } func ProvideLocalFinder(cfg *config.PluginManagementCfg) *Local { - return NewLocalFinder(cfg.DevMode, cfg.Features) + return NewLocalFinder(cfg.DevMode) } func (l *Local) Find(ctx context.Context, src plugins.PluginSource) ([]*plugins.FoundBundle, error) { diff --git a/pkg/plugins/manager/loader/finder/local_test.go b/pkg/plugins/manager/loader/finder/local_test.go index 738611a6abb..c7ccefbeeb9 100644 --- a/pkg/plugins/manager/loader/finder/local_test.go +++ b/pkg/plugins/manager/loader/finder/local_test.go @@ -13,7 +13,6 @@ 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" ) @@ -284,7 +283,7 @@ func TestFinder_Find(t *testing.T) { } for _, tc := range testCases { t.Run(tc.name, func(t *testing.T) { - f := NewLocalFinder(false, featuremgmt.WithFeatures(featuremgmt.FlagExternalCorePlugins)) + f := NewLocalFinder(false) pluginBundles, err := f.Find(context.Background(), &fakes.FakePluginSource{ PluginClassFunc: func(ctx context.Context) plugins.Class { return tc.pluginClass @@ -320,7 +319,7 @@ func TestFinder_getAbsPluginJSONPaths(t *testing.T) { walk = origWalk }) - finder := NewLocalFinder(false, featuremgmt.WithFeatures()) + finder := NewLocalFinder(false) paths, err := finder.getAbsPluginJSONPaths("test") require.NoError(t, err) require.Empty(t, paths) @@ -335,7 +334,7 @@ func TestFinder_getAbsPluginJSONPaths(t *testing.T) { walk = origWalk }) - finder := NewLocalFinder(false, featuremgmt.WithFeatures()) + finder := NewLocalFinder(false) paths, err := finder.getAbsPluginJSONPaths("test") require.NoError(t, err) require.Empty(t, paths) @@ -350,7 +349,7 @@ func TestFinder_getAbsPluginJSONPaths(t *testing.T) { walk = origWalk }) - finder := NewLocalFinder(false, featuremgmt.WithFeatures()) + finder := NewLocalFinder(false) paths, err := finder.getAbsPluginJSONPaths("test") require.Error(t, err) require.Empty(t, paths) diff --git a/pkg/plugins/manager/loader/loader_test.go b/pkg/plugins/manager/loader/loader_test.go index 38470968535..63471d8c535 100644 --- a/pkg/plugins/manager/loader/loader_test.go +++ b/pkg/plugins/manager/loader/loader_test.go @@ -20,7 +20,6 @@ import ( "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/sources" - "github.com/grafana/grafana/pkg/services/featuremgmt" "github.com/grafana/grafana/pkg/services/org" ) @@ -68,7 +67,7 @@ func TestLoader_Load(t *testing.T) { { name: "Load a Core plugin", class: plugins.ClassCore, - cfg: &config.PluginManagementCfg{Features: featuremgmt.WithFeatures()}, + cfg: &config.PluginManagementCfg{}, pluginPaths: []string{filepath.Join(corePluginDir, "app/plugins/datasource/cloudwatch")}, want: []*plugins.Plugin{ { @@ -117,7 +116,7 @@ func TestLoader_Load(t *testing.T) { { name: "Load a Bundled plugin", class: plugins.ClassBundled, - cfg: &config.PluginManagementCfg{Features: featuremgmt.WithFeatures()}, + cfg: &config.PluginManagementCfg{}, pluginPaths: []string{"../testdata/valid-v2-signature"}, want: []*plugins.Plugin{ { @@ -158,7 +157,7 @@ func TestLoader_Load(t *testing.T) { { name: "Load plugin with symbolic links", class: plugins.ClassExternal, - cfg: &config.PluginManagementCfg{Features: featuremgmt.WithFeatures()}, + cfg: &config.PluginManagementCfg{}, pluginPaths: []string{"../testdata/symbolic-plugin-dirs"}, want: []*plugins.Plugin{ { @@ -238,8 +237,7 @@ func TestLoader_Load(t *testing.T) { name: "Load an unsigned plugin (development)", class: plugins.ClassExternal, cfg: &config.PluginManagementCfg{ - DevMode: true, - Features: featuremgmt.WithFeatures(), + DevMode: true, }, pluginPaths: []string{"../testdata/unsigned-datasource"}, want: []*plugins.Plugin{ @@ -277,7 +275,7 @@ func TestLoader_Load(t *testing.T) { { name: "Load an unsigned plugin (production)", class: plugins.ClassExternal, - cfg: &config.PluginManagementCfg{Features: featuremgmt.WithFeatures()}, + cfg: &config.PluginManagementCfg{}, pluginPaths: []string{"../testdata/unsigned-datasource"}, want: []*plugins.Plugin{}, }, @@ -286,7 +284,6 @@ func TestLoader_Load(t *testing.T) { class: plugins.ClassExternal, cfg: &config.PluginManagementCfg{ PluginsAllowUnsigned: []string{"test-datasource"}, - Features: featuremgmt.WithFeatures(), }, pluginPaths: []string{"../testdata/unsigned-datasource"}, want: []*plugins.Plugin{ @@ -324,7 +321,7 @@ func TestLoader_Load(t *testing.T) { { name: "Load a plugin with v1 manifest should return signatureInvalid", class: plugins.ClassExternal, - cfg: &config.PluginManagementCfg{Features: featuremgmt.WithFeatures()}, + cfg: &config.PluginManagementCfg{}, pluginPaths: []string{"../testdata/lacking-files"}, want: []*plugins.Plugin{}, }, @@ -333,7 +330,6 @@ func TestLoader_Load(t *testing.T) { class: plugins.ClassExternal, cfg: &config.PluginManagementCfg{ PluginsAllowUnsigned: []string{"test-datasource"}, - Features: featuremgmt.WithFeatures(), }, pluginPaths: []string{"../testdata/lacking-files"}, want: []*plugins.Plugin{}, @@ -343,7 +339,6 @@ func TestLoader_Load(t *testing.T) { class: plugins.ClassExternal, cfg: &config.PluginManagementCfg{ PluginsAllowUnsigned: []string{"test-datasource"}, - Features: featuremgmt.WithFeatures(), }, pluginPaths: []string{"../testdata/invalid-v2-missing-file"}, want: []*plugins.Plugin{}, @@ -353,7 +348,6 @@ func TestLoader_Load(t *testing.T) { class: plugins.ClassExternal, cfg: &config.PluginManagementCfg{ PluginsAllowUnsigned: []string{"test-datasource"}, - Features: featuremgmt.WithFeatures(), }, pluginPaths: []string{"../testdata/invalid-v2-extra-file"}, want: []*plugins.Plugin{}, @@ -363,7 +357,6 @@ func TestLoader_Load(t *testing.T) { class: plugins.ClassExternal, cfg: &config.PluginManagementCfg{ PluginsAllowUnsigned: []string{"test-app"}, - Features: featuremgmt.WithFeatures(), }, pluginPaths: []string{"../testdata/test-app-with-includes"}, want: []*plugins.Plugin{ diff --git a/pkg/plugins/manager/pipeline/bootstrap/steps.go b/pkg/plugins/manager/pipeline/bootstrap/steps.go index dffb91df5df..9c14796830b 100644 --- a/pkg/plugins/manager/pipeline/bootstrap/steps.go +++ b/pkg/plugins/manager/pipeline/bootstrap/steps.go @@ -11,7 +11,6 @@ import ( "github.com/grafana/grafana/pkg/plugins/config" "github.com/grafana/grafana/pkg/plugins/log" "github.com/grafana/grafana/pkg/plugins/manager/loader/assetpath" - "github.com/grafana/grafana/pkg/services/featuremgmt" ) // DefaultConstructor implements the default ConstructFunc used for the Construct step of the Bootstrap stage. @@ -163,8 +162,7 @@ func configureAppChildPlugin(parent *plugins.Plugin, child *plugins.Plugin) { // ForwardHostEnvVars plugin ids list. func SkipHostEnvVarsDecorateFunc(cfg *config.PluginManagementCfg) DecorateFunc { return func(_ context.Context, p *plugins.Plugin) (*plugins.Plugin, error) { - p.SkipHostEnvVars = cfg.Features.IsEnabledGlobally(featuremgmt.FlagPluginsSkipHostEnvVars) && - !slices.Contains(cfg.ForwardHostEnvVars, p.ID) + p.SkipHostEnvVars = cfg.Features.SkipHostEnvVarsEnabled && !slices.Contains(cfg.ForwardHostEnvVars, p.ID) return p, nil } } diff --git a/pkg/plugins/manager/pipeline/bootstrap/steps_test.go b/pkg/plugins/manager/pipeline/bootstrap/steps_test.go index 2a5b15d9762..1fa03bac60e 100644 --- a/pkg/plugins/manager/pipeline/bootstrap/steps_test.go +++ b/pkg/plugins/manager/pipeline/bootstrap/steps_test.go @@ -10,7 +10,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/services/featuremgmt" ) func TestSetDefaultNavURL(t *testing.T) { @@ -143,17 +142,19 @@ func Test_configureAppChildPlugin(t *testing.T) { func TestSkipEnvVarsDecorateFunc(t *testing.T) { const pluginID = "plugin-id" - t.Run("feature flag is not present", func(t *testing.T) { - f := SkipHostEnvVarsDecorateFunc(&config.PluginManagementCfg{Features: featuremgmt.WithFeatures()}) + t.Run("config field is false", func(t *testing.T) { + f := SkipHostEnvVarsDecorateFunc(&config.PluginManagementCfg{ + Features: config.Features{SkipHostEnvVarsEnabled: false}, + }) p, err := f(context.Background(), &plugins.Plugin{JSONData: plugins.JSONData{ID: pluginID}}) require.NoError(t, err) require.False(t, p.SkipHostEnvVars) }) - t.Run("feature flag is present", func(t *testing.T) { + t.Run("config field is true", func(t *testing.T) { t.Run("no plugin settings should set SkipHostEnvVars to true", func(t *testing.T) { f := SkipHostEnvVarsDecorateFunc(&config.PluginManagementCfg{ - Features: featuremgmt.WithFeatures(featuremgmt.FlagPluginsSkipHostEnvVars), + Features: config.Features{SkipHostEnvVarsEnabled: true}, }) p, err := f(context.Background(), &plugins.Plugin{JSONData: plugins.JSONData{ID: pluginID}}) require.NoError(t, err) @@ -189,7 +190,9 @@ func TestSkipEnvVarsDecorateFunc(t *testing.T) { } { t.Run(tc.name, func(t *testing.T) { f := SkipHostEnvVarsDecorateFunc(&config.PluginManagementCfg{ - Features: featuremgmt.WithFeatures(featuremgmt.FlagPluginsSkipHostEnvVars), + Features: config.Features{ + SkipHostEnvVarsEnabled: true, + }, ForwardHostEnvVars: tc.forwardHostEnvVars, }) p, err := f(context.Background(), &plugins.Plugin{JSONData: plugins.JSONData{ID: pluginID}}) diff --git a/pkg/plugins/manager/pipeline/discovery/steps.go b/pkg/plugins/manager/pipeline/discovery/steps.go index 0256b216b20..6c5a1d07437 100644 --- a/pkg/plugins/manager/pipeline/discovery/steps.go +++ b/pkg/plugins/manager/pipeline/discovery/steps.go @@ -12,7 +12,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.PluginManagementCfg) FindFunc { - return finder.NewLocalFinder(cfg.DevMode, cfg.Features).Find + return finder.NewLocalFinder(cfg.DevMode).Find } // PermittedPluginTypesFilter is a filter step that will filter out any plugins that are not of a permitted type. diff --git a/pkg/services/pluginsintegration/loader/loader_test.go b/pkg/services/pluginsintegration/loader/loader_test.go index 4f09dc249c3..fe5696672ca 100644 --- a/pkg/services/pluginsintegration/loader/loader_test.go +++ b/pkg/services/pluginsintegration/loader/loader_test.go @@ -26,7 +26,6 @@ import ( "github.com/grafana/grafana/pkg/plugins/manager/sources" "github.com/grafana/grafana/pkg/plugins/pfs" "github.com/grafana/grafana/pkg/plugins/pluginscdn" - "github.com/grafana/grafana/pkg/services/featuremgmt" "github.com/grafana/grafana/pkg/services/org" "github.com/grafana/grafana/pkg/services/pluginsintegration/pipeline" "github.com/grafana/grafana/pkg/services/pluginsintegration/pluginerrs" @@ -68,7 +67,7 @@ func TestLoader_Load(t *testing.T) { { name: "Load a Core plugin", class: plugins.ClassCore, - cfg: &config.PluginManagementCfg{Features: featuremgmt.WithFeatures()}, + cfg: &config.PluginManagementCfg{}, pluginPaths: []string{filepath.Join(corePluginDir(t), "app/plugins/datasource/cloudwatch")}, want: []*plugins.Plugin{ { @@ -117,7 +116,7 @@ func TestLoader_Load(t *testing.T) { { name: "Load a Bundled plugin", class: plugins.ClassBundled, - cfg: &config.PluginManagementCfg{Features: featuremgmt.WithFeatures()}, + cfg: &config.PluginManagementCfg{}, pluginPaths: []string{filepath.Join(testDataDir(t), "valid-v2-signature")}, want: []*plugins.Plugin{ { @@ -158,7 +157,7 @@ func TestLoader_Load(t *testing.T) { { name: "Load plugin with symbolic links", class: plugins.ClassExternal, - cfg: &config.PluginManagementCfg{Features: featuremgmt.WithFeatures()}, + cfg: &config.PluginManagementCfg{}, pluginPaths: []string{filepath.Join(testDataDir(t), "symbolic-plugin-dirs")}, want: []*plugins.Plugin{ { @@ -238,8 +237,7 @@ func TestLoader_Load(t *testing.T) { name: "Load an unsigned plugin (development)", class: plugins.ClassExternal, cfg: &config.PluginManagementCfg{ - DevMode: true, - Features: featuremgmt.WithFeatures(), + DevMode: true, }, pluginPaths: []string{filepath.Join(testDataDir(t), "unsigned-datasource")}, want: []*plugins.Plugin{ @@ -277,7 +275,7 @@ func TestLoader_Load(t *testing.T) { { name: "Load an unsigned plugin (production)", class: plugins.ClassExternal, - cfg: &config.PluginManagementCfg{Features: featuremgmt.WithFeatures()}, + cfg: &config.PluginManagementCfg{}, pluginPaths: []string{filepath.Join(testDataDir(t), "unsigned-datasource")}, want: []*plugins.Plugin{}, pluginErrors: map[string]*plugins.SignatureError{ @@ -292,7 +290,6 @@ func TestLoader_Load(t *testing.T) { class: plugins.ClassExternal, cfg: &config.PluginManagementCfg{ PluginsAllowUnsigned: []string{"test-datasource"}, - Features: featuremgmt.WithFeatures(), }, pluginPaths: []string{filepath.Join(testDataDir(t), "unsigned-datasource")}, want: []*plugins.Plugin{ @@ -330,7 +327,7 @@ func TestLoader_Load(t *testing.T) { { name: "Load a plugin with v1 manifest should return signatureInvalid", class: plugins.ClassExternal, - cfg: &config.PluginManagementCfg{Features: featuremgmt.WithFeatures()}, + cfg: &config.PluginManagementCfg{}, pluginPaths: []string{filepath.Join(testDataDir(t), "lacking-files")}, want: []*plugins.Plugin{}, pluginErrors: map[string]*plugins.SignatureError{ @@ -345,7 +342,6 @@ func TestLoader_Load(t *testing.T) { class: plugins.ClassExternal, cfg: &config.PluginManagementCfg{ PluginsAllowUnsigned: []string{"test-datasource"}, - Features: featuremgmt.WithFeatures(), }, pluginPaths: []string{filepath.Join(testDataDir(t), "lacking-files")}, want: []*plugins.Plugin{}, @@ -361,7 +357,6 @@ func TestLoader_Load(t *testing.T) { class: plugins.ClassExternal, cfg: &config.PluginManagementCfg{ PluginsAllowUnsigned: []string{"test-datasource"}, - Features: featuremgmt.WithFeatures(), }, pluginPaths: []string{filepath.Join(testDataDir(t), "invalid-v2-missing-file")}, want: []*plugins.Plugin{}, @@ -377,7 +372,6 @@ func TestLoader_Load(t *testing.T) { class: plugins.ClassExternal, cfg: &config.PluginManagementCfg{ PluginsAllowUnsigned: []string{"test-datasource"}, - Features: featuremgmt.WithFeatures(), }, pluginPaths: []string{filepath.Join(testDataDir(t), "invalid-v2-extra-file")}, want: []*plugins.Plugin{}, @@ -393,7 +387,6 @@ func TestLoader_Load(t *testing.T) { class: plugins.ClassExternal, cfg: &config.PluginManagementCfg{ PluginsAllowUnsigned: []string{"test-app"}, - Features: featuremgmt.WithFeatures(), }, pluginPaths: []string{filepath.Join(testDataDir(t), "test-app-with-includes")}, want: []*plugins.Plugin{ @@ -471,7 +464,6 @@ func TestLoader_Load_ExternalRegistration(t *testing.T) { t.Run("Load a plugin with service account registration", func(t *testing.T) { cfg := &config.PluginManagementCfg{ - Features: featuremgmt.WithFeatures(featuremgmt.FlagExternalServiceAccounts), PluginsAllowUnsigned: []string{"grafana-test-datasource"}, } pluginPaths := []string{filepath.Join(testDataDir(t), "external-registration")} @@ -564,7 +556,6 @@ func TestLoader_Load_CustomSource(t *testing.T) { PluginSettings: setting.PluginSettings{ "grafana-worldmap-panel": {"cdn": "true"}, }, - Features: featuremgmt.WithFeatures(), } pluginPaths := []string{filepath.Join(testDataDir(t), "cdn")} @@ -647,7 +638,6 @@ func TestLoader_Load_MultiplePlugins(t *testing.T) { name: "Load multiple plugins (broken, valid, unsigned)", cfg: &config.PluginManagementCfg{ GrafanaAppURL: "http://localhost:3000", - Features: featuremgmt.WithFeatures(), }, pluginPaths: []string{ filepath.Join(testDataDir(t), "invalid-plugin-json"), // test-app @@ -744,7 +734,6 @@ func TestLoader_Load_RBACReady(t *testing.T) { name: "Load plugin defining one RBAC role", cfg: &config.PluginManagementCfg{ GrafanaAppURL: "http://localhost:3000", - Features: featuremgmt.WithFeatures(), }, pluginPaths: []string{filepath.Join(testDataDir(t), "test-app-with-roles")}, want: []*plugins.Plugin{ @@ -863,7 +852,7 @@ func TestLoader_Load_Signature_RootURL(t *testing.T) { reg := fakes.NewFakePluginRegistry() procPrvdr := fakes.NewFakeBackendProcessProvider() procMgr := fakes.NewFakeProcessManager() - cfg := &config.PluginManagementCfg{GrafanaAppURL: defaultAppURL, Features: featuremgmt.WithFeatures()} + cfg := &config.PluginManagementCfg{GrafanaAppURL: defaultAppURL} l := newLoader(t, cfg, reg, procMgr, procPrvdr, newFakeSignatureErrorTracker()) got, err := l.Load(context.Background(), &fakes.FakePluginSource{ PluginClassFunc: func(ctx context.Context) plugins.Class { @@ -940,7 +929,7 @@ func TestLoader_Load_DuplicatePlugins(t *testing.T) { reg := fakes.NewFakePluginRegistry() procPrvdr := fakes.NewFakeBackendProcessProvider() procMgr := fakes.NewFakeProcessManager() - cfg := &config.PluginManagementCfg{Features: featuremgmt.WithFeatures()} + cfg := &config.PluginManagementCfg{} l := newLoader(t, cfg, reg, procMgr, procPrvdr, newFakeSignatureErrorTracker()) got, err := l.Load(context.Background(), &fakes.FakePluginSource{ PluginClassFunc: func(ctx context.Context) plugins.Class { @@ -1030,7 +1019,7 @@ func TestLoader_Load_SkipUninitializedPlugins(t *testing.T) { } } procMgr := fakes.NewFakeProcessManager() - cfg := &config.PluginManagementCfg{Features: featuremgmt.WithFeatures()} + cfg := &config.PluginManagementCfg{} l := newLoader(t, cfg, reg, procMgr, procPrvdr, newFakeSignatureErrorTracker()) got, err := l.Load(context.Background(), &fakes.FakePluginSource{ PluginClassFunc: func(ctx context.Context) plugins.Class { @@ -1087,7 +1076,7 @@ func TestLoader_AngularClass(t *testing.T) { }, } // if angularDetected = true, it means that the detection has run - l := newLoaderWithOpts(t, &config.PluginManagementCfg{AngularSupportEnabled: true, Features: featuremgmt.WithFeatures()}, loaderDepOpts{ + l := newLoaderWithOpts(t, &config.PluginManagementCfg{AngularSupportEnabled: true}, loaderDepOpts{ angularInspector: angularinspector.AlwaysAngularFakeInspector, }) p, err := l.Load(context.Background(), fakePluginSource) @@ -1115,8 +1104,8 @@ func TestLoader_Load_Angular(t *testing.T) { name string cfg *config.PluginManagementCfg }{ - {name: "angular support enabled", cfg: &config.PluginManagementCfg{AngularSupportEnabled: true, Features: featuremgmt.WithFeatures()}}, - {name: "angular support disabled", cfg: &config.PluginManagementCfg{AngularSupportEnabled: false, Features: featuremgmt.WithFeatures()}}, + {name: "angular support enabled", cfg: &config.PluginManagementCfg{AngularSupportEnabled: true}}, + {name: "angular support disabled", cfg: &config.PluginManagementCfg{AngularSupportEnabled: false}}, } { t.Run(cfgTc.name, func(t *testing.T) { for _, tc := range []struct { @@ -1169,17 +1158,14 @@ func TestLoader_HideAngularDeprecation(t *testing.T) { {name: "with plugin id in HideAngularDeprecation list", cfg: &config.PluginManagementCfg{ AngularSupportEnabled: true, HideAngularDeprecation: []string{"one-app", "two-panel", "test-datasource", "three-datasource"}, - Features: featuremgmt.WithFeatures(), }, expHideAngularDeprecation: true}, {name: "without plugin id in HideAngularDeprecation list", cfg: &config.PluginManagementCfg{ AngularSupportEnabled: true, HideAngularDeprecation: []string{"one-app", "two-panel", "three-datasource"}, - Features: featuremgmt.WithFeatures(), }, expHideAngularDeprecation: false}, {name: "with empty HideAngularDeprecation", cfg: &config.PluginManagementCfg{ AngularSupportEnabled: true, HideAngularDeprecation: nil, - Features: featuremgmt.WithFeatures(), }, expHideAngularDeprecation: false}, } { t.Run(tc.name, func(t *testing.T) { @@ -1267,7 +1253,7 @@ func TestLoader_Load_NestedPlugins(t *testing.T) { procPrvdr := fakes.NewFakeBackendProcessProvider() procMgr := fakes.NewFakeProcessManager() reg := fakes.NewFakePluginRegistry() - cfg := &config.PluginManagementCfg{Features: featuremgmt.WithFeatures()} + cfg := &config.PluginManagementCfg{} l := newLoader(t, cfg, reg, procMgr, procPrvdr, newFakeSignatureErrorTracker()) got, err := l.Load(context.Background(), &fakes.FakePluginSource{ @@ -1444,7 +1430,7 @@ func TestLoader_Load_NestedPlugins(t *testing.T) { reg := fakes.NewFakePluginRegistry() procPrvdr := fakes.NewFakeBackendProcessProvider() procMgr := fakes.NewFakeProcessManager() - cfg := &config.PluginManagementCfg{Features: featuremgmt.WithFeatures()} + cfg := &config.PluginManagementCfg{} l := newLoader(t, cfg, reg, procMgr, procPrvdr, newFakeSignatureErrorTracker()) got, err := l.Load(context.Background(), &fakes.FakePluginSource{ PluginClassFunc: func(ctx context.Context) plugins.Class { @@ -1485,7 +1471,7 @@ func newLoader(t *testing.T, cfg *config.PluginManagementCfg, reg registry.Servi require.NoError(t, err) return ProvideService(pipeline.ProvideDiscoveryStage(cfg, - finder.NewLocalFinder(false, featuremgmt.WithFeatures()), reg), + finder.NewLocalFinder(false), reg), pipeline.ProvideBootstrapStage(cfg, signature.DefaultCalculator(cfg), assets), pipeline.ProvideValidationStage(cfg, signature.NewValidator(signature.NewUnsignedAuthorizer(cfg)), angularInspector, sigErrTracker), pipeline.ProvideInitializationStage(cfg, reg, backendFactory, proc, &fakes.FakeAuthService{}, fakes.NewFakeRoleRegistry(), fakes.NewFakePluginEnvProvider()), @@ -1517,7 +1503,7 @@ func newLoaderWithOpts(t *testing.T, cfg *config.PluginManagementCfg, opts loade } return ProvideService(pipeline.ProvideDiscoveryStage(cfg, - finder.NewLocalFinder(false, featuremgmt.WithFeatures()), reg), + finder.NewLocalFinder(false), reg), pipeline.ProvideBootstrapStage(cfg, signature.DefaultCalculator(cfg), assets), pipeline.ProvideValidationStage(cfg, signature.NewValidator(signature.NewUnsignedAuthorizer(cfg)), angularInspector, sigErrTracker), pipeline.ProvideInitializationStage(cfg, reg, backendFactoryProvider, proc, authServiceRegistry, fakes.NewFakeRoleRegistry(), fakes.NewFakePluginEnvProvider()), diff --git a/pkg/services/pluginsintegration/pipeline/steps.go b/pkg/services/pluginsintegration/pipeline/steps.go index 61a35a7ded0..d39babd1e5f 100644 --- a/pkg/services/pluginsintegration/pipeline/steps.go +++ b/pkg/services/pluginsintegration/pipeline/steps.go @@ -15,7 +15,6 @@ import ( "github.com/grafana/grafana/pkg/plugins/manager/registry" "github.com/grafana/grafana/pkg/plugins/manager/signature" "github.com/grafana/grafana/pkg/plugins/pfs" - "github.com/grafana/grafana/pkg/services/featuremgmt" "github.com/grafana/grafana/pkg/services/pluginsintegration/pluginerrs" ) @@ -177,7 +176,7 @@ func NewAsExternalStep(cfg *config.PluginManagementCfg) *AsExternal { // Filter will filter out any plugins that are marked to be disabled. func (c *AsExternal) Filter(cl plugins.Class, bundles []*plugins.FoundBundle) ([]*plugins.FoundBundle, error) { - if c.cfg.Features == nil || !c.cfg.Features.IsEnabledGlobally(featuremgmt.FlagExternalCorePlugins) { + if !c.cfg.Features.ExternalCorePluginsEnabled { return bundles, nil } diff --git a/pkg/services/pluginsintegration/pipeline/steps_test.go b/pkg/services/pluginsintegration/pipeline/steps_test.go index 09ac6c41df2..24b7def7bde 100644 --- a/pkg/services/pluginsintegration/pipeline/steps_test.go +++ b/pkg/services/pluginsintegration/pipeline/steps_test.go @@ -9,7 +9,6 @@ import ( "github.com/grafana/grafana/pkg/plugins" "github.com/grafana/grafana/pkg/plugins/config" "github.com/grafana/grafana/pkg/plugins/manager/registry" - "github.com/grafana/grafana/pkg/services/featuremgmt" "github.com/grafana/grafana/pkg/setting" ) @@ -69,7 +68,9 @@ func TestAsExternal(t *testing.T) { t.Run("should skip a core plugin", func(t *testing.T) { cfg := &config.PluginManagementCfg{ - Features: featuremgmt.WithFeatures(featuremgmt.FlagExternalCorePlugins), + Features: config.Features{ + ExternalCorePluginsEnabled: true, + }, PluginSettings: setting.PluginSettings{ "plugin1": map[string]string{ "as_external": "true", diff --git a/pkg/services/pluginsintegration/pluginconfig/config.go b/pkg/services/pluginsintegration/pluginconfig/config.go index 7542cbe1f1b..fae18205330 100644 --- a/pkg/services/pluginsintegration/pluginconfig/config.go +++ b/pkg/services/pluginsintegration/pluginconfig/config.go @@ -29,7 +29,10 @@ func ProvidePluginManagementConfig(cfg *setting.Cfg, settingProvider setting.Pro allowedUnsigned, cfg.PluginsCDNURLTemplate, cfg.AppURL, - features, + config.Features{ + ExternalCorePluginsEnabled: features.IsEnabledGlobally(featuremgmt.FlagExternalCorePlugins), + SkipHostEnvVarsEnabled: features.IsEnabledGlobally(featuremgmt.FlagPluginsSkipHostEnvVars), + }, cfg.AngularSupportEnabled, cfg.GrafanaComURL, cfg.DisablePlugins, diff --git a/pkg/services/pluginsintegration/test_helper.go b/pkg/services/pluginsintegration/test_helper.go index 1242bf2d3e9..0515e6a50cd 100644 --- a/pkg/services/pluginsintegration/test_helper.go +++ b/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, pCfg.Features), reg) + disc := pipeline.ProvideDiscoveryStage(pCfg, finder.NewLocalFinder(true), 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, provider.ProvideService(coreRegistry), proc, &fakes.FakeAuthService{}, fakes.NewFakeRoleRegistry(), nil) @@ -86,7 +86,7 @@ type LoaderOpts struct { func CreateTestLoader(t *testing.T, cfg *pluginsCfg.PluginManagementCfg, opts LoaderOpts) *loader.Loader { if opts.Discoverer == nil { - opts.Discoverer = pipeline.ProvideDiscoveryStage(cfg, finder.NewLocalFinder(cfg.DevMode, cfg.Features), registry.ProvideService()) + opts.Discoverer = pipeline.ProvideDiscoveryStage(cfg, finder.NewLocalFinder(cfg.DevMode), registry.ProvideService()) } if opts.Bootstrapper == nil {