From ca8da7e2984a92a9c024f8f0e6cac2b9ae87339b Mon Sep 17 00:00:00 2001 From: knight42 Date: Mon, 1 Jun 2020 12:28:01 +0800 Subject: [PATCH] feat(port-forward): warn users about UDP port Signed-off-by: knight42 --- .../k8s.io/kubectl/pkg/cmd/portforward/BUILD | 1 + .../pkg/cmd/portforward/portforward.go | 74 ++++++++++++ .../pkg/cmd/portforward/portforward_test.go | 114 ++++++++++++++++++ 3 files changed, 189 insertions(+) diff --git a/staging/src/k8s.io/kubectl/pkg/cmd/portforward/BUILD b/staging/src/k8s.io/kubectl/pkg/cmd/portforward/BUILD index 93311f0a7c9..222b0f60997 100644 --- a/staging/src/k8s.io/kubectl/pkg/cmd/portforward/BUILD +++ b/staging/src/k8s.io/kubectl/pkg/cmd/portforward/BUILD @@ -9,6 +9,7 @@ go_library( deps = [ "//staging/src/k8s.io/api/core/v1:go_default_library", "//staging/src/k8s.io/apimachinery/pkg/apis/meta/v1:go_default_library", + "//staging/src/k8s.io/apimachinery/pkg/util/sets:go_default_library", "//staging/src/k8s.io/cli-runtime/pkg/genericclioptions:go_default_library", "//staging/src/k8s.io/client-go/kubernetes/scheme:go_default_library", "//staging/src/k8s.io/client-go/kubernetes/typed/core/v1:go_default_library", diff --git a/staging/src/k8s.io/kubectl/pkg/cmd/portforward/portforward.go b/staging/src/k8s.io/kubectl/pkg/cmd/portforward/portforward.go index 8fd25b87d86..8800a9e726c 100644 --- a/staging/src/k8s.io/kubectl/pkg/cmd/portforward/portforward.go +++ b/staging/src/k8s.io/kubectl/pkg/cmd/portforward/portforward.go @@ -31,6 +31,7 @@ import ( corev1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/util/sets" "k8s.io/cli-runtime/pkg/genericclioptions" "k8s.io/client-go/kubernetes/scheme" corev1client "k8s.io/client-go/kubernetes/typed/core/v1" @@ -225,6 +226,71 @@ func convertPodNamedPortToNumber(ports []string, pod corev1.Pod) ([]string, erro return converted, nil } +func checkUDPPorts(udpOnlyPorts sets.Int, ports []string, obj metav1.Object) error { + for _, port := range ports { + _, remotePort := splitPort(port) + portNum, err := strconv.Atoi(remotePort) + if err != nil { + switch v := obj.(type) { + case *corev1.Service: + svcPort, err := util.LookupServicePortNumberByName(*v, remotePort) + if err != nil { + return err + } + portNum = int(svcPort) + + case *corev1.Pod: + ctPort, err := util.LookupContainerPortNumberByName(*v, remotePort) + if err != nil { + return err + } + portNum = int(ctPort) + + default: + return fmt.Errorf("unknown object: %v", obj) + } + } + if udpOnlyPorts.Has(portNum) { + return fmt.Errorf("UDP protocol is not supported for %s", remotePort) + } + } + return nil +} + +// checkUDPPortInService returns an error if remote port in Service is a UDP port +// TODO: remove this check after #47862 is solved +func checkUDPPortInService(ports []string, svc *corev1.Service) error { + udpOnlyPorts := sets.NewInt() + for _, port := range svc.Spec.Ports { + portNum := int(port.Port) + switch port.Protocol { + case corev1.ProtocolUDP: + udpOnlyPorts.Insert(portNum) + case corev1.ProtocolTCP: + udpOnlyPorts.Delete(portNum) + } + } + return checkUDPPorts(udpOnlyPorts, ports, svc) +} + +// checkUDPPortInPod returns an error if remote port in Pod is a UDP port +// TODO: remove this check after #47862 is solved +func checkUDPPortInPod(ports []string, pod *corev1.Pod) error { + udpOnlyPorts := sets.NewInt() + for _, ct := range pod.Spec.Containers { + for _, ctPort := range ct.Ports { + portNum := int(ctPort.ContainerPort) + switch ctPort.Protocol { + case corev1.ProtocolUDP: + udpOnlyPorts.Insert(portNum) + case corev1.ProtocolTCP: + udpOnlyPorts.Delete(portNum) + } + } + } + return checkUDPPorts(udpOnlyPorts, ports, pod) +} + // 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 @@ -265,11 +331,19 @@ func (o *PortForwardOptions) Complete(f cmdutil.Factory, cmd *cobra.Command, arg // handle service port mapping to target port if needed switch t := obj.(type) { case *corev1.Service: + err = checkUDPPortInService(args[1:], t) + if err != nil { + return err + } o.Ports, err = translateServicePortToTargetPort(args[1:], *t, *forwardablePod) if err != nil { return err } default: + err = checkUDPPortInPod(args[1:], forwardablePod) + if err != nil { + return err + } o.Ports, err = convertPodNamedPortToNumber(args[1:], *forwardablePod) if err != nil { return err diff --git a/staging/src/k8s.io/kubectl/pkg/cmd/portforward/portforward_test.go b/staging/src/k8s.io/kubectl/pkg/cmd/portforward/portforward_test.go index 0ec6b934f0c..6a04de55768 100644 --- a/staging/src/k8s.io/kubectl/pkg/cmd/portforward/portforward_test.go +++ b/staging/src/k8s.io/kubectl/pkg/cmd/portforward/portforward_test.go @@ -838,3 +838,117 @@ func TestConvertPodNamedPortToNumber(t *testing.T) { } } } + +func TestCheckUDPPort(t *testing.T) { + tests := []struct { + name string + pod *corev1.Pod + service *corev1.Service + ports []string + expectError bool + }{ + { + name: "forward to a UDP port in a Pod", + pod: &corev1.Pod{ + Spec: corev1.PodSpec{ + Containers: []corev1.Container{ + { + Ports: []corev1.ContainerPort{ + {Protocol: corev1.ProtocolUDP, ContainerPort: 53}, + }, + }, + }, + }, + }, + ports: []string{"53"}, + expectError: true, + }, + { + name: "forward to a named UDP port in a Pod", + pod: &corev1.Pod{ + Spec: corev1.PodSpec{ + Containers: []corev1.Container{ + { + Ports: []corev1.ContainerPort{ + {Protocol: corev1.ProtocolUDP, ContainerPort: 53, Name: "dns"}, + }, + }, + }, + }, + }, + ports: []string{"dns"}, + expectError: true, + }, + { + name: "Pod has ports with both TCP and UDP protocol", + pod: &corev1.Pod{ + Spec: corev1.PodSpec{ + Containers: []corev1.Container{ + { + Ports: []corev1.ContainerPort{ + {Protocol: corev1.ProtocolUDP, ContainerPort: 53}, + {Protocol: corev1.ProtocolTCP, ContainerPort: 53}, + }, + }, + }, + }, + }, + ports: []string{":53"}, + }, + + { + name: "forward to a UDP port in a Service", + service: &corev1.Service{ + Spec: corev1.ServiceSpec{ + Ports: []corev1.ServicePort{ + {Protocol: corev1.ProtocolUDP, Port: 53}, + }, + }, + }, + ports: []string{"53"}, + expectError: true, + }, + { + name: "forward to a named UDP port in a Service", + service: &corev1.Service{ + Spec: corev1.ServiceSpec{ + Ports: []corev1.ServicePort{ + {Protocol: corev1.ProtocolUDP, Port: 53, Name: "dns"}, + }, + }, + }, + ports: []string{"10053:dns"}, + expectError: true, + }, + { + name: "Service has ports with both TCP and UDP protocol", + service: &corev1.Service{ + Spec: corev1.ServiceSpec{ + Ports: []corev1.ServicePort{ + {Protocol: corev1.ProtocolUDP, Port: 53}, + {Protocol: corev1.ProtocolTCP, Port: 53}, + }, + }, + }, + ports: []string{"53"}, + }, + } + for _, tc := range tests { + var err error + if tc.pod != nil { + err = checkUDPPortInPod(tc.ports, tc.pod) + } else if tc.service != nil { + err = checkUDPPortInService(tc.ports, tc.service) + } + if err != nil { + if tc.expectError { + continue + } + t.Errorf("%v: unexpected error: %v", tc.name, err) + continue + } + if tc.expectError { + t.Errorf("%v: unexpected success", tc.name) + } + } +}