From 8f66e8613f36e52daca4c42d08dff8961e793628 Mon Sep 17 00:00:00 2001 From: Cole Wippern Date: Tue, 12 Nov 2019 14:37:48 -0800 Subject: [PATCH 1/4] Add new test for copy to symlink which should fail --- integration/dockerfiles/Dockerfile_test_add_dest_symlink_dir | 2 ++ 1 file changed, 2 insertions(+) create mode 100644 integration/dockerfiles/Dockerfile_test_add_dest_symlink_dir diff --git a/integration/dockerfiles/Dockerfile_test_add_dest_symlink_dir b/integration/dockerfiles/Dockerfile_test_add_dest_symlink_dir new file mode 100644 index 0000000000..a3e68e5937 --- /dev/null +++ b/integration/dockerfiles/Dockerfile_test_add_dest_symlink_dir @@ -0,0 +1,2 @@ +FROM phusion/baseimage:0.11 +ADD context/foo /etc/service/foo From 50f1373837c2a8c3af2aff05a5eb7412426e5e77 Mon Sep 17 00:00:00 2001 From: Cole Wippern Date: Tue, 12 Nov 2019 15:01:13 -0800 Subject: [PATCH 2/4] Update Add command RequiresUnpackedFS --- pkg/commands/add.go | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/pkg/commands/add.go b/pkg/commands/add.go index 7a3d6164ba..3ade7ea69f 100644 --- a/pkg/commands/add.go +++ b/pkg/commands/add.go @@ -141,3 +141,7 @@ func (a *AddCommand) FilesUsedFromContext(config *v1.Config, buildArgs *dockerfi func (a *AddCommand) MetadataOnly() bool { return false } + +func (a *AddCommand) RequiresUnpackedFS() bool { + return true +} From 2c13842451223261c5e585f527f1732e510db60d Mon Sep 17 00:00:00 2001 From: Cole Wippern Date: Tue, 12 Nov 2019 17:07:12 -0800 Subject: [PATCH 3/4] Resolve symlink paths --- pkg/commands/copy.go | 26 ++++++++++++++++++++++++++ pkg/util/fs_util.go | 8 +++++++- 2 files changed, 33 insertions(+), 1 deletion(-) diff --git a/pkg/commands/copy.go b/pkg/commands/copy.go index 15f778e8de..ad75f6f3f2 100644 --- a/pkg/commands/copy.go +++ b/pkg/commands/copy.go @@ -66,6 +66,14 @@ func (c *CopyCommand) ExecuteCommand(config *v1.Config, buildArgs *dockerfile.Bu if err != nil { return err } + + // If the destination dir is a symlink we need to resolve the path and use + // that instead of the symlink path + destPath, err = resolveIfSymlink(destPath) + if err != nil { + return err + } + if fi.IsDir() { if !filepath.IsAbs(dest) { // we need to add '/' to the end to indicate the destination is a directory @@ -178,3 +186,21 @@ func (cr *CachingCopyCommand) FilesToSnapshot() []string { func (cr *CachingCopyCommand) String() string { return cr.cmd.String() } + +func resolveIfSymlink(destPath string) (string, error) { + baseDir := filepath.Dir(destPath) + if info, err := os.Lstat(baseDir); err == nil { + switch mode := info.Mode(); { + case mode&os.ModeSymlink != 0: + linkPath, err := os.Readlink(baseDir) + if err != nil { + return "", errors.Wrap(err, "error reading 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 + } + } + return destPath, nil +} diff --git a/pkg/util/fs_util.go b/pkg/util/fs_util.go index 00562e56f3..10fccb7f7b 100644 --- a/pkg/util/fs_util.go +++ b/pkg/util/fs_util.go @@ -424,11 +424,17 @@ func FilepathExists(path string) bool { func CreateFile(path string, reader io.Reader, perm os.FileMode, uid uint32, gid uint32) error { // Create directory path if it doesn't exist baseDir := filepath.Dir(path) - if _, err := os.Lstat(baseDir); os.IsNotExist(err) { + if info, err := os.Lstat(baseDir); os.IsNotExist(err) { logrus.Tracef("baseDir %s for file %s does not exist. Creating.", baseDir, path) if err := os.MkdirAll(baseDir, 0755); err != nil { return err } + } else { + switch mode := info.Mode(); { + case mode&os.ModeSymlink != 0: + logrus.Infof("destination cannot be a symlink %v", baseDir) + return errors.New("destination cannot be a symlink") + } } dest, err := os.Create(path) if err != nil { From 2b26dfea61d9f10f62422bf3f90e4b5abba84525 Mon Sep 17 00:00:00 2001 From: Cole Wippern Date: Fri, 15 Nov 2019 10:28:23 -0800 Subject: [PATCH 4/4] Add unit tests for resolveIfSymlink --- pkg/commands/copy_test.go | 50 +++++++++++++++++++++++++++++++++++++++ 1 file changed, 50 insertions(+) diff --git a/pkg/commands/copy_test.go b/pkg/commands/copy_test.go index 9b16e92916..5f8fb7211d 100644 --- a/pkg/commands/copy_test.go +++ b/pkg/commands/copy_test.go @@ -16,6 +16,7 @@ limitations under the License. package commands import ( + "fmt" "io" "io/ioutil" "os" @@ -165,3 +166,52 @@ func copySetUpBuildArgs() *dockerfile.BuildArgs { buildArgs.AddArg("buildArg2", &d) return buildArgs } + +func Test_resolveIfSymlink(t *testing.T) { + type testCase struct { + destPath string + expectedPath string + err error + } + + tmpDir, err := ioutil.TempDir("", "copy-test") + if err != nil { + t.Error(err) + } + + baseDir, err := ioutil.TempDir(tmpDir, "not-linked") + if err != nil { + t.Error(err) + } + + path, err := ioutil.TempFile(baseDir, "foo.txt") + if err != nil { + t.Error(err) + } + + thepath, err := filepath.Abs(filepath.Dir(path.Name())) + if err != nil { + t.Error(err) + } + cases := []testCase{{destPath: thepath, expectedPath: thepath, err: nil}} + + baseDir = tmpDir + symLink := filepath.Join(baseDir, "symlink") + 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}) + + for i, c := range cases { + t.Run(fmt.Sprintf("%d", i), func(t *testing.T) { + res, e := resolveIfSymlink(c.destPath) + if e != c.err { + t.Errorf("%s: expected %v but got %v", c.destPath, c.err, e) + } + + if res != c.expectedPath { + t.Errorf("%s: expected %v but got %v", c.destPath, c.expectedPath, res) + } + }) + } +}