Refactor copy and add

Refactor copy and add

Add/copy pass integration tests

pkg/commands/add.go

@@ -54,59 +54,50 @@ func (a *AddCommand) ExecuteCommand(config *manifest.Schema2Config) error {
 	}
 	dest = resolvedEnvs[len(resolvedEnvs)-1]
 	// Get a map of [src]:[files rooted at src]
-	srcMap, err := util.ResolveSources(resolvedEnvs, a.buildcontext)
+	srcs, err = util.ResolveSources(resolvedEnvs, a.buildcontext)
 	if err != nil {
 		return err
 	}
+	var unresolvedSrcs []string
 	// If any of the sources are local tar archives:
 	// 	1. Unpack them to the specified destination
 	// 	2. Remove it as a source that needs to be copied over
 	// If any of the sources is a remote file URL:
 	//	1. Download and copy it to the specified dest
 	//  2. Remove it as a source that needs to be copied
-	for src, files := range srcMap {
-		for _, file := range files {
-			// If file is a local tar archive, then we unpack it to dest
-			filePath := filepath.Join(a.buildcontext, file)
-			isFilenameSource, err := isFilenameSource(srcMap, file)
+	for _, src := range srcs {
+		fullPath := filepath.Join(a.buildcontext, src)
+		if util.IsSrcRemoteFileURL(src) {
+			urlDest := util.URLDestinationFilepath(src, dest, config.WorkingDir)
+			logrus.Infof("Adding remote URL %s to %s", src, urlDest)
+			if err := util.DownloadFileToDest(src, urlDest); err != nil {
+				return err
+			}
+			a.snapshotFiles = append(a.snapshotFiles, urlDest)
+		} else if util.IsFileLocalTarArchive(fullPath) {
+			logrus.Infof("Unpacking local tar archive %s to %s", src, dest)
+			if err := util.UnpackLocalTarArchive(fullPath, dest); err != nil {
+				return err
+			}
+			// Add the unpacked files to the snapshotter
+			filesAdded, err := util.Files(dest)
 			if err != nil {
 				return err
 			}
-			if util.IsSrcRemoteFileURL(file) {
-				urlDest := util.URLDestinationFilepath(file, dest, config.WorkingDir)
-				logrus.Infof("Adding remote URL %s to %s", file, urlDest)
-				if err := util.DownloadFileToDest(file, urlDest); err != nil {
-					return err
-				}
-				a.snapshotFiles = append(a.snapshotFiles, urlDest)
-				delete(srcMap, src)
-			} else if isFilenameSource && util.IsFileLocalTarArchive(filePath) {
-				logrus.Infof("Unpacking local tar archive %s to %s", file, dest)
-				if err := util.UnpackLocalTarArchive(filePath, dest); err != nil {
-					return err
-				}
-				// Add the unpacked files to the snapshotter
-				filesAdded, err := util.Files(dest)
-				if err != nil {
-					return err
-				}
-				logrus.Debugf("Added %v from local tar archive %s", filesAdded, file)
-				a.snapshotFiles = append(a.snapshotFiles, filesAdded...)
-				delete(srcMap, src)
-			}
+			logrus.Debugf("Added %v from local tar archive %s", filesAdded, src)
+			a.snapshotFiles = append(a.snapshotFiles, filesAdded...)
+		} else {
+			unresolvedSrcs = append(unresolvedSrcs, src)
 		}
 	}
 	// With the remaining "normal" sources, create and execute a standard copy command
-	if len(srcMap) == 0 {
+	if len(unresolvedSrcs) == 0 {
 		return nil
 	}
-	var regularSrcs []string
-	for src := range srcMap {
-		regularSrcs = append(regularSrcs, src)
-	}
+
 	copyCmd := CopyCommand{
 		cmd: &instructions.CopyCommand{
-			SourcesAndDest: append(regularSrcs, dest),
+			SourcesAndDest: append(unresolvedSrcs, dest),
 		},
 		buildcontext: a.buildcontext,
 	}
@@ -117,19 +108,6 @@ func (a *AddCommand) ExecuteCommand(config *manifest.Schema2Config) error {
 	return nil
 }
 
-func isFilenameSource(srcMap map[string][]string, fileName string) (bool, error) {
-	for src := range srcMap {
-		matched, err := filepath.Match(src, fileName)
-		if err != nil {
-			return false, err
-		}
-		if matched || (src == fileName) {
-			return true, nil
-		}
-	}
-	return false, nil
-}
-
 // FilesToSnapshot should return an empty array if still nil; no files were changed
 func (a *AddCommand) FilesToSnapshot() []string {
 	return a.snapshotFiles

pkg/commands/copy.go

@@ -17,14 +17,13 @@ limitations under the License.
 package commands
 
 import (
-	"os"
-	"path/filepath"
-	"strings"
-
 	"github.com/GoogleContainerTools/kaniko/pkg/util"
 	"github.com/containers/image/manifest"
 	"github.com/docker/docker/builder/dockerfile/instructions"
 	"github.com/sirupsen/logrus"
+	"os"
+	"path/filepath"
+	"strings"
 )
 
 type CopyCommand struct {
@@ -47,51 +46,41 @@ func (c *CopyCommand) ExecuteCommand(config *manifest.Schema2Config) error {
 	}
 	dest = resolvedEnvs[len(resolvedEnvs)-1]
 	// Get a map of [src]:[files rooted at src]
-	srcMap, err := util.ResolveSources(resolvedEnvs, c.buildcontext)
+	srcs, err = util.ResolveSources(resolvedEnvs, c.buildcontext)
 	if err != nil {
 		return err
 	}
-	// For each source, iterate through each file within and copy it over
-	for src, files := range srcMap {
-		for _, file := range files {
-			fi, err := os.Lstat(filepath.Join(c.buildcontext, file))
+	// For each source, iterate through and copy it over
+	for _, src := range srcs {
+		fullPath := filepath.Join(c.buildcontext, src)
+		fi, err := os.Lstat(fullPath)
+		if err != nil {
+			return err
+		}
+		destPath, err := util.DestinationFilepath(src, src, dest, config.WorkingDir, c.buildcontext)
+		if err != nil {
+			return err
+		}
+		if fi.IsDir() {
+			if err := util.CopyDir(fullPath, dest); err != nil {
+				return err
+			}
+			copiedFiles, err := util.Files(dest)
 			if err != nil {
 				return err
 			}
-			destPath, err := util.DestinationFilepath(file, src, dest, config.WorkingDir, c.buildcontext)
-			if err != nil {
+			c.snapshotFiles = append(c.snapshotFiles, copiedFiles...)
+		} else if fi.Mode()&os.ModeSymlink != 0 {
+			// If file is a symlink, we want to create the same relative symlink
+			if err := util.CopySymlink(fullPath, destPath); err != nil {
 				return err
 			}
-			// If source file is a directory, we want to create a directory ...
-			if fi.IsDir() {
-				logrus.Infof("Creating directory %s", destPath)
-				if err := os.MkdirAll(destPath, fi.Mode()); err != nil {
-					return err
-				}
-			} else if fi.Mode()&os.ModeSymlink != 0 {
-				// If file is a symlink, we want to create the same relative symlink
-				link, err := os.Readlink(filepath.Join(c.buildcontext, file))
-				if err != nil {
-					return err
-				}
-				linkDst := filepath.Join(destPath, link)
-				if err := os.Symlink(linkDst, destPath); err != nil {
-					logrus.Errorf("unable to symlink %s to %s", linkDst, destPath)
-					return err
-				}
-			} else {
-				// ... Else, we want to copy over a file
-				logrus.Infof("Copying file %s to %s", file, destPath)
-				srcFile, err := os.Open(filepath.Join(c.buildcontext, file))
-				if err != nil {
-					return err
-				}
-				defer srcFile.Close()
-				if err := util.CreateFile(destPath, srcFile, fi.Mode()); err != nil {
-					return err
-				}
+			c.snapshotFiles = append(c.snapshotFiles, destPath)
+		} else {
+			// ... Else, we want to copy over a file
+			if err := util.CopyFile(fullPath, destPath); err != nil {
+				return err
 			}
-			// Append the destination file to the list of files that should be snapshotted later
 			c.snapshotFiles = append(c.snapshotFiles, destPath)
 		}
 	}

pkg/util/command_util.go

@@ -84,7 +84,7 @@ func ContainsWildcards(paths []string) bool {
 
 // ResolveSources resolves the given sources if the sources contains wildcards
 // It returns a map of [src]:[files rooted at src]
-func ResolveSources(srcsAndDest instructions.SourcesAndDest, root string) (map[string][]string, error) {
+func ResolveSources(srcsAndDest instructions.SourcesAndDest, root string) ([]string, error) {
 	srcs := srcsAndDest[:len(srcsAndDest)-1]
 	// If sources contain wildcards, we first need to resolve them to actual paths
 	if ContainsWildcards(srcs) {
@@ -99,13 +99,8 @@ func ResolveSources(srcsAndDest instructions.SourcesAndDest, root string) (map[s
 		}
 		logrus.Debugf("Resolved sources to %v", srcs)
 	}
-	// Now, get a map of [src]:[files rooted at src]
-	srcMap, err := SourcesToFilesMap(srcs, root)
-	if err != nil {
-		return nil, err
-	}
 	// Check to make sure the sources are valid
-	return srcMap, IsSrcsValid(srcsAndDest, srcMap)
+	return srcs, IsSrcsValid(srcsAndDest, srcs, root)
 }
 
 // matchSources returns a list of sources that match wildcards
@@ -206,10 +201,16 @@ func SourcesToFilesMap(srcs []string, root string) (map[string][]string, error)
 }
 
 // IsSrcsValid returns an error if the sources provided are invalid, or nil otherwise
-func IsSrcsValid(srcsAndDest instructions.SourcesAndDest, srcMap map[string][]string) error {
+func IsSrcsValid(srcsAndDest instructions.SourcesAndDest, resolvedSrcs []string, root string) error {
 	srcs := srcsAndDest[:len(srcsAndDest)-1]
 	dest := srcsAndDest[len(srcsAndDest)-1]
 
+	// Now, get a map of [src]:[files rooted at src]
+	srcMap, err := SourcesToFilesMap(resolvedSrcs, root)
+	if err != nil {
+		return err
+	}
+
 	totalFiles := 0
 	for _, files := range srcMap {
 		totalFiles += len(files)

pkg/util/fs_util.go

@@ -210,3 +210,59 @@ func DownloadFileToDest(rawurl, dest string) error {
 	}
 	return os.Chtimes(dest, mTime, mTime)
 }
+
+// CopyDir copies the file or directory at src to dest
+func CopyDir(src, dest string) error {
+	files, err := RelativeFiles("", src)
+	if err != nil {
+		return err
+	}
+	for _, file := range files {
+		fullPath := filepath.Join(src, file)
+		fi, err := os.Stat(fullPath)
+		if err != nil {
+			return err
+		}
+		destPath := filepath.Join(dest, file)
+		if fi.IsDir() {
+			logrus.Infof("Creating directory %s", destPath)
+			if err := os.MkdirAll(destPath, fi.Mode()); err != nil {
+				return err
+			}
+		} else if fi.Mode()&os.ModeSymlink != 0 {
+			// If file is a symlink, we want to create the same relative symlink
+			if err := CopySymlink(fullPath, destPath); err != nil {
+				return err
+			}
+		} else {
+			// ... Else, we want to copy over a file
+			if err := CopyFile(fullPath, destPath); err != nil {
+				return err
+			}
+		}
+	}
+	return nil
+}
+
+func CopySymlink(src, dest string) error {
+	link, err := os.Readlink(src)
+	if err != nil {
+		return err
+	}
+	linkDst := filepath.Join(dest, link)
+	return os.Symlink(linkDst, dest)
+}
+
+func CopyFile(src, dest string) error {
+	fi, err := os.Stat(src)
+	if err != nil {
+		return err
+	}
+	logrus.Infof("Copying file %s to %s", src, dest)
+	srcFile, err := os.Open(src)
+	if err != nil {
+		return err
+	}
+	defer srcFile.Close()
+	return CreateFile(dest, srcFile, fi.Mode())
+}