diff --git a/pkg/snapshot/snapshot.go b/pkg/snapshot/snapshot.go index 80b5c5ce04..59be2f0e0c 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 8e3f16598e..67b70b9c51 100644 --- a/pkg/snapshot/snapshot_test.go +++ b/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)) } diff --git a/pkg/util/tar_util.go b/pkg/util/tar_util.go index 213ef68e38..823e42f048 100644 --- a/pkg/util/tar_util.go +++ b/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