Merge pull request containers#10804 from matejvasek/fix-cp-sub-cmd

Implement --archive flag for podman cp

cmd/podman/containers/cp.go

@@ -28,13 +28,13 @@ var (
   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 can be a file or a directory.
 `
 	cpCommand = &cobra.Command{
-		Use:               "cp [CONTAINER:]SRC_PATH [CONTAINER:]DEST_PATH",
+		Use:               "cp [options] [CONTAINER:]SRC_PATH [CONTAINER:]DEST_PATH",
 		Short:             "Copy files/folders between a container and the local filesystem",
 		Long:              cpDescription,
 		Args:              cobra.ExactArgs(2),
 		RunE:              cp,
 		ValidArgsFunction: common.AutocompleteCpCommand,
-		Example:           "podman cp [CONTAINER:]SRC_PATH [CONTAINER:]DEST_PATH",
+		Example:           "podman cp [options] [CONTAINER:]SRC_PATH [CONTAINER:]DEST_PATH",
 	}
 
 	containerCpCommand = &cobra.Command{
@@ -50,12 +50,14 @@ var (
 
 var (
 	cpOpts entities.ContainerCpOptions
+	chown  bool
 )
 
 func cpFlags(cmd *cobra.Command) {
 	flags := cmd.Flags()
 	flags.BoolVar(&cpOpts.Extract, "extract", false, "Deprecated...")
 	flags.BoolVar(&cpOpts.Pause, "pause", true, "Deprecated")
+	flags.BoolVarP(&chown, "archive", "a", true, `Chown copied files to the primary uid/gid of the destination container.`)
 	_ = flags.MarkHidden("extract")
 	_ = flags.MarkHidden("pause")
 }
@@ -378,7 +380,7 @@ func copyToContainer(container string, containerPath string, hostPath string) er
 			target = filepath.Dir(target)
 		}
 
-		copyFunc, err := registry.ContainerEngine().ContainerCopyFromArchive(registry.GetContext(), container, target, reader)
+		copyFunc, err := registry.ContainerEngine().ContainerCopyFromArchive(registry.GetContext(), container, target, reader, entities.CopyOptions{Chown: chown})
 		if err != nil {
 			return err
 		}

docs/source/markdown/podman-cp.1.md

@@ -4,9 +4,9 @@
 podman\-cp - Copy files/folders between a container and the local filesystem
 
 ## SYNOPSIS
-**podman cp** [*container*:]*src_path* [*container*:]*dest_path*
+**podman cp** [*options*] [*container*:]*src_path* [*container*:]*dest_path*
 
-**podman container cp** [*container*:]*src_path* [*container*:]*dest_path*
+**podman container cp** [*options*] [*container*:]*src_path* [*container*:]*dest_path*
 
 ## DESCRIPTION
 Copy the contents of **src_path** to the **dest_path**. You can copy from the container's filesystem to the local machine or the reverse, from the local filesystem to the container.
@@ -61,6 +61,13 @@ Note that `podman cp` ignores permission errors when copying from a running root
 
 ## OPTIONS
 
+#### **--archive**, **-a**
+
+Archive mode (copy all uid/gid information).
+When set to true, files copied to a container will have changed ownership to the primary uid/gid of the container.
+When set to false, maintain uid/gid from archive sources instead of changing them to the primary uid/gid of the destination container.
+The default is *true*.
+
 ## ALTERNATIVES
 
 Podman has much stronger capabilities than just `podman cp` to achieve copy files between host and container.

libpod/container_api.go

@@ -840,7 +840,7 @@ func (c *Container) ShouldRestart(ctx context.Context) bool {
 
 // CopyFromArchive copies the contents from the specified tarStream to path
 // *inside* the container.
-func (c *Container) CopyFromArchive(ctx context.Context, containerPath string, tarStream io.Reader) (func() error, error) {
+func (c *Container) CopyFromArchive(ctx context.Context, containerPath string, chown bool, tarStream io.Reader) (func() error, error) {
 	if !c.batched {
 		c.lock.Lock()
 		defer c.lock.Unlock()
@@ -850,7 +850,7 @@ func (c *Container) CopyFromArchive(ctx context.Context, containerPath string, t
 		}
 	}
 
-	return c.copyFromArchive(ctx, containerPath, tarStream)
+	return c.copyFromArchive(ctx, containerPath, chown, tarStream)
 }
 
 // CopyToArchive copies the contents from the specified path *inside* the

libpod/container_copy_linux.go

@@ -23,7 +23,7 @@ import (
 	"golang.org/x/sys/unix"
 )
 
-func (c *Container) copyFromArchive(ctx context.Context, path string, reader io.Reader) (func() error, error) {
+func (c *Container) copyFromArchive(ctx context.Context, path string, chown bool, reader io.Reader) (func() error, error) {
 	var (
 		mountPoint   string
 		resolvedRoot string
@@ -62,13 +62,16 @@ func (c *Container) copyFromArchive(ctx context.Context, path string, reader io.
 		}
 	}
 
-	// Make sure we chown the files to the container's main user and group ID.
-	user, err := getContainerUser(c, mountPoint)
-	if err != nil {
-		unmount()
-		return nil, err
+	var idPair *idtools.IDPair
+	if chown {
+		// Make sure we chown the files to the container's main user and group ID.
+		user, err := getContainerUser(c, mountPoint)
+		if err != nil {
+			unmount()
+			return nil, err
+		}
+		idPair = &idtools.IDPair{UID: int(user.UID), GID: int(user.GID)}
 	}
-	idPair := idtools.IDPair{UID: int(user.UID), GID: int(user.GID)}
 
 	decompressed, err := archive.DecompressStream(reader)
 	if err != nil {
@@ -84,8 +87,8 @@ func (c *Container) copyFromArchive(ctx context.Context, path string, reader io.
 		putOptions := buildahCopiah.PutOptions{
 			UIDMap:     c.config.IDMappings.UIDMap,
 			GIDMap:     c.config.IDMappings.GIDMap,
-			ChownDirs:  &idPair,
-			ChownFiles: &idPair,
+			ChownDirs:  idPair,
+			ChownFiles: idPair,
 		}
 
 		return c.joinMountAndExec(ctx,

pkg/api/handlers/compat/containers_archive.go

@@ -9,6 +9,7 @@ import (
 	"github.com/containers/podman/v3/libpod/define"
 	"github.com/containers/podman/v3/pkg/api/handlers/utils"
 	"github.com/containers/podman/v3/pkg/copy"
+	"github.com/containers/podman/v3/pkg/domain/entities"
 	"github.com/containers/podman/v3/pkg/domain/infra/abi"
 	"github.com/gorilla/schema"
 	"github.com/pkg/errors"
@@ -92,11 +93,13 @@ func handleHeadAndGet(w http.ResponseWriter, r *http.Request, decoder *schema.De
 
 func handlePut(w http.ResponseWriter, r *http.Request, decoder *schema.Decoder, runtime *libpod.Runtime) {
 	query := struct {
-		Path string `schema:"path"`
+		Path  string `schema:"path"`
+		Chown bool   `schema:"copyUIDGID"`
 		// TODO handle params below
 		NoOverwriteDirNonDir bool `schema:"noOverwriteDirNonDir"`
-		CopyUIDGID           bool `schema:"copyUIDGID"`
-	}{}
+	}{
+		Chown: utils.IsLibpodRequest(r), // backward compatibility
+	}
 
 	err := decoder.Decode(&query, r.URL.Query())
 	if err != nil {
@@ -107,7 +110,7 @@ func handlePut(w http.ResponseWriter, r *http.Request, decoder *schema.Decoder,
 	containerName := utils.GetName(r)
 	containerEngine := abi.ContainerEngine{Libpod: runtime}
 
-	copyFunc, err := containerEngine.ContainerCopyFromArchive(r.Context(), containerName, query.Path, r.Body)
+	copyFunc, err := containerEngine.ContainerCopyFromArchive(r.Context(), containerName, query.Path, r.Body, entities.CopyOptions{Chown: query.Chown})
 	if errors.Cause(err) == define.ErrNoSuchCtr || os.IsNotExist(err) {
 		// 404 is returned for an absent container and path.  The
 		// clients must deal with it accordingly.

pkg/bindings/containers/archive.go

@@ -50,11 +50,21 @@ func Stat(ctx context.Context, nameOrID string, path string) (*entities.Containe
 }
 
 func CopyFromArchive(ctx context.Context, nameOrID string, path string, reader io.Reader) (entities.ContainerCopyFunc, error) {
+	return CopyFromArchiveWithOptions(ctx, nameOrID, path, reader, nil)
+}
+
+// CopyFromArchiveWithOptions FIXME: remove this function and make CopyFromArchive accept the option as the last parameter in podman 4.0
+func CopyFromArchiveWithOptions(ctx context.Context, nameOrID string, path string, reader io.Reader, options *CopyOptions) (entities.ContainerCopyFunc, error) {
 	conn, err := bindings.GetClient(ctx)
 	if err != nil {
 		return nil, err
 	}
-	params := url.Values{}
+
+	params, err := options.ToParams()
+	if err != nil {
+		return nil, err
+	}
+
 	params.Set("path", path)
 
 	return func() error {

pkg/bindings/containers/types.go

@@ -251,3 +251,11 @@ type ExistsOptions struct {
 	// External checks for containers created outside of Podman
 	External *bool
 }
+
+//go:generate go run ../generator/generator.go CopyOptions
+// CopyOptions are options for copying to containers.
+type CopyOptions struct {
+	// If used with CopyFromArchive and set to true it will change ownership of files from the source tar archive
+	// to the primary uid/gid of the target container.
+	Chown *bool `schema:"copyUIDGID"`
+}

pkg/bindings/containers/types_copy_options.go

@@ -0,0 +1,37 @@
+package containers
+
+import (
+	"net/url"
+
+	"github.com/containers/podman/v3/pkg/bindings/internal/util"
+)
+
+/*
+This file is generated automatically by go generate.  Do not edit.
+*/
+
+// Changed
+func (o *CopyOptions) Changed(fieldName string) bool {
+	return util.Changed(o, fieldName)
+}
+
+// ToParams
+func (o *CopyOptions) ToParams() (url.Values, error) {
+	return util.ToParams(o)
+}
+
+// WithChown
+func (o *CopyOptions) WithChown(value bool) *CopyOptions {
+	v := &value
+	o.Chown = v
+	return o
+}
+
+// GetChown
+func (o *CopyOptions) GetChown() bool {
+	var chown bool
+	if o.Chown == nil {
+		return chown
+	}
+	return *o.Chown
+}

pkg/bindings/internal/util/util.go

@@ -72,14 +72,18 @@ func ToParams(o interface{}) (url.Values, error) {
 		if reflect.Ptr == f.Kind() {
 			f = f.Elem()
 		}
+		paramName := fieldName
+		if pn, ok := sType.Field(i).Tag.Lookup("schema"); ok {
+			paramName = pn
+		}
 		switch {
 		case IsSimpleType(f):
-			params.Set(fieldName, SimpleTypeToParam(f))
+			params.Set(paramName, SimpleTypeToParam(f))
 		case f.Kind() == reflect.Slice:
 			for i := 0; i < f.Len(); i++ {
 				elem := f.Index(i)
 				if IsSimpleType(elem) {
-					params.Add(fieldName, SimpleTypeToParam(elem))
+					params.Add(paramName, SimpleTypeToParam(elem))
 				} else {
 					return nil, errors.New("slices must contain only simple types")
 				}
@@ -95,7 +99,7 @@ func ToParams(o interface{}) (url.Values, error) {
 				return nil, err
 			}
 
-			params.Set(fieldName, s)
+			params.Set(paramName, s)
 		}
 	}
 	return params, nil

pkg/domain/entities/containers.go

@@ -160,6 +160,13 @@ type CommitOptions struct {
 	Writer         io.Writer
 }
 
+type CopyOptions struct {
+	// If used with ContainerCopyFromArchive and set to true
+	// it will change ownership of files from the source tar archive
+	// to the primary uid/gid of the destination container.
+	Chown bool
+}
+
 type CommitReport struct {
 	Id string //nolint
 }

pkg/domain/entities/engine_container.go

@@ -20,7 +20,7 @@ type ContainerEngine interface {
 	ContainerCheckpoint(ctx context.Context, namesOrIds []string, options CheckpointOptions) ([]*CheckpointReport, error)
 	ContainerCleanup(ctx context.Context, namesOrIds []string, options ContainerCleanupOptions) ([]*ContainerCleanupReport, error)
 	ContainerCommit(ctx context.Context, nameOrID string, options CommitOptions) (*CommitReport, error)
-	ContainerCopyFromArchive(ctx context.Context, nameOrID string, path string, reader io.Reader) (ContainerCopyFunc, error)
+	ContainerCopyFromArchive(ctx context.Context, nameOrID, path string, reader io.Reader, options CopyOptions) (ContainerCopyFunc, error)
 	ContainerCopyToArchive(ctx context.Context, nameOrID string, path string, writer io.Writer) (ContainerCopyFunc, error)
 	ContainerCreate(ctx context.Context, s *specgen.SpecGenerator) (*ContainerCreateReport, error)
 	ContainerDiff(ctx context.Context, nameOrID string, options DiffOptions) (*DiffReport, error)

pkg/domain/infra/abi/archive.go

@@ -7,12 +7,12 @@ import (
 	"github.com/containers/podman/v3/pkg/domain/entities"
 )
 
-func (ic *ContainerEngine) ContainerCopyFromArchive(ctx context.Context, nameOrID string, containerPath string, reader io.Reader) (entities.ContainerCopyFunc, error) {
+func (ic *ContainerEngine) ContainerCopyFromArchive(ctx context.Context, nameOrID, containerPath string, reader io.Reader, options entities.CopyOptions) (entities.ContainerCopyFunc, error) {
 	container, err := ic.Libpod.LookupContainer(nameOrID)
 	if err != nil {
 		return nil, err
 	}
-	return container.CopyFromArchive(ctx, containerPath, reader)
+	return container.CopyFromArchive(ctx, containerPath, options.Chown, reader)
 }
 
 func (ic *ContainerEngine) ContainerCopyToArchive(ctx context.Context, nameOrID string, containerPath string, writer io.Writer) (entities.ContainerCopyFunc, error) {

pkg/domain/infra/tunnel/containers.go

@@ -833,8 +833,8 @@ func (ic *ContainerEngine) ContainerPort(ctx context.Context, nameOrID string, o
 	return reports, nil
 }
 
-func (ic *ContainerEngine) ContainerCopyFromArchive(ctx context.Context, nameOrID string, path string, reader io.Reader) (entities.ContainerCopyFunc, error) {
-	return containers.CopyFromArchive(ic.ClientCtx, nameOrID, path, reader)
+func (ic *ContainerEngine) ContainerCopyFromArchive(ctx context.Context, nameOrID, path string, reader io.Reader, options entities.CopyOptions) (entities.ContainerCopyFunc, error) {
+	return containers.CopyFromArchiveWithOptions(ic.ClientCtx, nameOrID, path, reader, new(containers.CopyOptions).WithChown(options.Chown))
 }
 
 func (ic *ContainerEngine) ContainerCopyToArchive(ctx context.Context, nameOrID string, path string, writer io.Writer) (entities.ContainerCopyFunc, error) {

test/apiv2/23-containersArchive.at

@@ -16,7 +16,7 @@ CTR="ArchiveTestingCtr"
 TMPD=$(mktemp -d podman-apiv2-test.archive.XXXXXXXX)
 HELLO_TAR="${TMPD}/hello.tar"
 echo "Hello" > $TMPD/hello.txt
-tar --format=posix -C $TMPD -cvf ${HELLO_TAR} hello.txt &> /dev/null
+tar --owner=1042 --group=1043 --format=posix -C $TMPD -cvf ${HELLO_TAR} hello.txt &> /dev/null
 
 podman run -d --name "${CTR}" "${IMAGE}" top
 
@@ -72,6 +72,20 @@ if [ "$(tar -xf "${TMPD}/body.tar" hello.txt  --to-stdout)" != "Hello" ]; then
   ARCHIVE_TEST_ERROR="1"
 fi
 
+# test if uid/gid was set correctly in the server
+uidngid=$($PODMAN_BIN --root $WORKDIR/server_root exec "${CTR}" stat -c "%u:%g" "/tmp/hello.txt")
+if [[ "${uidngid}" != "1042:1043" ]]; then
+  echo -e "${red}NOK: UID/GID of the file doesn't match.${nc}" 1>&2;
+  ARCHIVE_TEST_ERROR="1"
+fi
+
+# TODO: uid/gid should be also preserved on way back (GET request)
+# right now it ends up as root:root instead of 1042:1043
+#if [[ "$(tar -tvf "${TMPD}/body.tar")" != *"1042/1043"* ]]; then
+#  echo -e "${red}NOK: UID/GID of the file doesn't match.${nc}" 1>&2;
+#  ARCHIVE_TEST_ERROR="1"
+#fi
+
 cleanUpArchiveTest
 if [[ "${ARCHIVE_TEST_ERROR}" ]] ; then
   exit 1;

test/python/docker/compat/test_containers.py

@@ -1,13 +1,18 @@
+import io
 import subprocess
 import sys
 import time
 import unittest
+from typing import IO, Optional
 
 from docker import DockerClient, errors
+from docker.models.containers import Container
 
 from test.python.docker import Podman
 from test.python.docker.compat import common, constant
 
+import tarfile
+
 
 class TestContainers(unittest.TestCase):
     podman = None  # initialized podman configuration for tests
@@ -198,3 +203,37 @@ def test_filters(self):
         filters = {"name": "top"}
         ctnrs = self.client.containers.list(all=True, filters=filters)
         self.assertEqual(len(ctnrs), 1)
+
+    def test_copy_to_container(self):
+        ctr: Optional[Container] = None
+        try:
+            test_file_content = b"Hello World!"
+            ctr = self.client.containers.create(image="alpine", detach=True, command="top")
+            ctr.start()
+
+            buff: IO[bytes] = io.BytesIO()
+            with tarfile.open(fileobj=buff, mode="w:") as tf:
+                ti: tarfile.TarInfo = tarfile.TarInfo()
+                ti.uid = 1042
+                ti.gid = 1043
+                ti.name = "a.txt"
+                ti.path = "a.txt"
+                ti.mode = 0o644
+                ti.type = tarfile.REGTYPE
+                ti.size = len(test_file_content)
+                tf.addfile(ti, fileobj=io.BytesIO(test_file_content))
+
+            buff.seek(0)
+            ctr.put_archive("/tmp/", buff)
+            ret, out = ctr.exec_run(["stat", "-c", "%u:%g", "/tmp/a.txt"])
+
+            self.assertEqual(ret, 0)
+            self.assertTrue(out.startswith(b'1042:1043'), "assert correct uid/gid")
+
+            ret, out = ctr.exec_run(["cat", "/tmp/a.txt"])
+            self.assertEqual(ret, 0)
+            self.assertTrue(out.startswith(test_file_content), "assert file content")
+        finally:
+            if ctr is not None:
+                ctr.stop()
+                ctr.remove()