diff --git a/e2e/connect_repo_test.go b/e2e/connect_repo_test.go index 6370821d9..24a10976d 100644 --- a/e2e/connect_repo_test.go +++ b/e2e/connect_repo_test.go @@ -2,6 +2,7 @@ package e2e import ( "fmt" + "os" "strings" "testing" @@ -22,12 +23,15 @@ func TestConnectRepo(t *testing.T) { // test using user's personal organization org := user.Username() + tarball, err := os.ReadFile("../testdata/github.tar.gz") + require.NoError(t, err) + // create an otf daemon with a fake github backend, ready to sign in a user, // serve up a repo and its contents via tarball. daemon := &daemon{} daemon.withGithubUser(user) daemon.withGithubRepo(repo) - daemon.withGithubTarball(otf.NewTestTarball(t, `resource "null_resource" "e2e" {}`)) + daemon.withGithubTarball(tarball) hostname := daemon.start(t) url := "https://" + hostname @@ -36,7 +40,7 @@ func TestConnectRepo(t *testing.T) { orgSelector := fmt.Sprintf("#item-organization-%s a", org) - err := chromedp.Run(ctx, chromedp.Tasks{ + err = chromedp.Run(ctx, chromedp.Tasks{ chromedp.Navigate(url), // login chromedp.Click(".login-button-github", chromedp.NodeVisible), diff --git a/github.go b/github.go index 621e8c4c4..475d67290 100644 --- a/github.go +++ b/github.go @@ -1,13 +1,12 @@ package otf import ( - "bytes" - "compress/gzip" "context" "crypto/tls" "fmt" - "io" "net/http" + "os" + "path" "strings" "github.com/google/go-github/v41/github" @@ -206,18 +205,28 @@ func (g *githubProvider) GetRepoTarball(ctx context.Context, repo *VCSRepo) ([]b if err != nil { return nil, err } + defer resp.Body.Close() - // convert .tar to .tar.gz - tarball := new(bytes.Buffer) - compressor := gzip.NewWriter(tarball) - if _, err := io.Copy(compressor, resp.Body); err != nil { + // github tarball contains a parent directory of the format + // --. We need a tarball without this parent directory, + // so we untar it to a temp dir, then tar it up the contents of the parent + // directory. + untarpath, err := os.MkdirTemp("", fmt.Sprintf("github-%s-%s-*", repo.Owner(), repo.Repo())) + if err != nil { return nil, err } - if err := compressor.Close(); err != nil { + if err := Unpack(resp.Body, untarpath); err != nil { return nil, err } - - return tarball.Bytes(), nil + contents, err := os.ReadDir(untarpath) + if err != nil { + return nil, err + } + if len(contents) != 1 { + return nil, fmt.Errorf("malformed tarball archive") + } + parentDir := path.Join(untarpath, contents[0].Name()) + return Pack(parentDir) } func NewGithubEnterpriseClient(hostname string, httpClient *http.Client) (*github.Client, error) { diff --git a/github_test.go b/github_test.go index 4ccc6ad2d..2b97b2545 100644 --- a/github_test.go +++ b/github_test.go @@ -1,8 +1,10 @@ package otf import ( + "bytes" "context" "net/url" + "os" "testing" "github.com/stretchr/testify/assert" @@ -48,7 +50,8 @@ func TestGithub_GetUser(t *testing.T) { func TestGithub_GetRepoTarball(t *testing.T) { ctx := context.Background() - want := NewTestTarball(t, `file1 contents`, `file2 contents`) + want, err := os.ReadFile("testdata/github.tar.gz") + require.NoError(t, err) srv := NewTestGithubServer(t, WithGithubRepo(&Repo{Identifier: "acme/terraform", Branch: "master"}), @@ -76,5 +79,7 @@ func TestGithub_GetRepoTarball(t *testing.T) { }) require.NoError(t, err) - assert.Equal(t, compress(t, want), got) + dst := t.TempDir() + err = Unpack(bytes.NewReader(got), dst) + require.NoError(t, err) } diff --git a/test_helpers.go b/test_helpers.go index c88e3a355..afef8d0e4 100644 --- a/test_helpers.go +++ b/test_helpers.go @@ -4,7 +4,6 @@ import ( "archive/tar" "bytes" "compress/gzip" - "io" "testing" "github.com/google/uuid" @@ -83,12 +82,13 @@ func NewTestRepo() *Repo { } } -// NewTestTarball creates a tarball (.tar) consisting of files respectively populated with the +// NewTestTarball creates a tarball (.tar.gz) consisting of files respectively populated with the // given contents. The files are assigned random names with the terraform file // extension appended (.tf) func NewTestTarball(t *testing.T, contents ...string) []byte { var buf bytes.Buffer - tw := tar.NewWriter(&buf) + gw := gzip.NewWriter(&buf) + tw := tar.NewWriter(gw) for _, body := range contents { header := &tar.Header{ @@ -103,21 +103,10 @@ func NewTestTarball(t *testing.T, contents ...string) []byte { require.NoError(t, err) } - tw.Close() - return buf.Bytes() -} - -// NewTestTarGZ wraps NewTestTarball, creating a .tar.gz instead. -func NewTestTarGZ(t *testing.T, contents ...string) []byte { - return compress(t, NewTestTarball(t, contents...)) -} - -// compress runs gzip compression on the input bytes -func compress(t *testing.T, b []byte) []byte { - var buf bytes.Buffer - compressor := gzip.NewWriter(&buf) - _, err := io.Copy(compressor, bytes.NewReader(b)) + err := tw.Close() require.NoError(t, err) - compressor.Close() + err = gw.Close() + require.NoError(t, err) + return buf.Bytes() } diff --git a/testdata/github.tar.gz b/testdata/github.tar.gz new file mode 100644 index 000000000..f8c30f0ff Binary files /dev/null and b/testdata/github.tar.gz differ diff --git a/testdata/pack/dir/file b/testdata/pack/dir/file new file mode 100644 index 000000000..10ef80562 --- /dev/null +++ b/testdata/pack/dir/file @@ -0,0 +1 @@ +morestuff diff --git a/testdata/pack/dir/symlink b/testdata/pack/dir/symlink new file mode 120000 index 000000000..74bb61d1b --- /dev/null +++ b/testdata/pack/dir/symlink @@ -0,0 +1 @@ +../file \ No newline at end of file diff --git a/testdata/pack/file b/testdata/pack/file new file mode 100644 index 000000000..f2e41136e --- /dev/null +++ b/testdata/pack/file @@ -0,0 +1 @@ +stuff diff --git a/unpack.go b/unpack.go index a2da269e1..030e67463 100644 --- a/unpack.go +++ b/unpack.go @@ -2,6 +2,7 @@ package otf import ( "archive/tar" + "bytes" "compress/gzip" "fmt" "io" @@ -11,21 +12,15 @@ import ( // Unpack a .tar.gz byte stream to a directory func Unpack(r io.Reader, dst string) error { - // Decompress as we read. - // - // TODO: rename decompressor - decompressed, err := gzip.NewReader(r) + gr, err := gzip.NewReader(r) if err != nil { return fmt.Errorf("failed to decompress archive: %w", err) } - // TODO: close decompressed + tr := tar.NewReader(gr) - // Untar as we read. - untar := tar.NewReader(decompressed) - - // Unpackage all the contents into the directory. + // Unpackage contents to the destination directory. for { - header, err := untar.Next() + header, err := tr.Next() if err == io.EOF { break } @@ -40,7 +35,7 @@ func Unpack(r io.Reader, dst string) error { } path = filepath.Join(dst, path) - // Make the directories to the path. + // Ensure path's parent directories exist dir := filepath.Dir(path) if err := os.MkdirAll(dir, 0o755); err != nil { return fmt.Errorf("failed to create directory: %w", err) @@ -56,11 +51,8 @@ func Unpack(r io.Reader, dst string) error { } // Only unpack regular files from this point on. - if header.Typeflag == tar.TypeDir { + if header.Typeflag != tar.TypeReg { continue - } else if header.Typeflag != tar.TypeReg && header.Typeflag != tar.TypeRegA { - return fmt.Errorf("failed creating %q: unsupported type %c", path, - header.Typeflag) } // Open a handle to the destination. @@ -80,7 +72,8 @@ func Unpack(r io.Reader, dst string) error { } // Copy the contents. - _, err = io.Copy(fh, untar) + _, err = io.Copy(fh, tr) + // TODO: why are we closing it here? fh.Close() if err != nil { return fmt.Errorf("failed to copy file %q: %w", path, err) @@ -95,3 +88,69 @@ func Unpack(r io.Reader, dst string) error { } return nil } + +// Pack a directory into tarball (.tar.gz) and return its contents +func Pack(src string) ([]byte, error) { + var buf bytes.Buffer + gw := gzip.NewWriter(&buf) + tw := tar.NewWriter(gw) + + err := filepath.Walk(src, func(path string, info os.FileInfo, err error) error { + if err != nil { + return nil + } + + target := info.Name() + + if info.Mode()&os.ModeSymlink == os.ModeSymlink { + target, err = os.Readlink(path) + if err != nil { + return nil + } + } + + hdr, err := tar.FileInfoHeader(info, target) + if err != nil { + return nil + } + name, err := filepath.Rel(src, path) + if err != nil { + return nil + } + hdr.Name = name + + if err := tw.WriteHeader(hdr); err != nil { + return err + } + + // Unless it's a normal file there's nothing more to be done + if hdr.Typeflag != tar.TypeReg { + return nil + } + + f, err := os.Open(path) + if err != nil { + return err + } + defer f.Close() + + _, err = io.Copy(tw, f) + if err != nil { + return err + } + return nil + }) + if err != nil { + return nil, err + } + + // Write tar and gzip headers + if err := tw.Close(); err != nil { + return nil, err + } + if err := gw.Close(); err != nil { + return nil, err + } + + return buf.Bytes(), nil +} diff --git a/unpack_test.go b/unpack_test.go index c47bec7ce..ff47cc91a 100644 --- a/unpack_test.go +++ b/unpack_test.go @@ -1,7 +1,9 @@ package otf import ( + "bytes" "os" + "path" "path/filepath" "testing" @@ -32,3 +34,32 @@ func TestUnpack(t *testing.T) { "file", }, got) } + +// TestUnpack_Github tests unpacking a Github archive of a git repository. Their +// archive is somewhat idiosyncratic in that it uses the PAX format, with +// key-values embedded in the tar file. It's tripped up the unpack code before, +// so putting this test in here to prevent a regression. +func TestUnpack_Github(t *testing.T) { + tarball, err := os.Open("testdata/github.tar.gz") + require.NoError(t, err) + + require.NoError(t, Unpack(tarball, t.TempDir())) +} + +func TestPack(t *testing.T) { + tarball, err := Pack("testdata/pack") + require.NoError(t, err) + + // unpack tarball to test its contents + dst := t.TempDir() + err = Unpack(bytes.NewReader(tarball), dst) + require.NoError(t, err) + + assert.FileExists(t, path.Join(dst, "file")) + assert.DirExists(t, path.Join(dst, "dir")) + symlink, err := os.Readlink(path.Join(dst, "dir", "symlink")) + if assert.NoError(t, err) { + assert.Equal(t, "../file", symlink) + } + assert.FileExists(t, path.Join(dst, "dir", "file")) +}