Skip to content

Commit

Permalink
Merge pull request kubernetes#51215 from juanvallejo/jvallejo/preserv…
Browse files Browse the repository at this point in the history 
…e-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
  • Loading branch information
Kubernetes Submit Queue authored Oct 12, 2017
2 parents 8c8709d + 2371acc commit 7e38447
Show file tree
Hide file tree
Showing 2 changed files with 127 additions and 10 deletions.
51 changes: 43 additions & 8 deletions pkg/kubectl/cmd/cp.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ package cmd

import (
"archive/tar"
"bytes"
"errors"
"io"
"io/ioutil"
Expand Down Expand Up @@ -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)
}()

Expand Down Expand Up @@ -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
Expand All @@ -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
}
}
Expand All @@ -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
}
Expand Down
86 changes: 84 additions & 2 deletions pkg/kubectl/cmd/cp_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -17,13 +17,15 @@ limitations under the License.
package cmd

import (
"archive/tar"
"bytes"
"io"
"io/ioutil"
"os"
"os/exec"
"path"
"path/filepath"
"strings"
"testing"
)

Expand Down Expand Up @@ -129,7 +131,7 @@ func TestTarUntar(t *testing.T) {
data: "bazblahfoo",
},
{
name: "some/other/directory",
name: "some/other/directory/",
data: "with more data here",
},
{
Expand Down Expand Up @@ -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)
}

Expand Down Expand Up @@ -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)
}
}
}

0 comments on commit 7e38447

Please sign in to comment.