Skip to content

Commit 138442c

Browse files
kishorjTimothy-Dougherty
kishorj
authored and
Timothy-Dougherty
committed
filter redundant health check SG rules
1 parent 6a85c68 commit 138442c

File tree

3 files changed

+97
-107
lines changed

3 files changed

+97
-107
lines changed

‎pkg/service/model_build_target_group.go

Lines changed: 46 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -5,20 +5,21 @@ import (
55
"crypto/sha256"
66
"encoding/hex"
77
"fmt"
8+
"regexp"
9+
"sort"
10+
"strconv"
11+
"strings"
12+
813
"github.com/aws/aws-sdk-go/aws"
914
"github.com/pkg/errors"
1015
corev1 "k8s.io/api/core/v1"
1116
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
1217
"k8s.io/apimachinery/pkg/types"
1318
"k8s.io/apimachinery/pkg/util/intstr"
14-
"regexp"
1519
elbv2api "sigs.k8s.io/aws-load-balancer-controller/apis/elbv2/v1beta1"
1620
"sigs.k8s.io/aws-load-balancer-controller/pkg/annotations"
1721
"sigs.k8s.io/aws-load-balancer-controller/pkg/k8s"
1822
elbv2model "sigs.k8s.io/aws-load-balancer-controller/pkg/model/elbv2"
19-
"sort"
20-
"strconv"
21-
"strings"
2223
)
2324

2425
const (
@@ -395,9 +396,10 @@ func (t *defaultModelBuildTask) buildTargetGroupBindingSpec(ctx context.Context,
395396
}, nil
396397
}
397398

398-
func (t *defaultModelBuildTask) buildPeersFromSourceRanges(_ context.Context, defaultSourceRanges []string) []elbv2model.NetworkingPeer {
399+
func (t *defaultModelBuildTask) buildPeersFromSourceRanges(_ context.Context, defaultSourceRanges []string) ([]elbv2model.NetworkingPeer, bool) {
399400
var sourceRanges []string
400401
var peers []elbv2model.NetworkingPeer
402+
customSourceRangesConfigured := true
401403
for _, cidr := range t.service.Spec.LoadBalancerSourceRanges {
402404
sourceRanges = append(sourceRanges, cidr)
403405
}
@@ -406,6 +408,7 @@ func (t *defaultModelBuildTask) buildPeersFromSourceRanges(_ context.Context, de
406408
}
407409
if len(sourceRanges) == 0 {
408410
sourceRanges = defaultSourceRanges
411+
customSourceRangesConfigured = false
409412
}
410413
for _, cidr := range sourceRanges {
411414
peers = append(peers, elbv2model.NetworkingPeer{
@@ -414,7 +417,7 @@ func (t *defaultModelBuildTask) buildPeersFromSourceRanges(_ context.Context, de
414417
},
415418
})
416419
}
417-
return peers
420+
return peers, customSourceRangesConfigured
418421
}
419422

420423
func (t *defaultModelBuildTask) buildTargetGroupBindingNetworking(ctx context.Context, tgPort intstr.IntOrString, preserveClientIP bool,
@@ -438,8 +441,9 @@ func (t *defaultModelBuildTask) buildTargetGroupBindingNetworking(ctx context.Co
438441
},
439442
}
440443
trafficSource := fromVPC
444+
customSourceRangesConfigured := false
441445
if networkingProtocol == elbv2api.NetworkingProtocolUDP || preserveClientIP {
442-
trafficSource = t.buildPeersFromSourceRanges(ctx, defaultSourceRanges)
446+
trafficSource, customSourceRangesConfigured = t.buildPeersFromSourceRanges(ctx, defaultSourceRanges)
443447
}
444448
tgbNetworking := &elbv2model.TargetGroupBindingNetworking{
445449
Ingress: []elbv2model.NetworkingIngressRule{
@@ -449,21 +453,9 @@ func (t *defaultModelBuildTask) buildTargetGroupBindingNetworking(ctx context.Co
449453
},
450454
},
451455
}
452-
if preserveClientIP || tgProtocol == corev1.ProtocolUDP || (hcPort.String() != healthCheckPortTrafficPort && hcPort.IntValue() != tgPort.IntValue()) {
453-
var healthCheckPorts []elbv2api.NetworkingPort
454-
networkingProtocolTCP := elbv2api.NetworkingProtocolTCP
455-
networkingHealthCheckPort := hcPort
456-
if hcPort.String() == healthCheckPortTrafficPort {
457-
networkingHealthCheckPort = tgPort
458-
}
459-
healthCheckPorts = append(healthCheckPorts, elbv2api.NetworkingPort{
460-
Port: &networkingHealthCheckPort,
461-
Protocol: &networkingProtocolTCP,
462-
})
463-
tgbNetworking.Ingress = append(tgbNetworking.Ingress, elbv2model.NetworkingIngressRule{
464-
From: fromVPC,
465-
Ports: healthCheckPorts,
466-
})
456+
if hcIngressRules := t.buildHealthCheckNetworkingIngressRules(trafficSource, fromVPC, tgPort, hcPort, tgProtocol,
457+
preserveClientIP, customSourceRangesConfigured); len(hcIngressRules) > 0 {
458+
tgbNetworking.Ingress = append(tgbNetworking.Ingress, hcIngressRules...)
467459
}
468460
return tgbNetworking
469461
}
@@ -483,3 +475,35 @@ func (t *defaultModelBuildTask) buildTargetGroupBindingNodeSelector(_ context.Co
483475
MatchLabels: targetNodeLabels,
484476
}, nil
485477
}
478+
479+
func (t *defaultModelBuildTask) buildHealthCheckNetworkingIngressRules(trafficSource, hcSource []elbv2model.NetworkingPeer, tgPort, hcPort intstr.IntOrString,
480+
tgProtocol corev1.Protocol, preserveClientIP, customSoureRanges bool) []elbv2model.NetworkingIngressRule {
481+
if tgProtocol != corev1.ProtocolUDP &&
482+
(hcPort.String() == healthCheckPortTrafficPort || hcPort.IntValue() == tgPort.IntValue()) {
483+
if !preserveClientIP {
484+
return []elbv2model.NetworkingIngressRule{}
485+
}
486+
if !customSoureRanges {
487+
return []elbv2model.NetworkingIngressRule{}
488+
}
489+
for _, src := range trafficSource {
490+
if src.IPBlock.CIDR == "0.0.0.0/0" {
491+
return []elbv2model.NetworkingIngressRule{}
492+
}
493+
}
494+
}
495+
var healthCheckPorts []elbv2api.NetworkingPort
496+
networkingProtocolTCP := elbv2api.NetworkingProtocolTCP
497+
networkingHealthCheckPort := hcPort
498+
if hcPort.String() == healthCheckPortTrafficPort {
499+
networkingHealthCheckPort = tgPort
500+
}
501+
healthCheckPorts = append(healthCheckPorts, elbv2api.NetworkingPort{
502+
Port: &networkingHealthCheckPort,
503+
Protocol: &networkingProtocolTCP,
504+
})
505+
return []elbv2model.NetworkingIngressRule{{
506+
From: hcSource,
507+
Ports: healthCheckPorts,
508+
}}
509+
}

‎pkg/service/model_build_target_group_test.go

Lines changed: 51 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -624,26 +624,6 @@ func Test_defaultModelBuilderTask_buildTargetGroupBindingNetworking(t *testing.T
624624
},
625625
},
626626
},
627-
{
628-
From: []elbv2.NetworkingPeer{
629-
{
630-
IPBlock: &elbv2api.IPBlock{
631-
CIDR: "172.16.0.0/19",
632-
},
633-
},
634-
{
635-
IPBlock: &elbv2api.IPBlock{
636-
CIDR: "1.2.3.4/19",
637-
},
638-
},
639-
},
640-
Ports: []elbv2api.NetworkingPort{
641-
{
642-
Protocol: &networkingProtocolTCP,
643-
Port: &port80,
644-
},
645-
},
646-
},
647627
},
648628
},
649629
},
@@ -837,6 +817,57 @@ func Test_defaultModelBuilderTask_buildTargetGroupBindingNetworking(t *testing.T
837817
},
838818
},
839819
},
820+
{
821+
name: "tcp-service with preserve Client IP, hc is traffic port, source range specified and contains 0/0",
822+
svc: &corev1.Service{
823+
Spec: corev1.ServiceSpec{
824+
LoadBalancerSourceRanges: []string{"10.0.0.0/16", "1.2.3.4/24", "0.0.0.0/0"},
825+
},
826+
},
827+
tgPort: port80,
828+
hcPort: port80,
829+
subnets: []*ec2.Subnet{
830+
{
831+
CidrBlock: aws.String("172.16.0.0/19"),
832+
SubnetId: aws.String("sn-1"),
833+
},
834+
{
835+
CidrBlock: aws.String("1.2.3.4/19"),
836+
SubnetId: aws.String("sn-2"),
837+
},
838+
},
839+
tgProtocol: corev1.ProtocolTCP,
840+
preserveClientIP: true,
841+
want: &elbv2.TargetGroupBindingNetworking{
842+
Ingress: []elbv2.NetworkingIngressRule{
843+
{
844+
From: []elbv2.NetworkingPeer{
845+
{
846+
IPBlock: &elbv2api.IPBlock{
847+
CIDR: "10.0.0.0/16",
848+
},
849+
},
850+
{
851+
IPBlock: &elbv2api.IPBlock{
852+
CIDR: "1.2.3.4/24",
853+
},
854+
},
855+
{
856+
IPBlock: &elbv2api.IPBlock{
857+
CIDR: "0.0.0.0/0",
858+
},
859+
},
860+
},
861+
Ports: []elbv2api.NetworkingPort{
862+
{
863+
Protocol: &networkingProtocolTCP,
864+
Port: &port80,
865+
},
866+
},
867+
},
868+
},
869+
},
870+
},
840871
}
841872
for _, tt := range tests {
842873
t.Run(tt.name, func(t *testing.T) {

‎pkg/service/model_builder_test.go

Lines changed: 0 additions & 65 deletions
Original file line numberDiff line numberDiff line change
@@ -1265,31 +1265,6 @@ func Test_defaultModelBuilderTask_Build(t *testing.T) {
12651265
"port":31223
12661266
}
12671267
]
1268-
},
1269-
{
1270-
"from":[
1271-
{
1272-
"ipBlock":{
1273-
"cidr":"192.168.0.0/19"
1274-
}
1275-
},
1276-
{
1277-
"ipBlock":{
1278-
"cidr":"192.168.32.0/19"
1279-
}
1280-
},
1281-
{
1282-
"ipBlock":{
1283-
"cidr":"192.168.64.0/19"
1284-
}
1285-
}
1286-
],
1287-
"ports":[
1288-
{
1289-
"protocol":"TCP",
1290-
"port":31223
1291-
}
1292-
]
12931268
}
12941269
]
12951270
}
@@ -1330,31 +1305,6 @@ func Test_defaultModelBuilderTask_Build(t *testing.T) {
13301305
"port":32323
13311306
}
13321307
]
1333-
},
1334-
{
1335-
"from":[
1336-
{
1337-
"ipBlock":{
1338-
"cidr":"192.168.0.0/19"
1339-
}
1340-
},
1341-
{
1342-
"ipBlock":{
1343-
"cidr":"192.168.32.0/19"
1344-
}
1345-
},
1346-
{
1347-
"ipBlock":{
1348-
"cidr":"192.168.64.0/19"
1349-
}
1350-
}
1351-
],
1352-
"ports":[
1353-
{
1354-
"protocol":"TCP",
1355-
"port":32323
1356-
}
1357-
]
13581308
}
13591309
]
13601310
}
@@ -1974,21 +1924,6 @@ func Test_defaultModelBuilderTask_Build(t *testing.T) {
19741924
"port": 80
19751925
}
19761926
]
1977-
},
1978-
{
1979-
"from": [
1980-
{
1981-
"ipBlock": {
1982-
"cidr": "192.168.0.0/19"
1983-
}
1984-
}
1985-
],
1986-
"ports": [
1987-
{
1988-
"protocol": "TCP",
1989-
"port": 80
1990-
}
1991-
]
19921927
}
19931928
]
19941929
},

0 commit comments

Comments
 (0)