Make ADD untar tar.gz files (#309)

* Make ADD untar tar files

* Refactor tests

* Add test

Author: Yiran Wang

lib/builder/step/add_copy_step.go

@@ -58,12 +58,13 @@ type addCopyStep struct {
 	toPath        string
 	chown         string
 	preserveOwner bool
+	isAdd         bool
 }
 
 // newAddCopyStep returns a BuildStep from given arguments.
 func newAddCopyStep(
 	directive Directive, args, chown, fromStage string,
-	fromPaths []string, toPath string, commit, preserveOwner bool) (*addCopyStep, error) {
+	fromPaths []string, toPath string, commit, preserveOwner, isAdd bool) (*addCopyStep, error) {
 
 	toPath = strings.Trim(toPath, "\"'")
 	for i := range fromPaths {
@@ -80,6 +81,7 @@ func newAddCopyStep(
 		toPath:        toPath,
 		chown:         chown,
 		preserveOwner: preserveOwner,
+		isAdd:         isAdd,
 	}, nil
 }
 
@@ -136,14 +138,16 @@ func (s *addCopyStep) Execute(ctx *context.BuildContext, modifyFS bool) (err err
 
 	internal := s.fromStage != ""
 	blacklist := append(pathutils.DefaultBlacklist, ctx.ImageStore.RootDir)
-	copyOp, err := snapshot.NewCopyOperation(
-		relPaths, sourceRoot, s.workingDir, s.toPath, s.chown, blacklist, internal, s.preserveOwner)
+	copyOp, err := snapshot.NewCopyOperation(relPaths, sourceRoot, s.workingDir, s.toPath, s.chown,
+		s.preserveOwner, internal, blacklist, s.isAdd)
 	if err != nil {
 		return fmt.Errorf("invalid copy operation: %s", err)
 	}
 
 	ctx.CopyOps = append(ctx.CopyOps, copyOp)
 	if modifyFS {
+		// Note: We still want to keep this copyop in the list, just in case there
+		// is no RUN afterwards, and layer needs to be created from copyops.
 		return copyOp.Execute()
 	}
 	return nil

lib/builder/step/add_copy_step_test.go

@@ -24,21 +24,21 @@ func TestContextDirs(t *testing.T) {
 	require := require.New(t)
 
 	srcs := []string{}
-	ac, err := newAddCopyStep(Copy, "", "", "", srcs, "", false, false)
+	ac, err := newAddCopyStep(Copy, "", "", "", srcs, "", false, false, false)
 	require.NoError(err)
 	stage, paths := ac.ContextDirs()
 	require.Equal("", stage)
 	require.Len(paths, 0)
 
 	srcs = []string{"src"}
-	ac, err = newAddCopyStep(Copy, "", "", "", srcs, "", false, false)
+	ac, err = newAddCopyStep(Copy, "", "", "", srcs, "", false, false, false)
 	require.NoError(err)
 	stage, paths = ac.ContextDirs()
 	require.Equal("", stage)
 	require.Len(paths, 0)
 
 	srcs = []string{"src"}
-	ac, err = newAddCopyStep(Copy, "", "", "stage", srcs, "", false, false)
+	ac, err = newAddCopyStep(Copy, "", "", "stage", srcs, "", false, false, false)
 	require.NoError(err)
 	stage, paths = ac.ContextDirs()
 	require.Equal("stage", stage)
@@ -48,7 +48,7 @@ func TestContextDirs(t *testing.T) {
 func TestTrimmingPaths(t *testing.T) {
 	require := require.New(t)
 
-	ac, err := newAddCopyStep(Copy, "", "", "", []string{"\"/from/path\""}, "\"/to/path\"", false, false)
+	ac, err := newAddCopyStep(Copy, "", "", "", []string{"\"/from/path\""}, "\"/to/path\"", false, false, false)
 	require.NoError(err)
 
 	require.Equal("/from/path", ac.fromPaths[0])

lib/builder/step/add_step.go

@@ -23,7 +23,8 @@ type AddStep struct {
 
 // NewAddStep creates a new AddStep
 func NewAddStep(args, chown string, fromPaths []string, toPath string, commit, preserverOwner bool) (*AddStep, error) {
-	s, err := newAddCopyStep(Add, args, chown, "", fromPaths, toPath, commit, preserverOwner)
+	s, err := newAddCopyStep(
+		Add, args, chown, "", fromPaths, toPath, commit, preserverOwner, true)
 	if err != nil {
 		return nil, fmt.Errorf("new add/copy step: %s", err)
 	}

lib/builder/step/copy_step.go

@@ -26,7 +26,8 @@ func NewCopyStep(
 	args, chown, fromStage string, fromPaths []string, toPath string, commit, preserveOwner bool,
 ) (*CopyStep, error) {
 
-	s, err := newAddCopyStep(Copy, args, chown, fromStage, fromPaths, toPath, commit, preserveOwner)
+	s, err := newAddCopyStep(
+		Copy, args, chown, fromStage, fromPaths, toPath, commit, preserveOwner, false)
 	if err != nil {
 		return nil, fmt.Errorf("new add/copy step: %s", err)
 	}

lib/snapshot/copy_op.go

@@ -22,10 +22,10 @@ import (
 
 	"github.com/uber/makisu/lib/fileio"
 	"github.com/uber/makisu/lib/pathutils"
+	"github.com/uber/makisu/lib/tario"
 	"github.com/uber/makisu/lib/utils"
 )
 
-// CopyOperation defines a copy operation that occurred to generate a layer from.
 type CopyOperation struct {
 	srcRoot       string
 	srcs          []string
@@ -34,16 +34,20 @@ type CopyOperation struct {
 	gid           int
 	preserveOwner bool
 
-	blacklist []string
 	// Indicates if the copy op is used for copying from previous stages.
-	internal bool
+	// Blacklist is ignored in that case.
+	internal  bool
+	blacklist []string
+
+	// Set to true if the copy op was created by ADD step.
+	// Some functionalities are only available to ADD.
+	fromAdd bool
 }
 
 // NewCopyOperation initializes and validates a CopyOperation. Use "internal" to
 // specify if the copy op is used for copying from previous stages.
-func NewCopyOperation(
-	srcs []string, srcRoot, workDir, dst, chown string,
-	blacklist []string, internal, preserveOwner bool) (*CopyOperation, error) {
+func NewCopyOperation(srcs []string, srcRoot, workDir, dst, chown string,
+	preserveOwner, internal bool, blacklist []string, fromAdd bool) (*CopyOperation, error) {
 
 	if err := checkCopyParams(srcs, workDir, dst); err != nil {
 		return nil, fmt.Errorf("check copy param: %s", err)
@@ -70,6 +74,7 @@ func NewCopyOperation(
 		preserveOwner: preserveOwner,
 		blacklist:     blacklist,
 		internal:      internal,
+		fromAdd:       fromAdd,
 	}, nil
 }
 
@@ -86,6 +91,32 @@ func (c *CopyOperation) Execute() error {
 		if err != nil {
 			return fmt.Errorf("lstat %s: %s", src, err)
 		}
+
+		if strings.HasSuffix(src, ".tar.gz") && c.fromAdd {
+			// Special feature for ADD - Extract tar.gz into dst directory.
+			// If dst is an existing directory, untar.
+			// If dst doesn't exist, create it with root.
+			// If dst exists and is not a directory, fail.
+			reader, err := os.Open(src)
+			if err != nil {
+				return fmt.Errorf("open tar file: %s", err)
+			}
+			defer reader.Close()
+
+			if err := os.MkdirAll(c.dst, 0755); err != nil {
+				return fmt.Errorf("create/validate untar dst: %s", err)
+			}
+
+			gzipReader, err := tario.NewGzipReader(reader)
+			if err != nil {
+				return fmt.Errorf("unzip gz %s: %s", src, err)
+			}
+			if err := tario.Untar(gzipReader, c.dst); err != nil {
+				return fmt.Errorf("untar %s to dst %s: %s", src, c.dst, err)
+			}
+			return nil
+		}
+
 		var copier fileio.Copier
 		if c.internal {
 			copier = fileio.NewInternalCopier()
@@ -115,7 +146,6 @@ func (c *CopyOperation) Execute() error {
 					return fmt.Errorf("copy file %s to dir %s: %s", src, targetFilePath, err)
 				}
 			}
-
 		} else {
 			// File to file
 			if c.preserveOwner {