From cd9bbfaead63cad5b355fa04c3d3be3e4b3f2b7b Mon Sep 17 00:00:00 2001 From: Imran Ismail Date: Mon, 12 Oct 2020 20:57:08 +0800 Subject: [PATCH 1/5] Allow abs path for tar --- archive/tar/tar.go | 11 ++++- archive/tar/tar_test.go | 98 ++++++++++++++++++++++++++++++++++++----- 2 files changed, 97 insertions(+), 12 deletions(-) diff --git a/archive/tar/tar.go b/archive/tar/tar.go index 3272b1e2..b18fb42c 100644 --- a/archive/tar/tar.go +++ b/archive/tar/tar.go @@ -85,7 +85,14 @@ func writeToArchive(tw *tar.Writer, root string, skipSymlinks bool, written *int } } - name, err := relative(root, path) + var name string + + if strings.HasPrefix(path, "/") { + name, err = filepath.Abs(path) + } else { + name, err = relative(root, path) + } + if err != nil { return fmt.Errorf("relative name <%s>: <%s>, %w", path, root, err) } @@ -182,7 +189,7 @@ func (a *Archive) Extract(dst string, r io.Reader) (int64, error) { } var target string - if dst == h.Name { + if dst == h.Name || strings.HasPrefix(h.Name, "/") { target = h.Name } else { name, err := relative(dst, h.Name) diff --git a/archive/tar/tar_test.go b/archive/tar/tar_test.go index c9a36881..17f8d0bf 100644 --- a/archive/tar/tar_test.go +++ b/archive/tar/tar_test.go @@ -5,6 +5,7 @@ import ( "io/ioutil" "os" "path/filepath" + "strings" "testing" "github.com/meltwater/drone-cache/test" @@ -16,12 +17,21 @@ var ( testRoot = "testdata" testRootMounted = "testdata/mounted" testRootExtracted = "testdata/extracted" + testAbsPattern = "testdata_absolute" ) func TestCreate(t *testing.T) { test.Ok(t, os.MkdirAll(testRootMounted, 0755)) test.Ok(t, os.MkdirAll(testRootExtracted, 0755)) - t.Cleanup(func() { os.RemoveAll(testRoot) }) + + testAbs, err := ioutil.TempDir("", testAbsPattern) + test.Ok(t, err) + test.Equals(t, filepath.IsAbs(testAbs), true) + + t.Cleanup(func() { + os.RemoveAll(testRoot) + os.RemoveAll(testAbs) + }) for _, tc := range []struct { name string @@ -50,7 +60,7 @@ func TestCreate(t *testing.T) { { name: "existing mount paths", ta: New(log.NewNopLogger(), testRootMounted, true), - srcs: exampleFileTree(t, "tar_create"), + srcs: exampleFileTree(t, "tar_create", testRootMounted), written: 43, // 3 x tmpfile in dir, 1 tmpfile err: nil, }, @@ -68,12 +78,30 @@ func TestCreate(t *testing.T) { written: 43, err: nil, }, + { + name: "absolute mount paths", + ta: New(log.NewNopLogger(), testRootMounted, true), + srcs: exampleFileTree(t, "tar_create", testAbs), + written: 43, + err: nil, + }, } { tc := tc // NOTICE: https://github.com/golang/go/wiki/CommonMistakes#using-goroutines-on-loop-iterator-variables t.Run(tc.name, func(t *testing.T) { t.Parallel() // Setup + var absSrcs []string + var relativeSrcs []string + + for _, src := range tc.srcs { + if strings.HasPrefix(src, "/") { + absSrcs = append(absSrcs, src) + } else { + relativeSrcs = append(relativeSrcs, src) + } + } + dstDir, dstDirClean := test.CreateTempDir(t, "tar_create_archives", testRootMounted) t.Cleanup(dstDirClean) @@ -89,12 +117,21 @@ func TestCreate(t *testing.T) { } // Test + for _, src := range absSrcs { + test.Ok(t, os.RemoveAll(src)) + } + test.Exists(t, archivePath) test.Assert(t, written == tc.written, "case %q: written bytes got %d want %v", tc.name, written, tc.written) _, err = extract(tc.ta, archivePath, extDir) test.Ok(t, err) - test.EqualDirs(t, extDir, testRootMounted, tc.srcs) + + test.EqualDirs(t, extDir, testRootMounted, relativeSrcs) + + for _, src := range absSrcs { + test.Exists(t, src) + } }) } } @@ -102,7 +139,15 @@ func TestCreate(t *testing.T) { func TestExtract(t *testing.T) { test.Ok(t, os.MkdirAll(testRootMounted, 0755)) test.Ok(t, os.MkdirAll(testRootExtracted, 0755)) - t.Cleanup(func() { os.RemoveAll(testRoot) }) + + testAbs, err := ioutil.TempDir("", testAbsPattern) + test.Ok(t, err) + test.Equals(t, filepath.IsAbs(testAbs), true) + + t.Cleanup(func() { + os.RemoveAll(testRoot) + os.RemoveAll(testAbs) + }) // Setup ta := New(log.NewNopLogger(), testRootMounted, false) @@ -110,9 +155,9 @@ func TestExtract(t *testing.T) { arcDir, arcDirClean := test.CreateTempDir(t, "tar_extract_archives", testRootMounted) t.Cleanup(arcDirClean) - files := exampleFileTree(t, "tar_extract") + files := exampleFileTree(t, "tar_extract", testRootMounted) archivePath := filepath.Join(arcDir, "test.tar") - _, err := create(ta, files, archivePath) + _, err = create(ta, files, archivePath) test.Ok(t, err) nestedFiles := exampleNestedFileTree(t, "tar_extract_nested") @@ -137,6 +182,11 @@ func TestExtract(t *testing.T) { badArchivePath := filepath.Join(arcDir, "bad_test.tar") test.Ok(t, ioutil.WriteFile(badArchivePath, []byte("hello\ndrone\n"), 0644)) + filesAbs := exampleFileTree(t, ".tar_extract_absolute", testAbs) + archiveAbsPath := filepath.Join(arcDir, "test_absolute.tar") + _, err = create(ta, filesAbs, archiveAbsPath) + test.Ok(t, err) + for _, tc := range []struct { name string ta *Archive @@ -209,16 +259,38 @@ func TestExtract(t *testing.T) { written: 43, err: nil, }, + { + name: "absolute mount paths", + ta: New(log.NewNopLogger(), testRootMounted, true), + archivePath: archiveAbsPath, + srcs: filesAbs, + written: 43, + err: nil, + }, } { tc := tc // NOTE: https://github.com/golang/go/wiki/CommonMistakes#using-goroutines-on-loop-iterator-variables t.Run(tc.name, func(t *testing.T) { t.Parallel() // Setup + var absSrcs []string + var relativeSrcs []string + + for _, src := range tc.srcs { + if strings.HasPrefix(src, "/") { + absSrcs = append(absSrcs, src) + } else { + relativeSrcs = append(relativeSrcs, src) + } + } + dstDir, dstDirClean := test.CreateTempDir(t, "tar_extract_"+tc.name, testRootExtracted) t.Cleanup(dstDirClean) // Run + for _, src := range absSrcs { + test.Ok(t, os.RemoveAll(src)) + } written, err := extract(tc.ta, tc.archivePath, dstDir) if err != nil { test.Expected(t, err, tc.err) @@ -227,7 +299,13 @@ func TestExtract(t *testing.T) { // Test test.Assert(t, written == tc.written, "case %q: written bytes got %d want %v", tc.name, written, tc.written) - test.EqualDirs(t, dstDir, testRootMounted, tc.srcs) + + for _, src := range absSrcs { + test.Exists(t, src) + } + + test.EqualDirs(t, dstDir, testRootMounted, relativeSrcs) + }) } } @@ -286,11 +364,11 @@ func extract(a *Archive, src string, dst string) (int64, error) { // Fixtures -func exampleFileTree(t *testing.T, name string) []string { - file, fileClean := test.CreateTempFile(t, name, []byte("hello\ndrone!\n"), testRootMounted) // 13 bytes +func exampleFileTree(t *testing.T, name string, in string) []string { + file, fileClean := test.CreateTempFile(t, name, []byte("hello\ndrone!\n"), in) // 13 bytes t.Cleanup(fileClean) - dir, dirClean := test.CreateTempFilesInDir(t, name, []byte("hello\ngo!\n"), testRootMounted) // 10 bytes + dir, dirClean := test.CreateTempFilesInDir(t, name, []byte("hello\ngo!\n"), in) // 10 bytes t.Cleanup(dirClean) return []string{file, dir} From 452a24d80b806649849d3d83f302fcabe2c9834e Mon Sep 17 00:00:00 2001 From: hacktron95 Date: Tue, 13 Oct 2020 10:47:25 +0800 Subject: [PATCH 2/5] add absolute path mode for gzip --- archive/gzip/gzip_test.go | 103 +++++++++++++++++++++++++++++++++----- archive/tar/tar_test.go | 4 -- 2 files changed, 90 insertions(+), 17 deletions(-) diff --git a/archive/gzip/gzip_test.go b/archive/gzip/gzip_test.go index 91e81ed3..6c1e4671 100644 --- a/archive/gzip/gzip_test.go +++ b/archive/gzip/gzip_test.go @@ -7,6 +7,7 @@ import ( "io/ioutil" "os" "path/filepath" + "strings" "testing" "github.com/go-kit/kit/log" @@ -19,12 +20,21 @@ var ( testRoot = "testdata" testRootMounted = "testdata/mounted" testRootExtracted = "testdata/extracted" + testAbsPattern = "testdata_absolute" ) func TestCreate(t *testing.T) { test.Ok(t, os.MkdirAll(testRootMounted, 0755)) test.Ok(t, os.MkdirAll(testRootExtracted, 0755)) - t.Cleanup(func() { os.RemoveAll(testRoot) }) + + testAbs, err := ioutil.TempDir("", testAbsPattern) + test.Ok(t, err) + test.Equals(t, filepath.IsAbs(testAbs), true) + + t.Cleanup(func() { + os.RemoveAll(testRoot) + os.RemoveAll(testAbs) + }) for _, tc := range []struct { name string @@ -44,7 +54,7 @@ func TestCreate(t *testing.T) { name: "non-existing mount paths", tgz: New(log.NewNopLogger(), testRootMounted, true, flate.DefaultCompression), srcs: []string{ - "iamnotexists", + "idonotexist", "metoo", }, written: 0, @@ -53,7 +63,7 @@ func TestCreate(t *testing.T) { { name: "existing mount paths", tgz: New(log.NewNopLogger(), testRootMounted, true, flate.DefaultCompression), - srcs: exampleFileTree(t, "gzip_create"), + srcs: exampleFileTree(t, "gzip_create", testRootMounted), written: 43, // 3 x tmpfile in dir, 1 tmpfile err: nil, }, @@ -71,12 +81,30 @@ func TestCreate(t *testing.T) { written: 43, err: nil, }, + { + name: "absolute mount paths", + tgz: New(log.NewNopLogger(), testRootMounted, true, flate.DefaultCompression), + srcs: exampleFileTree(t, "tar_create", testAbs), + written: 43, + err: nil, + }, } { - tc := tc // NOTE: https://github.com/golang/go/wiki/CommonMistakes#using-goroutines-on-loop-iterator-variables + tc := tc // NOTICE: https://github.com/golang/go/wiki/CommonMistakes#using-goroutines-on-loop-iterator-variables t.Run(tc.name, func(t *testing.T) { t.Parallel() // Setup + var absSrcs []string + var relativeSrcs []string + + for _, src := range tc.srcs { + if strings.HasPrefix(src, "/") { + absSrcs = append(absSrcs, src) + } else { + relativeSrcs = append(relativeSrcs, src) + } + } + dstDir, dstDirClean := test.CreateTempDir(t, "gzip_create_archives", testRootMounted) t.Cleanup(dstDirClean) @@ -91,12 +119,21 @@ func TestCreate(t *testing.T) { return } + // Test + for _, src := range absSrcs { + test.Ok(t, os.RemoveAll(src)) + } + test.Exists(t, archivePath) test.Assert(t, written == tc.written, "case %q: written bytes got %d want %v", tc.name, written, tc.written) _, err = extract(tc.tgz, archivePath, extDir) test.Ok(t, err) - test.EqualDirs(t, extDir, testRootMounted, tc.srcs) + test.EqualDirs(t, extDir, testRootMounted, relativeSrcs) + + for _, src := range absSrcs { + test.Exists(t, src) + } }) } } @@ -104,7 +141,15 @@ func TestCreate(t *testing.T) { func TestExtract(t *testing.T) { test.Ok(t, os.MkdirAll(testRootMounted, 0755)) test.Ok(t, os.MkdirAll(testRootExtracted, 0755)) - t.Cleanup(func() { os.RemoveAll(testRoot) }) + + testAbs, err := ioutil.TempDir("", testAbsPattern) + test.Ok(t, err) + test.Equals(t, filepath.IsAbs(testAbs), true) + + t.Cleanup(func() { + os.RemoveAll(testRoot) + os.RemoveAll(testAbs) + }) // Setup tgz := New(log.NewNopLogger(), testRootMounted, false, flate.DefaultCompression) @@ -112,9 +157,9 @@ func TestExtract(t *testing.T) { arcDir, arcDirClean := test.CreateTempDir(t, "gzip_extract_archive") t.Cleanup(arcDirClean) - files := exampleFileTree(t, "gzip_extract") + files := exampleFileTree(t, "gzip_extract", testRootMounted) archivePath := filepath.Join(arcDir, "test.tar.gz") - _, err := create(tgz, files, archivePath) + _, err = create(tgz, files, archivePath) test.Ok(t, err) nestedFiles := exampleNestedFileTree(t, "gzip_extract_nested") @@ -134,6 +179,11 @@ func TestExtract(t *testing.T) { badArchivePath := filepath.Join(arcDir, "bad_test.tar.gz") test.Ok(t, ioutil.WriteFile(badArchivePath, []byte("hello\ndrone\n"), 0644)) + filesAbs := exampleFileTree(t, ".gzip_extract_absolute", testAbs) + archiveAbsPath := filepath.Join(arcDir, "test_absolute.tar.gz") + _, err = create(tgz, filesAbs, archiveAbsPath) + test.Ok(t, err) + for _, tc := range []struct { name string tgz *Archive @@ -198,22 +248,49 @@ func TestExtract(t *testing.T) { written: 43, err: nil, }, + { + name: "absolute mount paths", + tgz: New(log.NewNopLogger(), testRootMounted, true, flate.DefaultCompression), + archivePath: archiveAbsPath, + srcs: filesAbs, + written: 43, + err: nil, + }, } { tc := tc // NOTE: https://github.com/golang/go/wiki/CommonMistakes#using-goroutines-on-loop-iterator-variables t.Run(tc.name, func(t *testing.T) { t.Parallel() + // Setup + var absSrcs []string + var relativeSrcs []string + + for _, src := range tc.srcs { + if strings.HasPrefix(src, "/") { + absSrcs = append(absSrcs, src) + } else { + relativeSrcs = append(relativeSrcs, src) + } + } + dstDir, dstDirClean := test.CreateTempDir(t, "gzip_extract_"+tc.name, testRootExtracted) t.Cleanup(dstDirClean) - + // Run + for _, src := range absSrcs { + test.Ok(t, os.RemoveAll(src)) + } written, err := extract(tc.tgz, tc.archivePath, dstDir) if err != nil { test.Expected(t, err, tc.err) return } + // Test test.Assert(t, written == tc.written, "case %q: written bytes got %d want %v", tc.name, written, tc.written) - test.EqualDirs(t, dstDir, testRootMounted, tc.srcs) + for _, src := range absSrcs { + test.Exists(t, src) + } + test.EqualDirs(t, dstDir, testRootMounted, relativeSrcs) }) } } @@ -272,11 +349,11 @@ func extract(a *Archive, src string, dst string) (int64, error) { // Fixtures -func exampleFileTree(t *testing.T, name string) []string { - file, fileClean := test.CreateTempFile(t, name, []byte("hello\ndrone!\n"), testRootMounted) // 13 bytes +func exampleFileTree(t *testing.T, name string, in string) []string { + file, fileClean := test.CreateTempFile(t, name, []byte("hello\ndrone!\n"), in) // 13 bytes t.Cleanup(fileClean) - dir, dirClean := test.CreateTempFilesInDir(t, name, []byte("hello\ngo!\n"), testRootMounted) // 10 bytes + dir, dirClean := test.CreateTempFilesInDir(t, name, []byte("hello\ngo!\n"), in) // 10 bytes t.Cleanup(dirClean) return []string{file, dir} diff --git a/archive/tar/tar_test.go b/archive/tar/tar_test.go index 17f8d0bf..ed1264be 100644 --- a/archive/tar/tar_test.go +++ b/archive/tar/tar_test.go @@ -126,7 +126,6 @@ func TestCreate(t *testing.T) { _, err = extract(tc.ta, archivePath, extDir) test.Ok(t, err) - test.EqualDirs(t, extDir, testRootMounted, relativeSrcs) for _, src := range absSrcs { @@ -299,13 +298,10 @@ func TestExtract(t *testing.T) { // Test test.Assert(t, written == tc.written, "case %q: written bytes got %d want %v", tc.name, written, tc.written) - for _, src := range absSrcs { test.Exists(t, src) } - test.EqualDirs(t, dstDir, testRootMounted, relativeSrcs) - }) } } From 31dfe25fefe89916e117f2f9d66e73600c4ce103 Mon Sep 17 00:00:00 2001 From: hacktron95 Date: Tue, 13 Oct 2020 11:05:00 +0800 Subject: [PATCH 3/5] update change log to include pull #141 --- CHANGELOG.md | 15 +++++++++------ 1 file changed, 9 insertions(+), 6 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index f992393c..620f2188 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,7 +7,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## Unreleased -- [#133](https://github.com/meltwater/drone-cache/pull/133) bacnkend/s3: Fixed Anonymous Credentials Error on public buckets. +- [#141](https://github.com/meltwater/drone-cache/pull/141) archive/tar, archive-gzip: + add absolute path mode: fix an issue #130 with drone where it fails to make extraction if the passed path is an absoulte path. + +- [#133](https://github.com/meltwater/drone-cache/pull/133) bacnkend/s3: Fixed Anonymous Credentials Error on public buckets. - Fixes [#132](https://github.com/meltwater/drone-cache/issues/132) - [#138](https://github.com/meltwater/drone-cache/pull/138) backend/gcs: Fix GCS to pass credentials correctly when `GCS_ENDPOINT` is not set. - [#135](https://github.com/meltwater/drone-cache/issues/135) backend/gcs: Fixed parsing of GCS JSON key. @@ -18,11 +21,11 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - [#86](https://github.com/meltwater/drone-cache/pull/86) Add backend operation timeout option that cancels request if they take longer than given duration. `BACKEND_OPERATION_TIMEOUT`, `backend.operation-timeot`. Default value is `3 minutes`. - [#86](https://github.com/meltwater/drone-cache/pull/86) Customize the cache key in the path. Adds a new `remote_root` option to customize it. Defaults to `repo.name`. - Fixes [#97](https://github.com/meltwater/drone-cache/issues/97). -[#89](https://github.com/meltwater/drone-cache/pull/89) Add Azure Storage Backend. -[#84](https://github.com/meltwater/drone-cache/pull/84) Adds compression level option. -[#77](https://github.com/meltwater/drone-cache/pull/77) Adds a new hidden CLI flag to be used for tests. -[#73](https://github.com/meltwater/drone-cache/pull/73) Add Google Cloud storage support -[#68](https://github.com/meltwater/drone-cache/pull/68) Introduces new storage backend, sFTP. + [#89](https://github.com/meltwater/drone-cache/pull/89) Add Azure Storage Backend. + [#84](https://github.com/meltwater/drone-cache/pull/84) Adds compression level option. + [#77](https://github.com/meltwater/drone-cache/pull/77) Adds a new hidden CLI flag to be used for tests. + [#73](https://github.com/meltwater/drone-cache/pull/73) Add Google Cloud storage support + [#68](https://github.com/meltwater/drone-cache/pull/68) Introduces new storage backend, sFTP. ### Changed From f7d901469d9cdfe940f55f216031acec5e32164d Mon Sep 17 00:00:00 2001 From: Osama Nabil Date: Thu, 4 Feb 2021 12:59:35 +0800 Subject: [PATCH 4/5] Update CHANGELOG.md Co-authored-by: Kemal Akkoyun --- CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index e2a09f45..233434b9 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,7 +7,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## Unreleased -- [#141](https://github.com/meltwater/drone-cache/pull/141) archive/tar, archive-gzip: +- [#141](https://github.com/meltwater/drone-cache/pull/141) archive/tar, archive/gzip: add absolute path mode: fix an issue #130 with drone where it fails to make extraction if the passed path is an absoulte path. - [#133](https://github.com/meltwater/drone-cache/pull/133) bacnkend/s3: Fixed Anonymous Credentials Error on public buckets. From bd7acb0e5fc128019ca88cb6bcfe296e1f13693e Mon Sep 17 00:00:00 2001 From: hacktron95 Date: Sun, 14 Feb 2021 17:07:34 +0800 Subject: [PATCH 5/5] resolve before merge --- CHANGELOG.md | 10 +++++----- archive/gzip/gzip_test.go | 3 +-- 2 files changed, 6 insertions(+), 7 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 233434b9..a2b26bf4 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -21,11 +21,11 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - [#86](https://github.com/meltwater/drone-cache/pull/86) Add backend operation timeout option that cancels request if they take longer than given duration. `BACKEND_OPERATION_TIMEOUT`, `backend.operation-timeot`. Default value is `3 minutes`. - [#86](https://github.com/meltwater/drone-cache/pull/86) Customize the cache key in the path. Adds a new `remote_root` option to customize it. Defaults to `repo.name`. - Fixes [#97](https://github.com/meltwater/drone-cache/issues/97). - [#89](https://github.com/meltwater/drone-cache/pull/89) Add Azure Storage Backend. - [#84](https://github.com/meltwater/drone-cache/pull/84) Adds compression level option. - [#77](https://github.com/meltwater/drone-cache/pull/77) Adds a new hidden CLI flag to be used for tests. - [#73](https://github.com/meltwater/drone-cache/pull/73) Add Google Cloud storage support - [#68](https://github.com/meltwater/drone-cache/pull/68) Introduces new storage backend, sFTP. +[#89](https://github.com/meltwater/drone-cache/pull/89) Add Azure Storage Backend. +[#84](https://github.com/meltwater/drone-cache/pull/84) Adds compression level option. +[#77](https://github.com/meltwater/drone-cache/pull/77) Adds a new hidden CLI flag to be used for tests. +[#73](https://github.com/meltwater/drone-cache/pull/73) Add Google Cloud storage support +[#68](https://github.com/meltwater/drone-cache/pull/68) Introduces new storage backend, sFTP. ### Changed diff --git a/archive/gzip/gzip_test.go b/archive/gzip/gzip_test.go index 6c1e4671..c18ba9e8 100644 --- a/archive/gzip/gzip_test.go +++ b/archive/gzip/gzip_test.go @@ -54,7 +54,7 @@ func TestCreate(t *testing.T) { name: "non-existing mount paths", tgz: New(log.NewNopLogger(), testRootMounted, true, flate.DefaultCompression), srcs: []string{ - "idonotexist", + "iamnotexists", "metoo", }, written: 0, @@ -119,7 +119,6 @@ func TestCreate(t *testing.T) { return } - // Test for _, src := range absSrcs { test.Ok(t, os.RemoveAll(src)) }