Skip to content

Commit

Permalink
Write parent directories to tar before whiteout files
Browse files Browse the repository at this point in the history
Fixes GoogleContainerTools#1149

The OCI image spec does not specify this order but it's a good idea and Docker
does the same.

When manually comparing layers created by Docker and Kaniko there are still
some differences (that container-diff does not show):

* Kaniko adds / to layers
* For `mkdir /test`, docker adds `/test` and an opaque whiteout file
  `/test/.wh..wh..opq`. Kaniko only adds `/test/` (and /).
  • Loading branch information
andreasf committed May 31, 2022
1 parent 1c0e5a0 commit 5a58e6e
Show file tree
Hide file tree
Showing 2 changed files with 103 additions and 14 deletions.
36 changes: 22 additions & 14 deletions pkg/snapshot/snapshot.go
Original file line number Diff line number Diff line change
Expand Up @@ -220,28 +220,23 @@ func writeToTar(t util.Tar, files, whiteouts []string) error {
defer timing.DefaultRun.Stop(timer)

// Now create the tar.
addedPaths := make(map[string]bool)

for _, path := range whiteouts {
if err := addParentDirectories(t, addedPaths, path); err != nil {
return err
}
if err := t.Whiteout(path); err != nil {
return err
}
}

addedPaths := make(map[string]bool)
for _, path := range files {
if _, fileExists := addedPaths[path]; fileExists {
continue
if err := addParentDirectories(t, addedPaths, path); err != nil {
return err
}
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 _, pathAdded := addedPaths[path]; pathAdded {
continue
}
if err := t.AddFileToTar(path); err != nil {
return err
Expand All @@ -251,6 +246,19 @@ func writeToTar(t util.Tar, files, whiteouts []string) error {
return nil
}

func addParentDirectories(t util.Tar, addedPaths map[string]bool, path string) error {
for _, parentPath := range util.ParentDirectories(path) {
if _, pathAdded := addedPaths[parentPath]; pathAdded {
continue
}
if err := t.AddFileToTar(parentPath); err != nil {
return err
}
addedPaths[parentPath] = true
}
return nil
}

// filesWithLinks returns the symlink and the target path if its exists.
func filesWithLinks(path string) ([]string, error) {
link, err := util.GetSymLink(path)
Expand Down
81 changes: 81 additions & 0 deletions pkg/snapshot/snapshot_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -430,6 +430,67 @@ func TestSnapshotWithForceBuildMetadataIsNotSet(t *testing.T) {
}
}

func TestSnapshotIncludesParentDirBeforeWhiteoutFile(t *testing.T) {
testDir, snapshotter, cleanup, err := setUpTest(t)
defer cleanup()
if err != nil {
t.Fatal(err)
}

// Take a snapshot
filesToSnapshot := []string{filepath.Join(testDir, "kaniko/file", "bar/bat")}
_, err = snapshotter.TakeSnapshot(filesToSnapshot, false, false)
if err != nil {
t.Fatalf("Error taking snapshot of fs: %s", err)
}

// Add a file
newFiles := map[string]string{
"kaniko/new-file": "baz",
}
if err := testutil.SetupFiles(testDir, newFiles); err != nil {
t.Fatalf("Error setting up fs: %s", err)
}
filesToSnapshot = append(filesToSnapshot, filepath.Join(testDir, "kaniko/new-file"))

// Delete files
filesToDelete := []string{"kaniko/file", "bar"}
for _, fn := range filesToDelete {
err = os.RemoveAll(filepath.Join(testDir, fn))
if err != nil {
t.Fatalf("Error deleting file: %s", err)
}
}

// Take a snapshot again
tarPath, err := snapshotter.TakeSnapshot(filesToSnapshot, true, false)
if err != nil {
t.Fatalf("Error taking snapshot of fs: %s", err)
}

actualFiles, err := listFilesInTar(tarPath)
if err != nil {
t.Fatal(err)
}

testDirWithoutLeadingSlash := strings.TrimLeft(testDir, "/")
expectedFiles := []string{
filepath.Join(testDirWithoutLeadingSlash, "kaniko/.wh.file"),
filepath.Join(testDirWithoutLeadingSlash, "kaniko/new-file"),
filepath.Join(testDirWithoutLeadingSlash, ".wh.bar"),
"/",
}
for parentDir := filepath.Dir(expectedFiles[0]); parentDir != "."; parentDir = filepath.Dir(parentDir) {
expectedFiles = append(expectedFiles, parentDir+"/")
}

// Sorting does the right thing in this case. The expected order for a directory is:
// Parent dirs first, then whiteout files in the directory, then other files in that directory
sort.Strings(expectedFiles)

testutil.CheckErrorAndDeepEqual(t, false, nil, expectedFiles, actualFiles)
}

func TestSnasphotPreservesWhiteoutOrder(t *testing.T) {
newFiles := map[string]string{
"foo": "newbaz1",
Expand Down Expand Up @@ -599,3 +660,23 @@ func setUpTest(t *testing.T) (string, *Snapshotter, func(), error) {

return testDir, snapshotter, cleanup, nil
}

func listFilesInTar(path string) ([]string, error) {
f, err := os.Open(path)
if err != nil {
return nil, err
}
tr := tar.NewReader(f)
var files []string
for {
hdr, err := tr.Next()
if err == io.EOF {
break
}
if err != nil {
return nil, err
}
files = append(files, hdr.Name)
}
return files, nil
}

0 comments on commit 5a58e6e

Please sign in to comment.