From b1ecc5303338b1eef0e77a0ce79d67b88b64efe1 Mon Sep 17 00:00:00 2001 From: Cian Johnston Date: Mon, 11 Mar 2024 13:29:57 +0000 Subject: [PATCH] chore(coderd): improve tests for tar<->zip conversion (#12477) * improve tests for tar<->zip conversion * set mode and modtime correctly when converting from zip to tar (#12476) --- coderd/files_test.go | 31 +++--- coderd/fileszip.go | 10 +- coderd/fileszip_test.go | 213 +++++++++++++++++++++++++++++++++++++++ coderd/testdata/test.tar | Bin 0 -> 10240 bytes coderd/testdata/test.zip | Bin 0 -> 636 bytes 5 files changed, 235 insertions(+), 19 deletions(-) create mode 100644 coderd/fileszip_test.go create mode 100644 coderd/testdata/test.tar create mode 100644 coderd/testdata/test.zip diff --git a/coderd/files_test.go b/coderd/files_test.go index ff0eb60585..a5e6aab249 100644 --- a/coderd/files_test.go +++ b/coderd/files_test.go @@ -5,6 +5,8 @@ import ( "bytes" "context" "net/http" + "os" + "path/filepath" "testing" "github.com/google/uuid" @@ -13,7 +15,6 @@ import ( "github.com/coder/coder/v2/coderd" "github.com/coder/coder/v2/coderd/coderdtest" "github.com/coder/coder/v2/codersdk" - "github.com/coder/coder/v2/provisioner/echo" "github.com/coder/coder/v2/testutil" ) @@ -83,16 +84,20 @@ func TestDownload(t *testing.T) { // given ctx, cancel := context.WithTimeout(context.Background(), testutil.WaitLong) defer cancel() + tarball, err := os.ReadFile(filepath.Join("testdata", "test.tar")) + require.NoError(t, err) // when - resp, err := client.Upload(ctx, codersdk.ContentTypeTar, bytes.NewReader(make([]byte, 1024))) + resp, err := client.Upload(ctx, codersdk.ContentTypeTar, bytes.NewReader(tarball)) require.NoError(t, err) data, contentType, err := client.Download(ctx, resp.ID) require.NoError(t, err) // then - require.Len(t, data, 1024) + require.Len(t, data, len(tarball)) require.Equal(t, codersdk.ContentTypeTar, contentType) + require.Equal(t, tarball, data) + assertSampleTarFile(t, data) }) t.Run("InsertZip_DownloadTar", func(t *testing.T) { @@ -101,14 +106,7 @@ func TestDownload(t *testing.T) { _ = coderdtest.CreateFirstUser(t, client) // given - tarball, err := echo.Tar(&echo.Responses{ - Parse: echo.ParseComplete, - ProvisionApply: echo.ApplyComplete, - }) - require.NoError(t, err) - - tarReader := tar.NewReader(bytes.NewReader(tarball)) - zipContent, err := coderd.CreateZipFromTar(tarReader) + zipContent, err := os.ReadFile(filepath.Join("testdata", "test.zip")) require.NoError(t, err) ctx, cancel := context.WithTimeout(context.Background(), testutil.WaitLong) @@ -122,7 +120,10 @@ func TestDownload(t *testing.T) { // then require.Equal(t, codersdk.ContentTypeTar, contentType) - require.Equal(t, tarball, data) + + // Note: creating a zip from a tar will result in some loss of information + // as zip files do not store UNIX user:group data. + assertSampleTarFile(t, data) }) t.Run("InsertTar_DownloadZip", func(t *testing.T) { @@ -131,10 +132,7 @@ func TestDownload(t *testing.T) { _ = coderdtest.CreateFirstUser(t, client) // given - tarball, err := echo.Tar(&echo.Responses{ - Parse: echo.ParseComplete, - ProvisionApply: echo.ApplyComplete, - }) + tarball, err := os.ReadFile(filepath.Join("testdata", "test.tar")) require.NoError(t, err) tarReader := tar.NewReader(bytes.NewReader(tarball)) @@ -153,5 +151,6 @@ func TestDownload(t *testing.T) { // then require.Equal(t, codersdk.ContentTypeZip, contentType) require.Equal(t, expectedZip, data) + assertSampleZipFile(t, data) }) } diff --git a/coderd/fileszip.go b/coderd/fileszip.go index b001958912..0d9c6ae478 100644 --- a/coderd/fileszip.go +++ b/coderd/fileszip.go @@ -39,9 +39,13 @@ func processFileInZipArchive(file *zip.File, tarWriter *tar.Writer) error { defer fileReader.Close() err = tarWriter.WriteHeader(&tar.Header{ - Name: file.Name, - Size: file.FileInfo().Size(), - Mode: 0o644, + Name: file.Name, + Size: file.FileInfo().Size(), + Mode: int64(file.Mode()), + ModTime: file.Modified, + // Note: Zip archives do not store ownership information. + Uid: 1000, + Gid: 1000, }) if err != nil { return err diff --git a/coderd/fileszip_test.go b/coderd/fileszip_test.go new file mode 100644 index 0000000000..b5f5057e3b --- /dev/null +++ b/coderd/fileszip_test.go @@ -0,0 +1,213 @@ +package coderd_test + +import ( + "archive/tar" + "archive/zip" + "bytes" + "io" + "io/fs" + "os" + "os/exec" + "path/filepath" + "runtime" + "strings" + "testing" + "time" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + "golang.org/x/xerrors" + + "github.com/coder/coder/v2/coderd" + "github.com/coder/coder/v2/testutil" +) + +func TestCreateTarFromZip(t *testing.T) { + t.Parallel() + if runtime.GOOS != "linux" { + t.Skip("skipping this test on non-Linux platform") + } + + // Read a zip file we prepared earlier + ctx := testutil.Context(t, testutil.WaitShort) + zipBytes, err := os.ReadFile(filepath.Join("testdata", "test.zip")) + require.NoError(t, err, "failed to read sample zip file") + // Assert invariant + assertSampleZipFile(t, zipBytes) + + zr, err := zip.NewReader(bytes.NewReader(zipBytes), int64(len(zipBytes))) + require.NoError(t, err, "failed to parse sample zip file") + + tarBytes, err := coderd.CreateTarFromZip(zr) + require.NoError(t, err, "failed to convert zip to tar") + + assertSampleTarFile(t, tarBytes) + + tempDir := t.TempDir() + tempFilePath := filepath.Join(tempDir, "test.tar") + err = os.WriteFile(tempFilePath, tarBytes, 0o600) + require.NoError(t, err, "failed to write converted tar file") + + cmd := exec.CommandContext(ctx, "tar", "--extract", "--verbose", "--file", tempFilePath, "--directory", tempDir) + require.NoError(t, cmd.Run(), "failed to extract converted tar file") + assertExtractedFiles(t, tempDir, true) +} + +func TestCreateZipFromTar(t *testing.T) { + t.Parallel() + if runtime.GOOS != "linux" { + t.Skip("skipping this test on non-Linux platform") + } + + tarBytes, err := os.ReadFile(filepath.Join("testdata", "test.tar")) + require.NoError(t, err, "failed to read sample tar file") + + tr := tar.NewReader(bytes.NewReader(tarBytes)) + zipBytes, err := coderd.CreateZipFromTar(tr) + require.NoError(t, err) + + assertSampleZipFile(t, zipBytes) + + tempDir := t.TempDir() + tempFilePath := filepath.Join(tempDir, "test.zip") + err = os.WriteFile(tempFilePath, zipBytes, 0o600) + require.NoError(t, err, "failed to write converted zip file") + + ctx := testutil.Context(t, testutil.WaitShort) + cmd := exec.CommandContext(ctx, "unzip", tempFilePath, "-d", tempDir) + require.NoError(t, cmd.Run(), "failed to extract converted zip file") + + assertExtractedFiles(t, tempDir, false) +} + +// nolint:revive // this is a control flag but it's in a unit test +func assertExtractedFiles(t *testing.T, dir string, checkModePerm bool) { + t.Helper() + + _ = filepath.Walk(dir, func(path string, info fs.FileInfo, err error) error { + relPath := strings.TrimPrefix(path, dir) + switch relPath { + case "", "/test.zip", "/test.tar": // ignore + case "/test": + stat, err := os.Stat(path) + assert.NoError(t, err, "failed to stat path %q", path) + assert.True(t, stat.IsDir(), "expected path %q to be a directory") + if checkModePerm { + assert.Equal(t, fs.ModePerm&0o755, stat.Mode().Perm(), "expected mode 0755 on directory") + } + assert.Equal(t, archiveRefTime(t).UTC(), stat.ModTime().UTC(), "unexpected modtime of %q", path) + case "/test/hello.txt": + stat, err := os.Stat(path) + assert.NoError(t, err, "failed to stat path %q", path) + assert.False(t, stat.IsDir(), "expected path %q to be a file") + if checkModePerm { + assert.Equal(t, fs.ModePerm&0o644, stat.Mode().Perm(), "expected mode 0644 on file") + } + bs, err := os.ReadFile(path) + assert.NoError(t, err, "failed to read file %q", path) + assert.Equal(t, "hello", string(bs), "unexpected content in file %q", path) + case "/test/dir": + stat, err := os.Stat(path) + assert.NoError(t, err, "failed to stat path %q", path) + assert.True(t, stat.IsDir(), "expected path %q to be a directory") + if checkModePerm { + assert.Equal(t, fs.ModePerm&0o755, stat.Mode().Perm(), "expected mode 0755 on directory") + } + case "/test/dir/world.txt": + stat, err := os.Stat(path) + assert.NoError(t, err, "failed to stat path %q", path) + assert.False(t, stat.IsDir(), "expected path %q to be a file") + if checkModePerm { + assert.Equal(t, fs.ModePerm&0o644, stat.Mode().Perm(), "expected mode 0644 on file") + } + bs, err := os.ReadFile(path) + assert.NoError(t, err, "failed to read file %q", path) + assert.Equal(t, "world", string(bs), "unexpected content in file %q", path) + default: + assert.Fail(t, "unexpected path", relPath) + } + + return nil + }) +} + +func assertSampleTarFile(t *testing.T, tarBytes []byte) { + t.Helper() + + tr := tar.NewReader(bytes.NewReader(tarBytes)) + for { + hdr, err := tr.Next() + if err != nil { + if err == io.EOF { + return + } + require.NoError(t, err) + } + + // Note: ignoring timezones here. + require.Equal(t, archiveRefTime(t).UTC(), hdr.ModTime.UTC()) + + switch hdr.Name { + case "test/": + require.Equal(t, hdr.Typeflag, byte(tar.TypeDir)) + case "test/hello.txt": + require.Equal(t, hdr.Typeflag, byte(tar.TypeReg)) + bs, err := io.ReadAll(tr) + if err != nil && !xerrors.Is(err, io.EOF) { + require.NoError(t, err) + } + require.Equal(t, "hello", string(bs)) + case "test/dir/": + require.Equal(t, hdr.Typeflag, byte(tar.TypeDir)) + case "test/dir/world.txt": + require.Equal(t, hdr.Typeflag, byte(tar.TypeReg)) + bs, err := io.ReadAll(tr) + if err != nil && !xerrors.Is(err, io.EOF) { + require.NoError(t, err) + } + require.Equal(t, "world", string(bs)) + default: + require.Failf(t, "unexpected file in tar", hdr.Name) + } + } +} + +func assertSampleZipFile(t *testing.T, zipBytes []byte) { + t.Helper() + + zr, err := zip.NewReader(bytes.NewReader(zipBytes), int64(len(zipBytes))) + require.NoError(t, err) + + for _, f := range zr.File { + // Note: ignoring timezones here. + require.Equal(t, archiveRefTime(t).UTC(), f.Modified.UTC()) + switch f.Name { + case "test/", "test/dir/": + // directory + case "test/hello.txt": + rc, err := f.Open() + require.NoError(t, err) + bs, err := io.ReadAll(rc) + _ = rc.Close() + require.NoError(t, err) + require.Equal(t, "hello", string(bs)) + case "test/dir/world.txt": + rc, err := f.Open() + require.NoError(t, err) + bs, err := io.ReadAll(rc) + _ = rc.Close() + require.NoError(t, err) + require.Equal(t, "world", string(bs)) + default: + require.Failf(t, "unexpected file in zip", f.Name) + } + } +} + +// archiveRefTime is the Go reference time. The contents of the sample tar and zip files +// in testdata/ all have their modtimes set to the below in some timezone. +func archiveRefTime(t *testing.T) time.Time { + locMST, err := time.LoadLocation("MST") + require.NoError(t, err, "failed to load MST timezone") + return time.Date(2006, 1, 2, 3, 4, 5, 0, locMST) +} diff --git a/coderd/testdata/test.tar b/coderd/testdata/test.tar new file mode 100644 index 0000000000000000000000000000000000000000..09d7ff6f111ceb9227cd0a8671f565f056ae9867 GIT binary patch literal 10240 zcmeIy;R=E<5Ww+0%ATM$_4Iing#tmCi+cPvqmTqca1!{xF9T0^<99oK)BB@J8zw|0 zqQYV;G2=C+Vkm2JDML2P!h6;lD@se+W4(62_5S3vR_Cs6+)Tk=`I%4uIpt&j>#Iw7 z2V#R4_Ft-;{%>(_KK;LUuB{LL;n%Mk`(GBtc<$f-ftU&*U-O?AQYvlMKgx8+ozMT! z_NfX_b|Ns$|D|c$F3s#m#yH{2=07W?VE(tbfFqEVgm*|Iva56AY{k6k+@3&W}r4`%^j4Ush85qDs z08Z_kFzqRsMF=C-5HiA9u-u9j=mHQHLN=m2zbGd~ucV>`?wqrjW`Px$i^XsBXQ=U1|YJzmtp221t%^uS=oS!8CZbO4d^HZki!@NSZjf# literal 0 HcmV?d00001