Merge pull request #6066 from tiborvass/5693-volumes-from-symlink-path

5693 volumes from symlink path

daemon/container.go

@@ -85,7 +85,12 @@ type Container struct {
 }
 
 func (container *Container) FromDisk() error {
-	data, err := ioutil.ReadFile(container.jsonPath())
+	pth, err := container.jsonPath()
+	if err != nil {
+		return err
+	}
+
+	data, err := ioutil.ReadFile(pth)
 	if err != nil {
 		return err
 	}
@@ -101,15 +106,22 @@ func (container *Container) FromDisk() error {
 	return container.readHostConfig()
 }
 
-func (container *Container) ToDisk() (err error) {
+func (container *Container) ToDisk() error {
 	data, err := json.Marshal(container)
 	if err != nil {
-		return
+		return err
+	}
+
+	pth, err := container.jsonPath()
+	if err != nil {
+		return err
 	}
-	err = ioutil.WriteFile(container.jsonPath(), data, 0666)
+
+	err = ioutil.WriteFile(pth, data, 0666)
 	if err != nil {
-		return
+		return err
 	}
+
 	return container.WriteHostConfig()
 }
 
@@ -118,33 +130,45 @@ func (container *Container) readHostConfig() error {
 	// If the hostconfig file does not exist, do not read it.
 	// (We still have to initialize container.hostConfig,
 	// but that's OK, since we just did that above.)
-	_, err := os.Stat(container.hostConfigPath())
+	pth, err := container.hostConfigPath()
+	if err != nil {
+		return err
+	}
+
+	_, err = os.Stat(pth)
 	if os.IsNotExist(err) {
 		return nil
 	}
-	data, err := ioutil.ReadFile(container.hostConfigPath())
+
+	data, err := ioutil.ReadFile(pth)
 	if err != nil {
 		return err
 	}
 	return json.Unmarshal(data, container.hostConfig)
 }
 
-func (container *Container) WriteHostConfig() (err error) {
+func (container *Container) WriteHostConfig() error {
 	data, err := json.Marshal(container.hostConfig)
 	if err != nil {
-		return
+		return err
 	}
-	return ioutil.WriteFile(container.hostConfigPath(), data, 0666)
+
+	pth, err := container.hostConfigPath()
+	if err != nil {
+		return err
+	}
+
+	return ioutil.WriteFile(pth, data, 0666)
 }
 
-func (container *Container) getResourcePath(path string) string {
+func (container *Container) getResourcePath(path string) (string, error) {
 	cleanPath := filepath.Join("/", path)
-	return filepath.Join(container.basefs, cleanPath)
+	return symlink.FollowSymlinkInScope(filepath.Join(container.basefs, cleanPath), container.basefs)
 }
 
-func (container *Container) getRootResourcePath(path string) string {
+func (container *Container) getRootResourcePath(path string) (string, error) {
 	cleanPath := filepath.Join("/", path)
-	return filepath.Join(container.root, cleanPath)
+	return symlink.FollowSymlinkInScope(filepath.Join(container.root, cleanPath), container.root)
 }
 
 func populateCommand(c *Container, env []string) error {
@@ -324,7 +348,12 @@ func (container *Container) StderrLogPipe() io.ReadCloser {
 }
 
 func (container *Container) buildHostnameFile() error {
-	container.HostnamePath = container.getRootResourcePath("hostname")
+	hostnamePath, err := container.getRootResourcePath("hostname")
+	if err != nil {
+		return err
+	}
+	container.HostnamePath = hostnamePath
+
 	if container.Config.Domainname != "" {
 		return ioutil.WriteFile(container.HostnamePath, []byte(fmt.Sprintf("%s.%s\n", container.Config.Hostname, container.Config.Domainname)), 0644)
 	}
@@ -336,7 +365,11 @@ func (container *Container) buildHostnameAndHostsFiles(IP string) error {
 		return err
 	}
 
-	container.HostsPath = container.getRootResourcePath("hosts")
+	hostsPath, err := container.getRootResourcePath("hosts")
+	if err != nil {
+		return err
+	}
+	container.HostsPath = hostsPath
 
 	extraContent := make(map[string]string)
 
@@ -681,19 +714,23 @@ func (container *Container) Unmount() error {
 	return container.daemon.Unmount(container)
 }
 
-func (container *Container) logPath(name string) string {
+func (container *Container) logPath(name string) (string, error) {
 	return container.getRootResourcePath(fmt.Sprintf("%s-%s.log", container.ID, name))
 }
 
 func (container *Container) ReadLog(name string) (io.Reader, error) {
-	return os.Open(container.logPath(name))
+	pth, err := container.logPath(name)
+	if err != nil {
+		return nil, err
+	}
+	return os.Open(pth)
 }
 
-func (container *Container) hostConfigPath() string {
+func (container *Container) hostConfigPath() (string, error) {
 	return container.getRootResourcePath("hostconfig.json")
 }
 
-func (container *Container) jsonPath() string {
+func (container *Container) jsonPath() (string, error) {
 	return container.getRootResourcePath("config.json")
 }
 
@@ -756,8 +793,7 @@ func (container *Container) Copy(resource string) (io.ReadCloser, error) {
 
 	var filter []string
 
-	resPath := container.getResourcePath(resource)
-	basePath, err := symlink.FollowSymlinkInScope(resPath, container.basefs)
+	basePath, err := container.getResourcePath(resource)
 	if err != nil {
 		container.Unmount()
 		return nil, err
@@ -866,7 +902,13 @@ func (container *Container) setupContainerDns() error {
 		} else if len(daemon.config.DnsSearch) > 0 {
 			dnsSearch = daemon.config.DnsSearch
 		}
-		container.ResolvConfPath = container.getRootResourcePath("resolv.conf")
+
+		resolvConfPath, err := container.getRootResourcePath("resolv.conf")
+		if err != nil {
+			return err
+		}
+		container.ResolvConfPath = resolvConfPath
+
 		return resolvconf.Build(container.ResolvConfPath, dns, dnsSearch)
 	} else {
 		container.ResolvConfPath = "/etc/resolv.conf"
@@ -899,7 +941,12 @@ func (container *Container) initializeNetworking() error {
 			return err
 		}
 
-		container.HostsPath = container.getRootResourcePath("hosts")
+		hostsPath, err := container.getRootResourcePath("hosts")
+		if err != nil {
+			return err
+		}
+		container.HostsPath = hostsPath
+
 		return ioutil.WriteFile(container.HostsPath, content, 0644)
 	} else if container.hostConfig.NetworkMode.IsContainer() {
 		// we need to get the hosts files from the container to join
@@ -1015,12 +1062,18 @@ func (container *Container) setupWorkingDirectory() error {
 	if container.Config.WorkingDir != "" {
 		container.Config.WorkingDir = path.Clean(container.Config.WorkingDir)
 
-		pthInfo, err := os.Stat(container.getResourcePath(container.Config.WorkingDir))
+		pth, err := container.getResourcePath(container.Config.WorkingDir)
+		if err != nil {
+			return err
+		}
+
+		pthInfo, err := os.Stat(pth)
 		if err != nil {
 			if !os.IsNotExist(err) {
 				return err
 			}
-			if err := os.MkdirAll(container.getResourcePath(container.Config.WorkingDir), 0755); err != nil {
+
+			if err := os.MkdirAll(pth, 0755); err != nil {
 				return err
 			}
 		}
@@ -1033,12 +1086,19 @@ func (container *Container) setupWorkingDirectory() error {
 
 func (container *Container) startLoggingToDisk() error {
 	// Setup logging of stdout and stderr to disk
-	if err := container.daemon.LogToDisk(container.stdout, container.logPath("json"), "stdout"); err != nil {
+	pth, err := container.logPath("json")
+	if err != nil {
 		return err
 	}
-	if err := container.daemon.LogToDisk(container.stderr, container.logPath("json"), "stderr"); err != nil {
+
+	if err := container.daemon.LogToDisk(container.stdout, pth, "stdout"); err != nil {
 		return err
 	}
+
+	if err := container.daemon.LogToDisk(container.stderr, pth, "stderr"); err != nil {
+		return err
+	}
+
 	return nil
 }
 

daemon/volumes.go

@@ -98,12 +98,17 @@ func applyVolumesFrom(container *Container) error {
 					continue
 				}
 
-				stat, err := os.Stat(c.getResourcePath(volPath))
+				pth, err := c.getResourcePath(volPath)
 				if err != nil {
 					return err
 				}
 
-				if err := createIfNotExists(container.getResourcePath(volPath), stat.IsDir()); err != nil {
+				stat, err := os.Stat(pth)
+				if err != nil {
+					return err
+				}
+
+				if err := createIfNotExists(pth, stat.IsDir()); err != nil {
 					return err
 				}
 
@@ -280,8 +285,8 @@ func initializeVolume(container *Container, volPath string, binds map[string]Bin
 		delete(container.VolumesRW, volPath)
 	}
 
-	container.Volumes[newVolPath] = destination
-	container.VolumesRW[newVolPath] = srcRW
+	container.Volumes[volPath] = destination
+	container.VolumesRW[volPath] = srcRW
 
 	if err := createIfNotExists(source, volIsDir); err != nil {
 		return err

integration-cli/docker_cli_run_test.go

@@ -4,7 +4,6 @@ import (
 	"fmt"
 	"os"
 	"os/exec"
-	"path/filepath"
 	"reflect"
 	"regexp"
 	"sort"
@@ -444,29 +443,30 @@ func TestCreateVolume(t *testing.T) {
 
 // Test that creating a volume with a symlink in its path works correctly. Test for #5152.
 // Note that this bug happens only with symlinks with a target that starts with '/'.
-func TestVolumeWithSymlink(t *testing.T) {
-	buildDirectory := filepath.Join(workingDirectory, "run_tests", "TestVolumeWithSymlink")
-	buildCmd := exec.Command(dockerBinary, "build", "-t", "docker-test-volumewithsymlink", ".")
-	buildCmd.Dir = buildDirectory
+func TestCreateVolumeWithSymlink(t *testing.T) {
+	buildCmd := exec.Command(dockerBinary, "build", "-t", "docker-test-createvolumewithsymlink", "-")
+	buildCmd.Stdin = strings.NewReader(`FROM busybox
+		RUN mkdir /foo && ln -s /foo /bar`)
+	buildCmd.Dir = workingDirectory
 	err := buildCmd.Run()
 	if err != nil {
-		t.Fatalf("could not build 'docker-test-volumewithsymlink': %v", err)
+		t.Fatalf("could not build 'docker-test-createvolumewithsymlink': %v", err)
 	}
 
-	cmd := exec.Command(dockerBinary, "run", "-v", "/bar/foo", "--name", "test-volumewithsymlink", "docker-test-volumewithsymlink", "sh", "-c", "mount | grep -q /foo/foo")
+	cmd := exec.Command(dockerBinary, "run", "-v", "/bar/foo", "--name", "test-createvolumewithsymlink", "docker-test-createvolumewithsymlink", "sh", "-c", "mount | grep -q /foo/foo")
 	exitCode, err := runCommand(cmd)
 	if err != nil || exitCode != 0 {
 		t.Fatalf("[run] err: %v, exitcode: %d", err, exitCode)
 	}
 
 	var volPath string
-	cmd = exec.Command(dockerBinary, "inspect", "-f", "{{range .Volumes}}{{.}}{{end}}", "test-volumewithsymlink")
+	cmd = exec.Command(dockerBinary, "inspect", "-f", "{{range .Volumes}}{{.}}{{end}}", "test-createvolumewithsymlink")
 	volPath, exitCode, err = runCommandWithOutput(cmd)
 	if err != nil || exitCode != 0 {
 		t.Fatalf("[inspect] err: %v, exitcode: %d", err, exitCode)
 	}
 
-	cmd = exec.Command(dockerBinary, "rm", "-v", "test-volumewithsymlink")
+	cmd = exec.Command(dockerBinary, "rm", "-v", "test-createvolumewithsymlink")
 	exitCode, err = runCommand(cmd)
 	if err != nil || exitCode != 0 {
 		t.Fatalf("[rm] err: %v, exitcode: %d", err, exitCode)
@@ -478,10 +478,40 @@ func TestVolumeWithSymlink(t *testing.T) {
 		t.Fatalf("[open] (expecting 'file does not exist' error) err: %v, volPath: %s", err, volPath)
 	}
 
-	deleteImages("docker-test-volumewithsymlink")
+	deleteImages("docker-test-createvolumewithsymlink")
 	deleteAllContainers()
 
-	logDone("run - volume with symlink")
+	logDone("run - create volume with symlink")
+}
+
+// Tests that a volume path that has a symlink exists in a container mounting it with `--volumes-from`.
+func TestVolumesFromSymlinkPath(t *testing.T) {
+	buildCmd := exec.Command(dockerBinary, "build", "-t", "docker-test-volumesfromsymlinkpath", "-")
+	buildCmd.Stdin = strings.NewReader(`FROM busybox
+		RUN mkdir /baz && ln -s /baz /foo
+		VOLUME ["/foo/bar"]`)
+	buildCmd.Dir = workingDirectory
+	err := buildCmd.Run()
+	if err != nil {
+		t.Fatalf("could not build 'docker-test-volumesfromsymlinkpath': %v", err)
+	}
+
+	cmd := exec.Command(dockerBinary, "run", "--name", "test-volumesfromsymlinkpath", "docker-test-volumesfromsymlinkpath")
+	exitCode, err := runCommand(cmd)
+	if err != nil || exitCode != 0 {
+		t.Fatalf("[run] (volume) err: %v, exitcode: %d", err, exitCode)
+	}
+
+	cmd = exec.Command(dockerBinary, "run", "--volumes-from", "test-volumesfromsymlinkpath", "busybox", "sh", "-c", "ls /foo | grep -q bar")
+	exitCode, err = runCommand(cmd)
+	if err != nil || exitCode != 0 {
+		t.Fatalf("[run] err: %v, exitcode: %d", err, exitCode)
+	}
+
+	deleteImages("docker-test-volumesfromsymlinkpath")
+	deleteAllContainers()
+
+	logDone("run - volumes-from symlink path")
 }
 
 func TestExitCode(t *testing.T) {

integration-cli/run_tests/TestVolumeWithSymlink/Dockerfile

@@ -1,3 +0,0 @@
-FROM busybox
-
-RUN mkdir /foo && ln -s /foo /bar