Skip to content

chore(coderd): improve tests for tar<->zip conversion #12477

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 5 commits into from
Mar 11, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
31 changes: 15 additions & 16 deletions coderd/files_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,8 @@ import (
"bytes"
"context"
"net/http"
"os"
"path/filepath"
"testing"

"github.com/google/uuid"
Expand All @@ -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"
)

Expand Down Expand Up @@ -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) {
Expand All @@ -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)
Expand All @@ -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) {
Expand All @@ -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))
Expand All @@ -153,5 +151,6 @@ func TestDownload(t *testing.T) {
// then
require.Equal(t, codersdk.ContentTypeZip, contentType)
require.Equal(t, expectedZip, data)
assertSampleZipFile(t, data)
})
}
10 changes: 7 additions & 3 deletions coderd/fileszip.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
213 changes: 213 additions & 0 deletions coderd/fileszip_test.go
Original file line number Diff line number Diff line change
@@ -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")
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Windows is no bueno?

Copy link
Member Author

@johnstcn johnstcn Mar 8, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Generally yes, but I'm not going to assume the presence of tar and unzip on Windows for this test :-)

}

// 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)
}
Binary file added coderd/testdata/test.tar
Binary file not shown.
Binary file added coderd/testdata/test.zip
Binary file not shown.