From e5585fded80f92d90cc1b2dbe3319d53dc8e498b Mon Sep 17 00:00:00 2001
From: Gilbert Gilb's <gilbsgilbert@gmail.com>
Date: Sun, 29 Mar 2020 18:03:32 +0200
Subject: [PATCH] Always add parent directories of files to snapshots.

During a snapshot, when a file changed and not its parent directories,
the parent directories weren't added to the layer. This is inconsistent
with Docker's behavior which always add parent directories to the layer.
In some edge-cases, it could lead to problems with docker considering
that parent directories where owned by root in forthcoming layers
although they shouldn't (see #1163).

Also, Docker seems to be POSIX compliant regarding the name of
directories in the archive, which always have a slash appended. This
commit also fixes this.

Fixes #1163
---
 pkg/snapshot/snapshot.go      | 18 ++++++++++++++++++
 pkg/snapshot/snapshot_test.go | 22 +++++++++++++++++++++-
 pkg/util/tar_util.go          |  4 ++++
 3 files changed, 43 insertions(+), 1 deletion(-)

diff --git a/pkg/snapshot/snapshot.go b/pkg/snapshot/snapshot.go
index e64e28dd3..7e027fe8f 100644
--- a/pkg/snapshot/snapshot.go
+++ b/pkg/snapshot/snapshot.go
@@ -226,10 +226,28 @@ func writeToTar(t util.Tar, files, whiteouts []string) error {
 			return err
 		}
 	}
+
+	addedPaths := make(map[string]bool)
 	for _, path := range files {
+		if _, fileExists := addedPaths[path]; fileExists {
+			continue
+		}
+		for _, parentPath := range util.ParentDirectories(path) {
+			if parentPath == "/" {
+				continue
+			}
+			if _, dirExists := addedPaths[parentPath]; dirExists {
+				continue
+			}
+			if err := t.AddFileToTar(parentPath); err != nil {
+				return err
+			}
+			addedPaths[parentPath] = true
+		}
 		if err := t.AddFileToTar(path); err != nil {
 			return err
 		}
+		addedPaths[path] = true
 	}
 	return nil
 }
diff --git a/pkg/snapshot/snapshot_test.go b/pkg/snapshot/snapshot_test.go
index 19fc9c4cb..2e2d9cc21 100644
--- a/pkg/snapshot/snapshot_test.go
+++ b/pkg/snapshot/snapshot_test.go
@@ -63,6 +63,12 @@ func TestSnapshotFSFileChange(t *testing.T) {
 		fooPath: "newbaz1",
 		batPath: "baz",
 	}
+	for _, path := range util.ParentDirectoriesWithoutLeadingSlash(batPath) {
+		if path == "/" {
+			continue
+		}
+		snapshotFiles[path+"/"] = ""
+	}
 
 	actualFiles := []string{}
 	for {
@@ -76,6 +82,9 @@ func TestSnapshotFSFileChange(t *testing.T) {
 		if _, isFile := snapshotFiles[hdr.Name]; !isFile {
 			t.Fatalf("File %s unexpectedly in tar", hdr.Name)
 		}
+		if hdr.Typeflag == tar.TypeDir {
+			continue
+		}
 		contents, _ := ioutil.ReadAll(tr)
 		if string(contents) != snapshotFiles[hdr.Name] {
 			t.Fatalf("Contents of %s incorrect, expected: %s, actual: %s", hdr.Name, snapshotFiles[hdr.Name], string(contents))
@@ -152,6 +161,12 @@ func TestSnapshotFSChangePermissions(t *testing.T) {
 	snapshotFiles := map[string]string{
 		batPathWithoutLeadingSlash: "baz2",
 	}
+	for _, path := range util.ParentDirectoriesWithoutLeadingSlash(batPathWithoutLeadingSlash) {
+		if path == "/" {
+			continue
+		}
+		snapshotFiles[path+"/"] = ""
+	}
 
 	foundFiles := []string{}
 	for {
@@ -164,6 +179,9 @@ func TestSnapshotFSChangePermissions(t *testing.T) {
 		if _, isFile := snapshotFiles[hdr.Name]; !isFile {
 			t.Fatalf("File %s unexpectedly in tar", hdr.Name)
 		}
+		if hdr.Typeflag == tar.TypeDir {
+			continue
+		}
 		contents, _ := ioutil.ReadAll(tr)
 		if string(contents) != snapshotFiles[hdr.Name] {
 			t.Fatalf("Contents of %s incorrect, expected: %s, actual: %s", hdr.Name, snapshotFiles[hdr.Name], string(contents))
@@ -203,7 +221,9 @@ func TestSnapshotFiles(t *testing.T) {
 	expectedFiles := []string{
 		filepath.Join(testDirWithoutLeadingSlash, "foo"),
 	}
-	expectedFiles = append(expectedFiles, util.ParentDirectoriesWithoutLeadingSlash(filepath.Join(testDir, "foo"))...)
+	for _, path := range util.ParentDirectoriesWithoutLeadingSlash(filepath.Join(testDir, "foo")) {
+		expectedFiles = append(expectedFiles, strings.TrimRight(path, "/")+"/")
+	}
 
 	f, err := os.Open(tarPath)
 	if err != nil {
diff --git a/pkg/util/tar_util.go b/pkg/util/tar_util.go
index 823e42f04..76a6b5405 100644
--- a/pkg/util/tar_util.go
+++ b/pkg/util/tar_util.go
@@ -84,6 +84,10 @@ func (t *Tar) AddFileToTar(p string) error {
 		hdr.Name = p
 	}
 
+	if hdr.Typeflag == tar.TypeDir && !strings.HasSuffix(hdr.Name, "/") {
+		hdr.Name = hdr.Name + "/"
+	}
+
 	// rootfs may not have been extracted when using cache, preventing uname/gname from resolving
 	// this makes this layer unnecessarily differ from a cached layer which does contain this information
 	hdr.Uname = ""