Skip to content

Commit 09b6030

Browse files
kishorjoliviassss
kishorj
authored and
oliviassss
committed
refactor targetGroupBinding network builder
remove discovery from default code path, not needed for NLB with SG
1 parent 8664ad4 commit 09b6030

File tree

3 files changed

+493
-151
lines changed

3 files changed

+493
-151
lines changed

‎pkg/service/model_build_target_group.go

Lines changed: 84 additions & 100 deletions
Original file line numberDiff line numberDiff line change
@@ -262,7 +262,7 @@ func (t *defaultModelBuildTask) buildPreserveClientIPFlag(_ context.Context, tar
262262

263263
// buildTargetGroupPort constructs the TargetGroup's port.
264264
// Note: TargetGroup's port is not in the data path as we always register targets with port specified.
265-
// so this settings don't really matter to our controller, and we do our best to use the most appropriate port as targetGroup's port to avoid UX confusing.
265+
// so this setting don't really matter to our controller, and we do our best to use the most appropriate port as targetGroup's port to avoid UX confusion.
266266
func (t *defaultModelBuildTask) buildTargetGroupPort(_ context.Context, targetType elbv2model.TargetType, svcPort corev1.ServicePort) int64 {
267267
if targetType == elbv2model.TargetTypeInstance {
268268
return int64(svcPort.NodePort)
@@ -408,15 +408,11 @@ func (t *defaultModelBuildTask) buildTargetGroupBindingSpec(ctx context.Context,
408408
if targetType == elbv2api.TargetTypeInstance {
409409
targetPort = intstr.FromInt(int(port.NodePort))
410410
}
411-
defaultSourceRanges, err := t.getDefaultIPSourceRanges(ctx, *targetGroup.Spec.IPAddressType, port.Protocol, scheme)
412-
if err != nil {
413-
return elbv2model.TargetGroupBindingResourceSpec{}, err
414-
}
415411
var tgbNetworking *elbv2model.TargetGroupBindingNetworking
416412
if len(t.loadBalancer.Spec.SecurityGroups) == 0 {
417-
tgbNetworking, err = t.buildTargetGroupBindingNetworkingLegacy(ctx, targetPort, *hc.Port, port, defaultSourceRanges, *targetGroup.Spec.IPAddressType)
413+
tgbNetworking, err = t.buildTargetGroupBindingNetworkingLegacy(ctx, targetPort, *hc.Port, port, scheme, *targetGroup.Spec.IPAddressType)
418414
} else {
419-
tgbNetworking, err = t.buildTargetGroupBindingNetworking(ctx, port.Protocol, targetGroup.Spec.Port, *targetGroup.Spec.HealthCheckConfig.Port)
415+
tgbNetworking, err = t.buildTargetGroupBindingNetworking(ctx, targetPort, *hc.Port, port)
420416
}
421417
if err != nil {
422418
return elbv2model.TargetGroupBindingResourceSpec{}, err
@@ -442,60 +438,52 @@ func (t *defaultModelBuildTask) buildTargetGroupBindingSpec(ctx context.Context,
442438
}, nil
443439
}
444440

445-
func (t *defaultModelBuildTask) buildTargetGroupBindingNetworking(_ context.Context, tgProtocol corev1.Protocol, tgPort int64, healthCheckPort intstr.IntOrString) (*elbv2model.TargetGroupBindingNetworking, error) {
441+
func (t *defaultModelBuildTask) buildTargetGroupBindingNetworking(_ context.Context, tgPort intstr.IntOrString,
442+
hcPort intstr.IntOrString, port corev1.ServicePort) (*elbv2model.TargetGroupBindingNetworking, error) {
446443
if t.backendSGIDToken == nil {
447444
return nil, nil
448445
}
449446
protocolTCP := elbv2api.NetworkingProtocolTCP
450447
protocolUDP := elbv2api.NetworkingProtocolUDP
451448

449+
var ports []elbv2api.NetworkingPort
452450
if t.disableRestrictedSGRules {
453-
ports := []elbv2api.NetworkingPort{
454-
{
455-
Protocol: &protocolTCP,
451+
ports = append(ports, elbv2api.NetworkingPort{
452+
Protocol: &protocolTCP,
453+
Port: nil,
454+
})
455+
if port.Protocol == corev1.ProtocolUDP {
456+
ports = append(ports, elbv2api.NetworkingPort{
457+
Protocol: &protocolUDP,
456458
Port: nil,
457-
},
459+
})
458460
}
459-
if tgProtocol == corev1.ProtocolUDP {
461+
} else {
462+
switch port.Protocol {
463+
case corev1.ProtocolTCP:
464+
ports = append(ports, elbv2api.NetworkingPort{
465+
Protocol: &protocolTCP,
466+
Port: &tgPort,
467+
})
468+
case corev1.ProtocolUDP:
460469
ports = append(ports, elbv2api.NetworkingPort{
461470
Protocol: &protocolUDP,
462-
Port: nil,
471+
Port: &tgPort,
463472
})
473+
if hcPort.String() == healthCheckPortTrafficPort || (hcPort.Type == intstr.Int && hcPort.IntValue() == tgPort.IntValue()) {
474+
ports = append(ports, elbv2api.NetworkingPort{
475+
Protocol: &protocolTCP,
476+
Port: &tgPort,
477+
})
478+
}
464479
}
465-
return &elbv2model.TargetGroupBindingNetworking{
466-
Ingress: []elbv2model.NetworkingIngressRule{
467-
{
468-
From: []elbv2model.NetworkingPeer{
469-
{
470-
SecurityGroup: &elbv2model.SecurityGroup{
471-
GroupID: t.backendSGIDToken,
472-
},
473-
},
474-
},
475-
Ports: ports,
476-
},
477-
},
478-
}, nil
479-
}
480480

481-
targetGroupPort := intstr.FromInt(int(tgPort))
482-
ports := []elbv2api.NetworkingPort{
483-
{
484-
Protocol: &protocolTCP,
485-
Port: &targetGroupPort,
486-
},
487-
}
488-
if tgProtocol == corev1.ProtocolUDP {
489-
ports = append(ports, elbv2api.NetworkingPort{
490-
Protocol: &protocolUDP,
491-
Port: &targetGroupPort,
492-
})
493-
}
494-
if healthCheckPort.String() != healthCheckPortTrafficPort && (healthCheckPort.Type == intstr.Int && healthCheckPort.IntVal != int32(tgPort)) {
495-
ports = append(ports, elbv2api.NetworkingPort{
496-
Protocol: &protocolTCP,
497-
Port: &healthCheckPort,
498-
})
481+
if hcPort.String() != healthCheckPortTrafficPort && (hcPort.Type == intstr.Int && hcPort.IntValue() != tgPort.IntValue()) {
482+
ports = append(ports, elbv2api.NetworkingPort{
483+
Protocol: &protocolTCP,
484+
Port: &hcPort,
485+
})
486+
}
499487
}
500488
return &elbv2model.TargetGroupBindingNetworking{
501489
Ingress: []elbv2model.NetworkingIngressRule{
@@ -513,32 +501,31 @@ func (t *defaultModelBuildTask) buildTargetGroupBindingNetworking(_ context.Cont
513501
}, nil
514502
}
515503

516-
func (t *defaultModelBuildTask) buildPeersFromSourceRangesConfiguration(_ context.Context, defaultSourceRanges []string) ([]elbv2model.NetworkingPeer, bool) {
504+
func (t *defaultModelBuildTask) getLoadBalancerSourceRanges(_ context.Context) []string {
517505
var sourceRanges []string
518-
var peers []elbv2model.NetworkingPeer
519-
customSourceRangesConfigured := true
520506
for _, cidr := range t.service.Spec.LoadBalancerSourceRanges {
521507
sourceRanges = append(sourceRanges, cidr)
522508
}
523509
if len(sourceRanges) == 0 {
524510
t.annotationParser.ParseStringSliceAnnotation(annotations.SvcLBSuffixSourceRanges, &sourceRanges, t.service.Annotations)
525511
}
526-
if len(sourceRanges) == 0 {
527-
sourceRanges = defaultSourceRanges
528-
customSourceRangesConfigured = false
529-
}
512+
return sourceRanges
513+
}
514+
515+
func (t *defaultModelBuildTask) buildPeersFromSourceRangeCIDRs(_ context.Context, sourceRanges []string) []elbv2model.NetworkingPeer {
516+
var peers []elbv2model.NetworkingPeer
530517
for _, cidr := range sourceRanges {
531518
peers = append(peers, elbv2model.NetworkingPeer{
532519
IPBlock: &elbv2api.IPBlock{
533520
CIDR: cidr,
534521
},
535522
})
536523
}
537-
return peers, customSourceRangesConfigured
524+
return peers
538525
}
539526

540527
func (t *defaultModelBuildTask) buildTargetGroupBindingNetworkingLegacy(ctx context.Context, tgPort intstr.IntOrString,
541-
hcPort intstr.IntOrString, port corev1.ServicePort, defaultSourceRanges []string, targetGroupIPAddressType elbv2model.TargetGroupIPAddressType) (*elbv2model.TargetGroupBindingNetworking, error) {
528+
hcPort intstr.IntOrString, port corev1.ServicePort, scheme elbv2model.LoadBalancerScheme, targetGroupIPAddressType elbv2model.TargetGroupIPAddressType) (*elbv2model.TargetGroupBindingNetworking, error) {
542529
manageBackendSGRules, err := t.buildManageSecurityGroupRulesFlagLegacy(ctx)
543530
if err != nil {
544531
return nil, err
@@ -547,20 +534,28 @@ func (t *defaultModelBuildTask) buildTargetGroupBindingNetworkingLegacy(ctx cont
547534
return nil, nil
548535
}
549536
tgProtocol := port.Protocol
550-
loadBalancerSubnetsSourceRanges := t.getLoadBalancerSubnetsSourceRanges(targetGroupIPAddressType)
551537
networkingProtocol := elbv2api.NetworkingProtocolTCP
538+
healthCheckProtocol := elbv2api.NetworkingProtocolTCP
552539
if tgProtocol == corev1.ProtocolUDP {
553540
networkingProtocol = elbv2api.NetworkingProtocolUDP
554541
}
555-
trafficSource := loadBalancerSubnetsSourceRanges
556-
customSourceRangesConfigured := false
542+
loadBalancerSubnetCIDRs := t.getLoadBalancerSubnetsSourceRanges(targetGroupIPAddressType)
543+
trafficSource := loadBalancerSubnetCIDRs
544+
defaultRangeUsed := false
557545
if networkingProtocol == elbv2api.NetworkingProtocolUDP || t.preserveClientIP {
558-
trafficSource, customSourceRangesConfigured = t.buildPeersFromSourceRangesConfiguration(ctx, defaultSourceRanges)
546+
trafficSource = t.getLoadBalancerSourceRanges(ctx)
547+
if len(trafficSource) == 0 {
548+
trafficSource, err = t.getDefaultIPSourceRanges(ctx, targetGroupIPAddressType, port.Protocol, scheme)
549+
if err != nil {
550+
return nil, err
551+
}
552+
defaultRangeUsed = true
553+
}
559554
}
560555
tgbNetworking := &elbv2model.TargetGroupBindingNetworking{
561556
Ingress: []elbv2model.NetworkingIngressRule{
562557
{
563-
From: trafficSource,
558+
From: t.buildPeersFromSourceRangeCIDRs(ctx, trafficSource),
564559
Ports: []elbv2api.NetworkingPort{
565560
{
566561
Port: &tgPort,
@@ -570,9 +565,21 @@ func (t *defaultModelBuildTask) buildTargetGroupBindingNetworkingLegacy(ctx cont
570565
},
571566
},
572567
}
573-
if hcIngressRules := t.buildHealthCheckNetworkingIngressRules(trafficSource, loadBalancerSubnetsSourceRanges, tgPort, hcPort, tgProtocol,
574-
customSourceRangesConfigured); len(hcIngressRules) > 0 {
575-
tgbNetworking.Ingress = append(tgbNetworking.Ingress, hcIngressRules...)
568+
if healthCheckSourceCIDRs := t.buildHealthCheckSourceCIDRs(trafficSource, loadBalancerSubnetCIDRs, tgPort, hcPort,
569+
tgProtocol, defaultRangeUsed); len(healthCheckSourceCIDRs) > 0 {
570+
networkingHealthCheckPort := hcPort
571+
if hcPort.String() == healthCheckPortTrafficPort {
572+
networkingHealthCheckPort = tgPort
573+
}
574+
tgbNetworking.Ingress = append(tgbNetworking.Ingress, elbv2model.NetworkingIngressRule{
575+
From: t.buildPeersFromSourceRangeCIDRs(ctx, healthCheckSourceCIDRs),
576+
Ports: []elbv2api.NetworkingPort{
577+
{
578+
Port: &networkingHealthCheckPort,
579+
Protocol: &healthCheckProtocol,
580+
},
581+
},
582+
})
576583
}
577584
return tgbNetworking, nil
578585
}
@@ -597,28 +604,18 @@ func (t *defaultModelBuildTask) getDefaultIPSourceRanges(ctx context.Context, ta
597604
return defaultSourceRanges, nil
598605
}
599606

600-
func (t *defaultModelBuildTask) getLoadBalancerSubnetsSourceRanges(targetGroupIPAddressType elbv2model.TargetGroupIPAddressType) []elbv2model.NetworkingPeer {
601-
var subnetCIDRRanges []elbv2model.NetworkingPeer
607+
func (t *defaultModelBuildTask) getLoadBalancerSubnetsSourceRanges(targetGroupIPAddressType elbv2model.TargetGroupIPAddressType) []string {
608+
var subnetCIDRs []string
602609
for _, subnet := range t.ec2Subnets {
603610
if targetGroupIPAddressType == elbv2model.TargetGroupIPAddressTypeIPv4 {
604-
subnetCIDRRanges = append(subnetCIDRRanges, elbv2model.NetworkingPeer{
605-
IPBlock: &elbv2api.IPBlock{
606-
CIDR: aws.StringValue(subnet.CidrBlock),
607-
},
608-
})
611+
subnetCIDRs = append(subnetCIDRs, aws.StringValue(subnet.CidrBlock))
609612
} else {
610613
for _, ipv6CIDRBlockAssoc := range subnet.Ipv6CidrBlockAssociationSet {
611-
subnetCIDRRanges = append(subnetCIDRRanges, elbv2model.NetworkingPeer{
612-
IPBlock: &elbv2api.IPBlock{
613-
CIDR: aws.StringValue(ipv6CIDRBlockAssoc.Ipv6CidrBlock),
614-
},
615-
})
616-
614+
subnetCIDRs = append(subnetCIDRs, aws.StringValue(ipv6CIDRBlockAssoc.Ipv6CidrBlock))
617615
}
618616
}
619617
}
620-
621-
return subnetCIDRRanges
618+
return subnetCIDRs
622619
}
623620

624621
func (t *defaultModelBuildTask) buildTargetGroupIPAddressType(_ context.Context, svc *corev1.Service) (elbv2model.TargetGroupIPAddressType, error) {
@@ -654,36 +651,23 @@ func (t *defaultModelBuildTask) buildTargetGroupBindingNodeSelector(_ context.Co
654651
}, nil
655652
}
656653

657-
func (t *defaultModelBuildTask) buildHealthCheckNetworkingIngressRules(trafficSource, hcSource []elbv2model.NetworkingPeer, tgPort, hcPort intstr.IntOrString,
658-
tgProtocol corev1.Protocol, customSourceRanges bool) []elbv2model.NetworkingIngressRule {
654+
func (t *defaultModelBuildTask) buildHealthCheckSourceCIDRs(trafficSource, subnetCIDRs []string, tgPort, hcPort intstr.IntOrString,
655+
tgProtocol corev1.Protocol, defaultRangeUsed bool) []string {
659656
if tgProtocol != corev1.ProtocolUDP &&
660657
(hcPort.String() == healthCheckPortTrafficPort || hcPort.IntValue() == tgPort.IntValue()) {
661658
if !t.preserveClientIP {
662-
return []elbv2model.NetworkingIngressRule{}
659+
return nil
663660
}
664-
if !customSourceRanges {
665-
return []elbv2model.NetworkingIngressRule{}
661+
if defaultRangeUsed {
662+
return nil
666663
}
667664
for _, src := range trafficSource {
668-
if src.IPBlock.CIDR == "0.0.0.0/0" || src.IPBlock.CIDR == "::/0" {
669-
return []elbv2model.NetworkingIngressRule{}
665+
if src == "0.0.0.0/0" || src == "::/0" {
666+
return nil
670667
}
671668
}
672669
}
673-
var healthCheckPorts []elbv2api.NetworkingPort
674-
networkingProtocolTCP := elbv2api.NetworkingProtocolTCP
675-
networkingHealthCheckPort := hcPort
676-
if hcPort.String() == healthCheckPortTrafficPort {
677-
networkingHealthCheckPort = tgPort
678-
}
679-
healthCheckPorts = append(healthCheckPorts, elbv2api.NetworkingPort{
680-
Port: &networkingHealthCheckPort,
681-
Protocol: &networkingProtocolTCP,
682-
})
683-
return []elbv2model.NetworkingIngressRule{{
684-
From: hcSource,
685-
Ports: healthCheckPorts,
686-
}}
670+
return subnetCIDRs
687671
}
688672

689673
func (t *defaultModelBuildTask) buildManageSecurityGroupRulesFlagLegacy(_ context.Context) (bool, error) {

0 commit comments

Comments
 (0)