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

Plugins: Allow disabling "skip host environment variables" per-plugin (#78266)

Browse Source 
* Plugins: Allow disabling skipping host environment variables per-plugin

* Renamed SkipEnvVarsDecorateFunc to SkipHostEnvVarsDecorateFunc

* PR review feedback

* fix tests
This commit is contained in:
Giuseppe Guerra
2023-11-17 16:12:05 +02:00
committed by GitHub
parent ddb7406caa
commit 027a157898
9 changed files with 141 additions and 48 deletions
Download Patch File Download Diff File Expand all files Collapse all files
pkg/plugins/backendplugin/provider/provider.go
+7 -13
View File
@@ -10,7 +10,6 @@ import (
"github.com/grafana/grafana/pkg/plugins/backendplugin/pluginextensionv2"
"github.com/grafana/grafana/pkg/plugins/backendplugin/secretsmanagerplugin"
"github.com/grafana/grafana/pkg/plugins/log"
"github.com/grafana/grafana/pkg/services/featuremgmt"
)
// PluginBackendProvider is a function type for initializing a Plugin backend.
@@ -18,21 +17,19 @@ type PluginBackendProvider func(_ context.Context, _ *plugins.Plugin) backendplu
type Service struct {
providerChain []PluginBackendProvider
features featuremgmt.FeatureToggles
}
func New(features featuremgmt.FeatureToggles, providers ...PluginBackendProvider) *Service {
func New(providers ...PluginBackendProvider) *Service {
if len(providers) == 0 {
return New(features, RendererProvider, SecretsManagerProvider, DefaultProvider(features))
return New(RendererProvider, SecretsManagerProvider, DefaultProvider)
}
return &Service{
providerChain: providers,
features: features,
}
}
func ProvideService(features featuremgmt.FeatureToggles, coreRegistry *coreplugin.Registry) *Service {
return New(features, coreRegistry.BackendFactoryProvider(), RendererProvider, SecretsManagerProvider, DefaultProvider(features))
func ProvideService(coreRegistry *coreplugin.Registry) *Service {
return New(coreRegistry.BackendFactoryProvider(), RendererProvider, SecretsManagerProvider, DefaultProvider)
}
func (s *Service) BackendFactory(ctx context.Context, p *plugins.Plugin) backendplugin.PluginFactoryFunc {
@@ -68,9 +65,6 @@ var SecretsManagerProvider PluginBackendProvider = func(_ context.Context, p *pl
)
}
func DefaultProvider(features featuremgmt.FeatureToggles) PluginBackendProvider {
return func(_ context.Context, p *plugins.Plugin) backendplugin.PluginFactoryFunc {
skipEnvVars := features.IsEnabledGlobally(featuremgmt.FlagPluginsSkipHostEnvVars)
return grpcplugin.NewBackendPlugin(p.ID, p.ExecutablePath(), skipEnvVars)
}
}
var DefaultProvider = PluginBackendProvider(func(_ context.Context, p *plugins.Plugin) backendplugin.PluginFactoryFunc {
return grpcplugin.NewBackendPlugin(p.ID, p.ExecutablePath(), p.SkipHostEnvVars)
})
pkg/plugins/envvars/envvars.go
+3 -4
View File
@@ -17,7 +17,6 @@ import (
"github.com/grafana/grafana/pkg/plugins"
"github.com/grafana/grafana/pkg/plugins/auth"
"github.com/grafana/grafana/pkg/plugins/config"
"github.com/grafana/grafana/pkg/services/featuremgmt"
)
const (
@@ -86,10 +85,10 @@ func (s *Service) Get(ctx context.Context, p *plugins.Plugin) []string {
hostEnv = append(hostEnv, azsettings.WriteToEnvStr(s.cfg.Azure)...)
hostEnv = append(hostEnv, s.tracingEnvVars(p)...)
// If FlagPluginsSkipHostEnvVars is enabled, get some allowed variables from the current process and pass
// them down to the plugin. If the feature flag is not enabled, do not add anything else because ALL env vars
// If SkipHostEnvVars is enabled, get some allowed variables from the current process and pass
// them down to the plugin. If the flag is not set, do not add anything else because ALL env vars
// from the current process (os.Environ()) will be forwarded to the plugin's process by go-plugin
if s.cfg.Features != nil && s.cfg.Features.IsEnabledGlobally(featuremgmt.FlagPluginsSkipHostEnvVars) {
if p.SkipHostEnvVars {
hostEnv = append(hostEnv, s.allowedHostEnvVars()...)
}
pkg/plugins/envvars/envvars_test.go
+6 -5
View File
@@ -38,7 +38,6 @@ func TestInitializer_envVars(t *testing.T) {
"custom_env_var": "customVal",
},
},
Features: featuremgmt.WithFeatures(),
}, licensing)
envVars := envVarsProvider.Get(context.Background(), p)
@@ -77,10 +76,12 @@ func TestInitializer_skipHostEnvVars(t *testing.T) {
require.False(t, ok, "host env var should not be present")
})
t.Run("with FlagPluginsSkipHostEnvVars", func(t *testing.T) {
envVarsProvider := NewProvider(&config.Cfg{
Features: featuremgmt.WithFeatures(featuremgmt.FlagPluginsSkipHostEnvVars),
}, nil)
t.Run("with SkipHostEnvVars = true", func(t *testing.T) {
p := &plugins.Plugin{
JSONData: plugins.JSONData{ID: "test"},
SkipHostEnvVars: true,
}
envVarsProvider := NewProvider(&config.Cfg{}, nil)
t.Run("should populate allowed host env vars", func(t *testing.T) {
// Set all allowed variables
pkg/plugins/manager/loader/loader_test.go
+14 -6
View File
@@ -20,6 +20,7 @@ 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"
)
@@ -67,7 +68,7 @@ func TestLoader_Load(t *testing.T) {
{
name: "Load a Core plugin",
class: plugins.ClassCore,
cfg: &config.Cfg{},
cfg: &config.Cfg{Features: featuremgmt.WithFeatures()},
pluginPaths: []string{filepath.Join(corePluginDir, "app/plugins/datasource/cloudwatch")},
want: []*plugins.Plugin{
{
@@ -117,7 +118,7 @@ func TestLoader_Load(t *testing.T) {
{
name: "Load a Bundled plugin",
class: plugins.ClassBundled,
cfg: &config.Cfg{},
cfg: &config.Cfg{Features: featuremgmt.WithFeatures()},
pluginPaths: []string{"../testdata/valid-v2-signature"},
want: []*plugins.Plugin{
{
@@ -157,7 +158,7 @@ func TestLoader_Load(t *testing.T) {
}, {
name: "Load plugin with symbolic links",
class: plugins.ClassExternal,
cfg: &config.Cfg{},
cfg: &config.Cfg{Features: featuremgmt.WithFeatures()},
pluginPaths: []string{"../testdata/symbolic-plugin-dirs"},
want: []*plugins.Plugin{
{
@@ -234,7 +235,8 @@ func TestLoader_Load(t *testing.T) {
name: "Load an unsigned plugin (development)",
class: plugins.ClassExternal,
cfg: &config.Cfg{
DevMode: true,
DevMode: true,
Features: featuremgmt.WithFeatures(),
},
pluginPaths: []string{"../testdata/unsigned-datasource"},
want: []*plugins.Plugin{
@@ -272,7 +274,7 @@ func TestLoader_Load(t *testing.T) {
{
name: "Load an unsigned plugin (production)",
class: plugins.ClassExternal,
cfg: &config.Cfg{},
cfg: &config.Cfg{Features: featuremgmt.WithFeatures()},
pluginPaths: []string{"../testdata/unsigned-datasource"},
want: []*plugins.Plugin{},
},
@@ -281,6 +283,7 @@ func TestLoader_Load(t *testing.T) {
class: plugins.ClassExternal,
cfg: &config.Cfg{
PluginsAllowUnsigned: []string{"test-datasource"},
Features: featuremgmt.WithFeatures(),
},
pluginPaths: []string{"../testdata/unsigned-datasource"},
want: []*plugins.Plugin{
@@ -318,7 +321,7 @@ func TestLoader_Load(t *testing.T) {
{
name: "Load a plugin with v1 manifest should return signatureInvalid",
class: plugins.ClassExternal,
cfg: &config.Cfg{},
cfg: &config.Cfg{Features: featuremgmt.WithFeatures()},
pluginPaths: []string{"../testdata/lacking-files"},
want: []*plugins.Plugin{},
},
@@ -327,6 +330,7 @@ func TestLoader_Load(t *testing.T) {
class: plugins.ClassExternal,
cfg: &config.Cfg{
PluginsAllowUnsigned: []string{"test-datasource"},
Features: featuremgmt.WithFeatures(),
},
pluginPaths: []string{"../testdata/lacking-files"},
want: []*plugins.Plugin{},
@@ -336,6 +340,7 @@ func TestLoader_Load(t *testing.T) {
class: plugins.ClassExternal,
cfg: &config.Cfg{
PluginsAllowUnsigned: []string{"test-datasource"},
Features: featuremgmt.WithFeatures(),
},
pluginPaths: []string{"../testdata/invalid-v2-missing-file"},
want: []*plugins.Plugin{},
@@ -345,6 +350,7 @@ func TestLoader_Load(t *testing.T) {
class: plugins.ClassExternal,
cfg: &config.Cfg{
PluginsAllowUnsigned: []string{"test-datasource"},
Features: featuremgmt.WithFeatures(),
},
pluginPaths: []string{"../testdata/invalid-v2-extra-file"},
want: []*plugins.Plugin{},
@@ -354,6 +360,7 @@ func TestLoader_Load(t *testing.T) {
class: plugins.ClassExternal,
cfg: &config.Cfg{
PluginsAllowUnsigned: []string{"test-app"},
Features: featuremgmt.WithFeatures(),
},
pluginPaths: []string{"../testdata/test-app-with-includes"},
want: []*plugins.Plugin{
@@ -405,6 +412,7 @@ func TestLoader_Load(t *testing.T) {
cfg: &config.Cfg{
DevMode: true,
GrafanaAppSubURL: "grafana",
Features: featuremgmt.WithFeatures(),
},
pluginPaths: []string{"../testdata/unsigned-datasource"},
want: []*plugins.Plugin{
pkg/plugins/manager/pipeline/bootstrap/steps.go
+13
View File
@@ -10,6 +10,7 @@ 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.
@@ -32,6 +33,7 @@ func DefaultDecorateFuncs(cfg *config.Cfg) []DecorateFunc {
AppDefaultNavURLDecorateFunc,
TemplateDecorateFunc,
AppChildDecorateFunc(cfg),
SkipHostEnvVarsDecorateFunc(cfg),
}
}
@@ -153,3 +155,14 @@ func configureAppChildPlugin(cfg *config.Cfg, parent *plugins.Plugin, child *plu
child.Module = path.Join("/", cfg.GrafanaAppSubURL, "/public/plugins", parent.ID, appSubPath, "module.js")
}
}
// SkipHostEnvVarsDecorateFunc returns a DecorateFunc that configures the SkipHostEnvVars field of the plugin.
// It will be set to true if the FlagPluginsSkipHostEnvVars feature flag is set, and the plugin does not have
// forward_host_env_vars = true in its plugin settings.
func SkipHostEnvVarsDecorateFunc(cfg *config.Cfg) DecorateFunc {
return func(_ context.Context, p *plugins.Plugin) (*plugins.Plugin, error) {
p.SkipHostEnvVars = cfg.Features.IsEnabledGlobally(featuremgmt.FlagPluginsSkipHostEnvVars) &&
cfg.PluginSettings[p.ID]["forward_host_env_vars"] != "true"
return p, nil
}
}
pkg/plugins/manager/pipeline/bootstrap/steps_test.go
+63
View File
@@ -10,6 +10,8 @@ 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"
"github.com/grafana/grafana/pkg/setting"
)
func TestSetDefaultNavURL(t *testing.T) {
@@ -146,3 +148,64 @@ func Test_configureAppChildPlugin(t *testing.T) {
require.Equal(t, "/grafana/plugins/parent-app", child.BaseURL)
})
}
func TestSkipEnvVarsDecorateFunc(t *testing.T) {
const pluginID = "plugin-id"
t.Run("feature flag is not present", func(t *testing.T) {
f := SkipHostEnvVarsDecorateFunc(&config.Cfg{Features: featuremgmt.WithFeatures()})
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("no plugin settings should set SkipHostEnvVars to true", func(t *testing.T) {
f := SkipHostEnvVarsDecorateFunc(&config.Cfg{
Features: featuremgmt.WithFeatures(featuremgmt.FlagPluginsSkipHostEnvVars),
})
p, err := f(context.Background(), &plugins.Plugin{JSONData: plugins.JSONData{ID: pluginID}})
require.NoError(t, err)
require.True(t, p.SkipHostEnvVars)
})
t.Run("plugin setting", func(t *testing.T) {
for _, tc := range []struct {
name string
pluginSettings setting.PluginSettings
expSkipHostEnvVars bool
}{
{
name: "forward_host_env_vars = false should set SkipHostEnvVars to true",
pluginSettings: setting.PluginSettings{pluginID: map[string]string{"forward_host_env_vars": "false"}},
expSkipHostEnvVars: true,
},
{
name: "forward_host_env_vars = true should set SkipHostEnvVars to false",
pluginSettings: setting.PluginSettings{pluginID: map[string]string{"forward_host_env_vars": "true"}},
expSkipHostEnvVars: false,
},
{
name: "invalid forward_host_env_vars should set SkipHostEnvVars to true",
pluginSettings: setting.PluginSettings{pluginID: map[string]string{"forward_host_env_vars": "grilled cheese sandwich with bacon"}},
expSkipHostEnvVars: true,
},
{
name: "forward_host_env_vars absent should set SkipHostEnvVars to true",
pluginSettings: setting.PluginSettings{pluginID: nil},
expSkipHostEnvVars: true,
},
} {
t.Run(tc.name, func(t *testing.T) {
f := SkipHostEnvVarsDecorateFunc(&config.Cfg{
Features: featuremgmt.WithFeatures(featuremgmt.FlagPluginsSkipHostEnvVars),
PluginSettings: tc.pluginSettings,
})
p, err := f(context.Background(), &plugins.Plugin{JSONData: plugins.JSONData{ID: pluginID}})
require.NoError(t, err)
require.Equal(t, tc.expSkipHostEnvVars, p.SkipHostEnvVars)
})
}
})
})
}
pkg/plugins/plugins.go
+2
View File
@@ -64,6 +64,8 @@ type Plugin struct {
client backendplugin.Plugin
log log.Logger
SkipHostEnvVars bool
mu sync.Mutex
}