Plugins: Markdown fetch retry with lowercase (#65384)

* retry with lowercase

* undo incorrect err check

* re-add defer to close file

* fix test

pkg/api/plugins.go

@@ -43,6 +43,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")
@@ -539,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}
+	}
+
+	file, err := mdFilepath(strings.ToUpper(name))
+	if err != nil {
+		return make([]byte, 0), err
 	}
 
-	md, err := plugin.File(mdFilepath(strings.ToUpper(name)))
+	md, err := plugin.File(file)
 	if err != nil {
-		md, err = plugin.File(mdFilepath(strings.ToUpper(name)))
+		md, err = plugin.File(strings.ToLower(file))
 		if err != nil {
 			return make([]byte, 0), nil
 		}
@@ -554,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
@@ -562,6 +568,14 @@ 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)))
+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
+	}
 }

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"
@@ -22,6 +25,7 @@ import (
 	"github.com/grafana/grafana/pkg/infra/log"
 	"github.com/grafana/grafana/pkg/plugins"
 	"github.com/grafana/grafana/pkg/plugins/config"
+	"github.com/grafana/grafana/pkg/plugins/manager/fakes"
 	"github.com/grafana/grafana/pkg/plugins/pluginscdn"
 	ac "github.com/grafana/grafana/pkg/services/accesscontrol"
 	contextmodel "github.com/grafana/grafana/pkg/services/contexthandler/model"
@@ -464,6 +468,114 @@ 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
+		pluginFiles := &fakes.FakePluginFiles{
+			OpenFunc: func(name string) (fs.File, error) {
+				requestedFiles = append(requestedFiles, name)
+				return nil, errors.New("some error")
+			},
+		}
+
+		p := createPluginDTO(plugins.JSONData{ID: "test-app"}, plugins.External, pluginFiles)
+
+		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
+			pluginFiles := &fakes.FakePluginFiles{
+				OpenFunc: func(name string) (fs.File, error) {
+					requestedFiles = append(requestedFiles, name)
+					return &FakeFile{data: bytes.NewReader(nil)}, nil
+				},
+			}
+
+			p := createPluginDTO(plugins.JSONData{ID: "test-app"}, plugins.External, pluginFiles)
+
+			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, nil)
+		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)}
+
+		pluginFiles := &fakes.FakePluginFiles{
+			OpenFunc: func(name string) (fs.File, error) {
+				return fakeFile, nil
+			},
+		}
+
+		p := createPluginDTO(plugins.JSONData{ID: "test-app"}, plugins.External, pluginFiles)
+		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()
 }
@@ -607,3 +719,21 @@ func createPluginDTO(jd plugins.JSONData, class plugins.Class, files plugins.FS)
 
 	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
+}

pkg/plugins/manager/fakes/fakes.go

@@ -358,7 +358,7 @@ func (f *FakeRoleRegistry) DeclarePluginRoles(_ context.Context, _ string, _ str
 }
 
 type FakePluginFiles struct {
-	FS fs.FS
+	OpenFunc func(name string) (fs.File, error)
 
 	base string
 }
@@ -370,7 +370,10 @@ func NewFakePluginFiles(base string) *FakePluginFiles {
 }
 
 func (f *FakePluginFiles) Open(name string) (fs.File, error) {
-	return f.FS.Open(name)
+	if f.OpenFunc != nil {
+		return f.OpenFunc(name)
+	}
+	return nil, nil
 }
 
 func (f *FakePluginFiles) Base() string {