diff --git a/pkg/kubectl/cmd/portforward/portforward.go b/pkg/kubectl/cmd/portforward/portforward.go index 236293e1b5..af03b46ef2 100644 --- a/pkg/kubectl/cmd/portforward/portforward.go +++ b/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() diff --git a/pkg/kubectl/cmd/portforward/portforward_test.go b/pkg/kubectl/cmd/portforward/portforward_test.go index 52ef9902f1..18afc37f55 100644 --- a/pkg/kubectl/cmd/portforward/portforward_test.go +++ b/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) + } + } +} diff --git a/pkg/kubectl/util/BUILD b/pkg/kubectl/util/BUILD index 8eea40ed10..794e6e8a7d 100644 --- a/pkg/kubectl/util/BUILD +++ b/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", ], diff --git a/pkg/kubectl/util/pod_port.go b/pkg/kubectl/util/pod_port.go new file mode 100644 index 0000000000..6d78501a89 --- /dev/null +++ b/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) +} diff --git a/pkg/kubectl/util/pod_port_test.go b/pkg/kubectl/util/pod_port_test.go new file mode 100644 index 0000000000..cecf515161 --- /dev/null +++ b/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) + } + }) + } +} diff --git a/pkg/kubectl/util/service_port.go b/pkg/kubectl/util/service_port.go index 8c9caf91d8..18960b1486 100644 --- a/pkg/kubectl/util/service_port.go +++ b/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) +} diff --git a/pkg/kubectl/util/service_port_test.go b/pkg/kubectl/util/service_port_test.go index 3956dac398..f15ae3c353 100644 --- a/pkg/kubectl/util/service_port_test.go +++ b/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