Skip to content

Commit 74b2b3c

Browse files
wweiwei-liwweiwei-li
committed
Support BYOIP
1 parent 719a4cd commit 74b2b3c

File tree

4 files changed

+282
-13
lines changed

4 files changed

+282
-13
lines changed

‎pkg/deploy/aga/accelerator_manager.go‎

Lines changed: 34 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,8 @@ import (
44
"context"
55
"errors"
66
"fmt"
7+
"slices"
8+
79
"github.com/aws/aws-sdk-go-v2/aws"
810
awssdk "github.com/aws/aws-sdk-go-v2/aws"
911
"github.com/aws/aws-sdk-go-v2/service/globalaccelerator"
@@ -58,10 +60,10 @@ func (m *defaultAcceleratorManager) buildSDKCreateAcceleratorInput(_ context.Con
5860
IdempotencyToken: aws.String(idempotencyToken),
5961
}
6062

61-
//TODO: BYOIP feature
62-
//if len(resAccelerator.Spec.IpAddresses) > 0 {
63-
// createInput.IpAddresses = resAccelerator.Spec.IpAddresses
64-
//}
63+
// BYOIP feature: Set IP addresses if provided
64+
if len(resAccelerator.Spec.IpAddresses) > 0 {
65+
createInput.IpAddresses = resAccelerator.Spec.IpAddresses
66+
}
6567

6668
// Add tags
6769
tags := m.trackingProvider.ResourceTags(resAccelerator.Stack(), resAccelerator, resAccelerator.Spec.Tags)
@@ -102,10 +104,11 @@ func (m *defaultAcceleratorManager) buildSDKUpdateAcceleratorInput(ctx context.C
102104
Enabled: resAccelerator.Spec.Enabled,
103105
}
104106

105-
//TODO: BYOIP feature
106-
//if len(resAccelerator.Spec.IpAddresses) > 0 {
107-
// updateInput.IpAddresses = resAccelerator.Spec.IpAddresses
108-
//}
107+
// BYOIP feature: Include IP addresses for update when specified
108+
if len(resAccelerator.Spec.IpAddresses) > 0 {
109+
updateInput.IpAddresses = resAccelerator.Spec.IpAddresses
110+
}
111+
109112
return updateInput
110113
}
111114

@@ -240,7 +243,11 @@ func (m *defaultAcceleratorManager) isSDKAcceleratorSettingsDrifted(resAccelerat
240243
return true
241244
}
242245

243-
//TODO : BYOIP feature
246+
// Check if BYOIP differs, empty desired IPs means "don't update the IPs"
247+
if len(resAccelerator.Spec.IpAddresses) > 0 && !m.areIPAddressesEqual(resAccelerator.Spec.IpAddresses, sdkAccelerator.Accelerator.IpSets) {
248+
return true
249+
}
250+
244251
return false
245252
}
246253

@@ -272,3 +279,21 @@ func (m *defaultAcceleratorManager) buildAcceleratorStatus(accelerator *agatypes
272279

273280
return status
274281
}
282+
283+
func (m *defaultAcceleratorManager) areIPAddressesEqual(desiredIPs []string, actualIPSets []agatypes.IpSet) bool {
284+
285+
// IPv6 BYOIP is not supported at this time
286+
return m.areIPv4AddressesEqual(desiredIPs, actualIPSets)
287+
}
288+
289+
// areIPv4AddressesEqual compares desired IPv4 addresses with actual IP sets from AWS
290+
func (m *defaultAcceleratorManager) areIPv4AddressesEqual(desiredIPs []string, actualIPSets []agatypes.IpSet) bool {
291+
actualIPv4s := extractIPv4Addresses(actualIPSets)
292+
if len(desiredIPs) != len(actualIPv4s) {
293+
return false
294+
}
295+
296+
slices.Sort(desiredIPs)
297+
slices.Sort(actualIPv4s)
298+
return slices.Equal(desiredIPs, actualIPv4s)
299+
}

‎pkg/deploy/aga/accelerator_manager_test.go‎

Lines changed: 86 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,11 +3,13 @@ package aga
33
import (
44
"context"
55
"errors"
6-
"k8s.io/apimachinery/pkg/types"
76
"testing"
87

8+
"k8s.io/apimachinery/pkg/types"
9+
910
"github.com/aws/aws-sdk-go-v2/aws"
1011
"github.com/aws/aws-sdk-go-v2/service/globalaccelerator"
12+
agatypes "github.com/aws/aws-sdk-go-v2/service/globalaccelerator/types"
1113
gatypes "github.com/aws/aws-sdk-go-v2/service/globalaccelerator/types"
1214
"github.com/go-logr/logr"
1315
"github.com/golang/mock/gomock"
@@ -721,3 +723,86 @@ func Test_defaultAcceleratorManager_disableAccelerator(t *testing.T) {
721723
})
722724
}
723725
}
726+
727+
func Test_defaultAcceleratorManager_areIPv4AddressesEqual(t *testing.T) {
728+
tests := []struct {
729+
name string
730+
desiredIPs []string
731+
actualIPSets []agatypes.IpSet
732+
expectedResult bool
733+
description string
734+
}{
735+
{
736+
name: "Two BYOIPs match current - no drift",
737+
desiredIPs: []string{"169.254.8.13", "169.254.8.14"},
738+
actualIPSets: makeIPSets([]string{"169.254.8.13", "169.254.8.14"}),
739+
expectedResult: true,
740+
description: "Both BYOIPs match exactly",
741+
},
742+
{
743+
name: "One BYOIP matches current - drift",
744+
desiredIPs: []string{"169.254.8.13, 169.254.9.13"},
745+
actualIPSets: makeIPSets([]string{"169.254.8.13", "99.82.158.217"}),
746+
expectedResult: false,
747+
description: "BYOIP present, Amazon auto-assigned the other",
748+
},
749+
{
750+
name: "One BYOIP missing from current - drift detected",
751+
desiredIPs: []string{"169.254.9.13"},
752+
actualIPSets: makeIPSets([]string{"169.254.8.13", "99.82.158.217"}),
753+
expectedResult: false,
754+
description: "Desired BYOIP not in current",
755+
},
756+
{
757+
name: "Changing from one BYOIP to another - drift detected",
758+
desiredIPs: []string{"169.254.8.16"},
759+
actualIPSets: makeIPSets([]string{"169.254.9.13", "99.82.158.217"}),
760+
expectedResult: false,
761+
description: "Different BYOIP, count matches but IP doesn't",
762+
},
763+
{
764+
name: "Two BYOIPs in different order - no drift",
765+
desiredIPs: []string{"169.254.8.14", "169.254.8.13"},
766+
actualIPSets: makeIPSets([]string{"169.254.8.13", "169.254.8.14"}),
767+
expectedResult: true,
768+
description: "Order doesn't matter for exact match",
769+
},
770+
}
771+
772+
for _, tt := range tests {
773+
t.Run(tt.name, func(t *testing.T) {
774+
m := &defaultAcceleratorManager{
775+
logger: logr.Discard(),
776+
}
777+
result := m.areIPv4AddressesEqual(tt.desiredIPs, tt.actualIPSets)
778+
assert.Equal(t, tt.expectedResult, result, tt.description)
779+
})
780+
}
781+
}
782+
783+
// Helper function to create IPSets from IP strings
784+
func makeIPSets(ips []string) []agatypes.IpSet {
785+
if len(ips) == 0 {
786+
return []agatypes.IpSet{}
787+
}
788+
return []agatypes.IpSet{
789+
{
790+
IpAddressFamily: "IPv4",
791+
IpAddresses: ips,
792+
},
793+
}
794+
}
795+
796+
// Helper function to create dualstack IPSets with both IPv4 and IPv6
797+
func makeDualStackIPSets(ipv4s []string, ipv6s []string) []agatypes.IpSet {
798+
return []agatypes.IpSet{
799+
{
800+
IpAddressFamily: "IPv4",
801+
IpAddresses: ipv4s,
802+
},
803+
{
804+
IpAddressFamily: "IPv6",
805+
IpAddresses: ipv6s,
806+
},
807+
}
808+
}

‎pkg/deploy/aga/accelerator_synthesizer.go‎

Lines changed: 59 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,8 @@ package aga
22

33
import (
44
"context"
5+
"net"
6+
57
awssdk "github.com/aws/aws-sdk-go-v2/aws"
68
"github.com/aws/aws-sdk-go-v2/service/globalaccelerator"
79
agatypes "github.com/aws/aws-sdk-go-v2/service/globalaccelerator/types"
@@ -72,8 +74,21 @@ func (s *acceleratorSynthesizer) Synthesize(ctx context.Context) error {
7274

7375
// Accelerator exists, determine if it needs replacement or update
7476
if isSDKAcceleratorRequiresReplacement(sdkAccelerator, resAccelerator) {
77+
currentIPv4s := extractIPv4Addresses(sdkAccelerator.Accelerator.IpSets)
78+
desiredBYOIPs := resAccelerator.Spec.IpAddresses
79+
if len(desiredBYOIPs) == 1 {
80+
s.logger.Info("Accelerator requires replacement: BYOIP CIDR change",
81+
"acceleratorARN", *sdkAccelerator.Accelerator.AcceleratorArn,
82+
"currentIPv4s", currentIPv4s,
83+
"desiredBYOIPv4s", desiredBYOIPs[0],
84+
"note", "Amazon will auto-assign 1 additional IPv4 address")
85+
} else {
86+
s.logger.Info("Accelerator requires replacement: BYOIP CIDR change",
87+
"acceleratorARN", *sdkAccelerator.Accelerator.AcceleratorArn,
88+
"currentIPv4s", currentIPv4s,
89+
"desiredBYOIPv4s", desiredBYOIPs)
90+
}
7591
// Store for deletion in PostSynthesize, then recreate
76-
// TODO: We will test this for BYOIP feature
7792
s.unmatchedSDKAccelerators = []AcceleratorWithTags{sdkAccelerator}
7893
return s.handleCreateAccelerator(ctx, resAccelerator)
7994
} else {
@@ -183,7 +198,48 @@ func (s *acceleratorSynthesizer) isAcceleratorNotFound(err error) bool {
183198

184199
// isSDKAcceleratorRequiresReplacement checks whether a sdk Accelerator requires replacement to fulfill an Accelerator resource.
185200
func isSDKAcceleratorRequiresReplacement(sdkAccelerator AcceleratorWithTags, resAccelerator *agamodel.Accelerator) bool {
186-
// The accelerator will only need replacement in BYOIP scenarios. I will implement this later as a separate PR
187-
// TODO : BYOIP feature
201+
if len(resAccelerator.Spec.IpAddresses) == 0 {
202+
return false
203+
}
204+
205+
hasSharedCIDR := HasSharedIPv4CIDR(sdkAccelerator.Accelerator.IpSets, resAccelerator.Spec.IpAddresses)
206+
207+
// If no BYOIP CIDR is shared between current and desired. require replacement.
208+
return !hasSharedCIDR
209+
}
210+
211+
// extractIPv4Addresses extracts IPv4 addresses from IPSets
212+
func extractIPv4Addresses(ipSets []agatypes.IpSet) []string {
213+
ips := make([]string, 0)
214+
for _, ipSet := range ipSets {
215+
if ipSet.IpAddressFamily == "IPv4" {
216+
ips = append(ips, ipSet.IpAddresses...)
217+
}
218+
}
219+
return ips
220+
}
221+
222+
// HasSharedIPv4CIDR checks if current and desired IPs share any IPv4 CIDR block.
223+
func HasSharedIPv4CIDR(currentIPSets []agatypes.IpSet, desiredIPs []string) bool {
224+
if len(desiredIPs) == 0 {
225+
return true
226+
}
227+
228+
currentIPs := extractIPv4Addresses(currentIPSets)
229+
230+
for _, currentIP := range currentIPs {
231+
currentIPv4 := net.ParseIP(currentIP).To4()
232+
233+
for _, desiredIP := range desiredIPs {
234+
desiredIPv4 := net.ParseIP(desiredIP).To4()
235+
236+
// Compare first 3 octets (same /24 CIDR)
237+
if currentIPv4[0] == desiredIPv4[0] &&
238+
currentIPv4[1] == desiredIPv4[1] &&
239+
currentIPv4[2] == desiredIPv4[2] {
240+
return true
241+
}
242+
}
243+
}
188244
return false
189245
}

‎pkg/deploy/aga/accelerator_synthesizer_test.go‎

Lines changed: 103 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -648,3 +648,106 @@ func Test_acceleratorSynthesizer_handleUpdateAccelerator(t *testing.T) {
648648
})
649649
}
650650
}
651+
652+
func Test_HasSharedIPv4CIDR(t *testing.T) {
653+
tests := []struct {
654+
name string
655+
currentIPSets []agatypes.IpSet
656+
desiredIPs []string
657+
expectedResult bool
658+
description string
659+
}{
660+
{
661+
name: "No desired IPs - has shared CIDR",
662+
currentIPSets: makeIPSets([]string{"1.1.1.1", "2.2.2.2"}),
663+
desiredIPs: []string{},
664+
expectedResult: true,
665+
description: "Empty desired IPs means no change",
666+
},
667+
668+
{
669+
name: "Same IPs - has shared CIDR",
670+
currentIPSets: makeIPSets([]string{"1.1.1.1", "2.2.2.2"}),
671+
desiredIPs: []string{"1.1.1.1", "2.2.2.2"},
672+
expectedResult: true,
673+
description: "Shared IPv4 CIDRs: 1.1.1.0/24, 2.2.2.0/24",
674+
},
675+
{
676+
name: "Different IPs same CIDR - has shared CIDR",
677+
currentIPSets: makeIPSets([]string{"1.1.1.1", "2.2.2.2"}),
678+
desiredIPs: []string{"1.1.1.3", "2.2.2.4"},
679+
expectedResult: true,
680+
description: "Shared IPv4 CIDRs: 1.1.1.0/24, 2.2.2.0/24",
681+
},
682+
{
683+
name: "Different CIDRs - no shared CIDR",
684+
currentIPSets: makeIPSets([]string{"1.1.1.1", "2.2.2.2"}),
685+
desiredIPs: []string{"3.3.3.1", "4.4.4.2"},
686+
expectedResult: false,
687+
description: "No shared IPv4 CIDR (1.1.1.0/24, 2.2.2.0/24 vs 3.3.3.0/24, 4.4.4.0/24)",
688+
},
689+
{
690+
name: "Partial IP change same CIDR - has shared CIDR",
691+
currentIPSets: makeIPSets([]string{"1.1.1.1", "2.2.2.2"}),
692+
desiredIPs: []string{"1.1.1.3", "2.2.2.4"},
693+
expectedResult: true,
694+
description: "Shared IPv4 CIDRs: 1.1.1.0/24, 2.2.2.0/24",
695+
},
696+
{
697+
name: "Add new CIDR - has shared CIDR",
698+
currentIPSets: makeIPSets([]string{"1.1.1.1", "2.2.2.2"}),
699+
desiredIPs: []string{"1.1.1.3", "3.3.3.3"},
700+
expectedResult: true,
701+
description: "Shared IPv4 CIDR: 1.1.1.0/24",
702+
},
703+
{
704+
name: "Replace all with new CIDRs - no shared CIDR",
705+
currentIPSets: makeIPSets([]string{"1.1.1.1", "2.2.2.2"}),
706+
desiredIPs: []string{"3.3.3.1", "4.4.4.2"},
707+
expectedResult: false,
708+
description: "No shared IPv4 CIDR (1.1.1.0/24, 2.2.2.0/24 vs 3.3.3.0/24, 4.4.4.0/24)",
709+
},
710+
{
711+
name: "Multiple CIDRs no overlap - no shared CIDR",
712+
currentIPSets: makeIPSets([]string{"1.1.1.1", "2.2.2.2"}),
713+
desiredIPs: []string{"3.3.3.3", "4.4.4.4"},
714+
expectedResult: false,
715+
description: "No shared IPv4 CIDR",
716+
},
717+
{
718+
name: "Both have two IPs from different CIDRs with overlap",
719+
currentIPSets: makeIPSets([]string{"1.1.1.1", "2.2.2.2"}),
720+
desiredIPs: []string{"1.1.1.3", "3.3.3.3"},
721+
expectedResult: true,
722+
description: "Shared IPv4 CIDR: 1.1.1.0/24",
723+
},
724+
{
725+
name: "Partial CIDR overlap - has shared CIDR",
726+
currentIPSets: makeIPSets([]string{"1.1.1.1", "2.2.2.2"}),
727+
desiredIPs: []string{"1.1.1.2", "3.3.3.3"},
728+
expectedResult: true,
729+
description: "Shared IPv4 CIDR: 1.1.1.0/24",
730+
},
731+
{
732+
name: "Dualstack with IPv6 - has shared CIDR",
733+
currentIPSets: makeDualStackIPSets([]string{"1.1.1.1", "2.2.2.2"}, []string{"2600:9000::1", "2600:9000::2"}),
734+
desiredIPs: []string{"1.1.1.3", "3.3.3.3"},
735+
expectedResult: true,
736+
description: "IPv6 ignored, shared IPv4 CIDR: 1.1.1.0/24",
737+
},
738+
{
739+
name: "Dualstack different CIDR - no shared CIDR",
740+
currentIPSets: makeDualStackIPSets([]string{"1.1.1.1", "2.2.2.2"}, []string{"2600:9000::1", "2600:9000::2"}),
741+
desiredIPs: []string{"3.3.3.1", "4.4.4.2"},
742+
expectedResult: false,
743+
description: "IPv6 ignored, no shared IPv4 CIDR",
744+
},
745+
}
746+
747+
for _, tt := range tests {
748+
t.Run(tt.name, func(t *testing.T) {
749+
result := HasSharedIPv4CIDR(tt.currentIPSets, tt.desiredIPs)
750+
assert.Equal(t, tt.expectedResult, result, tt.description)
751+
})
752+
}
753+
}

0 commit comments

Comments
 (0)