Merge pull request #1032 from tinkerborg/consistent-sha

Ensure image SHA stays consistent when layer contents haven't changed

pkg/snapshot/snapshot.go

@@ -79,6 +79,8 @@ func (s *Snapshotter) TakeSnapshot(files []string) (string, error) {
 	// Also add parent directories to keep the permission of them correctly.
 	filesToAdd := filesWithParentDirs(files)
 
+	sort.Strings(filesToAdd)
+
 	// Add files to the layered map
 	for _, file := range filesToAdd {
 		if err := s.l.Add(file); err != nil {

pkg/snapshot/snapshot_test.go

@@ -302,6 +302,108 @@ func TestFileWithLinks(t *testing.T) {
 	}
 }
 
+func TestSnasphotPreservesFileOrder(t *testing.T) {
+	newFiles := map[string]string{
+		"foo":     "newbaz1",
+		"bar/bat": "baz",
+		"bar/qux": "quuz",
+		"qux":     "quuz",
+		"corge":   "grault",
+		"garply":  "waldo",
+		"fred":    "plugh",
+		"xyzzy":   "thud",
+	}
+
+	newFileNames := []string{}
+
+	for fileName := range newFiles {
+		newFileNames = append(newFileNames, fileName)
+	}
+
+	filesInTars := [][]string{}
+
+	for i := 0; i <= 2; i++ {
+		testDir, snapshotter, cleanup, err := setUpTest()
+		testDirWithoutLeadingSlash := strings.TrimLeft(testDir, "/")
+		defer cleanup()
+
+		if err != nil {
+			t.Fatal(err)
+		}
+		// Make some changes to the filesystem
+		if err := testutil.SetupFiles(testDir, newFiles); err != nil {
+			t.Fatalf("Error setting up fs: %s", err)
+		}
+
+		filesToSnapshot := []string{}
+		for _, file := range newFileNames {
+			filesToSnapshot = append(filesToSnapshot, filepath.Join(testDir, file))
+		}
+
+		// Take a snapshot
+		tarPath, err := snapshotter.TakeSnapshot(filesToSnapshot)
+
+		if err != nil {
+			t.Fatalf("Error taking snapshot of fs: %s", err)
+		}
+
+		f, err := os.Open(tarPath)
+		if err != nil {
+			t.Fatal(err)
+		}
+		tr := tar.NewReader(f)
+		filesInTars = append(filesInTars, []string{})
+		for {
+			hdr, err := tr.Next()
+			if err == io.EOF {
+				break
+			}
+			if err != nil {
+				t.Fatal(err)
+			}
+			filesInTars[i] = append(filesInTars[i], strings.TrimPrefix(hdr.Name, testDirWithoutLeadingSlash))
+		}
+	}
+
+	// Check contents of all snapshots, make sure files appear in consistent order
+	for i := 1; i < len(filesInTars); i++ {
+		testutil.CheckErrorAndDeepEqual(t, false, nil, filesInTars[0], filesInTars[i])
+	}
+}
+
+func TestSnapshotOmitsUnameGname(t *testing.T) {
+	_, snapshotter, cleanup, err := setUpTest()
+
+	defer cleanup()
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	tarPath, err := snapshotter.TakeSnapshotFS()
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	f, err := os.Open(tarPath)
+	if err != nil {
+		t.Fatal(err)
+	}
+	tr := tar.NewReader(f)
+	for {
+		hdr, err := tr.Next()
+		if err == io.EOF {
+			break
+		}
+		if err != nil {
+			t.Fatal(err)
+		}
+		if hdr.Uname != "" || hdr.Gname != "" {
+			t.Fatalf("Expected Uname/Gname for %s to be empty: Uname = '%s', Gname = '%s'", hdr.Name, hdr.Uname, hdr.Gname)
+		}
+	}
+
+}
+
 func setupSymlink(dir string, link string, target string) error {
 	return os.Symlink(target, filepath.Join(dir, link))
 }

pkg/util/tar_util.go

@@ -84,6 +84,11 @@ func (t *Tar) AddFileToTar(p string) error {
 		hdr.Name = p
 	}
 
+	// 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 = ""
+	hdr.Gname = ""
+
 	hardlink, linkDst := t.checkHardlink(p, i)
 	if hardlink {
 		hdr.Linkname = linkDst