Merge pull request #3939 from chriswachira/introduce-annotation-to-enable-icmp-rule-for-path-mtu-discovery

feat(NLB): Introduce Service annotation to allow ICMP for Path MTU Discovery

Author: k8s-ci-robot

docs/guide/service/annotations.md

@@ -57,6 +57,7 @@
 | [service.beta.kubernetes.io/aws-load-balancer-enable-prefix-for-ipv6-source-nat](#enable-prefix-for-ipv6-source-nat)         | string                  | off                       | Optional annotation. dualstack lb only. Allowed values - on and off                                                                                                                                                                 |
 | [service.beta.kubernetes.io/aws-load-balancer-source-nat-ipv6-prefixes](#source-nat-ipv6-prefixes)        | stringList                  |                           | Optional annotation. dualstack lb only. This annotation is only applicable when user has to set the service.beta.kubernetes.io/aws-load-balancer-enable-prefix-for-ipv6-source-nat to "on". Length must match the number of subnets |
 | [service.beta.kubernetes.io/aws-load-balancer-minimum-load-balancer-capacity](#load-balancer-capacity-reservation)                 | stringMap                  |                           |
+| [service.beta.kubernetes.io/aws-load-balancer-enable-icmp-for-path-mtu-discovery](#icmp-path-mtu-discovery)        | string                  |                           | If specified, a security group rule is added to the managed security group to allow explicit ICMP traffic for [Path MTU discovery](https://docs.aws.amazon.com/AWSEC2/latest/UserGuide/network_mtu.html#path_mtu_discovery) for IPv4 and dual-stack VPCs. Creates a rule for each source range if `service.beta.kubernetes.io/load-balancer-source-ranges` is present.  |
 
 ## Traffic Routing
 Traffic Routing can be controlled with following annotations:
@@ -192,6 +193,13 @@ on the load balancer.
         service.beta.kubernetes.io/aws-load-balancer-ipv6-addresses: 2600:1f13:837:8501::1, 2600:1f13:837:8504::1
         ```
 
+- <a name="icmp-path-mtu-discovery">`service.beta.kubernetes.io/aws-load-balancer-enable-icmp-for-path-mtu-discovery`</a> enables the creation of security group rules to the managed security group to allow explicit ICMP traffic for [Path MTU discovery](https://docs.aws.amazon.com/AWSEC2/latest/UserGuide/network_mtu.html#path_mtu_discovery) for IPv4 and dual-stack VPCs. Creates a rule for each source range if `service.beta.kubernetes.io/load-balancer-source-ranges` is present.
+
+    !!!example
+        ```
+        service.beta.kubernetes.io/aws-load-balancer-enable-icmp-for-path-mtu-discovery: "on"
+        ```
+
 ## Traffic Listening
 Traffic Listening can be controlled with following annotations:
 

pkg/annotations/constants.go

@@ -100,7 +100,8 @@ const (
 	SvcLBSuffixSecurityGroupPrefixLists                  = "aws-load-balancer-security-group-prefix-lists"
 	SvcLBSuffixlsAttsAnnotationPrefix                    = "aws-load-balancer-listener-attributes"
 	SvcLBSuffixMultiClusterTargetGroup                   = "aws-load-balancer-multi-cluster-target-group"
-	ScvLBSuffixEnablePrefixForIpv6SourceNat              = "aws-load-balancer-enable-prefix-for-ipv6-source-nat"
-	ScvLBSuffixSourceNatIpv6Prefixes                     = "aws-load-balancer-source-nat-ipv6-prefixes"
+	SvcLBSuffixEnablePrefixForIpv6SourceNat              = "aws-load-balancer-enable-prefix-for-ipv6-source-nat"
+	SvcLBSuffixSourceNatIpv6Prefixes                     = "aws-load-balancer-source-nat-ipv6-prefixes"
 	SvcLBSuffixLoadBalancerCapacityReservation           = "aws-load-balancer-minimum-load-balancer-capacity"
+	SvcLBSuffixEnableIcmpForPathMtuDiscovery             = "aws-load-balancer-enable-icmp-for-path-mtu-discovery"
 )

pkg/service/model_build_load_balancer.go

@@ -199,7 +199,7 @@ func (t *defaultModelBuildTask) buildLoadBalancerIPAddressType(_ context.Context
 
 func (t *defaultModelBuildTask) buildLoadBalancerEnablePrefixForIpv6SourceNat(_ context.Context, ipAddressType elbv2model.IPAddressType, ec2Subnets []ec2types.Subnet) (elbv2model.EnablePrefixForIpv6SourceNat, error) {
 	rawEnablePrefixForIpv6SourceNat := ""
-	if exists := t.annotationParser.ParseStringAnnotation(annotations.ScvLBSuffixEnablePrefixForIpv6SourceNat, &rawEnablePrefixForIpv6SourceNat, t.service.Annotations); !exists {
+	if exists := t.annotationParser.ParseStringAnnotation(annotations.SvcLBSuffixEnablePrefixForIpv6SourceNat, &rawEnablePrefixForIpv6SourceNat, t.service.Annotations); !exists {
 		return elbv2model.EnablePrefixForIpv6SourceNatOff, nil
 	}
 
@@ -382,7 +382,7 @@ func (t *defaultModelBuildTask) buildLoadBalancerSubnetMappings(_ context.Contex
 	var isPrefixForIpv6SourceNatEnabled = enablePrefixForIpv6SourceNat == elbv2model.EnablePrefixForIpv6SourceNatOn
 
 	var sourceNatIpv6Prefixes []string
-	sourceNatIpv6PrefixesConfigured := t.annotationParser.ParseStringSliceAnnotation(annotations.ScvLBSuffixSourceNatIpv6Prefixes, &sourceNatIpv6Prefixes, t.service.Annotations)
+	sourceNatIpv6PrefixesConfigured := t.annotationParser.ParseStringSliceAnnotation(annotations.SvcLBSuffixSourceNatIpv6Prefixes, &sourceNatIpv6Prefixes, t.service.Annotations)
 	if sourceNatIpv6PrefixesConfigured {
 		sourceNatIpv6PrefixesError := networking.ValidateSourceNatPrefixes(sourceNatIpv6Prefixes, ipAddressType, isPrefixForIpv6SourceNatEnabled, ec2Subnets)
 		if sourceNatIpv6PrefixesError != nil {

pkg/service/model_build_managed_sg.go

@@ -17,6 +17,15 @@ import (
 )
 
 const (
+	icmpv4Protocol = "icmp"
+	icmpv6Protocol = "icmpv6"
+
+	icmpv4TypeForPathMtu = 3 // https://www.iana.org/assignments/icmp-parameters/icmp-parameters.xhtml#icmp-parameters-codes-3
+	icmpv4CodeForPathMtu = 4
+
+	icmpv6TypeForPathMtu = 2 // https://www.iana.org/assignments/icmpv6-parameters/icmpv6-parameters.xhtml#icmpv6-parameters-codes-2
+	icmpv6CodeForPathMtu = 0
+
 	resourceIDManagedSecurityGroup = "ManagedLBSecurityGroup"
 )
 
@@ -65,7 +74,11 @@ func (t *defaultModelBuildTask) buildManagedSecurityGroupName(_ context.Context)
 func (t *defaultModelBuildTask) buildManagedSecurityGroupIngressPermissions(ctx context.Context, ipAddressType elbv2model.IPAddressType) ([]ec2model.IPPermission, error) {
 	var permissions []ec2model.IPPermission
 	var prefixListIDs []string
+	var icmpForPathMtuConfiguredFlag string
+
+	icmpForPathMtuConfigured := t.annotationParser.ParseStringAnnotation(annotations.SvcLBSuffixEnableIcmpForPathMtuDiscovery, &icmpForPathMtuConfiguredFlag, t.service.Annotations)
 	prefixListsConfigured := t.annotationParser.ParseStringSliceAnnotation(annotations.SvcLBSuffixSecurityGroupPrefixLists, &prefixListIDs, t.service.Annotations)
+
 	cidrs, err := t.buildCIDRsFromSourceRanges(ctx, ipAddressType, prefixListsConfigured)
 	if err != nil {
 		return nil, err
@@ -84,6 +97,18 @@ func (t *defaultModelBuildTask) buildManagedSecurityGroupIngressPermissions(ctx
 						},
 					},
 				})
+				if icmpForPathMtuConfigured && icmpForPathMtuConfiguredFlag == "on" {
+					permissions = append(permissions, ec2model.IPPermission{
+						IPProtocol: string(icmpv4Protocol),
+						FromPort:   awssdk.Int32(icmpv4TypeForPathMtu),
+						ToPort:     awssdk.Int32(icmpv4CodeForPathMtu),
+						IPRanges: []ec2model.IPRange{
+							{
+								CIDRIP: cidr,
+							},
+						},
+					})
+				}
 			} else {
 				permissions = append(permissions, ec2model.IPPermission{
 					IPProtocol: strings.ToLower(string(port.Protocol)),
@@ -95,6 +120,18 @@ func (t *defaultModelBuildTask) buildManagedSecurityGroupIngressPermissions(ctx
 						},
 					},
 				})
+				if icmpForPathMtuConfigured && icmpForPathMtuConfiguredFlag == "on" {
+					permissions = append(permissions, ec2model.IPPermission{
+						IPProtocol: string(icmpv6Protocol),
+						FromPort:   awssdk.Int32(icmpv6TypeForPathMtu),
+						ToPort:     awssdk.Int32(icmpv6CodeForPathMtu),
+						IPv6Range: []ec2model.IPv6Range{
+							{
+								CIDRIPv6: cidr,
+							},
+						},
+					})
+				}
 			}
 		}
 		if prefixListsConfigured {
@@ -112,6 +149,7 @@ func (t *defaultModelBuildTask) buildManagedSecurityGroupIngressPermissions(ctx
 			}
 		}
 	}
+
 	return permissions, nil
 }
 

pkg/service/model_builder_test.go

@@ -2,11 +2,12 @@ package service
 
 import (
 	"context"
-	ec2types "github.com/aws/aws-sdk-go-v2/service/ec2/types"
-	elbv2types "github.com/aws/aws-sdk-go-v2/service/elasticloadbalancingv2/types"
 	"testing"
 	"time"
 
+	ec2types "github.com/aws/aws-sdk-go-v2/service/ec2/types"
+	elbv2types "github.com/aws/aws-sdk-go-v2/service/elasticloadbalancingv2/types"
+
 	awssdk "github.com/aws/aws-sdk-go-v2/aws"
 	"github.com/go-logr/logr"
 	"github.com/golang/mock/gomock"
@@ -6503,6 +6504,7 @@ func Test_defaultModelBuilderTask_Build(t *testing.T) {
              "securityGroupsInboundRulesOnPrivateLink":"on",
              "enablePrefixForIpv6SourceNat": "off",
              "ipAddressType":"ipv4",
+             "enablePrefixForIpv6SourceNat": "off",
              "subnetMapping":[
                 {
                    "subnetID":"subnet-1"