Merge pull request #69477 from m1kola/port-forward_named_port

Adds support of named ports for kubectl port-forward

pkg/kubectl/cmd/portforward/portforward.go

@@ -146,6 +146,17 @@ func (f *defaultPortForwarder) ForwardPorts(method string, url *url.URL, opts Po
 	return fw.ForwardPorts()
 }
 
+// splitPort splits port string which is in form of [LOCAL PORT]:REMOTE PORT
+// and returns local and remote ports separately
+func splitPort(port string) (local, remote string) {
+	parts := strings.Split(port, ":")
+	if len(parts) == 2 {
+		return parts[0], parts[1]
+	}
+
+	return parts[0], parts[0]
+}
+
 // Translates service port to target port
 // It rewrites ports as needed if the Service port declares targetPort.
 // It returns an error when a named targetPort can't find a match in the pod, or the Service did not declare
@@ -153,31 +164,63 @@ func (f *defaultPortForwarder) ForwardPorts(method string, url *url.URL, opts Po
 func translateServicePortToTargetPort(ports []string, svc corev1.Service, pod corev1.Pod) ([]string, error) {
 	var translated []string
 	for _, port := range ports {
-		// port is in the form of [LOCAL PORT]:REMOTE PORT
-		parts := strings.Split(port, ":")
-		input := parts[0]
-		if len(parts) == 2 {
-			input = parts[1]
-		}
-		portnum, err := strconv.Atoi(input)
+		localPort, remotePort := splitPort(port)
+
+		portnum, err := strconv.Atoi(remotePort)
 		if err != nil {
-			return ports, err
+			svcPort, err := util.LookupServicePortNumberByName(svc, remotePort)
+			if err != nil {
+				return nil, err
+			}
+			portnum = int(svcPort)
+
+			if localPort == remotePort {
+				localPort = strconv.Itoa(portnum)
+			}
 		}
 		containerPort, err := util.LookupContainerPortNumberByServicePort(svc, pod, int32(portnum))
 		if err != nil {
 			// can't resolve a named port, or Service did not declare this port, return an error
 			return nil, err
+		}
+
+		if int32(portnum) != containerPort {
+			translated = append(translated, fmt.Sprintf("%s:%d", localPort, containerPort))
 		} else {
-			if int32(portnum) != containerPort {
-				translated = append(translated, fmt.Sprintf("%s:%d", parts[0], containerPort))
-			} else {
-				translated = append(translated, port)
-			}
+			translated = append(translated, port)
 		}
 	}
 	return translated, nil
 }
 
+// convertPodNamedPortToNumber converts named ports into port numbers
+// It returns an error when a named port can't be found in the pod containers
+func convertPodNamedPortToNumber(ports []string, pod corev1.Pod) ([]string, error) {
+	var converted []string
+	for _, port := range ports {
+		localPort, remotePort := splitPort(port)
+
+		containerPortStr := remotePort
+		_, err := strconv.Atoi(remotePort)
+		if err != nil {
+			containerPort, err := util.LookupContainerPortNumberByName(pod, remotePort)
+			if err != nil {
+				return nil, err
+			}
+
+			containerPortStr = strconv.Itoa(int(containerPort))
+		}
+
+		if localPort != remotePort {
+			converted = append(converted, fmt.Sprintf("%s:%s", localPort, containerPortStr))
+		} else {
+			converted = append(converted, containerPortStr)
+		}
+	}
+
+	return converted, nil
+}
+
 // Complete completes all the required options for port-forward cmd.
 func (o *PortForwardOptions) Complete(f cmdutil.Factory, cmd *cobra.Command, args []string) error {
 	var err error
@@ -223,7 +266,10 @@ func (o *PortForwardOptions) Complete(f cmdutil.Factory, cmd *cobra.Command, arg
 			return err
 		}
 	default:
-		o.Ports = args[1:]
+		o.Ports, err = convertPodNamedPortToNumber(args[1:], *forwardablePod)
+		if err != nil {
+			return err
+		}
 	}
 
 	clientset, err := f.KubernetesClientSet()

pkg/kubectl/cmd/portforward/portforward_test.go

@@ -180,6 +180,35 @@ func TestTranslateServicePortToTargetPort(t *testing.T) {
 			translated: []string{"80:8080"},
 			err:        false,
 		},
+		{
+			name: "test success 1 (int port with random local port)",
+			svc: corev1.Service{
+				Spec: corev1.ServiceSpec{
+					Ports: []corev1.ServicePort{
+						{
+							Port:       80,
+							TargetPort: intstr.FromInt(8080),
+						},
+					},
+				},
+			},
+			pod: corev1.Pod{
+				Spec: corev1.PodSpec{
+					Containers: []corev1.Container{
+						{
+							Ports: []corev1.ContainerPort{
+								{
+									Name:          "http",
+									ContainerPort: int32(8080)},
+							},
+						},
+					},
+				},
+			},
+			ports:      []string{":80"},
+			translated: []string{":8080"},
+			err:        false,
+		},
 		{
 			name: "test success 2 (clusterIP: None)",
 			svc: corev1.Service{
@@ -211,7 +240,37 @@ func TestTranslateServicePortToTargetPort(t *testing.T) {
 			err:        false,
 		},
 		{
-			name: "test success 3 (named port)",
+			name: "test success 2 (clusterIP: None with random local port)",
+			svc: corev1.Service{
+				Spec: corev1.ServiceSpec{
+					ClusterIP: "None",
+					Ports: []corev1.ServicePort{
+						{
+							Port:       80,
+							TargetPort: intstr.FromInt(8080),
+						},
+					},
+				},
+			},
+			pod: corev1.Pod{
+				Spec: corev1.PodSpec{
+					Containers: []corev1.Container{
+						{
+							Ports: []corev1.ContainerPort{
+								{
+									Name:          "http",
+									ContainerPort: int32(8080)},
+							},
+						},
+					},
+				},
+			},
+			ports:      []string{":80"},
+			translated: []string{":80"},
+			err:        false,
+		},
+		{
+			name: "test success 3 (named target port)",
 			svc: corev1.Service{
 				Spec: corev1.ServiceSpec{
 					Ports: []corev1.ServicePort{
@@ -246,6 +305,114 @@ func TestTranslateServicePortToTargetPort(t *testing.T) {
 			translated: []string{"80:8080", "443:8443"},
 			err:        false,
 		},
+		{
+			name: "test success 3 (named target port with random local port)",
+			svc: corev1.Service{
+				Spec: corev1.ServiceSpec{
+					Ports: []corev1.ServicePort{
+						{
+							Port:       80,
+							TargetPort: intstr.FromString("http"),
+						},
+						{
+							Port:       443,
+							TargetPort: intstr.FromString("https"),
+						},
+					},
+				},
+			},
+			pod: corev1.Pod{
+				Spec: corev1.PodSpec{
+					Containers: []corev1.Container{
+						{
+							Ports: []corev1.ContainerPort{
+								{
+									Name:          "http",
+									ContainerPort: int32(8080)},
+								{
+									Name:          "https",
+									ContainerPort: int32(8443)},
+							},
+						},
+					},
+				},
+			},
+			ports:      []string{":80", ":443"},
+			translated: []string{":8080", ":8443"},
+			err:        false,
+		},
+		{
+			name: "test success 4 (named service port)",
+			svc: corev1.Service{
+				Spec: corev1.ServiceSpec{
+					Ports: []corev1.ServicePort{
+						{
+							Port:       80,
+							Name:       "http",
+							TargetPort: intstr.FromInt(8080),
+						},
+						{
+							Port:       443,
+							Name:       "https",
+							TargetPort: intstr.FromInt(8443),
+						},
+					},
+				},
+			},
+			pod: corev1.Pod{
+				Spec: corev1.PodSpec{
+					Containers: []corev1.Container{
+						{
+							Ports: []corev1.ContainerPort{
+								{
+									ContainerPort: int32(8080)},
+								{
+									ContainerPort: int32(8443)},
+							},
+						},
+					},
+				},
+			},
+			ports:      []string{"http", "https"},
+			translated: []string{"80:8080", "443:8443"},
+			err:        false,
+		},
+		{
+			name: "test success 4 (named service port with random local port)",
+			svc: corev1.Service{
+				Spec: corev1.ServiceSpec{
+					Ports: []corev1.ServicePort{
+						{
+							Port:       80,
+							Name:       "http",
+							TargetPort: intstr.FromInt(8080),
+						},
+						{
+							Port:       443,
+							Name:       "https",
+							TargetPort: intstr.FromInt(8443),
+						},
+					},
+				},
+			},
+			pod: corev1.Pod{
+				Spec: corev1.PodSpec{
+					Containers: []corev1.Container{
+						{
+							Ports: []corev1.ContainerPort{
+								{
+									ContainerPort: int32(8080)},
+								{
+									ContainerPort: int32(8443)},
+							},
+						},
+					},
+				},
+			},
+			ports:      []string{":http", ":https"},
+			translated: []string{":8080", ":8443"},
+			err:        false,
+		},
 		{
 			name: "test success (targetPort omitted)",
 			svc: corev1.Service{
@@ -275,7 +442,35 @@ func TestTranslateServicePortToTargetPort(t *testing.T) {
 			err:        false,
 		},
 		{
-			name: "test failure 1 (named port lookup failure)",
+			name: "test success (targetPort omitted with random local port)",
+			svc: corev1.Service{
+				Spec: corev1.ServiceSpec{
+					Ports: []corev1.ServicePort{
+						{
+							Port: 80,
+						},
+					},
+				},
+			},
+			pod: corev1.Pod{
+				Spec: corev1.PodSpec{
+					Containers: []corev1.Container{
+						{
+							Ports: []corev1.ContainerPort{
+								{
+									Name:          "http",
+									ContainerPort: int32(80)},
+							},
+						},
+					},
+				},
+			},
+			ports:      []string{":80"},
+			translated: []string{":80"},
+			err:        false,
+		},
+		{
+			name: "test failure 1 (named target port lookup failure)",
 			svc: corev1.Service{
 				Spec: corev1.ServiceSpec{
 					Ports: []corev1.ServicePort{
@@ -303,6 +498,35 @@ func TestTranslateServicePortToTargetPort(t *testing.T) {
 			translated: []string{},
 			err:        true,
 		},
+		{
+			name: "test failure 1 (named service port lookup failure)",
+			svc: corev1.Service{
+				Spec: corev1.ServiceSpec{
+					Ports: []corev1.ServicePort{
+						{
+							Port:       80,
+							TargetPort: intstr.FromString("http"),
+						},
+					},
+				},
+			},
+			pod: corev1.Pod{
+				Spec: corev1.PodSpec{
+					Containers: []corev1.Container{
+						{
+							Ports: []corev1.ContainerPort{
+								{
+									Name:          "http",
+									ContainerPort: int32(8080)},
+							},
+						},
+					},
+				},
+			},
+			ports:      []string{"https"},
+			translated: []string{},
+			err:        true,
+		},
 		{
 			name: "test failure 2 (service port not declared)",
 			svc: corev1.Service{
@@ -373,3 +597,185 @@ func execPod() *corev1.Pod {
 		},
 	}
 }
+
+func TestConvertPodNamedPortToNumber(t *testing.T) {
+	cases := []struct {
+		name      string
+		pod       corev1.Pod
+		ports     []string
+		converted []string
+		err       bool
+	}{
+		{
+			name: "port number without local port",
+			pod: corev1.Pod{
+				Spec: corev1.PodSpec{
+					Containers: []corev1.Container{
+						{
+							Ports: []corev1.ContainerPort{
+								{
+									Name:          "http",
+									ContainerPort: int32(80)},
+							},
+						},
+					},
+				},
+			},
+			ports:     []string{"80"},
+			converted: []string{"80"},
+			err:       false,
+		},
+		{
+			name: "port number with local port",
+			pod: corev1.Pod{
+				Spec: corev1.PodSpec{
+					Containers: []corev1.Container{
+						{
+							Ports: []corev1.ContainerPort{
+								{
+									Name:          "http",
+									ContainerPort: int32(80)},
+							},
+						},
+					},
+				},
+			},
+			ports:     []string{"8000:80"},
+			converted: []string{"8000:80"},
+			err:       false,
+		},
+		{
+			name: "port number with random local port",
+			pod: corev1.Pod{
+				Spec: corev1.PodSpec{
+					Containers: []corev1.Container{
+						{
+							Ports: []corev1.ContainerPort{
+								{
+									Name:          "http",
+									ContainerPort: int32(80)},
+							},
+						},
+					},
+				},
+			},
+			ports:     []string{":80"},
+			converted: []string{":80"},
+			err:       false,
+		},
+		{
+			name: "named port without local port",
+			pod: corev1.Pod{
+				Spec: corev1.PodSpec{
+					Containers: []corev1.Container{
+						{
+							Ports: []corev1.ContainerPort{
+								{
+									Name:          "http",
+									ContainerPort: int32(80)},
+							},
+						},
+					},
+				},
+			},
+			ports:     []string{"http"},
+			converted: []string{"80"},
+			err:       false,
+		},
+		{
+			name: "named port with local port",
+			pod: corev1.Pod{
+				Spec: corev1.PodSpec{
+					Containers: []corev1.Container{
+						{
+							Ports: []corev1.ContainerPort{
+								{
+									Name:          "http",
+									ContainerPort: int32(80)},
+							},
+						},
+					},
+				},
+			},
+			ports:     []string{"8000:http"},
+			converted: []string{"8000:80"},
+			err:       false,
+		},
+		{
+			name: "named port with random local port",
+			pod: corev1.Pod{
+				Spec: corev1.PodSpec{
+					Containers: []corev1.Container{
+						{
+							Ports: []corev1.ContainerPort{
+								{
+									Name:          "http",
+									ContainerPort: int32(80)},
+							},
+						},
+					},
+				},
+			},
+			ports:     []string{":http"},
+			converted: []string{":80"},
+			err:       false,
+		},
+		{
+			name: "named port can not be found",
+			pod: corev1.Pod{
+				Spec: corev1.PodSpec{
+					Containers: []corev1.Container{
+						{
+							Ports: []corev1.ContainerPort{
+								{
+									Name:          "https",
+									ContainerPort: int32(443)},
+							},
+						},
+					},
+				},
+			},
+			ports: []string{"http"},
+			err:   true,
+		},
+		{
+			name: "one of the requested named ports can not be found",
+			pod: corev1.Pod{
+				Spec: corev1.PodSpec{
+					Containers: []corev1.Container{
+						{
+							Ports: []corev1.ContainerPort{
+								{
+									Name:          "https",
+									ContainerPort: int32(443)},
+							},
+						},
+					},
+				},
+			},
+			ports: []string{"https", "http"},
+			err:   true,
+		},
+	}
+
+	for _, tc := range cases {
+		converted, err := convertPodNamedPortToNumber(tc.ports, tc.pod)
+		if err != nil {
+			if tc.err {
+				continue
+			}
+
+			t.Errorf("%v: unexpected error: %v", tc.name, err)
+			continue
+		}
+
+		if tc.err {
+			t.Errorf("%v: unexpected success", tc.name)
+			continue
+		}
+
+		if !reflect.DeepEqual(converted, tc.converted) {
+			t.Errorf("%v: expected %v; got %v", tc.name, tc.converted, converted)
+		}
+	}
+}

pkg/kubectl/util/BUILD

@@ -7,6 +7,7 @@ load(
 go_library(
     name = "go_default_library",
     srcs = [
+        "pod_port.go",
         "service_port.go",
         "umask.go",
         "umask_windows.go",
@@ -78,6 +79,7 @@ filegroup(
 go_test(
     name = "go_default_test",
     srcs = [
+        "pod_port_test.go",
         "service_port_test.go",
         "util_test.go",
     ],

pkg/kubectl/util/pod_port.go

@@ -0,0 +1,36 @@
+/*
+Copyright 2018 The Kubernetes Authors.
+
+Licensed under the Apache License, Version 2.0 (the "License");
+you may not use this file except in compliance with the License.
+You may obtain a copy of the License at
+
+    http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+package util
+
+import (
+	"fmt"
+
+	"k8s.io/api/core/v1"
+)
+
+// LookupContainerPortNumberByName find containerPort number by its named port name
+func LookupContainerPortNumberByName(pod v1.Pod, name string) (int32, error) {
+	for _, ctr := range pod.Spec.Containers {
+		for _, ctrportspec := range ctr.Ports {
+			if ctrportspec.Name == name {
+				return ctrportspec.ContainerPort, nil
+			}
+		}
+	}
+
+	return int32(-1), fmt.Errorf("Pod '%s' does not have a named port '%s'", pod.Name, name)
+}

pkg/kubectl/util/pod_port_test.go

@@ -0,0 +1,98 @@
+/*
+Copyright 2018 The Kubernetes Authors.
+
+Licensed under the Apache License, Version 2.0 (the "License");
+you may not use this file except in compliance with the License.
+You may obtain a copy of the License at
+
+    http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+package util
+
+import (
+	"testing"
+
+	"k8s.io/api/core/v1"
+)
+
+func TestLookupContainerPortNumberByName(t *testing.T) {
+	tests := []struct {
+		name     string
+		pod      v1.Pod
+		portname string
+		portnum  int32
+		err      bool
+	}{
+		{
+			name: "test success 1",
+			pod: v1.Pod{
+				Spec: v1.PodSpec{
+					Containers: []v1.Container{
+						{
+							Ports: []v1.ContainerPort{
+								{
+									Name:          "https",
+									ContainerPort: int32(443)},
+								{
+									Name:          "http",
+									ContainerPort: int32(80)},
+							},
+						},
+					},
+				},
+			},
+			portname: "http",
+			portnum:  int32(80),
+			err:      false,
+		},
+		{
+			name: "test faulure 1",
+			pod: v1.Pod{
+				Spec: v1.PodSpec{
+					Containers: []v1.Container{
+						{
+							Ports: []v1.ContainerPort{
+								{
+									Name:          "https",
+									ContainerPort: int32(443)},
+							},
+						},
+					},
+				},
+			},
+			portname: "www",
+			portnum:  int32(0),
+			err:      true,
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			portnum, err := LookupContainerPortNumberByName(tt.pod, tt.portname)
+			if err != nil {
+				if tt.err {
+					return
+				}
+
+				t.Errorf("%v: unexpected error: %v", tt.name, err)
+				return
+			}
+
+			if tt.err {
+				t.Errorf("%v: unexpected success", tt.name)
+				return
+			}
+
+			if portnum != tt.portnum {
+				t.Errorf("%v: expected port number %v; got %v", tt.name, tt.portnum, portnum)
+			}
+		})
+	}
+}

pkg/kubectl/util/service_port.go

@@ -23,19 +23,6 @@ import (
 	"k8s.io/apimachinery/pkg/util/intstr"
 )
 
-// Lookup containerPort number by its named port name
-func lookupContainerPortNumberByName(pod v1.Pod, name string) (int32, error) {
-	for _, ctr := range pod.Spec.Containers {
-		for _, ctrportspec := range ctr.Ports {
-			if ctrportspec.Name == name {
-				return ctrportspec.ContainerPort, nil
-			}
-		}
-	}
-
-	return int32(-1), fmt.Errorf("Pod '%s' does not have a named port '%s'", pod.Name, name)
-}
-
 // Lookup containerPort number from Service port number
 // It implements the handling of resolving container named port, as well as ignoring targetPort when clusterIP=None
 // It returns an error when a named port can't find a match (with -1 returned), or when the service does not
@@ -56,8 +43,19 @@ func LookupContainerPortNumberByServicePort(svc v1.Service, pod v1.Pod, port int
 				return int32(svcportspec.TargetPort.IntValue()), nil
 			}
 		} else {
-			return lookupContainerPortNumberByName(pod, svcportspec.TargetPort.String())
+			return LookupContainerPortNumberByName(pod, svcportspec.TargetPort.String())
 		}
 	}
 	return port, fmt.Errorf("Service %s does not have a service port %d", svc.Name, port)
 }
+
+// LookupServicePortNumberByName find service port number by its named port name
+func LookupServicePortNumberByName(svc v1.Service, name string) (int32, error) {
+	for _, svcportspec := range svc.Spec.Ports {
+		if svcportspec.Name == name {
+			return svcportspec.Port, nil
+		}
+	}
+
+	return int32(-1), fmt.Errorf("Service '%s' does not have a named port '%s'", svc.Name, name)
+}

pkg/kubectl/util/service_port_test.go

@@ -23,81 +23,6 @@ import (
 	"k8s.io/apimachinery/pkg/util/intstr"
 )
 
-func TestLookupContainerPortNumberByName(t *testing.T) {
-	tests := []struct {
-		name     string
-		pod      v1.Pod
-		portname string
-		portnum  int32
-		err      bool
-	}{
-		{
-			name: "test success 1",
-			pod: v1.Pod{
-				Spec: v1.PodSpec{
-					Containers: []v1.Container{
-						{
-							Ports: []v1.ContainerPort{
-								{
-									Name:          "https",
-									ContainerPort: int32(443)},
-								{
-									Name:          "http",
-									ContainerPort: int32(80)},
-							},
-						},
-					},
-				},
-			},
-			portname: "http",
-			portnum:  int32(80),
-			err:      false,
-		},
-		{
-			name: "test faulure 1",
-			pod: v1.Pod{
-				Spec: v1.PodSpec{
-					Containers: []v1.Container{
-						{
-							Ports: []v1.ContainerPort{
-								{
-									Name:          "https",
-									ContainerPort: int32(443)},
-							},
-						},
-					},
-				},
-			},
-			portname: "www",
-			portnum:  int32(0),
-			err:      true,
-		},
-	}
-
-	for _, tt := range tests {
-		t.Run(tt.name, func(t *testing.T) {
-			portnum, err := lookupContainerPortNumberByName(tt.pod, tt.portname)
-			if err != nil {
-				if tt.err {
-					return
-				}
-
-				t.Errorf("%v: unexpected error: %v", tt.name, err)
-				return
-			}
-
-			if tt.err {
-				t.Errorf("%v: unexpected success", tt.name)
-				return
-			}
-
-			if portnum != tt.portnum {
-				t.Errorf("%v: expected port number %v; got %v", tt.name, tt.portnum, portnum)
-			}
-		})
-	}
-}
-
 func TestLookupContainerPortNumberByServicePort(t *testing.T) {
 	tests := []struct {
 		name          string