Revamp plugin loading error management (#85939)

This commit is contained in:
Andres Martinez Gotor
2024-04-11 16:18:04 +02:00
committed by GitHub
parent bb56f4a605
commit ab5a065256
17 changed files with 298 additions and 161 deletions

View File

@ -2,6 +2,7 @@ package loader
import (
"context"
"errors"
"sort"
"strings"
"time"
@ -13,29 +14,44 @@ import (
"github.com/grafana/grafana/pkg/plugins/manager/pipeline/initialization"
"github.com/grafana/grafana/pkg/plugins/manager/pipeline/termination"
"github.com/grafana/grafana/pkg/plugins/manager/pipeline/validation"
"github.com/grafana/grafana/pkg/services/pluginsintegration/pluginerrs"
)
type Loader struct {
discovery discovery.Discoverer
bootstrap bootstrap.Bootstrapper
initializer initialization.Initializer
termination termination.Terminator
validation validation.Validator
log log.Logger
discovery discovery.Discoverer
bootstrap bootstrap.Bootstrapper
initializer initialization.Initializer
termination termination.Terminator
validation validation.Validator
errorTracker pluginerrs.ErrorTracker
log log.Logger
}
func New(discovery discovery.Discoverer, bootstrap bootstrap.Bootstrapper, validation validation.Validator,
initializer initialization.Initializer, termination termination.Terminator) *Loader {
initializer initialization.Initializer, termination termination.Terminator, errorTracker pluginerrs.ErrorTracker) *Loader {
return &Loader{
discovery: discovery,
bootstrap: bootstrap,
validation: validation,
initializer: initializer,
termination: termination,
log: log.New("plugin.loader"),
discovery: discovery,
bootstrap: bootstrap,
validation: validation,
initializer: initializer,
termination: termination,
errorTracker: errorTracker,
log: log.New("plugin.loader"),
}
}
func (l *Loader) recordError(ctx context.Context, p *plugins.Plugin, err error) {
var pErr *plugins.Error
if errors.As(err, &pErr) {
l.errorTracker.Record(ctx, pErr)
return
}
l.errorTracker.Record(ctx, &plugins.Error{
PluginID: p.ID,
ErrorCode: plugins.ErrorCode(err.Error()),
})
}
func (l *Loader) Load(ctx context.Context, src plugins.PluginSource) ([]*plugins.Plugin, error) {
end := l.instrumentLoad(ctx, src)
@ -48,7 +64,10 @@ func (l *Loader) Load(ctx context.Context, src plugins.PluginSource) ([]*plugins
for _, foundBundle := range discoveredPlugins {
bootstrappedPlugin, err := l.bootstrap.Bootstrap(ctx, src, foundBundle)
if err != nil {
// TODO: Add error to registry
l.errorTracker.Record(ctx, &plugins.Error{
PluginID: foundBundle.Primary.JSONData.ID,
ErrorCode: plugins.ErrorCode(err.Error()),
})
continue
}
bootstrappedPlugins = append(bootstrappedPlugins, bootstrappedPlugin...)
@ -58,7 +77,7 @@ func (l *Loader) Load(ctx context.Context, src plugins.PluginSource) ([]*plugins
for _, bootstrappedPlugin := range bootstrappedPlugins {
err := l.validation.Validate(ctx, bootstrappedPlugin)
if err != nil {
// TODO: Add error to registry
l.recordError(ctx, bootstrappedPlugin, err)
continue
}
validatedPlugins = append(validatedPlugins, bootstrappedPlugin)
@ -68,12 +87,17 @@ func (l *Loader) Load(ctx context.Context, src plugins.PluginSource) ([]*plugins
for _, validatedPlugin := range validatedPlugins {
initializedPlugin, err := l.initializer.Initialize(ctx, validatedPlugin)
if err != nil {
// TODO: Add error to registry
l.recordError(ctx, validatedPlugin, err)
continue
}
initializedPlugins = append(initializedPlugins, initializedPlugin)
}
// Clean errors from registry for initialized plugins
for _, p := range initializedPlugins {
l.errorTracker.Clear(ctx, p.ID)
}
end(initializedPlugins)
return initializedPlugins, nil

View File

@ -21,6 +21,7 @@ import (
"github.com/grafana/grafana/pkg/plugins/manager/pipeline/validation"
"github.com/grafana/grafana/pkg/plugins/manager/sources"
"github.com/grafana/grafana/pkg/services/org"
"github.com/grafana/grafana/pkg/services/pluginsintegration/pluginerrs"
)
var compareOpts = []cmp.Option{cmpopts.IgnoreFields(plugins.Plugin{}, "client", "log", "mu"), fsComparer}
@ -408,10 +409,11 @@ func TestLoader_Load(t *testing.T) {
t.Run(tt.name, func(t *testing.T) {
terminationStage, err := termination.New(tt.cfg, termination.Opts{})
require.NoError(t, err)
et := pluginerrs.ProvideErrorTracker()
l := New(discovery.New(tt.cfg, discovery.Opts{}), bootstrap.New(tt.cfg, bootstrap.Opts{}),
validation.New(tt.cfg, validation.Opts{}), initialization.New(tt.cfg, initialization.Opts{}),
terminationStage)
terminationStage, et)
got, err := l.Load(context.Background(), sources.NewLocalSource(tt.class, tt.pluginPaths))
require.NoError(t, err)
@ -433,6 +435,7 @@ func TestLoader_Load(t *testing.T) {
return plugins.Signature{}, false
},
}
et := pluginerrs.ProvideErrorTracker()
pluginJSON := plugins.JSONData{ID: "test-datasource", Type: plugins.TypeDataSource, Info: plugins.Info{Version: "1.0.0"}}
plugin := &plugins.Plugin{
JSONData: pluginJSON,
@ -469,17 +472,139 @@ func TestLoader_Load(t *testing.T) {
steps = append(steps, "initialize")
return ps, nil
},
}, &fakes.FakeTerminator{})
}, &fakes.FakeTerminator{}, et)
got, err := l.Load(context.Background(), src)
require.NoError(t, err)
require.Equal(t, []*plugins.Plugin{plugin}, got)
require.Equal(t, []string{"discover", "bootstrap", "validate", "initialize"}, steps)
})
t.Run("With error", func(t *testing.T) {
src := &fakes.FakePluginSource{
PluginClassFunc: func(ctx context.Context) plugins.Class {
return plugins.ClassExternal
},
PluginURIsFunc: func(ctx context.Context) []string {
return []string{"http://example.com"}
},
DefaultSignatureFunc: func(ctx context.Context) (plugins.Signature, bool) {
return plugins.Signature{}, false
},
}
et := pluginerrs.ProvideErrorTracker()
pluginJSON := plugins.JSONData{ID: "test-datasource", Type: plugins.TypeDataSource, Info: plugins.Info{Version: "1.0.0"}}
plugin := &plugins.Plugin{
JSONData: pluginJSON,
Signature: plugins.SignatureStatusValid,
SignatureType: plugins.SignatureTypeCommunity,
FS: plugins.NewFakeFS(),
}
var steps []string
l := New(
&fakes.FakeDiscoverer{
DiscoverFunc: func(ctx context.Context, s plugins.PluginSource) ([]*plugins.FoundBundle, error) {
require.Equal(t, src, s)
steps = append(steps, "discover")
return []*plugins.FoundBundle{{Primary: plugins.FoundPlugin{JSONData: pluginJSON}}}, nil
},
}, &fakes.FakeBootstrapper{
BootstrapFunc: func(ctx context.Context, s plugins.PluginSource, b *plugins.FoundBundle) ([]*plugins.Plugin, error) {
require.Equal(t, b.Primary.JSONData, pluginJSON)
require.Equal(t, src, s)
steps = append(steps, "bootstrap")
return []*plugins.Plugin{plugin}, nil
},
}, &fakes.FakeValidator{ValidateFunc: func(ctx context.Context, ps *plugins.Plugin) error {
require.Equal(t, plugin, ps)
steps = append(steps, "validate")
return errors.New("validation error")
}},
&fakes.FakeInitializer{
IntializeFunc: func(ctx context.Context, ps *plugins.Plugin) (*plugins.Plugin, error) {
require.Equal(t, ps.JSONData, pluginJSON)
steps = append(steps, "initialize")
return ps, nil
},
}, &fakes.FakeTerminator{}, et)
got, err := l.Load(context.Background(), src)
require.NoError(t, err)
require.Equal(t, []*plugins.Plugin{}, got)
// Initialize should not be executed if validation fails
require.Equal(t, []string{"discover", "bootstrap", "validate"}, steps)
errs := et.Errors(context.Background())
require.Len(t, errs, 1)
require.ErrorContains(t, errs[0], "validation error")
})
t.Run("Cleans up a previous error", func(t *testing.T) {
src := &fakes.FakePluginSource{
PluginClassFunc: func(ctx context.Context) plugins.Class {
return plugins.ClassExternal
},
PluginURIsFunc: func(ctx context.Context) []string {
return []string{"http://example.com"}
},
DefaultSignatureFunc: func(ctx context.Context) (plugins.Signature, bool) {
return plugins.Signature{}, false
},
}
et := pluginerrs.ProvideErrorTracker()
pluginJSON := plugins.JSONData{ID: "test-datasource", Type: plugins.TypeDataSource, Info: plugins.Info{Version: "1.0.0"}}
plugin := &plugins.Plugin{
JSONData: pluginJSON,
Signature: plugins.SignatureStatusValid,
SignatureType: plugins.SignatureTypeCommunity,
FS: plugins.NewFakeFS(),
}
et.Record(context.Background(), &plugins.Error{PluginID: "test-datasource", ErrorCode: plugins.ErrorCode("previous error")})
var steps []string
l := New(
&fakes.FakeDiscoverer{
DiscoverFunc: func(ctx context.Context, s plugins.PluginSource) ([]*plugins.FoundBundle, error) {
require.Equal(t, src, s)
steps = append(steps, "discover")
return []*plugins.FoundBundle{{Primary: plugins.FoundPlugin{JSONData: pluginJSON}}}, nil
},
}, &fakes.FakeBootstrapper{
BootstrapFunc: func(ctx context.Context, s plugins.PluginSource, b *plugins.FoundBundle) ([]*plugins.Plugin, error) {
require.Equal(t, b.Primary.JSONData, pluginJSON)
require.Equal(t, src, s)
steps = append(steps, "bootstrap")
return []*plugins.Plugin{plugin}, nil
},
}, &fakes.FakeValidator{ValidateFunc: func(ctx context.Context, ps *plugins.Plugin) error {
require.Equal(t, plugin, ps)
steps = append(steps, "validate")
return nil
}},
&fakes.FakeInitializer{
IntializeFunc: func(ctx context.Context, ps *plugins.Plugin) (*plugins.Plugin, error) {
require.Equal(t, ps.JSONData, pluginJSON)
steps = append(steps, "initialize")
return ps, nil
},
}, &fakes.FakeTerminator{}, et)
got, err := l.Load(context.Background(), src)
require.NoError(t, err)
require.Equal(t, []*plugins.Plugin{plugin}, got)
require.Equal(t, []string{"discover", "bootstrap", "validate", "initialize"}, steps)
errs := et.Errors(context.Background())
require.Len(t, errs, 0)
})
}
func TestLoader_Unload(t *testing.T) {
t.Run("Termination stage error is returned from Unload", func(t *testing.T) {
et := pluginerrs.ProvideErrorTracker()
plugin := &plugins.Plugin{
JSONData: plugins.JSONData{ID: "test-datasource", Type: plugins.TypeDataSource, Info: plugins.Info{Version: "1.0.0"}},
}
@ -504,7 +629,7 @@ func TestLoader_Unload(t *testing.T) {
require.Equal(t, plugin, p)
return p, tc.expectedErr
},
})
}, et)
_, err := l.Unload(context.Background(), plugin)
require.ErrorIs(t, err, tc.expectedErr)

View File

@ -20,7 +20,7 @@ func ProvideService() *Service {
}
func (s *Service) Start(ctx context.Context, p *plugins.Plugin) error {
if !p.IsManaged() || !p.Backend || p.SignatureError != nil {
if !p.IsManaged() || !p.Backend || p.Error != nil {
return nil
}

View File

@ -23,7 +23,7 @@ func TestProcessManager_Start(t *testing.T) {
name string
managed bool
backend bool
signatureError *plugins.SignatureError
Error *plugins.Error
expectedStartCount int
}{
{
@ -42,7 +42,7 @@ func TestProcessManager_Start(t *testing.T) {
name: "Managed backend plugin with signature error will not be started",
managed: true,
backend: true,
signatureError: &plugins.SignatureError{
Error: &plugins.Error{
SignatureStatus: plugins.SignatureStatusUnsigned,
},
expectedStartCount: 0,
@ -65,7 +65,7 @@ func TestProcessManager_Start(t *testing.T) {
bp := fakes.NewFakeBackendPlugin(tc.managed)
p := createPlugin(t, bp, func(plugin *plugins.Plugin) {
plugin.Backend = tc.backend
plugin.SignatureError = tc.signatureError
plugin.Error = tc.Error
})
m := ProvideService()

View File

@ -57,7 +57,7 @@ func (s *Validation) ValidateSignature(plugin *plugins.Plugin) error {
case plugins.SignatureStatusUnsigned:
if authorized := s.authorizer.CanLoadPlugin(plugin); !authorized {
s.log.Debug("Plugin is unsigned", "pluginId", plugin.ID)
return &plugins.SignatureError{
return &plugins.Error{
PluginID: plugin.ID,
SignatureStatus: plugins.SignatureStatusUnsigned,
}
@ -66,20 +66,20 @@ func (s *Validation) ValidateSignature(plugin *plugins.Plugin) error {
return nil
case plugins.SignatureStatusInvalid:
s.log.Debug("Plugin has an invalid signature", "pluginId", plugin.ID)
return &plugins.SignatureError{
return &plugins.Error{
PluginID: plugin.ID,
SignatureStatus: plugins.SignatureStatusInvalid,
}
case plugins.SignatureStatusModified:
s.log.Debug("Plugin has a modified signature", "pluginId", plugin.ID)
return &plugins.SignatureError{
return &plugins.Error{
PluginID: plugin.ID,
SignatureStatus: plugins.SignatureStatusModified,
}
default:
s.log.Debug("Plugin has an unrecognized plugin signature state", "pluginId", plugin.ID, "signature",
plugin.Signature)
return &plugins.SignatureError{
return &plugins.Error{
PluginID: plugin.ID,
}
}