From c32dde5baadc8c472666ef9d5cead13ab6de28ea Mon Sep 17 00:00:00 2001 From: Josh Hawn Date: Tue, 12 May 2015 18:21:26 -0700 Subject: [PATCH] daemon: container ArchivePath and ExtractToDir The following methods will deprecate the Copy method and introduce two new, well-behaved methods for creating a tar archive of a resource in a container and for extracting a tar archive into a directory in a container. Docker-DCO-1.1-Signed-off-by: Josh Hawn (github: jlhawn) --- api/types/types.go | 11 ++ daemon/archive.go | 297 ++++++++++++++++++++++++++++++++++++++++++++ daemon/container.go | 82 ++++++++---- daemon/copy.go | 16 --- daemon/volumes.go | 15 +++ 5 files changed, 378 insertions(+), 43 deletions(-) create mode 100644 daemon/archive.go delete mode 100644 daemon/copy.go diff --git a/api/types/types.go b/api/types/types.go index a27755f69f..97f1ad19dc 100644 --- a/api/types/types.go +++ b/api/types/types.go @@ -1,6 +1,7 @@ package types import ( + "os" "time" "github.com/docker/docker/daemon/network" @@ -127,6 +128,16 @@ type CopyConfig struct { Resource string } +// ContainerPathStat is used to encode the response from +// GET /containers/{name:.*}/stat-path +type ContainerPathStat struct { + Name string `json:"name"` + Path string `json:"path"` + Size int64 `json:"size"` + Mode os.FileMode `json:"mode"` + Mtime time.Time `json:"mtime"` +} + // GET "/containers/{name:.*}/top" type ContainerProcessList struct { Processes [][]string diff --git a/daemon/archive.go b/daemon/archive.go new file mode 100644 index 0000000000..f6b5698353 --- /dev/null +++ b/daemon/archive.go @@ -0,0 +1,297 @@ +package daemon + +import ( + "errors" + "io" + "os" + "path/filepath" + + "github.com/docker/docker/api/types" + "github.com/docker/docker/pkg/archive" + "github.com/docker/docker/pkg/chrootarchive" + "github.com/docker/docker/pkg/ioutils" +) + +// ErrExtractPointNotDirectory is used to convey that the operation to extract +// a tar archive to a directory in a container has failed because the specified +// path does not refer to a directory. +var ErrExtractPointNotDirectory = errors.New("extraction point is not a directory") + +// ContainerCopy performs a depracated operation of archiving the resource at +// the specified path in the conatiner identified by the given name. +func (daemon *Daemon) ContainerCopy(name string, res string) (io.ReadCloser, error) { + container, err := daemon.Get(name) + if err != nil { + return nil, err + } + + if res[0] == '/' { + res = res[1:] + } + + return container.Copy(res) +} + +// ContainerStatPath stats the filesystem resource at the specified path in the +// container identified by the given name. +func (daemon *Daemon) ContainerStatPath(name string, path string) (stat *types.ContainerPathStat, err error) { + container, err := daemon.Get(name) + if err != nil { + return nil, err + } + + return container.StatPath(path) +} + +// ContainerArchivePath creates an archive of the filesystem resource at the +// specified path in the container identified by the given name. Returns a +// tar archive of the resource and whether it was a directory or a single file. +func (daemon *Daemon) ContainerArchivePath(name string, path string) (content io.ReadCloser, stat *types.ContainerPathStat, err error) { + container, err := daemon.Get(name) + if err != nil { + return nil, nil, err + } + + return container.ArchivePath(path) +} + +// ContainerExtractToDir extracts the given archive to the specified location +// in the filesystem of the container identified by the given name. The given +// path must be of a directory in the container. If it is not, the error will +// be ErrExtractPointNotDirectory. If noOverwriteDirNonDir is true then it will +// be an error if unpacking the given content would cause an existing directory +// to be replaced with a non-directory and vice versa. +func (daemon *Daemon) ContainerExtractToDir(name, path string, noOverwriteDirNonDir bool, content io.Reader) error { + container, err := daemon.Get(name) + if err != nil { + return err + } + + return container.ExtractToDir(path, noOverwriteDirNonDir, content) +} + +// StatPath stats the filesystem resource at the specified path in this +// container. Returns stat info about the resource. +func (container *Container) StatPath(path string) (stat *types.ContainerPathStat, err error) { + container.Lock() + defer container.Unlock() + + if err = container.Mount(); err != nil { + return nil, err + } + defer container.Unmount() + + err = container.mountVolumes() + defer container.UnmountVolumes(true) + if err != nil { + return nil, err + } + + // Consider the given path as an absolute path in the container. + absPath := path + if !filepath.IsAbs(absPath) { + absPath = archive.PreserveTrailingDotOrSeparator(filepath.Join("/", path), path) + } + + resolvedPath, err := container.GetResourcePath(absPath) + if err != nil { + return nil, err + } + + // A trailing "." or separator has important meaning. For example, if + // `"foo"` is a symlink to some directory `"dir"`, then `os.Lstat("foo")` + // will stat the link itself, while `os.Lstat("foo/")` will stat the link + // target. If the basename of the path is ".", it means to archive the + // contents of the directory with "." as the first path component rather + // than the name of the directory. This would cause extraction of the + // archive to *not* make another directory, but instead use the current + // directory. + resolvedPath = archive.PreserveTrailingDotOrSeparator(resolvedPath, absPath) + + lstat, err := os.Lstat(resolvedPath) + if err != nil { + return nil, err + } + + return &types.ContainerPathStat{ + Name: lstat.Name(), + Path: absPath, + Size: lstat.Size(), + Mode: lstat.Mode(), + Mtime: lstat.ModTime(), + }, nil +} + +// ArchivePath creates an archive of the filesystem resource at the specified +// path in this container. Returns a tar archive of the resource and stat info +// about the resource. +func (container *Container) ArchivePath(path string) (content io.ReadCloser, stat *types.ContainerPathStat, err error) { + container.Lock() + + defer func() { + if err != nil { + // Wait to unlock the container until the archive is fully read + // (see the ReadCloseWrapper func below) or if there is an error + // before that occurs. + container.Unlock() + } + }() + + if err = container.Mount(); err != nil { + return nil, nil, err + } + + defer func() { + if err != nil { + // unmount any volumes + container.UnmountVolumes(true) + // unmount the container's rootfs + container.Unmount() + } + }() + + if err = container.mountVolumes(); err != nil { + return nil, nil, err + } + + // Consider the given path as an absolute path in the container. + absPath := path + if !filepath.IsAbs(absPath) { + absPath = archive.PreserveTrailingDotOrSeparator(filepath.Join("/", path), path) + } + + resolvedPath, err := container.GetResourcePath(absPath) + if err != nil { + return nil, nil, err + } + + // A trailing "." or separator has important meaning. For example, if + // `"foo"` is a symlink to some directory `"dir"`, then `os.Lstat("foo")` + // will stat the link itself, while `os.Lstat("foo/")` will stat the link + // target. If the basename of the path is ".", it means to archive the + // contents of the directory with "." as the first path component rather + // than the name of the directory. This would cause extraction of the + // archive to *not* make another directory, but instead use the current + // directory. + resolvedPath = archive.PreserveTrailingDotOrSeparator(resolvedPath, absPath) + + lstat, err := os.Lstat(resolvedPath) + if err != nil { + return nil, nil, err + } + + stat = &types.ContainerPathStat{ + Name: lstat.Name(), + Path: absPath, + Size: lstat.Size(), + Mode: lstat.Mode(), + Mtime: lstat.ModTime(), + } + + data, err := archive.TarResource(resolvedPath) + if err != nil { + return nil, nil, err + } + + content = ioutils.NewReadCloserWrapper(data, func() error { + err := data.Close() + container.UnmountVolumes(true) + container.Unmount() + container.Unlock() + return err + }) + + container.LogEvent("archive-path") + + return content, stat, nil +} + +// ExtractToDir extracts the given tar archive to the specified location in the +// filesystem of this container. The given path must be of a directory in the +// container. If it is not, the error will be ErrExtractPointNotDirectory. If +// noOverwriteDirNonDir is true then it will be an error if unpacking the +// given content would cause an existing directory to be replaced with a non- +// directory and vice versa. +func (container *Container) ExtractToDir(path string, noOverwriteDirNonDir bool, content io.Reader) (err error) { + container.Lock() + defer container.Unlock() + + if err = container.Mount(); err != nil { + return err + } + defer container.Unmount() + + err = container.mountVolumes() + defer container.UnmountVolumes(true) + if err != nil { + return err + } + + // Consider the given path as an absolute path in the container. + absPath := path + if !filepath.IsAbs(absPath) { + absPath = archive.PreserveTrailingDotOrSeparator(filepath.Join("/", path), path) + } + + resolvedPath, err := container.GetResourcePath(absPath) + if err != nil { + return err + } + + // A trailing "." or separator has important meaning. For example, if + // `"foo"` is a symlink to some directory `"dir"`, then `os.Lstat("foo")` + // will stat the link itself, while `os.Lstat("foo/")` will stat the link + // target. If the basename of the path is ".", it means to archive the + // contents of the directory with "." as the first path component rather + // than the name of the directory. This would cause extraction of the + // archive to *not* make another directory, but instead use the current + // directory. + resolvedPath = archive.PreserveTrailingDotOrSeparator(resolvedPath, absPath) + + stat, err := os.Lstat(resolvedPath) + if err != nil { + return err + } + + if !stat.IsDir() { + return ErrExtractPointNotDirectory + } + + baseRel, err := filepath.Rel(container.basefs, resolvedPath) + if err != nil { + return err + } + absPath = filepath.Join("/", baseRel) + + // Need to check if the path is in a volume. If it is, it cannot be in a + // read-only volume. If it is not in a volume, the container cannot be + // configured with a read-only rootfs. + var toVolume bool + for _, mnt := range container.MountPoints { + if toVolume = mnt.hasResource(absPath); toVolume { + if mnt.RW { + break + } + return ErrVolumeReadonly + } + } + + if !toVolume && container.hostConfig.ReadonlyRootfs { + return ErrContainerRootfsReadonly + } + + options := &archive.TarOptions{ + ChownOpts: &archive.TarChownOptions{ + UID: 0, GID: 0, // TODO: use config.User? Remap to userns root? + }, + NoOverwriteDirNonDir: noOverwriteDirNonDir, + } + + if err := chrootarchive.Untar(content, resolvedPath, options); err != nil { + return err + } + + container.LogEvent("extract-to-dir") + + return nil +} diff --git a/daemon/container.go b/daemon/container.go index bc9d5ab043..6060d0149f 100644 --- a/daemon/container.go +++ b/daemon/container.go @@ -35,10 +35,11 @@ import ( ) var ( - ErrNotATTY = errors.New("The PTY is not a file") - ErrNoTTY = errors.New("No PTY found") - ErrContainerStart = errors.New("The container failed to start. Unknown error") - ErrContainerStartTimeout = errors.New("The container failed to start due to timed out.") + ErrNotATTY = errors.New("The PTY is not a file") + ErrNoTTY = errors.New("No PTY found") + ErrContainerStart = errors.New("The container failed to start. Unknown error") + ErrContainerStartTimeout = errors.New("The container failed to start due to timed out.") + ErrContainerRootfsReadonly = errors.New("container rootfs is marked read-only") ) type StreamConfig struct { @@ -616,13 +617,22 @@ func validateID(id string) error { return nil } -func (container *Container) Copy(resource string) (io.ReadCloser, error) { +func (container *Container) Copy(resource string) (rc io.ReadCloser, err error) { container.Lock() - defer container.Unlock() - var err error + + defer func() { + if err != nil { + // Wait to unlock the container until the archive is fully read + // (see the ReadCloseWrapper func below) or if there is an error + // before that occurs. + container.Unlock() + } + }() + if err := container.Mount(); err != nil { return nil, err } + defer func() { if err != nil { // unmount any volumes @@ -631,28 +641,11 @@ func (container *Container) Copy(resource string) (io.ReadCloser, error) { container.Unmount() } }() - mounts, err := container.setupMounts() - if err != nil { + + if err := container.mountVolumes(); err != nil { return nil, err } - for _, m := range mounts { - var dest string - dest, err = container.GetResourcePath(m.Destination) - if err != nil { - return nil, err - } - var stat os.FileInfo - stat, err = os.Stat(m.Source) - if err != nil { - return nil, err - } - if err = fileutils.CreateIfNotExists(dest, stat.IsDir()); err != nil { - return nil, err - } - if err = mount.Mount(m.Source, dest, "bind", "rbind,ro"); err != nil { - return nil, err - } - } + basePath, err := container.GetResourcePath(resource) if err != nil { return nil, err @@ -688,6 +681,7 @@ func (container *Container) Copy(resource string) (io.ReadCloser, error) { container.CleanupStorage() container.UnmountVolumes(true) container.Unmount() + container.Unlock() return err }) container.LogEvent("copy") @@ -1190,6 +1184,40 @@ func (container *Container) shouldRestart() bool { (container.hostConfig.RestartPolicy.Name == "on-failure" && container.ExitCode != 0) } +func (container *Container) mountVolumes() error { + mounts, err := container.setupMounts() + if err != nil { + return err + } + + for _, m := range mounts { + dest, err := container.GetResourcePath(m.Destination) + if err != nil { + return err + } + + var stat os.FileInfo + stat, err = os.Stat(m.Source) + if err != nil { + return err + } + if err = fileutils.CreateIfNotExists(dest, stat.IsDir()); err != nil { + return err + } + + opts := "rbind,ro" + if m.Writable { + opts = "rbind,rw" + } + + if err := mount.Mount(m.Source, dest, "bind", opts); err != nil { + return err + } + } + + return nil +} + func (container *Container) copyImagePathContent(v volume.Volume, destination string) error { rootfs, err := symlink.FollowSymlinkInScope(filepath.Join(container.basefs, destination), container.basefs) if err != nil { diff --git a/daemon/copy.go b/daemon/copy.go deleted file mode 100644 index dec30d8f37..0000000000 --- a/daemon/copy.go +++ /dev/null @@ -1,16 +0,0 @@ -package daemon - -import "io" - -func (daemon *Daemon) ContainerCopy(name string, res string) (io.ReadCloser, error) { - container, err := daemon.Get(name) - if err != nil { - return nil, err - } - - if res[0] == '/' { - res = res[1:] - } - - return container.Copy(res) -} diff --git a/daemon/volumes.go b/daemon/volumes.go index 71a3ed941f..556e304977 100644 --- a/daemon/volumes.go +++ b/daemon/volumes.go @@ -1,6 +1,7 @@ package daemon import ( + "errors" "fmt" "io/ioutil" "os" @@ -17,6 +18,10 @@ import ( "github.com/opencontainers/runc/libcontainer/label" ) +// ErrVolumeReadonly is used to signal an error when trying to copy data into +// a volume mount that is not writable. +var ErrVolumeReadonly = errors.New("mounted volume is marked read-only") + type mountPoint struct { Name string Destination string @@ -47,6 +52,16 @@ func (m *mountPoint) Setup() (string, error) { return "", fmt.Errorf("Unable to setup mount point, neither source nor volume defined") } +// hasResource checks whether the given absolute path for a container is in +// this mount point. If the relative path starts with `../` then the resource +// is outside of this mount point, but we can't simply check for this prefix +// because it misses `..` which is also outside of the mount, so check both. +func (m *mountPoint) hasResource(absolutePath string) bool { + relPath, err := filepath.Rel(m.Destination, absolutePath) + + return err == nil && relPath != ".." && !strings.HasPrefix(relPath, fmt.Sprintf("..%c", filepath.Separator)) +} + func (m *mountPoint) Path() string { if m.Volume != nil { return m.Volume.Path()