From 69b6bb91bcd733d30fb797ae3942f58c01214994 Mon Sep 17 00:00:00 2001 From: Nolan Brubaker Date: Wed, 18 Mar 2026 14:36:03 -0400 Subject: [PATCH 1/7] Add support for dual networking stack services - Create load balancers according to the Kubernetes Service API Signed-off-by: Nolan Brubaker Assisted-by: Claude Sonnet 4.5 --- docs/service_controller.md | 64 ++++++- pkg/providers/v1/aws.go | 96 +++++++++-- pkg/providers/v1/aws_loadbalancer.go | 130 ++++++++++++-- pkg/providers/v1/aws_loadbalancer_test.go | 199 ++++++++++++++++++++++ pkg/providers/v1/aws_test.go | 187 +++++++++++++++++++- pkg/providers/v1/aws_validations.go | 53 ++++++ pkg/providers/v1/aws_validations_test.go | 91 ++++++++++ 7 files changed, 787 insertions(+), 33 deletions(-) diff --git a/docs/service_controller.md b/docs/service_controller.md index e9cd74b205..fb244ba98a 100644 --- a/docs/service_controller.md +++ b/docs/service_controller.md @@ -85,4 +85,66 @@ metadata: service.beta.kubernetes.io/aws-load-balancer-internal: true service.beta.kubernetes.io/aws-load-balancer-target-group-attributes: preserve_client_ip.enabled=false,proxy_protocol_v2.enabled=true [...] -``` \ No newline at end of file +``` + +## Dual stack services with IPv6 + +Services can be created using solely IPv4 networking (the default), or with dual stack support per the [Kubernetes Service specification](https://kubernetes.io/docs/concepts/services-networking/dual-stack/). +The service must be created with a Network Load Balancer, and the Kubernetes control plane must be configured to support IPv6 CIDRs. + +Note: When using the [AWS Load Balancer Controller](https://kubernetes-sigs.github.io/aws-load-balancer-controller/latest/), Services will default to having the `spec.loadBalancerClass` field populated via a MutatingWebhookConfiguration. +This webhook must be disabled to allow the cloud controller manager to handle services. + +Some limitations to be aware of when using dual stack load balancers: + +- The `spec.ipFamilies` field can have a second family added or removed, but the first entry is immutable after Service creation. +- Load balanced targets are registered based on the instances, not their IP addresses. +- A Service cannot be IPv6 only; it must either be IPv4 or dual stack, even if IPv6 is the only IP family specified. + +### Usage Example 1 - creating a dual stack load balancer, requiring both stacks + +```yaml +apiVersion: v1 +kind: Service +metadata: + name: $SVC_NAME + namespace: ${APP_NAMESPACE} + annotations: + service.beta.kubernetes.io/aws-load-balancer-type: nlb +spec: + type: LoadBalancer + ipFamilies: + - IPv6 + - IPv4 + ipFamilyPolicy: RequireDualStack # Require both stacks are present on the service. + selector: + app: myapp + ports: + - port: 80 + targetPort: 8080 + protocol: TCP +``` + +### Usage Example 2 - creating a dual stack load balancer, falling back to IPv4 + +```yaml +apiVersion: v1 +kind: Service +metadata: + name: $SVC_NAME + namespace: ${APP_NAMESPACE} + annotations: + service.beta.kubernetes.io/aws-load-balancer-type: nlb +spec: + type: LoadBalancer + ipFamilies: + - IPv4 + - IPv6 + ipFamilyPolicy: PreferDualStack # If dual stack is not configured or present, fall back to IPv4. + selector: + app: myapp + ports: + - port: 80 + targetPort: 8080 + protocol: TCP +``` diff --git a/pkg/providers/v1/aws.go b/pkg/providers/v1/aws.go index 89b1d742da..0c2d856aa9 100644 --- a/pkg/providers/v1/aws.go +++ b/pkg/providers/v1/aws.go @@ -2001,10 +2001,11 @@ func (c *Cloud) buildELBSecurityGroupList(ctx context.Context, serviceName types // - sgID: The ID of the security group to configure. // - rules: An existing permission set of rules to be added to the security group. // - ec2SourceRanges: A slice of *ec2.IpRange objects specifying the source IP ranges for the rules. +// - ec2Ipv6SourceRanges: A slice of *ec2.Ipv6Range objects specifying the source IPv6 ranges for the rules. // // Returns: // - error: An error if any issue occurs while creating or applying the security group rules. -func (c *Cloud) createSecurityGroupRules(ctx context.Context, sgID string, rules IPPermissionSet, ec2SourceRanges []ec2types.IpRange) error { +func (c *Cloud) createSecurityGroupRules(ctx context.Context, sgID string, rules IPPermissionSet, ec2SourceRanges []ec2types.IpRange, ec2Ipv6SourceRanges []ec2types.Ipv6Range) error { if len(sgID) == 0 { return fmt.Errorf("security group ID cannot be empty") } @@ -2014,6 +2015,7 @@ func (c *Cloud) createSecurityGroupRules(ctx context.Context, sgID string, rules FromPort: aws.Int32(3), ToPort: aws.Int32(4), IpRanges: ec2SourceRanges, + Ipv6Ranges: ec2Ipv6SourceRanges, } rules.Insert(permission) @@ -2116,7 +2118,16 @@ func (c *Cloud) getSubnetCidrs(ctx context.Context, subnetIDs []string) ([]strin cidrs := make([]string, 0, len(subnets)) for _, subnet := range subnets { + // Add IPv4 CIDR cidrs = append(cidrs, aws.ToString(subnet.CidrBlock)) + + // Add IPv6 CIDRs if present + for _, ipv6Association := range subnet.Ipv6CidrBlockAssociationSet { + if ipv6Association.Ipv6CidrBlockState != nil && + ipv6Association.Ipv6CidrBlockState.State == ec2types.SubnetCidrBlockStateCodeAssociated { + cidrs = append(cidrs, aws.ToString(ipv6Association.Ipv6CidrBlock)) + } + } } return cidrs, nil } @@ -2278,6 +2289,32 @@ func (c *Cloud) ensureNLBSecurityGroup(ctx context.Context, loadBalancerName, cl return []string{securityGroupID}, nil } +// separateIPv4AndIPv6CIDRs separates a list of CIDR strings into IPv4 and IPv6 ranges +// Returns EC2 IpRange and Ipv6Range slices for use in security group rules +func separateIPv4AndIPv6CIDRs(cidrs []string) ([]ec2types.IpRange, []ec2types.Ipv6Range) { + var ipv4Ranges []ec2types.IpRange + var ipv6Ranges []ec2types.Ipv6Range + + for _, cidr := range cidrs { + _, ipNet, err := net.ParseCIDR(cidr) + if err != nil { + klog.Warningf("Failed to parse CIDR %q: %v", cidr, err) + continue + } + + // Check if this is an IPv4 or IPv6 CIDR + if ipNet.IP.To4() != nil { + // IPv4 + ipv4Ranges = append(ipv4Ranges, ec2types.IpRange{CidrIp: aws.String(cidr)}) + } else { + // IPv6 + ipv6Ranges = append(ipv6Ranges, ec2types.Ipv6Range{CidrIpv6: aws.String(cidr)}) + } + } + + return ipv4Ranges, ipv6Ranges +} + // ensureNLBSecurityGroupRules ensures the NLB frontend security group rules are created and configured // for the specified security groups based on the load balancer port mappings (Load Balancer listeners), // allowing traffic from the specified source ranges. @@ -2285,27 +2322,41 @@ func (c *Cloud) ensureNLBSecurityGroup(ctx context.Context, loadBalancerName, cl // Parameters: // - ctx: The context for the request. // - securityGroups: The security group IDs to configure rules for (only first SG is used). -// - ec2SourceRanges: The CIDR ranges allowed to access the load balancer. +// - sourceCIDRs: The CIDR ranges (IPv4 and/or IPv6) allowed to access the load balancer. // - v2Mappings: The NLB port mappings defining frontend ports and protocols. // // Returns: // - error: An error if any issue occurs while ensuring the NLB security group rules. -func (c *Cloud) ensureNLBSecurityGroupRules(ctx context.Context, securityGroups []string, ec2SourceRanges []ec2types.IpRange, v2Mappings []nlbPortMapping) error { +func (c *Cloud) ensureNLBSecurityGroupRules(ctx context.Context, securityGroups []string, sourceCIDRs []string, v2Mappings []nlbPortMapping) error { if len(securityGroups) == 0 { return nil } securityGroupID := securityGroups[0] + // Separate source CIDRs into IPv4 and IPv6 ranges + ec2SourceRanges, ec2Ipv6SourceRanges := separateIPv4AndIPv6CIDRs(sourceCIDRs) + ingressRules := NewIPPermissionSet() for _, mapping := range v2Mappings { - ingressRules.Insert(ec2types.IpPermission{ + permission := ec2types.IpPermission{ FromPort: aws.Int32(int32(mapping.FrontendPort)), ToPort: aws.Int32(int32(mapping.FrontendPort)), IpProtocol: aws.String(strings.ToLower(string((mapping.FrontendProtocol)))), - IpRanges: ec2SourceRanges, - }) + } + + // Add IPv4 ranges if present + if len(ec2SourceRanges) > 0 { + permission.IpRanges = ec2SourceRanges + } + + // Add IPv6 ranges if present + if len(ec2Ipv6SourceRanges) > 0 { + permission.Ipv6Ranges = ec2Ipv6SourceRanges + } + + ingressRules.Insert(permission) } - if err := c.createSecurityGroupRules(ctx, securityGroupID, ingressRules, ec2SourceRanges); err != nil { + if err := c.createSecurityGroupRules(ctx, securityGroupID, ingressRules, ec2SourceRanges, ec2Ipv6SourceRanges); err != nil { return fmt.Errorf("error while updating rules to security group %q: %w", securityGroupID, err) } return nil @@ -2328,6 +2379,10 @@ func (c *Cloud) EnsureLoadBalancer(ctx context.Context, clusterName string, apiS return nil, err } + if !isNLB(annotations) && serviceRequestsIPv6(apiService) { + return nil, fmt.Errorf("classic load balancer for service %s does not support IPv6", apiService.Name) + } + if apiService.Spec.SessionAffinity != v1.ServiceAffinityNone { // ELB supports sticky sessions, but only when configured for HTTP/HTTPS return nil, fmt.Errorf("unsupported load balancer affinity: %v", apiService.Spec.SessionAffinity) @@ -2348,9 +2403,18 @@ func (c *Cloud) EnsureLoadBalancer(ctx context.Context, clusterName string, apiS if err != nil { return nil, err } - ec2SourceRanges := []ec2types.IpRange{} - for _, srcRange := range sourceRanges.StringSlice() { - ec2SourceRanges = append(ec2SourceRanges, ec2types.IpRange{CidrIp: aws.String(srcRange)}) + + sourceCIDRs := sourceRanges.StringSlice() + + // If no source ranges specified, add defaults based on service IP families + // This should be populated by GetLoadBalancerSourceRanges most of the time. + if len(sourceCIDRs) == 0 { + sourceCIDRs = append(sourceCIDRs, "0.0.0.0/0") + } + + // Add IPv6 default range if service supports IPv6. + if serviceRequestsIPv6(apiService) && !contains(sourceCIDRs, "::/0") { + sourceCIDRs = append(sourceCIDRs, "::/0") } sslPorts := getPortSets(annotations[ServiceAnnotationLoadBalancerSSLPorts]) @@ -2452,13 +2516,14 @@ func (c *Cloud) EnsureLoadBalancer(ctx context.Context, clusterName string, apiS internalELB, annotations, securityGroups, + apiService, ) if err != nil { return nil, err } // Ensure SG rules only if the LB reconciliator finished successfully. - if err := c.ensureNLBSecurityGroupRules(ctx, securityGroups, ec2SourceRanges, v2Mappings); err != nil { + if err := c.ensureNLBSecurityGroupRules(ctx, securityGroups, sourceCIDRs, v2Mappings); err != nil { return nil, fmt.Errorf("error ensuring NLB security group rules: %w", err) } @@ -2483,6 +2548,10 @@ func (c *Cloud) EnsureLoadBalancer(ctx context.Context, clusterName string, apiS } if len(sourceRangeCidrs) == 0 { sourceRangeCidrs = append(sourceRangeCidrs, "0.0.0.0/0") + // Add IPv6 default range if service supports IPv6 + } + if serviceRequestsIPv6(apiService) && !contains(sourceRangeCidrs, "::/0") { + sourceRangeCidrs = append(sourceRangeCidrs, "::/0") } err = c.updateInstanceSecurityGroupsForNLB(ctx, loadBalancerName, instances, subnetCidrs, sourceRangeCidrs, v2Mappings) @@ -2629,6 +2698,9 @@ func (c *Cloud) EnsureLoadBalancer(ctx context.Context, clusterName string, apiS } if setupSg { + // Separate source CIDRs into IPv4 and IPv6 ranges for classic ELB + ec2SourceRanges, ec2Ipv6SourceRanges := separateIPv4AndIPv6CIDRs(sourceCIDRs) + permissions := NewIPPermissionSet() for _, port := range apiService.Spec.Ports { protocol := strings.ToLower(string(port.Protocol)) @@ -2642,7 +2714,7 @@ func (c *Cloud) EnsureLoadBalancer(ctx context.Context, clusterName string, apiS permissions.Insert(permission) } - if err = c.createSecurityGroupRules(ctx, securityGroupIDs[0], permissions, ec2SourceRanges); err != nil { + if err = c.createSecurityGroupRules(ctx, securityGroupIDs[0], permissions, ec2SourceRanges, ec2Ipv6SourceRanges); err != nil { return nil, err } } diff --git a/pkg/providers/v1/aws_loadbalancer.go b/pkg/providers/v1/aws_loadbalancer.go index 62f68690ad..ae8eaa04bf 100644 --- a/pkg/providers/v1/aws_loadbalancer.go +++ b/pkg/providers/v1/aws_loadbalancer.go @@ -22,8 +22,10 @@ import ( "encoding/hex" "errors" "fmt" + "net/netip" "reflect" "regexp" + "slices" "strconv" "strings" "time" @@ -94,6 +96,40 @@ func isLBExternal(annotations map[string]string) bool { return false } +// getTargetGroupIPAddressTypeFromService determines the IP address type for the target group +// based on the Service's spec.ipFamilies field. According to Kubernetes dual-stack documentation: +// The target group will match the family of the first entry in spec.ipFamilies, which is defaulted by +// the Kube API server. +func getTargetGroupIPAddressTypeFromService(service *v1.Service) elbv2types.TargetGroupIpAddressTypeEnum { + if service != nil && len(service.Spec.IPFamilies) > 0 { + if service.Spec.IPFamilies[0] == v1.IPv6Protocol { + return elbv2types.TargetGroupIpAddressTypeEnumIpv6 + } + } + // Default to IPv4 + return elbv2types.TargetGroupIpAddressTypeEnumIpv4 +} + +// getLoadBalancerIPAddressTypeFromService determines the IP address type of the load balancer. +// This will either be IPv4 (the default), or dual stack. +// If nil is passed, IPv4 will be returned. +func getLoadBalancerIPAddressTypeFromService(service *v1.Service) elbv2types.IpAddressType { + if serviceRequestsIPv6(service) { + return elbv2types.IpAddressTypeDualstack + } + // Default to single stack, IPv4 + return elbv2types.IpAddressTypeIpv4 +} + +// serviceRequestsIPv6 checks if the Service has IPv6 configured in its ipFamilies +func serviceRequestsIPv6(service *v1.Service) bool { + if service == nil || len(service.Spec.IPFamilies) == 0 { + return false + } + + return slices.Contains(service.Spec.IPFamilies, v1.IPv6Protocol) +} + type healthCheckConfig struct { Port string Path string @@ -145,7 +181,7 @@ func getKeyValuePropertiesFromAnnotation(annotations map[string]string, annotati } // ensureLoadBalancerv2 ensures a v2 load balancer is created -func (c *Cloud) ensureLoadBalancerv2(ctx context.Context, namespacedName types.NamespacedName, loadBalancerName string, mappings []nlbPortMapping, instanceIDs, discoveredSubnetIDs []string, internalELB bool, annotations map[string]string, securityGroups []string) (*elbv2types.LoadBalancer, error) { +func (c *Cloud) ensureLoadBalancerv2(ctx context.Context, namespacedName types.NamespacedName, loadBalancerName string, mappings []nlbPortMapping, instanceIDs, discoveredSubnetIDs []string, internalELB bool, annotations map[string]string, securityGroups []string, service *v1.Service) (*elbv2types.LoadBalancer, error) { loadBalancer, err := c.describeLoadBalancerv2(ctx, loadBalancerName) if err != nil { return nil, err @@ -159,11 +195,22 @@ func (c *Cloud) ensureLoadBalancerv2(ctx context.Context, namespacedName types.N tags[TagNameKubernetesService] = namespacedName.String() tags = c.tagging.buildTags(ResourceLifecycleOwned, tags) + // Determine target group IP address type based on Service spec.ipFamilies + targetGroupIPAddressType := getTargetGroupIPAddressTypeFromService(service) + + ipv6Requested := serviceRequestsIPv6(service) + + // Validate that single stack IPv6 is not being used (not supported on NLB) + if err := validateIPFamilyInfo(service, ipv6Requested); err != nil { + return nil, err + } + if loadBalancer == nil { // Create the LB createRequest := &elbv2.CreateLoadBalancerInput{ - Type: elbv2types.LoadBalancerTypeEnumNetwork, - Name: aws.String(loadBalancerName), + Type: elbv2types.LoadBalancerTypeEnumNetwork, + Name: aws.String(loadBalancerName), + IpAddressType: getLoadBalancerIPAddressTypeFromService(service), } if internalELB { createRequest.Scheme = elbv2types.LoadBalancerSchemeEnumInternal @@ -208,7 +255,7 @@ func (c *Cloud) ensureLoadBalancerv2(ctx context.Context, namespacedName types.N for i := range mappings { // It is easier to keep track of updates by having possibly // duplicate target groups where the backend port is the same - _, err := c.createListenerV2(ctx, createResponse.LoadBalancers[0].LoadBalancerArn, mappings[i], namespacedName, instanceIDs, *createResponse.LoadBalancers[0].VpcId, tags) + _, err := c.createListenerV2(ctx, createResponse.LoadBalancers[0].LoadBalancerArn, mappings[i], namespacedName, instanceIDs, *createResponse.LoadBalancers[0].VpcId, tags, targetGroupIPAddressType) if err != nil { return nil, fmt.Errorf("error creating listener: %q", err) } @@ -286,8 +333,9 @@ func (c *Cloud) ensureLoadBalancerv2(ctx context.Context, namespacedName types.N } } - // recreate targetGroup if trafficPort, protocol or HealthCheckProtocol changed + // recreate targetGroup if trafficPort, protocol, HealthCheckProtocol, or IpAddressType changed healthCheckModified := false + ipAddressTypeChanged := false targetGroupRecreated := false targetGroup, ok := nodePortTargetGroup[nodePort] @@ -296,7 +344,14 @@ func (c *Cloud) ensureLoadBalancerv2(ctx context.Context, namespacedName types.N healthCheckModified = true } - if !ok || targetGroup.Protocol != mapping.TrafficProtocol || healthCheckModified { + // Check if IP address type has changed (requires target group recreation) + if targetGroup != nil && targetGroup.IpAddressType != targetGroupIPAddressType { + klog.Infof("Target group IP address type changed from %s to %s for %v, will recreate target group", + targetGroup.IpAddressType, targetGroupIPAddressType, namespacedName) + ipAddressTypeChanged = true + } + + if !ok || targetGroup.Protocol != mapping.TrafficProtocol || healthCheckModified || ipAddressTypeChanged { // create new target group targetGroup, err = c.ensureTargetGroup(ctx, nil, @@ -305,6 +360,7 @@ func (c *Cloud) ensureLoadBalancerv2(ctx context.Context, namespacedName types.N instanceIDs, *loadBalancer.VpcId, tags, + targetGroupIPAddressType, ) if err != nil { return nil, err @@ -354,6 +410,7 @@ func (c *Cloud) ensureLoadBalancerv2(ctx context.Context, namespacedName types.N instanceIDs, *loadBalancer.VpcId, tags, + targetGroupIPAddressType, ) if err != nil { return nil, err @@ -364,7 +421,7 @@ func (c *Cloud) ensureLoadBalancerv2(ctx context.Context, namespacedName types.N } // Additions - _, err := c.createListenerV2(ctx, loadBalancer.LoadBalancerArn, mapping, namespacedName, instanceIDs, *loadBalancer.VpcId, tags) + _, err := c.createListenerV2(ctx, loadBalancer.LoadBalancerArn, mapping, namespacedName, instanceIDs, *loadBalancer.VpcId, tags, targetGroupIPAddressType) if err != nil { return nil, err } @@ -685,7 +742,7 @@ func (c *Cloud) buildTargetGroupName(serviceName types.NamespacedName, servicePo return fmt.Sprintf("k8s-%.8s-%.8s-%.10s", sanitizedNamespace, sanitizedServiceName, tgUUID) } -func (c *Cloud) createListenerV2(ctx context.Context, loadBalancerArn *string, mapping nlbPortMapping, namespacedName types.NamespacedName, instanceIDs []string, vpcID string, tags map[string]string) (listener *elbv2types.Listener, err error) { +func (c *Cloud) createListenerV2(ctx context.Context, loadBalancerArn *string, mapping nlbPortMapping, namespacedName types.NamespacedName, instanceIDs []string, vpcID string, tags map[string]string, ipAddressType elbv2types.TargetGroupIpAddressTypeEnum) (listener *elbv2types.Listener, err error) { target, err := c.ensureTargetGroup(ctx, nil, namespacedName, @@ -693,6 +750,7 @@ func (c *Cloud) createListenerV2(ctx context.Context, loadBalancerArn *string, m instanceIDs, vpcID, tags, + ipAddressType, ) if err != nil { return nil, err @@ -749,19 +807,20 @@ func (c *Cloud) deleteListenerV2(ctx context.Context, listener *elbv2types.Liste } // ensureTargetGroup creates a target group with a set of instances. -func (c *Cloud) ensureTargetGroup(ctx context.Context, targetGroup *elbv2types.TargetGroup, serviceName types.NamespacedName, mapping nlbPortMapping, instances []string, vpcID string, tags map[string]string) (*elbv2types.TargetGroup, error) { +func (c *Cloud) ensureTargetGroup(ctx context.Context, targetGroup *elbv2types.TargetGroup, serviceName types.NamespacedName, mapping nlbPortMapping, instances []string, vpcID string, tags map[string]string, ipAddressType elbv2types.TargetGroupIpAddressTypeEnum) (*elbv2types.TargetGroup, error) { dirty := false expectedTargets := c.computeTargetGroupExpectedTargets(instances, mapping.TrafficPort) if targetGroup == nil { targetType := elbv2types.TargetTypeEnumInstance name := c.buildTargetGroupName(serviceName, mapping.FrontendPort, mapping.TrafficPort, mapping.TrafficProtocol, targetType, mapping) - klog.Infof("Creating load balancer target group for %v with name: %s", serviceName, name) + klog.Infof("Creating load balancer target group for %v with name: %s (IP address type: %s)", serviceName, name, ipAddressType) input := &elbv2.CreateTargetGroupInput{ VpcId: aws.String(vpcID), Name: aws.String(name), Port: aws.Int32(mapping.TrafficPort), Protocol: mapping.TrafficProtocol, TargetType: targetType, + IpAddressType: ipAddressType, HealthCheckIntervalSeconds: aws.Int32(mapping.HealthCheckConfig.Interval), HealthCheckPort: aws.String(mapping.HealthCheckConfig.Port), HealthCheckProtocol: mapping.HealthCheckConfig.Protocol, @@ -1045,23 +1104,49 @@ func (c *Cloud) updateInstanceSecurityGroupsForNLB(ctx context.Context, lbName s return nil } +// isIPv6CIDR returns true if the given CIDR is an IPv6 CIDR. +// It uses netip.ParsePrefix to properly parse and validate the CIDR notation. +func isIPv6CIDR(cidr string) bool { + prefix, err := netip.ParsePrefix(cidr) + if err != nil { + // If parsing fails, fall back to simple string check for backward compatibility + // This shouldn't happen with valid AWS CIDR blocks, but we handle it gracefully + klog.Warningf("Failed to parse CIDR %s: %v, falling back to string-based detection", cidr, err) + return strings.Contains(cidr, ":") + } + return prefix.Addr().Is6() +} + // updateInstanceSecurityGroupForNLBTraffic will manage permissions set(identified by ruleDesc) on securityGroup to match desired set(allow protocol traffic from ports/cidr). // Note: sgPerms will be updated to reflect the current permission set on SG after update. func (c *Cloud) updateInstanceSecurityGroupForNLBTraffic(ctx context.Context, sgID string, sgPerms IPPermissionSet, ruleDesc string, protocol string, ports sets.Set[int32], cidrs []string) error { desiredPerms := NewIPPermissionSet() for port := range ports { for _, cidr := range cidrs { - desiredPerms.Insert(ec2types.IpPermission{ + perm := ec2types.IpPermission{ IpProtocol: aws.String(protocol), FromPort: aws.Int32(int32(port)), ToPort: aws.Int32(int32(port)), - IpRanges: []ec2types.IpRange{ + } + + // Use Ipv6Ranges for IPv6 CIDRs, IpRanges for IPv4 CIDRs + if isIPv6CIDR(cidr) { + perm.Ipv6Ranges = []ec2types.Ipv6Range{ + { + CidrIpv6: aws.String(cidr), + Description: aws.String(ruleDesc), + }, + } + } else { + perm.IpRanges = []ec2types.IpRange{ { CidrIp: aws.String(cidr), Description: aws.String(ruleDesc), }, - }, - }) + } + } + + desiredPerms.Insert(perm) } } @@ -1099,6 +1184,7 @@ func (c *Cloud) updateInstanceSecurityGroupForNLBTraffic(ctx context.Context, sg func (c *Cloud) updateInstanceSecurityGroupForNLBMTU(ctx context.Context, sgID string, sgPerms IPPermissionSet) error { desiredPerms := NewIPPermissionSet() for _, perm := range sgPerms { + // Handle IPv4 ranges for _, ipRange := range perm.IpRanges { if strings.Contains(aws.ToString(ipRange.Description), NLBClientRuleDescription) { desiredPerms.Insert(ec2types.IpPermission{ @@ -1114,6 +1200,22 @@ func (c *Cloud) updateInstanceSecurityGroupForNLBMTU(ctx context.Context, sgID s }) } } + // Handle IPv6 ranges + for _, ipv6Range := range perm.Ipv6Ranges { + if strings.Contains(aws.ToString(ipv6Range.Description), NLBClientRuleDescription) { + desiredPerms.Insert(ec2types.IpPermission{ + IpProtocol: aws.String("icmpv6"), + FromPort: aws.Int32(2), + ToPort: aws.Int32(-1), + Ipv6Ranges: []ec2types.Ipv6Range{ + { + CidrIpv6: ipv6Range.CidrIpv6, + Description: aws.String(NLBMtuDiscoveryRuleDescription), + }, + }, + }) + } + } } permsToGrant := desiredPerms.Difference(sgPerms) diff --git a/pkg/providers/v1/aws_loadbalancer_test.go b/pkg/providers/v1/aws_loadbalancer_test.go index 1b19e93d33..a18bf2d3ad 100644 --- a/pkg/providers/v1/aws_loadbalancer_test.go +++ b/pkg/providers/v1/aws_loadbalancer_test.go @@ -1951,6 +1951,7 @@ func TestCloud_ensureTargetGroupTargets(t *testing.T) { tests := []struct { name string + service *v1.Service maxTargets int expectedTargets []*elbv2types.TargetDescription actualTargets []*elbv2types.TargetDescription @@ -2075,3 +2076,201 @@ func TestCloud_ensureTargetGroupTargets(t *testing.T) { }) } } + +func TestGetTargetGroupIPAddressTypeFromService(t *testing.T) { + tests := []struct { + name string + service *v1.Service + expected elbv2types.TargetGroupIpAddressTypeEnum + }{ + { + name: "IPv6 as first IP family", + service: &v1.Service{ + Spec: v1.ServiceSpec{ + IPFamilies: []v1.IPFamily{v1.IPv6Protocol, v1.IPv4Protocol}, + }, + }, + expected: elbv2types.TargetGroupIpAddressTypeEnumIpv6, + }, + { + name: "IPv6 as only IP family", + service: &v1.Service{ + Spec: v1.ServiceSpec{ + IPFamilies: []v1.IPFamily{v1.IPv6Protocol}, + }, + }, + expected: elbv2types.TargetGroupIpAddressTypeEnumIpv6, + }, + { + name: "IPv4 as first IP family", + service: &v1.Service{ + Spec: v1.ServiceSpec{ + IPFamilies: []v1.IPFamily{v1.IPv4Protocol, v1.IPv6Protocol}, + }, + }, + expected: elbv2types.TargetGroupIpAddressTypeEnumIpv4, + }, + { + name: "IPv4 as only IP family", + service: &v1.Service{ + Spec: v1.ServiceSpec{ + IPFamilies: []v1.IPFamily{v1.IPv4Protocol}, + }, + }, + expected: elbv2types.TargetGroupIpAddressTypeEnumIpv4, + }, + { + name: "No IP families specified (defaults to IPv4)", + service: &v1.Service{ + Spec: v1.ServiceSpec{ + IPFamilies: []v1.IPFamily{}, + }, + }, + expected: elbv2types.TargetGroupIpAddressTypeEnumIpv4, + }, + { + name: "Nil service (defaults to IPv4)", + service: nil, + expected: elbv2types.TargetGroupIpAddressTypeEnumIpv4, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + result := getTargetGroupIPAddressTypeFromService(tt.service) + assert.Equal(t, tt.expected, result, "IP address type should match expected for test case: %s", tt.name) + }) + } +} + +func TestServiceRequestsIPv6(t *testing.T) { + tests := []struct { + name string + service *v1.Service + expected bool + }{ + { + name: "IPv6 only", + service: &v1.Service{ + Spec: v1.ServiceSpec{ + IPFamilies: []v1.IPFamily{v1.IPv6Protocol}, + }, + }, + expected: true, + }, + { + name: "Dual-stack IPv4 first", + service: &v1.Service{ + Spec: v1.ServiceSpec{ + IPFamilies: []v1.IPFamily{v1.IPv4Protocol, v1.IPv6Protocol}, + }, + }, + expected: true, + }, + { + name: "Dual-stack IPv6 first", + service: &v1.Service{ + Spec: v1.ServiceSpec{ + IPFamilies: []v1.IPFamily{v1.IPv6Protocol, v1.IPv4Protocol}, + }, + }, + expected: true, + }, + { + name: "IPv4 only", + service: &v1.Service{ + Spec: v1.ServiceSpec{ + IPFamilies: []v1.IPFamily{v1.IPv4Protocol}, + }, + }, + expected: false, + }, + { + name: "No IP families", + service: &v1.Service{ + Spec: v1.ServiceSpec{ + IPFamilies: []v1.IPFamily{}, + }, + }, + expected: false, + }, + { + name: "Nil service", + service: nil, + expected: false, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + result := serviceRequestsIPv6(tt.service) + assert.Equal(t, tt.expected, result, "IPv6 support detection should match expected for test case: %s", tt.name) + }) + } +} + +func TestGetLoadBalancerIpAddressType(t *testing.T) { + tests := []struct { + name string + service *v1.Service + expected elbv2types.IpAddressType + }{ + { + name: "No IP families defined", + service: &v1.Service{ + Spec: v1.ServiceSpec{ + IPFamilies: []v1.IPFamily{}, + }, + }, + expected: elbv2types.IpAddressTypeIpv4, + }, + { + name: "Only IPv4 defined", + service: &v1.Service{ + Spec: v1.ServiceSpec{ + IPFamilies: []v1.IPFamily{v1.IPv4Protocol}, + }, + }, + expected: elbv2types.IpAddressTypeIpv4, + }, + { + name: "Only IPv6 defined", + service: &v1.Service{ + Spec: v1.ServiceSpec{ + IPFamilies: []v1.IPFamily{v1.IPv6Protocol}, + }, + }, + expected: elbv2types.IpAddressTypeDualstack, + }, + { + name: "IPv6 and IPv4", + service: &v1.Service{ + Spec: v1.ServiceSpec{ + IPFamilies: []v1.IPFamily{v1.IPv6Protocol, v1.IPv4Protocol}, + }, + }, + expected: elbv2types.IpAddressTypeDualstack, + }, + { + name: "IPv4 and IPv6", + service: &v1.Service{ + Spec: v1.ServiceSpec{ + IPFamilies: []v1.IPFamily{v1.IPv4Protocol, v1.IPv6Protocol}, + }, + }, + expected: elbv2types.IpAddressTypeDualstack, + }, + { + name: "No service", + service: nil, + expected: elbv2types.IpAddressTypeIpv4, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + result := getLoadBalancerIPAddressTypeFromService(tt.service) + assert.Equal(t, tt.expected, result, "IP address type did not expected for test case: %s", tt.name) + }) + } +} diff --git a/pkg/providers/v1/aws_test.go b/pkg/providers/v1/aws_test.go index 3188b24b03..3068ab476e 100644 --- a/pkg/providers/v1/aws_test.go +++ b/pkg/providers/v1/aws_test.go @@ -4053,6 +4053,8 @@ func TestEnsureLoadBalancer(t *testing.T) { name string annotations map[string]string config func() config.CloudConfig + ipFamilies []v1.IPFamily + ipFamilyPolicy *v1.IPFamilyPolicy want *v1.LoadBalancerStatus wantErr bool HookPostChecks func(*testCase, *Cloud, *v1.Service) @@ -4093,6 +4095,13 @@ func TestEnsureLoadBalancer(t *testing.T) { } }, }, + { + name: "reject single stack IPv6 on NLB", + annotations: map[string]string{ServiceAnnotationLoadBalancerType: "nlb"}, + ipFamilies: []v1.IPFamily{v1.IPv6Protocol}, + ipFamilyPolicy: func() *v1.IPFamilyPolicy { p := v1.IPFamilyPolicySingleStack; return &p }(), + wantErr: true, + }, } for _, test := range tests { @@ -4269,6 +4278,12 @@ func TestEnsureLoadBalancer(t *testing.T) { if len(test.annotations) > 0 { testService.Annotations = test.annotations } + if len(test.ipFamilies) > 0 { + testService.Spec.IPFamilies = test.ipFamilies + } + if test.ipFamilyPolicy != nil { + testService.Spec.IPFamilyPolicy = test.ipFamilyPolicy + } // Test svcStatus, err := c.EnsureLoadBalancer(context.TODO(), TestClusterName, testService, nodes) @@ -4293,11 +4308,12 @@ func TestCreateSecurityGroupRules(t *testing.T) { c.vpcID = "vpc-mac0" testCases := []struct { - name string - sgID string - rules IPPermissionSet - ec2SourceRanges []ec2types.IpRange - expectError bool + name string + sgID string + rules IPPermissionSet + ec2SourceRanges []ec2types.IpRange + ec2Ipv6SourceRanges []ec2types.Ipv6Range + expectError bool }{ { name: "successful security group rule creation", @@ -4316,6 +4332,28 @@ func TestCreateSecurityGroupRules(t *testing.T) { }, expectError: false, }, + { + name: "successful security group dual stack rule creation", + sgID: "sg-123456", + rules: IPPermissionSet{ + "tcp-80-80": ec2types.IpPermission{ + IpProtocol: aws.String("tcp"), + FromPort: aws.Int32(80), + ToPort: aws.Int32(80), + }, + }, + ec2SourceRanges: []ec2types.IpRange{ + { + CidrIp: aws.String("0.0.0.0/0"), + }, + }, + ec2Ipv6SourceRanges: []ec2types.Ipv6Range{ + { + CidrIpv6: aws.String("::/128"), + }, + }, + expectError: false, + }, { name: "empty security group ID", sgID: "", @@ -4342,6 +4380,27 @@ func TestCreateSecurityGroupRules(t *testing.T) { CidrIp: aws.String("0.0.0.0/0"), }, }, + ec2Ipv6SourceRanges: []ec2types.Ipv6Range{ + { + CidrIpv6: aws.String("::/128"), + }, + }, + expectError: false, + }, + { + name: "empty dual stack rule set", + sgID: "sg-123456", + rules: IPPermissionSet{}, + ec2SourceRanges: []ec2types.IpRange{ + { + CidrIp: aws.String("0.0.0.0/0"), + }, + }, + ec2Ipv6SourceRanges: []ec2types.Ipv6Range{ + { + CidrIpv6: aws.String("::/128"), + }, + }, expectError: false, }, { @@ -4355,6 +4414,22 @@ func TestCreateSecurityGroupRules(t *testing.T) { }, expectError: false, }, + { + name: "internal dual stack sources", + sgID: "sg-123456", + rules: IPPermissionSet{}, + ec2SourceRanges: []ec2types.IpRange{ + { + CidrIp: aws.String("10.0.0.0/16"), + }, + }, + ec2Ipv6SourceRanges: []ec2types.Ipv6Range{ + { + CidrIpv6: aws.String("fc00::/8"), + }, + }, + expectError: false, + }, } for _, tc := range testCases { @@ -4371,7 +4446,7 @@ func TestCreateSecurityGroupRules(t *testing.T) { ).Maybe() // Execute test - err := c.createSecurityGroupRules(context.TODO(), tc.sgID, tc.rules, tc.ec2SourceRanges) + err := c.createSecurityGroupRules(context.TODO(), tc.sgID, tc.rules, tc.ec2SourceRanges, tc.ec2Ipv6SourceRanges) // Verify results if tc.expectError { @@ -5204,3 +5279,103 @@ func TestCloud_GetSecurityGroupNameForNLB(t *testing.T) { }) } } + +func TestSeparateIPv4AndIPv6CIDRs(t *testing.T) { + tests := []struct { + name string + cidrs []string + expectedIPv4 []ec2types.IpRange + expectedIPv6 []ec2types.Ipv6Range + }{ + { + name: "Only IPv4 CIDRs", + cidrs: []string{"192.168.1.0/24", "10.0.0.0/8"}, + expectedIPv4: []ec2types.IpRange{ + {CidrIp: aws.String("192.168.1.0/24")}, + {CidrIp: aws.String("10.0.0.0/8")}, + }, + expectedIPv6: []ec2types.Ipv6Range{}, + }, + { + name: "Only IPv6 CIDRs", + cidrs: []string{"2001:db8::/32", "fd00::/8"}, + expectedIPv4: []ec2types.IpRange{}, + expectedIPv6: []ec2types.Ipv6Range{ + {CidrIpv6: aws.String("2001:db8::/32")}, + {CidrIpv6: aws.String("fd00::/8")}, + }, + }, + { + name: "Mixed IPv4 and IPv6 CIDRs", + cidrs: []string{"192.168.1.0/24", "2001:db8::/32", "10.0.0.0/8", "fd00::/8"}, + expectedIPv4: []ec2types.IpRange{ + {CidrIp: aws.String("192.168.1.0/24")}, + {CidrIp: aws.String("10.0.0.0/8")}, + }, + expectedIPv6: []ec2types.Ipv6Range{ + {CidrIpv6: aws.String("2001:db8::/32")}, + {CidrIpv6: aws.String("fd00::/8")}, + }, + }, + { + name: "Default IPv4 and IPv6", + cidrs: []string{"0.0.0.0/0", "::/0"}, + expectedIPv4: []ec2types.IpRange{ + {CidrIp: aws.String("0.0.0.0/0")}, + }, + expectedIPv6: []ec2types.Ipv6Range{ + {CidrIpv6: aws.String("::/0")}, + }, + }, + { + name: "Empty CIDR list", + cidrs: []string{}, + expectedIPv4: []ec2types.IpRange{}, + expectedIPv6: []ec2types.Ipv6Range{}, + }, + { + name: "Invalid CIDR is skipped", + cidrs: []string{"192.168.1.0/24", "invalid-cidr", "2001:db8::/32"}, + expectedIPv4: []ec2types.IpRange{ + {CidrIp: aws.String("192.168.1.0/24")}, + }, + expectedIPv6: []ec2types.Ipv6Range{ + {CidrIpv6: aws.String("2001:db8::/32")}, + }, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + ipv4Ranges, ipv6Ranges := separateIPv4AndIPv6CIDRs(tt.cidrs) + + // Compare IPv4 ranges + if len(ipv4Ranges) != len(tt.expectedIPv4) { + t.Errorf("IPv4 range count mismatch: got %d, expected %d", len(ipv4Ranges), len(tt.expectedIPv4)) + } + for i, r := range ipv4Ranges { + if i >= len(tt.expectedIPv4) { + break + } + if aws.ToString(r.CidrIp) != aws.ToString(tt.expectedIPv4[i].CidrIp) { + t.Errorf("IPv4 range[%d] mismatch: got %s, expected %s", + i, aws.ToString(r.CidrIp), aws.ToString(tt.expectedIPv4[i].CidrIp)) + } + } + + // Compare IPv6 ranges + if len(ipv6Ranges) != len(tt.expectedIPv6) { + t.Errorf("IPv6 range count mismatch: got %d, expected %d", len(ipv6Ranges), len(tt.expectedIPv6)) + } + for i, r := range ipv6Ranges { + if i >= len(tt.expectedIPv6) { + break + } + if aws.ToString(r.CidrIpv6) != aws.ToString(tt.expectedIPv6[i].CidrIpv6) { + t.Errorf("IPv6 range[%d] mismatch: got %s, expected %s", + i, aws.ToString(r.CidrIpv6), aws.ToString(tt.expectedIPv6[i].CidrIpv6)) + } + } + }) + } +} diff --git a/pkg/providers/v1/aws_validations.go b/pkg/providers/v1/aws_validations.go index cc8c440238..42b710418c 100644 --- a/pkg/providers/v1/aws_validations.go +++ b/pkg/providers/v1/aws_validations.go @@ -21,6 +21,7 @@ import ( "strings" v1 "k8s.io/api/core/v1" + "k8s.io/utils/ptr" ) // validationInput is the input parameters for validations. @@ -150,3 +151,55 @@ func validateServiceAnnotationTargetGroupAttributes(v *awsValidationInput) error return nil } + +// validateIPFamilyInfo validates that a Service's IP Families and IP Family Policies are supported. +// Special cases: +// - Cannot have an IPv6, single stack service (AWS limitation) +// - RequireDualStack policy *must* have 2 entries in IP Family Policies +// +// input: +// service: the target v1.Service +// +// returns: +// - error: validation errors. +func validateIPFamilyInfo(service *v1.Service, ipv6Requested bool) error { + // Sanity checks in case they're missed earlier up the call stack. + if service == nil { + return fmt.Errorf("service required") + } + + // Make sure we have a usable zero value for IPFamilies + if service.Spec.IPFamilies == nil { + service.Spec.IPFamilies = make([]v1.IPFamily, 0) + } + + // If we somehow got an unset IP familiy policy, (most likely in tests) set it explicitly for our use. + ipFamilyPolicy := service.Spec.IPFamilyPolicy + if ipFamilyPolicy == nil { + ipFamilyPolicy = ptr.To(v1.IPFamilyPolicySingleStack) + } + + // Kube server will ensure that Spec.IPFamilyPolicy and Spec.IPFamilies are populated + // See kubernetes/pkg/registry/core/service/storage/{alloc,storage}.go + ipFamilies := service.Spec.IPFamilies + if len(ipFamilies) >= 3 { + return fmt.Errorf("ipFamilies requires 1 or 2 entries. got %d", len(ipFamilies)) + } + + // Single stack IPv6 not supported by AWS + if *ipFamilyPolicy == v1.IPFamilyPolicySingleStack && ipv6Requested { + return fmt.Errorf("single stack IPv6 is not supported for network load balancers") + } + + // RequireDualStack must have 2 entries + if *ipFamilyPolicy == v1.IPFamilyPolicyRequireDualStack && len(ipFamilies) != 2 { + return fmt.Errorf("policy %s requires 2 entries in the ipFamilies field. got %d", v1.IPFamilyPolicyRequireDualStack, len(ipFamilies)) + } + + // PreferDualStack supports 1 or 2 entries. + if *ipFamilyPolicy == v1.IPFamilyPolicyPreferDualStack && (len(ipFamilies) >= 3) { + return fmt.Errorf("policy %s requires 1 or 2 entries. got %d", v1.IPFamilyPolicyPreferDualStack, len(ipFamilies)) + } + + return nil +} diff --git a/pkg/providers/v1/aws_validations_test.go b/pkg/providers/v1/aws_validations_test.go index 01556de518..80834fec64 100644 --- a/pkg/providers/v1/aws_validations_test.go +++ b/pkg/providers/v1/aws_validations_test.go @@ -564,3 +564,94 @@ func TestValidateServiceAnnotations(t *testing.T) { }) } } + +func TestValidateIPFamilyInfo(t *testing.T) { + tests := []struct { + name string + ipFamilyPolicy v1.IPFamilyPolicy + ipFamilies []v1.IPFamily + expectedError string + }{ + { + name: "SingleStack IPv6 errors", + ipFamilyPolicy: v1.IPFamilyPolicySingleStack, + ipFamilies: []v1.IPFamily{v1.IPv6Protocol}, + expectedError: "single stack IPv6 is not supported for network load balancers", + }, + { + name: "SingleStack IPv4 works", + ipFamilyPolicy: v1.IPFamilyPolicySingleStack, + ipFamilies: []v1.IPFamily{v1.IPv4Protocol}, + expectedError: "", + }, + { + name: "RequireDualStack with one family errors", + ipFamilyPolicy: v1.IPFamilyPolicyRequireDualStack, + ipFamilies: []v1.IPFamily{v1.IPv6Protocol}, + expectedError: "policy RequireDualStack requires 2 entries in the ipFamilies field. got 1", + }, + { + name: "PreferDualStack with too many entries errors", + ipFamilyPolicy: v1.IPFamilyPolicyPreferDualStack, + ipFamilies: []v1.IPFamily{v1.IPv6Protocol, v1.IPv4Protocol, v1.IPv4Protocol}, + expectedError: "ipFamilies requires 1 or 2 entries. got 3", + }, + { + name: "PreferDualStack with one entry works", + ipFamilyPolicy: v1.IPFamilyPolicyPreferDualStack, + ipFamilies: []v1.IPFamily{v1.IPv6Protocol}, + expectedError: "", + }, + { + name: "PreferDualStack with two entries works", + ipFamilyPolicy: v1.IPFamilyPolicyPreferDualStack, + ipFamilies: []v1.IPFamily{v1.IPv6Protocol, v1.IPv4Protocol}, + expectedError: "", + }, + { + name: "PreferDualStack with two entries works", + ipFamilyPolicy: v1.IPFamilyPolicyPreferDualStack, + ipFamilies: []v1.IPFamily{v1.IPv6Protocol, v1.IPv4Protocol}, + expectedError: "", + }, + { + name: "RequireDualStack with two entries works", + ipFamilyPolicy: v1.IPFamilyPolicyPreferDualStack, + ipFamilies: []v1.IPFamily{v1.IPv6Protocol, v1.IPv4Protocol}, + expectedError: "", + }, + { + name: "RequireDualStack with two entries works", + ipFamilyPolicy: v1.IPFamilyPolicyPreferDualStack, + ipFamilies: []v1.IPFamily{v1.IPv6Protocol, v1.IPv4Protocol}, + expectedError: "", + }, + { + name: "IPFamily fields empty works (backwards compat, implies SingleStack IPv4)", + ipFamilyPolicy: "", + ipFamilies: []v1.IPFamily{}, + expectedError: "", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + s := &v1.Service{ + Spec: v1.ServiceSpec{ + IPFamilyPolicy: &tt.ipFamilyPolicy, + IPFamilies: tt.ipFamilies, + }, + } + + err := validateIPFamilyInfo(s, serviceRequestsIPv6(s)) + + if tt.expectedError == "" { + assert.NoError(t, err, "Expected no error for test case: %s", tt.name) + } else { + assert.Error(t, err, "Expected error for test case: %s", tt.name) + assert.Equal(t, err.Error(), tt.expectedError, "Expected error for test case: %s", tt.name) + assert.Contains(t, err.Error(), tt.expectedError, "Error message should contain expected text for test case: %s", tt.name) + } + }) + } +} From eb56af1699fb965d1eebf8896d464163a6dd2b51 Mon Sep 17 00:00:00 2001 From: Nolan Brubaker Date: Mon, 2 Mar 2026 15:46:48 -0500 Subject: [PATCH 2/7] Add dual-stack e2e test cases Signed-off-by: Nolan Brubaker --- pkg/providers/v1/aws.go | 27 +++-- pkg/providers/v1/aws_test.go | 37 ++++-- tests/e2e/loadbalancer.go | 224 +++++++++++++++++++++++++++++++++++ 3 files changed, 266 insertions(+), 22 deletions(-) diff --git a/pkg/providers/v1/aws.go b/pkg/providers/v1/aws.go index 0c2d856aa9..9360e2e86c 100644 --- a/pkg/providers/v1/aws.go +++ b/pkg/providers/v1/aws.go @@ -2009,15 +2009,24 @@ func (c *Cloud) createSecurityGroupRules(ctx context.Context, sgID string, rules if len(sgID) == 0 { return fmt.Errorf("security group ID cannot be empty") } - // Allow ICMP fragmentation packets, important for MTU discovery - permission := ec2types.IpPermission{ - IpProtocol: aws.String("icmp"), - FromPort: aws.Int32(3), - ToPort: aws.Int32(4), - IpRanges: ec2SourceRanges, - Ipv6Ranges: ec2Ipv6SourceRanges, - } - rules.Insert(permission) + // Allow ICMP fragmentation packets, important for IPv4 MTU discovery + if len(ec2SourceRanges) > 0 { + rules.Insert(ec2types.IpPermission{ + IpProtocol: aws.String("icmp"), + FromPort: aws.Int32(3), + ToPort: aws.Int32(4), + IpRanges: ec2SourceRanges, + }) + } + // Allow ICMPv6 "Packet Too Big" messages, important for IPv6 MTU discovery + if len(ec2Ipv6SourceRanges) > 0 { + rules.Insert(ec2types.IpPermission{ + IpProtocol: aws.String("icmpv6"), + FromPort: aws.Int32(2), + ToPort: aws.Int32(-1), + Ipv6Ranges: ec2Ipv6SourceRanges, + }) + } // Setup ingress rules if _, err := c.setSecurityGroupIngress(ctx, sgID, rules); err != nil { diff --git a/pkg/providers/v1/aws_test.go b/pkg/providers/v1/aws_test.go index 3068ab476e..2fba37d8c4 100644 --- a/pkg/providers/v1/aws_test.go +++ b/pkg/providers/v1/aws_test.go @@ -4455,23 +4455,34 @@ func TestCreateSecurityGroupRules(t *testing.T) { } assert.NoError(t, err) - // Verify that the rules include the ICMP permission for MTU discovery - foundMTURule := false - for _, rule := range tc.rules { - if aws.ToString(rule.IpProtocol) == "icmp" && - aws.ToInt32(rule.FromPort) == 3 && - aws.ToInt32(rule.ToPort) == 4 { - foundMTURule = true - break + // Verify ICMP rule for IPv4 MTU discovery + if len(tc.ec2SourceRanges) > 0 { + foundICMPRule := false + for _, rule := range tc.rules { + if aws.ToString(rule.IpProtocol) == "icmp" && + aws.ToInt32(rule.FromPort) == 3 && + aws.ToInt32(rule.ToPort) == 4 { + foundICMPRule = true + assert.Equal(t, tc.ec2SourceRanges, rule.IpRanges, "ICMP rule should have correct IPv4 ranges") + assert.Empty(t, rule.Ipv6Ranges, "ICMP rule should not have IPv6 ranges") + } } + assert.True(t, foundICMPRule, "ICMP MTU discovery rule should be added for IPv4") } - assert.True(t, foundMTURule, "MTU discovery rule should be added") - // Verify the ec2SourceRanges were properly set - for _, rule := range tc.rules { - if aws.ToString(rule.IpProtocol) == "icmp" { - assert.Equal(t, tc.ec2SourceRanges, rule.IpRanges) + // Verify ICMPv6 rule for IPv6 MTU discovery + if len(tc.ec2Ipv6SourceRanges) > 0 { + foundICMPv6Rule := false + for _, rule := range tc.rules { + if aws.ToString(rule.IpProtocol) == "icmpv6" && + aws.ToInt32(rule.FromPort) == 2 && + aws.ToInt32(rule.ToPort) == -1 { + foundICMPv6Rule = true + assert.Equal(t, tc.ec2Ipv6SourceRanges, rule.Ipv6Ranges, "ICMPv6 rule should have correct IPv6 ranges") + assert.Empty(t, rule.IpRanges, "ICMPv6 rule should not have IPv4 ranges") + } } + assert.True(t, foundICMPv6Rule, "ICMPv6 MTU discovery rule should be added for IPv6") } }) } diff --git a/tests/e2e/loadbalancer.go b/tests/e2e/loadbalancer.go index 9012112abc..acf02cf542 100644 --- a/tests/e2e/loadbalancer.go +++ b/tests/e2e/loadbalancer.go @@ -246,6 +246,230 @@ var _ = Describe("[cloud-provider-aws-e2e] loadbalancer", func() { } }, }, + // Dual-stack test cases + { + name: "NLB dual-stack (IPv4 primary) should be reachable", + resourceSuffix: "nlb-ds-v4", + extraAnnotations: map[string]string{annotationLBType: "nlb"}, + listenerCount: 1, + hookPostServiceConfig: func(cfg *e2eTestConfig) { + framework.Logf("running hook post-service-config: configuring dual-stack with IPv4 primary") + // Set dual-stack configuration with IPv4 as primary + cfg.svc.Spec.IPFamilies = []v1.IPFamily{v1.IPv4Protocol, v1.IPv6Protocol} + ipFamilyPolicy := v1.IPFamilyPolicyPreferDualStack + cfg.svc.Spec.IPFamilyPolicy = &ipFamilyPolicy + framework.Logf("Service configured: IPFamilies=%v, IPFamilyPolicy=%v", + cfg.svc.Spec.IPFamilies, *cfg.svc.Spec.IPFamilyPolicy) + }, + hookPostServiceCreate: func(cfg *e2eTestConfig) { + framework.Logf("running hook post-service-create: validating dual-stack AWS load balancer configuration") + + if len(cfg.svc.Status.LoadBalancer.Ingress) == 0 { + framework.Failf("No ingress found in LoadBalancer status for service %s/%s", + cfg.svc.Namespace, cfg.svc.Name) + } + + lbDNS := cfg.svc.Status.LoadBalancer.Ingress[0].Hostname + framework.Logf("Load balancer DNS: %s", lbDNS) + + // Get AWS ELB client + elbClient, err := getAWSClientLoadBalancer(cfg.ctx) + framework.ExpectNoError(err, "failed to create AWS ELB client") + + // Find load balancer by DNS name + lb, err := getAWSLoadBalancerFromDNSName(cfg.ctx, elbClient, lbDNS) + framework.ExpectNoError(err, "failed to find load balancer with DNS name %s", lbDNS) + + if lb == nil { + framework.Failf("Load balancer is nil for DNS name %s", lbDNS) + } + + // Validate Load Balancer IP Address Type is Dualstack + if lb.IpAddressType != elbv2types.IpAddressTypeDualstack { + framework.Failf("Load balancer IpAddressType mismatch: expected %s, got %s", + elbv2types.IpAddressTypeDualstack, lb.IpAddressType) + } + framework.Logf("✓ Load Balancer IpAddressType: %s", lb.IpAddressType) + + // Get Target Groups + tgResp, err := elbClient.DescribeTargetGroups(cfg.ctx, &elbv2.DescribeTargetGroupsInput{ + LoadBalancerArn: lb.LoadBalancerArn, + }) + framework.ExpectNoError(err, "failed to describe target groups") + + if len(tgResp.TargetGroups) == 0 { + framework.Failf("No target groups found for load balancer") + } + + // Validate Target Group IP Address Type is ipv4 (based on first IP family) + tg := tgResp.TargetGroups[0] + expectedTGType := elbv2types.TargetGroupIpAddressTypeEnumIpv4 + if tg.IpAddressType != expectedTGType { + framework.Failf("Target group IpAddressType mismatch: expected %s, got %s", + expectedTGType, tg.IpAddressType) + } + framework.Logf("✓ Target Group IpAddressType: %s, TargetType: %s", + tg.IpAddressType, tg.TargetType) + }, + }, + { + name: "NLB dual-stack (IPv6 primary) should be reachable", + resourceSuffix: "nlb-ds-v6", + extraAnnotations: map[string]string{annotationLBType: "nlb"}, + listenerCount: 1, + hookPostServiceConfig: func(cfg *e2eTestConfig) { + framework.Logf("running hook post-service-config: configuring dual-stack with IPv6 primary") + // Set dual-stack configuration with IPv6 as primary + cfg.svc.Spec.IPFamilies = []v1.IPFamily{v1.IPv6Protocol, v1.IPv4Protocol} + ipFamilyPolicy := v1.IPFamilyPolicyPreferDualStack + cfg.svc.Spec.IPFamilyPolicy = &ipFamilyPolicy + framework.Logf("Service configured: IPFamilies=%v, IPFamilyPolicy=%v", + cfg.svc.Spec.IPFamilies, *cfg.svc.Spec.IPFamilyPolicy) + }, + hookPostServiceCreate: func(cfg *e2eTestConfig) { + framework.Logf("running hook post-service-create: validating dual-stack AWS load balancer configuration with IPv6 target group") + + if len(cfg.svc.Status.LoadBalancer.Ingress) == 0 { + framework.Failf("No ingress found in LoadBalancer status for service %s/%s", + cfg.svc.Namespace, cfg.svc.Name) + } + + lbDNS := cfg.svc.Status.LoadBalancer.Ingress[0].Hostname + elbClient, err := getAWSClientLoadBalancer(cfg.ctx) + framework.ExpectNoError(err, "failed to create AWS ELB client") + + lb, err := getAWSLoadBalancerFromDNSName(cfg.ctx, elbClient, lbDNS) + framework.ExpectNoError(err, "failed to find load balancer") + + // Validate Load Balancer IP Address Type + if lb.IpAddressType != elbv2types.IpAddressTypeDualstack { + framework.Failf("Load balancer IpAddressType mismatch: expected %s, got %s", + elbv2types.IpAddressTypeDualstack, lb.IpAddressType) + } + framework.Logf("✓ Load Balancer IpAddressType: %s", lb.IpAddressType) + + // Get Target Groups and validate IPv6 type + tgResp, err := elbClient.DescribeTargetGroups(cfg.ctx, &elbv2.DescribeTargetGroupsInput{ + LoadBalancerArn: lb.LoadBalancerArn, + }) + framework.ExpectNoError(err, "failed to describe target groups") + + if len(tgResp.TargetGroups) == 0 { + framework.Failf("No target groups found for load balancer") + } + + // Validate Target Group IP Address Type is ipv6 (based on first IP family) + tg := tgResp.TargetGroups[0] + expectedTGType := elbv2types.TargetGroupIpAddressTypeEnumIpv6 + if tg.IpAddressType != expectedTGType { + framework.Failf("Target group IpAddressType mismatch: expected %s, got %s", + expectedTGType, tg.IpAddressType) + } + framework.Logf("✓ Target Group IpAddressType: %s, TargetType: %s", + tg.IpAddressType, tg.TargetType) + }, + }, + { + name: "NLB internal dual-stack should be reachable", + resourceSuffix: "nlb-ds-internal", + extraAnnotations: map[string]string{ + annotationLBType: "nlb", + annotationLBInternal: "true", + }, + overrideTestRunInClusterReachableHTTP: true, + requireAffinity: true, + listenerCount: 1, + hookPostServiceConfig: func(cfg *e2eTestConfig) { + framework.Logf("running hook post-service-config: configuring internal dual-stack NLB with node affinity") + // Set dual-stack configuration + cfg.svc.Spec.IPFamilies = []v1.IPFamily{v1.IPv4Protocol, v1.IPv6Protocol} + ipFamilyPolicy := v1.IPFamilyPolicyPreferDualStack + cfg.svc.Spec.IPFamilyPolicy = &ipFamilyPolicy + + // Pin to single node for hairpin testing + if cfg.svc.Annotations == nil { + cfg.svc.Annotations = map[string]string{} + } + cfg.svc.Annotations[annotationLBTargetNodeLabels] = fmt.Sprintf("kubernetes.io/hostname=%s", cfg.nodeSingleSample) + framework.Logf("Service configured: IPFamilies=%v, IPFamilyPolicy=%v, Target node=%s", + cfg.svc.Spec.IPFamilies, *cfg.svc.Spec.IPFamilyPolicy, cfg.nodeSingleSample) + }, + hookPostServiceCreate: func(cfg *e2eTestConfig) { + framework.Logf("running hook post-service-create: validating internal dual-stack load balancer") + + if len(cfg.svc.Status.LoadBalancer.Ingress) == 0 { + framework.Failf("No ingress found in LoadBalancer status for service %s/%s", + cfg.svc.Namespace, cfg.svc.Name) + } + + lbDNS := cfg.svc.Status.LoadBalancer.Ingress[0].Hostname + elbClient, err := getAWSClientLoadBalancer(cfg.ctx) + framework.ExpectNoError(err, "failed to create AWS ELB client") + + lb, err := getAWSLoadBalancerFromDNSName(cfg.ctx, elbClient, lbDNS) + framework.ExpectNoError(err, "failed to find load balancer") + + // Validate Load Balancer is internal and dual-stack + if lb.Scheme != elbv2types.LoadBalancerSchemeEnumInternal { + framework.Failf("Load balancer scheme mismatch: expected %s, got %s", + elbv2types.LoadBalancerSchemeEnumInternal, lb.Scheme) + } + framework.Logf("✓ Load Balancer Scheme: %s", lb.Scheme) + + if lb.IpAddressType != elbv2types.IpAddressTypeDualstack { + framework.Failf("Load balancer IpAddressType mismatch: expected %s, got %s", + elbv2types.IpAddressTypeDualstack, lb.IpAddressType) + } + framework.Logf("✓ Load Balancer IpAddressType: %s", lb.IpAddressType) + + // Validate target count matches single node + framework.ExpectNoError(getLBTargetCount(cfg.ctx, lbDNS, 1), + "AWS LB target count validation failed for single-node affinity") + }, + }, + // NOTE: This test documents current behavior for Classic ELB with dual-stack configuration. + // Classic ELB does NOT properly support dual-stack (Issue #7 - missing validation). + // This test validates that Classic ELB is created but may not handle IPv6 correctly. + { + name: "CLB dual-stack documents current behavior", + resourceSuffix: "clb-ds", + extraAnnotations: map[string]string{}, // No annotation = Classic ELB + listenerCount: 1, + skipTestFailure: true, // May fail due to lack of IPv6 support + hookPostServiceConfig: func(cfg *e2eTestConfig) { + framework.Logf("running hook post-service-config: configuring dual-stack with Classic ELB (unsupported)") + // Set dual-stack configuration with Classic ELB + cfg.svc.Spec.IPFamilies = []v1.IPFamily{v1.IPv4Protocol, v1.IPv6Protocol} + ipFamilyPolicy := v1.IPFamilyPolicyPreferDualStack + cfg.svc.Spec.IPFamilyPolicy = &ipFamilyPolicy + framework.Logf("WARNING: Classic ELB does not properly support dual-stack") + framework.Logf("Service configured: IPFamilies=%v, IPFamilyPolicy=%v", + cfg.svc.Spec.IPFamilies, *cfg.svc.Spec.IPFamilyPolicy) + }, + hookPostServiceCreate: func(cfg *e2eTestConfig) { + framework.Logf("running hook post-service-create: documenting Classic ELB behavior with dual-stack") + + // Note: Classic ELB may be created but won't have dual-stack support + // This test documents what actually happens rather than asserting correct behavior + + if len(cfg.svc.Status.LoadBalancer.Ingress) == 0 { + framework.Logf("No ingress found - Classic ELB may have failed to provision with dual-stack") + return + } + + framework.Logf("Classic ELB provisioned with dual-stack config") + framework.Logf("NOTE: Classic ELB does NOT have IpAddressType field - cannot validate dual-stack") + framework.Logf("TODO: Add validation in Issue #7 to reject dual-stack for Classic ELB") + + // Document what was created + lbIngress := cfg.svc.Status.LoadBalancer.Ingress[0] + if lbIngress.Hostname != "" { + framework.Logf("Classic ELB DNS: %s", lbIngress.Hostname) + } else if lbIngress.IP != "" { + framework.Logf("Classic ELB IP: %s", lbIngress.IP) + } + }, + }, } serviceNameBase := "lbconfig-test" From fa180ecef92d45768be71bd0048bfb78d6c7b6df Mon Sep 17 00:00:00 2001 From: Nolan Brubaker Date: Tue, 24 Mar 2026 15:01:50 -0400 Subject: [PATCH 3/7] Support IP address type changes on update Signed-off-by: Nolan Brubaker --- pkg/providers/v1/aws.go | 1 + pkg/providers/v1/aws_fakes.go | 5 +++++ pkg/providers/v1/aws_loadbalancer.go | 15 +++++++++++++++ 3 files changed, 21 insertions(+) diff --git a/pkg/providers/v1/aws.go b/pkg/providers/v1/aws.go index 9360e2e86c..eb6a478528 100644 --- a/pkg/providers/v1/aws.go +++ b/pkg/providers/v1/aws.go @@ -358,6 +358,7 @@ type ELBV2 interface { DescribeLoadBalancers(ctx context.Context, input *elbv2.DescribeLoadBalancersInput, optFns ...func(*elbv2.Options)) (*elbv2.DescribeLoadBalancersOutput, error) DeleteLoadBalancer(ctx context.Context, input *elbv2.DeleteLoadBalancerInput, optFns ...func(*elbv2.Options)) (*elbv2.DeleteLoadBalancerOutput, error) SetSecurityGroups(ctx context.Context, input *elbv2.SetSecurityGroupsInput, optFns ...func(*elbv2.Options)) (*elbv2.SetSecurityGroupsOutput, error) + SetIpAddressType(ctx context.Context, input *elbv2.SetIpAddressTypeInput, optFns ...func(*elbv2.Options)) (*elbv2.SetIpAddressTypeOutput, error) ModifyLoadBalancerAttributes(ctx context.Context, input *elbv2.ModifyLoadBalancerAttributesInput, optFns ...func(*elbv2.Options)) (*elbv2.ModifyLoadBalancerAttributesOutput, error) DescribeLoadBalancerAttributes(ctx context.Context, input *elbv2.DescribeLoadBalancerAttributesInput, optFns ...func(*elbv2.Options)) (*elbv2.DescribeLoadBalancerAttributesOutput, error) diff --git a/pkg/providers/v1/aws_fakes.go b/pkg/providers/v1/aws_fakes.go index 3b0d4b8b13..c685818cb7 100644 --- a/pkg/providers/v1/aws_fakes.go +++ b/pkg/providers/v1/aws_fakes.go @@ -715,6 +715,11 @@ func (elb *FakeELBV2) SetSecurityGroups(ctx context.Context, input *elbv2.SetSec panic("Not implemented") } +// SetIpAddressType is not implemented but is required for interface conformance +func (elb *FakeELBV2) SetIpAddressType(ctx context.Context, input *elbv2.SetIpAddressTypeInput, optFns ...func(*elbv2.Options)) (*elbv2.SetIpAddressTypeOutput, error) { + panic("Not implemented") +} + // ModifyLoadBalancerAttributes is not implemented but is required for interface conformance func (elb *FakeELBV2) ModifyLoadBalancerAttributes(ctx context.Context, input *elbv2.ModifyLoadBalancerAttributesInput, optFns ...func(*elbv2.Options)) (*elbv2.ModifyLoadBalancerAttributesOutput, error) { panic("Not implemented") diff --git a/pkg/providers/v1/aws_loadbalancer.go b/pkg/providers/v1/aws_loadbalancer.go index ae8eaa04bf..91662faa32 100644 --- a/pkg/providers/v1/aws_loadbalancer.go +++ b/pkg/providers/v1/aws_loadbalancer.go @@ -266,6 +266,21 @@ func (c *Cloud) ensureLoadBalancerv2(ctx context.Context, namespacedName types.N } else { // TODO: Sync internal vs non-internal + // Reconcile LB IpAddressType: if the Service's desired address family + // differs from what the existing NLB has, update it via SetIpAddressType. + desiredIPAddressType := getLoadBalancerIPAddressTypeFromService(service) + if loadBalancer.IpAddressType != desiredIPAddressType { + klog.Infof("Updating load balancer %s IpAddressType from %s to %s for %v", + loadBalancerName, loadBalancer.IpAddressType, desiredIPAddressType, namespacedName) + if _, err := c.elbv2.SetIpAddressType(ctx, &elbv2.SetIpAddressTypeInput{ + LoadBalancerArn: loadBalancer.LoadBalancerArn, + IpAddressType: desiredIPAddressType, + }); err != nil { + return nil, fmt.Errorf("error updating load balancer IpAddressType: %q", err) + } + dirty = true + } + // sync mappings { listenerDescriptions, err := c.elbv2.DescribeListeners(ctx, From aa4d51bf3de7e62fca29e8e753fa57c2228e3a06 Mon Sep 17 00:00:00 2001 From: Nolan Brubaker Date: Tue, 24 Mar 2026 15:13:14 -0400 Subject: [PATCH 4/7] Hash target group names with IP address family Changing the IP address type would invalidate the target group name, because you cannot have an IPv4 and IPv6 target for the same port/protocol set. Signed-off-by: Nolan Brubaker --- pkg/providers/v1/aws_loadbalancer.go | 7 ++-- pkg/providers/v1/aws_loadbalancer_test.go | 44 +++++++++++++++++------ 2 files changed, 38 insertions(+), 13 deletions(-) diff --git a/pkg/providers/v1/aws_loadbalancer.go b/pkg/providers/v1/aws_loadbalancer.go index 91662faa32..93b5c2d33a 100644 --- a/pkg/providers/v1/aws_loadbalancer.go +++ b/pkg/providers/v1/aws_loadbalancer.go @@ -738,8 +738,8 @@ var invalidELBV2NameRegex = regexp.MustCompile("[^[:alnum:]]") // buildTargetGroupName will build unique name for targetGroup of service & port. // the name is in format k8s-{namespace:8}-{name:8}-{uuid:10} (chosen to benefit most common use cases). -// Note: nodePort & targetProtocol & targetType are included since they cannot be modified on existing targetGroup. -func (c *Cloud) buildTargetGroupName(serviceName types.NamespacedName, servicePort int32, nodePort int32, targetProtocol elbv2types.ProtocolEnum, targetType elbv2types.TargetTypeEnum, mapping nlbPortMapping) string { +// Note: nodePort & targetProtocol & targetType & ipAddressType are included since they cannot be modified on existing targetGroup. +func (c *Cloud) buildTargetGroupName(serviceName types.NamespacedName, servicePort int32, nodePort int32, targetProtocol elbv2types.ProtocolEnum, targetType elbv2types.TargetTypeEnum, mapping nlbPortMapping, ipAddressType elbv2types.TargetGroupIpAddressTypeEnum) string { hasher := sha1.New() _, _ = hasher.Write([]byte(c.tagging.clusterID())) _, _ = hasher.Write([]byte(serviceName.Namespace)) @@ -750,6 +750,7 @@ func (c *Cloud) buildTargetGroupName(serviceName types.NamespacedName, servicePo _, _ = hasher.Write([]byte(targetType)) _, _ = hasher.Write([]byte(mapping.HealthCheckConfig.Protocol)) _, _ = hasher.Write([]byte(strconv.FormatInt(int64(mapping.HealthCheckConfig.Interval), 10))) + _, _ = hasher.Write([]byte(ipAddressType)) tgUUID := hex.EncodeToString(hasher.Sum(nil)) sanitizedNamespace := invalidELBV2NameRegex.ReplaceAllString(serviceName.Namespace, "") @@ -827,7 +828,7 @@ func (c *Cloud) ensureTargetGroup(ctx context.Context, targetGroup *elbv2types.T expectedTargets := c.computeTargetGroupExpectedTargets(instances, mapping.TrafficPort) if targetGroup == nil { targetType := elbv2types.TargetTypeEnumInstance - name := c.buildTargetGroupName(serviceName, mapping.FrontendPort, mapping.TrafficPort, mapping.TrafficProtocol, targetType, mapping) + name := c.buildTargetGroupName(serviceName, mapping.FrontendPort, mapping.TrafficPort, mapping.TrafficProtocol, targetType, mapping, ipAddressType) klog.Infof("Creating load balancer target group for %v with name: %s (IP address type: %s)", serviceName, name, ipAddressType) input := &elbv2.CreateTargetGroupInput{ VpcId: aws.String(vpcID), diff --git a/pkg/providers/v1/aws_loadbalancer_test.go b/pkg/providers/v1/aws_loadbalancer_test.go index a18bf2d3ad..8d45aa01e2 100644 --- a/pkg/providers/v1/aws_loadbalancer_test.go +++ b/pkg/providers/v1/aws_loadbalancer_test.go @@ -347,6 +347,7 @@ func TestBuildTargetGroupName(t *testing.T) { targetProtocol elbv2types.ProtocolEnum targetType elbv2types.TargetTypeEnum nlbConfig nlbPortMapping + ipAddressType elbv2types.TargetGroupIpAddressTypeEnum } tests := []struct { name string @@ -364,8 +365,9 @@ func TestBuildTargetGroupName(t *testing.T) { targetProtocol: elbv2types.ProtocolEnumTcp, targetType: elbv2types.TargetTypeEnumInstance, nlbConfig: nlbPortMapping{}, + ipAddressType: elbv2types.TargetGroupIpAddressTypeEnumIpv4, }, - want: "k8s-default-servicea-7fa2e07508", + want: "k8s-default-servicea-d09db77308", }, { name: "base case & clusterID changed", @@ -377,8 +379,9 @@ func TestBuildTargetGroupName(t *testing.T) { targetProtocol: elbv2types.ProtocolEnumTcp, targetType: elbv2types.TargetTypeEnumInstance, nlbConfig: nlbPortMapping{}, + ipAddressType: elbv2types.TargetGroupIpAddressTypeEnumIpv4, }, - want: "k8s-default-servicea-719ee635da", + want: "k8s-default-servicea-b8ce450922", }, { name: "base case & serviceNamespace changed", @@ -390,8 +393,9 @@ func TestBuildTargetGroupName(t *testing.T) { targetProtocol: elbv2types.ProtocolEnumTcp, targetType: elbv2types.TargetTypeEnumInstance, nlbConfig: nlbPortMapping{}, + ipAddressType: elbv2types.TargetGroupIpAddressTypeEnumIpv4, }, - want: "k8s-another-servicea-f66e09847d", + want: "k8s-another-servicea-8c06319cd6", }, { name: "base case & serviceName changed", @@ -403,8 +407,9 @@ func TestBuildTargetGroupName(t *testing.T) { targetProtocol: elbv2types.ProtocolEnumTcp, targetType: elbv2types.TargetTypeEnumInstance, nlbConfig: nlbPortMapping{}, + ipAddressType: elbv2types.TargetGroupIpAddressTypeEnumIpv4, }, - want: "k8s-default-serviceb-196c19c881", + want: "k8s-default-serviceb-138b54c161", }, { name: "base case & servicePort changed", @@ -416,8 +421,9 @@ func TestBuildTargetGroupName(t *testing.T) { targetProtocol: elbv2types.ProtocolEnumTcp, targetType: elbv2types.TargetTypeEnumInstance, nlbConfig: nlbPortMapping{}, + ipAddressType: elbv2types.TargetGroupIpAddressTypeEnumIpv4, }, - want: "k8s-default-servicea-06876706cb", + want: "k8s-default-servicea-3398ce2582", }, { name: "base case & nodePort changed", @@ -429,8 +435,9 @@ func TestBuildTargetGroupName(t *testing.T) { targetProtocol: elbv2types.ProtocolEnumTcp, targetType: elbv2types.TargetTypeEnumInstance, nlbConfig: nlbPortMapping{}, + ipAddressType: elbv2types.TargetGroupIpAddressTypeEnumIpv4, }, - want: "k8s-default-servicea-119f844ec0", + want: "k8s-default-servicea-c829356629", }, { name: "base case & targetProtocol changed", @@ -442,8 +449,9 @@ func TestBuildTargetGroupName(t *testing.T) { targetProtocol: elbv2types.ProtocolEnumUdp, targetType: elbv2types.TargetTypeEnumInstance, nlbConfig: nlbPortMapping{}, + ipAddressType: elbv2types.TargetGroupIpAddressTypeEnumIpv4, }, - want: "k8s-default-servicea-3868761686", + want: "k8s-default-servicea-57da8753a8", }, { name: "base case & targetType changed", @@ -455,8 +463,9 @@ func TestBuildTargetGroupName(t *testing.T) { targetProtocol: elbv2types.ProtocolEnumTcp, targetType: elbv2types.TargetTypeEnumIp, nlbConfig: nlbPortMapping{}, + ipAddressType: elbv2types.TargetGroupIpAddressTypeEnumIpv4, }, - want: "k8s-default-servicea-0fa31f4b0f", + want: "k8s-default-servicea-5160ded19b", }, { name: "custom healthcheck config", @@ -473,8 +482,23 @@ func TestBuildTargetGroupName(t *testing.T) { Interval: 10, }, }, + ipAddressType: elbv2types.TargetGroupIpAddressTypeEnumIpv4, }, - want: "k8s-default-servicea-4028e49618", + want: "k8s-default-servicea-c3f46cd4ed", + }, + { + name: "base case & ipAddressType changed to ipv6", + clusterID: "cluster-a", + args: args{ + serviceName: types.NamespacedName{Namespace: "default", Name: "service-a"}, + servicePort: 80, + nodePort: 8080, + targetProtocol: elbv2types.ProtocolEnumTcp, + targetType: elbv2types.TargetTypeEnumInstance, + nlbConfig: nlbPortMapping{}, + ipAddressType: elbv2types.TargetGroupIpAddressTypeEnumIpv6, + }, + want: "k8s-default-servicea-6abd575e99", }, } for _, tt := range tests { @@ -482,7 +506,7 @@ func TestBuildTargetGroupName(t *testing.T) { c := &Cloud{ tagging: awsTagging{ClusterID: tt.clusterID}, } - if got := c.buildTargetGroupName(tt.args.serviceName, tt.args.servicePort, tt.args.nodePort, tt.args.targetProtocol, tt.args.targetType, tt.args.nlbConfig); got != tt.want { + if got := c.buildTargetGroupName(tt.args.serviceName, tt.args.servicePort, tt.args.nodePort, tt.args.targetProtocol, tt.args.targetType, tt.args.nlbConfig, tt.args.ipAddressType); got != tt.want { assert.Equal(t, tt.want, got) } }) From 573c4ca3f47617df79533c57ae18d39819010e97 Mon Sep 17 00:00:00 2001 From: Nolan Brubaker Date: Tue, 24 Mar 2026 15:19:00 -0400 Subject: [PATCH 5/7] Take IPv6 CIDRs into account for all open traffic Signed-off-by: Nolan Brubaker --- pkg/providers/v1/aws_loadbalancer.go | 13 ++++++++++++- 1 file changed, 12 insertions(+), 1 deletion(-) diff --git a/pkg/providers/v1/aws_loadbalancer.go b/pkg/providers/v1/aws_loadbalancer.go index 93b5c2d33a..d0afff5bb2 100644 --- a/pkg/providers/v1/aws_loadbalancer.go +++ b/pkg/providers/v1/aws_loadbalancer.go @@ -1094,7 +1094,18 @@ func (c *Cloud) updateInstanceSecurityGroupsForNLB(ctx context.Context, lbName s if desiredSGIDs.Has(sgID) { // If the client rule is 1) all addresses 2) tcp and 3) has same ports as the healthcheck, // then the health rules are a subset of the client rule and are not needed. - if len(clientCIDRs) != 1 || clientCIDRs[0] != "0.0.0.0/0" || clientProtocol != "tcp" || !healthCheckPorts.Equal(clientPorts) { + // "All addresses" means 0.0.0.0/0 for IPv4-only, or both 0.0.0.0/0 and ::/0 for dual-stack. + var ipv4ClientCIDRs, ipv6ClientCIDRs []string + for _, cidr := range clientCIDRs { + if isIPv6CIDR(cidr) { + ipv6ClientCIDRs = append(ipv6ClientCIDRs, cidr) + } else { + ipv4ClientCIDRs = append(ipv4ClientCIDRs, cidr) + } + } + clientCIDRsAllOpen := len(ipv4ClientCIDRs) == 1 && ipv4ClientCIDRs[0] == "0.0.0.0/0" && + (len(ipv6ClientCIDRs) == 0 || (len(ipv6ClientCIDRs) == 1 && ipv6ClientCIDRs[0] == "::/0")) + if !clientCIDRsAllOpen || clientProtocol != "tcp" || !healthCheckPorts.Equal(clientPorts) { if err := c.updateInstanceSecurityGroupForNLBTraffic(ctx, sgID, sgPerms, healthRuleAnnotation, "tcp", healthCheckPorts, subnetCIDRs); err != nil { return err } From 5d22fe292aca572449b23759c6a083a949e2d056 Mon Sep 17 00:00:00 2001 From: Nolan Brubaker Date: Tue, 24 Mar 2026 16:01:09 -0400 Subject: [PATCH 6/7] fixup! Support IP address type changes on update Signed-off-by: Nolan Brubaker --- pkg/providers/v1/aws_fakes.go | 9 ++++++--- pkg/providers/v1/aws_test.go | 12 ++++++++++++ 2 files changed, 18 insertions(+), 3 deletions(-) diff --git a/pkg/providers/v1/aws_fakes.go b/pkg/providers/v1/aws_fakes.go index c685818cb7..001c6fd776 100644 --- a/pkg/providers/v1/aws_fakes.go +++ b/pkg/providers/v1/aws_fakes.go @@ -34,6 +34,7 @@ import ( elb "github.com/aws/aws-sdk-go-v2/service/elasticloadbalancing" elbtypes "github.com/aws/aws-sdk-go-v2/service/elasticloadbalancing/types" elbv2 "github.com/aws/aws-sdk-go-v2/service/elasticloadbalancingv2" + elbv2types "github.com/aws/aws-sdk-go-v2/service/elasticloadbalancingv2/types" "github.com/aws/aws-sdk-go-v2/service/kms" "k8s.io/klog/v2" @@ -681,7 +682,8 @@ func (e *FakeELB) ModifyLoadBalancerAttributes(ctx context.Context, input *elb.M // FakeELBV2 is a fake ELBV2 client used for testing type FakeELBV2 struct { - aws *FakeAWSServices + aws *FakeAWSServices + IpAddressType elbv2types.IpAddressType } // AddTags is not implemented but is required for interface conformance @@ -715,9 +717,10 @@ func (elb *FakeELBV2) SetSecurityGroups(ctx context.Context, input *elbv2.SetSec panic("Not implemented") } -// SetIpAddressType is not implemented but is required for interface conformance +// SetIpAddressType stores the given IpAddressType for later inspection and returns success. func (elb *FakeELBV2) SetIpAddressType(ctx context.Context, input *elbv2.SetIpAddressTypeInput, optFns ...func(*elbv2.Options)) (*elbv2.SetIpAddressTypeOutput, error) { - panic("Not implemented") + elb.IpAddressType = input.IpAddressType + return &elbv2.SetIpAddressTypeOutput{IpAddressType: input.IpAddressType}, nil } // ModifyLoadBalancerAttributes is not implemented but is required for interface conformance diff --git a/pkg/providers/v1/aws_test.go b/pkg/providers/v1/aws_test.go index 2fba37d8c4..e3320b14fc 100644 --- a/pkg/providers/v1/aws_test.go +++ b/pkg/providers/v1/aws_test.go @@ -2518,6 +2518,7 @@ func (m *MockedFakeELBV2) CreateLoadBalancer(ctx context.Context, input *elbv2.C LoadBalancerArn: aws.String(arn), LoadBalancerName: input.Name, Type: elbv2types.LoadBalancerTypeEnumNetwork, + IpAddressType: input.IpAddressType, VpcId: aws.String("vpc-abc123def456abc78"), AvailabilityZones: []elbv2types.AvailabilityZone{ { @@ -2588,6 +2589,17 @@ func (m *MockedFakeELBV2) DeleteLoadBalancer(ctx context.Context, input *elbv2.D panic("Not implemented") } +func (m *MockedFakeELBV2) SetIpAddressType(ctx context.Context, input *elbv2.SetIpAddressTypeInput, optFns ...func(*elbv2.Options)) (*elbv2.SetIpAddressTypeOutput, error) { + for _, lb := range m.LoadBalancers { + if aws.ToString(lb.LoadBalancerArn) == aws.ToString(input.LoadBalancerArn) { + lb.IpAddressType = input.IpAddressType + break + } + } + m.FakeELBV2.IpAddressType = input.IpAddressType + return &elbv2.SetIpAddressTypeOutput{IpAddressType: input.IpAddressType}, nil +} + func (m *MockedFakeELBV2) ModifyLoadBalancerAttributes(ctx context.Context, input *elbv2.ModifyLoadBalancerAttributesInput, optFns ...func(*elbv2.Options)) (*elbv2.ModifyLoadBalancerAttributesOutput, error) { attrMap, present := m.LoadBalancerAttributes[aws.ToString(input.LoadBalancerArn)] From 1dc0c0308f9ed9accac9f763d7ad03e36eb7bc84 Mon Sep 17 00:00:00 2001 From: Nolan Brubaker Date: Wed, 25 Mar 2026 15:21:25 -0400 Subject: [PATCH 7/7] Gracefully fall back to IPv4 for CLBs if able Signed-off-by: Nolan Brubaker --- pkg/providers/v1/aws.go | 2 +- pkg/providers/v1/aws_validations.go | 32 ++++++ pkg/providers/v1/aws_validations_test.go | 134 +++++++++++++++++++++++ 3 files changed, 167 insertions(+), 1 deletion(-) diff --git a/pkg/providers/v1/aws.go b/pkg/providers/v1/aws.go index eb6a478528..1bab22ca52 100644 --- a/pkg/providers/v1/aws.go +++ b/pkg/providers/v1/aws.go @@ -2389,7 +2389,7 @@ func (c *Cloud) EnsureLoadBalancer(ctx context.Context, clusterName string, apiS return nil, err } - if !isNLB(annotations) && serviceRequestsIPv6(apiService) { + if !isNLB(annotations) && !canFallbackToIPv4(apiService) { return nil, fmt.Errorf("classic load balancer for service %s does not support IPv6", apiService.Name) } diff --git a/pkg/providers/v1/aws_validations.go b/pkg/providers/v1/aws_validations.go index 42b710418c..ead393db3c 100644 --- a/pkg/providers/v1/aws_validations.go +++ b/pkg/providers/v1/aws_validations.go @@ -152,6 +152,38 @@ func validateServiceAnnotationTargetGroupAttributes(v *awsValidationInput) error return nil } +// canFallbackToIPv4 reports whether a Service can be provisioned as an IPv4-only Classic Load +// Balancer even when IPv6 appears in spec.ipFamilies. It returns true when the service's IP +// family policy allows an IPv4-only load balancer: +// - nil policy or SingleStack with only IPv4 (no IPv6 requested) +// - PreferDualStack (IPv4-only is an acceptable fallback) +// +// It returns false when the policy demands IPv6 participation: +// - SingleStack with IPv6 in ipFamilies +// - RequireDualStack (CLB cannot satisfy a dual-stack requirement) +func canFallbackToIPv4(service *v1.Service) bool { + if service == nil { + return true + } + + policy := service.Spec.IPFamilyPolicy + if policy == nil { + // Implicit SingleStack: acceptable only if no IPv6 family is present. + return !serviceRequestsIPv6(service) + } + + switch *policy { + case v1.IPFamilyPolicySingleStack: + return !serviceRequestsIPv6(service) + case v1.IPFamilyPolicyPreferDualStack: + return true + case v1.IPFamilyPolicyRequireDualStack: + return false + } + + return true +} + // validateIPFamilyInfo validates that a Service's IP Families and IP Family Policies are supported. // Special cases: // - Cannot have an IPv6, single stack service (AWS limitation) diff --git a/pkg/providers/v1/aws_validations_test.go b/pkg/providers/v1/aws_validations_test.go index 80834fec64..567656bbe4 100644 --- a/pkg/providers/v1/aws_validations_test.go +++ b/pkg/providers/v1/aws_validations_test.go @@ -565,6 +565,140 @@ func TestValidateServiceAnnotations(t *testing.T) { } } +func TestCanFallbackToIPv4(t *testing.T) { + singleStack := v1.IPFamilyPolicySingleStack + preferDualStack := v1.IPFamilyPolicyPreferDualStack + requireDualStack := v1.IPFamilyPolicyRequireDualStack + + tests := []struct { + name string + service *v1.Service + want bool + }{ + // nil service + { + name: "nil service", + service: nil, + want: true, + }, + + // nil policy (implicit SingleStack) + { + name: "nil policy, no families", + service: &v1.Service{Spec: v1.ServiceSpec{}}, + want: true, + }, + { + name: "nil policy, IPv4 only", + service: &v1.Service{Spec: v1.ServiceSpec{IPFamilies: []v1.IPFamily{v1.IPv4Protocol}}}, + want: true, + }, + { + name: "nil policy, IPv6 only", + service: &v1.Service{Spec: v1.ServiceSpec{IPFamilies: []v1.IPFamily{v1.IPv6Protocol}}}, + want: false, + }, + { + name: "nil policy, IPv4 then IPv6", + service: &v1.Service{Spec: v1.ServiceSpec{IPFamilies: []v1.IPFamily{v1.IPv4Protocol, v1.IPv6Protocol}}}, + want: false, + }, + { + name: "nil policy, IPv6 then IPv4", + service: &v1.Service{Spec: v1.ServiceSpec{IPFamilies: []v1.IPFamily{v1.IPv6Protocol, v1.IPv4Protocol}}}, + want: false, + }, + + // SingleStack + { + name: "SingleStack, no families", + service: &v1.Service{Spec: v1.ServiceSpec{IPFamilyPolicy: &singleStack}}, + want: true, + }, + { + name: "SingleStack, IPv4 only", + service: &v1.Service{Spec: v1.ServiceSpec{IPFamilyPolicy: &singleStack, IPFamilies: []v1.IPFamily{v1.IPv4Protocol}}}, + want: true, + }, + { + name: "SingleStack, IPv6 only", + service: &v1.Service{Spec: v1.ServiceSpec{IPFamilyPolicy: &singleStack, IPFamilies: []v1.IPFamily{v1.IPv6Protocol}}}, + want: false, + }, + { + name: "SingleStack, IPv4 then IPv6", + service: &v1.Service{Spec: v1.ServiceSpec{IPFamilyPolicy: &singleStack, IPFamilies: []v1.IPFamily{v1.IPv4Protocol, v1.IPv6Protocol}}}, + want: false, + }, + { + name: "SingleStack, IPv6 then IPv4", + service: &v1.Service{Spec: v1.ServiceSpec{IPFamilyPolicy: &singleStack, IPFamilies: []v1.IPFamily{v1.IPv6Protocol, v1.IPv4Protocol}}}, + want: false, + }, + + // PreferDualStack + { + name: "PreferDualStack, no families", + service: &v1.Service{Spec: v1.ServiceSpec{IPFamilyPolicy: &preferDualStack}}, + want: true, + }, + { + name: "PreferDualStack, IPv4 only", + service: &v1.Service{Spec: v1.ServiceSpec{IPFamilyPolicy: &preferDualStack, IPFamilies: []v1.IPFamily{v1.IPv4Protocol}}}, + want: true, + }, + { + name: "PreferDualStack, IPv6 only", + service: &v1.Service{Spec: v1.ServiceSpec{IPFamilyPolicy: &preferDualStack, IPFamilies: []v1.IPFamily{v1.IPv6Protocol}}}, + want: true, + }, + { + name: "PreferDualStack, IPv4 then IPv6", + service: &v1.Service{Spec: v1.ServiceSpec{IPFamilyPolicy: &preferDualStack, IPFamilies: []v1.IPFamily{v1.IPv4Protocol, v1.IPv6Protocol}}}, + want: true, + }, + { + name: "PreferDualStack, IPv6 then IPv4", + service: &v1.Service{Spec: v1.ServiceSpec{IPFamilyPolicy: &preferDualStack, IPFamilies: []v1.IPFamily{v1.IPv6Protocol, v1.IPv4Protocol}}}, + want: true, + }, + + // RequireDualStack + { + name: "RequireDualStack, no families", + service: &v1.Service{Spec: v1.ServiceSpec{IPFamilyPolicy: &requireDualStack}}, + want: false, + }, + { + name: "RequireDualStack, IPv4 only", + service: &v1.Service{Spec: v1.ServiceSpec{IPFamilyPolicy: &requireDualStack, IPFamilies: []v1.IPFamily{v1.IPv4Protocol}}}, + want: false, + }, + { + name: "RequireDualStack, IPv6 only", + service: &v1.Service{Spec: v1.ServiceSpec{IPFamilyPolicy: &requireDualStack, IPFamilies: []v1.IPFamily{v1.IPv6Protocol}}}, + want: false, + }, + { + name: "RequireDualStack, IPv4 then IPv6", + service: &v1.Service{Spec: v1.ServiceSpec{IPFamilyPolicy: &requireDualStack, IPFamilies: []v1.IPFamily{v1.IPv4Protocol, v1.IPv6Protocol}}}, + want: false, + }, + { + name: "RequireDualStack, IPv6 then IPv4", + service: &v1.Service{Spec: v1.ServiceSpec{IPFamilyPolicy: &requireDualStack, IPFamilies: []v1.IPFamily{v1.IPv6Protocol, v1.IPv4Protocol}}}, + want: false, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got := canFallbackToIPv4(tt.service) + assert.Equal(t, tt.want, got, "canFallbackToIPv4 mismatch for test case: %s", tt.name) + }) + } +} + func TestValidateIPFamilyInfo(t *testing.T) { tests := []struct { name string