Plugins: Add file store abstraction for handling plugin files (#65432)

* add file store

* fix markdown fetch bug

* add markdown tests

* fix var name
This commit is contained in:
Will Browne
2023-03-29 11:55:55 +01:00
committed by GitHub
parent 562d8dba5d
commit 7bbe255150
13 changed files with 212 additions and 189 deletions

View File

@@ -133,6 +133,7 @@ type HTTPServer struct {
pluginClient plugins.Client pluginClient plugins.Client
pluginStore plugins.Store pluginStore plugins.Store
pluginInstaller plugins.Installer pluginInstaller plugins.Installer
pluginFileStore plugins.FileStore
pluginDashboardService plugindashboards.Service pluginDashboardService plugindashboards.Service
pluginStaticRouteResolver plugins.StaticRouteResolver pluginStaticRouteResolver plugins.StaticRouteResolver
pluginErrorResolver plugins.ErrorResolver pluginErrorResolver plugins.ErrorResolver
@@ -230,7 +231,7 @@ func ProvideHTTPServer(opts ServerOptions, cfg *setting.Cfg, routeRegister routi
quotaService quota.Service, socialService social.Service, tracer tracing.Tracer, quotaService quota.Service, socialService social.Service, tracer tracing.Tracer,
encryptionService encryption.Internal, grafanaUpdateChecker *updatechecker.GrafanaService, encryptionService encryption.Internal, grafanaUpdateChecker *updatechecker.GrafanaService,
pluginsUpdateChecker *updatechecker.PluginsService, searchUsersService searchusers.Service, pluginsUpdateChecker *updatechecker.PluginsService, searchUsersService searchusers.Service,
dataSourcesService datasources.DataSourceService, queryDataService query.Service, dataSourcesService datasources.DataSourceService, queryDataService query.Service, pluginFileStore plugins.FileStore,
teamGuardian teamguardian.TeamGuardian, serviceaccountsService serviceaccounts.Service, teamGuardian teamguardian.TeamGuardian, serviceaccountsService serviceaccounts.Service,
authInfoService login.AuthInfoService, storageService store.StorageService, httpEntityStore httpentitystore.HTTPEntityStore, authInfoService login.AuthInfoService, storageService store.StorageService, httpEntityStore httpentitystore.HTTPEntityStore,
notificationService *notifications.NotificationService, dashboardService dashboards.DashboardService, notificationService *notifications.NotificationService, dashboardService dashboards.DashboardService,
@@ -271,6 +272,7 @@ func ProvideHTTPServer(opts ServerOptions, cfg *setting.Cfg, routeRegister routi
pluginStaticRouteResolver: pluginStaticRouteResolver, pluginStaticRouteResolver: pluginStaticRouteResolver,
pluginDashboardService: pluginDashboardService, pluginDashboardService: pluginDashboardService,
pluginErrorResolver: pluginErrorResolver, pluginErrorResolver: pluginErrorResolver,
pluginFileStore: pluginFileStore,
grafanaUpdateChecker: grafanaUpdateChecker, grafanaUpdateChecker: grafanaUpdateChecker,
pluginsUpdateChecker: pluginsUpdateChecker, pluginsUpdateChecker: pluginsUpdateChecker,
SettingsProvider: settingsProvider, SettingsProvider: settingsProvider,

View File

@@ -6,7 +6,6 @@ import (
"encoding/json" "encoding/json"
"errors" "errors"
"fmt" "fmt"
"io"
"net/http" "net/http"
"path" "path"
"path/filepath" "path/filepath"
@@ -350,7 +349,7 @@ func (hs *HTTPServer) getPluginAssets(c *contextmodel.ReqContext) {
// serveLocalPluginAsset returns the content of a plugin asset file from the local filesystem to the http client. // serveLocalPluginAsset returns the content of a plugin asset file from the local filesystem to the http client.
func (hs *HTTPServer) serveLocalPluginAsset(c *contextmodel.ReqContext, plugin plugins.PluginDTO, assetPath string) { func (hs *HTTPServer) serveLocalPluginAsset(c *contextmodel.ReqContext, plugin plugins.PluginDTO, assetPath string) {
f, err := plugin.File(assetPath) f, err := hs.pluginFileStore.File(c.Req.Context(), plugin.ID, assetPath)
if err != nil { if err != nil {
if errors.Is(err, plugins.ErrFileNotExist) { if errors.Is(err, plugins.ErrFileNotExist) {
c.JsonApiErr(404, "Plugin file not found", nil) c.JsonApiErr(404, "Plugin file not found", nil)
@@ -359,17 +358,6 @@ func (hs *HTTPServer) serveLocalPluginAsset(c *contextmodel.ReqContext, plugin p
c.JsonApiErr(500, "Could not open plugin file", err) c.JsonApiErr(500, "Could not open plugin file", err)
return return
} }
defer func() {
if err = f.Close(); err != nil {
hs.log.Error("Failed to close plugin file", "err", err)
}
}()
fi, err := f.Stat()
if err != nil {
c.JsonApiErr(500, "Plugin file exists but could not open", err)
return
}
if hs.Cfg.Env == setting.Dev { if hs.Cfg.Env == setting.Dev {
c.Resp.Header().Set("Cache-Control", "max-age=0, must-revalidate, no-cache") c.Resp.Header().Set("Cache-Control", "max-age=0, must-revalidate, no-cache")
@@ -377,17 +365,7 @@ func (hs *HTTPServer) serveLocalPluginAsset(c *contextmodel.ReqContext, plugin p
c.Resp.Header().Set("Cache-Control", "public, max-age=3600") c.Resp.Header().Set("Cache-Control", "public, max-age=3600")
} }
if rs, ok := f.(io.ReadSeeker); ok { http.ServeContent(c.Resp, c.Req, assetPath, f.ModTime, bytes.NewReader(f.Content))
http.ServeContent(c.Resp, c.Req, assetPath, fi.ModTime(), rs)
return
}
b, err := io.ReadAll(f)
if err != nil {
c.JsonApiErr(500, "Plugin file exists but could not read", err)
return
}
http.ServeContent(c.Resp, c.Req, assetPath, fi.ModTime(), bytes.NewReader(b))
} }
// redirectCDNPluginAsset redirects the http request to specified asset path on the configured plugins CDN. // redirectCDNPluginAsset redirects the http request to specified asset path on the configured plugins CDN.
@@ -538,34 +516,24 @@ func translatePluginRequestErrorToAPIError(err error) response.Response {
return response.Error(500, "Plugin request failed", err) return response.Error(500, "Plugin request failed", err)
} }
func (hs *HTTPServer) pluginMarkdown(ctx context.Context, pluginId string, name string) ([]byte, error) { func (hs *HTTPServer) pluginMarkdown(ctx context.Context, pluginID string, name string) ([]byte, error) {
plugin, exists := hs.pluginStore.Plugin(ctx, pluginId)
if !exists {
return make([]byte, 0), plugins.NotFoundError{PluginID: pluginId}
}
file, err := mdFilepath(strings.ToUpper(name)) file, err := mdFilepath(strings.ToUpper(name))
if err != nil { if err != nil {
return make([]byte, 0), err return make([]byte, 0), err
} }
md, err := plugin.File(file) md, err := hs.pluginFileStore.File(ctx, pluginID, file)
if err != nil { if err != nil {
md, err = plugin.File(strings.ToLower(file)) if errors.Is(err, plugins.ErrPluginNotInstalled) {
return make([]byte, 0), plugins.NotFoundError{PluginID: pluginID}
}
md, err = hs.pluginFileStore.File(ctx, pluginID, strings.ToLower(file))
if err != nil { if err != nil {
return make([]byte, 0), nil return make([]byte, 0), nil
} }
} }
defer func() { return md.Content, nil
if err = md.Close(); err != nil {
hs.log.Error("Failed to close plugin markdown file", "err", err)
}
}()
d, err := io.ReadAll(md)
if err != nil {
return make([]byte, 0), nil
}
return d, nil
} }
func mdFilepath(mdFilename string) (string, error) { func mdFilepath(mdFilename string) (string, error) {

View File

@@ -1,13 +1,11 @@
package api package api
import ( import (
"bytes"
"context" "context"
"encoding/json" "encoding/json"
"errors" "errors"
"fmt" "fmt"
"io" "io"
"io/fs"
"net/http" "net/http"
"net/http/httptest" "net/http/httptest"
"os" "os"
@@ -27,6 +25,9 @@ import (
"github.com/grafana/grafana/pkg/plugins" "github.com/grafana/grafana/pkg/plugins"
"github.com/grafana/grafana/pkg/plugins/config" "github.com/grafana/grafana/pkg/plugins/config"
"github.com/grafana/grafana/pkg/plugins/manager/fakes" "github.com/grafana/grafana/pkg/plugins/manager/fakes"
"github.com/grafana/grafana/pkg/plugins/manager/filestore"
"github.com/grafana/grafana/pkg/plugins/manager/registry"
"github.com/grafana/grafana/pkg/plugins/manager/store"
"github.com/grafana/grafana/pkg/plugins/pluginscdn" "github.com/grafana/grafana/pkg/plugins/pluginscdn"
ac "github.com/grafana/grafana/pkg/services/accesscontrol" ac "github.com/grafana/grafana/pkg/services/accesscontrol"
contextmodel "github.com/grafana/grafana/pkg/services/contexthandler/model" contextmodel "github.com/grafana/grafana/pkg/services/contexthandler/model"
@@ -140,6 +141,7 @@ func Test_PluginsInstallAndUninstall_AccessControl(t *testing.T) {
PluginAdminExternalManageEnabled: tc.pluginAdminExternalManageEnabled} PluginAdminExternalManageEnabled: tc.pluginAdminExternalManageEnabled}
hs.orgService = &orgtest.FakeOrgService{ExpectedOrg: &org.Org{}} hs.orgService = &orgtest.FakeOrgService{ExpectedOrg: &org.Org{}}
hs.pluginInstaller = NewFakePluginInstaller() hs.pluginInstaller = NewFakePluginInstaller()
hs.pluginFileStore = &fakes.FakePluginFileStore{}
}) })
t.Run(testName("Install", tc), func(t *testing.T) { t.Run(testName("Install", tc), func(t *testing.T) {
@@ -172,10 +174,10 @@ func Test_GetPluginAssetCDNRedirect(t *testing.T) {
nonCdnPlugin := &plugins.Plugin{ nonCdnPlugin := &plugins.Plugin{
JSONData: plugins.JSONData{ID: nonCDNPluginID, Info: plugins.Info{Version: "2.0.0"}}, JSONData: plugins.JSONData{ID: nonCDNPluginID, Info: plugins.Info{Version: "2.0.0"}},
} }
service := &plugins.FakePluginStore{ registry := &fakes.FakePluginRegistry{
PluginList: []plugins.PluginDTO{ Store: map[string]*plugins.Plugin{
cdnPlugin.ToDTO(), cdnPluginID: cdnPlugin,
nonCdnPlugin.ToDTO(), nonCDNPluginID: nonCdnPlugin,
}, },
} }
cfg := setting.NewCfg() cfg := setting.NewCfg()
@@ -200,7 +202,7 @@ func Test_GetPluginAssetCDNRedirect(t *testing.T) {
"When calling GET for a CDN plugin on", "When calling GET for a CDN plugin on",
fmt.Sprintf("/public/plugins/%s/%s", cdnPluginID, cas.assetURL), fmt.Sprintf("/public/plugins/%s/%s", cdnPluginID, cas.assetURL),
"/public/plugins/:pluginId/*", "/public/plugins/:pluginId/*",
cfg, service, func(sc *scenarioContext) { cfg, registry, func(sc *scenarioContext) {
// Get the prometheus metric (to test that the handler is instrumented correctly) // Get the prometheus metric (to test that the handler is instrumented correctly)
counter := pluginsCDNFallbackRedirectRequests.With(prometheus.Labels{ counter := pluginsCDNFallbackRedirectRequests.With(prometheus.Labels{
"plugin_id": cdnPluginID, "plugin_id": cdnPluginID,
@@ -230,7 +232,7 @@ func Test_GetPluginAssetCDNRedirect(t *testing.T) {
"When calling GET for a non-CDN plugin on", "When calling GET for a non-CDN plugin on",
fmt.Sprintf("/public/plugins/%s/%s", nonCDNPluginID, "module.js"), fmt.Sprintf("/public/plugins/%s/%s", nonCDNPluginID, "module.js"),
"/public/plugins/:pluginId/*", "/public/plugins/:pluginId/*",
cfg, service, func(sc *scenarioContext) { cfg, registry, func(sc *scenarioContext) {
// Here the metric should not increment // Here the metric should not increment
var m dto.Metric var m dto.Metric
counter := pluginsCDNFallbackRedirectRequests.With(prometheus.Labels{ counter := pluginsCDNFallbackRedirectRequests.With(prometheus.Labels{
@@ -275,14 +277,16 @@ func Test_GetPluginAssets(t *testing.T) {
requestedFile := filepath.Clean(tmpFile.Name()) requestedFile := filepath.Clean(tmpFile.Name())
t.Run("Given a request for an existing plugin file", func(t *testing.T) { t.Run("Given a request for an existing plugin file", func(t *testing.T) {
p := createPluginDTO(plugins.JSONData{ID: pluginID}, plugins.External, plugins.NewLocalFS(map[string]struct{}{requestedFile: {}}, filepath.Dir(requestedFile))) p := createPlugin(plugins.JSONData{ID: pluginID}, plugins.External, plugins.NewLocalFS(map[string]struct{}{requestedFile: {}}, filepath.Dir(requestedFile)))
service := &plugins.FakePluginStore{ pluginRegistry := &fakes.FakePluginRegistry{
PluginList: []plugins.PluginDTO{p}, Store: map[string]*plugins.Plugin{
p.ID: p,
},
} }
url := fmt.Sprintf("/public/plugins/%s/%s", pluginID, requestedFile) url := fmt.Sprintf("/public/plugins/%s/%s", pluginID, requestedFile)
pluginAssetScenario(t, "When calling GET on", url, "/public/plugins/:pluginId/*", pluginAssetScenario(t, "When calling GET on", url, "/public/plugins/:pluginId/*",
setting.NewCfg(), service, func(sc *scenarioContext) { setting.NewCfg(), pluginRegistry, func(sc *scenarioContext) {
callGetPluginAsset(sc) callGetPluginAsset(sc)
require.Equal(t, 200, sc.resp.Code) require.Equal(t, 200, sc.resp.Code)
@@ -291,14 +295,16 @@ func Test_GetPluginAssets(t *testing.T) {
}) })
t.Run("Given a request for a relative path", func(t *testing.T) { t.Run("Given a request for a relative path", func(t *testing.T) {
p := createPluginDTO(plugins.JSONData{ID: pluginID}, plugins.External, plugins.NewLocalFS(map[string]struct{}{}, "")) p := createPlugin(plugins.JSONData{ID: pluginID}, plugins.External, plugins.NewLocalFS(map[string]struct{}{}, ""))
service := &plugins.FakePluginStore{ pluginRegistry := &fakes.FakePluginRegistry{
PluginList: []plugins.PluginDTO{p}, Store: map[string]*plugins.Plugin{
p.ID: p,
},
} }
url := fmt.Sprintf("/public/plugins/%s/%s", pluginID, tmpFileInParentDir.Name()) url := fmt.Sprintf("/public/plugins/%s/%s", pluginID, tmpFileInParentDir.Name())
pluginAssetScenario(t, "When calling GET on", url, "/public/plugins/:pluginId/*", pluginAssetScenario(t, "When calling GET on", url, "/public/plugins/:pluginId/*",
setting.NewCfg(), service, func(sc *scenarioContext) { setting.NewCfg(), pluginRegistry, func(sc *scenarioContext) {
callGetPluginAsset(sc) callGetPluginAsset(sc)
require.Equal(t, 404, sc.resp.Code) require.Equal(t, 404, sc.resp.Code)
@@ -306,16 +312,18 @@ func Test_GetPluginAssets(t *testing.T) {
}) })
t.Run("Given a request for an existing plugin file that is not listed as a signature covered file", func(t *testing.T) { t.Run("Given a request for an existing plugin file that is not listed as a signature covered file", func(t *testing.T) {
p := createPluginDTO(plugins.JSONData{ID: pluginID}, plugins.Core, plugins.NewLocalFS(map[string]struct{}{ p := createPlugin(plugins.JSONData{ID: pluginID}, plugins.Core, plugins.NewLocalFS(map[string]struct{}{
requestedFile: {}, requestedFile: {},
}, "")) }, ""))
service := &plugins.FakePluginStore{ pluginRegistry := &fakes.FakePluginRegistry{
PluginList: []plugins.PluginDTO{p}, Store: map[string]*plugins.Plugin{
p.ID: p,
},
} }
url := fmt.Sprintf("/public/plugins/%s/%s", pluginID, requestedFile) url := fmt.Sprintf("/public/plugins/%s/%s", pluginID, requestedFile)
pluginAssetScenario(t, "When calling GET on", url, "/public/plugins/:pluginId/*", pluginAssetScenario(t, "When calling GET on", url, "/public/plugins/:pluginId/*",
setting.NewCfg(), service, func(sc *scenarioContext) { setting.NewCfg(), pluginRegistry, func(sc *scenarioContext) {
callGetPluginAsset(sc) callGetPluginAsset(sc)
require.Equal(t, 200, sc.resp.Code) require.Equal(t, 200, sc.resp.Code)
@@ -324,9 +332,11 @@ func Test_GetPluginAssets(t *testing.T) {
}) })
t.Run("Given a request for an non-existing plugin file", func(t *testing.T) { t.Run("Given a request for an non-existing plugin file", func(t *testing.T) {
p := createPluginDTO(plugins.JSONData{ID: pluginID}, plugins.External, plugins.NewLocalFS(map[string]struct{}{}, "")) p := createPlugin(plugins.JSONData{ID: pluginID}, plugins.External, plugins.NewLocalFS(map[string]struct{}{}, ""))
service := &plugins.FakePluginStore{ service := &fakes.FakePluginRegistry{
PluginList: []plugins.PluginDTO{p}, Store: map[string]*plugins.Plugin{
p.ID: p,
},
} }
requestedFile := "nonExistent" requestedFile := "nonExistent"
@@ -344,14 +354,10 @@ func Test_GetPluginAssets(t *testing.T) {
}) })
t.Run("Given a request for an non-existing plugin", func(t *testing.T) { t.Run("Given a request for an non-existing plugin", func(t *testing.T) {
service := &plugins.FakePluginStore{
PluginList: []plugins.PluginDTO{},
}
requestedFile := "nonExistent" requestedFile := "nonExistent"
url := fmt.Sprintf("/public/plugins/%s/%s", pluginID, requestedFile) url := fmt.Sprintf("/public/plugins/%s/%s", pluginID, requestedFile)
pluginAssetScenario(t, "When calling GET on", url, "/public/plugins/:pluginId/*", pluginAssetScenario(t, "When calling GET on", url, "/public/plugins/:pluginId/*",
setting.NewCfg(), service, func(sc *scenarioContext) { setting.NewCfg(), fakes.NewFakePluginRegistry(), func(sc *scenarioContext) {
callGetPluginAsset(sc) callGetPluginAsset(sc)
var respJson map[string]interface{} var respJson map[string]interface{}
@@ -471,11 +477,12 @@ func TestMakePluginResourceRequestContentTypeEmpty(t *testing.T) {
func TestPluginMarkdown(t *testing.T) { func TestPluginMarkdown(t *testing.T) {
t.Run("Plugin not installed returns error", func(t *testing.T) { t.Run("Plugin not installed returns error", func(t *testing.T) {
hs := HTTPServer{ pluginFileStore := &fakes.FakePluginFileStore{
pluginStore: &plugins.FakePluginStore{ FileFunc: func(ctx context.Context, pluginID, filename string) (*plugins.File, error) {
PluginList: []plugins.PluginDTO{}, return nil, plugins.ErrPluginNotInstalled
}, },
} }
hs := HTTPServer{pluginFileStore: pluginFileStore}
pluginID := "test-datasource" pluginID := "test-datasource"
md, err := hs.pluginMarkdown(context.Background(), pluginID, "test") md, err := hs.pluginMarkdown(context.Background(), pluginID, "test")
@@ -485,20 +492,16 @@ func TestPluginMarkdown(t *testing.T) {
t.Run("File fetch will be retried using different casing if error occurs", func(t *testing.T) { t.Run("File fetch will be retried using different casing if error occurs", func(t *testing.T) {
var requestedFiles []string var requestedFiles []string
pluginFiles := &fakes.FakePluginFiles{ pluginFileStore := &fakes.FakePluginFileStore{
OpenFunc: func(name string) (fs.File, error) { FileFunc: func(ctx context.Context, pluginID, filename string) (*plugins.File, error) {
requestedFiles = append(requestedFiles, name) requestedFiles = append(requestedFiles, filename)
return nil, errors.New("some error") return nil, errors.New("some error")
}, },
} }
p := createPluginDTO(plugins.JSONData{ID: "test-app"}, plugins.External, pluginFiles) hs := HTTPServer{pluginFileStore: pluginFileStore}
hs := HTTPServer{ md, err := hs.pluginMarkdown(context.Background(), "", "reAdMe")
pluginStore: &plugins.FakePluginStore{PluginList: []plugins.PluginDTO{p}},
}
md, err := hs.pluginMarkdown(context.Background(), p.ID, "reAdMe")
require.NoError(t, err) require.NoError(t, err)
require.Equal(t, []byte{}, md) require.Equal(t, []byte{}, md)
require.Equal(t, []string{"README.md", "readme.md"}, requestedFiles) require.Equal(t, []string{"README.md", "readme.md"}, requestedFiles)
@@ -524,54 +527,44 @@ func TestPluginMarkdown(t *testing.T) {
} }
for _, tc := range tcs { for _, tc := range tcs {
data := []byte{123}
var requestedFiles []string var requestedFiles []string
pluginFiles := &fakes.FakePluginFiles{ pluginFileStore := &fakes.FakePluginFileStore{
OpenFunc: func(name string) (fs.File, error) { FileFunc: func(ctx context.Context, pluginID, filename string) (*plugins.File, error) {
requestedFiles = append(requestedFiles, name) requestedFiles = append(requestedFiles, filename)
return &FakeFile{data: bytes.NewReader(nil)}, nil return &plugins.File{Content: data}, nil
}, },
} }
p := createPluginDTO(plugins.JSONData{ID: "test-app"}, plugins.External, pluginFiles) hs := HTTPServer{pluginFileStore: pluginFileStore}
hs := HTTPServer{ md, err := hs.pluginMarkdown(context.Background(), "test-datasource", tc.filePath)
pluginStore: &plugins.FakePluginStore{PluginList: []plugins.PluginDTO{p}},
}
md, err := hs.pluginMarkdown(context.Background(), p.ID, tc.filePath)
require.NoError(t, err) require.NoError(t, err)
require.Equal(t, []byte{}, md) require.Equal(t, data, md)
require.Equal(t, tc.expected, requestedFiles) require.Equal(t, tc.expected, requestedFiles)
} }
}) })
t.Run("Non markdown file request returns an error", func(t *testing.T) { t.Run("Non markdown file request returns an error", func(t *testing.T) {
p := createPluginDTO(plugins.JSONData{ID: "test-app"}, plugins.External, nil) hs := HTTPServer{pluginFileStore: &fakes.FakePluginFileStore{}}
hs := HTTPServer{
pluginStore: &plugins.FakePluginStore{PluginList: []plugins.PluginDTO{p}},
}
md, err := hs.pluginMarkdown(context.Background(), p.ID, "test.json") md, err := hs.pluginMarkdown(context.Background(), "", "test.json")
require.ErrorIs(t, err, ErrUnexpectedFileExtension) require.ErrorIs(t, err, ErrUnexpectedFileExtension)
require.Equal(t, []byte{}, md) require.Equal(t, []byte{}, md)
}) })
t.Run("Happy path", func(t *testing.T) { t.Run("Happy path", func(t *testing.T) {
data := []byte{1, 2, 3} data := []byte{1, 2, 3}
fakeFile := &FakeFile{data: bytes.NewReader(data)}
pluginFiles := &fakes.FakePluginFiles{ pluginFileStore := &fakes.FakePluginFileStore{
OpenFunc: func(name string) (fs.File, error) { FileFunc: func(ctx context.Context, pluginID, filename string) (*plugins.File, error) {
return fakeFile, nil return &plugins.File{Content: data}, nil
}, },
} }
p := createPluginDTO(plugins.JSONData{ID: "test-app"}, plugins.External, pluginFiles) hs := HTTPServer{pluginFileStore: pluginFileStore}
hs := HTTPServer{
pluginStore: &plugins.FakePluginStore{PluginList: []plugins.PluginDTO{p}},
}
md, err := hs.pluginMarkdown(context.Background(), p.ID, "someFile") md, err := hs.pluginMarkdown(context.Background(), "", "someFile")
require.NoError(t, err) require.NoError(t, err)
require.Equal(t, data, md) require.Equal(t, data, md)
}) })
@@ -582,13 +575,14 @@ func callGetPluginAsset(sc *scenarioContext) {
} }
func pluginAssetScenario(t *testing.T, desc string, url string, urlPattern string, func pluginAssetScenario(t *testing.T, desc string, url string, urlPattern string,
cfg *setting.Cfg, pluginStore plugins.Store, fn scenarioFunc) { cfg *setting.Cfg, pluginRegistry registry.Service, fn scenarioFunc) {
t.Run(fmt.Sprintf("%s %s", desc, url), func(t *testing.T) { t.Run(fmt.Sprintf("%s %s", desc, url), func(t *testing.T) {
cfg.IsFeatureToggleEnabled = func(_ string) bool { return false } cfg.IsFeatureToggleEnabled = func(_ string) bool { return false }
hs := HTTPServer{ hs := HTTPServer{
Cfg: cfg, Cfg: cfg,
pluginStore: pluginStore, pluginStore: store.New(pluginRegistry),
log: log.NewNopLogger(), pluginFileStore: filestore.ProvideService(pluginRegistry),
log: log.NewNopLogger(),
pluginsCDNService: pluginscdn.ProvideService(&config.Cfg{ pluginsCDNService: pluginscdn.ProvideService(&config.Cfg{
PluginsCDNURLTemplate: cfg.PluginsCDNURLTemplate, PluginsCDNURLTemplate: cfg.PluginsCDNURLTemplate,
PluginSettings: cfg.PluginSettings, PluginSettings: cfg.PluginSettings,
@@ -648,19 +642,24 @@ func (c *fakePluginClient) QueryData(ctx context.Context, req *backend.QueryData
} }
func Test_PluginsList_AccessControl(t *testing.T) { func Test_PluginsList_AccessControl(t *testing.T) {
p1 := createPluginDTO(plugins.JSONData{ p1 := createPlugin(plugins.JSONData{
ID: "test-app", Type: "app", Name: "test-app", ID: "test-app", Type: "app", Name: "test-app",
Info: plugins.Info{ Info: plugins.Info{
Version: "1.0.0", Version: "1.0.0",
}}, plugins.External, plugins.NewLocalFS(map[string]struct{}{}, "")) }}, plugins.External, plugins.NewLocalFS(map[string]struct{}{}, ""))
p2 := createPluginDTO( p2 := createPlugin(
plugins.JSONData{ID: "mysql", Type: "datasource", Name: "MySQL", plugins.JSONData{ID: "mysql", Type: "datasource", Name: "MySQL",
Info: plugins.Info{ Info: plugins.Info{
Author: plugins.InfoLink{Name: "Grafana Labs", URL: "https://grafana.com"}, Author: plugins.InfoLink{Name: "Grafana Labs", URL: "https://grafana.com"},
Description: "Data source for MySQL databases", Description: "Data source for MySQL databases",
}}, plugins.Core, plugins.NewLocalFS(map[string]struct{}{}, "")) }}, plugins.Core, plugins.NewLocalFS(map[string]struct{}{}, ""))
pluginStore := plugins.FakePluginStore{PluginList: []plugins.PluginDTO{p1, p2}} pluginRegistry := &fakes.FakePluginRegistry{
Store: map[string]*plugins.Plugin{
p1.ID: p1,
p2.ID: p2,
},
}
pluginSettings := pluginsettings.FakePluginSettings{Plugins: map[string]*pluginsettings.DTO{ pluginSettings := pluginsettings.FakePluginSettings{Plugins: map[string]*pluginsettings.DTO{
"test-app": {ID: 0, OrgID: 1, PluginID: "test-app", PluginVersion: "1.0.0", Enabled: true}, "test-app": {ID: 0, OrgID: 1, PluginID: "test-app", PluginVersion: "1.0.0", Enabled: true},
@@ -693,9 +692,10 @@ func Test_PluginsList_AccessControl(t *testing.T) {
server := SetupAPITestServer(t, func(hs *HTTPServer) { server := SetupAPITestServer(t, func(hs *HTTPServer) {
hs.Cfg = setting.NewCfg() hs.Cfg = setting.NewCfg()
hs.PluginSettings = &pluginSettings hs.PluginSettings = &pluginSettings
hs.pluginStore = pluginStore hs.pluginStore = store.New(pluginRegistry)
hs.pluginFileStore = filestore.ProvideService(pluginRegistry)
var err error var err error
hs.pluginsUpdateChecker, err = updatechecker.ProvidePluginsService(hs.Cfg, pluginStore, tracing.InitializeTracerForTest()) hs.pluginsUpdateChecker, err = updatechecker.ProvidePluginsService(hs.Cfg, nil, tracing.InitializeTracerForTest())
require.NoError(t, err) require.NoError(t, err)
}) })
@@ -713,30 +713,10 @@ func Test_PluginsList_AccessControl(t *testing.T) {
} }
} }
func createPluginDTO(jd plugins.JSONData, class plugins.Class, files plugins.FS) plugins.PluginDTO { func createPlugin(jd plugins.JSONData, class plugins.Class, files plugins.FS) *plugins.Plugin {
p := &plugins.Plugin{ return &plugins.Plugin{
JSONData: jd, JSONData: jd,
Class: class, Class: class,
FS: files, FS: files,
} }
return p.ToDTO()
}
var _ fs.File = (*FakeFile)(nil)
type FakeFile struct {
data io.Reader
}
func (f *FakeFile) Stat() (fs.FileInfo, error) {
return nil, nil
}
func (f *FakeFile) Read(bytes []byte) (int, error) {
return f.data.Read(bytes)
}
func (f *FakeFile) Close() error {
return nil
} }

View File

@@ -3,6 +3,7 @@ package plugins
import ( import (
"context" "context"
"io/fs" "io/fs"
"time"
"github.com/grafana/grafana-plugin-sdk-go/backend" "github.com/grafana/grafana-plugin-sdk-go/backend"
@@ -30,6 +31,16 @@ type PluginSource interface {
DefaultSignature(ctx context.Context) (Signature, bool) DefaultSignature(ctx context.Context) (Signature, bool)
} }
type FileStore interface {
// File retrieves a plugin file.
File(ctx context.Context, pluginID, filename string) (*File, error)
}
type File struct {
Content []byte
ModTime time.Time
}
type CompatOpts struct { type CompatOpts struct {
GrafanaVersion string GrafanaVersion string
OS string OS string

View File

@@ -3,7 +3,6 @@ package dashboards
import ( import (
"context" "context"
"fmt" "fmt"
"io/fs"
"strings" "strings"
"github.com/grafana/grafana/pkg/plugins" "github.com/grafana/grafana/pkg/plugins"
@@ -13,17 +12,24 @@ import (
var _ FileStore = (*FileStoreManager)(nil) var _ FileStore = (*FileStoreManager)(nil)
type FileStoreManager struct { type FileStoreManager struct {
pluginStore plugins.Store pluginStore plugins.Store
pluginFileStore plugins.FileStore
} }
func ProvideFileStoreManager(pluginStore plugins.Store) *FileStoreManager { func ProvideFileStoreManager(pluginStore plugins.Store, pluginFileStore plugins.FileStore) *FileStoreManager {
return &FileStoreManager{ return &FileStoreManager{
pluginStore: pluginStore, pluginStore: pluginStore,
pluginFileStore: pluginFileStore,
} }
} }
var openDashboardFile = func(p plugins.PluginDTO, name string) (fs.File, error) { var openDashboardFile = func(ctx context.Context, pluginFileStore plugins.FileStore, pluginID, name string) (*plugins.File, error) {
return p.File(name) f, err := pluginFileStore.File(ctx, pluginID, name)
if err != nil {
return &plugins.File{}, err
}
return f, nil
} }
func (m *FileStoreManager) ListPluginDashboardFiles(ctx context.Context, args *ListPluginDashboardFilesArgs) (*ListPluginDashboardFilesResult, error) { func (m *FileStoreManager) ListPluginDashboardFiles(ctx context.Context, args *ListPluginDashboardFilesArgs) (*ListPluginDashboardFilesResult, error) {
@@ -86,12 +92,12 @@ func (m *FileStoreManager) GetPluginDashboardFileContents(ctx context.Context, a
return nil, err return nil, err
} }
file, err := openDashboardFile(plugin, cleanPath) file, err := openDashboardFile(ctx, m.pluginFileStore, plugin.ID, cleanPath)
if err != nil { if err != nil {
return nil, err return nil, err
} }
return &GetPluginDashboardFileContentsResult{ return &GetPluginDashboardFileContentsResult{
Content: file, Content: file.Content,
}, nil }, nil
} }

View File

@@ -3,7 +3,6 @@ package dashboards
import ( import (
"context" "context"
"io" "io"
"io/fs"
"testing" "testing"
"testing/fstest" "testing/fstest"
@@ -131,10 +130,13 @@ func TestDashboardFileStore(t *testing.T) {
Data: []byte("dash2"), Data: []byte("dash2"),
}, },
} }
openDashboardFile = func(p plugins.PluginDTO, name string) (fs.File, error) { openDashboardFile = func(ctx context.Context, pluginFiles plugins.FileStore, pluginID, name string) (*plugins.File, error) {
f, err := mapFs.Open(name) f, err := mapFs.Open(name)
require.NoError(t, err) require.NoError(t, err)
return f, nil
b, err := io.ReadAll(f)
require.NoError(t, err)
return &plugins.File{Content: b}, nil
} }
t.Cleanup(func() { t.Cleanup(func() {
openDashboardFile = origOpenDashboardFile openDashboardFile = origOpenDashboardFile
@@ -157,10 +159,7 @@ func TestDashboardFileStore(t *testing.T) {
}) })
require.NoError(t, err) require.NoError(t, err)
require.NotNil(t, res) require.NotNil(t, res)
require.NotNil(t, res.Content) require.Equal(t, "dash1", string(res.Content))
b, err := io.ReadAll(res.Content)
require.NoError(t, err)
require.Equal(t, "dash1", string(b))
}) })
t.Run("Should return file content for dashboards/dash2.json", func(t *testing.T) { t.Run("Should return file content for dashboards/dash2.json", func(t *testing.T) {
@@ -170,10 +169,7 @@ func TestDashboardFileStore(t *testing.T) {
}) })
require.NoError(t, err) require.NoError(t, err)
require.NotNil(t, res) require.NotNil(t, res)
require.NotNil(t, res.Content) require.Equal(t, "dash2", string(res.Content))
b, err := io.ReadAll(res.Content)
require.NoError(t, err)
require.Equal(t, "dash2", string(b))
}) })
t.Run("Should return error when trying to read relative file", func(t *testing.T) { t.Run("Should return error when trying to read relative file", func(t *testing.T) {

View File

@@ -2,7 +2,6 @@ package dashboards
import ( import (
"context" "context"
"io"
) )
// FileStore is the interface for plugin dashboard file storage. // FileStore is the interface for plugin dashboard file storage.
@@ -31,5 +30,5 @@ type GetPluginDashboardFileContentsArgs struct {
// GetPluginDashboardFileContentsResult get plugin dashboard file content result model. // GetPluginDashboardFileContentsResult get plugin dashboard file content result model.
type GetPluginDashboardFileContentsResult struct { type GetPluginDashboardFileContentsResult struct {
Content io.ReadCloser Content []byte
} }

View File

@@ -421,3 +421,14 @@ func (s *FakePluginSource) DefaultSignature(ctx context.Context) (plugins.Signat
} }
return plugins.Signature{}, false return plugins.Signature{}, false
} }
type FakePluginFileStore struct {
FileFunc func(ctx context.Context, pluginID, filename string) (*plugins.File, error)
}
func (f *FakePluginFileStore) File(ctx context.Context, pluginID, filename string) (*plugins.File, error) {
if f.FileFunc != nil {
return f.FileFunc(ctx, pluginID, filename)
}
return nil, nil
}

View File

@@ -0,0 +1,54 @@
package filestore
import (
"context"
"io"
"github.com/grafana/grafana/pkg/plugins"
"github.com/grafana/grafana/pkg/plugins/log"
"github.com/grafana/grafana/pkg/plugins/manager/registry"
)
type Service struct {
pluginRegistry registry.Service
log log.Logger
}
func ProvideService(pluginRegistry registry.Service) *Service {
return &Service{
pluginRegistry: pluginRegistry,
log: log.New("plugin.fs"),
}
}
func (s *Service) File(ctx context.Context, pluginID, filename string) (*plugins.File, error) {
if p, exists := s.pluginRegistry.Plugin(ctx, pluginID); exists {
f, err := p.File(filename)
if err != nil {
return nil, err
}
defer func() {
err = f.Close()
if err != nil {
s.log.Error("Could not close plugin file", "pluginID", p.ID, "file", filename)
}
}()
b, err := io.ReadAll(f)
if err != nil {
return nil, err
}
fi, err := f.Stat()
if err != nil {
return nil, err
}
return &plugins.File{
Content: b,
ModTime: fi.ModTime(),
}, nil
} else {
return nil, plugins.ErrPluginNotInstalled
}
}

View File

@@ -95,25 +95,6 @@ func (p PluginDTO) IsCorePlugin() bool {
return p.Class == Core return p.Class == Core
} }
func (p PluginDTO) File(name string) (fs.File, error) {
cleanPath, err := util.CleanRelativePath(name)
if err != nil {
// CleanRelativePath should clean and make the path relative so this is not expected to fail
return nil, err
}
if p.fs == nil {
return nil, ErrFileNotExist
}
f, err := p.fs.Open(cleanPath)
if err != nil {
return nil, err
}
return f, nil
}
// JSONData represents the plugin's plugin.json // JSONData represents the plugin's plugin.json
type JSONData struct { type JSONData struct {
// Common settings // Common settings
@@ -325,6 +306,25 @@ func (p *Plugin) RunStream(ctx context.Context, req *backend.RunStreamRequest, s
return pluginClient.RunStream(ctx, req, sender) return pluginClient.RunStream(ctx, req, sender)
} }
func (p *Plugin) File(name string) (fs.File, error) {
cleanPath, err := util.CleanRelativePath(name)
if err != nil {
// CleanRelativePath should clean and make the path relative so this is not expected to fail
return nil, err
}
if p.FS == nil {
return nil, ErrFileNotExist
}
f, err := p.FS.Open(cleanPath)
if err != nil {
return nil, err
}
return f, nil
}
func (p *Plugin) RegisterClient(c backendplugin.Plugin) { func (p *Plugin) RegisterClient(c backendplugin.Plugin) {
p.client = c p.client = c
} }

View File

@@ -5,6 +5,7 @@ import (
"fmt" "fmt"
"github.com/grafana/grafana/pkg/components/simplejson" "github.com/grafana/grafana/pkg/components/simplejson"
"github.com/grafana/grafana/pkg/infra/log" "github.com/grafana/grafana/pkg/infra/log"
pluginDashboardsManager "github.com/grafana/grafana/pkg/plugins/manager/dashboards" pluginDashboardsManager "github.com/grafana/grafana/pkg/plugins/manager/dashboards"
"github.com/grafana/grafana/pkg/services/dashboards" "github.com/grafana/grafana/pkg/services/dashboards"
@@ -115,13 +116,7 @@ func (s Service) LoadPluginDashboard(ctx context.Context, req *plugindashboards.
return nil, err return nil, err
} }
defer func() { data, err := simplejson.NewJson(resp.Content)
if err = resp.Content.Close(); err != nil {
s.logger.Warn("Failed to close plugin dashboard file", "reference", req.Reference, "err", err)
}
}()
data, err := simplejson.NewFromReader(resp.Content)
if err != nil { if err != nil {
return nil, err return nil, err
} }

View File

@@ -1,10 +1,8 @@
package service package service
import ( import (
"bytes"
"context" "context"
"fmt" "fmt"
"io"
"sort" "sort"
"testing" "testing"
@@ -193,7 +191,7 @@ func (m pluginDashboardStoreMock) GetPluginDashboardFileContents(ctx context.Con
if dashboardFiles, exists := m.pluginDashboardFiles[args.PluginID]; exists { if dashboardFiles, exists := m.pluginDashboardFiles[args.PluginID]; exists {
if content, exists := dashboardFiles[args.FileReference]; exists { if content, exists := dashboardFiles[args.FileReference]; exists {
return &dashboards.GetPluginDashboardFileContentsResult{ return &dashboards.GetPluginDashboardFileContentsResult{
Content: io.NopCloser(bytes.NewReader(content)), Content: content,
}, nil }, nil
} }
} else if !exists { } else if !exists {

View File

@@ -10,6 +10,7 @@ import (
"github.com/grafana/grafana/pkg/plugins/config" "github.com/grafana/grafana/pkg/plugins/config"
"github.com/grafana/grafana/pkg/plugins/manager" "github.com/grafana/grafana/pkg/plugins/manager"
"github.com/grafana/grafana/pkg/plugins/manager/client" "github.com/grafana/grafana/pkg/plugins/manager/client"
"github.com/grafana/grafana/pkg/plugins/manager/filestore"
"github.com/grafana/grafana/pkg/plugins/manager/loader" "github.com/grafana/grafana/pkg/plugins/manager/loader"
"github.com/grafana/grafana/pkg/plugins/manager/loader/assetpath" "github.com/grafana/grafana/pkg/plugins/manager/loader/assetpath"
"github.com/grafana/grafana/pkg/plugins/manager/loader/finder" "github.com/grafana/grafana/pkg/plugins/manager/loader/finder"
@@ -60,6 +61,8 @@ var WireSet = wire.NewSet(
sources.ProvideService, sources.ProvideService,
pluginSettings.ProvideService, pluginSettings.ProvideService,
wire.Bind(new(pluginsettings.Service), new(*pluginSettings.Service)), wire.Bind(new(pluginsettings.Service), new(*pluginSettings.Service)),
filestore.ProvideService,
wire.Bind(new(plugins.FileStore), new(*filestore.Service)),
) )
// WireExtensionSet provides a wire.ProviderSet of plugin providers that can be // WireExtensionSet provides a wire.ProviderSet of plugin providers that can be