enable archive flag (#297)

* enable archive flag

* address comments and add an integration test

* address lint issue and check file permission

lib/builder/step/add_copy_step.go

@@ -53,16 +53,17 @@ import (
 type addCopyStep struct {
 	*baseStep
 
-	fromStage string
-	fromPaths []string
-	toPath    string
-	chown     string
+	fromStage     string
+	fromPaths     []string
+	toPath        string
+	chown         string
+	preserveOwner bool
 }
 
 // newAddCopyStep returns a BuildStep from given arguments.
 func newAddCopyStep(
 	directive Directive, args, chown, fromStage string,
-	fromPaths []string, toPath string, commit bool) (*addCopyStep, error) {
+	fromPaths []string, toPath string, commit, preserveOwner bool) (*addCopyStep, error) {
 
 	toPath = strings.Trim(toPath, "\"'")
 	for i := range fromPaths {
@@ -73,11 +74,12 @@ func newAddCopyStep(
 		return nil, fmt.Errorf("copying multiple source files, target must be a directory ending in \"/\"")
 	}
 	return &addCopyStep{
-		baseStep:  newBaseStep(directive, args, commit),
-		fromStage: fromStage,
-		fromPaths: fromPaths,
-		toPath:    toPath,
-		chown:     chown,
+		baseStep:      newBaseStep(directive, args, commit),
+		fromStage:     fromStage,
+		fromPaths:     fromPaths,
+		toPath:        toPath,
+		chown:         chown,
+		preserveOwner: preserveOwner,
 	}, nil
 }
 
@@ -135,7 +137,7 @@ 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)
+		relPaths, sourceRoot, s.workingDir, s.toPath, s.chown, blacklist, internal, s.preserveOwner)
 	if err != nil {
 		return fmt.Errorf("invalid copy operation: %s", err)
 	}

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)
+	ac, err := newAddCopyStep(Copy, "", "", "", srcs, "", 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)
+	ac, err = newAddCopyStep(Copy, "", "", "", srcs, "", 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)
+	ac, err = newAddCopyStep(Copy, "", "", "stage", srcs, "", 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)
+	ac, err := newAddCopyStep(Copy, "", "", "", []string{"\"/from/path\""}, "\"/to/path\"", false, false)
 	require.NoError(err)
 
 	require.Equal("/from/path", ac.fromPaths[0])

lib/builder/step/add_step.go

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

lib/builder/step/copy_step.go

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

lib/builder/step/copy_step_test.go

@@ -44,7 +44,7 @@ func abs(path string) string {
 func TestNewCopyStep(t *testing.T) {
 	require := require.New(t)
 
-	_, err := NewCopyStep("", validChown, "", []string{"src", "src"}, "dst", false)
+	_, err := NewCopyStep("", validChown, "", []string{"src", "src"}, "dst", false, false)
 	require.Error(err)
 }
 
@@ -68,7 +68,7 @@ func TestCopyStepSetCacheID(t *testing.T) {
 		require.NoError(err)
 		defer sourceFileOne.Close()
 
-		step := CopyStepFixture("", "", []string{"."}, "tmp", false)
+		step := CopyStepFixture("", "", []string{"."}, "tmp", false, false)
 		err = step.SetCacheID(context, "")
 		hash1 := step.CacheID()
 		require.NoError(err)
@@ -98,7 +98,7 @@ func TestCopyStepSetCacheID(t *testing.T) {
 		require.NoError(err)
 		defer sourceFileOne.Close()
 
-		step := CopyStepFixture("", "", []string{"."}, "tmp", false)
+		step := CopyStepFixture("", "", []string{"."}, "tmp", false, false)
 		err = step.SetCacheID(context, "")
 		hash1 := step.CacheID()
 		require.NoError(err)
@@ -128,12 +128,12 @@ func TestCopyStepSetCacheID(t *testing.T) {
 		require.NoError(err)
 		defer sourceFileOne.Close()
 
-		step := CopyStepFixture("", "", []string{"."}, "tmp", false)
+		step := CopyStepFixture("", "", []string{"."}, "tmp", false, false)
 		err = step.SetCacheID(context, "")
 		hash1 := step.CacheID()
 		require.NoError(err)
 
-		step2 := CopyStepFixture("", "", []string{"."}, "tmp2", false)
+		step2 := CopyStepFixture("", "", []string{"."}, "tmp2", false, false)
 		err = step2.SetCacheID(context, hash1)
 		require.NoError(err)
 
@@ -153,7 +153,7 @@ func TestCopyStepSetCacheID(t *testing.T) {
 		context, cleanup := context.BuildContextFixture()
 		defer cleanup()
 
-		step := CopyStepFixture("", "stage", []string{"."}, "tmp", false)
+		step := CopyStepFixture("", "stage", []string{"."}, "tmp", false, false)
 		err := step.SetCacheID(context, "seed")
 		hash1 := step.CacheID()
 		require.NoError(err)
@@ -210,7 +210,7 @@ func TestCopyStepExecuteOnCriticalPath(t *testing.T) {
 		defer os.Remove(targetPath)
 
 		srcs := []string{sourceFileOneRelPath}
-		step := CopyStepFixture("", "", srcs, abs(targetPath), false)
+		step := CopyStepFixture("", "", srcs, abs(targetPath), false, false)
 		err = step.Execute(context, true)
 		require.NoError(err)
 
@@ -229,7 +229,7 @@ func TestCopyStepExecuteOnCriticalPath(t *testing.T) {
 		defer os.RemoveAll(targetPath)
 
 		srcs := []string{sourceFileOneRelPath}
-		step := CopyStepFixture("", "", srcs, abs(targetPath)+"/", false)
+		step := CopyStepFixture("", "", srcs, abs(targetPath)+"/", false, false)
 		err = step.Execute(context, true)
 		require.NoError(err)
 
@@ -250,7 +250,7 @@ func TestCopyStepExecuteOnCriticalPath(t *testing.T) {
 		defer os.RemoveAll(targetPath)
 
 		srcs := []string{sourceFileOneRelPath, sourceFileTwoRelPath}
-		step := CopyStepFixture("", "", srcs, abs(targetPath)+"/", false)
+		step := CopyStepFixture("", "", srcs, abs(targetPath)+"/", false, false)
 		err = step.Execute(context, true)
 		require.NoError(err)
 
@@ -271,7 +271,7 @@ func TestCopyStepExecuteOnCriticalPath(t *testing.T) {
 
 		// Copy to local path.
 		srcs := []string{path.Base(sourceDir)}
-		step := CopyStepFixture("", "", srcs, abs(targetDir)+"/", false)
+		step := CopyStepFixture("", "", srcs, abs(targetDir)+"/", false, false)
 		err = step.Execute(context, true)
 		require.NoError(err)
 
@@ -296,7 +296,7 @@ func TestCopyStepExecuteOnCriticalPath(t *testing.T) {
 
 		// Copy to local path.
 		srcs := []string{path.Base(sourceDir)}
-		step := CopyStepFixture("", "", srcs, abs(targetDir), false)
+		step := CopyStepFixture("", "", srcs, abs(targetDir), false, false)
 		err = step.Execute(context, true)
 		require.NoError(err)
 
@@ -324,7 +324,7 @@ func TestCopyStepExecuteOnCriticalPath(t *testing.T) {
 
 		// Copy to local path.
 		srcs := []string{path.Base(sourceDir), sourceDirOneRelPath}
-		step := CopyStepFixture("", "", srcs, abs(targetDir)+"/", false)
+		step := CopyStepFixture("", "", srcs, abs(targetDir)+"/", false, false)
 		err = step.Execute(context, true)
 		require.NoError(err)
 
@@ -378,7 +378,7 @@ func TestCopyStepCommitOnNonCriticalPath(t *testing.T) {
 		srcs := []string{sourceFileOneRelPath}
 
 		// Copy to layer tar store.
-		step := CopyStepFixture("", "", srcs, filepath.Join(workingDir, target), true)
+		step := CopyStepFixture("", "", srcs, filepath.Join(workingDir, target), true, false)
 		require.NoError(step.Execute(context, false))
 		digestPairs, err := step.Commit(context)
 		require.NoError(err)

lib/builder/step/fixtures.go

@@ -41,35 +41,35 @@ func FromStepFixture(args, image, alias string) *FromStep {
 }
 
 // AddStepFixture returns a AddStep, panicing if it fails, for testing purposes.
-func AddStepFixture(args string, srcs []string, dst string, commit bool) *AddStep {
-	c, err := NewAddStep(args, validChown, srcs, dst, commit)
+func AddStepFixture(args string, srcs []string, dst string, commit, preserveOwner bool) *AddStep {
+	c, err := NewAddStep(args, validChown, srcs, dst, commit, preserveOwner)
 	if err != nil {
 		panic(err)
 	}
 	return c
 }
 
 // AddStepFixtureNoChown returns a AddStep, panicing if it fails, for testing purposes.
-func AddStepFixtureNoChown(args string, srcs []string, dst string, commit bool) *AddStep {
-	c, err := NewAddStep(args, "", srcs, dst, commit)
+func AddStepFixtureNoChown(args string, srcs []string, dst string, commit, preserveOwner bool) *AddStep {
+	c, err := NewAddStep(args, "", srcs, dst, commit, preserveOwner)
 	if err != nil {
 		panic(err)
 	}
 	return c
 }
 
 // CopyStepFixture returns a CopyStep, panicing if it fails, for testing purposes.
-func CopyStepFixture(args, fromStage string, srcs []string, dst string, commit bool) *CopyStep {
-	c, err := NewCopyStep(args, validChown, fromStage, srcs, dst, commit)
+func CopyStepFixture(args, fromStage string, srcs []string, dst string, commit, preserveOwner bool) *CopyStep {
+	c, err := NewCopyStep(args, validChown, fromStage, srcs, dst, commit, preserveOwner)
 	if err != nil {
 		panic(err)
 	}
 	return c
 }
 
 // CopyStepFixtureNoChown returns a CopyStep, panicing if it fails, for testing purposes.
-func CopyStepFixtureNoChown(args, fromStage string, srcs []string, dst string, commit bool) *CopyStep {
-	c, err := NewCopyStep(args, "", fromStage, srcs, dst, commit)
+func CopyStepFixtureNoChown(args, fromStage string, srcs []string, dst string, commit, preserveOwner bool) *CopyStep {
+	c, err := NewCopyStep(args, "", fromStage, srcs, dst, commit, preserveOwner)
 	if err != nil {
 		panic(err)
 	}

lib/builder/step/fixtures_test.go

@@ -34,15 +34,15 @@ func TestFixtures(t *testing.T) {
 
 	// Valid.
 	require.NotNil(FromStepFixture("", "image", "alias"))
-	require.NotNil(CopyStepFixture("", "", []string{"."}, "/", false))
-	require.NotNil(CopyStepFixtureNoChown("", "", []string{"."}, "/", false))
-	require.NotNil(AddStepFixture("", []string{"."}, "/", false))
-	require.NotNil(AddStepFixtureNoChown("", []string{"."}, "/", false))
+	require.NotNil(CopyStepFixture("", "", []string{"."}, "/", false, false))
+	require.NotNil(CopyStepFixtureNoChown("", "", []string{"."}, "/", false, false))
+	require.NotNil(AddStepFixture("", []string{"."}, "/", false, false))
+	require.NotNil(AddStepFixtureNoChown("", []string{"."}, "/", false, false))
 
 	// Invalid.
 	testAndRecover(t, func() { FromStepFixture("", "image:", "") }, "FROM bad image")
-	testAndRecover(t, func() { CopyStepFixture("", "", []string{".", "."}, "/file", false) }, "invalid COPY")
-	testAndRecover(t, func() { CopyStepFixtureNoChown("", "", []string{".", "."}, "/file", false) }, "invalid COPY")
-	testAndRecover(t, func() { AddStepFixture("", []string{".", "."}, "/file", false) }, "invalid COPY")
-	testAndRecover(t, func() { AddStepFixtureNoChown("", []string{".", "."}, "/file", false) }, "invalid COPY")
+	testAndRecover(t, func() { CopyStepFixture("", "", []string{".", "."}, "/file", false, false) }, "invalid COPY")
+	testAndRecover(t, func() { CopyStepFixtureNoChown("", "", []string{".", "."}, "/file", false, false) }, "invalid COPY")
+	testAndRecover(t, func() { AddStepFixture("", []string{".", "."}, "/file", false, false) }, "invalid COPY")
+	testAndRecover(t, func() { AddStepFixtureNoChown("", []string{".", "."}, "/file", false, false) }, "invalid COPY")
 }

lib/builder/step/step.go

@@ -92,7 +92,7 @@ func NewDockerfileStep(
 	switch t := d.(type) {
 	case *dockerfile.AddDirective:
 		s, _ := d.(*dockerfile.AddDirective)
-		step, err = NewAddStep(s.Args, s.Chown, s.Srcs, s.Dst, s.Commit)
+		step, err = NewAddStep(s.Args, s.Chown, s.Srcs, s.Dst, s.Commit, s.PreserveOwner)
 	case *dockerfile.ArgDirective:
 		s, _ := d.(*dockerfile.ArgDirective)
 		step = NewArgStep(s.Args, s.Name, s.ResolvedVal, s.Commit)
@@ -101,7 +101,7 @@ func NewDockerfileStep(
 		step = NewCmdStep(s.Args, s.Cmd, s.Commit)
 	case *dockerfile.CopyDirective:
 		s, _ := d.(*dockerfile.CopyDirective)
-		step, err = NewCopyStep(s.Args, s.Chown, s.FromStage, s.Srcs, s.Dst, s.Commit)
+		step, err = NewCopyStep(s.Args, s.Chown, s.FromStage, s.Srcs, s.Dst, s.Commit, s.PreserveOwner)
 	case *dockerfile.EntrypointDirective:
 		s, _ := d.(*dockerfile.EntrypointDirective)
 		step = NewEntrypointStep(s.Args, s.Entrypoint, s.Commit)

lib/fileio/copy.go

@@ -310,6 +310,9 @@ func mkdirAll(dst string, mode os.FileMode, uid, gid int, preserveOwner bool) er
 			} else if err := os.Mkdir(absDir, mode); err != nil {
 				return fmt.Errorf("mkdir %s: %s", absDir, err)
 			}
+
+			// Update file info
+			fi, _ = os.Lstat(absDir)
 			if preserveOwner {
 				uid, gid = fileOwners(fi)
 			}

lib/parser/dockerfile/add_copy.go

@@ -14,30 +14,59 @@
 
 package dockerfile
 
-import "strings"
+import (
+	"fmt"
+	"strings"
+)
 
 type addCopyDirective struct {
 	*baseDirective
-	Chown string
-	Srcs  []string
-	Dst   string
+	Chown         string
+	PreserveOwner bool
+	Srcs          []string
+	Dst           string
 }
 
 // Variables:
 //   Replaced from ARGs and ENVs from within our stage.
 // Formats:
+//   ADD/COPY [--archive] <src>... <dest>
+//   ADD/COPY [--archive] ["<src>",... "<dest>"]
 //   ADD/COPY [--chown=<user>:<group>] ["<src>",... "<dest>"]
 //   ADD/COPY [--chown=<user>:<group>] <src>... <dest>
 func newAddCopyDirective(base *baseDirective, args []string) (*addCopyDirective, error) {
 	if len(args) == 0 {
 		return nil, base.err(errMissingArgs)
 	}
 
+	// Check the flag numbers here since we only allow zero or one flag here.
+	var chownCount, archiveCount int
 	var chown string
-	if val, ok, err := parseFlag(args[0], "chown"); err != nil {
-		return nil, base.err(err)
-	} else if ok {
-		chown = val
+	var preserveOwner bool
+	for _, arg := range args[:len(args)-1] {
+		if strings.HasPrefix(arg, "--chown") {
+			if val, ok, err := parseStringFlag(arg, "chown"); err != nil {
+				return nil, base.err(err)
+			} else if ok {
+				chown = val
+				chownCount++
+				continue
+			}
+		}
+
+		if strings.HasPrefix(arg, "--archive") {
+			if err := parseBoolFlag(arg, "archive"); err == nil {
+				archiveCount++
+				preserveOwner = true
+			} else {
+				return nil, fmt.Errorf("archive flag format is wrong")
+			}
+		}
+	}
+
+	if archiveCount+chownCount >= 2 {
+		return nil, base.err(fmt.Errorf("argument shouldn't contain more than one flag [--chown or --archive]"))
+	} else if archiveCount+chownCount == 1 {
 		args = args[1:]
 	}
 
@@ -52,6 +81,5 @@ func newAddCopyDirective(base *baseDirective, args []string) (*addCopyDirective,
 	}
 	srcs := parsed[:len(parsed)-1]
 	dst := parsed[len(parsed)-1]
-
-	return &addCopyDirective{base, chown, srcs, dst}, nil
+	return &addCopyDirective{base, chown, preserveOwner, srcs, dst}, nil
 }