Merge pull request #729 from xanonid/Remove_leading_slash_in_layer_tarball_paths

Remove leading slash in layer tarball paths (Closes: #726)

pkg/snapshot/snapshot_test.go

@@ -22,6 +22,7 @@ import (
 	"os"
 	"path/filepath"
 	"sort"
+	"strings"
 	"testing"
 
 	"github.com/GoogleContainerTools/kaniko/pkg/util"
@@ -31,6 +32,7 @@ import (
 
 func TestSnapshotFSFileChange(t *testing.T) {
 	testDir, snapshotter, cleanup, err := setUpTestDir()
+	testDirWithoutLeadingSlash := strings.TrimLeft(testDir, "/")
 	defer cleanup()
 	if err != nil {
 		t.Fatal(err)
@@ -55,16 +57,16 @@ func TestSnapshotFSFileChange(t *testing.T) {
 	}
 	// Check contents of the snapshot, make sure contents is equivalent to snapshotFiles
 	tr := tar.NewReader(f)
-	fooPath := filepath.Join(testDir, "foo")
-	batPath := filepath.Join(testDir, "bar/bat")
+	fooPath := filepath.Join(testDirWithoutLeadingSlash, "foo")
+	batPath := filepath.Join(testDirWithoutLeadingSlash, "bar/bat")
 	snapshotFiles := map[string]string{
 		fooPath: "newbaz1",
 		batPath: "baz",
 	}
-	for _, dir := range util.ParentDirectories(fooPath) {
+	for _, dir := range util.ParentDirectoriesWithoutLeadingSlash(fooPath) {
 		snapshotFiles[dir] = ""
 	}
-	for _, dir := range util.ParentDirectories(batPath) {
+	for _, dir := range util.ParentDirectoriesWithoutLeadingSlash(batPath) {
 		snapshotFiles[dir] = ""
 	}
 	numFiles := 0
@@ -128,12 +130,14 @@ func TestSnapshotFSIsReproducible(t *testing.T) {
 
 func TestSnapshotFSChangePermissions(t *testing.T) {
 	testDir, snapshotter, cleanup, err := setUpTestDir()
+	testDirWithoutLeadingSlash := strings.TrimLeft(testDir, "/")
 	defer cleanup()
 	if err != nil {
 		t.Fatal(err)
 	}
 	// Change permissions on a file
 	batPath := filepath.Join(testDir, "bar/bat")
+	batPathWithoutLeadingSlash := filepath.Join(testDirWithoutLeadingSlash, "bar/bat")
 	if err := os.Chmod(batPath, 0600); err != nil {
 		t.Fatalf("Error changing permissions on %s: %v", batPath, err)
 	}
@@ -149,9 +153,9 @@ func TestSnapshotFSChangePermissions(t *testing.T) {
 	// Check contents of the snapshot, make sure contents is equivalent to snapshotFiles
 	tr := tar.NewReader(f)
 	snapshotFiles := map[string]string{
-		batPath: "baz2",
+		batPathWithoutLeadingSlash: "baz2",
 	}
-	for _, dir := range util.ParentDirectories(batPath) {
+	for _, dir := range util.ParentDirectoriesWithoutLeadingSlash(batPath) {
 		snapshotFiles[dir] = ""
 	}
 	numFiles := 0
@@ -160,6 +164,7 @@ func TestSnapshotFSChangePermissions(t *testing.T) {
 		if err == io.EOF {
 			break
 		}
+		t.Logf("Info %s in tar", hdr.Name)
 		numFiles++
 		if _, isFile := snapshotFiles[hdr.Name]; !isFile {
 			t.Fatalf("File %s unexpectedly in tar", hdr.Name)
@@ -176,6 +181,7 @@ func TestSnapshotFSChangePermissions(t *testing.T) {
 
 func TestSnapshotFiles(t *testing.T) {
 	testDir, snapshotter, cleanup, err := setUpTestDir()
+	testDirWithoutLeadingSlash := strings.TrimLeft(testDir, "/")
 	defer cleanup()
 	if err != nil {
 		t.Fatal(err)
@@ -197,9 +203,9 @@ func TestSnapshotFiles(t *testing.T) {
 	defer os.Remove(tarPath)
 
 	expectedFiles := []string{
-		filepath.Join(testDir, "foo"),
+		filepath.Join(testDirWithoutLeadingSlash, "foo"),
 	}
-	expectedFiles = append(expectedFiles, util.ParentDirectories(filepath.Join(testDir, "foo"))...)
+	expectedFiles = append(expectedFiles, util.ParentDirectoriesWithoutLeadingSlash(filepath.Join(testDir, "foo"))...)
 
 	f, err := os.Open(tarPath)
 	if err != nil {

pkg/util/fs_util.go

@@ -377,6 +377,24 @@ func ParentDirectories(path string) []string {
 	return paths
 }
 
+// ParentDirectoriesWithoutLeadingSlash returns a list of paths to all parent directories
+// all subdirectories do not contain a leading /
+// Ex. /some/temp/dir -> [/, some, some/temp, some/temp/dir]
+func ParentDirectoriesWithoutLeadingSlash(path string) []string {
+	path = filepath.Clean(path)
+	dirs := strings.Split(path, "/")
+	dirPath := ""
+	paths := []string{constants.RootDir}
+	for index, dir := range dirs {
+		if dir == "" || index == (len(dirs)-1) {
+			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

@@ -177,6 +177,38 @@ func Test_ParentDirectories(t *testing.T) {
 	}
 }
 
+func Test_ParentDirectoriesWithoutLeadingSlash(t *testing.T) {
+	tests := []struct {
+		name     string
+		path     string
+		expected []string
+	}{
+		{
+			name: "regular path",
+			path: "/path/to/dir",
+			expected: []string{
+				"/",
+				"path",
+				"path/to",
+			},
+		},
+		{
+			name: "current directory",
+			path: ".",
+			expected: []string{
+				"/",
+			},
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			actual := ParentDirectoriesWithoutLeadingSlash(tt.path)
+			testutil.CheckErrorAndDeepEqual(t, false, nil, tt.expected, actual)
+		})
+	}
+}
+
 func Test_CheckWhitelist(t *testing.T) {
 	type args struct {
 		path      string

pkg/util/tar_util.go

@@ -25,6 +25,7 @@ import (
 	"io/ioutil"
 	"os"
 	"path/filepath"
+	"strings"
 	"syscall"
 
 	"github.com/docker/docker/pkg/archive"
@@ -74,7 +75,14 @@ func (t *Tar) AddFileToTar(p string) error {
 	if err != nil {
 		return err
 	}
-	hdr.Name = p
+
+	if p != "/" {
+		// Docker uses no leading / in the tarball
+		hdr.Name = strings.TrimLeft(p, "/")
+	} else {
+		// allow entry for / to preserve permission changes etc. (currently ignored anyway by Docker runtime)
+		hdr.Name = p
+	}
 
 	hardlink, linkDst := t.checkHardlink(p, i)
 	if hardlink {
@@ -104,7 +112,8 @@ func (t *Tar) Whiteout(p string) error {
 	name := ".wh." + filepath.Base(p)
 
 	th := &tar.Header{
-		Name: filepath.Join(dir, name),
+		// Docker uses no leading / in the tarball
+		Name: strings.TrimLeft(filepath.Join(dir, name), "/"),
 		Size: 0,
 	}
 	if err := t.w.WriteHeader(th); err != nil {