From 2cd68d2e2f4ac4f9504514559cd676b53c91a507 Mon Sep 17 00:00:00 2001 From: Tejal Desai Date: Wed, 5 Feb 2020 14:40:52 -0800 Subject: [PATCH 1/2] fix flake in copy symlink --- .../dockerfiles/Dockerfile_test_copy_symlink | 10 +++++++++- pkg/commands/copy.go | 4 ++-- pkg/executor/build.go | 4 +++- pkg/util/fs_util.go | 16 ++++++++++------ pkg/util/fs_util_test.go | 2 +- 5 files changed, 25 insertions(+), 11 deletions(-) diff --git a/integration/dockerfiles/Dockerfile_test_copy_symlink b/integration/dockerfiles/Dockerfile_test_copy_symlink index e517d8ce7a..82f5774c68 100644 --- a/integration/dockerfiles/Dockerfile_test_copy_symlink +++ b/integration/dockerfiles/Dockerfile_test_copy_symlink @@ -2,5 +2,13 @@ FROM busybox as t RUN echo "hello" > /tmp/target RUN ln -s /tmp/target /tmp/link +## Relative link +RUN cd tmp && ln -s target relative_link + +## Relative link with paths +RUN mkdir workdir && cd workdir && ln -s ../tmp/target relative_dir_link + FROM scratch -COPY --from=t /tmp/link /tmp \ No newline at end of file +COPY --from=t /tmp/link /tmp/ +COPY --from=t /tmp/relative_link /tmp/ +COPY --from=t /workdir/relative_dir_link /workdir/ \ No newline at end of file diff --git a/pkg/commands/copy.go b/pkg/commands/copy.go index 25a78cdb93..aed03cb3a5 100644 --- a/pkg/commands/copy.go +++ b/pkg/commands/copy.go @@ -102,8 +102,8 @@ func (c *CopyCommand) ExecuteCommand(config *v1.Config, buildArgs *dockerfile.Bu } c.snapshotFiles = append(c.snapshotFiles, copiedFiles...) } else if util.IsSymlink(fi) { - // If file is a symlink, we want to create the same relative symlink - exclude, err := util.CopySymlink(fullPath, destPath, c.buildcontext) + // If file is a symlink, we want to copy the target file to destPath + exclude, err := util.CopySymlink(fullPath, destPath, c.buildcontext, uid, gid) if err != nil { return err } diff --git a/pkg/executor/build.go b/pkg/executor/build.go index b8e7f85b89..a4f85a6c76 100644 --- a/pkg/executor/build.go +++ b/pkg/executor/build.go @@ -575,7 +575,9 @@ func DoBuild(opts *config.KanikoOptions) (v1.Image, error) { } for _, p := range filesToSave { logrus.Infof("Saving file %s for later use", p) - util.CopyFileOrSymlink(p, dstDir) + if err := util.CopyFileOrSymlink(p, dstDir); err != nil { + return nil, err + } } // Delete the filesystem diff --git a/pkg/util/fs_util.go b/pkg/util/fs_util.go index 9236effc8c..afd20e65b8 100644 --- a/pkg/util/fs_util.go +++ b/pkg/util/fs_util.go @@ -584,7 +584,7 @@ func CopyDir(src, dest, buildcontext string, uid, gid int64) ([]string, error) { } } else if IsSymlink(fi) { // If file is a symlink, we want to create the same relative symlink - if _, err := CopySymlink(fullPath, destPath, buildcontext); err != nil { + if _, err := CopySymlink(fullPath, destPath, buildcontext, uid, gid); err != nil { return nil, err } } else { @@ -599,12 +599,12 @@ func CopyDir(src, dest, buildcontext string, uid, gid int64) ([]string, error) { } // CopySymlink copies the symlink at src to dest -func CopySymlink(src, dest, buildcontext string) (bool, error) { +func CopySymlink(src, dest, buildcontext string, uid int64, gid int64) (bool, error) { if ExcludeFile(src, buildcontext) { logrus.Debugf("%s found in .dockerignore, ignoring", src) return true, nil } - link, err := os.Readlink(src) + link, err := filepath.EvalSymlinks(src) if err != nil { return false, err } @@ -616,7 +616,7 @@ func CopySymlink(src, dest, buildcontext string) (bool, error) { if err := createParentDirectory(dest); err != nil { return false, err } - return false, os.Symlink(link, dest) + return CopyFile(link, dest, buildcontext, uid, gid) } // CopyFile copies the file at src to dest @@ -789,11 +789,15 @@ func getSymlink(path string) error { func CopyFileOrSymlink(src string, destDir string) error { destFile := filepath.Join(destDir, src) if fi, _ := os.Lstat(src); IsSymlink(fi) { - link, err := os.Readlink(src) + link, err := EvalSymLink(src) if err != nil { return err } - return os.Symlink(link, destFile) + linkPath := filepath.Join(destDir, link) + if err := createParentDirectory(destFile); err != nil { + return err + } + return os.Symlink(linkPath, destFile) } return otiai10Cpy.Copy(src, destFile) } diff --git a/pkg/util/fs_util_test.go b/pkg/util/fs_util_test.go index 388292da49..b9c1131844 100644 --- a/pkg/util/fs_util_test.go +++ b/pkg/util/fs_util_test.go @@ -835,7 +835,7 @@ func TestCopySymlink(t *testing.T) { if err := os.Symlink(tc.linkTarget, link); err != nil { t.Fatal(err) } - if _, err := CopySymlink(link, dest, ""); err != nil { + if _, err := CopySymlink(link, dest, "", 0, 0); err != nil { t.Fatal(err) } got, err := os.Readlink(dest) From 9e17ffd6cbbc1d9d9b25a83184412d5049b23eeb Mon Sep 17 00:00:00 2001 From: Tejal Desai Date: Wed, 5 Feb 2020 15:59:07 -0800 Subject: [PATCH 2/2] fix unit tests --- pkg/util/fs_util_test.go | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-) diff --git a/pkg/util/fs_util_test.go b/pkg/util/fs_util_test.go index b9c1131844..dcb17771e1 100644 --- a/pkg/util/fs_util_test.go +++ b/pkg/util/fs_util_test.go @@ -835,16 +835,12 @@ func TestCopySymlink(t *testing.T) { if err := os.Symlink(tc.linkTarget, link); err != nil { t.Fatal(err) } - if _, err := CopySymlink(link, dest, "", 0, 0); err != nil { + if _, err := CopySymlink(link, dest, "", DoNotChangeUID, DoNotChangeGID); err != nil { t.Fatal(err) } - got, err := os.Readlink(dest) - if err != nil { + if _, err := os.Lstat(dest); err != nil { t.Fatalf("error reading link %s: %s", link, err) } - if got != tc.linkTarget { - t.Errorf("link target does not match: %s != %s", got, tc.linkTarget) - } }) } }