Make sure to snapshot parent directories of specific files for add/copy

pkg/commands/copy.go

@@ -17,6 +17,7 @@ limitations under the License.
 package commands
 
 import (
+	"github.com/GoogleContainerTools/kaniko/pkg/constants"
 	"github.com/GoogleContainerTools/kaniko/pkg/util"
 	"github.com/containers/image/manifest"
 	"github.com/docker/docker/builder/dockerfile/instructions"
@@ -57,11 +58,19 @@ func (c *CopyCommand) ExecuteCommand(config *manifest.Schema2Config) error {
 		if err != nil {
 			return err
 		}
-		destPath, err := util.DestinationFilepath(src, dest, config.WorkingDir)
+		cwd := config.WorkingDir
+		if cwd == "" {
+			cwd = constants.RootDir
+		}
+		destPath, err := util.DestinationFilepath(src, dest, cwd)
 		if err != nil {
 			return err
 		}
 		if fi.IsDir() {
+			if !filepath.IsAbs(dest) {
+				// 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
 			}

pkg/snapshot/snapshot.go

@@ -19,10 +19,8 @@ package snapshot
 import (
 	"archive/tar"
 	"bytes"
-
 	"github.com/GoogleContainerTools/kaniko/pkg/util"
 	"github.com/sirupsen/logrus"
-
 	"io"
 	"io/ioutil"
 	"os"
@@ -52,12 +50,14 @@ func (s *Snapshotter) Init() error {
 // TakeSnapshot takes a snapshot of the filesystem, avoiding directories in the whitelist, and creates
 // a tarball of the changed files. Return contents of the tarball, and whether or not any files were changed
 func (s *Snapshotter) TakeSnapshot(files []string) ([]byte, error) {
-	if files != nil {
-		return s.TakeSnapshotOfFiles(files)
-	}
-	logrus.Info("Taking snapshot of full filesystem...")
 	buf := bytes.NewBuffer([]byte{})
-	filesAdded, err := s.snapShotFS(buf)
+	var filesAdded bool
+	var err error
+	if files == nil {
+		filesAdded, err = s.snapShotFS(buf)
+	} else {
+		filesAdded, err = s.snapshotFiles(buf, files)
+	}
 	if err != nil {
 		return nil, err
 	}
@@ -68,45 +68,57 @@ func (s *Snapshotter) TakeSnapshot(files []string) ([]byte, error) {
 	return contents, err
 }
 
-// TakeSnapshotOfFiles takes a snapshot of specific files
+// snapshotFiles takes a snapshot of specific files
 // Used for ADD/COPY commands, when we know which files have changed
-func (s *Snapshotter) TakeSnapshotOfFiles(files []string) ([]byte, error) {
+func (s *Snapshotter) snapshotFiles(f io.Writer, files []string) (bool, error) {
-	logrus.Infof("Taking snapshot of files %v...", files)
+	s.hardlinks = map[uint64]string{}
 	s.l.Snapshot()
 	if len(files) == 0 {
 		logrus.Info("No files changed in this command, skipping snapshotting.")
-		return nil, nil
+		return false, nil
 	}
-	buf := bytes.NewBuffer([]byte{})
+	logrus.Infof("Taking snapshot of files %v...", files)
-	w := tar.NewWriter(buf)
+	snapshottedFiles := make(map[string]bool)
-	defer w.Close()
-	filesAdded := false
 	for _, file := range files {
-		info, err := os.Lstat(file)
+		parentDirs := util.ParentDirectories(file)
-		if err != nil {
+		files = append(parentDirs, files...)
-			return nil, err
+	}
+	filesAdded := false
+	w := tar.NewWriter(f)
+	defer w.Close()
+
+	// Now create the tar.
+	for _, file := range files {
+		file = filepath.Clean(file)
+		if val, ok := snapshottedFiles[file]; ok && val {
+			continue
 		}
 		if util.PathInWhitelist(file, s.directory) {
-			logrus.Debugf("Not adding %s to layer, as it is whitelisted", file)
+			logrus.Debugf("Not adding %s to layer, as it's whitelisted", file)
 			continue
 		}
+		snapshottedFiles[file] = true
+		info, err := os.Lstat(file)
+		if err != nil {
+			return false, err
+		}
 		// Only add to the tar if we add it to the layeredmap.
 		maybeAdd, err := s.l.MaybeAdd(file)
 		if err != nil {
-			return nil, err
+			return false, err
 		}
 		if maybeAdd {
 			filesAdded = true
-			util.AddToTar(file, info, s.hardlinks, w)
+			if err := util.AddToTar(file, info, s.hardlinks, w); err != nil {
+				return false, err
 			}
 		}
-	if !filesAdded {
-		return nil, nil
 	}
-	return ioutil.ReadAll(buf)
+	return filesAdded, nil
 }
 
 func (s *Snapshotter) snapShotFS(f io.Writer) (bool, error) {
+	logrus.Info("Taking snapshot of full filesystem...")
 	s.hardlinks = map[uint64]string{}
 	s.l.Snapshot()
 	existingPaths := s.l.GetFlattenedPathsForWhiteOut()

pkg/snapshot/snapshot_test.go

@@ -149,30 +149,23 @@ func TestSnapshotFiles(t *testing.T) {
 	if err != nil {
 		t.Fatal(err)
 	}
-	expectedContents := map[string]string{
+	expectedFiles := []string{"/tmp", filepath.Join(testDir, "foo")}
-		filepath.Join(testDir, "foo"): "newbaz1",
+
-	}
 	// Check contents of the snapshot, make sure contents is equivalent to snapshotFiles
 	reader := bytes.NewReader(contents)
 	tr := tar.NewReader(reader)
-	numFiles := 0
+	var actualFiles []string
 	for {
 		hdr, err := tr.Next()
 		if err == io.EOF {
 			break
 		}
-		numFiles = numFiles + 1
+		if err != nil {
-		if _, isFile := expectedContents[hdr.Name]; !isFile {
+			t.Fatal(err)
-			t.Fatalf("File %s unexpectedly in tar", hdr.Name)
 		}
-		contents, _ := ioutil.ReadAll(tr)
+		actualFiles = append(actualFiles, hdr.Name)
-		if string(contents) != expectedContents[hdr.Name] {
-			t.Fatalf("Contents of %s incorrect, expected: %s, actual: %s", hdr.Name, expectedContents[hdr.Name], string(contents))
-		}
-	}
-	if numFiles != 1 {
-		t.Fatalf("%s was not added.", filepath.Join(testDir, "foo"))
 	}
+	testutil.CheckErrorAndDeepEqual(t, false, nil, expectedFiles, actualFiles)
 }
 
 func TestEmptySnapshot(t *testing.T) {

pkg/util/fs_util.go

@@ -141,6 +141,23 @@ func Files(root string) ([]string, error) {
 	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 {
+	path = filepath.Clean(path)
+	dirs := strings.Split(path, "/")
+	dirPath := constants.RootDir
+	var paths []string
+	for _, dir := range dirs {
+		if dir == "" {
+			continue
+		}
+		dirPath = filepath.Join(dirPath, dir)
+		paths = append(paths, dirPath)
+	}
+	return paths
+}
+
 // FilepathExists returns true if the path exists
 func FilepathExists(path string) bool {
 	_, err := os.Lstat(path)

pkg/util/fs_util_test.go

@@ -131,3 +131,35 @@ func Test_RelativeFiles(t *testing.T) {
 		testutil.CheckErrorAndDeepEqual(t, false, err, test.expectedFiles, actualFiles)
 	}
 }
+
+func Test_ParentDirectories(t *testing.T) {
+	tests := []struct {
+		name     string
+		path     string
+		expected []string
+	}{
+		{
+			name: "regular path",
+			path: "/path/to/dir",
+			expected: []string{
+				"/path",
+				"/path/to",
+				"/path/to/dir",
+			},
+		},
+		{
+			name: "current directory",
+			path: ".",
+			expected: []string{
+				"/",
+			},
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			actual := ParentDirectories(tt.path)
+			testutil.CheckErrorAndDeepEqual(t, false, nil, tt.expected, actual)
+		})
+	}
+}