From 92600bdec1284f9031868751f61bef476d2e1dbd Mon Sep 17 00:00:00 2001 From: Zhang Wei Date: Thu, 1 Oct 2015 15:56:39 +0800 Subject: [PATCH] Add '-L' option for `cp` Fixes #16555 Original docker `cp` always copy symbol link itself instead of target, now we provide '-L' option to allow docker to follow symbol link to real target. Signed-off-by: Zhang Wei --- api/client/cp.go | 66 +++-- docs/reference/commandline/cp.md | 92 ++++--- integration-cli/docker_cli_cp_test.go | 54 ++++ man/docker-cp.1.md | 113 ++++---- pkg/archive/changes_test.go | 3 + pkg/archive/copy.go | 101 ++++++-- pkg/archive/copy_test.go | 357 +++++++++++++++++++++++++- 7 files changed, 644 insertions(+), 142 deletions(-) diff --git a/api/client/cp.go b/api/client/cp.go index c055fced51..bd97441ca5 100644 --- a/api/client/cp.go +++ b/api/client/cp.go @@ -26,22 +26,26 @@ const ( acrossContainers = fromContainer | toContainer ) +type cpConfig struct { + followLink bool +} + // CmdCp copies files/folders to or from a path in a container. // -// When copying from a container, if LOCALPATH is '-' the data is written as a +// When copying from a container, if DEST_PATH is '-' the data is written as a // tar archive file to STDOUT. // -// When copying to a container, if LOCALPATH is '-' the data is read as a tar -// archive file from STDIN, and the destination CONTAINER:PATH, must specify +// When copying to a container, if SRC_PATH is '-' the data is read as a tar +// archive file from STDIN, and the destination CONTAINER:DEST_PATH, must specify // a directory. // // Usage: -// docker cp CONTAINER:PATH LOCALPATH|- -// docker cp LOCALPATH|- CONTAINER:PATH +// docker cp CONTAINER:SRC_PATH DEST_PATH|- +// docker cp SRC_PATH|- CONTAINER:DEST_PATH func (cli *DockerCli) CmdCp(args ...string) error { cmd := Cli.Subcmd( "cp", - []string{"CONTAINER:PATH LOCALPATH|-", "LOCALPATH|- CONTAINER:PATH"}, + []string{"CONTAINER:SRC_PATH DEST_PATH|-", "SRC_PATH|- CONTAINER:DEST_PATH"}, strings.Join([]string{ Cli.DockerCommands["cp"].Description, "\nUse '-' as the source to read a tar archive from stdin\n", @@ -52,6 +56,8 @@ func (cli *DockerCli) CmdCp(args ...string) error { true, ) + followLink := cmd.Bool([]string{"L", "-follow-link"}, false, "Always follow symbol link in SRC_PATH") + cmd.Require(flag.Exact, 2) cmd.ParseFlags(args, true) @@ -73,11 +79,15 @@ func (cli *DockerCli) CmdCp(args ...string) error { direction |= toContainer } + cpParam := &cpConfig{ + followLink: *followLink, + } + switch direction { case fromContainer: - return cli.copyFromContainer(srcContainer, srcPath, dstPath) + return cli.copyFromContainer(srcContainer, srcPath, dstPath, cpParam) case toContainer: - return cli.copyToContainer(srcPath, dstContainer, dstPath) + return cli.copyToContainer(srcPath, dstContainer, dstPath, cpParam) case acrossContainers: // Copying between containers isn't supported. return fmt.Errorf("copying between containers is not supported") @@ -161,7 +171,7 @@ func resolveLocalPath(localPath string) (absPath string, err error) { return archive.PreserveTrailingDotOrSeparator(absPath, localPath), nil } -func (cli *DockerCli) copyFromContainer(srcContainer, srcPath, dstPath string) (err error) { +func (cli *DockerCli) copyFromContainer(srcContainer, srcPath, dstPath string, cpParam *cpConfig) (err error) { if dstPath != "-" { // Get an absolute destination path. dstPath, err = resolveLocalPath(dstPath) @@ -170,6 +180,26 @@ func (cli *DockerCli) copyFromContainer(srcContainer, srcPath, dstPath string) ( } } + // if client requests to follow symbol link, then must decide target file to be copied + var rebaseName string + if cpParam.followLink { + srcStat, err := cli.statContainerPath(srcContainer, srcPath) + + // If the destination is a symbolic link, we should follow it. + if err == nil && srcStat.Mode&os.ModeSymlink != 0 { + linkTarget := srcStat.LinkTarget + if !system.IsAbs(linkTarget) { + // Join with the parent directory. + srcParent, _ := archive.SplitPathDirEntry(srcPath) + linkTarget = filepath.Join(srcParent, linkTarget) + } + + linkTarget, rebaseName = archive.GetRebaseName(srcPath, linkTarget) + srcPath = linkTarget + } + + } + query := make(url.Values, 1) query.Set("path", filepath.ToSlash(srcPath)) // Normalize the paths used in the API. @@ -205,18 +235,24 @@ func (cli *DockerCli) copyFromContainer(srcContainer, srcPath, dstPath string) ( // Prepare source copy info. srcInfo := archive.CopyInfo{ - Path: srcPath, - Exists: true, - IsDir: stat.Mode.IsDir(), + Path: srcPath, + Exists: true, + IsDir: stat.Mode.IsDir(), + RebaseName: rebaseName, } + preArchive := response.body + if len(srcInfo.RebaseName) != 0 { + _, srcBase := archive.SplitPathDirEntry(srcInfo.Path) + preArchive = archive.RebaseArchiveEntries(response.body, srcBase, srcInfo.RebaseName) + } // See comments in the implementation of `archive.CopyTo` for exactly what // goes into deciding how and whether the source archive needs to be // altered for the correct copy behavior. - return archive.CopyTo(response.body, srcInfo, dstPath) + return archive.CopyTo(preArchive, srcInfo, dstPath) } -func (cli *DockerCli) copyToContainer(srcPath, dstContainer, dstPath string) (err error) { +func (cli *DockerCli) copyToContainer(srcPath, dstContainer, dstPath string, cpParam *cpConfig) (err error) { if srcPath != "-" { // Get an absolute source path. srcPath, err = resolveLocalPath(srcPath) @@ -271,7 +307,7 @@ func (cli *DockerCli) copyToContainer(srcPath, dstContainer, dstPath string) (er } } else { // Prepare source copy info. - srcInfo, err := archive.CopyInfoSourcePath(srcPath) + srcInfo, err := archive.CopyInfoSourcePath(srcPath, cpParam.followLink) if err != nil { return err } diff --git a/docs/reference/commandline/cp.md b/docs/reference/commandline/cp.md index da27c20376..8a1a6264d7 100644 --- a/docs/reference/commandline/cp.md +++ b/docs/reference/commandline/cp.md @@ -10,81 +10,79 @@ parent = "smn_cli" # cp - Usage: docker cp [OPTIONS] CONTAINER:PATH LOCALPATH|- - docker cp [OPTIONS] LOCALPATH|- CONTAINER:PATH + Usage: docker cp [OPTIONS] CONTAINER:SRC_PATH DEST_PATH | - + docker cp [OPTIONS] SRC_PATH | - CONTAINER:DEST_PATH Copy files/folders between a container and the local filesystem - --help=false Print usage + -L, --follow-link=false Always follow symbol link in SRC_PATH + --help=false Print usage -In the first synopsis form, the `docker cp` utility copies the contents of -`PATH` from the filesystem of `CONTAINER` to the `LOCALPATH` (or stream as -a tar archive to `STDOUT` if `-` is specified). +The `docker cp` utility copies the contents of `SRC_PATH` to the `DEST_PATH`. +You can copy from the container's file system to the local machine or the +reverse, from the local filesystem to the container. If `-` is specified for +either the `SRC_PATH` or `DEST_PATH`, you can also stream a tar archive from +`STDIN` or to `STDOUT`. The `CONTAINER` can be a running or stopped container. +The `SRC_PATH` or `DEST_PATH` be a file or directory. -In the second synopsis form, the contents of `LOCALPATH` (or a tar archive -streamed from `STDIN` if `-` is specified) are copied from the local machine to -`PATH` in the filesystem of `CONTAINER`. +The `docker cp` command assumes container paths are relative to the container's +`/` (root) directory. This means supplying the initial forward slash is optional; +The command sees `compassionate_darwin:/tmp/foo/myfile.txt` and +`compassionate_darwin:tmp/foo/myfile.txt` as identical. Local machine paths can +be an absolute or relative value. The command interprets a local machine's +relative paths as relative to the current working directory where `docker cp` is +run. -You can copy to or from either a running or stopped container. The `PATH` can -be a file or directory. The `docker cp` command assumes all `CONTAINER:PATH` -values are relative to the `/` (root) directory of the container. This means -supplying the initial forward slash is optional; The command sees -`compassionate_darwin:/tmp/foo/myfile.txt` and -`compassionate_darwin:tmp/foo/myfile.txt` as identical. If a `LOCALPATH` value -is not absolute, is it considered relative to the current working directory. - -Behavior is similar to the common Unix utility `cp -a` in that directories are +The `cp` command behaves like the Unix `cp -a` command in that directories are copied recursively with permissions preserved if possible. Ownership is set to -the user and primary group on the receiving end of the transfer. For example, -files copied to a container will be created with `UID:GID` of the root user. -Files copied to the local machine will be created with the `UID:GID` of the -user which invoked the `docker cp` command. +the user and primary group at the destination. For example, files copied to a +container are created with `UID:GID` of the root user. Files copied to the local +machine are created with the `UID:GID` of the user which invoked the `docker cp` +command. If you specify the `-L` option, `docker cp` follows any symbolic link +in the `SRC_PATH`. Assuming a path separator of `/`, a first argument of `SRC_PATH` and second -argument of `DST_PATH`, the behavior is as follows: +argument of `DEST_PATH`, the behavior is as follows: - `SRC_PATH` specifies a file - - `DST_PATH` does not exist - - the file is saved to a file created at `DST_PATH` - - `DST_PATH` does not exist and ends with `/` + - `DEST_PATH` does not exist + - the file is saved to a file created at `DEST_PATH` + - `DEST_PATH` does not exist and ends with `/` - Error condition: the destination directory must exist. - - `DST_PATH` exists and is a file - - the destination is overwritten with the contents of the source file - - `DST_PATH` exists and is a directory + - `DEST_PATH` exists and is a file + - the destination is overwritten with the source file's contents + - `DEST_PATH` exists and is a directory - the file is copied into this directory using the basename from `SRC_PATH` - `SRC_PATH` specifies a directory - - `DST_PATH` does not exist - - `DST_PATH` is created as a directory and the *contents* of the source + - `DEST_PATH` does not exist + - `DEST_PATH` is created as a directory and the *contents* of the source directory are copied into this directory - - `DST_PATH` exists and is a file + - `DEST_PATH` exists and is a file - Error condition: cannot copy a directory to a file - - `DST_PATH` exists and is a directory + - `DEST_PATH` exists and is a directory - `SRC_PATH` does not end with `/.` - the source directory is copied into this directory - `SRC_PATH` does end with `/.` - the *content* of the source directory is copied into this directory -The command requires `SRC_PATH` and `DST_PATH` to exist according to the above +The command requires `SRC_PATH` and `DEST_PATH` to exist according to the above rules. If `SRC_PATH` is local and is a symbolic link, the symbolic link, not -the target, is copied. +the target, is copied by default. To copy the link target and not the link, specify +the `-L` option. -A colon (`:`) is used as a delimiter between `CONTAINER` and `PATH`, but `:` -could also be in a valid `LOCALPATH`, like `file:name.txt`. This ambiguity is -resolved by requiring a `LOCALPATH` with a `:` to be made explicit with a -relative or absolute path, for example: +A colon (`:`) is used as a delimiter between `CONTAINER` and its path. You can +also use `:` when specifying paths to a `SRC_PATH` or `DEST_PATH` on a local +machine, for example `file:name.txt`. If you use a `:` in a local machine path, +you must be explicit with a relative or absolute path, for example: `/path/to/file:name.txt` or `./file:name.txt` It is not possible to copy certain system files such as resources under `/proc`, `/sys`, `/dev`, and mounts created by the user in the container. -Using `-` as the first argument in place of a `LOCALPATH` will stream the -contents of `STDIN` as a tar archive which will be extracted to the `PATH` in -the filesystem of the destination container. In this case, `PATH` must specify -a directory. - -Using `-` as the second argument in place of a `LOCALPATH` will stream the -contents of the resource from the source container as a tar archive to -`STDOUT`. +Using `-` as the `SRC_PATH` streams the contents of `STDIN` as a tar archive. +The command extracts the content of the tar to the `DEST_PATH` in container's +filesystem. In this case, `DEST_PATH` must specify a directory. Using `-` as +`DEST_PATH` streams the contents of the resource as a tar archive to `STDOUT`. diff --git a/integration-cli/docker_cli_cp_test.go b/integration-cli/docker_cli_cp_test.go index 384473551a..fd48cd6c96 100644 --- a/integration-cli/docker_cli_cp_test.go +++ b/integration-cli/docker_cli_cp_test.go @@ -609,3 +609,57 @@ func (s *DockerSuite) TestCopyCreatedContainer(c *check.C) { defer os.RemoveAll(tmpDir) dockerCmd(c, "cp", "test_cp:/bin/sh", tmpDir) } + +// test copy with option `-L`: following symbol link +// Check that symlinks to a file behave as expected when copying one from +// a container to host following symbol link +func (s *DockerSuite) TestCpSymlinkFromConToHostFollowSymlink(c *check.C) { + testRequires(c, DaemonIsLinux) + out, exitCode := dockerCmd(c, "run", "-d", "busybox", "/bin/sh", "-c", "mkdir -p '"+cpTestPath+"' && echo -n '"+cpContainerContents+"' > "+cpFullPath+" && ln -s "+cpFullPath+" /dir_link") + if exitCode != 0 { + c.Fatal("failed to create a container", out) + } + + cleanedContainerID := strings.TrimSpace(out) + + out, _ = dockerCmd(c, "wait", cleanedContainerID) + if strings.TrimSpace(out) != "0" { + c.Fatal("failed to set up container", out) + } + + testDir, err := ioutil.TempDir("", "test-cp-symlink-container-to-host-follow-symlink") + if err != nil { + c.Fatal(err) + } + defer os.RemoveAll(testDir) + + // This copy command should copy the symlink, not the target, into the + // temporary directory. + dockerCmd(c, "cp", "-L", cleanedContainerID+":"+"/dir_link", testDir) + + expectedPath := filepath.Join(testDir, "dir_link") + + expected := []byte(cpContainerContents) + actual, err := ioutil.ReadFile(expectedPath) + + if !bytes.Equal(actual, expected) { + c.Fatalf("Expected copied file to be duplicate of the container symbol link target") + } + os.Remove(expectedPath) + + // now test copy symbol link to an non-existing file in host + expectedPath = filepath.Join(testDir, "somefile_host") + // expectedPath shouldn't exist, if exists, remove it + if _, err := os.Lstat(expectedPath); err == nil { + os.Remove(expectedPath) + } + + dockerCmd(c, "cp", "-L", cleanedContainerID+":"+"/dir_link", expectedPath) + + actual, err = ioutil.ReadFile(expectedPath) + + if !bytes.Equal(actual, expected) { + c.Fatalf("Expected copied file to be duplicate of the container symbol link target") + } + defer os.Remove(expectedPath) +} diff --git a/man/docker-cp.1.md b/man/docker-cp.1.md index 667fc543b1..bd15625e1b 100644 --- a/man/docker-cp.1.md +++ b/man/docker-cp.1.md @@ -7,87 +7,87 @@ docker-cp - Copy files/folders between a container and the local filesystem. # SYNOPSIS **docker cp** [**--help**] -CONTAINER:PATH LOCALPATH|- +CONTAINER:SRC_PATH DEST_PATH|- **docker cp** [**--help**] -LOCALPATH|- CONTAINER:PATH +SRC_PATH|- CONTAINER:DEST_PATH # DESCRIPTION -In the first synopsis form, the `docker cp` utility copies the contents of -`PATH` from the filesystem of `CONTAINER` to the `LOCALPATH` (or stream as -a tar archive to `STDOUT` if `-` is specified). +The `docker cp` utility copies the contents of `SRC_PATH` to the `DEST_PATH`. +You can copy from the container's file system to the local machine or the +reverse, from the local filesystem to the container. If `-` is specified for +either the `SRC_PATH` or `DEST_PATH`, you can also stream a tar archive from +`STDIN` or to `STDOUT`. The `CONTAINER` can be a running or stopped container. +The `SRC_PATH` or `DEST_PATH` be a file or directory. -In the second synopsis form, the contents of `LOCALPATH` (or a tar archive -streamed from `STDIN` if `-` is specified) are copied from the local machine to -`PATH` in the filesystem of `CONTAINER`. +The `docker cp` command assumes container paths are relative to the container's +`/` (root) directory. This means supplying the initial forward slash is optional; +The command sees `compassionate_darwin:/tmp/foo/myfile.txt` and +`compassionate_darwin:tmp/foo/myfile.txt` as identical. Local machine paths can +be an absolute or relative value. The command interprets a local machine's +relative paths as relative to the current working directory where `docker cp` is +run. -You can copy to or from either a running or stopped container. The `PATH` can -be a file or directory. The `docker cp` command assumes all `CONTAINER:PATH` -values are relative to the `/` (root) directory of the container. This means -supplying the initial forward slash is optional; The command sees -`compassionate_darwin:/tmp/foo/myfile.txt` and -`compassionate_darwin:tmp/foo/myfile.txt` as identical. If a `LOCALPATH` value -is not absolute, is it considered relative to the current working directory. - -Behavior is similar to the common Unix utility `cp -a` in that directories are +The `cp` command behaves like the Unix `cp -a` command in that directories are copied recursively with permissions preserved if possible. Ownership is set to -the user and primary group on the receiving end of the transfer. For example, -files copied to a container will be created with `UID:GID` of the root user. -Files copied to the local machine will be created with the `UID:GID` of the -user which invoked the `docker cp` command. +the user and primary group at the destination. For example, files copied to a +container are created with `UID:GID` of the root user. Files copied to the local +machine are created with the `UID:GID` of the user which invoked the `docker cp` +command. If you specify the `-L` option, `docker cp` follows any symbolic link +in the `SRC_PATH`. Assuming a path separator of `/`, a first argument of `SRC_PATH` and second -argument of `DST_PATH`, the behavior is as follows: +argument of `DEST_PATH`, the behavior is as follows: - `SRC_PATH` specifies a file - - `DST_PATH` does not exist - - the file is saved to a file created at `DST_PATH` - - `DST_PATH` does not exist and ends with `/` + - `DEST_PATH` does not exist + - the file is saved to a file created at `DEST_PATH` + - `DEST_PATH` does not exist and ends with `/` - Error condition: the destination directory must exist. - - `DST_PATH` exists and is a file - - the destination is overwritten with the contents of the source file - - `DST_PATH` exists and is a directory + - `DEST_PATH` exists and is a file + - the destination is overwritten with the source file's contents + - `DEST_PATH` exists and is a directory - the file is copied into this directory using the basename from `SRC_PATH` - `SRC_PATH` specifies a directory - - `DST_PATH` does not exist - - `DST_PATH` is created as a directory and the *contents* of the source + - `DEST_PATH` does not exist + - `DEST_PATH` is created as a directory and the *contents* of the source directory are copied into this directory - - `DST_PATH` exists and is a file + - `DEST_PATH` exists and is a file - Error condition: cannot copy a directory to a file - - `DST_PATH` exists and is a directory + - `DEST_PATH` exists and is a directory - `SRC_PATH` does not end with `/.` - the source directory is copied into this directory - `SRC_PATH` does end with `/.` - the *content* of the source directory is copied into this directory -The command requires `SRC_PATH` and `DST_PATH` to exist according to the above +The command requires `SRC_PATH` and `DEST_PATH` to exist according to the above rules. If `SRC_PATH` is local and is a symbolic link, the symbolic link, not -the target, is copied. +the target, is copied by default. To copy the link target and not the link, +specify the `-L` option. -A colon (`:`) is used as a delimiter between `CONTAINER` and `PATH`, but `:` -could also be in a valid `LOCALPATH`, like `file:name.txt`. This ambiguity is -resolved by requiring a `LOCALPATH` with a `:` to be made explicit with a -relative or absolute path, for example: +A colon (`:`) is used as a delimiter between `CONTAINER` and its path. You can +also use `:` when specifying paths to a `SRC_PATH` or `DEST_PATH` on a local +machine, for example `file:name.txt`. If you use a `:` in a local machine path, +you must be explicit with a relative or absolute path, for example: `/path/to/file:name.txt` or `./file:name.txt` It is not possible to copy certain system files such as resources under `/proc`, `/sys`, `/dev`, and mounts created by the user in the container. -Using `-` as the first argument in place of a `LOCALPATH` will stream the -contents of `STDIN` as a tar archive which will be extracted to the `PATH` in -the filesystem of the destination container. In this case, `PATH` must specify -a directory. - -Using `-` as the second argument in place of a `LOCALPATH` will stream the -contents of the resource from the source container as a tar archive to -`STDOUT`. +Using `-` as the `SRC_PATH` streams the contents of `STDIN` as a tar archive. +The command extracts the content of the tar to the `DEST_PATH` in container's +filesystem. In this case, `DEST_PATH` must specify a directory. Using `-` as +`DEST_PATH` streams the contents of the resource as a tar archive to `STDOUT`. # OPTIONS +**-L**, **--follow-link**=*true*|*false* + Follow symbol link in SRC_PATH + **--help** Print usage statement @@ -102,13 +102,13 @@ If you want to copy the `/tmp/foo` directory from a container to the existing `/tmp` directory on your host. If you run `docker cp` in your `~` (home) directory on the local host: - $ docker cp compassionate_darwin:tmp/foo /tmp + $ docker cp compassionate_darwin:tmp/foo /tmp Docker creates a `/tmp/foo` directory on your host. Alternatively, you can omit the leading slash in the command. If you execute this command from your home directory: - $ docker cp compassionate_darwin:tmp/foo tmp + $ docker cp compassionate_darwin:tmp/foo tmp If `~/tmp` does not exist, Docker will create it and copy the contents of `/tmp/foo` from the container into this new directory. If `~/tmp` already @@ -120,7 +120,7 @@ will either overwrite the contents of `LOCALPATH` if it is a file or place it into `LOCALPATH` if it is a directory, overwriting an existing file of the same name if one exists. For example, this command: - $ docker cp sharp_ptolemy:/tmp/foo/myfile.txt /test + $ docker cp sharp_ptolemy:/tmp/foo/myfile.txt /test If `/test` does not exist on the local machine, it will be created as a file with the contents of `/tmp/foo/myfile.txt` from the container. If `/test` @@ -137,16 +137,27 @@ If you have a file, `config.yml`, in the current directory on your local host and wish to copy it to an existing directory at `/etc/my-app.d` in a container, this command can be used: - $ docker cp config.yml myappcontainer:/etc/my-app.d + $ docker cp config.yml myappcontainer:/etc/my-app.d If you have several files in a local directory `/config` which you need to copy to a directory `/etc/my-app.d` in a container: - $ docker cp /config/. myappcontainer:/etc/my-app.d + $ docker cp /config/. myappcontainer:/etc/my-app.d The above command will copy the contents of the local `/config` directory into the directory `/etc/my-app.d` in the container. +Finally, if you want to copy a symbolic link into a container, you typically +want to copy the linked target and not the link itself. To copy the target, use +the `-L` option, for example: + + $ ln -s /tmp/somefile /tmp/somefile.ln + $ docker cp -L /tmp/somefile.ln myappcontainer:/tmp/ + +This command copies content of the local `/tmp/somefile` into the file +`/tmp/somefile.ln` in the container. Without `-L` option, the `/tmp/somefile.ln` +preserves its symbolic link but not its content. + # HISTORY April 2014, Originally compiled by William Henry (whenry at redhat dot com) based on docker.com source material and internal work. diff --git a/pkg/archive/changes_test.go b/pkg/archive/changes_test.go index 52daaa6433..f4316ce21e 100644 --- a/pkg/archive/changes_test.go +++ b/pkg/archive/changes_test.go @@ -63,6 +63,9 @@ func createSampleDir(t *testing.T, root string) { {Regular, "dir4/file3-2", "file4-2\n", 0666}, {Symlink, "symlink1", "target1", 0666}, {Symlink, "symlink2", "target2", 0666}, + {Symlink, "symlink3", root + "/file1", 0666}, + {Symlink, "symlink4", root + "/symlink3", 0666}, + {Symlink, "dirSymlink", root + "/dir1", 0740}, } now := time.Now() diff --git a/pkg/archive/copy.go b/pkg/archive/copy.go index ecfd0a9c69..e1fa73f374 100644 --- a/pkg/archive/copy.go +++ b/pkg/archive/copy.go @@ -135,30 +135,17 @@ type CopyInfo struct { // operation. The given path should be an absolute local path. A source path // has all symlinks evaluated that appear before the last path separator ("/" // on Unix). As it is to be a copy source, the path must exist. -func CopyInfoSourcePath(path string) (CopyInfo, error) { - // Split the given path into its Directory and Base components. We will - // evaluate symlinks in the directory component then append the base. +func CopyInfoSourcePath(path string, followLink bool) (CopyInfo, error) { + // normalize the file path and then evaluate the symbol link + // we will use the target file instead of the symbol link if + // followLink is set path = normalizePath(path) - dirPath, basePath := filepath.Split(path) - resolvedDirPath, err := filepath.EvalSymlinks(dirPath) + resolvedPath, rebaseName, err := ResolveHostSourcePath(path, followLink) if err != nil { return CopyInfo{}, err } - // resolvedDirPath will have been cleaned (no trailing path separators) so - // we can manually join it with the base path element. - resolvedPath := resolvedDirPath + string(filepath.Separator) + basePath - - var rebaseName string - if hasTrailingPathSeparator(path) && filepath.Base(path) != filepath.Base(resolvedPath) { - // In the case where the path had a trailing separator and a symlink - // evaluation has changed the last path component, we will need to - // rebase the name in the archive that is being copied to match the - // originally requested name. - rebaseName = filepath.Base(path) - } - stat, err := os.Lstat(resolvedPath) if err != nil { return CopyInfo{}, err @@ -279,7 +266,10 @@ func PrepareArchiveCopy(srcContent Reader, srcInfo, dstInfo CopyInfo) (dstDir st // The destination exists as some type of file and the source content // is also a file. The source content entry will have to be renamed to // have a basename which matches the destination path's basename. - return dstDir, rebaseArchiveEntries(srcContent, srcBase, dstBase), nil + if len(srcInfo.RebaseName) != 0 { + srcBase = srcInfo.RebaseName + } + return dstDir, RebaseArchiveEntries(srcContent, srcBase, dstBase), nil case srcInfo.IsDir: // The destination does not exist and the source content is an archive // of a directory. The archive should be extracted to the parent of @@ -287,7 +277,10 @@ func PrepareArchiveCopy(srcContent Reader, srcInfo, dstInfo CopyInfo) (dstDir st // created as a result should take the name of the destination path. // The source content entries will have to be renamed to have a // basename which matches the destination path's basename. - return dstDir, rebaseArchiveEntries(srcContent, srcBase, dstBase), nil + if len(srcInfo.RebaseName) != 0 { + srcBase = srcInfo.RebaseName + } + return dstDir, RebaseArchiveEntries(srcContent, srcBase, dstBase), nil case assertsDirectory(dstInfo.Path): // The destination does not exist and is asserted to be created as a // directory, but the source content is not a directory. This is an @@ -301,14 +294,17 @@ func PrepareArchiveCopy(srcContent Reader, srcInfo, dstInfo CopyInfo) (dstDir st // to be created when the archive is extracted and the source content // entry will have to be renamed to have a basename which matches the // destination path's basename. - return dstDir, rebaseArchiveEntries(srcContent, srcBase, dstBase), nil + if len(srcInfo.RebaseName) != 0 { + srcBase = srcInfo.RebaseName + } + return dstDir, RebaseArchiveEntries(srcContent, srcBase, dstBase), nil } } -// rebaseArchiveEntries rewrites the given srcContent archive replacing +// RebaseArchiveEntries rewrites the given srcContent archive replacing // an occurrence of oldBase with newBase at the beginning of entry names. -func rebaseArchiveEntries(srcContent Reader, oldBase, newBase string) Archive { +func RebaseArchiveEntries(srcContent Reader, oldBase, newBase string) Archive { if oldBase == string(os.PathSeparator) { // If oldBase specifies the root directory, use an empty string as // oldBase instead so that newBase doesn't replace the path separator @@ -355,7 +351,7 @@ func rebaseArchiveEntries(srcContent Reader, oldBase, newBase string) Archive { // CopyResource performs an archive copy from the given source path to the // given destination path. The source path MUST exist and the destination // path's parent directory must exist. -func CopyResource(srcPath, dstPath string) error { +func CopyResource(srcPath, dstPath string, followLink bool) error { var ( srcInfo CopyInfo err error @@ -369,7 +365,7 @@ func CopyResource(srcPath, dstPath string) error { srcPath = PreserveTrailingDotOrSeparator(filepath.Clean(srcPath), srcPath) dstPath = PreserveTrailingDotOrSeparator(filepath.Clean(dstPath), dstPath) - if srcInfo, err = CopyInfoSourcePath(srcPath); err != nil { + if srcInfo, err = CopyInfoSourcePath(srcPath, followLink); err != nil { return err } @@ -405,3 +401,58 @@ func CopyTo(content Reader, srcInfo CopyInfo, dstPath string) error { return Untar(copyArchive, dstDir, options) } + +// ResolveHostSourcePath decides real path need to be copied with parameters such as +// whether to follow symbol link or not, if followLink is true, resolvedPath will return +// link target of any symbol link file, else it will only resolve symlink of directory +// but return symbol link file itself without resolving. +func ResolveHostSourcePath(path string, followLink bool) (resolvedPath, rebaseName string, err error) { + if followLink { + resolvedPath, err = filepath.EvalSymlinks(path) + if err != nil { + return + } + + resolvedPath, rebaseName = GetRebaseName(path, resolvedPath) + } else { + dirPath, basePath := filepath.Split(path) + + // if not follow symbol link, then resolve symbol link of parent dir + var resolvedDirPath string + resolvedDirPath, err = filepath.EvalSymlinks(dirPath) + if err != nil { + return + } + // resolvedDirPath will have been cleaned (no trailing path separators) so + // we can manually join it with the base path element. + resolvedPath = resolvedDirPath + string(filepath.Separator) + basePath + if hasTrailingPathSeparator(path) && filepath.Base(path) != filepath.Base(resolvedPath) { + rebaseName = filepath.Base(path) + } + } + return resolvedPath, rebaseName, nil +} + +// GetRebaseName normalizes and compares path and resolvedPath, +// return completed resolved path and rebased file name +func GetRebaseName(path, resolvedPath string) (string, string) { + // linkTarget will have been cleaned (no trailing path separators and dot) so + // we can manually join it with them + var rebaseName string + if specifiesCurrentDir(path) && !specifiesCurrentDir(resolvedPath) { + resolvedPath += string(filepath.Separator) + "." + } + + if hasTrailingPathSeparator(path) && !hasTrailingPathSeparator(resolvedPath) { + resolvedPath += string(filepath.Separator) + } + + if filepath.Base(path) != filepath.Base(resolvedPath) { + // In the case where the path had a trailing separator and a symlink + // evaluation has changed the last path component, we will need to + // rebase the name in the archive that is being copied to match the + // originally requested name. + rebaseName = filepath.Base(path) + } + return resolvedPath, rebaseName +} diff --git a/pkg/archive/copy_test.go b/pkg/archive/copy_test.go index 25f1811a96..f1dc23824d 100644 --- a/pkg/archive/copy_test.go +++ b/pkg/archive/copy_test.go @@ -120,9 +120,15 @@ func logDirContents(t *testing.T, dirPath string) { } func testCopyHelper(t *testing.T, srcPath, dstPath string) (err error) { - t.Logf("copying from %q to %q", srcPath, dstPath) + t.Logf("copying from %q to %q (not follow symbol link)", srcPath, dstPath) - return CopyResource(srcPath, dstPath) + return CopyResource(srcPath, dstPath, false) +} + +func testCopyHelperFSym(t *testing.T, srcPath, dstPath string) (err error) { + t.Logf("copying from %q to %q (follow symbol link)", srcPath, dstPath) + + return CopyResource(srcPath, dstPath, true) } // Basic assumptions about SRC and DST: @@ -138,7 +144,7 @@ func TestCopyErrSrcNotExists(t *testing.T) { tmpDirA, tmpDirB := getTestTempDirs(t) defer removeAllPaths(tmpDirA, tmpDirB) - if _, err := CopyInfoSourcePath(filepath.Join(tmpDirA, "file1")); !os.IsNotExist(err) { + if _, err := CopyInfoSourcePath(filepath.Join(tmpDirA, "file1"), false); !os.IsNotExist(err) { t.Fatalf("expected IsNotExist error, but got %T: %s", err, err) } } @@ -152,7 +158,7 @@ func TestCopyErrSrcNotDir(t *testing.T) { // Load A with some sample files and directories. createSampleDir(t, tmpDirA) - if _, err := CopyInfoSourcePath(joinTrailingSep(tmpDirA, "file1")); !isNotDir(err) { + if _, err := CopyInfoSourcePath(joinTrailingSep(tmpDirA, "file1"), false); !isNotDir(err) { t.Fatalf("expected IsNotDir error, but got %T: %s", err, err) } } @@ -286,6 +292,27 @@ func TestCopyCaseA(t *testing.T) { if err = fileContentsEqual(t, srcPath, dstPath); err != nil { t.Fatal(err) } + os.Remove(dstPath) + + symlinkPath := filepath.Join(tmpDirA, "symlink3") + symlinkPath1 := filepath.Join(tmpDirA, "symlink4") + linkTarget := filepath.Join(tmpDirA, "file1") + + if err = testCopyHelperFSym(t, symlinkPath, dstPath); err != nil { + t.Fatalf("unexpected error %T: %s", err, err) + } + + if err = fileContentsEqual(t, linkTarget, dstPath); err != nil { + t.Fatal(err) + } + os.Remove(dstPath) + if err = testCopyHelperFSym(t, symlinkPath1, dstPath); err != nil { + t.Fatalf("unexpected error %T: %s", err, err) + } + + if err = fileContentsEqual(t, linkTarget, dstPath); err != nil { + t.Fatal(err) + } } // B. SRC specifies a file and DST (with trailing path separator) doesn't @@ -310,6 +337,16 @@ func TestCopyCaseB(t *testing.T) { if err != ErrDirNotExists { t.Fatalf("expected ErrDirNotExists error, but got %T: %s", err, err) } + + symlinkPath := filepath.Join(tmpDirA, "symlink3") + + if err = testCopyHelperFSym(t, symlinkPath, dstDir); err == nil { + t.Fatal("expected ErrDirNotExists error, but got nil instead") + } + if err != ErrDirNotExists { + t.Fatalf("expected ErrDirNotExists error, but got %T: %s", err, err) + } + } // C. SRC specifies a file and DST exists as a file. This should overwrite @@ -341,6 +378,44 @@ func TestCopyCaseC(t *testing.T) { } } +// C. Symbol link following version: +// SRC specifies a file and DST exists as a file. This should overwrite +// the file at DST with the contents of the source file. +func TestCopyCaseCFSym(t *testing.T) { + tmpDirA, tmpDirB := getTestTempDirs(t) + defer removeAllPaths(tmpDirA, tmpDirB) + + // Load A and B with some sample files and directories. + createSampleDir(t, tmpDirA) + createSampleDir(t, tmpDirB) + + symlinkPathBad := filepath.Join(tmpDirA, "symlink1") + symlinkPath := filepath.Join(tmpDirA, "symlink3") + linkTarget := filepath.Join(tmpDirA, "file1") + dstPath := filepath.Join(tmpDirB, "file2") + + var err error + + // first to test broken link + if err = testCopyHelperFSym(t, symlinkPathBad, dstPath); err == nil { + t.Fatalf("unexpected error %T: %s", err, err) + } + + // test symbol link -> symbol link -> target + // Ensure they start out different. + if err = fileContentsEqual(t, linkTarget, dstPath); err == nil { + t.Fatal("expected different file contents") + } + + if err = testCopyHelperFSym(t, symlinkPath, dstPath); err != nil { + t.Fatalf("unexpected error %T: %s", err, err) + } + + if err = fileContentsEqual(t, linkTarget, dstPath); err != nil { + t.Fatal(err) + } +} + // D. SRC specifies a file and DST exists as a directory. This should place // a copy of the source file inside it using the basename from SRC. Ensure // this works whether DST has a trailing path separator or not. @@ -392,6 +467,59 @@ func TestCopyCaseD(t *testing.T) { } } +// D. Symbol link following version: +// SRC specifies a file and DST exists as a directory. This should place +// a copy of the source file inside it using the basename from SRC. Ensure +// this works whether DST has a trailing path separator or not. +func TestCopyCaseDFSym(t *testing.T) { + tmpDirA, tmpDirB := getTestTempDirs(t) + defer removeAllPaths(tmpDirA, tmpDirB) + + // Load A and B with some sample files and directories. + createSampleDir(t, tmpDirA) + createSampleDir(t, tmpDirB) + + srcPath := filepath.Join(tmpDirA, "symlink4") + linkTarget := filepath.Join(tmpDirA, "file1") + dstDir := filepath.Join(tmpDirB, "dir1") + dstPath := filepath.Join(dstDir, "symlink4") + + var err error + + // Ensure that dstPath doesn't exist. + if _, err = os.Stat(dstPath); !os.IsNotExist(err) { + t.Fatalf("did not expect dstPath %q to exist", dstPath) + } + + if err = testCopyHelperFSym(t, srcPath, dstDir); err != nil { + t.Fatalf("unexpected error %T: %s", err, err) + } + + if err = fileContentsEqual(t, linkTarget, dstPath); err != nil { + t.Fatal(err) + } + + // Now try again but using a trailing path separator for dstDir. + + if err = os.RemoveAll(dstDir); err != nil { + t.Fatalf("unable to remove dstDir: %s", err) + } + + if err = os.MkdirAll(dstDir, os.FileMode(0755)); err != nil { + t.Fatalf("unable to make dstDir: %s", err) + } + + dstDir = joinTrailingSep(tmpDirB, "dir1") + + if err = testCopyHelperFSym(t, srcPath, dstDir); err != nil { + t.Fatalf("unexpected error %T: %s", err, err) + } + + if err = fileContentsEqual(t, linkTarget, dstPath); err != nil { + t.Fatal(err) + } +} + // E. SRC specifies a directory and DST does not exist. This should create a // directory at DST and copy the contents of the SRC directory into the DST // directory. Ensure this works whether DST has a trailing path separator or @@ -436,6 +564,52 @@ func TestCopyCaseE(t *testing.T) { } } +// E. Symbol link following version: +// SRC specifies a directory and DST does not exist. This should create a +// directory at DST and copy the contents of the SRC directory into the DST +// directory. Ensure this works whether DST has a trailing path separator or +// not. +func TestCopyCaseEFSym(t *testing.T) { + tmpDirA, tmpDirB := getTestTempDirs(t) + defer removeAllPaths(tmpDirA, tmpDirB) + + // Load A with some sample files and directories. + createSampleDir(t, tmpDirA) + + srcDir := filepath.Join(tmpDirA, "dirSymlink") + linkTarget := filepath.Join(tmpDirA, "dir1") + dstDir := filepath.Join(tmpDirB, "testDir") + + var err error + + if err = testCopyHelperFSym(t, srcDir, dstDir); err != nil { + t.Fatalf("unexpected error %T: %s", err, err) + } + + if err = dirContentsEqual(t, dstDir, linkTarget); err != nil { + t.Log("dir contents not equal") + logDirContents(t, tmpDirA) + logDirContents(t, tmpDirB) + t.Fatal(err) + } + + // Now try again but using a trailing path separator for dstDir. + + if err = os.RemoveAll(dstDir); err != nil { + t.Fatalf("unable to remove dstDir: %s", err) + } + + dstDir = joinTrailingSep(tmpDirB, "testDir") + + if err = testCopyHelperFSym(t, srcDir, dstDir); err != nil { + t.Fatalf("unexpected error %T: %s", err, err) + } + + if err = dirContentsEqual(t, dstDir, linkTarget); err != nil { + t.Fatal(err) + } +} + // F. SRC specifies a directory and DST exists as a file. This should cause an // error as it is not possible to overwrite a file with a directory. func TestCopyCaseF(t *testing.T) { @@ -447,6 +621,7 @@ func TestCopyCaseF(t *testing.T) { createSampleDir(t, tmpDirB) srcDir := filepath.Join(tmpDirA, "dir1") + symSrcDir := filepath.Join(tmpDirA, "dirSymlink") dstFile := filepath.Join(tmpDirB, "file1") var err error @@ -458,6 +633,15 @@ func TestCopyCaseF(t *testing.T) { if err != ErrCannotCopyDir { t.Fatalf("expected ErrCannotCopyDir error, but got %T: %s", err, err) } + + // now test with symbol link + if err = testCopyHelperFSym(t, symSrcDir, dstFile); err == nil { + t.Fatal("expected ErrCannotCopyDir error, but got nil instead") + } + + if err != ErrCannotCopyDir { + t.Fatalf("expected ErrCannotCopyDir error, but got %T: %s", err, err) + } } // G. SRC specifies a directory and DST exists as a directory. This should copy @@ -506,6 +690,54 @@ func TestCopyCaseG(t *testing.T) { } } +// G. Symbol link version: +// SRC specifies a directory and DST exists as a directory. This should copy +// the SRC directory and all its contents to the DST directory. Ensure this +// works whether DST has a trailing path separator or not. +func TestCopyCaseGFSym(t *testing.T) { + tmpDirA, tmpDirB := getTestTempDirs(t) + defer removeAllPaths(tmpDirA, tmpDirB) + + // Load A and B with some sample files and directories. + createSampleDir(t, tmpDirA) + createSampleDir(t, tmpDirB) + + srcDir := filepath.Join(tmpDirA, "dirSymlink") + linkTarget := filepath.Join(tmpDirA, "dir1") + dstDir := filepath.Join(tmpDirB, "dir2") + resultDir := filepath.Join(dstDir, "dirSymlink") + + var err error + + if err = testCopyHelperFSym(t, srcDir, dstDir); err != nil { + t.Fatalf("unexpected error %T: %s", err, err) + } + + if err = dirContentsEqual(t, resultDir, linkTarget); err != nil { + t.Fatal(err) + } + + // Now try again but using a trailing path separator for dstDir. + + if err = os.RemoveAll(dstDir); err != nil { + t.Fatalf("unable to remove dstDir: %s", err) + } + + if err = os.MkdirAll(dstDir, os.FileMode(0755)); err != nil { + t.Fatalf("unable to make dstDir: %s", err) + } + + dstDir = joinTrailingSep(tmpDirB, "dir2") + + if err = testCopyHelperFSym(t, srcDir, dstDir); err != nil { + t.Fatalf("unexpected error %T: %s", err, err) + } + + if err = dirContentsEqual(t, resultDir, linkTarget); err != nil { + t.Fatal(err) + } +} + // H. SRC specifies a directory's contents only and DST does not exist. This // should create a directory at DST and copy the contents of the SRC // directory (but not the directory itself) into the DST directory. Ensure @@ -553,6 +785,55 @@ func TestCopyCaseH(t *testing.T) { } } +// H. Symbol link following version: +// SRC specifies a directory's contents only and DST does not exist. This +// should create a directory at DST and copy the contents of the SRC +// directory (but not the directory itself) into the DST directory. Ensure +// this works whether DST has a trailing path separator or not. +func TestCopyCaseHFSym(t *testing.T) { + tmpDirA, tmpDirB := getTestTempDirs(t) + defer removeAllPaths(tmpDirA, tmpDirB) + + // Load A with some sample files and directories. + createSampleDir(t, tmpDirA) + + srcDir := joinTrailingSep(tmpDirA, "dirSymlink") + "." + linkTarget := filepath.Join(tmpDirA, "dir1") + dstDir := filepath.Join(tmpDirB, "testDir") + + var err error + + if err = testCopyHelperFSym(t, srcDir, dstDir); err != nil { + t.Fatalf("unexpected error %T: %s", err, err) + } + + if err = dirContentsEqual(t, dstDir, linkTarget); err != nil { + t.Log("dir contents not equal") + logDirContents(t, tmpDirA) + logDirContents(t, tmpDirB) + t.Fatal(err) + } + + // Now try again but using a trailing path separator for dstDir. + + if err = os.RemoveAll(dstDir); err != nil { + t.Fatalf("unable to remove dstDir: %s", err) + } + + dstDir = joinTrailingSep(tmpDirB, "testDir") + + if err = testCopyHelperFSym(t, srcDir, dstDir); err != nil { + t.Fatalf("unexpected error %T: %s", err, err) + } + + if err = dirContentsEqual(t, dstDir, linkTarget); err != nil { + t.Log("dir contents not equal") + logDirContents(t, tmpDirA) + logDirContents(t, tmpDirB) + t.Fatal(err) + } +} + // I. SRC specifies a directory's contents only and DST exists as a file. This // should cause an error as it is not possible to overwrite a file with a // directory. @@ -565,6 +846,7 @@ func TestCopyCaseI(t *testing.T) { createSampleDir(t, tmpDirB) srcDir := joinTrailingSep(tmpDirA, "dir1") + "." + symSrcDir := filepath.Join(tmpDirB, "dirSymlink") dstFile := filepath.Join(tmpDirB, "file1") var err error @@ -576,6 +858,15 @@ func TestCopyCaseI(t *testing.T) { if err != ErrCannotCopyDir { t.Fatalf("expected ErrCannotCopyDir error, but got %T: %s", err, err) } + + // now try with symbol link of dir + if err = testCopyHelperFSym(t, symSrcDir, dstFile); err == nil { + t.Fatal("expected ErrCannotCopyDir error, but got nil instead") + } + + if err != ErrCannotCopyDir { + t.Fatalf("expected ErrCannotCopyDir error, but got %T: %s", err, err) + } } // J. SRC specifies a directory's contents only and DST exists as a directory. @@ -595,6 +886,11 @@ func TestCopyCaseJ(t *testing.T) { var err error + // first to create an empty dir + if err = os.MkdirAll(dstDir, os.FileMode(0755)); err != nil { + t.Fatalf("unable to make dstDir: %s", err) + } + if err = testCopyHelper(t, srcDir, dstDir); err != nil { t.Fatalf("unexpected error %T: %s", err, err) } @@ -623,3 +919,56 @@ func TestCopyCaseJ(t *testing.T) { t.Fatal(err) } } + +// J. Symbol link following version: +// SRC specifies a directory's contents only and DST exists as a directory. +// This should copy the contents of the SRC directory (but not the directory +// itself) into the DST directory. Ensure this works whether DST has a +// trailing path separator or not. +func TestCopyCaseJFSym(t *testing.T) { + tmpDirA, tmpDirB := getTestTempDirs(t) + defer removeAllPaths(tmpDirA, tmpDirB) + + // Load A and B with some sample files and directories. + createSampleDir(t, tmpDirA) + createSampleDir(t, tmpDirB) + + srcDir := joinTrailingSep(tmpDirA, "dirSymlink") + "." + linkTarget := filepath.Join(tmpDirA, "dir1") + dstDir := filepath.Join(tmpDirB, "dir5") + + var err error + + // first to create an empty dir + if err = os.MkdirAll(dstDir, os.FileMode(0755)); err != nil { + t.Fatalf("unable to make dstDir: %s", err) + } + + if err = testCopyHelperFSym(t, srcDir, dstDir); err != nil { + t.Fatalf("unexpected error %T: %s", err, err) + } + + if err = dirContentsEqual(t, dstDir, linkTarget); err != nil { + t.Fatal(err) + } + + // Now try again but using a trailing path separator for dstDir. + + if err = os.RemoveAll(dstDir); err != nil { + t.Fatalf("unable to remove dstDir: %s", err) + } + + if err = os.MkdirAll(dstDir, os.FileMode(0755)); err != nil { + t.Fatalf("unable to make dstDir: %s", err) + } + + dstDir = joinTrailingSep(tmpDirB, "dir5") + + if err = testCopyHelperFSym(t, srcDir, dstDir); err != nil { + t.Fatalf("unexpected error %T: %s", err, err) + } + + if err = dirContentsEqual(t, dstDir, linkTarget); err != nil { + t.Fatal(err) + } +}