From 643f33d6b30849e4aaf2a6e7d50003e56435a1ad Mon Sep 17 00:00:00 2001 From: Weiwei Li Date: Sun, 15 Dec 2024 23:42:41 -0800 Subject: [PATCH] fix: Cannot set the IPv6 addresses in dualstack mode during modification --- pkg/deploy/elbv2/load_balancer_manager.go | 24 ++++++++- .../elbv2/load_balancer_manager_test.go | 51 +++++++++++++++++++ 2 files changed, 74 insertions(+), 1 deletion(-) diff --git a/pkg/deploy/elbv2/load_balancer_manager.go b/pkg/deploy/elbv2/load_balancer_manager.go index 742a987eb..bc76c45aa 100644 --- a/pkg/deploy/elbv2/load_balancer_manager.go +++ b/pkg/deploy/elbv2/load_balancer_manager.go @@ -158,6 +158,7 @@ func (m *defaultLoadBalancerManager) updateSDKLoadBalancerWithIPAddressType(ctx func (m *defaultLoadBalancerManager) updateSDKLoadBalancerWithSubnetMappings(ctx context.Context, resLB *elbv2model.LoadBalancer, sdkLB LoadBalancerWithTags) error { desiredSubnets := sets.NewString() + desiredIPv6Addresses := sets.NewString() desiredSubnetsSourceNATPrefixes := sets.NewString() currentSubnetsSourceNATPrefixes := sets.NewString() for _, mapping := range resLB.Spec.SubnetMappings { @@ -165,13 +166,20 @@ func (m *defaultLoadBalancerManager) updateSDKLoadBalancerWithSubnetMappings(ctx if mapping.SourceNatIpv6Prefix != nil { desiredSubnetsSourceNATPrefixes.Insert(awssdk.ToString(mapping.SourceNatIpv6Prefix)) } + if mapping.IPv6Address != nil { + desiredIPv6Addresses.Insert(awssdk.ToString(mapping.IPv6Address)) + } } currentSubnets := sets.NewString() + currentIPv6Addresses := sets.NewString() for _, az := range sdkLB.LoadBalancer.AvailabilityZones { currentSubnets.Insert(awssdk.ToString(az.SubnetId)) if len(az.SourceNatIpv6Prefixes) != 0 { currentSubnetsSourceNATPrefixes.Insert(az.SourceNatIpv6Prefixes[0]) } + if len(az.LoadBalancerAddresses) > 0 && az.LoadBalancerAddresses[0].IPv6Address != nil { + currentIPv6Addresses.Insert(awssdk.ToString(az.LoadBalancerAddresses[0].IPv6Address)) + } } sdkLBEnablePrefixForIpv6SourceNatValue := string(elbv2model.EnablePrefixForIpv6SourceNatOff) resLBEnablePrefixForIpv6SourceNatValue := string(elbv2model.EnablePrefixForIpv6SourceNatOff) @@ -180,7 +188,9 @@ func (m *defaultLoadBalancerManager) updateSDKLoadBalancerWithSubnetMappings(ctx resLBEnablePrefixForIpv6SourceNatValue = string(resLB.Spec.EnablePrefixForIpv6SourceNat) - if desiredSubnets.Equal(currentSubnets) && desiredSubnetsSourceNATPrefixes.Equal(currentSubnetsSourceNATPrefixes) && ((sdkLBEnablePrefixForIpv6SourceNatValue == resLBEnablePrefixForIpv6SourceNatValue) || (resLBEnablePrefixForIpv6SourceNatValue == "")) { + isFirstTimeIPv6Setup := currentIPv6Addresses.Len() == 0 && desiredIPv6Addresses.Len() > 0 + needsDualstackIPv6Update := isIPv4ToDualstackUpdate(resLB, sdkLB) && isFirstTimeIPv6Setup + if !needsDualstackIPv6Update && desiredSubnets.Equal(currentSubnets) && desiredSubnetsSourceNATPrefixes.Equal(currentSubnetsSourceNATPrefixes) && ((sdkLBEnablePrefixForIpv6SourceNatValue == resLBEnablePrefixForIpv6SourceNatValue) || (resLBEnablePrefixForIpv6SourceNatValue == "")) { return nil } req := &elbv2sdk.SetSubnetsInput{ @@ -355,3 +365,15 @@ func isEnforceSGInboundRulesOnPrivateLinkUpdated(resLB *elbv2model.LoadBalancer, return true, currentEnforceSecurityGroupInboundRulesOnPrivateLinkTraffic, desiredEnforceSecurityGroupInboundRulesOnPrivateLinkTraffic } + +func isIPv4ToDualstackUpdate(resLB *elbv2model.LoadBalancer, sdkLB LoadBalancerWithTags) bool { + if &resLB.Spec.IPAddressType == nil { + return false + } + desiredIPAddressType := string(resLB.Spec.IPAddressType) + currentIPAddressType := sdkLB.LoadBalancer.IpAddressType + isIPAddressTypeUpdated := desiredIPAddressType != string(currentIPAddressType) + return isIPAddressTypeUpdated && + resLB.Spec.Type == elbv2model.LoadBalancerTypeNetwork && + desiredIPAddressType == string(elbv2model.IPAddressTypeDualStack) +} diff --git a/pkg/deploy/elbv2/load_balancer_manager_test.go b/pkg/deploy/elbv2/load_balancer_manager_test.go index 520dc9e85..e2b8567d1 100644 --- a/pkg/deploy/elbv2/load_balancer_manager_test.go +++ b/pkg/deploy/elbv2/load_balancer_manager_test.go @@ -5,6 +5,7 @@ import ( "testing" elbv2types "github.com/aws/aws-sdk-go-v2/service/elasticloadbalancingv2/types" + "github.com/aws/aws-sdk-go/aws" "github.com/go-logr/logr" "sigs.k8s.io/aws-load-balancer-controller/pkg/aws/services" @@ -612,6 +613,55 @@ func Test_defaultLoadBalancerManager_updateSDKLoadBalancerWithSubnetMappings(t * }, wantErr: nil, }, + { + name: "Set NLB IPv6 address in dualstack mode during ip address type modification ", + fields: fields{ + setSubnetsWithContextCall: setSubnetsWithContextCall{ + req: &elbv2sdk.SetSubnetsInput{ + LoadBalancerArn: awssdk.String("LoadBalancerArn"), + SubnetMappings: []elbv2types.SubnetMapping{ + { + SubnetId: awssdk.String("subnet-A"), + IPv6Address: aws.String("2600:1f18::1"), + }, + { + SubnetId: awssdk.String("subnet-B"), + IPv6Address: aws.String("2600:1f18::2"), + }, + }, + }, + resp: &elbv2sdk.SetSubnetsOutput{}, + }, + }, + args: args{ + resLB: &elbv2model.LoadBalancer{ + ResourceMeta: coremodel.NewResourceMeta(stack, "AWS::ElasticLoadBalancingV2::LoadBalancer", "id-1"), + Spec: elbv2model.LoadBalancerSpec{ + SubnetMappings: []elbv2model.SubnetMapping{ + { + SubnetID: "subnet-A", + IPv6Address: aws.String("2600:1f18::1"), + }, + { + SubnetID: "subnet-B", + IPv6Address: aws.String("2600:1f18::2"), + }, + }, + Type: elbv2model.LoadBalancerTypeNetwork, + IPAddressType: elbv2model.IPAddressTypeDualStack, + }, + }, + sdkLB: LoadBalancerWithTags{ + LoadBalancer: &elbv2types.LoadBalancer{ + LoadBalancerArn: awssdk.String("LoadBalancerArn"), + Type: elbv2types.LoadBalancerTypeEnumNetwork, + AvailabilityZones: []elbv2types.AvailabilityZone{{SubnetId: awssdk.String("subnet-A")}, {SubnetId: awssdk.String("subnet-B")}}, + IpAddressType: elbv2types.IpAddressTypeIpv4, + }, + }, + }, + wantErr: nil, + }, } for _, tt := range tests { @@ -632,6 +682,7 @@ func Test_defaultLoadBalancerManager_updateSDKLoadBalancerWithSubnetMappings(t * } else { assert.NoError(t, err) } + }) } }