diff --git a/pkg/plugins/manager/loader/loader.go b/pkg/plugins/manager/loader/loader.go index 3b784c44842..2abc88ef735 100644 --- a/pkg/plugins/manager/loader/loader.go +++ b/pkg/plugins/manager/loader/loader.go @@ -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 diff --git a/pkg/plugins/manager/loader/loader_test.go b/pkg/plugins/manager/loader/loader_test.go index 0a52639758d..76a2a3ba65b 100644 --- a/pkg/plugins/manager/loader/loader_test.go +++ b/pkg/plugins/manager/loader/loader_test.go @@ -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) diff --git a/pkg/plugins/manager/process/process.go b/pkg/plugins/manager/process/process.go index 2219e5f2180..d2a663d0983 100644 --- a/pkg/plugins/manager/process/process.go +++ b/pkg/plugins/manager/process/process.go @@ -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 } diff --git a/pkg/plugins/manager/process/process_test.go b/pkg/plugins/manager/process/process_test.go index 4654832f65a..5dca9b5eddc 100644 --- a/pkg/plugins/manager/process/process_test.go +++ b/pkg/plugins/manager/process/process_test.go @@ -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() diff --git a/pkg/plugins/manager/signature/signature.go b/pkg/plugins/manager/signature/signature.go index 74a1cf355f6..57a205c58df 100644 --- a/pkg/plugins/manager/signature/signature.go +++ b/pkg/plugins/manager/signature/signature.go @@ -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, } } diff --git a/pkg/plugins/models.go b/pkg/plugins/models.go index 5553ad331a5..f9e263eefe4 100644 --- a/pkg/plugins/models.go +++ b/pkg/plugins/models.go @@ -39,41 +39,6 @@ func (e DuplicateError) Is(err error) bool { return ok } -type SignatureError struct { - PluginID string `json:"pluginId"` - SignatureStatus SignatureStatus `json:"status"` -} - -func (e SignatureError) Error() string { - switch e.SignatureStatus { - case SignatureStatusInvalid: - return fmt.Sprintf("plugin '%s' has an invalid signature", e.PluginID) - case SignatureStatusModified: - return fmt.Sprintf("plugin '%s' has an modified signature", e.PluginID) - case SignatureStatusUnsigned: - return fmt.Sprintf("plugin '%s' has no signature", e.PluginID) - case SignatureStatusInternal, SignatureStatusValid: - return "" - } - - return fmt.Sprintf("plugin '%s' has an unknown signature state", e.PluginID) -} - -func (e SignatureError) AsErrorCode() ErrorCode { - switch e.SignatureStatus { - case SignatureStatusInvalid: - return errorCodeSignatureInvalid - case SignatureStatusModified: - return errorCodeSignatureModified - case SignatureStatusUnsigned: - return errorCodeSignatureMissing - case SignatureStatusInternal, SignatureStatusValid: - return "" - } - - return "" -} - type Dependencies struct { GrafanaDependency string `json:"grafanaDependency"` GrafanaVersion string `json:"grafanaVersion"` @@ -278,8 +243,41 @@ const ( type ErrorCode string type Error struct { - ErrorCode `json:"errorCode"` - PluginID string `json:"pluginId,omitempty"` + ErrorCode `json:"errorCode"` + PluginID string `json:"pluginId,omitempty"` + SignatureStatus SignatureStatus `json:"status,omitempty"` +} + +func (e Error) Error() string { + if e.SignatureStatus != "" { + switch e.SignatureStatus { + case SignatureStatusInvalid: + return fmt.Sprintf("plugin '%s' has an invalid signature", e.PluginID) + case SignatureStatusModified: + return fmt.Sprintf("plugin '%s' has an modified signature", e.PluginID) + case SignatureStatusUnsigned: + return fmt.Sprintf("plugin '%s' has no signature", e.PluginID) + case SignatureStatusInternal, SignatureStatusValid: + return "" + } + } + + return fmt.Sprintf("plugin '%s' failed: %s", e.PluginID, e.ErrorCode) +} + +func (e Error) AsErrorCode() ErrorCode { + switch e.SignatureStatus { + case SignatureStatusInvalid: + return errorCodeSignatureInvalid + case SignatureStatusModified: + return errorCodeSignatureModified + case SignatureStatusUnsigned: + return errorCodeSignatureMissing + case SignatureStatusInternal, SignatureStatusValid: + return "" + } + + return "" } // Access-Control related definitions diff --git a/pkg/plugins/plugins.go b/pkg/plugins/plugins.go index 15e43ad07d9..1a045651277 100644 --- a/pkg/plugins/plugins.go +++ b/pkg/plugins/plugins.go @@ -44,12 +44,12 @@ type Plugin struct { Pinned bool // Signature fields - Signature SignatureStatus - SignatureType SignatureType - SignatureOrg string - Parent *Plugin - Children []*Plugin - SignatureError *SignatureError + Signature SignatureStatus + SignatureType SignatureType + SignatureOrg string + Parent *Plugin + Children []*Plugin + Error *Error // SystemJS fields Module string diff --git a/pkg/services/pluginsintegration/loader/fakes.go b/pkg/services/pluginsintegration/loader/fakes.go index fc636ea2e76..108882835ab 100644 --- a/pkg/services/pluginsintegration/loader/fakes.go +++ b/pkg/services/pluginsintegration/loader/fakes.go @@ -6,33 +6,33 @@ import ( "github.com/grafana/grafana/pkg/plugins" ) -type fakeSignatureErrorTracker struct { - RecordFunc func(ctx context.Context, err *plugins.SignatureError) - ClearFunc func(ctx context.Context, pluginID string) - SignatureErrorsFunc func(ctx context.Context) []*plugins.SignatureError +type fakeErrorTracker struct { + RecordFunc func(ctx context.Context, err *plugins.Error) + ClearFunc func(ctx context.Context, pluginID string) + ErrorsFunc func(ctx context.Context) []*plugins.Error } -func newFakeSignatureErrorTracker() *fakeSignatureErrorTracker { - return &fakeSignatureErrorTracker{} +func newFakeErrorTracker() *fakeErrorTracker { + return &fakeErrorTracker{} } -func (t *fakeSignatureErrorTracker) Record(ctx context.Context, err *plugins.SignatureError) { +func (t *fakeErrorTracker) Record(ctx context.Context, err *plugins.Error) { if t.RecordFunc != nil { t.RecordFunc(ctx, err) return } } -func (t *fakeSignatureErrorTracker) Clear(ctx context.Context, pluginID string) { +func (t *fakeErrorTracker) Clear(ctx context.Context, pluginID string) { if t.ClearFunc != nil { t.ClearFunc(ctx, pluginID) return } } -func (t *fakeSignatureErrorTracker) SignatureErrors(ctx context.Context) []*plugins.SignatureError { - if t.SignatureErrorsFunc != nil { - return t.SignatureErrorsFunc(ctx) +func (t *fakeErrorTracker) Errors(ctx context.Context) []*plugins.Error { + if t.ErrorsFunc != nil { + return t.ErrorsFunc(ctx) } return nil } diff --git a/pkg/services/pluginsintegration/loader/loader.go b/pkg/services/pluginsintegration/loader/loader.go index c55553739c3..4852e84d1b4 100644 --- a/pkg/services/pluginsintegration/loader/loader.go +++ b/pkg/services/pluginsintegration/loader/loader.go @@ -10,6 +10,7 @@ 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" ) var _ pluginsLoader.Service = (*Loader)(nil) @@ -19,10 +20,10 @@ type Loader struct { } func ProvideService(discovery discovery.Discoverer, bootstrap bootstrap.Bootstrapper, validation validation.Validator, - initializer initialization.Initializer, termination termination.Terminator, + initializer initialization.Initializer, termination termination.Terminator, errorTracker pluginerrs.ErrorTracker, ) *Loader { return &Loader{ - loader: pluginsLoader.New(discovery, bootstrap, validation, initializer, termination), + loader: pluginsLoader.New(discovery, bootstrap, validation, initializer, termination, errorTracker), } } diff --git a/pkg/services/pluginsintegration/loader/loader_test.go b/pkg/services/pluginsintegration/loader/loader_test.go index a844289c97c..fb1f19d41d4 100644 --- a/pkg/services/pluginsintegration/loader/loader_test.go +++ b/pkg/services/pluginsintegration/loader/loader_test.go @@ -63,7 +63,7 @@ func TestLoader_Load(t *testing.T) { cfg *config.PluginManagementCfg pluginPaths []string want []*plugins.Plugin - pluginErrors map[string]*plugins.SignatureError + pluginErrors map[string]*plugins.Error }{ { name: "Load a Core plugin", @@ -279,7 +279,7 @@ func TestLoader_Load(t *testing.T) { cfg: &config.PluginManagementCfg{}, pluginPaths: []string{filepath.Join(testDataDir(t), "unsigned-datasource")}, want: []*plugins.Plugin{}, - pluginErrors: map[string]*plugins.SignatureError{ + pluginErrors: map[string]*plugins.Error{ "test-datasource": { PluginID: "test-datasource", SignatureStatus: plugins.SignatureStatusUnsigned, @@ -331,7 +331,7 @@ func TestLoader_Load(t *testing.T) { cfg: &config.PluginManagementCfg{}, pluginPaths: []string{filepath.Join(testDataDir(t), "lacking-files")}, want: []*plugins.Plugin{}, - pluginErrors: map[string]*plugins.SignatureError{ + pluginErrors: map[string]*plugins.Error{ "test-datasource": { PluginID: "test-datasource", SignatureStatus: plugins.SignatureStatusInvalid, @@ -346,7 +346,7 @@ func TestLoader_Load(t *testing.T) { }, pluginPaths: []string{filepath.Join(testDataDir(t), "lacking-files")}, want: []*plugins.Plugin{}, - pluginErrors: map[string]*plugins.SignatureError{ + pluginErrors: map[string]*plugins.Error{ "test-datasource": { PluginID: "test-datasource", SignatureStatus: plugins.SignatureStatusInvalid, @@ -361,7 +361,7 @@ func TestLoader_Load(t *testing.T) { }, pluginPaths: []string{filepath.Join(testDataDir(t), "invalid-v2-missing-file")}, want: []*plugins.Plugin{}, - pluginErrors: map[string]*plugins.SignatureError{ + pluginErrors: map[string]*plugins.Error{ "test-datasource": { PluginID: "test-datasource", SignatureStatus: plugins.SignatureStatusModified, @@ -376,7 +376,7 @@ func TestLoader_Load(t *testing.T) { }, pluginPaths: []string{filepath.Join(testDataDir(t), "invalid-v2-extra-file")}, want: []*plugins.Plugin{}, - pluginErrors: map[string]*plugins.SignatureError{ + pluginErrors: map[string]*plugins.Error{ "test-datasource": { PluginID: "test-datasource", SignatureStatus: plugins.SignatureStatusModified, @@ -439,7 +439,7 @@ func TestLoader_Load(t *testing.T) { reg := fakes.NewFakePluginRegistry() procPrvdr := fakes.NewFakeBackendProcessProvider() procMgr := fakes.NewFakeProcessManager() - errTracker := pluginerrs.ProvideSignatureErrorTracker() + errTracker := pluginerrs.ProvideErrorTracker() l := newLoader(t, tt.cfg, reg, procMgr, procPrvdr, errTracker) t.Run(tt.name, func(t *testing.T) { @@ -449,7 +449,7 @@ func TestLoader_Load(t *testing.T) { t.Fatalf("Result mismatch (-want +got):\n%s", cmp.Diff(got, tt.want, compareOpts...)) } - pluginErrs := errTracker.SignatureErrors(context.Background()) + pluginErrs := errTracker.Errors(context.Background()) require.Equal(t, len(tt.pluginErrors), len(pluginErrs)) for _, pluginErr := range pluginErrs { require.Equal(t, tt.pluginErrors[pluginErr.PluginID], pluginErr) @@ -603,7 +603,7 @@ func TestLoader_Load_CustomSource(t *testing.T) { Module: "https://cdn.example.com/grafana-worldmap-panel/0.3.3/public/plugins/grafana-worldmap-panel/module.js", }} - l := newLoader(t, cfg, fakes.NewFakePluginRegistry(), fakes.NewFakeProcessManager(), fakes.NewFakeBackendProcessProvider(), newFakeSignatureErrorTracker()) + l := newLoader(t, cfg, fakes.NewFakePluginRegistry(), fakes.NewFakeProcessManager(), fakes.NewFakeBackendProcessProvider(), newFakeErrorTracker()) got, err := l.Load(context.Background(), &fakes.FakePluginSource{ PluginClassFunc: func(ctx context.Context) plugins.Class { return plugins.ClassBundled @@ -633,7 +633,7 @@ func TestLoader_Load_MultiplePlugins(t *testing.T) { pluginPaths []string existingPlugins map[string]struct{} want []*plugins.Plugin - pluginErrors map[string]*plugins.SignatureError + pluginErrors map[string]*plugins.Error }{ { name: "Load multiple plugins (broken, valid, unsigned)", @@ -680,7 +680,7 @@ func TestLoader_Load_MultiplePlugins(t *testing.T) { SignatureOrg: "Will Browne", }, }, - pluginErrors: map[string]*plugins.SignatureError{ + pluginErrors: map[string]*plugins.Error{ "test-panel": { PluginID: "test-panel", SignatureStatus: plugins.SignatureStatusUnsigned, @@ -693,7 +693,7 @@ func TestLoader_Load_MultiplePlugins(t *testing.T) { reg := fakes.NewFakePluginRegistry() procPrvdr := fakes.NewFakeBackendProcessProvider() procMgr := fakes.NewFakeProcessManager() - errTracker := pluginerrs.ProvideSignatureErrorTracker() + errTracker := pluginerrs.ProvideErrorTracker() l := newLoader(t, tt.cfg, reg, procMgr, procPrvdr, errTracker) t.Run(tt.name, func(t *testing.T) { @@ -712,7 +712,7 @@ func TestLoader_Load_MultiplePlugins(t *testing.T) { if !cmp.Equal(got, tt.want, compareOpts...) { t.Fatalf("Result mismatch (-want +got):\n%s", cmp.Diff(got, tt.want, compareOpts...)) } - pluginErrs := errTracker.SignatureErrors(context.Background()) + pluginErrs := errTracker.Errors(context.Background()) require.Equal(t, len(tt.pluginErrors), len(pluginErrs)) for _, pluginErr := range pluginErrs { require.Equal(t, tt.pluginErrors[pluginErr.PluginID], pluginErr) @@ -796,7 +796,7 @@ func TestLoader_Load_RBACReady(t *testing.T) { reg := fakes.NewFakePluginRegistry() procPrvdr := fakes.NewFakeBackendProcessProvider() procMgr := fakes.NewFakeProcessManager() - l := newLoader(t, tt.cfg, reg, procMgr, procPrvdr, newFakeSignatureErrorTracker()) + l := newLoader(t, tt.cfg, reg, procMgr, procPrvdr, newFakeErrorTracker()) got, err := l.Load(context.Background(), &fakes.FakePluginSource{ PluginClassFunc: func(ctx context.Context) plugins.Class { @@ -854,7 +854,7 @@ func TestLoader_Load_Signature_RootURL(t *testing.T) { procPrvdr := fakes.NewFakeBackendProcessProvider() procMgr := fakes.NewFakeProcessManager() cfg := &config.PluginManagementCfg{GrafanaAppURL: defaultAppURL} - l := newLoader(t, cfg, reg, procMgr, procPrvdr, newFakeSignatureErrorTracker()) + l := newLoader(t, cfg, reg, procMgr, procPrvdr, newFakeErrorTracker()) got, err := l.Load(context.Background(), &fakes.FakePluginSource{ PluginClassFunc: func(ctx context.Context) plugins.Class { return plugins.ClassExternal @@ -931,7 +931,7 @@ func TestLoader_Load_DuplicatePlugins(t *testing.T) { procPrvdr := fakes.NewFakeBackendProcessProvider() procMgr := fakes.NewFakeProcessManager() cfg := &config.PluginManagementCfg{} - l := newLoader(t, cfg, reg, procMgr, procPrvdr, newFakeSignatureErrorTracker()) + l := newLoader(t, cfg, reg, procMgr, procPrvdr, newFakeErrorTracker()) got, err := l.Load(context.Background(), &fakes.FakePluginSource{ PluginClassFunc: func(ctx context.Context) plugins.Class { return plugins.ClassExternal @@ -1021,7 +1021,7 @@ func TestLoader_Load_SkipUninitializedPlugins(t *testing.T) { } procMgr := fakes.NewFakeProcessManager() cfg := &config.PluginManagementCfg{} - l := newLoader(t, cfg, reg, procMgr, procPrvdr, newFakeSignatureErrorTracker()) + l := newLoader(t, cfg, reg, procMgr, procPrvdr, newFakeErrorTracker()) got, err := l.Load(context.Background(), &fakes.FakePluginSource{ PluginClassFunc: func(ctx context.Context) plugins.Class { return plugins.ClassExternal @@ -1255,7 +1255,7 @@ func TestLoader_Load_NestedPlugins(t *testing.T) { procMgr := fakes.NewFakeProcessManager() reg := fakes.NewFakePluginRegistry() cfg := &config.PluginManagementCfg{} - l := newLoader(t, cfg, reg, procMgr, procPrvdr, newFakeSignatureErrorTracker()) + l := newLoader(t, cfg, reg, procMgr, procPrvdr, newFakeErrorTracker()) got, err := l.Load(context.Background(), &fakes.FakePluginSource{ PluginClassFunc: func(ctx context.Context) plugins.Class { @@ -1432,7 +1432,7 @@ func TestLoader_Load_NestedPlugins(t *testing.T) { procPrvdr := fakes.NewFakeBackendProcessProvider() procMgr := fakes.NewFakeProcessManager() cfg := &config.PluginManagementCfg{} - l := newLoader(t, cfg, reg, procMgr, procPrvdr, newFakeSignatureErrorTracker()) + l := newLoader(t, cfg, reg, procMgr, procPrvdr, newFakeErrorTracker()) got, err := l.Load(context.Background(), &fakes.FakePluginSource{ PluginClassFunc: func(ctx context.Context) plugins.Class { return plugins.ClassExternal @@ -1463,7 +1463,7 @@ type loaderDepOpts struct { } func newLoader(t *testing.T, cfg *config.PluginManagementCfg, reg registry.Service, proc process.Manager, - backendFactory plugins.BackendFactoryProvider, sigErrTracker pluginerrs.SignatureErrorTracker, + backendFactory plugins.BackendFactoryProvider, errTracker pluginerrs.ErrorTracker, ) *Loader { assets := assetpath.ProvideService(cfg, pluginscdn.ProvideService(cfg)) angularInspector := angularinspector.NewStaticInspector() @@ -1474,9 +1474,9 @@ func newLoader(t *testing.T, cfg *config.PluginManagementCfg, reg registry.Servi return ProvideService(pipeline.ProvideDiscoveryStage(cfg, finder.NewLocalFinder(false), reg), pipeline.ProvideBootstrapStage(cfg, signature.DefaultCalculator(cfg), assets), - pipeline.ProvideValidationStage(cfg, signature.NewValidator(signature.NewUnsignedAuthorizer(cfg)), angularInspector, sigErrTracker), + pipeline.ProvideValidationStage(cfg, signature.NewValidator(signature.NewUnsignedAuthorizer(cfg)), angularInspector), pipeline.ProvideInitializationStage(cfg, reg, backendFactory, proc, &fakes.FakeAuthService{}, fakes.NewFakeRoleRegistry(), fakes.NewFakePluginEnvProvider(), tracing.InitializeTracerForTest()), - terminate) + terminate, errTracker) } func newLoaderWithOpts(t *testing.T, cfg *config.PluginManagementCfg, opts loaderDepOpts) *Loader { @@ -1486,7 +1486,7 @@ func newLoaderWithOpts(t *testing.T, cfg *config.PluginManagementCfg, opts loade terminate, err := pipeline.ProvideTerminationStage(cfg, reg, proc) require.NoError(t, err) - sigErrTracker := pluginerrs.ProvideSignatureErrorTracker() + errTracker := pluginerrs.ProvideErrorTracker() angularInspector := opts.angularInspector if opts.angularInspector == nil { @@ -1506,9 +1506,9 @@ func newLoaderWithOpts(t *testing.T, cfg *config.PluginManagementCfg, opts loade return ProvideService(pipeline.ProvideDiscoveryStage(cfg, finder.NewLocalFinder(false), reg), pipeline.ProvideBootstrapStage(cfg, signature.DefaultCalculator(cfg), assets), - pipeline.ProvideValidationStage(cfg, signature.NewValidator(signature.NewUnsignedAuthorizer(cfg)), angularInspector, sigErrTracker), + pipeline.ProvideValidationStage(cfg, signature.NewValidator(signature.NewUnsignedAuthorizer(cfg)), angularInspector), pipeline.ProvideInitializationStage(cfg, reg, backendFactoryProvider, proc, authServiceRegistry, fakes.NewFakeRoleRegistry(), fakes.NewFakePluginEnvProvider(), tracing.InitializeTracerForTest()), - terminate) + terminate, errTracker) } func verifyState(t *testing.T, ps []*plugins.Plugin, reg registry.Service, diff --git a/pkg/services/pluginsintegration/pipeline/pipeline.go b/pkg/services/pluginsintegration/pipeline/pipeline.go index 7b3e9cf3d08..8a49857047a 100644 --- a/pkg/services/pluginsintegration/pipeline/pipeline.go +++ b/pkg/services/pluginsintegration/pipeline/pipeline.go @@ -19,7 +19,6 @@ import ( "github.com/grafana/grafana/pkg/plugins/manager/process" "github.com/grafana/grafana/pkg/plugins/manager/registry" "github.com/grafana/grafana/pkg/plugins/manager/signature" - "github.com/grafana/grafana/pkg/services/pluginsintegration/pluginerrs" ) func ProvideDiscoveryStage(cfg *config.PluginManagementCfg, pf finder.Finder, pr registry.Service) *discovery.Discovery { @@ -49,11 +48,10 @@ func ProvideBootstrapStage(cfg *config.PluginManagementCfg, sc plugins.Signature }) } -func ProvideValidationStage(cfg *config.PluginManagementCfg, sv signature.Validator, ai angularinspector.Inspector, - et pluginerrs.SignatureErrorTracker) *validation.Validate { +func ProvideValidationStage(cfg *config.PluginManagementCfg, sv signature.Validator, ai angularinspector.Inspector) *validation.Validate { return validation.New(cfg, validation.Opts{ ValidateFuncs: []validation.ValidateFunc{ - SignatureValidationStep(sv, et), + SignatureValidationStep(sv), validation.ModuleJSValidationStep(), validation.AngularDetectionStep(cfg, ai), }, diff --git a/pkg/services/pluginsintegration/pipeline/steps.go b/pkg/services/pluginsintegration/pipeline/steps.go index 0ac7dd69c08..85adcc6c60d 100644 --- a/pkg/services/pluginsintegration/pipeline/steps.go +++ b/pkg/services/pluginsintegration/pipeline/steps.go @@ -20,7 +20,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/pluginsintegration/pluginerrs" ) // ExternalServiceRegistration implements an InitializeFunc for registering external services. @@ -105,15 +104,12 @@ func ReportBuildMetrics(_ context.Context, p *plugins.Plugin) (*plugins.Plugin, // SignatureValidation implements a ValidateFunc for validating plugin signatures. type SignatureValidation struct { signatureValidator signature.Validator - errs pluginerrs.SignatureErrorTracker log log.Logger } // SignatureValidationStep returns a new ValidateFunc for validating plugin signatures. -func SignatureValidationStep(signatureValidator signature.Validator, - sigErr pluginerrs.SignatureErrorTracker) validation.ValidateFunc { +func SignatureValidationStep(signatureValidator signature.Validator) validation.ValidateFunc { sv := &SignatureValidation{ - errs: sigErr, signatureValidator: signatureValidator, log: log.New("plugins.signature.validation"), } @@ -121,23 +117,19 @@ func SignatureValidationStep(signatureValidator signature.Validator, } // Validate validates the plugin signature. If a signature error is encountered, the error is recorded with the -// pluginerrs.SignatureErrorTracker. +// pluginerrs.ErrorTracker. func (v *SignatureValidation) Validate(ctx context.Context, p *plugins.Plugin) error { err := v.signatureValidator.ValidateSignature(p) if err != nil { - var sigErr *plugins.SignatureError + var sigErr *plugins.Error if errors.As(err, &sigErr) { v.log.Warn("Skipping loading plugin due to problem with signature", "pluginId", p.ID, "status", sigErr.SignatureStatus) - p.SignatureError = sigErr - v.errs.Record(ctx, sigErr) + p.Error = sigErr } return err } - // clear plugin error if a pre-existing error has since been resolved - v.errs.Clear(ctx, p.ID) - return nil } diff --git a/pkg/services/pluginsintegration/pluginerrs/errors.go b/pkg/services/pluginsintegration/pluginerrs/errors.go index d2a425f3f6a..3cf2a08e090 100644 --- a/pkg/services/pluginsintegration/pluginerrs/errors.go +++ b/pkg/services/pluginsintegration/pluginerrs/errors.go @@ -10,60 +10,56 @@ import ( var _ plugins.ErrorResolver = (*Store)(nil) type Store struct { - signatureErrs SignatureErrorTracker + errs ErrorTracker } -func ProvideStore(signatureErrs SignatureErrorTracker) *Store { +func ProvideStore(errs ErrorTracker) *Store { return &Store{ - signatureErrs: signatureErrs, + errs: errs, } } func (s *Store) PluginErrors(ctx context.Context) []*plugins.Error { - sigErrs := s.signatureErrs.SignatureErrors(ctx) - errs := make([]*plugins.Error, 0, len(sigErrs)) - for _, err := range sigErrs { - errs = append(errs, &plugins.Error{ - PluginID: err.PluginID, - ErrorCode: err.AsErrorCode(), - }) + errs := s.errs.Errors(ctx) + for _, err := range errs { + err.ErrorCode = err.AsErrorCode() } return errs } -type SignatureErrorRegistry struct { - errs map[string]*plugins.SignatureError +type ErrorRegistry struct { + errs map[string]*plugins.Error log log.Logger } -type SignatureErrorTracker interface { - Record(ctx context.Context, err *plugins.SignatureError) +type ErrorTracker interface { + Record(ctx context.Context, err *plugins.Error) Clear(ctx context.Context, pluginID string) - SignatureErrors(ctx context.Context) []*plugins.SignatureError + Errors(ctx context.Context) []*plugins.Error } -func ProvideSignatureErrorTracker() *SignatureErrorRegistry { - return newSignatureErrorRegistry() +func ProvideErrorTracker() *ErrorRegistry { + return newErrorRegistry() } -func newSignatureErrorRegistry() *SignatureErrorRegistry { - return &SignatureErrorRegistry{ - errs: make(map[string]*plugins.SignatureError), +func newErrorRegistry() *ErrorRegistry { + return &ErrorRegistry{ + errs: make(map[string]*plugins.Error), log: log.New("plugins.errors"), } } -func (r *SignatureErrorRegistry) Record(_ context.Context, signatureErr *plugins.SignatureError) { - r.errs[signatureErr.PluginID] = signatureErr +func (r *ErrorRegistry) Record(_ context.Context, err *plugins.Error) { + r.errs[err.PluginID] = err } -func (r *SignatureErrorRegistry) Clear(_ context.Context, pluginID string) { +func (r *ErrorRegistry) Clear(_ context.Context, pluginID string) { delete(r.errs, pluginID) } -func (r *SignatureErrorRegistry) SignatureErrors(_ context.Context) []*plugins.SignatureError { - errs := make([]*plugins.SignatureError, 0, len(r.errs)) +func (r *ErrorRegistry) Errors(_ context.Context) []*plugins.Error { + errs := make([]*plugins.Error, 0, len(r.errs)) for _, err := range r.errs { errs = append(errs, err) } diff --git a/pkg/services/pluginsintegration/pluginsintegration.go b/pkg/services/pluginsintegration/pluginsintegration.go index 58fa082f156..b0987027ac1 100644 --- a/pkg/services/pluginsintegration/pluginsintegration.go +++ b/pkg/services/pluginsintegration/pluginsintegration.go @@ -92,8 +92,8 @@ var WireSet = wire.NewSet( wire.Bind(new(signature.Validator), new(*signature.Validation)), loader.ProvideService, wire.Bind(new(pluginLoader.Service), new(*loader.Loader)), - pluginerrs.ProvideSignatureErrorTracker, - wire.Bind(new(pluginerrs.SignatureErrorTracker), new(*pluginerrs.SignatureErrorRegistry)), + pluginerrs.ProvideErrorTracker, + wire.Bind(new(pluginerrs.ErrorTracker), new(*pluginerrs.ErrorRegistry)), pluginerrs.ProvideStore, wire.Bind(new(plugins.ErrorResolver), new(*pluginerrs.Store)), registry.ProvideService, diff --git a/pkg/services/pluginsintegration/pluginstore/plugins.go b/pkg/services/pluginsintegration/pluginstore/plugins.go index 45bd6fb1061..273e20ef3e9 100644 --- a/pkg/services/pluginsintegration/pluginstore/plugins.go +++ b/pkg/services/pluginsintegration/pluginstore/plugins.go @@ -21,10 +21,11 @@ type Plugin struct { Pinned bool // Signature fields - Signature plugins.SignatureStatus - SignatureType plugins.SignatureType - SignatureOrg string - SignatureError *plugins.SignatureError + Signature plugins.SignatureStatus + SignatureType plugins.SignatureType + SignatureOrg string + + Error *plugins.Error // SystemJS fields Module string @@ -69,7 +70,7 @@ func ToGrafanaDTO(p *plugins.Plugin) Plugin { Signature: p.Signature, SignatureType: p.SignatureType, SignatureOrg: p.SignatureOrg, - SignatureError: p.SignatureError, + Error: p.Error, Module: p.Module, BaseURL: p.BaseURL, ExternalService: p.ExternalService, diff --git a/pkg/services/pluginsintegration/renderer/renderer.go b/pkg/services/pluginsintegration/renderer/renderer.go index 178c37740be..9eb71fc7d7d 100644 --- a/pkg/services/pluginsintegration/renderer/renderer.go +++ b/pkg/services/pluginsintegration/renderer/renderer.go @@ -20,6 +20,7 @@ import ( "github.com/grafana/grafana/pkg/plugins/manager/signature" "github.com/grafana/grafana/pkg/plugins/manager/sources" "github.com/grafana/grafana/pkg/services/pluginsintegration/pipeline" + "github.com/grafana/grafana/pkg/services/pluginsintegration/pluginerrs" "github.com/grafana/grafana/pkg/services/rendering" ) @@ -136,5 +137,7 @@ func createLoader(cfg *config.PluginManagementCfg, pluginEnvProvider envvars.Pro return nil, err } - return loader.New(d, b, v, i, t), nil + et := pluginerrs.ProvideErrorTracker() + + return loader.New(d, b, v, i, t, et), nil } diff --git a/pkg/services/pluginsintegration/test_helper.go b/pkg/services/pluginsintegration/test_helper.go index 2048eb0a797..737cf84d47c 100644 --- a/pkg/services/pluginsintegration/test_helper.go +++ b/pkg/services/pluginsintegration/test_helper.go @@ -50,11 +50,10 @@ func CreateIntegrationTestCtx(t *testing.T, cfg *setting.Cfg, coreRegistry *core reg := registry.ProvideService() angularInspector := angularinspector.NewStaticInspector() proc := process.ProvideService() - errTracker := pluginerrs.ProvideSignatureErrorTracker() 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) + valid := pipeline.ProvideValidationStage(pCfg, signature.NewValidator(signature.NewUnsignedAuthorizer(pCfg)), angularInspector) init := pipeline.ProvideInitializationStage(pCfg, reg, provider.ProvideService(coreRegistry), proc, &fakes.FakeAuthService{}, fakes.NewFakeRoleRegistry(), nil, tracing.InitializeTracerForTest()) term, err := pipeline.ProvideTerminationStage(pCfg, reg, proc) require.NoError(t, err) @@ -95,7 +94,7 @@ func CreateTestLoader(t *testing.T, cfg *pluginsCfg.PluginManagementCfg, opts Lo } if opts.Validator == nil { - opts.Validator = pipeline.ProvideValidationStage(cfg, signature.NewValidator(signature.NewUnsignedAuthorizer(cfg)), angularinspector.NewStaticInspector(), pluginerrs.ProvideSignatureErrorTracker()) + opts.Validator = pipeline.ProvideValidationStage(cfg, signature.NewValidator(signature.NewUnsignedAuthorizer(cfg)), angularinspector.NewStaticInspector()) } if opts.Initializer == nil { @@ -111,5 +110,5 @@ func CreateTestLoader(t *testing.T, cfg *pluginsCfg.PluginManagementCfg, opts Lo require.NoError(t, err) } - return loader.New(opts.Discoverer, opts.Bootstrapper, opts.Validator, opts.Initializer, opts.Terminator) + return loader.New(opts.Discoverer, opts.Bootstrapper, opts.Validator, opts.Initializer, opts.Terminator, pluginerrs.ProvideErrorTracker()) }