diff --git a/integration/dockerfiles/TestReplaceFolderWithFile b/integration/dockerfiles/TestReplaceFolderWithFile new file mode 100644 index 000000000..ca3e6f2a7 --- /dev/null +++ b/integration/dockerfiles/TestReplaceFolderWithFile @@ -0,0 +1,5 @@ +# Not prefixed Dockerfile_test to exclude it from TestRun() +FROM busybox + +RUN mkdir /a /b /c && echo a > /a/d +RUN rm -r /a && echo "foo" > /a \ No newline at end of file diff --git a/integration/dockerfiles/TestReplaceFolderWithLink b/integration/dockerfiles/TestReplaceFolderWithLink new file mode 100644 index 000000000..d22a550df --- /dev/null +++ b/integration/dockerfiles/TestReplaceFolderWithLink @@ -0,0 +1,5 @@ +# Not prefixed Dockerfile_test to exclude it from TestRun() +FROM busybox + +RUN mkdir /a /b /c && echo a > /a/d +RUN rm -r /a && ln -sf /b /a \ No newline at end of file diff --git a/integration/integration_test.go b/integration/integration_test.go index 323901d00..b4785ea30 100644 --- a/integration/integration_test.go +++ b/integration/integration_test.go @@ -17,10 +17,12 @@ limitations under the License. package integration import ( + "archive/tar" "context" "encoding/json" "flag" "fmt" + "io" "log" "math" "os" @@ -33,6 +35,7 @@ import ( "github.com/google/go-containerregistry/pkg/name" "github.com/google/go-containerregistry/pkg/v1/daemon" + "github.com/google/go-containerregistry/pkg/v1/remote" "github.com/pkg/errors" "google.golang.org/api/option" @@ -549,6 +552,28 @@ func TestLayers(t *testing.T) { } } +func TestReplaceFolderWithFileOrLink(t *testing.T) { + dockerfiles := []string{"TestReplaceFolderWithFile", "TestReplaceFolderWithLink"} + for _, dockerfile := range dockerfiles { + t.Run(dockerfile, func(t *testing.T) { + buildImage(t, dockerfile, imageBuilder) + kanikoImage := GetKanikoImage(config.imageRepo, dockerfile) + + kanikoFiles, err := getLastLayerFiles(kanikoImage) + if err != nil { + t.Fatal(err) + } + fmt.Println(kanikoFiles) + + for _, file := range kanikoFiles { + if strings.HasPrefix(file, "a/.wh.") { + t.Errorf("Last layer should not add whiteout files to deleted directory but found %s", file) + } + } + }) + } +} + func buildImage(t *testing.T, dockerfile string, imageBuilder *DockerFileBuilder) { t.Logf("Building image '%v'...", dockerfile) @@ -868,6 +893,40 @@ func getImageDetails(image string) (*imageDetails, error) { }, nil } +func getLastLayerFiles(image string) ([]string, error) { + ref, err := name.ParseReference(image, name.WeakValidation) + if err != nil { + return nil, fmt.Errorf("Couldn't parse referance to image %s: %w", image, err) + } + + imgRef, err := remote.Image(ref) + if err != nil { + return nil, fmt.Errorf("Couldn't get reference to image %s from daemon: %w", image, err) + } + layers, err := imgRef.Layers() + if err != nil { + return nil, fmt.Errorf("Error getting layers for image %s: %w", image, err) + } + readCloser, err := layers[len(layers)-1].Uncompressed() + if err != nil { + return nil, err + } + + tr := tar.NewReader(readCloser) + var files []string + for { + hdr, err := tr.Next() + if errors.Is(err, io.EOF) { + break + } + if err != nil { + return nil, err + } + files = append(files, hdr.Name) + } + return files, nil +} + func logBenchmarks(benchmark string) error { if b, err := strconv.ParseBool(os.Getenv("BENCHMARK")); err == nil && b { f, err := os.Create(benchmark) diff --git a/pkg/snapshot/snapshot.go b/pkg/snapshot/snapshot.go index 7aab450fe..018221ffc 100644 --- a/pkg/snapshot/snapshot.go +++ b/pkg/snapshot/snapshot.go @@ -224,6 +224,14 @@ func writeToTar(t util.Tar, files, whiteouts []string) error { addedPaths := make(map[string]bool) for _, path := range whiteouts { + skipWhiteout, err := parentPathIncludesNonDirectory(path) + if err != nil { + return err + } + if skipWhiteout { + continue + } + if err := addParentDirectories(t, addedPaths, path); err != nil { return err } @@ -247,6 +255,21 @@ func writeToTar(t util.Tar, files, whiteouts []string) error { return nil } +// Returns true if a parent of the given path has been replaced with anything other than a directory +func parentPathIncludesNonDirectory(path string) (bool, error) { + for _, parentPath := range util.ParentDirectories(path) { + lstat, err := os.Lstat(parentPath) + if err != nil { + return false, err + } + + if !lstat.IsDir() { + return true, nil + } + } + return false, nil +} + func addParentDirectories(t util.Tar, addedPaths map[string]bool, path string) error { for _, parentPath := range util.ParentDirectories(path) { if _, pathAdded := addedPaths[parentPath]; pathAdded { diff --git a/pkg/snapshot/snapshot_test.go b/pkg/snapshot/snapshot_test.go index d5d19c4e9..e343fd3bd 100644 --- a/pkg/snapshot/snapshot_test.go +++ b/pkg/snapshot/snapshot_test.go @@ -189,6 +189,51 @@ func TestSnapshotFSChangePermissions(t *testing.T) { } } +func TestSnapshotFSReplaceDirWithLink(t *testing.T) { + testDir, snapshotter, cleanup, err := setUpTest(t) + defer cleanup() + if err != nil { + t.Fatal(err) + } + + // replace non-empty directory "bar" with link to file "foo" + bar := filepath.Join(testDir, "bar") + err = os.RemoveAll(bar) + if err != nil { + t.Fatal(err) + } + foo := filepath.Join(testDir, "foo") + err = os.Symlink(foo, bar) + if err != nil { + t.Fatal(err) + } + + tarPath, err := snapshotter.TakeSnapshotFS() + if err != nil { + t.Fatalf("Error taking snapshot of fs: %s", err) + } + + actualFiles, err := listFilesInTar(tarPath) + if err != nil { + t.Fatal(err) + } + + // Expect "bar", which used to be a non-empty directory but now is a symlink. We don't want whiteout files for + // the deleted files in bar, because without a parent directory for them the tar cannot be extracted. + testDirWithoutLeadingSlash := strings.TrimLeft(testDir, "/") + expectedFiles := []string{ + filepath.Join(testDirWithoutLeadingSlash, "bar"), + filepath.Join(testDirWithoutLeadingSlash, "foo"), + } + for _, path := range util.ParentDirectoriesWithoutLeadingSlash(filepath.Join(testDir, "foo")) { + expectedFiles = append(expectedFiles, strings.TrimRight(path, "/")+"/") + } + + sort.Strings(expectedFiles) + sort.Strings(actualFiles) + testutil.CheckErrorAndDeepEqual(t, false, nil, expectedFiles, actualFiles) +} + func TestSnapshotFiles(t *testing.T) { testDir, snapshotter, cleanup, err := setUpTest(t) testDirWithoutLeadingSlash := strings.TrimLeft(testDir, "/")