Merge pull request #857 from cvgw/u/cgwippern/symlink-bug

Resolve symlink targets to abs path before copying

integration/dockerfiles/Dockerfile_test_add_dest_symlink_dir

@@ -0,0 +1,2 @@
+FROM phusion/baseimage:0.11
+ADD context/foo /etc/service/foo

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
+}

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
+}

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)
+			}
+		})
+	}
+}

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 {