Merge pull request #65189 from juanvallejo/jvallejo/path-fixes-cmd-copy

fix paths w shortcuts when copying from pods

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:]
 	}

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")

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

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

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