Skip to content

Commit aae1a15

Browse files
komer3komer3
committed
Refactor frontend VPC annotation precedence and validation logic
- Change annotation precedence order: subnet-id first, then vpc-name/subnet-name, then optional IP ranges - Consolidate IPv4/IPv6 validation into single validateNodeBalancerFrontendIPRange function - Simplify getFrontendVPCCreateOptions to reduce code duplication and improve readability - Make IP range annotations optional when subnet is specified - Add error when IP ranges are provided without subnet selector - Update tests to cover more senarios
1 parent 7ef297b commit aae1a15

File tree

3 files changed

+192
-114
lines changed

3 files changed

+192
-114
lines changed

‎cloud/linode/loadbalancers.go‎

Lines changed: 29 additions & 75 deletions
Original file line numberDiff line numberDiff line change
@@ -850,86 +850,55 @@ func (l *loadbalancers) getVPCCreateOptions(ctx context.Context, service *v1.Ser
850850

851851
// getFrontendVPCCreateOptions returns the VPC options for the NodeBalancer frontend VPC creation.
852852
// Order of precedence:
853-
// 1. Frontend IPv4/IPv6 Range Annotations - Explicit CIDR ranges
853+
// 1. Frontend Subnet ID Annotation - Direct subnet ID
854854
// 2. Frontend VPC/Subnet Name Annotations - Resolve by name
855-
// 3. Frontend Subnet ID Annotation - Direct subnet ID
855+
// 3. Frontend IPv4/IPv6 Range Annotations - Optional CIDR ranges
856856
func (l *loadbalancers) getFrontendVPCCreateOptions(ctx context.Context, service *v1.Service) ([]linodego.NodeBalancerVPCOptions, error) {
857857
frontendIPv4Range, hasIPv4Range := service.GetAnnotations()[annotations.NodeBalancerFrontendIPv4Range]
858858
frontendIPv6Range, hasIPv6Range := service.GetAnnotations()[annotations.NodeBalancerFrontendIPv6Range]
859859
_, hasVPCName := service.GetAnnotations()[annotations.NodeBalancerFrontendVPCName]
860860
_, hasSubnetName := service.GetAnnotations()[annotations.NodeBalancerFrontendSubnetName]
861861
_, hasSubnetID := service.GetAnnotations()[annotations.NodeBalancerFrontendSubnetID]
862862

863-
// If no frontend VPC annotations are present, return empty slice
863+
// If no frontend VPC annotations are present, do not configure a frontend VPC.
864864
if !hasIPv4Range && !hasIPv6Range && !hasVPCName && !hasSubnetName && !hasSubnetID {
865865
return nil, nil
866866
}
867867

868-
var subnetID int
869-
var err error
870-
871-
// Precedence 1: IPv4/IPv6 Range Annotations - Explicit CIDR ranges
872-
if hasIPv4Range || hasIPv6Range {
873-
if err = validateNodeBalancerFrontendIPv4Range(frontendIPv4Range); err != nil {
874-
return nil, err
875-
}
876-
if err = validateNodeBalancerFrontendIPv6Range(frontendIPv6Range); err != nil {
877-
return nil, err
878-
}
879-
if frontendSubnetID, ok := service.GetAnnotations()[annotations.NodeBalancerFrontendSubnetID]; ok {
880-
subnetID, err = strconv.Atoi(frontendSubnetID)
881-
if err != nil {
882-
return nil, fmt.Errorf("invalid frontend subnet ID: %w", err)
883-
}
884-
} else {
885-
subnetID, err = l.getFrontendSubnetIDForSVC(ctx, service)
886-
if err != nil {
887-
return nil, err
888-
}
889-
}
890-
891-
vpcCreateOpts := []linodego.NodeBalancerVPCOptions{
892-
{
893-
SubnetID: subnetID,
894-
IPv4Range: frontendIPv4Range,
895-
IPv6Range: frontendIPv6Range,
896-
},
897-
}
898-
return vpcCreateOpts, nil
868+
if err := validateNodeBalancerFrontendIPRange(frontendIPv4Range, "IPv4"); err != nil {
869+
return nil, err
899870
}
900-
901-
// Precedence 2: VPC/Subnet Name Annotations - Resolve by name
902-
if hasVPCName || hasSubnetName {
903-
subnetID, err = l.getFrontendSubnetIDForSVC(ctx, service)
904-
if err != nil {
905-
return nil, err
906-
}
907-
908-
vpcCreateOpts := []linodego.NodeBalancerVPCOptions{
909-
{
910-
SubnetID: subnetID,
911-
},
912-
}
913-
return vpcCreateOpts, nil
871+
if err := validateNodeBalancerFrontendIPRange(frontendIPv6Range, "IPv6"); err != nil {
872+
return nil, err
914873
}
915874

916-
// Precedence 3: Subnet ID Annotation - Direct subnet ID
875+
var subnetID int
876+
var err error
877+
917878
if hasSubnetID {
918879
frontendSubnetID := service.GetAnnotations()[annotations.NodeBalancerFrontendSubnetID]
919880
subnetID, err = strconv.Atoi(frontendSubnetID)
920881
if err != nil {
921882
return nil, fmt.Errorf("invalid frontend subnet ID: %w", err)
922883
}
923-
924-
vpcCreateOpts := []linodego.NodeBalancerVPCOptions{
925-
{
926-
SubnetID: subnetID,
927-
},
884+
} else if hasVPCName || hasSubnetName {
885+
subnetID, err = l.getFrontendSubnetIDForSVC(ctx, service)
886+
if err != nil {
887+
return nil, err
928888
}
929-
return vpcCreateOpts, nil
889+
} else {
890+
// Ranges are optional but still require a subnet to target.
891+
return nil, fmt.Errorf("frontend VPC configuration requires either subnet-id or both vpc-name and subnet-name annotations")
930892
}
931893

932-
return nil, nil
894+
vpcCreateOpts := []linodego.NodeBalancerVPCOptions{
895+
{
896+
SubnetID: subnetID,
897+
IPv4Range: frontendIPv4Range,
898+
IPv6Range: frontendIPv6Range,
899+
},
900+
}
901+
return vpcCreateOpts, nil
933902
}
934903

935904
// getFrontendSubnetIDForSVC returns the subnet ID for the frontend VPC configuration.
@@ -1540,28 +1509,13 @@ func validateNodeBalancerBackendIPv4Range(backendIPv4Range string) error {
15401509
return nil
15411510
}
15421511

1543-
// validateNodeBalancerFrontendIPv4Range validates the frontend IPv4 range annotation.
1544-
// Performs basic CIDR format validation.
1545-
func validateNodeBalancerFrontendIPv4Range(frontendIPv4Range string) error {
1546-
if frontendIPv4Range == "" {
1547-
return nil
1548-
}
1549-
_, _, err := net.ParseCIDR(frontendIPv4Range)
1550-
if err != nil {
1551-
return fmt.Errorf("invalid frontend IPv4 range '%s': %w", frontendIPv4Range, err)
1552-
}
1553-
return nil
1554-
}
1555-
1556-
// validateNodeBalancerFrontendIPv6Range validates the frontend IPv6 range annotation.
1557-
// Performs basic CIDR format validation.
1558-
func validateNodeBalancerFrontendIPv6Range(frontendIPv6Range string) error {
1559-
if frontendIPv6Range == "" {
1512+
func validateNodeBalancerFrontendIPRange(frontendIPRange, ipVersion string) error {
1513+
if frontendIPRange == "" {
15601514
return nil
15611515
}
1562-
_, _, err := net.ParseCIDR(frontendIPv6Range)
1516+
_, _, err := net.ParseCIDR(frontendIPRange)
15631517
if err != nil {
1564-
return fmt.Errorf("invalid frontend IPv6 range '%s': %w", frontendIPv6Range, err)
1518+
return fmt.Errorf("invalid frontend %s range '%s': %w", ipVersion, frontendIPRange, err)
15651519
}
15661520
return nil
15671521
}

‎cloud/linode/loadbalancers_test.go‎

Lines changed: 119 additions & 39 deletions
Original file line numberDiff line numberDiff line change
@@ -5433,9 +5433,10 @@ func Test_validateNodeBalancerBackendIPv4Range(t *testing.T) {
54335433
}
54345434
}
54355435

5436-
func Test_validateNodeBalancerFrontendIPv4Range(t *testing.T) {
5436+
func Test_validateNodeBalancerFrontendIPRange(t *testing.T) {
54375437
type args struct {
5438-
frontendIPv4Range string
5438+
frontendIPRange string
5439+
ipVersion string
54395440
}
54405441
tests := []struct {
54415442
name string
@@ -5444,70 +5445,50 @@ func Test_validateNodeBalancerFrontendIPv4Range(t *testing.T) {
54445445
}{
54455446
{
54465447
name: "Valid IPv4 range",
5447-
args: args{frontendIPv4Range: "10.100.5.0/24"},
5448+
args: args{frontendIPRange: "10.100.5.0/24", ipVersion: "IPv4"},
54485449
wantErr: false,
54495450
},
54505451
{
54515452
name: "Invalid IPv4 range - no CIDR",
5452-
args: args{frontendIPv4Range: "10.100.5.0"},
5453+
args: args{frontendIPRange: "10.100.5.0", ipVersion: "IPv4"},
54535454
wantErr: true,
54545455
},
54555456
{
54565457
name: "Invalid IPv4 range - malformed",
5457-
args: args{frontendIPv4Range: "not-an-ip"},
5458+
args: args{frontendIPRange: "not-an-ip", ipVersion: "IPv4"},
54585459
wantErr: true,
54595460
},
54605461
{
5461-
name: "Empty range should pass",
5462-
args: args{frontendIPv4Range: ""},
5462+
name: "Empty IPv4 range should pass",
5463+
args: args{frontendIPRange: "", ipVersion: "IPv4"},
54635464
wantErr: false,
54645465
},
5465-
}
5466-
5467-
for _, tt := range tests {
5468-
t.Run(tt.name, func(t *testing.T) {
5469-
if err := validateNodeBalancerFrontendIPv4Range(tt.args.frontendIPv4Range); (err != nil) != tt.wantErr {
5470-
t.Errorf("validateNodeBalancerFrontendIPv4Range() error = %v, wantErr %v", err, tt.wantErr)
5471-
}
5472-
})
5473-
}
5474-
}
5475-
5476-
func Test_validateNodeBalancerFrontendIPv6Range(t *testing.T) {
5477-
type args struct {
5478-
frontendIPv6Range string
5479-
}
5480-
tests := []struct {
5481-
name string
5482-
args args
5483-
wantErr bool
5484-
}{
54855466
{
54865467
name: "Valid IPv6 range",
5487-
args: args{frontendIPv6Range: "2001:db80:1005::/48"},
5468+
args: args{frontendIPRange: "2001:db80:1005::/48", ipVersion: "IPv6"},
54885469
wantErr: false,
54895470
},
54905471
{
54915472
name: "Invalid IPv6 range - no CIDR",
5492-
args: args{frontendIPv6Range: "2001:db80:1005::"},
5473+
args: args{frontendIPRange: "2001:db80:1005::", ipVersion: "IPv6"},
54935474
wantErr: true,
54945475
},
54955476
{
54965477
name: "Invalid IPv6 range - malformed",
5497-
args: args{frontendIPv6Range: "not-an-ipv6"},
5478+
args: args{frontendIPRange: "not-an-ipv6", ipVersion: "IPv6"},
54985479
wantErr: true,
54995480
},
55005481
{
5501-
name: "Empty range should pass",
5502-
args: args{frontendIPv6Range: ""},
5482+
name: "Empty IPv6 range should pass",
5483+
args: args{frontendIPRange: "", ipVersion: "IPv6"},
55035484
wantErr: false,
55045485
},
55055486
}
55065487

55075488
for _, tt := range tests {
55085489
t.Run(tt.name, func(t *testing.T) {
5509-
if err := validateNodeBalancerFrontendIPv6Range(tt.args.frontendIPv6Range); (err != nil) != tt.wantErr {
5510-
t.Errorf("validateNodeBalancerFrontendIPv6Range() error = %v, wantErr %v", err, tt.wantErr)
5490+
if err := validateNodeBalancerFrontendIPRange(tt.args.frontendIPRange, tt.args.ipVersion); (err != nil) != tt.wantErr {
5491+
t.Errorf("validateNodeBalancerFrontendIPRange() error = %v, wantErr %v", err, tt.wantErr)
55115492
}
55125493
})
55135494
}
@@ -5615,7 +5596,25 @@ func Test_getFrontendVPCCreateOptions(t *testing.T) {
56155596
wantErr: false,
56165597
},
56175598
{
5618-
name: "Frontend IPv4 range annotation",
5599+
name: "Frontend subnet id only",
5600+
args: args{
5601+
service: &v1.Service{
5602+
ObjectMeta: metav1.ObjectMeta{
5603+
Annotations: map[string]string{
5604+
annotations.NodeBalancerFrontendSubnetID: "123",
5605+
},
5606+
},
5607+
},
5608+
},
5609+
want: []linodego.NodeBalancerVPCOptions{
5610+
{
5611+
SubnetID: 123,
5612+
},
5613+
},
5614+
wantErr: false,
5615+
},
5616+
{
5617+
name: "Frontend IPv4 range + subnet id",
56195618
args: args{
56205619
service: &v1.Service{
56215620
ObjectMeta: metav1.ObjectMeta{
@@ -5635,7 +5634,7 @@ func Test_getFrontendVPCCreateOptions(t *testing.T) {
56355634
wantErr: false,
56365635
},
56375636
{
5638-
name: "Frontend IPv6 range annotation",
5637+
name: "Frontend IPv6 range + subnet id",
56395638
args: args{
56405639
service: &v1.Service{
56415640
ObjectMeta: metav1.ObjectMeta{
@@ -5654,6 +5653,62 @@ func Test_getFrontendVPCCreateOptions(t *testing.T) {
56545653
},
56555654
wantErr: false,
56565655
},
5656+
{
5657+
name: "Frontend ranges without subnet selector should error",
5658+
args: args{
5659+
service: &v1.Service{
5660+
ObjectMeta: metav1.ObjectMeta{
5661+
Annotations: map[string]string{
5662+
annotations.NodeBalancerFrontendIPv4Range: "10.100.5.0/24",
5663+
},
5664+
},
5665+
},
5666+
},
5667+
want: nil,
5668+
wantErr: true,
5669+
},
5670+
{
5671+
name: "Frontend vpc-name only should error",
5672+
args: args{
5673+
service: &v1.Service{
5674+
ObjectMeta: metav1.ObjectMeta{
5675+
Annotations: map[string]string{
5676+
annotations.NodeBalancerFrontendVPCName: "my-vpc",
5677+
},
5678+
},
5679+
},
5680+
},
5681+
want: nil,
5682+
wantErr: true,
5683+
},
5684+
{
5685+
name: "Frontend subnet-name only should error",
5686+
args: args{
5687+
service: &v1.Service{
5688+
ObjectMeta: metav1.ObjectMeta{
5689+
Annotations: map[string]string{
5690+
annotations.NodeBalancerFrontendSubnetName: "frontend-subnet",
5691+
},
5692+
},
5693+
},
5694+
},
5695+
want: nil,
5696+
wantErr: true,
5697+
},
5698+
{
5699+
name: "Frontend invalid subnet-id should error",
5700+
args: args{
5701+
service: &v1.Service{
5702+
ObjectMeta: metav1.ObjectMeta{
5703+
Annotations: map[string]string{
5704+
annotations.NodeBalancerFrontendSubnetID: "abc",
5705+
},
5706+
},
5707+
},
5708+
},
5709+
want: nil,
5710+
wantErr: true,
5711+
},
56575712
{
56585713
name: "Frontend VPC and subnet names",
56595714
args: args{
@@ -5666,11 +5721,36 @@ func Test_getFrontendVPCCreateOptions(t *testing.T) {
56665721
},
56675722
},
56685723
},
5669-
want: nil, // Will return error due to missing client setup
5670-
wantErr: true,
5724+
want: []linodego.NodeBalancerVPCOptions{
5725+
{
5726+
SubnetID: 456,
5727+
},
5728+
},
5729+
wantErr: false,
56715730
prepareMock: func(m *mocks.MockClient) {
5672-
m.EXPECT().ListVPCs(gomock.Any(), gomock.Any()).Return(nil, stderrors.New("mock error"))
5731+
m.EXPECT().ListVPCs(gomock.Any(), gomock.Any()).Return([]linodego.VPC{{ID: 111, Label: "my-vpc"}}, nil)
5732+
m.EXPECT().ListVPCSubnets(gomock.Any(), 111, gomock.Any()).Return([]linodego.VPCSubnet{{ID: 456, Label: "frontend-subnet"}}, nil)
5733+
},
5734+
},
5735+
{
5736+
name: "Frontend subnet-id should take precedence over names",
5737+
args: args{
5738+
service: &v1.Service{
5739+
ObjectMeta: metav1.ObjectMeta{
5740+
Annotations: map[string]string{
5741+
annotations.NodeBalancerFrontendSubnetID: "123",
5742+
annotations.NodeBalancerFrontendVPCName: "my-vpc",
5743+
annotations.NodeBalancerFrontendSubnetName: "frontend-subnet",
5744+
},
5745+
},
5746+
},
5747+
},
5748+
want: []linodego.NodeBalancerVPCOptions{
5749+
{
5750+
SubnetID: 123,
5751+
},
56735752
},
5753+
wantErr: false,
56745754
},
56755755
}
56765756

0 commit comments

Comments
 (0)