public/grafana
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Plugins: Remove direct featuremgmt.FeatureToggles dependency from plugins config (#84482)

Browse Source
This commit is contained in:
Will Browne
2024-03-15 10:58:51 +01:00
committed by GitHub
parent c13e248384
commit 9d453d0dcc
13 changed files with 58 additions and 75 deletions
Download Patch File Download Diff File Expand all files Collapse all files
pkg/plugins/manager/loader/finder/local.go
+2 -5
View File
@@ -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) {
pkg/plugins/manager/loader/finder/local_test.go
+4 -5
View File
@@ -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)
pkg/plugins/manager/loader/loader_test.go
+6 -13
View File
@@ -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{
pkg/plugins/manager/pipeline/bootstrap/steps.go
+1 -3
View File
@@ -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
}
}
pkg/plugins/manager/pipeline/bootstrap/steps_test.go
+9 -6
View File
@@ -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}})
pkg/plugins/manager/pipeline/discovery/steps.go
+1 -1
View File
@@ -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.