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

@@ -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)
 	})
 }

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

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)
+}