forked from kubernetes/cloud-provider-aws
-
Notifications
You must be signed in to change notification settings - Fork 25
OCPCLOUD-3215: Add support for dual networking stack services #135
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Merged
openshift-merge-bot
merged 6 commits into
openshift:main
from
nrb:OCPCLOUD-3215-dual-stack
Mar 27, 2026
Merged
Changes from all commits
Commits
Show all changes
6 commits
Select commit
Hold shift + click to select a range
0e32dfc
UPSTREAM: 1313: Add support for dual networking stack services
nrb 4572a28
UPSTREAM: 1313: Support IP address type changes on update
nrb d0d2ea5
UPSTREAM: 1313: Hash target group names with IP address family
nrb 1abf771
UPSTREAM: 1313: Take IPv6 CIDRs into account for all open traffic
nrb e0139d8
UPSTREAM: 1313: fixup! Support IP address type changes on update
nrb 412a5fa
UPSTREAM: 1313: Gracefully fall back to IPv4 for CLBs if able
nrb File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -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) | ||
|
|
@@ -2001,10 +2002,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 +2016,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 +2119,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,34 +2290,74 @@ 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. | ||
| // | ||
| // 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 +2380,10 @@ func (c *Cloud) EnsureLoadBalancer(ctx context.Context, clusterName string, apiS | |
| return nil, err | ||
| } | ||
|
|
||
| if !isNLB(annotations) && !canFallbackToIPv4(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 +2404,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 +2517,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 +2549,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") | ||
|
Comment on lines
+2554
to
+2555
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Same question as above ☝️ |
||
| } | ||
|
|
||
| err = c.updateInstanceSecurityGroupsForNLB(ctx, loadBalancerName, instances, subnetCidrs, sourceRangeCidrs, v2Mappings) | ||
|
|
@@ -2629,6 +2699,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 +2715,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 | ||
| } | ||
| } | ||
|
|
||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I guess this block will always force
::/0even if the user defines a restricted set of allowed CIDRs via:service.spec.loadBalancerSourceRangesservice.beta.kubernetes.io/load-balancer-source-rangesReference: https://github.com/kubernetes/cloud-provider/blob/2e08bbe9b7f1623207c046a892270baf27599b84/service/helpers/helper.go#L61-L85
I think
!contains(sourceCIDRs, "::/0")should be replaced withthere is no defined allowed IPv6 CIDR, right?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe, we can:
And check if
len(ec2Ipv6SourceRanges) == 0here, then add::/0.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, these changes should go in. Will incorporate into later work.