From ef60c90dfa74bc4a666348f78285784e9aac7ce1 Mon Sep 17 00:00:00 2001 From: Will Browne Date: Tue, 19 Dec 2023 11:01:48 +0100 Subject: [PATCH] Plugins: Make file system path handling in tests OS agnostic (#79651) * make path handling OS agnostic * PR feedback * fix input for test case --- .../http_logger_middleware_test.go | 2 ++ pkg/plugins/manager/signature/manifest_test.go | 2 +- pkg/plugins/manager/sources/sources_test.go | 3 ++- pkg/plugins/pfs/pfs_test.go | 4 ++-- pkg/plugins/storage/fs_test.go | 8 ++++---- pkg/util/filepath_test.go | 14 ++++++++------ 6 files changed, 19 insertions(+), 14 deletions(-) diff --git a/pkg/infra/httpclient/httpclientprovider/http_logger_middleware_test.go b/pkg/infra/httpclient/httpclientprovider/http_logger_middleware_test.go index 5f4362cd23e..006c87411e9 100644 --- a/pkg/infra/httpclient/httpclientprovider/http_logger_middleware_test.go +++ b/pkg/infra/httpclient/httpclientprovider/http_logger_middleware_test.go @@ -47,6 +47,8 @@ func TestHTTPLoggerMiddleware(t *testing.T) { f, err := os.CreateTemp("", "example_*.har") require.NoError(t, err) defer func() { + err = f.Close() + require.NoError(t, err) err := os.Remove(f.Name()) require.NoError(t, err) }() diff --git a/pkg/plugins/manager/signature/manifest_test.go b/pkg/plugins/manager/signature/manifest_test.go index 4e74e70b280..6f950e54dde 100644 --- a/pkg/plugins/manager/signature/manifest_test.go +++ b/pkg/plugins/manager/signature/manifest_test.go @@ -365,7 +365,7 @@ func TestFSPathSeparatorFiles(t *testing.T) { } { t.Run(tc.name, func(t *testing.T) { pfs, err := newPathSeparatorOverrideFS( - "/", plugins.NewInMemoryFS( + tc.sep, plugins.NewInMemoryFS( map[string][]byte{"a": nil, strings.Join([]string{"a", "b", "c"}, tc.sep): nil}, ), ) diff --git a/pkg/plugins/manager/sources/sources_test.go b/pkg/plugins/manager/sources/sources_test.go index 6afb5806304..9bffb3e7245 100644 --- a/pkg/plugins/manager/sources/sources_test.go +++ b/pkg/plugins/manager/sources/sources_test.go @@ -2,6 +2,7 @@ package sources import ( "context" + "path/filepath" "testing" "github.com/stretchr/testify/require" @@ -36,7 +37,7 @@ func TestSources_List(t *testing.T) { require.Len(t, srcs, 3) require.Equal(t, srcs[0].PluginClass(ctx), plugins.ClassCore) - require.Equal(t, srcs[0].PluginURIs(ctx), []string{"app/plugins/datasource", "app/plugins/panel"}) + require.Equal(t, srcs[0].PluginURIs(ctx), []string{filepath.Join("app", "plugins", "datasource"), filepath.Join("app", "plugins", "panel")}) sig, exists := srcs[0].DefaultSignature(ctx) require.True(t, exists) require.Equal(t, plugins.SignatureStatusInternal, sig.Status) diff --git a/pkg/plugins/pfs/pfs_test.go b/pkg/plugins/pfs/pfs_test.go index ebff347be95..e98fd66b83e 100644 --- a/pkg/plugins/pfs/pfs_test.go +++ b/pkg/plugins/pfs/pfs_test.go @@ -147,7 +147,7 @@ func TestParsePluginTestdata(t *testing.T) { }, } - staticRootPath, err := filepath.Abs("../manager/testdata") + staticRootPath, err := filepath.Abs(filepath.Join("..", "manager", "testdata")) require.NoError(t, err) dfs := os.DirFS(staticRootPath) ents, err := fs.ReadDir(dfs, ".") @@ -240,7 +240,7 @@ func TestParseTreeZips(t *testing.T) { }, } - staticRootPath, err := filepath.Abs("../storage/testdata") + staticRootPath, err := filepath.Abs(filepath.Join("..", "storage", "testdata")) require.NoError(t, err) ents, err := os.ReadDir(staticRootPath) require.NoError(t, err) diff --git a/pkg/plugins/storage/fs_test.go b/pkg/plugins/storage/fs_test.go index fa504248dd1..9086a9111de 100644 --- a/pkg/plugins/storage/fs_test.go +++ b/pkg/plugins/storage/fs_test.go @@ -119,7 +119,7 @@ func TestExtractFiles(t *testing.T) { skipWindows(t) pluginID := "plugin-with-absolute-symlink" - path, err := i.extractFiles(context.Background(), zipFile(t, "testdata/plugin-with-absolute-symlink.zip"), pluginID, SimpleDirNameGeneratorFunc) + path, err := i.extractFiles(context.Background(), zipFile(t, filepath.Join("testdata", "plugin-with-absolute-symlink.zip")), pluginID, SimpleDirNameGeneratorFunc) require.Equal(t, filepath.Join(pluginsDir, pluginID), path) require.NoError(t, err) @@ -131,7 +131,7 @@ func TestExtractFiles(t *testing.T) { skipWindows(t) pluginID := "plugin-with-absolute-symlink-dir" - path, err := i.extractFiles(context.Background(), zipFile(t, "testdata/plugin-with-absolute-symlink-dir.zip"), pluginID, SimpleDirNameGeneratorFunc) + path, err := i.extractFiles(context.Background(), zipFile(t, filepath.Join("testdata", "plugin-with-absolute-symlink-dir.zip")), pluginID, SimpleDirNameGeneratorFunc) require.Equal(t, filepath.Join(pluginsDir, pluginID), path) require.NoError(t, err) @@ -140,7 +140,7 @@ func TestExtractFiles(t *testing.T) { }) t.Run("Should detect if archive members point outside of the destination directory", func(t *testing.T) { - path, err := i.extractFiles(context.Background(), zipFile(t, "testdata/plugin-with-parent-member.zip"), "plugin-with-parent-member", SimpleDirNameGeneratorFunc) + path, err := i.extractFiles(context.Background(), zipFile(t, filepath.Join("testdata", "plugin-with-parent-member.zip")), "plugin-with-parent-member", SimpleDirNameGeneratorFunc) require.Empty(t, path) require.EqualError(t, err, fmt.Sprintf( `archive member "../member.txt" tries to write outside of plugin directory: %q, this can be a security risk`, @@ -149,7 +149,7 @@ func TestExtractFiles(t *testing.T) { }) t.Run("Should detect if archive members are absolute", func(t *testing.T) { - path, err := i.extractFiles(context.Background(), zipFile(t, "testdata/plugin-with-absolute-member.zip"), "plugin-with-absolute-member", SimpleDirNameGeneratorFunc) + path, err := i.extractFiles(context.Background(), zipFile(t, filepath.Join("testdata", "plugin-with-absolute-member.zip")), "plugin-with-absolute-member", SimpleDirNameGeneratorFunc) require.Empty(t, path) require.EqualError(t, err, fmt.Sprintf( `archive member "/member.txt" tries to write outside of plugin directory: %q, this can be a security risk`, diff --git a/pkg/util/filepath_test.go b/pkg/util/filepath_test.go index 38116c5dda9..dcaf10a53f9 100644 --- a/pkg/util/filepath_test.go +++ b/pkg/util/filepath_test.go @@ -1,6 +1,7 @@ package util import ( + "path/filepath" "testing" "github.com/stretchr/testify/assert" @@ -16,16 +17,17 @@ func TestCleanRelativePath(t *testing.T) { expectedPath: ".", }, { - input: "/test/test.txt", - expectedPath: "test/test.txt", + input: filepath.Join(string(filepath.Separator), "test", "test.txt"), + expectedPath: filepath.Join("test", "test.txt"), }, { - input: "../../test/test.txt", - expectedPath: "test/test.txt", + input: filepath.Join("..", "..", "test", "test.txt"), + expectedPath: filepath.Join("test", "test.txt"), }, { - input: "./../test/test.txt", - expectedPath: "test/test.txt", + // since filepath.Join will remove the leading dot, we need to build the path manually + input: "." + string(filepath.Separator) + filepath.Join("..", "test", "test.txt"), + expectedPath: filepath.Join("test", "test.txt"), }, }