From c4c92e66cdb9fa4c141b4fa4872af37037e1bbe2 Mon Sep 17 00:00:00 2001 From: Tibor Vass Date: Tue, 27 May 2014 14:49:43 -0700 Subject: [PATCH 1/5] add integration test Docker-DCO-1.1-Signed-off-by: Tibor Vass (github: tiborvass) --- integration-cli/docker_cli_run_test.go | 30 ++++++++++++++++++++++++++ 1 file changed, 30 insertions(+) diff --git a/integration-cli/docker_cli_run_test.go b/integration-cli/docker_cli_run_test.go index e937863a4..3efc0f2b2 100644 --- a/integration-cli/docker_cli_run_test.go +++ b/integration-cli/docker_cli_run_test.go @@ -484,6 +484,36 @@ func TestVolumeWithSymlink(t *testing.T) { logDone("run - 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) { cmd := exec.Command(dockerBinary, "run", "busybox", "/bin/sh", "-c", "exit 72") From def86d0cf4d80e037f2ecabdff68bab6652cb741 Mon Sep 17 00:00:00 2001 From: Tibor Vass Date: Tue, 27 May 2014 14:54:38 -0700 Subject: [PATCH 2/5] rename TestVolumeWithSymlink to TestCreateVolumeWithSymlink and remove run_tests folder Docker-DCO-1.1-Signed-off-by: Tibor Vass (github: tiborvass) --- integration-cli/docker_cli_run_test.go | 22 +++++++++---------- .../TestVolumeWithSymlink/Dockerfile | 3 --- 2 files changed, 11 insertions(+), 14 deletions(-) delete mode 100644 integration-cli/run_tests/TestVolumeWithSymlink/Dockerfile diff --git a/integration-cli/docker_cli_run_test.go b/integration-cli/docker_cli_run_test.go index 3efc0f2b2..fc71f0182 100644 --- a/integration-cli/docker_cli_run_test.go +++ b/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,10 @@ 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`. diff --git a/integration-cli/run_tests/TestVolumeWithSymlink/Dockerfile b/integration-cli/run_tests/TestVolumeWithSymlink/Dockerfile deleted file mode 100644 index 46bed8540..000000000 --- a/integration-cli/run_tests/TestVolumeWithSymlink/Dockerfile +++ /dev/null @@ -1,3 +0,0 @@ -FROM busybox - -RUN mkdir /foo && ln -s /foo /bar From 65d4047cb60ec77a4009d25c91b29f95375c8fe5 Mon Sep 17 00:00:00 2001 From: Tibor Vass Date: Tue, 27 May 2014 17:10:00 -0700 Subject: [PATCH 3/5] expose unresolved path for volumes and resolve symlink in container.getResourcePath Docker-DCO-1.1-Signed-off-by: Tibor Vass (github: tiborvass) --- daemon/container.go | 6 +++++- daemon/volumes.go | 4 ++-- 2 files changed, 7 insertions(+), 3 deletions(-) diff --git a/daemon/container.go b/daemon/container.go index 57b3684d4..b2782ee22 100644 --- a/daemon/container.go +++ b/daemon/container.go @@ -139,7 +139,11 @@ func (container *Container) WriteHostConfig() (err error) { func (container *Container) getResourcePath(path string) string { cleanPath := filepath.Join("/", path) - return filepath.Join(container.basefs, cleanPath) + result, err := symlink.FollowSymlinkInScope(filepath.Join(container.basefs, cleanPath), container.basefs) + if err != nil { + utils.Errorf("getResourcePath failed: %v", err) + } + return result } func (container *Container) getRootResourcePath(path string) string { diff --git a/daemon/volumes.go b/daemon/volumes.go index d9719369a..262e8c61e 100644 --- a/daemon/volumes.go +++ b/daemon/volumes.go @@ -280,8 +280,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 From 5c069940db2042d83bd4ef462a8a50d565aaf874 Mon Sep 17 00:00:00 2001 From: cyphar Date: Wed, 28 May 2014 12:15:42 +1000 Subject: [PATCH 4/5] daemon: *: updated getResourcePath and getRootResourcePath signatures This patch updates container.getResourcePath and container.getRootResourcePath to return the error from symlink.FollowSymlinkInScope (rather than using utils). Docker-DCO-1.1-Signed-off-by: Aleksa Sarai (github: cyphar) Remove Inject to help rebase Docker-DCO-1.1-Signed-off-by: Tibor Vass (github: tiborvass) Docker-DCO-1.1-Signed-off-by: cyphar (github: tiborvass) --- daemon/container.go | 134 +++++++++++++++++++++++++++++++++----------- daemon/volumes.go | 9 ++- 2 files changed, 109 insertions(+), 34 deletions(-) diff --git a/daemon/container.go b/daemon/container.go index b2782ee22..09806cffa 100644 --- a/daemon/container.go +++ b/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 } - err = ioutil.WriteFile(container.jsonPath(), data, 0666) + + pth, err := container.jsonPath() if err != nil { - return + return err } + + err = ioutil.WriteFile(pth, data, 0666) + if err != nil { + return err + } + return container.WriteHostConfig() } @@ -118,37 +130,59 @@ 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) -} -func (container *Container) getResourcePath(path string) string { - cleanPath := filepath.Join("/", path) - result, err := symlink.FollowSymlinkInScope(filepath.Join(container.basefs, cleanPath), container.basefs) + pth, err := container.hostConfigPath() if err != nil { - utils.Errorf("getResourcePath failed: %v", err) + return err } - return result + + return ioutil.WriteFile(pth, data, 0666) } -func (container *Container) getRootResourcePath(path string) string { +func (container *Container) getResourcePath(path string) (string, error) { cleanPath := filepath.Join("/", path) - return filepath.Join(container.root, cleanPath) + fullPath := filepath.Join(container.basefs, cleanPath) + + result, err := symlink.FollowSymlinkInScope(fullPath, container.basefs) + if err != nil { + return "", err + } + + return result, nil +} + +func (container *Container) getRootResourcePath(path string) (string, error) { + cleanPath := filepath.Join("/", path) + fullPath := filepath.Join(container.root, cleanPath) + + result, err := symlink.FollowSymlinkInScope(fullPath, container.basefs) + if err != nil { + return "", err + } + + return result, nil } func populateCommand(c *Container, env []string) error { @@ -328,7 +362,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) } @@ -340,7 +379,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) @@ -685,19 +728,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") } @@ -760,8 +807,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 @@ -870,7 +916,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" @@ -903,7 +955,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 @@ -1019,12 +1076,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 } } @@ -1037,12 +1100,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 } diff --git a/daemon/volumes.go b/daemon/volumes.go index 262e8c61e..f4b3921c9 100644 --- a/daemon/volumes.go +++ b/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 } From 48907d57ede696c68e210cb93cb405124a49cbd3 Mon Sep 17 00:00:00 2001 From: Tibor Vass Date: Wed, 28 May 2014 16:12:29 -0700 Subject: [PATCH 5/5] fix bug in getRootResourcePath in previous commit Docker-DCO-1.1-Signed-off-by: Tibor Vass (github: tiborvass) --- daemon/container.go | 18 ++---------------- 1 file changed, 2 insertions(+), 16 deletions(-) diff --git a/daemon/container.go b/daemon/container.go index 09806cffa..2fd827eb9 100644 --- a/daemon/container.go +++ b/daemon/container.go @@ -163,26 +163,12 @@ func (container *Container) WriteHostConfig() error { func (container *Container) getResourcePath(path string) (string, error) { cleanPath := filepath.Join("/", path) - fullPath := filepath.Join(container.basefs, cleanPath) - - result, err := symlink.FollowSymlinkInScope(fullPath, container.basefs) - if err != nil { - return "", err - } - - return result, nil + return symlink.FollowSymlinkInScope(filepath.Join(container.basefs, cleanPath), container.basefs) } func (container *Container) getRootResourcePath(path string) (string, error) { cleanPath := filepath.Join("/", path) - fullPath := filepath.Join(container.root, cleanPath) - - result, err := symlink.FollowSymlinkInScope(fullPath, container.basefs) - if err != nil { - return "", err - } - - return result, nil + return symlink.FollowSymlinkInScope(filepath.Join(container.root, cleanPath), container.root) } func populateCommand(c *Container, env []string) error {