From cc2c9a0663a64e3a2c2e22eac9d987b998daa48a Mon Sep 17 00:00:00 2001
From: tinkerborg <15373049+tinkerborg@users.noreply.github.com>
Date: Tue, 14 Jan 2020 14:55:06 -0500
Subject: [PATCH] sort filesToAdd in TakeSnapshot

filesToAdd is sorted in TakeSnapshotFS, but not here. This makes ordering unpredictable within the layer's tarball,
causing the SHA to differ even if layer contents haven't changed
---
 pkg/snapshot/snapshot.go      |  2 +
 pkg/snapshot/snapshot_test.go | 69 +++++++++++++++++++++++++++++++++++
 2 files changed, 71 insertions(+)

diff --git a/pkg/snapshot/snapshot.go b/pkg/snapshot/snapshot.go
index 80b5c5ce0..59be2f0e0 100644
--- a/pkg/snapshot/snapshot.go
+++ b/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 {
diff --git a/pkg/snapshot/snapshot_test.go b/pkg/snapshot/snapshot_test.go
index 8e3f16598..3a1ec7353 100644
--- a/pkg/snapshot/snapshot_test.go
+++ b/pkg/snapshot/snapshot_test.go
@@ -302,6 +302,75 @@ 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 := setUpTestDir()
+		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 setupSymlink(dir string, link string, target string) error {
 	return os.Symlink(target, filepath.Join(dir, link))
 }