Skip to content

Commit

Permalink
podman cp: fix copying to a non-existent dir
Browse files Browse the repository at this point in the history 
Copy is full of perils.  Some of them are the nuances when copying
directories.  Who would have thought that
 * cp dir   foo
 * cp dir/  foo
 * cp dir/. foo
are all supposed to yield the same result when foo does not exist.

`podman cp` now supports all three notations, which required to massage
the front-end code in `cmd/podman` a bit.  The tests have been extended
and partially rewritten to test container->host and host->container
copy operations.

Signed-off-by: Valentin Rothberg <[email protected]>
  • Loading branch information
@vrothberg
vrothberg committed Mar 9, 2021
1 parent a61d70c commit 31b11b5
Show file tree
Hide file tree
Showing 2 changed files with 73 additions and 38 deletions.
49 changes: 41 additions & 8 deletions cmd/podman/containers/cp.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -160,6 +160,25 @@ func copyFromContainer(container string, containerPath string, hostPath string)
}
}

// If we copy a directory via the "." notation and the host path does
// not exist, we need to make sure that the destination on the host
// gets created; otherwise the contents of the source directory will be
// written to the destination's parent directory.
//
// While we could cut it short on the host and do create the directory
// ourselves, we would run into problems trying to that the other way
// around when copying into a container. Instead, to keep both
// implementations symmetrical, we need to massage the code a bit to
// let Buildah's copier package create the destination.
//
// Hence, whenever "." is the source and the destination does not exist,
// we copy the source's parent and let the copier package create the
// destination via the Rename option.
containerTarget := containerInfo.LinkTarget
if hostInfoErr != nil && containerInfo.IsDir && strings.HasSuffix(containerTarget, ".") {
containerTarget = filepath.Dir(containerTarget)
}

reader, writer := io.Pipe()
hostCopy := func() error {
defer reader.Close()
Expand Down Expand Up @@ -193,10 +212,10 @@ func copyFromContainer(container string, containerPath string, hostPath string)
ChownFiles: &idPair,
IgnoreDevices: true,
}
if !containerInfo.IsDir && (!hostInfo.IsDir || hostInfoErr != nil) {
if (!containerInfo.IsDir && !hostInfo.IsDir) || hostInfoErr != nil {
// If we're having a file-to-file copy, make sure to
// rename accordingly.
putOptions.Rename = map[string]string{filepath.Base(containerInfo.LinkTarget): hostBaseName}
putOptions.Rename = map[string]string{filepath.Base(containerTarget): hostBaseName}
}
dir := hostInfo.LinkTarget
if !hostInfo.IsDir {
Expand All @@ -210,7 +229,7 @@ func copyFromContainer(container string, containerPath string, hostPath string)

containerCopy := func() error {
defer writer.Close()
copyFunc, err := registry.ContainerEngine().ContainerCopyToArchive(registry.GetContext(), container, containerInfo.LinkTarget, writer)
copyFunc, err := registry.ContainerEngine().ContainerCopyToArchive(registry.GetContext(), container, containerTarget, writer)
if err != nil {
return err
}
Expand Down Expand Up @@ -278,6 +297,19 @@ func copyToContainer(container string, containerPath string, hostPath string) er
containerBaseName = filepath.Base(containerInfo.LinkTarget)
}

// If we copy a directory via the "." notation and the container path
// does not exist, we need to make sure that the destination on the
// container gets created; otherwise the contents of the source
// directory will be written to the destination's parent directory.
//
// Hence, whenever "." is the source and the destination does not
// exist, we copy the source's parent and let the copier package create
// the destination via the Rename option.
hostTarget := hostInfo.LinkTarget
if containerInfoErr != nil && hostInfo.IsDir && strings.HasSuffix(hostTarget, ".") {
hostTarget = filepath.Dir(hostTarget)
}

var stdinFile string
if isStdin {
if !containerInfo.IsDir {
Expand Down Expand Up @@ -318,15 +350,16 @@ func copyToContainer(container string, containerPath string, hostPath string) er
}

getOptions := buildahCopiah.GetOptions{
// Unless the specified points to ".", we want to copy the base directory.
KeepDirectoryNames: hostInfo.IsDir && filepath.Base(hostPath) != ".",
// Unless the specified path points to ".", we want to
// copy the base directory.
KeepDirectoryNames: hostInfo.IsDir && filepath.Base(hostTarget) != ".",
}
if !hostInfo.IsDir && (!containerInfo.IsDir || containerInfoErr != nil) {
if (!hostInfo.IsDir && !containerInfo.IsDir) || containerInfoErr != nil {
// If we're having a file-to-file copy, make sure to
// rename accordingly.
getOptions.Rename = map[string]string{filepath.Base(hostInfo.LinkTarget): containerBaseName}
getOptions.Rename = map[string]string{filepath.Base(hostTarget): containerBaseName}
}
if err := buildahCopiah.Get("/", "", getOptions, []string{hostInfo.LinkTarget}, writer); err != nil {
if err := buildahCopiah.Get("/", "", getOptions, []string{hostTarget}, writer); err != nil {
return errors.Wrap(err, "error copying from host")
}
return nil
Expand Down
62 changes: 32 additions & 30 deletions test/system/065-cp.bats
View file
Original file line number Diff line number Diff line change
Expand Up @@ -192,20 +192,19 @@ load helpers


@test "podman cp dir from host to container" {
dirname=dir-test
srcdir=$PODMAN_TMPDIR/$dirname
mkdir -p $srcdir
srcdir=$PODMAN_TMPDIR
mkdir -p $srcdir/dir/sub
local -a randomcontent=(
random-0-$(random_string 10)
random-1-$(random_string 15)
)
echo "${randomcontent[0]}" > $srcdir/hostfile0
echo "${randomcontent[1]}" > $srcdir/hostfile1
echo "${randomcontent[0]}" > $srcdir/dir/sub/hostfile0
echo "${randomcontent[1]}" > $srcdir/dir/sub/hostfile1

# "." and "dir/." will copy the contents, so make sure that a dir ending
# with dot is treated correctly.
mkdir -p $srcdir.
cp $srcdir/* $srcdir./
mkdir -p $srcdir/dir.
cp -r $srcdir/dir/* $srcdir/dir.

run_podman run -d --name cpcontainer --workdir=/srv $IMAGE sleep infinity
run_podman exec cpcontainer mkdir /srv/subdir
Expand All @@ -216,12 +215,15 @@ load helpers

# format is: <source arg to cp (appended to srcdir)> | <destination arg to cp> | <full dest path> | <test name>
tests="
| / | /dir-test | copy to root
. | / | /dir-test. | copy dotdir to root
/ | /tmp | /tmp/dir-test | copy to tmp
/. | /usr/ | /usr/ | copy contents of dir to usr/
| . | /srv/dir-test | copy to workdir (rel path)
| subdir/. | /srv/subdir/dir-test | copy to workdir subdir (rel path)
dir | / | /dir/sub | copy dir to root
dir. | / | /dir./sub | copy dir. to root
dir/ | /tmp | /tmp/dir/sub | copy dir/ to tmp
dir/. | /usr/ | /usr/sub | copy dir/. usr/
dir/sub | . | /srv/sub | copy dir/sub to workdir (rel path)
dir/sub/. | subdir/. | /srv/subdir | copy dir/sub/. to workdir subdir (rel path)
dir | /newdir1 | /newdir1/sub | copy dir to newdir1
dir/ | /newdir2 | /newdir2/sub | copy dir/ to newdir2
dir/. | /newdir3 | /newdir3/sub | copy dir/. to newdir3
"

# RUNNING container
Expand All @@ -230,12 +232,10 @@ load helpers
if [[ $src == "''" ]];then
unset src
fi
run_podman cp $srcdir$src cpcontainer:$dest
run_podman exec cpcontainer ls $dest_fullname
run_podman exec cpcontainer cat $dest_fullname/hostfile0
is "$output" "${randomcontent[0]}" "$description (cp -> ctr:$dest)"
run_podman exec cpcontainer cat $dest_fullname/hostfile1
is "$output" "${randomcontent[1]}" "$description (cp -> ctr:$dest)"
run_podman cp $srcdir/$src cpcontainer:$dest
run_podman exec cpcontainer cat $dest_fullname/hostfile0 $dest_fullname/hostfile1
is "${lines[0]}" "${randomcontent[0]}" "$description (cp -> ctr:$dest)"
is "${lines[1]}" "${randomcontent[1]}" "$description (cp -> ctr:$dest)"
done < <(parse_table "$tests")
run_podman kill cpcontainer
run_podman rm -f cpcontainer
Expand All @@ -247,7 +247,7 @@ load helpers
unset src
fi
run_podman create --name cpcontainer --workdir=/srv $cpimage sleep infinity
run_podman cp $srcdir$src cpcontainer:$dest
run_podman cp $srcdir/$src cpcontainer:$dest
run_podman start cpcontainer
run_podman exec cpcontainer cat $dest_fullname/hostfile0 $dest_fullname/hostfile1
is "${lines[0]}" "${randomcontent[0]}" "$description (cp -> ctr:$dest)"
Expand Down Expand Up @@ -280,17 +280,19 @@ load helpers
run_podman commit -q cpcontainer
cpimage="$output"

# format is: <source arg to cp (appended to /srv)> | <full dest path> | <test name>
# format is: <source arg to cp (appended to /srv)> | <dest> | <full dest path> | <test name>
tests="
/srv | /srv/subdir | copy /srv
/srv/ | /srv/subdir | copy /srv/
/srv/. | /subdir | copy /srv/.
/srv/subdir/. | | copy /srv/subdir/.
/tmp/subdir. | /subdir. | copy /tmp/subdir.
/srv | | /srv/subdir | copy /srv
/srv | /newdir | /newdir/subdir | copy /srv to /newdir
/srv/ | | /srv/subdir | copy /srv/
/srv/. | | /subdir | copy /srv/.
/srv/. | /newdir | /newdir/subdir | copy /srv/. to /newdir
/srv/subdir/. | | | copy /srv/subdir/.
/tmp/subdir. | | /subdir. | copy /tmp/subdir.
"

# RUNNING container
while read src dest_fullname description; do
while read src dest dest_fullname description; do
if [[ $src == "''" ]];then
unset src
fi
Expand All @@ -300,7 +302,7 @@ load helpers
if [[ $dest_fullname == "''" ]];then
unset dest_fullname
fi
run_podman cp cpcontainer:$src $destdir
run_podman cp cpcontainer:$src $destdir$dest
is "$(< $destdir$dest_fullname/containerfile0)" "${randomcontent[0]}" "$description"
is "$(< $destdir$dest_fullname/containerfile1)" "${randomcontent[1]}" "$description"
rm -rf $destdir/*
Expand All @@ -310,7 +312,7 @@ load helpers

# CREATED container
run_podman create --name cpcontainer --workdir=/srv $cpimage
while read src dest_fullname description; do
while read src dest dest_fullname description; do
if [[ $src == "''" ]];then
unset src
fi
Expand All @@ -320,7 +322,7 @@ load helpers
if [[ $dest_fullname == "''" ]];then
unset dest_fullname
fi
run_podman cp cpcontainer:$src $destdir
run_podman cp cpcontainer:$src $destdir$dest
is "$(< $destdir$dest_fullname/containerfile0)" "${randomcontent[0]}" "$description"
is "$(< $destdir$dest_fullname/containerfile1)" "${randomcontent[1]}" "$description"
rm -rf $destdir/*
Expand Down

0 comments on commit 31b11b5

Please sign in to comment.