From e55a28d1691e85815c5f7e124a6d97f8fa4e8cb6 Mon Sep 17 00:00:00 2001 From: juanvallejo Date: Mon, 18 Jun 2018 12:44:36 -0400 Subject: [PATCH] fix paths w shortcuts when copying from pods Addresses an issue where copying from a remote location containing path shortcuts (podName:../../../tmp/foo) causes an index out of range panic. --- pkg/kubectl/cmd/cp/cp.go | 12 ++++ pkg/kubectl/cmd/cp/cp_test.go | 56 +++++++++++++++++++ test/e2e/kubectl/kubectl.go | 42 ++++++++++++++ .../kubectl/busybox-pod.yaml | 13 +++++ test/test_owners.csv | 1 + 5 files changed, 124 insertions(+) create mode 100644 test/e2e/testing-manifests/kubectl/busybox-pod.yaml diff --git a/pkg/kubectl/cmd/cp/cp.go b/pkg/kubectl/cmd/cp/cp.go index 3a4ea6d331b..bc804ceb5c5 100644 --- a/pkg/kubectl/cmd/cp/cp.go +++ b/pkg/kubectl/cmd/cp/cp.go @@ -303,6 +303,18 @@ func (o *CopyOptions) copyFromPod(src, dest fileSpec) error { // stripPathShortcuts removes any leading or trailing "../" from a given path func stripPathShortcuts(p string) string { newPath := path.Clean(p) + trimmed := strings.TrimPrefix(newPath, "../") + + for trimmed != newPath { + newPath = trimmed + trimmed = strings.TrimPrefix(newPath, "../") + } + + // trim leftover ".." + if newPath == ".." { + newPath = "" + } + if len(newPath) > 0 && string(newPath[0]) == "/" { return newPath[1:] } diff --git a/pkg/kubectl/cmd/cp/cp_test.go b/pkg/kubectl/cmd/cp/cp_test.go index 89e56c31651..f1e7f38f1a1 100644 --- a/pkg/kubectl/cmd/cp/cp_test.go +++ b/pkg/kubectl/cmd/cp/cp_test.go @@ -127,6 +127,62 @@ func TestGetPrefix(t *testing.T) { } } +func TestStripPathShortcuts(t *testing.T) { + tests := []struct { + name string + input string + expected string + }{ + { + name: "test single path shortcut prefix", + input: "../foo/bar", + expected: "foo/bar", + }, + { + name: "test multiple path shortcuts", + input: "../../foo/bar", + expected: "foo/bar", + }, + { + name: "test multiple path shortcuts with absolute path", + input: "/tmp/one/two/../../foo/bar", + expected: "tmp/foo/bar", + }, + { + name: "test multiple path shortcuts with no named directory", + input: "../../", + expected: "", + }, + { + name: "test multiple path shortcuts with no named directory and no trailing slash", + input: "../..", + expected: "", + }, + { + name: "test multiple path shortcuts with absolute path and filename containing leading dots", + input: "/tmp/one/two/../../foo/..bar", + expected: "tmp/foo/..bar", + }, + { + name: "test multiple path shortcuts with no named directory and filename containing leading dots", + input: "../...foo", + expected: "...foo", + }, + { + name: "test filename containing leading dots", + input: "...foo", + expected: "...foo", + }, + } + + for _, test := range tests { + out := stripPathShortcuts(test.input) + if out != test.expected { + t.Errorf("expected: %s, saw: %s", test.expected, out) + } + } +} + func TestTarUntar(t *testing.T) { dir, err := ioutil.TempDir("", "input") dir2, err2 := ioutil.TempDir("", "output") diff --git a/test/e2e/kubectl/kubectl.go b/test/e2e/kubectl/kubectl.go index cd486dd09cf..fafda661ea1 100644 --- a/test/e2e/kubectl/kubectl.go +++ b/test/e2e/kubectl/kubectl.go @@ -79,6 +79,8 @@ const ( simplePodPort = 80 pausePodSelector = "name=pause" pausePodName = "pause" + busyboxPodSelector = "app=busybox1" + busyboxPodName = "busybox1" runJobTimeout = 5 * time.Minute kubeCtlManifestPath = "test/e2e/testing-manifests/kubectl" redisControllerFilename = "redis-master-controller.json.in" @@ -1078,6 +1080,46 @@ metadata: }) }) + framework.KubeDescribe("Kubectl copy", func() { + var podYaml string + var nsFlag string + BeforeEach(func() { + By("creating the pod") + nsFlag = fmt.Sprintf("--namespace=%v", ns) + podYaml = substituteImageName(string(readTestFileOrDie("busybox-pod.yaml"))) + framework.RunKubectlOrDieInput(podYaml, "create", "-f", "-", nsFlag) + Expect(framework.CheckPodsRunningReady(c, ns, []string{busyboxPodName}, framework.PodStartTimeout)).To(BeTrue()) + }) + AfterEach(func() { + cleanupKubectlInputs(podYaml, ns, busyboxPodSelector) + }) + + /* + Release : v1.12 + Testname: Kubectl, copy + Description: When a Pod is running, copy a known file from it to a temporary local destination. + */ + It("should copy a file from a running Pod", func() { + remoteContents := "foobar\n" + podSource := fmt.Sprintf("%s:/root/foo/bar/foo.bar", busyboxPodName) + tempDestination, err := ioutil.TempFile(os.TempDir(), "copy-foobar") + if err != nil { + framework.Failf("Failed creating temporary destination file: %v", err) + } + + By("specifying a remote filepath " + podSource + " on the pod") + framework.RunKubectlOrDie("cp", podSource, tempDestination.Name(), nsFlag) + By("verifying that the contents of the remote file " + podSource + " have been copied to a local file " + tempDestination.Name()) + localData, err := ioutil.ReadAll(tempDestination) + if err != nil { + framework.Failf("Failed reading temporary local file: %v", err) + } + if string(localData) != remoteContents { + framework.Failf("Failed copying remote file contents. Expected %s but got %s", remoteContents, string(localData)) + } + }) + }) + framework.KubeDescribe("Kubectl logs", func() { var nsFlag string var rc string diff --git a/test/e2e/testing-manifests/kubectl/busybox-pod.yaml b/test/e2e/testing-manifests/kubectl/busybox-pod.yaml new file mode 100644 index 00000000000..bbe1bea94aa --- /dev/null +++ b/test/e2e/testing-manifests/kubectl/busybox-pod.yaml @@ -0,0 +1,13 @@ +apiVersion: v1 +kind: Pod +metadata: + name: busybox1 + labels: + app: busybox1 +spec: + containers: + - image: busybox + command: ["/bin/sh", "-c", "mkdir -p /root/foo/bar && echo 'foobar' > /root/foo/bar/foo.bar && sleep 3600"] + imagePullPolicy: IfNotPresent + name: busybox + restartPolicy: Always diff --git a/test/test_owners.csv b/test/test_owners.csv index a38a69470ae..0e98e68d3b1 100644 --- a/test/test_owners.csv +++ b/test/test_owners.csv @@ -191,6 +191,7 @@ Kubectl client Kubectl api-versions should check if v1 is in available api versi Kubectl client Kubectl apply should apply a new configuration to an existing RC,pwittrock,0,cli Kubectl client Kubectl apply should reuse port when apply to an existing SVC,deads2k,0,cli Kubectl client Kubectl cluster-info should check if Kubernetes master services is included in cluster-info,pwittrock,0,cli +Kubectl client Kubectl copy should copy a file from a running Pod,juanvallejo,0,cli Kubectl client Kubectl create quota should create a quota with scopes,rrati,0,cli Kubectl client Kubectl create quota should create a quota without scopes,xiang90,1,cli Kubectl client Kubectl create quota should reject quota with invalid scopes,brendandburns,1,cli