Skip to content

Commit 218bcdf

Browse files
committed
fix(ingress): count against chosen subnets
1 parent ec6fbe8 commit 218bcdf

File tree

2 files changed

+35
-5
lines changed

2 files changed

+35
-5
lines changed

pkg/ingress/model_build_frontend_nlb.go

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -130,9 +130,6 @@ func (t *defaultModelBuildTask) buildFrontendNlbSubnetMappings(ctx context.Conte
130130
return nil, errors.Errorf("all EIP allocations for the ingress group must be the same: %v | %v", chosenEipAllocations, eipAllocations)
131131
}
132132
}
133-
if len(eipAllocationsList) != len(explicitSubnetNameOrIDsList) {
134-
return nil, errors.Errorf("count of EIP allocations (%d) and subnets (%d) must match", len(eipAllocationsList), len(explicitSubnetNameOrIDsList))
135-
}
136133
}
137134

138135
if len(explicitSubnetNameOrIDsList) != 0 {
@@ -150,15 +147,18 @@ func (t *defaultModelBuildTask) buildFrontendNlbSubnetMappings(ctx context.Conte
150147
return nil, err
151148
}
152149

150+
if len(chosenEipAllocations) != len(chosenSubnets) {
151+
return nil, errors.Errorf("count of EIP allocations (%d) and subnets (%d) must match", len(chosenEipAllocations), len(explicitSubnetNameOrIDsList))
152+
}
153+
153154
return buildFrontendNlbSubnetMappingsWithSubnets(chosenSubnets, chosenEipAllocations), nil
154155
}
155156

156-
157157
return nil, nil
158158

159159
}
160160

161-
// I found a bug in the current implementation where EIP allocations require both subnets AND EIPs to be provided together, but there's an indexing issue in buildFrontendNlbSubnetMappingsWithSubnets.
161+
// I found a bug in the current implementation where EIP allocations require both subnets AND EIPs to be provided together, but there's an indexing issue in buildFrontendNlbSubnetMappingsWithSubnets.
162162
// The function tries to access eipAllocation[subnetIndex][0] but the data structure is eipAllocation[ingressIndex][eipIndex].
163163
func buildFrontendNlbSubnetMappingsWithSubnets(subnets []ec2types.Subnet, eipAllocation []string) []elbv2model.SubnetMapping {
164164
subnetMappings := make([]elbv2model.SubnetMapping, 0, len(subnets))

pkg/ingress/model_build_frontend_nlb_test.go

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -244,6 +244,36 @@ func Test_buildFrontendNlbSubnetMappings(t *testing.T) {
244244
{SubnetID: "subnet-2", AllocationID: nil},
245245
},
246246
},
247+
{
248+
name: "with subnets and eip allocations",
249+
fields: fields{
250+
ingGroup: Group{
251+
ID: GroupID{
252+
Namespace: "awesome-ns",
253+
Name: "my-ingress",
254+
},
255+
Members: []ClassifiedIngress{
256+
{
257+
Ing: &networking.Ingress{
258+
ObjectMeta: metav1.ObjectMeta{
259+
Namespace: "awesome-ns",
260+
Name: "ing-3",
261+
Annotations: map[string]string{
262+
"alb.ingress.kubernetes.io/frontend-nlb-subnets": "subnet-1,subnet-2",
263+
"alb.ingress.kubernetes.io/frontend-nlb-eip-allocations": "eip-1,eip-2",
264+
},
265+
},
266+
},
267+
},
268+
},
269+
},
270+
scheme: elbv2.LoadBalancerSchemeInternetFacing, // EIPs require internet-facing
271+
},
272+
wantMappings: []expectedMapping{
273+
{SubnetID: "subnet-1", AllocationID: awssdk.String("eip-1")},
274+
{SubnetID: "subnet-2", AllocationID: awssdk.String("eip-2")},
275+
},
276+
},
247277
}
248278

249279
for _, tt := range tests {

0 commit comments

Comments
 (0)