Fix lazy-write of tarballs (#2016)

Author: eandre

pkg/dockerbuild/dockerbuild.go

@@ -1,8 +1,6 @@
 package dockerbuild
 
 import (
-	"archive/tar"
-	"bytes"
 	"context"
 	"encoding/json"
 	"io"
@@ -154,18 +152,7 @@ func resolveBaseImage(ctx context.Context, baseImgTag string, overrideBaseImage
 }
 
 func buildImageFilesystem(ctx context.Context, spec *ImageSpec, cfg *ImageBuildConfig) (opener tarball.Opener, err error) {
-	tarFile, err := os.CreateTemp("", "docker-img")
-	if err != nil {
-		return nil, errors.Wrap(err, "mktemp")
-	}
-	defer func() {
-		if e := tarFile.Close(); e != nil && err == nil {
-			err = errors.Wrap(e, "close docker-img file")
-		}
-	}()
-
-	tw := tar.NewWriter(tarFile)
-	tc := newTarCopier(tw, setFileTimes(cfg.BuildTime))
+	tc := newTarCopier(setFileTimes(cfg.BuildTime))
 
 	// Bundle the source code, if requested.
 	if bundle, ok := spec.BundleSource.Get(); ok {
@@ -199,24 +186,17 @@ func buildImageFilesystem(ctx context.Context, spec *ImageSpec, cfg *ImageBuildC
 		if caCertsDest == "" {
 			caCertsDest = DefaultCACertsPath
 		}
-		if err := addCACerts(ctx, tw, caCertsDest); err != nil {
+		if err := addCACerts(ctx, tc, caCertsDest); err != nil {
 			return nil, errors.Wrap(err, "add ca certs")
 		}
 	}
 
-	if err := tw.Close(); err != nil {
-		return nil, errors.Wrap(err, "complete tar")
-	}
-
-	opener = func() (io.ReadCloser, error) {
-		return os.Open(tarFile.Name())
-	}
-	return opener, nil
+	return tc.Opener(), nil
 }
 
 func writeExtraFiles(tc *tarCopier, files map[ImagePath][]byte) error {
 	for path, data := range files {
-		if err := tc.WriteFile(path.String(), 0644, data); err != nil {
+		if err := tc.WriteFile(path, 0644, data); err != nil {
 			return errors.Wrap(err, "write image data")
 		}
 	}
@@ -254,7 +234,7 @@ func setupSupervisor(tc *tarCopier, spec *ImageSpec, cfg *ImageBuildConfig) erro
 		if err != nil {
 			return errors.Wrap(err, "marshal supervisor config")
 		}
-		if err := tc.WriteFile(string(super.ConfigPath), 0644, data); err != nil {
+		if err := tc.WriteFile(super.ConfigPath, 0644, data); err != nil {
 			return errors.Wrap(err, "write supervisor config")
 		}
 	}
@@ -290,7 +270,7 @@ func writeBuildInfo(tc *tarCopier, spec BuildInfoSpec) error {
 		return errors.Wrap(err, "marshal build info")
 	}
 
-	err = tc.WriteFile(string(spec.InfoPath), 0644, info)
+	err = tc.WriteFile(spec.InfoPath, 0644, info)
 	return errors.Wrap(err, "write build info")
 }
 
@@ -312,7 +292,7 @@ func tryFetch(ctx context.Context, url string) (*http.Response, error) {
 	return resp, nil
 }
 
-func addCACerts(ctx context.Context, tw *tar.Writer, dest ImagePath) error {
+func addCACerts(ctx context.Context, tc *tarCopier, dest ImagePath) error {
 	const (
 		encoreCachedRootCerts = "https://api.encore.cloud/artifacts/build/root-certs"
 		curlCACertStore       = "https://curl.se/ca/cacert.pem"
@@ -333,34 +313,14 @@ func addCACerts(ctx context.Context, tw *tar.Writer, dest ImagePath) error {
 	}
 	defer func() { _ = resp.Body.Close() }()
 
-	// We need to populate the body of the tar file before writing the contents.
-	// Use the content length if it was provided. Otherwise, read the whole response
-	// into memory and use its length.
-	var body io.Reader = resp.Body
-	size := resp.ContentLength
-	if size < 0 {
-		// Unknown body; read the whole response into memory
-		data, err := io.ReadAll(resp.Body)
-		if err != nil {
-			return errors.Wrap(err, "read cert data")
-		}
-		size = int64(len(data))
-		body = bytes.NewReader(data)
+	data, err := io.ReadAll(resp.Body)
+	if err != nil {
+		return errors.Wrap(err, "read cert data")
 	}
 
 	// Add the file
-	err = tw.WriteHeader(&tar.Header{
-		Typeflag: tar.TypeReg,
-		Name:     string(dest),
-		Size:     size,
-	})
-	if err != nil {
-		return errors.Wrap(err, "create cert file")
-	}
-	if _, err := io.Copy(tw, body); err != nil {
-		return errors.Wrap(err, "write cert data")
-	}
-	return nil
+	err = tc.WriteFile(dest, 0644, data)
+	return err
 }
 
 type envMap map[string]string

pkg/dockerbuild/tarcopy.go

@@ -2,6 +2,8 @@ package dockerbuild
 
 import (
 	"archive/tar"
+	"bytes"
+	"fmt"
 	"io"
 	"io/fs"
 	"os"
@@ -11,21 +13,24 @@ import (
 	"strings"
 	"time"
 
-	"encr.dev/pkg/xos"
-	"encr.dev/v2/compiler/build"
 	"github.com/cockroachdb/errors"
+	"github.com/google/go-containerregistry/pkg/v1/tarball"
 	"github.com/rs/zerolog/log"
+
+	"encr.dev/pkg/option"
+	"encr.dev/pkg/tarstream"
+	"encr.dev/pkg/xos"
+	"encr.dev/v2/compiler/build"
 )
 
 type tarCopier struct {
 	fileTimes *time.Time
-	tw        *tar.Writer
+	entries   []*tarEntry
 	seenDirs  map[ImagePath]bool
 }
 
-func newTarCopier(tw *tar.Writer, opts ...tarCopyOption) *tarCopier {
+func newTarCopier(opts ...tarCopyOption) *tarCopier {
 	tc := &tarCopier{
-		tw:       tw,
 		seenDirs: make(map[ImagePath]bool),
 	}
 	for _, opt := range opts {
@@ -260,12 +265,12 @@ func (tc *tarCopier) MkdirAll(dstPath ImagePath, mode fs.FileMode) (err error) {
 			header := &tar.Header{
 				Typeflag: tar.TypeDir,
 				ModTime:  modTime,
-				Name:     (dstPath + "/").String(), // from [archive/tar.FileInfoHeader]
+				Name:     tarHeaderName(dstPath, true),
 				Mode:     int64(mode.Perm()),
 			}
-			if err := tc.tw.WriteHeader(header); err != nil {
-				return errors.Wrap(err, "write tar header")
-			}
+			tc.entries = append(tc.entries, &tarEntry{
+				header: header,
+			})
 			tc.seenDirs[dstPath] = true
 		}
 
@@ -292,10 +297,9 @@ func (tc *tarCopier) CopyFile(dstPath ImagePath, srcPath HostPath, fi fs.FileInf
 		header.Mode = 0755
 	}
 
-	header.Name = filepath.ToSlash(dstPath.String())
-	if err := tc.tw.WriteHeader(header); err != nil {
-		return errors.Wrap(err, "write tar header")
-	}
+	header.Name = tarHeaderName(dstPath, fi.IsDir())
+	entry := &tarEntry{header: header}
+	tc.entries = append(tc.entries, entry)
 
 	if fi.IsDir() {
 		tc.seenDirs[dstPath] = true
@@ -304,28 +308,15 @@ func (tc *tarCopier) CopyFile(dstPath ImagePath, srcPath HostPath, fi fs.FileInf
 
 	// If this is not a symlink, write the file.
 	if (fi.Mode() & fs.ModeSymlink) != fs.ModeSymlink {
-		// Write the file
-		f, err := os.Open(srcPath.String())
-		if err != nil {
-			return errors.Wrap(err, "open file")
-		}
-		defer func() {
-			if closeErr := f.Close(); err == nil {
-				err = errors.Wrap(closeErr, "close file")
-			}
-		}()
-
-		if _, err = io.Copy(tc.tw, f); err != nil {
-			return errors.Wrap(err, "copy file")
-		}
+		entry.hostPath = option.Some(srcPath)
 	}
 
 	return nil
 }
 
-func (tc *tarCopier) WriteFile(dstPath string, mode fs.FileMode, data []byte) (err error) {
+func (tc *tarCopier) WriteFile(dstPath ImagePath, mode fs.FileMode, data []byte) (err error) {
 	header := &tar.Header{
-		Name:     dstPath,
+		Name:     tarHeaderName(dstPath, false),
 		Typeflag: tar.TypeReg,
 		Mode:     int64(mode.Perm()),
 		Size:     int64(len(data)),
@@ -337,11 +328,84 @@ func (tc *tarCopier) WriteFile(dstPath string, mode fs.FileMode, data []byte) (e
 		header.ChangeTime = t
 	}
 
-	header.Name = filepath.ToSlash(dstPath)
-	if err := tc.tw.WriteHeader(header); err != nil {
-		return errors.Wrap(err, "write tar header")
+	tc.entries = append(tc.entries, &tarEntry{
+		header: header,
+		data:   option.Some(data),
+	})
+	return nil
+}
+
+type tarEntry struct {
+	header *tar.Header
+
+	data     option.Option[[]byte]
+	hostPath option.Option[HostPath]
+}
+
+func (tc *tarCopier) Opener() tarball.Opener {
+	errThunk := func(err error) tarball.Opener {
+		return func() (io.ReadCloser, error) {
+			return nil, err
+		}
+	}
+
+	var dvecs []tarstream.Datavec
+	for _, e := range tc.entries {
+		log.Info().Str("file", e.header.Name).Interface("header", e.header).Msg("processing file")
+		// create buffer to write tar header to
+		buf := new(bytes.Buffer)
+		tw := tar.NewWriter(buf)
+
+		// write tar header to buffer
+		if err := tw.WriteHeader(e.header); err != nil {
+			return errThunk(errors.Wrap(err, fmt.Sprintf("writing header %v", e)))
+		}
+
+		memv := tarstream.MemVec{
+			Data: buf.Bytes(),
+		}
+
+		// add the tar header mem buffer to the tarvec
+		dvecs = append(dvecs, memv)
+
+		var dataEntry tarstream.Datavec
+		if hostPath, ok := e.hostPath.Get(); ok {
+			fi := e.header.FileInfo()
+			dataEntry = &tarstream.PathVec{
+				Path: hostPath.String(),
+				Info: fi,
+			}
+		} else if data, ok := e.data.Get(); ok {
+			dataEntry = tarstream.MemVec{Data: data}
+		}
+
+		if dataEntry != nil {
+			// add the file path info to the tarvec
+			dvecs = append(dvecs, dataEntry)
+
+			// tar requires file entries to be padded out to 512 bytes.
+			if !e.header.FileInfo().IsDir() {
+				if size := dataEntry.GetSize(); size%512 != 0 {
+					padv := tarstream.PadVec{
+						Size: 512 - (size % 512),
+					}
+					dvecs = append(dvecs, padv)
+				}
+			}
+		}
 	}
 
-	_, err = tc.tw.Write(data)
-	return errors.Wrap(err, "write file")
+	tv := tarstream.NewTarVec(dvecs)
+	return func() (io.ReadCloser, error) {
+		tv2 := tv.Clone()
+		return tv2, nil
+	}
+}
+
+func tarHeaderName(p ImagePath, isDir bool) string {
+	name := strings.TrimPrefix(filepath.ToSlash(p.String()), "/")
+	if isDir {
+		name += "/"
+	}
+	return name
 }

pkg/tarstream/LICENSE

@@ -0,0 +1,21 @@
+MIT License
+
+Copyright (c) 2016 Ben McClelland
+
+Permission is hereby granted, free of charge, to any person obtaining a copy
+of this software and associated documentation files (the "Software"), to deal
+in the Software without restriction, including without limitation the rights
+to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
+copies of the Software, and to permit persons to whom the Software is
+furnished to do so, subject to the following conditions:
+
+The above copyright notice and this permission notice shall be included in all
+copies or substantial portions of the Software.
+
+THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
+AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
+OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE
+SOFTWARE.