diff --git a/pkg/plugins/manager/loader/finder/local_test.go b/pkg/plugins/manager/loader/finder/local_test.go index 8b2be26565e..0e3c5a8bcfb 100644 --- a/pkg/plugins/manager/loader/finder/local_test.go +++ b/pkg/plugins/manager/loader/finder/local_test.go @@ -336,6 +336,45 @@ func TestFinder_getAbsPluginJSONPaths(t *testing.T) { }) } +func TestFinder_getAbsPluginJSONPaths_PluginClass(t *testing.T) { + t.Run("When a dist folder exists as a direct child of the plugins path, it will always be resolved", func(t *testing.T) { + dir, err := filepath.Abs("../../testdata/pluginRootWithDist") + require.NoError(t, err) + + tcs := []struct { + name string + followDist bool + expected []string + }{ + { + name: "When followDistFolder is enabled, a nested dist folder will also be resolved", + followDist: true, + expected: []string{ + filepath.Join(dir, "datasource/plugin.json"), + filepath.Join(dir, "dist/plugin.json"), + filepath.Join(dir, "panel/dist/plugin.json"), + }, + }, + { + name: "When followDistFolder is disabled, a nested dist folder will not be resolved", + followDist: false, + expected: []string{ + filepath.Join(dir, "datasource/plugin.json"), + filepath.Join(dir, "dist/plugin.json"), + filepath.Join(dir, "panel/src/plugin.json"), + }, + }, + } + for _, tc := range tcs { + pluginBundles, err := NewLocalFinder(false).getAbsPluginJSONPaths(dir, tc.followDist) + require.NoError(t, err) + + sort.Strings(pluginBundles) + require.Equal(t, tc.expected, pluginBundles) + } + }) +} + var fsComparer = cmp.Comparer(func(fs1 plugins.FS, fs2 plugins.FS) bool { fs1Files, err := fs1.Files() if err != nil { diff --git a/pkg/plugins/manager/testdata/pluginRootWithDist/datasource/plugin.json b/pkg/plugins/manager/testdata/pluginRootWithDist/datasource/plugin.json new file mode 100644 index 00000000000..e69de29bb2d diff --git a/pkg/plugins/manager/testdata/pluginRootWithDist/dist/plugin.json b/pkg/plugins/manager/testdata/pluginRootWithDist/dist/plugin.json new file mode 100644 index 00000000000..e69de29bb2d diff --git a/pkg/plugins/manager/testdata/pluginRootWithDist/panel/dist/plugin.json b/pkg/plugins/manager/testdata/pluginRootWithDist/panel/dist/plugin.json new file mode 100644 index 00000000000..e69de29bb2d diff --git a/pkg/plugins/manager/testdata/pluginRootWithDist/panel/src/plugin.json b/pkg/plugins/manager/testdata/pluginRootWithDist/panel/src/plugin.json new file mode 100644 index 00000000000..e69de29bb2d diff --git a/pkg/plugins/pfs/pfs_test.go b/pkg/plugins/pfs/pfs_test.go index 92d25d74b3b..377d5a7a125 100644 --- a/pkg/plugins/pfs/pfs_test.go +++ b/pkg/plugins/pfs/pfs_test.go @@ -121,6 +121,10 @@ func TestParsePluginTestdata(t *testing.T) { err: ErrInvalidLineage, skip: "TODO implement BindOption in thema, SatisfiesJoinSchema, then use it here", }, + "pluginRootWithDist": { + err: ErrNoRootFile, + skip: "This folder is used to test multiple plugins in the same folder", + }, "name-mismatch-panel": { err: ErrInvalidGrafanaPluginInstance, }, diff --git a/pkg/util/filepath.go b/pkg/util/filepath.go index 527457aadbb..5d642f7b13c 100644 --- a/pkg/util/filepath.go +++ b/pkg/util/filepath.go @@ -14,6 +14,15 @@ var ErrWalkSkipDir = errors.New("skip this directory") // If resolvedPath != "", then we are following symbolic links. type WalkFunc func(resolvedPath string, info os.FileInfo, err error) error +type walker struct { + rootDir string +} + +// newWalker creates a new walker +func newWalker(rootDir string) *walker { + return &walker{rootDir: rootDir} +} + // Walk walks a path, optionally following symbolic links, and for each path, // it calls the walkFn passed. // @@ -34,7 +43,8 @@ func Walk(path string, followSymlinks bool, detectSymlinkInfiniteLoop bool, foll symlinkPathsFollowed = make(map[string]bool, 8) } } - return walk(path, info, resolvedPath, symlinkPathsFollowed, followDistFolder, walkFn) + + return newWalker(path).walk(path, info, resolvedPath, symlinkPathsFollowed, followDistFolder, walkFn) } // walk walks the path. It is a helper/sibling function to Walk. @@ -43,7 +53,7 @@ func Walk(path string, followSymlinks bool, detectSymlinkInfiniteLoop bool, foll // // If resolvedPath is "", then we are not following symbolic links. // If symlinkPathsFollowed is not nil, then we need to detect infinite loop. -func walk(path string, info os.FileInfo, resolvedPath string, symlinkPathsFollowed map[string]bool, followDistFolder bool, walkFn WalkFunc) error { +func (w *walker) walk(path string, info os.FileInfo, resolvedPath string, symlinkPathsFollowed map[string]bool, followDistFolder bool, walkFn WalkFunc) error { if info == nil { return errors.New("walk: Nil FileInfo passed") } @@ -81,7 +91,7 @@ func walk(path string, info os.FileInfo, resolvedPath string, symlinkPathsFollow if err != nil { return err } - return walk(path, info2, path2, symlinkPathsFollowed, followDistFolder, walkFn) + return w.walk(path, info2, path2, symlinkPathsFollowed, followDistFolder, walkFn) } else if info.IsDir() { list, err := os.ReadDir(path) if err != nil { @@ -102,50 +112,52 @@ func walk(path string, info os.FileInfo, resolvedPath string, symlinkPathsFollow subFiles = append(subFiles, subFile{path: path2, resolvedPath: resolvedPath2, fileInfo: fileInfo}) } - if containsDistFolder(subFiles) && followDistFolder { - err := walk( - filepath.Join(path, "dist"), - info, - filepath.Join(resolvedPath, "dist"), - symlinkPathsFollowed, - followDistFolder, - walkFn) - - if err != nil { - return err - } + // If we have found a dist directory in a subdirectory (IE not at root path), and followDistFolder is true, + // then we want to follow only the dist directory and ignore all other subdirectories. + atRootDir := w.rootDir == path + if followDistFolder && w.containsDistFolder(subFiles) && !atRootDir { + return w.walk(filepath.Join(path, "dist"), info, filepath.Join(resolvedPath, "dist"), symlinkPathsFollowed, + followDistFolder, walkFn) } else { + // Follow all subdirectories, with special handling for dist directories. for _, p := range subFiles { - err = walk(p.path, p.fileInfo, p.resolvedPath, symlinkPathsFollowed, followDistFolder, walkFn) + // We only want to skip a dist directory if it is not in the root directory, and followDistFolder is false. + if p.isDistDir() && !atRootDir && !followDistFolder { + continue + } + err = w.walk(p.path, p.fileInfo, p.resolvedPath, symlinkPathsFollowed, followDistFolder, walkFn) if err != nil { return err } } } - return nil } return nil } +// containsDistFolder returns true if the provided subFiles is a folder named "dist". +func (w *walker) containsDistFolder(subFiles []subFile) bool { + for _, p := range subFiles { + if p.fileInfo.IsDir() && p.fileInfo.Name() == "dist" { + return true + } + } + return false +} + type subFile struct { path, resolvedPath string fileInfo os.FileInfo } -func containsDistFolder(subFiles []subFile) bool { - for _, p := range subFiles { - if p.fileInfo.IsDir() && p.fileInfo.Name() == "dist" { - return true - } - } - - return false +func (s subFile) isDistDir() bool { + return s.fileInfo.IsDir() && s.fileInfo.Name() == "dist" } // CleanRelativePath returns the shortest path name equivalent to path -// by purely lexical processing. It make sure the provided path is rooted +// by purely lexical processing. It makes sure the provided path is rooted // and then uses filepath.Clean and filepath.Rel to make sure the path // doesn't include any separators or elements that shouldn't be there // like ., .., //.