From 6054d7e6533d549d57dfc645ac1f5420fd224915 Mon Sep 17 00:00:00 2001 From: Priya Wadhwa Date: Thu, 19 Apr 2018 14:23:30 -0700 Subject: [PATCH] Refactor copy and add Refactor copy and add Add/copy pass integration tests --- pkg/commands/add.go | 72 ++++++++++++++-------------------------- pkg/commands/copy.go | 69 ++++++++++++++++---------------------- pkg/util/command_util.go | 17 +++++----- pkg/util/fs_util.go | 56 +++++++++++++++++++++++++++++++ 4 files changed, 119 insertions(+), 95 deletions(-) diff --git a/pkg/commands/add.go b/pkg/commands/add.go index f6aee688c..c124661c3 100644 --- a/pkg/commands/add.go +++ b/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 diff --git a/pkg/commands/copy.go b/pkg/commands/copy.go index 845e7dfcd..5cdcd55ce 100644 --- a/pkg/commands/copy.go +++ b/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) } } diff --git a/pkg/util/command_util.go b/pkg/util/command_util.go index e74745796..251b96980 100644 --- a/pkg/util/command_util.go +++ b/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) diff --git a/pkg/util/fs_util.go b/pkg/util/fs_util.go index e2d5f588f..60631adce 100644 --- a/pkg/util/fs_util.go +++ b/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()) +}