From d6fe98aa496e259fad0ba290aa9fafa6d186a8d7 Mon Sep 17 00:00:00 2001 From: ohchang-kwon Date: Fri, 3 Jan 2020 16:28:12 +0900 Subject: [PATCH] Fixed an issue where the image is broken if there are symlink in the destination path when ADD, COPY --- pkg/commands/copy.go | 56 +++++++++++++++++++++++------------ pkg/commands/copy_test.go | 14 ++++++++- pkg/util/command_util.go | 25 +++++++++------- pkg/util/command_util_test.go | 6 ++-- 4 files changed, 68 insertions(+), 33 deletions(-) mode change 100644 => 100755 pkg/commands/copy_test.go diff --git a/pkg/commands/copy.go b/pkg/commands/copy.go index fd4b18a04..113689585 100644 --- a/pkg/commands/copy.go +++ b/pkg/commands/copy.go @@ -18,14 +18,13 @@ package commands import ( "fmt" - "os" - "path/filepath" - + "github.com/GoogleContainerTools/kaniko/pkg/constants" "github.com/moby/buildkit/frontend/dockerfile/instructions" "github.com/pkg/errors" "github.com/sirupsen/logrus" - - "github.com/GoogleContainerTools/kaniko/pkg/constants" + "os" + "path/filepath" + "strings" "github.com/GoogleContainerTools/kaniko/pkg/dockerfile" "github.com/GoogleContainerTools/kaniko/pkg/util" @@ -63,6 +62,7 @@ func (c *CopyCommand) ExecuteCommand(config *v1.Config, buildArgs *dockerfile.Bu if cwd == "" { cwd = constants.RootDir } + destPath, err := util.DestinationFilepath(src, dest, cwd) if err != nil { return err @@ -76,11 +76,7 @@ func (c *CopyCommand) ExecuteCommand(config *v1.Config, buildArgs *dockerfile.Bu } if fi.IsDir() { - if !filepath.IsAbs(dest) { - // we need to add '/' to the end to indicate the destination is a directory - dest = filepath.Join(cwd, dest) + "/" - } - copiedFiles, err := util.CopyDir(fullPath, dest, c.buildcontext) + copiedFiles, err := util.CopyDir(fullPath, destPath, c.buildcontext) if err != nil { return err } @@ -197,21 +193,43 @@ func (cr *CachingCopyCommand) From() string { } func resolveIfSymlink(destPath string) (string, error) { - baseDir := filepath.Dir(destPath) - if info, err := os.Lstat(baseDir); err == nil { + if !filepath.IsAbs(destPath) { + return "", errors.New("dest path must be abs") + } + + pathPrefix := "/" + pathTokens := strings.Split(destPath, string(os.PathSeparator)) + for i, pathToken := range pathTokens { + currentPath := filepath.Join(pathPrefix, pathToken) + info, err := os.Lstat(currentPath) + if err != nil { + if os.IsNotExist(err) { + pathPrefix = filepath.Join(append([]string{currentPath}, pathTokens[i+1:]...)...) + break + } else { + return "", errors.Wrap(err, "failed to lstat") + } + } + switch mode := info.Mode(); { case mode&os.ModeSymlink != 0: - linkPath, err := os.Readlink(baseDir) + linkPath, err := os.Readlink(currentPath) if err != nil { - return "", errors.Wrap(err, "error reading symlink") + return "", errors.Wrap(err, "failed to read symlink") } - absLinkPath := filepath.Join(filepath.Dir(baseDir), linkPath) - newPath := filepath.Join(absLinkPath, filepath.Base(destPath)) - logrus.Tracef("Updating destination path from %v to %v due to symlink", destPath, newPath) - return newPath, nil + if filepath.IsAbs(linkPath) { + pathPrefix = linkPath + } else { + pathPrefix = filepath.Join(pathPrefix, linkPath) + } + default: + pathPrefix = filepath.Join(pathPrefix, pathToken) } } - return destPath, nil + if destPath != pathPrefix { + logrus.Tracef("Updating destination path from %v to %v due to symlink", destPath, pathPrefix) + } + return filepath.Clean(pathPrefix), nil } func copyCmdFilesUsedFromContext( diff --git a/pkg/commands/copy_test.go b/pkg/commands/copy_test.go old mode 100644 new mode 100755 index 492211951..b52977c4f --- a/pkg/commands/copy_test.go +++ b/pkg/commands/copy_test.go @@ -201,7 +201,19 @@ func Test_resolveIfSymlink(t *testing.T) { if err := os.Symlink(filepath.Base(thepath), symLink); err != nil { t.Error(err) } - cases = append(cases, testCase{filepath.Join(symLink, "foo.txt"), filepath.Join(thepath, "foo.txt"), nil}) + cases = append(cases, + testCase{filepath.Join(symLink, "foo.txt"), filepath.Join(thepath, "foo.txt"), nil}, + testCase{filepath.Join(symLink, "inner", "foo.txt"), filepath.Join(thepath, "inner", "foo.txt"), nil}, + ) + + absSymlink := filepath.Join(tmpDir, "abs-symlink") + if err := os.Symlink(thepath, absSymlink); err != nil { + t.Error(err) + } + cases = append(cases, + testCase{filepath.Join(absSymlink, "foo.txt"), filepath.Join(thepath, "foo.txt"), nil}, + testCase{filepath.Join(absSymlink, "inner", "foo.txt"), filepath.Join(thepath, "inner", "foo.txt"), nil}, + ) for i, c := range cases { t.Run(fmt.Sprintf("%d", i), func(t *testing.T) { diff --git a/pkg/util/command_util.go b/pkg/util/command_util.go index 103d22e0b..563d1a08d 100644 --- a/pkg/util/command_util.go +++ b/pkg/util/command_util.go @@ -164,20 +164,25 @@ func IsDestDir(path string) bool { // If dest is a dir, copy it to /dest/relpath // If dest is a file, copy directly to dest // If source is a dir: -// Assume dest is also a dir, and copy to dest/relpath +// Assume dest is also a dir, and copy to dest/ // If dest is not an absolute filepath, add /cwd to the beginning func DestinationFilepath(src, dest, cwd string) (string, error) { - if IsDestDir(dest) { - destPath := filepath.Join(dest, filepath.Base(src)) - if filepath.IsAbs(dest) { - return destPath, nil - } - return filepath.Join(cwd, destPath), nil + _, srcFileName := filepath.Split(src) + newDest := dest + + if IsDestDir(newDest) { + newDest = filepath.Join(newDest, srcFileName) } - if filepath.IsAbs(dest) { - return dest, nil + + if !filepath.IsAbs(newDest) { + newDest = filepath.Join(cwd, newDest) } - return filepath.Join(cwd, dest), nil + + if len(srcFileName) <= 0 && !strings.HasSuffix(newDest, "/") { + newDest += "/" + } + + return newDest, nil } // URLDestinationFilepath gives the destination a file from a remote URL should be saved to diff --git a/pkg/util/command_util_test.go b/pkg/util/command_util_test.go index 97a8fd7d4..8e85b3e25 100644 --- a/pkg/util/command_util_test.go +++ b/pkg/util/command_util_test.go @@ -153,13 +153,13 @@ var destinationFilepathTests = []struct { src: "context/bar/", cwd: "/", dest: "pkg/", - expectedFilepath: "/pkg/bar", + expectedFilepath: "/pkg/", }, { src: "context/bar/", cwd: "/newdir", dest: "pkg/", - expectedFilepath: "/newdir/pkg/bar", + expectedFilepath: "/newdir/pkg/", }, { src: "./context/empty", @@ -177,7 +177,7 @@ var destinationFilepathTests = []struct { src: "./", cwd: "/", dest: "/dir", - expectedFilepath: "/dir", + expectedFilepath: "/dir/", }, { src: "context/foo",