Merge pull request #51215 from juanvallejo/jvallejo/preserve-specified-destination-path

Automatic merge from submit-queue (batch tested with PRs 53668, 53624, 52639, 53581, 51215). If you want to cherry-pick this change to another branch, please follow the instructions <a href="https://github.com/kubernetes/community/blob/master/contributors/devel/cherry-picks.md">here</a>.

preserve specified destination path

**Release note**:
```release-note
"kubectl cp" updated to honor destination names 
```

**Before**
```
$ kubectl cp foo_dir pod_name:/tmp/bar_dir
$ kubectl exec pod_name -it -- /bin/sh
sh-4.2$
sh-4.2$ ls /tmp
sh-4.2$ foo_dir
```

**After**
```
$ kubectl cp foo_dir pod_name:/tmp/bar_dir
$ kubectl exec pod_name -it -- /bin/sh
sh-4.2$
sh-4.2$ ls /tmp
sh-4.2$ bar_dir
```


**Notable changes to `kubectl cp` After This Patch**
- Copying a directory `bar_dir` to an existing directory in the pod will copy the directory itself, rather than just the file contents:

```bash
*Before*
> remote-pod-shell$ ls /tmp
                    existing_remote_dir              

$ kubectl cp ./my/local/awesome_dir mypod:/tmp/existing_remote_dir
> remote-pod-shell$ ls /tmp
                    existing_remote_dir
                    awesome_dir
```
```bash
*After*
> remote-pod-shell$ ls /tmp
                    existing_remote_dir              

$ kubectl cp ./my/local/awesome_dir mypod:/tmp/existing_remote_dir
> remote-pod-shell$ ls /tmp
                    existing_remote_dir
> remote-pod-shell$ ls /tmp/existing_remote_dir
                    awesome_dir
```

```
*Before*: Directory contents were merged if a local and remote directory shared the same name
*After*:  A new name will be honored for the copied local directory on the remote pod.
          If a new name was not specified for the local directory being copied, and it shares the
          same name as an already-existing directory on the pod, current behavior will follow and
          its contents will be added to those of the already-existing directory.
```

```
*Before*: If a trailing slash (e.g. kubectl cp ./local/dir pod:/tmp) was not added to a directory
          name in the destination path (...:/tmp vs /tmp/...), when copying to a pod, `kubectl`
          would attempt to copy the local directory under the parent of the remote directory
          rather than inside of it.
*After*:  Slashes do not alter the behavior of the command, or destination of the intended 
          source file or directory. With a command such as (kubectl cp ./local_dir pod:/tmp),
          `local_dir` would be copied inside of <pod:/tmp> (an error is returned if pod:/tmp is
           a file).
```

Related downstream bugzilla: https://bugzilla.redhat.com/show_bug.cgi?id=1469411

@fabianofranz @kubernetes/sig-cli-misc

pkg/kubectl/cmd/cp.go

@@ -18,6 +18,7 @@ package cmd
 
 import (
 	"archive/tar"
+	"bytes"
 	"errors"
 	"io"
 	"io/ioutil"
@@ -134,11 +135,44 @@ func runCopy(f cmdutil.Factory, cmd *cobra.Command, out, cmderr io.Writer, args
 	return cmdutil.UsageErrorf(cmd, "One of src or dest must be a remote file specification")
 }
 
+// checkDestinationIsDir receives a destination fileSpec and
+// determines if the provided destination path exists on the
+// pod. If the destination path does not exist or is _not_ a
+// directory, an error is returned with the exit code received.
+func checkDestinationIsDir(dest fileSpec, f cmdutil.Factory, cmd *cobra.Command) error {
+	options := &ExecOptions{
+		StreamOptions: StreamOptions{
+			Out: bytes.NewBuffer([]byte{}),
+			Err: bytes.NewBuffer([]byte{}),
+
+			Namespace: dest.PodNamespace,
+			PodName:   dest.PodName,
+		},
+
+		Command:  []string{"test", "-d", dest.File},
+		Executor: &DefaultRemoteExecutor{},
+	}
+
+	return execute(f, cmd, options)
+}
+
 func copyToPod(f cmdutil.Factory, cmd *cobra.Command, stdout, stderr io.Writer, src, dest fileSpec) error {
 	reader, writer := io.Pipe()
+
+	// strip trailing slash (if any)
+	if strings.HasSuffix(string(dest.File[len(dest.File)-1]), "/") {
+		dest.File = dest.File[:len(dest.File)-1]
+	}
+
+	if err := checkDestinationIsDir(dest, f, cmd); err == nil {
+		// If no error, dest.File was found to be a directory.
+		// Copy specified src into it
+		dest.File = dest.File + "/" + path.Base(src.File)
+	}
+
 	go func() {
 		defer writer.Close()
-		err := makeTar(src.File, writer)
+		err := makeTar(src.File, dest.File, writer)
 		cmdutil.CheckErr(err)
 	}()
 
@@ -192,17 +226,18 @@ func copyFromPod(f cmdutil.Factory, cmd *cobra.Command, cmderr io.Writer, src, d
 	return untarAll(reader, dest.File, prefix)
 }
 
-func makeTar(filepath string, writer io.Writer) error {
+func makeTar(srcPath, destPath string, writer io.Writer) error {
 	// TODO: use compression here?
 	tarWriter := tar.NewWriter(writer)
 	defer tarWriter.Close()
 
-	filepath = path.Clean(filepath)
-	return recursiveTar(path.Dir(filepath), path.Base(filepath), tarWriter)
+	srcPath = path.Clean(srcPath)
+	destPath = path.Clean(destPath)
+	return recursiveTar(path.Dir(srcPath), path.Base(srcPath), path.Dir(destPath), path.Base(destPath), tarWriter)
 }
 
-func recursiveTar(base, file string, tw *tar.Writer) error {
-	filepath := path.Join(base, file)
+func recursiveTar(srcBase, srcFile, destBase, destFile string, tw *tar.Writer) error {
+	filepath := path.Join(srcBase, srcFile)
 	stat, err := os.Stat(filepath)
 	if err != nil {
 		return err
@@ -213,7 +248,7 @@ func recursiveTar(base, file string, tw *tar.Writer) error {
 			return err
 		}
 		for _, f := range files {
-			if err := recursiveTar(base, path.Join(file, f.Name()), tw); err != nil {
+			if err := recursiveTar(srcBase, path.Join(srcFile, f.Name()), destBase, path.Join(destFile, f.Name()), tw); err != nil {
 				return err
 			}
 		}
@@ -223,7 +258,7 @@ func recursiveTar(base, file string, tw *tar.Writer) error {
 	if err != nil {
 		return err
 	}
-	hdr.Name = file
+	hdr.Name = destFile
 	if err := tw.WriteHeader(hdr); err != nil {
 		return err
 	}

pkg/kubectl/cmd/cp_test.go

@@ -17,6 +17,7 @@ limitations under the License.
 package cmd
 
 import (
+	"archive/tar"
 	"bytes"
 	"io"
 	"io/ioutil"
@@ -24,6 +25,7 @@ import (
 	"os/exec"
 	"path"
 	"path/filepath"
+	"strings"
 	"testing"
 )
 
@@ -129,7 +131,7 @@ func TestTarUntar(t *testing.T) {
 			data: "bazblahfoo",
 		},
 		{
-			name: "some/other/directory",
+			name: "some/other/directory/",
 			data: "with more data here",
 		},
 		{
@@ -157,7 +159,7 @@ func TestTarUntar(t *testing.T) {
 	}
 
 	writer := &bytes.Buffer{}
-	if err := makeTar(dir, writer); err != nil {
+	if err := makeTar(dir, dir2, writer); err != nil {
 		t.Errorf("unexpected error: %v", err)
 	}
 
@@ -292,3 +294,83 @@ func TestCopyToLocalFileOrDir(t *testing.T) {
 	}
 
 }
+
+func TestTarDestinationName(t *testing.T) {
+	dir, err := ioutil.TempDir(os.TempDir(), "input")
+	dir2, err2 := ioutil.TempDir(os.TempDir(), "output")
+	if err != nil || err2 != nil {
+		t.Errorf("unexpected error: %v | %v", err, err2)
+		t.FailNow()
+	}
+	defer func() {
+		if err := os.RemoveAll(dir); err != nil {
+			t.Errorf("Unexpected error cleaning up: %v", err)
+		}
+		if err := os.RemoveAll(dir2); err != nil {
+			t.Errorf("Unexpected error cleaning up: %v", err)
+		}
+	}()
+
+	files := []struct {
+		name string
+		data string
+	}{
+		{
+			name: "foo",
+			data: "foobarbaz",
+		},
+		{
+			name: "dir/blah",
+			data: "bazblahfoo",
+		},
+		{
+			name: "some/other/directory",
+			data: "with more data here",
+		},
+		{
+			name: "blah",
+			data: "same file name different data",
+		},
+	}
+
+	// ensure files exist on disk
+	for _, file := range files {
+		filepath := path.Join(dir, file.name)
+		if err := os.MkdirAll(path.Dir(filepath), 0755); err != nil {
+			t.Errorf("unexpected error: %v", err)
+			t.FailNow()
+		}
+		f, err := os.Create(filepath)
+		if err != nil {
+			t.Errorf("unexpected error: %v", err)
+			t.FailNow()
+		}
+		defer f.Close()
+		if _, err := io.Copy(f, bytes.NewBuffer([]byte(file.data))); err != nil {
+			t.Errorf("unexpected error: %v", err)
+			t.FailNow()
+		}
+	}
+
+	reader, writer := io.Pipe()
+	go func() {
+		if err := makeTar(dir, dir2, writer); err != nil {
+			t.Errorf("unexpected error: %v", err)
+		}
+	}()
+
+	tarReader := tar.NewReader(reader)
+	for {
+		hdr, err := tarReader.Next()
+		if err == io.EOF {
+			break
+		} else if err != nil {
+			t.Errorf("unexpected error: %v", err)
+			t.FailNow()
+		}
+
+		if !strings.HasPrefix(hdr.Name, path.Base(dir2)) {
+			t.Errorf("expected %q as destination filename prefix, saw: %q", path.Base(dir2), hdr.Name)
+		}
+	}
+}