Merge pull request #59809 from phsiao/59733_port_forward_with_target_port

Automatic merge from submit-queue (batch tested with PRs 59809, 59955). 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>.

kubectl port-forward should resolve service port to target port

**What this PR does / why we need it**:

Continues on the work in #59705, this PR adds additional support for looking up targetPort for a service, as well as enable using svc/name to select a pod.

**Which issue(s) this PR fixes**:
Fixes #15180
Fixes #59733

**Special notes for your reviewer**:

I decided to create pkg/kubectl/util/service_port.go to contain two functions that might be re-usable.

**Release note**:
```release-note
`kubectl port-forward` now supports specifying a service to port forward to: `kubectl port-forward svc/myservice 8443:443`
```

pkg/kubectl/cmd/portforward.go

@@ -23,6 +23,8 @@ import (
 	"net/url"
 	"os"
 	"os/signal"
+	"strconv"
+	"strings"
 	"time"
 
 	"github.com/spf13/cobra"
@@ -35,6 +37,7 @@ import (
 	coreclient "k8s.io/kubernetes/pkg/client/clientset_generated/internalclientset/typed/core/internalversion"
 	"k8s.io/kubernetes/pkg/kubectl/cmd/templates"
 	cmdutil "k8s.io/kubernetes/pkg/kubectl/cmd/util"
+	"k8s.io/kubernetes/pkg/kubectl/util"
 	"k8s.io/kubernetes/pkg/kubectl/util/i18n"
 )
 
@@ -131,6 +134,38 @@ func (f *defaultPortForwarder) ForwardPorts(method string, url *url.URL, opts Po
 	return fw.ForwardPorts()
 }
 
+// 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
+// the port.
+func translateServicePortToTargetPort(ports []string, svc api.Service, pod api.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)
+		if err != nil {
+			return ports, err
+		}
+		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
+		} else {
+			if int32(portnum) != containerPort {
+				translated = append(translated, fmt.Sprintf("%s:%d", parts[0], containerPort))
+			} else {
+				translated = append(translated, port)
+			}
+		}
+	}
+	return translated, 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
@@ -167,7 +202,17 @@ func (o *PortForwardOptions) Complete(f cmdutil.Factory, cmd *cobra.Command, arg
 	}
 
 	o.PodName = forwardablePod.Name
-	o.Ports = args[1:]
+
+	// handle service port mapping to target port if needed
+	switch t := obj.(type) {
+	case *api.Service:
+		o.Ports, err = translateServicePortToTargetPort(args[1:], *t, *forwardablePod)
+		if err != nil {
+			return err
+		}
+	default:
+		o.Ports = args[1:]
+	}
 
 	clientset, err := f.ClientSet()
 	if err != nil {

pkg/kubectl/cmd/portforward_test.go

@@ -21,11 +21,13 @@ import (
 	"net/http"
 	"net/url"
 	"os"
+	"reflect"
 	"testing"
 
 	"github.com/spf13/cobra"
 
 	"k8s.io/apimachinery/pkg/runtime/schema"
+	"k8s.io/apimachinery/pkg/util/intstr"
 	"k8s.io/client-go/rest/fake"
 	api "k8s.io/kubernetes/pkg/apis/core"
 	cmdtesting "k8s.io/kubernetes/pkg/kubectl/cmd/testing"
@@ -132,3 +134,217 @@ func testPortForward(t *testing.T, flags map[string]string, args []string) {
 func TestPortForward(t *testing.T) {
 	testPortForward(t, nil, []string{"foo", ":5000", ":1000"})
 }
+
+func TestTranslateServicePortToTargetPort(t *testing.T) {
+	cases := []struct {
+		name       string
+		svc        api.Service
+		pod        api.Pod
+		ports      []string
+		translated []string
+		err        bool
+	}{
+		{
+			name: "test success 1 (int port)",
+			svc: api.Service{
+				Spec: api.ServiceSpec{
+					Ports: []api.ServicePort{
+						{
+							Port:       80,
+							TargetPort: intstr.FromInt(8080),
+						},
+					},
+				},
+			},
+			pod: api.Pod{
+				Spec: api.PodSpec{
+					Containers: []api.Container{
+						{
+							Ports: []api.ContainerPort{
+								{
+									Name:          "http",
+									ContainerPort: int32(8080)},
+							},
+						},
+					},
+				},
+			},
+			ports:      []string{"80"},
+			translated: []string{"80:8080"},
+			err:        false,
+		},
+		{
+			name: "test success 2 (clusterIP: None)",
+			svc: api.Service{
+				Spec: api.ServiceSpec{
+					ClusterIP: "None",
+					Ports: []api.ServicePort{
+						{
+							Port:       80,
+							TargetPort: intstr.FromInt(8080),
+						},
+					},
+				},
+			},
+			pod: api.Pod{
+				Spec: api.PodSpec{
+					Containers: []api.Container{
+						{
+							Ports: []api.ContainerPort{
+								{
+									Name:          "http",
+									ContainerPort: int32(8080)},
+							},
+						},
+					},
+				},
+			},
+			ports:      []string{"80"},
+			translated: []string{"80"},
+			err:        false,
+		},
+		{
+			name: "test success 3 (named port)",
+			svc: api.Service{
+				Spec: api.ServiceSpec{
+					Ports: []api.ServicePort{
+						{
+							Port:       80,
+							TargetPort: intstr.FromString("http"),
+						},
+						{
+							Port:       443,
+							TargetPort: intstr.FromString("https"),
+						},
+					},
+				},
+			},
+			pod: api.Pod{
+				Spec: api.PodSpec{
+					Containers: []api.Container{
+						{
+							Ports: []api.ContainerPort{
+								{
+									Name:          "http",
+									ContainerPort: int32(8080)},
+								{
+									Name:          "https",
+									ContainerPort: int32(8443)},
+							},
+						},
+					},
+				},
+			},
+			ports:      []string{"80", "443"},
+			translated: []string{"80:8080", "443:8443"},
+			err:        false,
+		},
+		{
+			name: "test success (targetPort omitted)",
+			svc: api.Service{
+				Spec: api.ServiceSpec{
+					Ports: []api.ServicePort{
+						{
+							Port: 80,
+						},
+					},
+				},
+			},
+			pod: api.Pod{
+				Spec: api.PodSpec{
+					Containers: []api.Container{
+						{
+							Ports: []api.ContainerPort{
+								{
+									Name:          "http",
+									ContainerPort: int32(80)},
+							},
+						},
+					},
+				},
+			},
+			ports:      []string{"80"},
+			translated: []string{"80"},
+			err:        false,
+		},
+		{
+			name: "test failure 1 (named port lookup failure)",
+			svc: api.Service{
+				Spec: api.ServiceSpec{
+					Ports: []api.ServicePort{
+						{
+							Port:       80,
+							TargetPort: intstr.FromString("http"),
+						},
+					},
+				},
+			},
+			pod: api.Pod{
+				Spec: api.PodSpec{
+					Containers: []api.Container{
+						{
+							Ports: []api.ContainerPort{
+								{
+									Name:          "https",
+									ContainerPort: int32(443)},
+							},
+						},
+					},
+				},
+			},
+			ports:      []string{"80"},
+			translated: []string{},
+			err:        true,
+		},
+		{
+			name: "test failure 2 (service port not declared)",
+			svc: api.Service{
+				Spec: api.ServiceSpec{
+					Ports: []api.ServicePort{
+						{
+							Port:       80,
+							TargetPort: intstr.FromString("http"),
+						},
+					},
+				},
+			},
+			pod: api.Pod{
+				Spec: api.PodSpec{
+					Containers: []api.Container{
+						{
+							Ports: []api.ContainerPort{
+								{
+									Name:          "https",
+									ContainerPort: int32(443)},
+							},
+						},
+					},
+				},
+			},
+			ports:      []string{"443"},
+			translated: []string{},
+			err:        true,
+		},
+	}
+
+	for _, tc := range cases {
+		translated, err := translateServicePortToTargetPort(tc.ports, tc.svc, 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(translated, tc.translated) {
+			t.Errorf("%v: expected %v; got %v", tc.name, tc.translated, translated)
+		}
+	}
+}

pkg/kubectl/cmd/util/factory_object_mapping.go

@@ -403,6 +403,13 @@ func (f *ring1Factory) AttachablePodForObject(object runtime.Object, timeout tim
 			return nil, fmt.Errorf("invalid label selector: %v", err)
 		}
 
+	case *api.Service:
+		namespace = t.Namespace
+		if t.Spec.Selector == nil || len(t.Spec.Selector) == 0 {
+			return nil, fmt.Errorf("invalid service '%s': Service is defined without a selector", t.Name)
+		}
+		selector = labels.SelectorFromSet(t.Spec.Selector)
+
 	case *api.Pod:
 		return t, nil
 

pkg/kubectl/util/BUILD

@@ -7,6 +7,7 @@ load(
 go_library(
     name = "go_default_library",
     srcs = [
+        "service_port.go",
         "util.go",
     ] + select({
         "@io_bazel_rules_go//go/platform:android": [
@@ -47,8 +48,10 @@ go_library(
     importpath = "k8s.io/kubernetes/pkg/kubectl/util",
     visibility = ["//build/visible_to:pkg_kubectl_util_CONSUMERS"],
     deps = [
+        "//pkg/apis/core:go_default_library",
         "//vendor/k8s.io/apimachinery/pkg/apis/meta/v1:go_default_library",
         "//vendor/k8s.io/apimachinery/pkg/runtime:go_default_library",
+        "//vendor/k8s.io/apimachinery/pkg/util/intstr:go_default_library",
     ] + select({
         "@io_bazel_rules_go//go/platform:android": [
             "//vendor/golang.org/x/sys/unix:go_default_library",
@@ -109,7 +112,14 @@ filegroup(
 
 go_test(
     name = "go_default_test",
-    srcs = ["util_test.go"],
+    srcs = [
+        "service_port_test.go",
+        "util_test.go",
+    ],
     embed = [":go_default_library"],
     importpath = "k8s.io/kubernetes/pkg/kubectl/util",
+    deps = [
+        "//pkg/apis/core:go_default_library",
+        "//vendor/k8s.io/apimachinery/pkg/util/intstr:go_default_library",
+    ],
 )

pkg/kubectl/util/service_port.go

@@ -0,0 +1,63 @@
+/*
+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/apimachinery/pkg/util/intstr"
+	api "k8s.io/kubernetes/pkg/apis/core"
+)
+
+// Lookup containerPort number by its named port name
+func LookupContainerPortNumberByName(pod api.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
+// declare such port (with the input port number returned).
+func LookupContainerPortNumberByServicePort(svc api.Service, pod api.Pod, port int32) (int32, error) {
+	for _, svcportspec := range svc.Spec.Ports {
+		if svcportspec.Port != port {
+			continue
+		}
+		if svc.Spec.ClusterIP == api.ClusterIPNone {
+			return port, nil
+		}
+		if svcportspec.TargetPort.Type == intstr.Int {
+			if svcportspec.TargetPort.IntValue() == 0 {
+				// targetPort is omitted, and the IntValue() would be zero
+				return svcportspec.Port, nil
+			} else {
+				return int32(svcportspec.TargetPort.IntValue()), nil
+			}
+		} else {
+			return LookupContainerPortNumberByName(pod, svcportspec.TargetPort.String())
+		}
+	}
+	return port, fmt.Errorf("Service %s does not have a service port %d", svc.Name, port)
+}

pkg/kubectl/util/service_port_test.go

@@ -0,0 +1,337 @@
+/*
+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/apimachinery/pkg/util/intstr"
+	api "k8s.io/kubernetes/pkg/apis/core"
+)
+
+func TestLookupContainerPortNumberByName(t *testing.T) {
+	cases := []struct {
+		name     string
+		pod      api.Pod
+		portname string
+		portnum  int32
+		err      bool
+	}{
+		{
+			name: "test success 1",
+			pod: api.Pod{
+				Spec: api.PodSpec{
+					Containers: []api.Container{
+						{
+							Ports: []api.ContainerPort{
+								{
+									Name:          "https",
+									ContainerPort: int32(443)},
+								{
+									Name:          "http",
+									ContainerPort: int32(80)},
+							},
+						},
+					},
+				},
+			},
+			portname: "http",
+			portnum:  int32(80),
+			err:      false,
+		},
+		{
+			name: "test faulure 1",
+			pod: api.Pod{
+				Spec: api.PodSpec{
+					Containers: []api.Container{
+						{
+							Ports: []api.ContainerPort{
+								{
+									Name:          "https",
+									ContainerPort: int32(443)},
+							},
+						},
+					},
+				},
+			},
+			portname: "www",
+			portnum:  int32(0),
+			err:      true,
+		},
+	}
+
+	for _, tc := range cases {
+		portnum, err := LookupContainerPortNumberByName(tc.pod, tc.portname)
+		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 portnum != tc.portnum {
+			t.Errorf("%v: expected port number %v; got %v", tc.name, tc.portnum, portnum)
+		}
+	}
+}
+
+func TestLookupContainerPortNumberByServicePort(t *testing.T) {
+	cases := []struct {
+		name          string
+		svc           api.Service
+		pod           api.Pod
+		port          int32
+		containerPort int32
+		err           bool
+	}{
+		{
+			name: "test success 1 (int port)",
+			svc: api.Service{
+				Spec: api.ServiceSpec{
+					Ports: []api.ServicePort{
+						{
+							Port:       80,
+							TargetPort: intstr.FromInt(8080),
+						},
+					},
+				},
+			},
+			pod: api.Pod{
+				Spec: api.PodSpec{
+					Containers: []api.Container{
+						{
+							Ports: []api.ContainerPort{
+								{
+									Name:          "http",
+									ContainerPort: int32(8080)},
+							},
+						},
+					},
+				},
+			},
+			port:          80,
+			containerPort: 8080,
+			err:           false,
+		},
+		{
+			name: "test success 2 (clusterIP: None)",
+			svc: api.Service{
+				Spec: api.ServiceSpec{
+					ClusterIP: api.ClusterIPNone,
+					Ports: []api.ServicePort{
+						{
+							Port:       80,
+							TargetPort: intstr.FromInt(8080),
+						},
+					},
+				},
+			},
+			pod: api.Pod{
+				Spec: api.PodSpec{
+					Containers: []api.Container{
+						{
+							Ports: []api.ContainerPort{
+								{
+									Name:          "http",
+									ContainerPort: int32(8080)},
+							},
+						},
+					},
+				},
+			},
+			port:          80,
+			containerPort: 80,
+			err:           false,
+		},
+		{
+			name: "test success 3 (named port)",
+			svc: api.Service{
+				Spec: api.ServiceSpec{
+					Ports: []api.ServicePort{
+						{
+							Port:       80,
+							TargetPort: intstr.FromString("http"),
+						},
+					},
+				},
+			},
+			pod: api.Pod{
+				Spec: api.PodSpec{
+					Containers: []api.Container{
+						{
+							Ports: []api.ContainerPort{
+								{
+									Name:          "http",
+									ContainerPort: int32(8080)},
+							},
+						},
+					},
+				},
+			},
+			port:          80,
+			containerPort: 8080,
+			err:           false,
+		},
+		{
+			name: "test success (targetPort omitted)",
+			svc: api.Service{
+				Spec: api.ServiceSpec{
+					Ports: []api.ServicePort{
+						{
+							Port: 80,
+						},
+					},
+				},
+			},
+			pod: api.Pod{
+				Spec: api.PodSpec{
+					Containers: []api.Container{
+						{
+							Ports: []api.ContainerPort{
+								{
+									Name:          "http",
+									ContainerPort: int32(80)},
+							},
+						},
+					},
+				},
+			},
+			port:          80,
+			containerPort: 80,
+			err:           false,
+		},
+		{
+			name: "test failure 1 (cannot find a matching named port)",
+			svc: api.Service{
+				Spec: api.ServiceSpec{
+					Ports: []api.ServicePort{
+						{
+							Port:       80,
+							TargetPort: intstr.FromString("http"),
+						},
+					},
+				},
+			},
+			pod: api.Pod{
+				Spec: api.PodSpec{
+					Containers: []api.Container{
+						{
+							Ports: []api.ContainerPort{
+								{
+									Name:          "https",
+									ContainerPort: int32(443)},
+							},
+						},
+					},
+				},
+			},
+			port:          80,
+			containerPort: -1,
+			err:           true,
+		},
+		{
+			name: "test failure 2 (cannot find a matching service port)",
+			svc: api.Service{
+				Spec: api.ServiceSpec{
+					Ports: []api.ServicePort{
+						{
+							Port:       80,
+							TargetPort: intstr.FromString("http"),
+						},
+					},
+				},
+			},
+			pod: api.Pod{
+				Spec: api.PodSpec{
+					Containers: []api.Container{
+						{
+							Ports: []api.ContainerPort{
+								{
+									Name:          "https",
+									ContainerPort: int32(443)},
+							},
+						},
+					},
+				},
+			},
+			port:          443,
+			containerPort: 443,
+			err:           true,
+		},
+		{
+			name: "test failure 2 (cannot find a matching service port, but ClusterIP: None)",
+			svc: api.Service{
+				Spec: api.ServiceSpec{
+					ClusterIP: api.ClusterIPNone,
+					Ports: []api.ServicePort{
+						{
+							Port:       80,
+							TargetPort: intstr.FromString("http"),
+						},
+					},
+				},
+			},
+			pod: api.Pod{
+				Spec: api.PodSpec{
+					Containers: []api.Container{
+						{
+							Ports: []api.ContainerPort{
+								{
+									Name:          "http",
+									ContainerPort: int32(80)},
+							},
+						},
+					},
+				},
+			},
+			port:          443,
+			containerPort: 443,
+			err:           true,
+		},
+	}
+
+	for _, tc := range cases {
+		containerPort, err := LookupContainerPortNumberByServicePort(tc.svc, tc.pod, tc.port)
+		if err != nil {
+			if tc.err {
+				if containerPort != tc.containerPort {
+					t.Errorf("%v: expected port %v; got %v", tc.name, tc.containerPort, containerPort)
+				}
+				continue
+			}
+
+			t.Errorf("%v: unexpected error: %v", tc.name, err)
+			continue
+		}
+
+		if tc.err {
+			t.Errorf("%v: unexpected success", tc.name)
+			continue
+		}
+
+		if containerPort != tc.containerPort {
+			t.Errorf("%v: expected port %v; got %v", tc.name, tc.containerPort, containerPort)
+		}
+	}
+}