From 0d7efc076c5f5eeb13716aef737c5c6b67392e32 Mon Sep 17 00:00:00 2001 From: Will Browne Date: Tue, 28 Mar 2023 11:19:20 +0100 Subject: [PATCH] [v9.4.x] Plugins: Markdown fetch retry with lowercase (#65388) cherry pick --- pkg/api/plugins.go | 31 +++++++-- pkg/api/plugins_test.go | 137 ++++++++++++++++++++++++++++++++++++++++ 2 files changed, 162 insertions(+), 6 deletions(-) diff --git a/pkg/api/plugins.go b/pkg/api/plugins.go index 2658391d2cc..92ae6fcd830 100644 --- a/pkg/api/plugins.go +++ b/pkg/api/plugins.go @@ -7,6 +7,7 @@ import ( "errors" "fmt" "io" + "io/fs" "net/http" "path" "path/filepath" @@ -43,6 +44,8 @@ var pluginsCDNFallbackRedirectRequests = promauto.NewCounterVec(prometheus.Count Help: "Number of requests to the plugins CDN backend redirect fallback handler.", }, []string{"plugin_id", "plugin_version"}) +var ErrUnexpectedFileExtension = errors.New("unexpected file extension") + func (hs *HTTPServer) GetPluginList(c *contextmodel.ReqContext) response.Response { typeFilter := c.Query("type") enabledFilter := c.Query("enabled") @@ -538,12 +541,17 @@ func translatePluginRequestErrorToAPIError(err error) response.Response { func (hs *HTTPServer) pluginMarkdown(ctx context.Context, pluginId string, name string) ([]byte, error) { plugin, exists := hs.pluginStore.Plugin(ctx, pluginId) if !exists { - return nil, plugins.NotFoundError{PluginID: pluginId} + return make([]byte, 0), plugins.NotFoundError{PluginID: pluginId} } - md, err := plugin.File(mdFilepath(strings.ToUpper(name))) + file, err := mdFilepath(strings.ToUpper(name)) if err != nil { - md, err = plugin.File(mdFilepath(strings.ToUpper(name))) + return make([]byte, 0), err + } + + md, err := readPluginFile(plugin, file) + if err != nil { + md, err = readPluginFile(plugin, strings.ToLower(file)) if err != nil { return make([]byte, 0), nil } @@ -553,7 +561,6 @@ func (hs *HTTPServer) pluginMarkdown(ctx context.Context, pluginId string, name hs.log.Error("Failed to close plugin markdown file", "err", err) } }() - d, err := io.ReadAll(md) if err != nil { return make([]byte, 0), nil @@ -561,6 +568,18 @@ func (hs *HTTPServer) pluginMarkdown(ctx context.Context, pluginId string, name return d, nil } -func mdFilepath(mdFilename string) string { - return filepath.Clean(filepath.Join("/", fmt.Sprintf("%s.md", mdFilename))) +var readPluginFile = func(p plugins.PluginDTO, name string) (fs.File, error) { + return p.File(name) +} + +func mdFilepath(mdFilename string) (string, error) { + fileExt := filepath.Ext(mdFilename) + switch fileExt { + case "md": + return util.CleanRelativePath(mdFilename) + case "": + return util.CleanRelativePath(fmt.Sprintf("%s.md", mdFilename)) + default: + return "", ErrUnexpectedFileExtension + } } diff --git a/pkg/api/plugins_test.go b/pkg/api/plugins_test.go index d1ca104a912..fadb180d978 100644 --- a/pkg/api/plugins_test.go +++ b/pkg/api/plugins_test.go @@ -1,10 +1,13 @@ package api import ( + "bytes" "context" "encoding/json" + "errors" "fmt" "io" + "io/fs" "net/http" "net/http/httptest" "os" @@ -474,6 +477,122 @@ func TestMakePluginResourceRequestContentTypeEmpty(t *testing.T) { require.Zero(t, resp.Header().Get("Content-Type")) } +func TestPluginMarkdown(t *testing.T) { + t.Run("Plugin not installed returns error", func(t *testing.T) { + hs := HTTPServer{ + pluginStore: &plugins.FakePluginStore{ + PluginList: []plugins.PluginDTO{}, + }, + } + + pluginID := "test-datasource" + md, err := hs.pluginMarkdown(context.Background(), pluginID, "test") + require.ErrorAs(t, err, &plugins.NotFoundError{PluginID: pluginID}) + require.Equal(t, []byte{}, md) + }) + + t.Run("File fetch will be retried using different casing if error occurs", func(t *testing.T) { + var requestedFiles []string + + origReadPluginFile := readPluginFile + readPluginFile = func(p plugins.PluginDTO, name string) (fs.File, error) { + requestedFiles = append(requestedFiles, name) + return nil, errors.New("some error") + } + t.Cleanup(func() { + readPluginFile = origReadPluginFile + }) + + p := createPluginDTO(plugins.JSONData{ID: "test-app"}, plugins.External, "") + + hs := HTTPServer{ + pluginStore: &plugins.FakePluginStore{PluginList: []plugins.PluginDTO{p}}, + } + + md, err := hs.pluginMarkdown(context.Background(), p.ID, "reAdMe") + require.NoError(t, err) + require.Equal(t, []byte{}, md) + require.Equal(t, []string{"README.md", "readme.md"}, requestedFiles) + }) + + t.Run("File fetch receive cleaned file paths", func(t *testing.T) { + tcs := []struct { + filePath string + expected []string + }{ + { + filePath: "../../docs", + expected: []string{"DOCS.md"}, + }, + { + filePath: "/../../docs/../docs", + expected: []string{"DOCS.md"}, + }, + { + filePath: "readme.md/../../secrets", + expected: []string{"SECRETS.md"}, + }, + } + + for _, tc := range tcs { + var requestedFiles []string + + origReadPluginFile := readPluginFile + readPluginFile = func(p plugins.PluginDTO, name string) (fs.File, error) { + requestedFiles = append(requestedFiles, name) + return &FakeFile{data: bytes.NewReader(nil)}, nil + } + t.Cleanup(func() { + readPluginFile = origReadPluginFile + }) + + p := createPluginDTO(plugins.JSONData{ID: "test-app"}, plugins.External, "") + + hs := HTTPServer{ + pluginStore: &plugins.FakePluginStore{PluginList: []plugins.PluginDTO{p}}, + } + + md, err := hs.pluginMarkdown(context.Background(), p.ID, tc.filePath) + require.NoError(t, err) + require.Equal(t, []byte{}, md) + require.Equal(t, tc.expected, requestedFiles) + } + }) + + t.Run("Non markdown file request returns an error", func(t *testing.T) { + p := createPluginDTO(plugins.JSONData{ID: "test-app"}, plugins.External, "") + hs := HTTPServer{ + pluginStore: &plugins.FakePluginStore{PluginList: []plugins.PluginDTO{p}}, + } + + md, err := hs.pluginMarkdown(context.Background(), p.ID, "test.json") + require.ErrorIs(t, err, ErrUnexpectedFileExtension) + require.Equal(t, []byte{}, md) + }) + + t.Run("Happy path", func(t *testing.T) { + data := []byte{1, 2, 3} + fakeFile := &FakeFile{data: bytes.NewReader(data)} + + origReadPluginFile := readPluginFile + readPluginFile = func(p plugins.PluginDTO, name string) (fs.File, error) { + return fakeFile, nil + } + t.Cleanup(func() { + readPluginFile = origReadPluginFile + }) + + p := createPluginDTO(plugins.JSONData{ID: "test-app"}, plugins.External, "") + hs := HTTPServer{ + pluginStore: &plugins.FakePluginStore{PluginList: []plugins.PluginDTO{p}}, + } + + md, err := hs.pluginMarkdown(context.Background(), p.ID, "someFile") + require.NoError(t, err) + require.Equal(t, data, md) + }) +} + func callGetPluginAsset(sc *scenarioContext) { sc.fakeReqWithParams("GET", sc.url, map[string]string{}).exec() } @@ -648,3 +767,21 @@ func createPluginDTO(jd plugins.JSONData, class plugins.Class, pluginDir string) } 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 +}