Add support for TCP_UDP to NLB TargetGroups and Listeners

Previously, aws-load-balancer-controller ignored extra overlapping ServicePorts defined in the Kubernetes Service spec if the external port numbers were the same even if the protocols were different (e.g. TCP:53, UDP:53). This behavior prevented users from exposing services that support TCP and UDP on the same external load balancer port number. This patch solves the problem by detecting when a user defines multiple ServicePorts for the same external load balancer port number but using TCP and UDP protocols separately. In such situations, a TCP_UDP TargetGroup and Listener are created and SecurityGroup rules are updated accordingly. If more than two ServicePorts are defined, only the first two mergeable ServicePorts are used. Otherwise, the first ServicePort is used. Note: rebasing errors would be my fault -- Kevin Lyda Signed-off-by: Kevin Lyda <[email protected]>
kubernetes-sigs · Oct 18, 2024 · 17f6eb4 · 17f6eb4
1 parent cf8e9b5
commit 17f6eb4
Show file tree

Hide file tree

Showing 9 changed files with 755 additions and 146 deletions.
diff --git a/.gitignore b/.gitignore
@@ -24,3 +24,5 @@ site
 *.swo
 *~
 *.bak
+
+/gomock_reflect*
diff --git a/apis/elbv2/v1beta1/targetgroupbinding_types.go b/apis/elbv2/v1beta1/targetgroupbinding_types.go
@@ -87,6 +87,9 @@ const (
 
 	// NetworkingProtocolUDP is the UDP protocol.
 	NetworkingProtocolUDP NetworkingProtocol = "UDP"
+
+	// NetworkingProtocolTCP_UDP is the TCP_UDP protocol.
+	NetworkingProtocolTCP_UDP NetworkingProtocol = "TCP_UDP"
 )
 
 // NetworkingPort defines the port and protocol for networking rules.

diff --git a/config/webhook/manifests.yaml b/config/webhook/manifests.yaml
@@ -2,130 +2,130 @@
 apiVersion: admissionregistration.k8s.io/v1
 kind: MutatingWebhookConfiguration
 metadata:
-  name: webhook
+  name: mutating-webhook-configuration
 webhooks:
-  - admissionReviewVersions:
-      - v1beta1
-    clientConfig:
-      service:
-        name: webhook-service
-        namespace: system
-        path: /mutate-v1-pod
-    failurePolicy: Fail
-    name: mpod.elbv2.k8s.aws
-    rules:
-      - apiGroups:
-          - ""
-        apiVersions:
-          - v1
-        operations:
-          - CREATE
-        resources:
-          - pods
-    sideEffects: None
-  - admissionReviewVersions:
-      - v1beta1
-    clientConfig:
-      service:
-        name: webhook-service
-        namespace: system
-        path: /mutate-v1-service
-    failurePolicy: Fail
-    name: mservice.elbv2.k8s.aws
-    rules:
-      - apiGroups:
-          - ""
-        apiVersions:
-          - v1
-        operations:
-          - CREATE
-        resources:
-          - services
-    sideEffects: None
-  - admissionReviewVersions:
-      - v1beta1
-    clientConfig:
-      service:
-        name: webhook-service
-        namespace: system
-        path: /mutate-elbv2-k8s-aws-v1beta1-targetgroupbinding
-    failurePolicy: Fail
-    name: mtargetgroupbinding.elbv2.k8s.aws
-    rules:
-      - apiGroups:
-          - elbv2.k8s.aws
-        apiVersions:
-          - v1beta1
-        operations:
-          - CREATE
-          - UPDATE
-        resources:
-          - targetgroupbindings
-    sideEffects: None
+- admissionReviewVersions:
+  - v1beta1
+  clientConfig:
+    service:
+      name: webhook-service
+      namespace: system
+      path: /mutate-v1-pod
+  failurePolicy: Fail
+  name: mpod.elbv2.k8s.aws
+  rules:
+  - apiGroups:
+    - ""
+    apiVersions:
+    - v1
+    operations:
+    - CREATE
+    resources:
+    - pods
+  sideEffects: None
+- admissionReviewVersions:
+  - v1beta1
+  clientConfig:
+    service:
+      name: webhook-service
+      namespace: system
+      path: /mutate-v1-service
+  failurePolicy: Fail
+  name: mservice.elbv2.k8s.aws
+  rules:
+  - apiGroups:
+    - ""
+    apiVersions:
+    - v1
+    operations:
+    - CREATE
+    resources:
+    - services
+  sideEffects: None
+- admissionReviewVersions:
+  - v1beta1
+  clientConfig:
+    service:
+      name: webhook-service
+      namespace: system
+      path: /mutate-elbv2-k8s-aws-v1beta1-targetgroupbinding
+  failurePolicy: Fail
+  name: mtargetgroupbinding.elbv2.k8s.aws
+  rules:
+  - apiGroups:
+    - elbv2.k8s.aws
+    apiVersions:
+    - v1beta1
+    operations:
+    - CREATE
+    - UPDATE
+    resources:
+    - targetgroupbindings
+  sideEffects: None
 ---
 apiVersion: admissionregistration.k8s.io/v1
 kind: ValidatingWebhookConfiguration
 metadata:
-  name: webhook
+  name: validating-webhook-configuration
 webhooks:
-  - admissionReviewVersions:
-      - v1beta1
-    clientConfig:
-      service:
-        name: webhook-service
-        namespace: system
-        path: /validate-elbv2-k8s-aws-v1beta1-ingressclassparams
-    failurePolicy: Fail
-    name: vingressclassparams.elbv2.k8s.aws
-    rules:
-      - apiGroups:
-          - elbv2.k8s.aws
-        apiVersions:
-          - v1beta1
-        operations:
-          - CREATE
-          - UPDATE
-        resources:
-          - ingressclassparams
-    sideEffects: None
-  - admissionReviewVersions:
-      - v1beta1
-    clientConfig:
-      service:
-        name: webhook-service
-        namespace: system
-        path: /validate-elbv2-k8s-aws-v1beta1-targetgroupbinding
-    failurePolicy: Fail
-    name: vtargetgroupbinding.elbv2.k8s.aws
-    rules:
-      - apiGroups:
-          - elbv2.k8s.aws
-        apiVersions:
-          - v1beta1
-        operations:
-          - CREATE
-          - UPDATE
-        resources:
-          - targetgroupbindings
-    sideEffects: None
-  - admissionReviewVersions:
-      - v1beta1
-    clientConfig:
-      service:
-        name: webhook-service
-        namespace: system
-        path: /validate-networking-v1-ingress
-    failurePolicy: Fail
-    matchPolicy: Equivalent
-    name: vingress.elbv2.k8s.aws
-    rules:
-      - apiGroups:
-          - networking.k8s.io
-        apiVersions:
-          - v1
-        operations:
-          - CREATE
-          - UPDATE
-        resources:
-          - ingresses
-    sideEffects: None
+- admissionReviewVersions:
+  - v1beta1
+  clientConfig:
+    service:
+      name: webhook-service
+      namespace: system
+      path: /validate-elbv2-k8s-aws-v1beta1-ingressclassparams
+  failurePolicy: Fail
+  name: vingressclassparams.elbv2.k8s.aws
+  rules:
+  - apiGroups:
+    - elbv2.k8s.aws
+    apiVersions:
+    - v1beta1
+    operations:
+    - CREATE
+    - UPDATE
+    resources:
+    - ingressclassparams
+  sideEffects: None
+- admissionReviewVersions:
+  - v1beta1
+  clientConfig:
+    service:
+      name: webhook-service
+      namespace: system
+      path: /validate-elbv2-k8s-aws-v1beta1-targetgroupbinding
+  failurePolicy: Fail
+  name: vtargetgroupbinding.elbv2.k8s.aws
+  rules:
+  - apiGroups:
+    - elbv2.k8s.aws
+    apiVersions:
+    - v1beta1
+    operations:
+    - CREATE
+    - UPDATE
+    resources:
+    - targetgroupbindings
+  sideEffects: None
+- admissionReviewVersions:
+  - v1beta1
+  clientConfig:
+    service:
+      name: webhook-service
+      namespace: system
+      path: /validate-networking-v1-ingress
+  failurePolicy: Fail
+  matchPolicy: Equivalent
+  name: vingress.elbv2.k8s.aws
+  rules:
+  - apiGroups:
+    - networking.k8s.io
+    apiVersions:
+    - v1
+    operations:
+    - CREATE
+    - UPDATE
+    resources:
+    - ingresses
+  sideEffects: None
diff --git a/pkg/ingress/model_build_target_group.go b/pkg/ingress/model_build_target_group.go
@@ -5,10 +5,11 @@ import (
 	"crypto/sha256"
 	"encoding/hex"
 	"fmt"
-	awssdk "github.com/aws/aws-sdk-go-v2/aws"
 	"regexp"
 	"strconv"
 
+	awssdk "github.com/aws/aws-sdk-go-v2/aws"
+
 	"github.com/pkg/errors"
 	corev1 "k8s.io/api/core/v1"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
@@ -84,7 +85,7 @@ func (t *defaultModelBuildTask) buildTargetGroupBindingSpec(ctx context.Context,
 	}
 }
 
-func (t *defaultModelBuildTask) buildTargetGroupBindingNetworking(ctx context.Context, targetPort intstr.IntOrString, healthCheckPort intstr.IntOrString) *elbv2model.TargetGroupBindingNetworking {
+func (t *defaultModelBuildTask) buildTargetGroupBindingNetworking(_ context.Context, targetPort intstr.IntOrString, healthCheckPort intstr.IntOrString) *elbv2model.TargetGroupBindingNetworking {
 	if t.backendSGIDToken == nil {
 		return nil
 	}

diff --git a/pkg/service/model_build_listener.go b/pkg/service/model_build_listener.go
@@ -19,16 +19,38 @@ func (t *defaultModelBuildTask) buildListeners(ctx context.Context, scheme elbv2
 		return err
 	}
 
+	// group by listener port number
+	portMap := make(map[int32][]corev1.ServicePort)
 	for _, port := range t.service.Spec.Ports {
-		_, err := t.buildListener(ctx, port, *cfg, scheme)
-		if err != nil {
-			return err
+		key := port.Port
+		if vals, exists := portMap[key]; exists {
+			portMap[key] = append(vals, port)
+		} else {
+			portMap[key] = []corev1.ServicePort{port}
 		}
 	}
+
+	// execute build listener
+	for _, port := range t.service.Spec.Ports {
+		key := port.Port
+		if vals, exists := portMap[key]; exists {
+			if len(vals) > 1 {
+				port = mergeServicePortsForListener(vals)
+			} else {
+				port = vals[0]
+			}
+			_, err := t.buildListener(ctx, port, cfg, scheme)
+			if err != nil {
+				return err
+			}
+			delete(portMap, key)
+		}
+	}
+
 	return nil
 }
 
-func (t *defaultModelBuildTask) buildListener(ctx context.Context, port corev1.ServicePort, cfg listenerConfig,
+func (t *defaultModelBuildTask) buildListener(ctx context.Context, port corev1.ServicePort, cfg *listenerConfig,
 	scheme elbv2model.LoadBalancerScheme) (*elbv2model.Listener, error) {
 	lsSpec, err := t.buildListenerSpec(ctx, port, cfg, scheme)
 	if err != nil {
@@ -39,7 +61,7 @@ func (t *defaultModelBuildTask) buildListener(ctx context.Context, port corev1.S
 	return ls, nil
 }
 
-func (t *defaultModelBuildTask) buildListenerSpec(ctx context.Context, port corev1.ServicePort, cfg listenerConfig,
+func (t *defaultModelBuildTask) buildListenerSpec(ctx context.Context, port corev1.ServicePort, cfg *listenerConfig,
 	scheme elbv2model.LoadBalancerScheme) (elbv2model.ListenerSpec, error) {
 	tgProtocol := elbv2model.Protocol(port.Protocol)
 	listenerProtocol := elbv2model.Protocol(port.Protocol)
@@ -149,7 +171,7 @@ func validateTLSPortsSet(rawTLSPorts []string, ports []corev1.ServicePort) error
 	return nil
 }
 
-func (t *defaultModelBuildTask) buildTLSPortsSet(_ context.Context) (sets.String, error) {
+func (t *defaultModelBuildTask) buildTLSPortsSet(_ context.Context) (sets.Set[string], error) {
 	var rawTLSPorts []string
 
 	_ = t.annotationParser.ParseStringSliceAnnotation(annotations.SvcLBSuffixSSLPorts, &rawTLSPorts, t.service.Annotations)
@@ -160,7 +182,7 @@ func (t *defaultModelBuildTask) buildTLSPortsSet(_ context.Context) (sets.String
 		return nil, err
 	}
 
-	return sets.NewString(rawTLSPorts...), nil
+	return sets.New[string](rawTLSPorts...), nil
 }
 
 func (t *defaultModelBuildTask) buildBackendProtocol(_ context.Context) string {
@@ -191,7 +213,7 @@ func (t *defaultModelBuildTask) buildListenerALPNPolicy(ctx context.Context, lis
 
 type listenerConfig struct {
 	certificates    []elbv2model.Certificate
-	tlsPortsSet     sets.String
+	tlsPortsSet     sets.Set[string]
 	sslPolicy       *string
 	backendProtocol string
 }
@@ -234,3 +256,24 @@ func (t *defaultModelBuildTask) buildListenerAttributes(ctx context.Context, svc
 	}
 	return attributes, nil
 }
+
+func mergeServicePortsForListener(ports []corev1.ServicePort) corev1.ServicePort {
+	port0 := ports[0]
+	mergeableProtocols := map[corev1.Protocol]bool{
+		corev1.ProtocolTCP: true,
+		corev1.ProtocolUDP: true,
+	}
+	if _, ok := mergeableProtocols[port0.Protocol]; !ok {
+		return port0
+	}
+	for _, port := range ports[1:] {
+		if _, ok := mergeableProtocols[port.Protocol]; !ok {
+			continue
+		}
+		if port.NodePort == port0.NodePort && port.Protocol != port0.Protocol {
+			port0.Protocol = corev1.Protocol("TCP_UDP")
+			break
+		}
+	}
+	return port0
+}