From 9a93f5bad9a5729ca248861bc01e63dd3e8c38e1 Mon Sep 17 00:00:00 2001 From: Priya Wadhwa Date: Mon, 27 Aug 2018 11:39:00 -0700 Subject: [PATCH 1/2] Snapshot only specific files for COPY Before #289 was merged, when copying over directories for COPY kaniko would get a list of all files at the destination specified and add them to the list of files to be snapshotted. If the destination was root it would add all files. This worked because the snapshotter made sure the file had been changed before adding it to the layer. After #289, we changed the logic to add all files snapshotted to a layer without checking if the files had been changed. This created the bug in got all the files at root and added them to the layer without checking if they had been changed. This change should fix this bug. Now, the CopyDir function returns a list of files it copied over and only those files are added to the list of files to be snapshotted. Should fix #314 --- pkg/commands/copy.go | 5 +---- pkg/util/fs_util.go | 19 +++++++++++-------- 2 files changed, 12 insertions(+), 12 deletions(-) diff --git a/pkg/commands/copy.go b/pkg/commands/copy.go index 5bd66282d..020a61bb4 100644 --- a/pkg/commands/copy.go +++ b/pkg/commands/copy.go @@ -79,10 +79,7 @@ func (c *CopyCommand) ExecuteCommand(config *v1.Config, buildArgs *dockerfile.Bu // we need to add '/' to the end to indicate the destination is a directory dest = filepath.Join(cwd, dest) + "/" } - if err := util.CopyDir(fullPath, dest); err != nil { - return err - } - copiedFiles, err := util.Files(dest) + copiedFiles, err := util.CopyDir(fullPath, dest) if err != nil { return err } diff --git a/pkg/util/fs_util.go b/pkg/util/fs_util.go index 1a4f958bb..858312b24 100644 --- a/pkg/util/fs_util.go +++ b/pkg/util/fs_util.go @@ -459,16 +459,18 @@ func DownloadFileToDest(rawurl, dest string) error { } // CopyDir copies the file or directory at src to dest -func CopyDir(src, dest string) error { +// It returns a list of files it copied over +func CopyDir(src, dest string) ([]string, error) { files, err := RelativeFiles("", src) if err != nil { - return err + return nil, err } + var copiedFiles []string for _, file := range files { fullPath := filepath.Join(src, file) fi, err := os.Lstat(fullPath) if err != nil { - return err + return nil, err } destPath := filepath.Join(dest, file) if fi.IsDir() { @@ -478,24 +480,25 @@ func CopyDir(src, dest string) error { gid := int(fi.Sys().(*syscall.Stat_t).Gid) if err := os.MkdirAll(destPath, fi.Mode()); err != nil { - return err + return nil, err } if err := os.Chown(destPath, uid, gid); err != nil { - return err + return nil, 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 + return nil, err } } else { // ... Else, we want to copy over a file if err := CopyFile(fullPath, destPath); err != nil { - return err + return nil, err } } + copiedFiles = append(copiedFiles, destPath) } - return nil + return copiedFiles, nil } // CopySymlink copies the symlink at src to dest From 7080a8dd69912813ca4944feebfffab77ef92973 Mon Sep 17 00:00:00 2001 From: Priya Wadhwa Date: Mon, 27 Aug 2018 12:00:20 -0700 Subject: [PATCH 2/2] Add specific files from tar archives to list of snapshotted filesa I changed UnpackLocalTarArchive to return a list of files that were extracted, so that the list of snapshotted files for ADD is more accurate. Previously, we used to add all files in the extracted dir to be snapshotted, but this could result in preexisting files being snapshotted again. --- pkg/commands/add.go | 10 +++------- pkg/util/fs_util.go | 29 +++++++---------------------- pkg/util/tar_util.go | 15 ++++++++------- 3 files changed, 18 insertions(+), 36 deletions(-) diff --git a/pkg/commands/add.go b/pkg/commands/add.go index cf6ff7772..e0e317967 100644 --- a/pkg/commands/add.go +++ b/pkg/commands/add.go @@ -79,16 +79,12 @@ func (a *AddCommand) ExecuteCommand(config *v1.Config, buildArgs *dockerfile.Bui 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) + extractedFiles, err := util.UnpackLocalTarArchive(fullPath, dest) if err != nil { return err } - logrus.Debugf("Added %v from local tar archive %s", filesAdded, src) - a.snapshotFiles = append(a.snapshotFiles, filesAdded...) + logrus.Debugf("Added %v from local tar archive %s", extractedFiles, src) + a.snapshotFiles = append(a.snapshotFiles, extractedFiles...) } else { unresolvedSrcs = append(unresolvedSrcs, src) } diff --git a/pkg/util/fs_util.go b/pkg/util/fs_util.go index 858312b24..67a972002 100644 --- a/pkg/util/fs_util.go +++ b/pkg/util/fs_util.go @@ -155,7 +155,9 @@ func ChildDirInWhitelist(path, directory string) bool { return false } -func unTar(r io.Reader, dest string) error { +// unTar returns a list of files that have been extracted from the tar archive at r to the path at dest +func unTar(r io.Reader, dest string) ([]string, error) { + var extractedFiles []string tr := tar.NewReader(r) for { hdr, err := tr.Next() @@ -163,13 +165,14 @@ func unTar(r io.Reader, dest string) error { break } if err != nil { - return err + return nil, err } if err := extractFile(dest, hdr, tr); err != nil { - return err + return nil, err } + extractedFiles = append(extractedFiles, dest) } - return nil + return extractedFiles, nil } func extractFile(dest string, hdr *tar.Header, tr io.Reader) error { @@ -349,24 +352,6 @@ func RelativeFiles(fp string, root string) ([]string, error) { return files, err } -// Files returns a list of all files rooted at root -func Files(root string) ([]string, error) { - var files []string - logrus.Debugf("Getting files and contents at root %s", root) - err := filepath.Walk(root, func(path string, info os.FileInfo, err error) error { - whitelisted, err := CheckWhitelist(path) - if err != nil { - return err - } - if whitelisted { - return nil - } - files = append(files, path) - return err - }) - return files, err -} - // ParentDirectories returns a list of paths to all parent directories // Ex. /some/temp/dir -> [/, /some, /some/temp, /some/temp/dir] func ParentDirectories(path string) []string { diff --git a/pkg/util/tar_util.go b/pkg/util/tar_util.go index f95574deb..898844784 100644 --- a/pkg/util/tar_util.go +++ b/pkg/util/tar_util.go @@ -137,17 +137,17 @@ func (t *Tar) checkHardlink(p string, i os.FileInfo) (bool, string) { } // UnpackLocalTarArchive unpacks the tar archive at path to the directory dest -// Returns true if the path was actually unpacked -func UnpackLocalTarArchive(path, dest string) error { +// Returns the files extracted from the tar archive +func UnpackLocalTarArchive(path, dest string) ([]string, error) { // First, we need to check if the path is a local tar archive if compressed, compressionLevel := fileIsCompressedTar(path); compressed { file, err := os.Open(path) if err != nil { - return err + return nil, err } defer file.Close() if compressionLevel == archive.Gzip { - return UnpackCompressedTar(path, dest) + return nil, UnpackCompressedTar(path, dest) } else if compressionLevel == archive.Bzip2 { bzr := bzip2.NewReader(file) return unTar(bzr, dest) @@ -156,12 +156,12 @@ func UnpackLocalTarArchive(path, dest string) error { if fileIsUncompressedTar(path) { file, err := os.Open(path) if err != nil { - return err + return nil, err } defer file.Close() return unTar(file, dest) } - return errors.New("path does not lead to local tar archive") + return nil, errors.New("path does not lead to local tar archive") } //IsFileLocalTarArchive returns true if the file is a local tar archive @@ -223,5 +223,6 @@ func UnpackCompressedTar(path, dir string) error { return err } defer gzr.Close() - return unTar(gzr, dir) + _, err = unTar(gzr, dir) + return err }