Skip to content

Commit f95f3f1

Browse files
johnstcnmtojek
authored andcommitted
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)
1 parent 8caafec commit f95f3f1

File tree

5 files changed

+235
-19
lines changed

5 files changed

+235
-19
lines changed

coderd/files_test.go

Lines changed: 15 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,8 @@ import (
55
"bytes"
66
"context"
77
"net/http"
8+
"os"
9+
"path/filepath"
810
"testing"
911

1012
"github.com/google/uuid"
@@ -13,7 +15,6 @@ import (
1315
"github.com/coder/coder/v2/coderd"
1416
"github.com/coder/coder/v2/coderd/coderdtest"
1517
"github.com/coder/coder/v2/codersdk"
16-
"github.com/coder/coder/v2/provisioner/echo"
1718
"github.com/coder/coder/v2/testutil"
1819
)
1920

@@ -83,16 +84,20 @@ func TestDownload(t *testing.T) {
8384
// given
8485
ctx, cancel := context.WithTimeout(context.Background(), testutil.WaitLong)
8586
defer cancel()
87+
tarball, err := os.ReadFile(filepath.Join("testdata", "test.tar"))
88+
require.NoError(t, err)
8689

8790
// when
88-
resp, err := client.Upload(ctx, codersdk.ContentTypeTar, bytes.NewReader(make([]byte, 1024)))
91+
resp, err := client.Upload(ctx, codersdk.ContentTypeTar, bytes.NewReader(tarball))
8992
require.NoError(t, err)
9093
data, contentType, err := client.Download(ctx, resp.ID)
9194
require.NoError(t, err)
9295

9396
// then
94-
require.Len(t, data, 1024)
97+
require.Len(t, data, len(tarball))
9598
require.Equal(t, codersdk.ContentTypeTar, contentType)
99+
require.Equal(t, tarball, data)
100+
assertSampleTarFile(t, data)
96101
})
97102

98103
t.Run("InsertZip_DownloadTar", func(t *testing.T) {
@@ -101,14 +106,7 @@ func TestDownload(t *testing.T) {
101106
_ = coderdtest.CreateFirstUser(t, client)
102107

103108
// given
104-
tarball, err := echo.Tar(&echo.Responses{
105-
Parse: echo.ParseComplete,
106-
ProvisionApply: echo.ApplyComplete,
107-
})
108-
require.NoError(t, err)
109-
110-
tarReader := tar.NewReader(bytes.NewReader(tarball))
111-
zipContent, err := coderd.CreateZipFromTar(tarReader)
109+
zipContent, err := os.ReadFile(filepath.Join("testdata", "test.zip"))
112110
require.NoError(t, err)
113111

114112
ctx, cancel := context.WithTimeout(context.Background(), testutil.WaitLong)
@@ -122,7 +120,10 @@ func TestDownload(t *testing.T) {
122120

123121
// then
124122
require.Equal(t, codersdk.ContentTypeTar, contentType)
125-
require.Equal(t, tarball, data)
123+
124+
// Note: creating a zip from a tar will result in some loss of information
125+
// as zip files do not store UNIX user:group data.
126+
assertSampleTarFile(t, data)
126127
})
127128

128129
t.Run("InsertTar_DownloadZip", func(t *testing.T) {
@@ -131,10 +132,7 @@ func TestDownload(t *testing.T) {
131132
_ = coderdtest.CreateFirstUser(t, client)
132133

133134
// given
134-
tarball, err := echo.Tar(&echo.Responses{
135-
Parse: echo.ParseComplete,
136-
ProvisionApply: echo.ApplyComplete,
137-
})
135+
tarball, err := os.ReadFile(filepath.Join("testdata", "test.tar"))
138136
require.NoError(t, err)
139137

140138
tarReader := tar.NewReader(bytes.NewReader(tarball))
@@ -153,5 +151,6 @@ func TestDownload(t *testing.T) {
153151
// then
154152
require.Equal(t, codersdk.ContentTypeZip, contentType)
155153
require.Equal(t, expectedZip, data)
154+
assertSampleZipFile(t, data)
156155
})
157156
}

coderd/fileszip.go

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -39,9 +39,13 @@ func processFileInZipArchive(file *zip.File, tarWriter *tar.Writer) error {
3939
defer fileReader.Close()
4040

4141
err = tarWriter.WriteHeader(&tar.Header{
42-
Name: file.Name,
43-
Size: file.FileInfo().Size(),
44-
Mode: 0o644,
42+
Name: file.Name,
43+
Size: file.FileInfo().Size(),
44+
Mode: int64(file.Mode()),
45+
ModTime: file.Modified,
46+
// Note: Zip archives do not store ownership information.
47+
Uid: 1000,
48+
Gid: 1000,
4549
})
4650
if err != nil {
4751
return err

coderd/fileszip_test.go

Lines changed: 213 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,213 @@
1+
package coderd_test
2+
3+
import (
4+
"archive/tar"
5+
"archive/zip"
6+
"bytes"
7+
"io"
8+
"io/fs"
9+
"os"
10+
"os/exec"
11+
"path/filepath"
12+
"runtime"
13+
"strings"
14+
"testing"
15+
"time"
16+
17+
"github.com/stretchr/testify/assert"
18+
"github.com/stretchr/testify/require"
19+
"golang.org/x/xerrors"
20+
21+
"github.com/coder/coder/v2/coderd"
22+
"github.com/coder/coder/v2/testutil"
23+
)
24+
25+
func TestCreateTarFromZip(t *testing.T) {
26+
t.Parallel()
27+
if runtime.GOOS != "linux" {
28+
t.Skip("skipping this test on non-Linux platform")
29+
}
30+
31+
// Read a zip file we prepared earlier
32+
ctx := testutil.Context(t, testutil.WaitShort)
33+
zipBytes, err := os.ReadFile(filepath.Join("testdata", "test.zip"))
34+
require.NoError(t, err, "failed to read sample zip file")
35+
// Assert invariant
36+
assertSampleZipFile(t, zipBytes)
37+
38+
zr, err := zip.NewReader(bytes.NewReader(zipBytes), int64(len(zipBytes)))
39+
require.NoError(t, err, "failed to parse sample zip file")
40+
41+
tarBytes, err := coderd.CreateTarFromZip(zr)
42+
require.NoError(t, err, "failed to convert zip to tar")
43+
44+
assertSampleTarFile(t, tarBytes)
45+
46+
tempDir := t.TempDir()
47+
tempFilePath := filepath.Join(tempDir, "test.tar")
48+
err = os.WriteFile(tempFilePath, tarBytes, 0o600)
49+
require.NoError(t, err, "failed to write converted tar file")
50+
51+
cmd := exec.CommandContext(ctx, "tar", "--extract", "--verbose", "--file", tempFilePath, "--directory", tempDir)
52+
require.NoError(t, cmd.Run(), "failed to extract converted tar file")
53+
assertExtractedFiles(t, tempDir, true)
54+
}
55+
56+
func TestCreateZipFromTar(t *testing.T) {
57+
t.Parallel()
58+
if runtime.GOOS != "linux" {
59+
t.Skip("skipping this test on non-Linux platform")
60+
}
61+
62+
tarBytes, err := os.ReadFile(filepath.Join("testdata", "test.tar"))
63+
require.NoError(t, err, "failed to read sample tar file")
64+
65+
tr := tar.NewReader(bytes.NewReader(tarBytes))
66+
zipBytes, err := coderd.CreateZipFromTar(tr)
67+
require.NoError(t, err)
68+
69+
assertSampleZipFile(t, zipBytes)
70+
71+
tempDir := t.TempDir()
72+
tempFilePath := filepath.Join(tempDir, "test.zip")
73+
err = os.WriteFile(tempFilePath, zipBytes, 0o600)
74+
require.NoError(t, err, "failed to write converted zip file")
75+
76+
ctx := testutil.Context(t, testutil.WaitShort)
77+
cmd := exec.CommandContext(ctx, "unzip", tempFilePath, "-d", tempDir)
78+
require.NoError(t, cmd.Run(), "failed to extract converted zip file")
79+
80+
assertExtractedFiles(t, tempDir, false)
81+
}
82+
83+
// nolint:revive // this is a control flag but it's in a unit test
84+
func assertExtractedFiles(t *testing.T, dir string, checkModePerm bool) {
85+
t.Helper()
86+
87+
_ = filepath.Walk(dir, func(path string, info fs.FileInfo, err error) error {
88+
relPath := strings.TrimPrefix(path, dir)
89+
switch relPath {
90+
case "", "/test.zip", "/test.tar": // ignore
91+
case "/test":
92+
stat, err := os.Stat(path)
93+
assert.NoError(t, err, "failed to stat path %q", path)
94+
assert.True(t, stat.IsDir(), "expected path %q to be a directory")
95+
if checkModePerm {
96+
assert.Equal(t, fs.ModePerm&0o755, stat.Mode().Perm(), "expected mode 0755 on directory")
97+
}
98+
assert.Equal(t, archiveRefTime(t).UTC(), stat.ModTime().UTC(), "unexpected modtime of %q", path)
99+
case "/test/hello.txt":
100+
stat, err := os.Stat(path)
101+
assert.NoError(t, err, "failed to stat path %q", path)
102+
assert.False(t, stat.IsDir(), "expected path %q to be a file")
103+
if checkModePerm {
104+
assert.Equal(t, fs.ModePerm&0o644, stat.Mode().Perm(), "expected mode 0644 on file")
105+
}
106+
bs, err := os.ReadFile(path)
107+
assert.NoError(t, err, "failed to read file %q", path)
108+
assert.Equal(t, "hello", string(bs), "unexpected content in file %q", path)
109+
case "/test/dir":
110+
stat, err := os.Stat(path)
111+
assert.NoError(t, err, "failed to stat path %q", path)
112+
assert.True(t, stat.IsDir(), "expected path %q to be a directory")
113+
if checkModePerm {
114+
assert.Equal(t, fs.ModePerm&0o755, stat.Mode().Perm(), "expected mode 0755 on directory")
115+
}
116+
case "/test/dir/world.txt":
117+
stat, err := os.Stat(path)
118+
assert.NoError(t, err, "failed to stat path %q", path)
119+
assert.False(t, stat.IsDir(), "expected path %q to be a file")
120+
if checkModePerm {
121+
assert.Equal(t, fs.ModePerm&0o644, stat.Mode().Perm(), "expected mode 0644 on file")
122+
}
123+
bs, err := os.ReadFile(path)
124+
assert.NoError(t, err, "failed to read file %q", path)
125+
assert.Equal(t, "world", string(bs), "unexpected content in file %q", path)
126+
default:
127+
assert.Fail(t, "unexpected path", relPath)
128+
}
129+
130+
return nil
131+
})
132+
}
133+
134+
func assertSampleTarFile(t *testing.T, tarBytes []byte) {
135+
t.Helper()
136+
137+
tr := tar.NewReader(bytes.NewReader(tarBytes))
138+
for {
139+
hdr, err := tr.Next()
140+
if err != nil {
141+
if err == io.EOF {
142+
return
143+
}
144+
require.NoError(t, err)
145+
}
146+
147+
// Note: ignoring timezones here.
148+
require.Equal(t, archiveRefTime(t).UTC(), hdr.ModTime.UTC())
149+
150+
switch hdr.Name {
151+
case "test/":
152+
require.Equal(t, hdr.Typeflag, byte(tar.TypeDir))
153+
case "test/hello.txt":
154+
require.Equal(t, hdr.Typeflag, byte(tar.TypeReg))
155+
bs, err := io.ReadAll(tr)
156+
if err != nil && !xerrors.Is(err, io.EOF) {
157+
require.NoError(t, err)
158+
}
159+
require.Equal(t, "hello", string(bs))
160+
case "test/dir/":
161+
require.Equal(t, hdr.Typeflag, byte(tar.TypeDir))
162+
case "test/dir/world.txt":
163+
require.Equal(t, hdr.Typeflag, byte(tar.TypeReg))
164+
bs, err := io.ReadAll(tr)
165+
if err != nil && !xerrors.Is(err, io.EOF) {
166+
require.NoError(t, err)
167+
}
168+
require.Equal(t, "world", string(bs))
169+
default:
170+
require.Failf(t, "unexpected file in tar", hdr.Name)
171+
}
172+
}
173+
}
174+
175+
func assertSampleZipFile(t *testing.T, zipBytes []byte) {
176+
t.Helper()
177+
178+
zr, err := zip.NewReader(bytes.NewReader(zipBytes), int64(len(zipBytes)))
179+
require.NoError(t, err)
180+
181+
for _, f := range zr.File {
182+
// Note: ignoring timezones here.
183+
require.Equal(t, archiveRefTime(t).UTC(), f.Modified.UTC())
184+
switch f.Name {
185+
case "test/", "test/dir/":
186+
// directory
187+
case "test/hello.txt":
188+
rc, err := f.Open()
189+
require.NoError(t, err)
190+
bs, err := io.ReadAll(rc)
191+
_ = rc.Close()
192+
require.NoError(t, err)
193+
require.Equal(t, "hello", string(bs))
194+
case "test/dir/world.txt":
195+
rc, err := f.Open()
196+
require.NoError(t, err)
197+
bs, err := io.ReadAll(rc)
198+
_ = rc.Close()
199+
require.NoError(t, err)
200+
require.Equal(t, "world", string(bs))
201+
default:
202+
require.Failf(t, "unexpected file in zip", f.Name)
203+
}
204+
}
205+
}
206+
207+
// archiveRefTime is the Go reference time. The contents of the sample tar and zip files
208+
// in testdata/ all have their modtimes set to the below in some timezone.
209+
func archiveRefTime(t *testing.T) time.Time {
210+
locMST, err := time.LoadLocation("MST")
211+
require.NoError(t, err, "failed to load MST timezone")
212+
return time.Date(2006, 1, 2, 3, 4, 5, 0, locMST)
213+
}

coderd/testdata/test.tar

10 KB
Binary file not shown.

coderd/testdata/test.zip

636 Bytes
Binary file not shown.

0 commit comments

Comments
 (0)