podman cp: do not overwrite non-dirs with dirs and vice versa

Add a new `--overwrite` flag to `podman cp` to allow for overwriting in
case existing users depend on the behavior; they will have a workaround.
By default, the flag is turned off to be compatible with Docker and to
have a more sane behavior.

Fixes: #14420
Signed-off-by: Valentin Rothberg <vrothberg@redhat.com>

9 files changed, 142 insertions, 10 deletions

diff --git a/cmd/podman/containers/cp.go b/cmd/podman/containers/cp.go
index eb18dfce4..bf591cf65 100644
--- a/cmd/podman/containers/cp.go
+++ b/cmd/podman/containers/cp.go
@@ -55,10 +55,13 @@ var (
 
 func cpFlags(cmd *cobra.Command) {
 	flags := cmd.Flags()
-	flags.BoolVar(&cpOpts.Extract, "extract", false, "Deprecated...")
-	flags.BoolVar(&cpOpts.Pause, "pause", true, "Deprecated")
+	flags.BoolVar(&cpOpts.OverwriteDirNonDir, "overwrite", false, "Allow to overwrite directories with non-directories and vice versa")
 	flags.BoolVarP(&chown, "archive", "a", true, `Chown copied files to the primary uid/gid of the destination container.`)
+
+	// Deprecated flags (both are NOPs): exist for backwards compat
+	flags.BoolVar(&cpOpts.Extract, "extract", false, "Deprecated...")
 	_ = flags.MarkHidden("extract")
+	flags.BoolVar(&cpOpts.Pause, "pause", true, "Deprecated")
 	_ = flags.MarkHidden("pause")
 }
 
@@ -175,7 +178,7 @@ func copyContainerToContainer(sourceContainer string, sourcePath string, destCon
 	destContainerCopy := func() error {
 		defer reader.Close()
 
-		copyOptions := entities.CopyOptions{Chown: chown}
+		copyOptions := entities.CopyOptions{Chown: chown, NoOverwriteDirNonDir: !cpOpts.OverwriteDirNonDir}
 		if (!sourceContainerInfo.IsDir && !destContainerInfo.IsDir) || destResolvedToParentDir {
 			// If we're having a file-to-file copy, make sure to
 			// rename accordingly.
@@ -294,9 +297,11 @@ func copyFromContainer(container string, containerPath string, hostPath string)
 		}
 
 		putOptions := buildahCopiah.PutOptions{
-			ChownDirs:     &idPair,
-			ChownFiles:    &idPair,
-			IgnoreDevices: true,
+			ChownDirs:            &idPair,
+			ChownFiles:           &idPair,
+			IgnoreDevices:        true,
+			NoOverwriteDirNonDir: !cpOpts.OverwriteDirNonDir,
+			NoOverwriteNonDirDir: !cpOpts.OverwriteDirNonDir,
 		}
 		if (!containerInfo.IsDir && !hostInfo.IsDir) || resolvedToHostParentDir {
 			// If we're having a file-to-file copy, make sure to
@@ -429,7 +434,7 @@ func copyToContainer(container string, containerPath string, hostPath string) er
 			target = filepath.Dir(target)
 		}
 
-		copyFunc, err := registry.ContainerEngine().ContainerCopyFromArchive(registry.GetContext(), container, target, reader, entities.CopyOptions{Chown: chown})
+		copyFunc, err := registry.ContainerEngine().ContainerCopyFromArchive(registry.GetContext(), container, target, reader, entities.CopyOptions{Chown: chown, NoOverwriteDirNonDir: !cpOpts.OverwriteDirNonDir})
 		if err != nil {
 			return err
 		}
diff --git a/docs/source/markdown/podman-cp.1.md b/docs/source/markdown/podman-cp.1.md
index 0c375675d..bb86e3f13 100644
--- a/docs/source/markdown/podman-cp.1.md
+++ b/docs/source/markdown/podman-cp.1.md
@@ -63,6 +63,10 @@ When set to true, files copied to a container will have changed ownership to the
 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**.
 
+#### **--overwrite**
+
+Allow directories to be overwritten with non-directories and vice versa.  By default, `podman cp` errors out when attempting to overwrite, for instance, a regular file with a directory.  Use this option, if you want to allow this behavior.
+
 ## ALTERNATIVES
 
 Podman has much stronger capabilities than just `podman cp` to achieve copying files between the host and containers.
diff --git a/libpod/container_copy_linux.go b/libpod/container_copy_linux.go
index 9528cd06b..6835b2f1f 100644
--- a/libpod/container_copy_linux.go
+++ b/libpod/container_copy_linux.go
@@ -94,6 +94,7 @@ func (c *Container) copyFromArchive(path string, chown, noOverwriteDirNonDir boo
 			ChownDirs:            idPair,
 			ChownFiles:           idPair,
 			NoOverwriteDirNonDir: noOverwriteDirNonDir,
+			NoOverwriteNonDirDir: noOverwriteDirNonDir,
 			Rename:               rename,
 		}
 
diff --git a/pkg/bindings/containers/types.go b/pkg/bindings/containers/types.go
index 81d491bb7..f640ba756 100644
--- a/pkg/bindings/containers/types.go
+++ b/pkg/bindings/containers/types.go
@@ -287,4 +287,7 @@ type CopyOptions struct {
 	Chown *bool `schema:"copyUIDGID"`
 	// Map to translate path names.
 	Rename map[string]string
+	// NoOverwriteDirNonDir when true prevents an existing directory or file from being overwritten
+	// by the other type.
+	NoOverwriteDirNonDir *bool
 }
diff --git a/pkg/bindings/containers/types_copy_options.go b/pkg/bindings/containers/types_copy_options.go
index 8fcfe71a6..e43d79752 100644
--- a/pkg/bindings/containers/types_copy_options.go
+++ b/pkg/bindings/containers/types_copy_options.go
@@ -46,3 +46,18 @@ func (o *CopyOptions) GetRename() map[string]string {
 	}
 	return o.Rename
 }
+
+// WithNoOverwriteDirNonDir set field NoOverwriteDirNonDir to given value
+func (o *CopyOptions) WithNoOverwriteDirNonDir(value bool) *CopyOptions {
+	o.NoOverwriteDirNonDir = &value
+	return o
+}
+
+// GetNoOverwriteDirNonDir returns value of field NoOverwriteDirNonDir
+func (o *CopyOptions) GetNoOverwriteDirNonDir() bool {
+	if o.NoOverwriteDirNonDir == nil {
+		var z bool
+		return z
+	}
+	return *o.NoOverwriteDirNonDir
+}
diff --git a/pkg/domain/entities/containers.go b/pkg/domain/entities/containers.go
index 37711ca58..750f49590 100644
--- a/pkg/domain/entities/containers.go
+++ b/pkg/domain/entities/containers.go
@@ -443,6 +443,9 @@ type ContainerCpOptions struct {
 	Pause bool
 	// Extract the tarfile into the destination directory.
 	Extract bool
+	// OverwriteDirNonDir allows for overwriting a directory with a
+	// non-directory and vice versa.
+	OverwriteDirNonDir bool
 }
 
 // ContainerStatsOptions describes input options for getting
diff --git a/pkg/domain/infra/tunnel/containers.go b/pkg/domain/infra/tunnel/containers.go
index 82e8fbb5b..b68bc46d4 100644
--- a/pkg/domain/infra/tunnel/containers.go
+++ b/pkg/domain/infra/tunnel/containers.go
@@ -949,7 +949,7 @@ func (ic *ContainerEngine) ContainerPort(ctx context.Context, nameOrID string, o
 }
 
 func (ic *ContainerEngine) ContainerCopyFromArchive(ctx context.Context, nameOrID, path string, reader io.Reader, options entities.CopyOptions) (entities.ContainerCopyFunc, error) {
-	copyOptions := new(containers.CopyOptions).WithChown(options.Chown).WithRename(options.Rename)
+	copyOptions := new(containers.CopyOptions).WithChown(options.Chown).WithRename(options.Rename).WithNoOverwriteDirNonDir(options.NoOverwriteDirNonDir)
 	return containers.CopyFromArchiveWithOptions(ic.ClientCtx, nameOrID, path, reader, copyOptions)
 }
 
diff --git a/test/buildah-bud/apply-podman-deltas b/test/buildah-bud/apply-podman-deltas
index 5ebc3151d..50db0255e 100755
--- a/test/buildah-bud/apply-podman-deltas
+++ b/test/buildah-bud/apply-podman-deltas
@@ -218,7 +218,10 @@ skip_if_remote "--output option not implemented in podman-remote" \
                "build with custom build output must fail for bad input"
 
 # https://github.com/containers/podman/issues/14544
-skip_if_remote "bud-logfile-with-split-logfile-by-platform"
+skip_if_remote "logfile not implemented on remote" "bud-logfile-with-split-logfile-by-platform"
+
+# https://github.com/containers/podman/issues/14547
+skip_if_remote "omit-history not implemented on remote" "build-with-omit-history-to-true should not add history"
 
 ###############################################################################
 # BEGIN tests which are skipped due to actual podman or podman-remote bugs.
diff --git a/test/system/065-cp.bats b/test/system/065-cp.bats
index cfbeff3ae..12c6e1a01 100644
--- a/test/system/065-cp.bats
+++ b/test/system/065-cp.bats
@@ -949,9 +949,107 @@ ${randomcontent[1]}" "$description"
     run_podman rm -t 0 -f cpcontainer
 }
 
+@test "podman cp --overwrite file - ctr/ctr" {
+    rand_content_file=$(random_string 50)
+    rand_content_dir=$(random_string 50)
+
+    run_podman run -d --name ctr-file $IMAGE sh -c "echo '$rand_content_file' > /tmp/foo; sleep infinity"
+    run_podman run -d --name ctr-dir  $IMAGE sh -c "mkdir /tmp/foo; echo '$rand_content_dir' > /tmp/foo/file.txt; sleep infinity"
+
+    # overwrite a directory with a file
+    run_podman 125 cp ctr-file:/tmp/foo ctr-dir:/tmp
+    if ! is_remote; then # remote just returns a 500
+        is "$output" ".* error creating \"/tmp/foo\": .*: file exists.*"
+    fi
+    run_podman cp --overwrite ctr-file:/tmp/foo ctr-dir:/tmp
+    run_podman exec ctr-dir cat /tmp/foo
+    is "$output" "$rand_content_file"
+
+    # reset the ctr-dir container
+    run_podman exec ctr-dir sh -c "rm -rf /tmp/foo; mkdir /tmp/foo; echo '$rand_content_dir' > /tmp/foo/file.txt"
+
+    # overwrite a file with a directory
+    run_podman 125 cp ctr-dir:/tmp/foo ctr-file:/tmp
+    if ! is_remote; then # remote just returns a 500
+        is "$output" ".* error creating \"/tmp/foo\": .*: file exists.*"
+    fi
+    run_podman cp --overwrite ctr-dir:/tmp/foo ctr-file:/tmp
+    run_podman exec ctr-file cat /tmp/foo/file.txt
+    is "$output" "$rand_content_dir"
+
+    run_podman rm -t 0 -f ctr-file ctr-dir
+}
+
+@test "podman cp --overwrite file - ctr/host" {
+    hostdir=$PODMAN_TMPDIR/cp-test
+    mkdir -p $hostdir
+
+    rand_content_file=$(random_string 50)
+    rand_content_dir=$(random_string 50)
+
+    run_podman run -d --name ctr-file $IMAGE sh -c "echo '$rand_content_file' > /tmp/foo; sleep infinity"
+    run_podman run -d --name ctr-dir  $IMAGE sh -c "mkdir /tmp/foo; echo '$rand_content_dir' > /tmp/foo/file.txt; sleep infinity"
+
+    # overwrite a directory with a file
+    mkdir $hostdir/foo
+    run_podman 125 cp ctr-file:/tmp/foo $hostdir
+    if ! is_remote; then # remote just returns a 500
+        is "$output" ".* error creating \"/foo\": .*: file exists.*"
+    fi
+    run_podman cp --overwrite ctr-file:/tmp/foo $hostdir
+    is "$(< $hostdir/foo)" "$rand_content_file"
+
+    # overwrite a file with a directory
+    rm -rf $hostdir/foo
+    touch $hostdir/foo
+    run_podman 125 cp ctr-dir:/tmp/foo $hostdir
+    if ! is_remote; then # remote just returns a 500
+        is "$output" ".* error creating \"/foo\": .*: file exists.*"
+    fi
+    run_podman cp --overwrite ctr-dir:/tmp/foo $hostdir
+    is "$(< $hostdir/foo/file.txt)" "$rand_content_dir"
+
+    run_podman rm -t 0 -f ctr-file ctr-dir
+}
+
+@test "podman cp --overwrite file - host/ctr" {
+    hostdir=$PODMAN_TMPDIR/cp-test
+    mkdir -p $hostdir
+
+    rand_content_file=$(random_string 50)
+    rand_content_dir=$(random_string 50)
+
+    run_podman run -d --name ctr-dir  $IMAGE sh -c "mkdir /tmp/foo; sleep infinity"
+    run_podman run -d --name ctr-file $IMAGE sh -c "touch /tmp/foo; sleep infinity"
+
+    # overwrite a directory with a file
+    echo "$rand_content_file" > $hostdir/foo
+    run_podman 125 cp  $hostdir/foo ctr-dir:/tmp
+    if ! is_remote; then # remote just returns a 500
+        is "$output" ".* error creating \"/tmp/foo\": .*: file exists.*"
+    fi
+    run_podman cp --overwrite $hostdir/foo ctr-dir:/tmp
+    run_podman exec ctr-dir cat /tmp/foo
+    is "$output" "$rand_content_file"
+
+    # overwrite a file with a directory
+    rm -f $hostdir/foo
+    mkdir $hostdir/foo
+    echo "$rand_content_dir" > $hostdir/foo/file.txt
+    run_podman 125 cp $hostdir/foo ctr-file:/tmp
+    if ! is_remote; then # remote just returns a 500
+        is "$output" ".* error creating \"/tmp/foo\": .*: file exists.*"
+    fi
+    run_podman cp --overwrite $hostdir/foo ctr-file:/tmp
+    run_podman exec ctr-file cat /tmp/foo/file.txt
+    is "$output" "$rand_content_dir"
+
+    run_podman rm -t 0 -f ctr-file ctr-dir
+}
+
 function teardown() {
     # In case any test fails, clean up the container we left behind
-    run_podman rm -t 0 f cpcontainer
+    run_podman rm -t 0 -f --ignore cpcontainer
     basic_teardown
 }