Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly
Sign up
Appearance settings

Commit e964100

Browse files
refactor attaching listeners
1 parent c7b0836 commit e964100

File tree

2 files changed

+123
-221
lines changed

2 files changed

+123
-221
lines changed

‎internal/controller/state/graph/route_common.go

Lines changed: 58 additions & 144 deletions
Original file line numberDiff line numberDiff line change
@@ -307,37 +307,24 @@ func buildSectionNameRefs(
307307
gwNsName: gwNsName,
308308
}
309309

310-
// If there is no section name, handle based on whether port is specified
311-
// FIXME(sarthyparty): this logic seems to be duplicated in findAttachableListeners so we should refactor this,
312-
// either here or in findAttachableListeners
310+
// If there is no section name, expand to all listeners (port filtering will be done in findAttachableListeners)
313311
if p.SectionName == nil {
314-
// If port is specified, preserve the port-only nature for proper validation
315-
if p.Port != nil {
312+
for _, l := range gw.Listeners {
313+
k.sectionName = string(l.Source.Name)
314+
315+
if err := checkUniqueSections(k); err != nil {
316+
return nil, err
317+
}
318+
316319
sectionNameRefs = append(sectionNameRefs, ParentRef{
320+
// if the ParentRefs we create are for each listener in the same gateway, we keep the
321+
// parentRefIndex the same so when we look at a route's parentRef's we can see
322+
// if the parentRef is a unique parentRef or one we created internally
317323
Idx: i,
318324
Gateway: CreateParentRefGateway(gw),
319-
SectionName: nil, // Keep as nil to preserve port-only semantics
325+
SectionName: &l.Source.Name,
320326
Port: p.Port,
321327
})
322-
} else {
323-
// If no port and no sectionName, expand to all listeners
324-
for _, l := range gw.Listeners {
325-
k.sectionName = string(l.Source.Name)
326-
327-
if err := checkUniqueSections(k); err != nil {
328-
return nil, err
329-
}
330-
331-
sectionNameRefs = append(sectionNameRefs, ParentRef{
332-
// if the ParentRefs we create are for each listener in the same gateway, we keep the
333-
// parentRefIndex the same so when we look at a route's parentRef's we can see
334-
// if the parentRef is a unique parentRef or one we created internally
335-
Idx: i,
336-
Gateway: CreateParentRefGateway(gw),
337-
SectionName: &l.Source.Name,
338-
Port: nil,
339-
})
340-
}
341328
}
342329

343330
continue
@@ -538,14 +525,14 @@ func removeHostnames(hostnames []string, toRemove map[string]struct{}) []string
538525
func validateParentRef(
539526
ref *ParentRef,
540527
gw *Gateway,
541-
) (status *ParentRefAttachmentStatus, attachableListeners []*Listener) {
528+
) (status *ParentRefAttachmentStatus, attachableListener*Listener) {
542529
attachment := &ParentRefAttachmentStatus{
543530
AcceptedHostnames: make(map[string][]string),
544531
}
545532

546533
ref.Attachment = attachment
547534

548-
attachableListeners, listenerExists := findAttachableListeners(
535+
attachableListener, listenerExists := findAttachableListener(
549536
ref,
550537
gw.Listeners,
551538
)
@@ -561,10 +548,10 @@ func validateParentRef(
561548

562549
if !gw.Valid {
563550
attachment.FailedConditions = append(attachment.FailedConditions, conditions.NewRouteInvalidGateway())
564-
return attachment, attachableListeners
551+
return attachment, attachableListener
565552
}
566553

567-
return attachment, attachableListeners
554+
return attachment, attachableListener
568555
}
569556

570557
func bindL4RouteToListeners(
@@ -589,7 +576,7 @@ func bindL4RouteToListeners(
589576
continue
590577
}
591578

592-
attachment, attachableListeners := validateParentRef(ref, gw)
579+
attachment, attachableListener := validateParentRef(ref, gw)
593580

594581
if len(attachment.FailedConditions) > 0 {
595582
continue
@@ -599,11 +586,10 @@ func bindL4RouteToListeners(
599586
attachment.FailedConditions = append(attachment.FailedConditions, cond)
600587
}
601588

602-
// Try to attach Route to all matching listeners
603-
604-
cond, attached := tryToAttachL4RouteToListeners(
589+
// Try to attach Route to the matching listener
590+
cond, attached := tryToAttachL4RouteToListener(
605591
ref.Attachment,
606-
attachableListeners,
592+
attachableListener,
607593
route,
608594
gw,
609595
namespaces,
@@ -621,56 +607,29 @@ func bindL4RouteToListeners(
621607
}
622608
}
623609

624-
// tryToAttachL4RouteToListeners tries to attach the L4Route to listeners that match the parentRef and the hostnames.
625-
// There are two cases:
626-
// (1) If it succeeds in attaching at least one listener it will return true. The returned condition will be empty if
627-
// at least one of the listeners is valid. Otherwise, it will return the failure condition.
628-
// (2) If it fails to attach the route, it will return false and the failure condition.
629-
func tryToAttachL4RouteToListeners(
610+
// tryToAttachL4RouteToListener attempts to attach a L4 route to the given listener.
611+
// Returns a condition and whether the attachment was successful.
612+
func tryToAttachL4RouteToListener(
630613
refStatus *ParentRefAttachmentStatus,
631-
attachableListeners []*Listener,
614+
attachableListener*Listener,
632615
route *L4Route,
633616
gw *Gateway,
634617
namespaces map[types.NamespacedName]*apiv1.Namespace,
635618
portHostnamesMap map[string]struct{},
636619
) (conditions.Condition, bool) {
637-
if len(attachableListeners) == 0 {
620+
if attachableListener == nil {
638621
return conditions.NewRouteInvalidListener(), false
639622
}
640623

641-
var (
642-
attachedToAtLeastOneValidListener bool
643-
allowed, attached, hostnamesUnique bool
624+
allowed, attached, hostnamesUnique := bindToListenerL4(
625+
attachableListener,
626+
route,
627+
gw,
628+
namespaces,
629+
portHostnamesMap,
630+
refStatus,
644631
)
645632

646-
// Sorting the listeners from most specific hostname to the least specific hostname
647-
sort.Slice(attachableListeners, func(i, j int) bool {
648-
h1 := ""
649-
h2 := ""
650-
if attachableListeners[i].Source.Hostname != nil {
651-
h1 = string(*attachableListeners[i].Source.Hostname)
652-
}
653-
if attachableListeners[j].Source.Hostname != nil {
654-
h2 = string(*attachableListeners[j].Source.Hostname)
655-
}
656-
return h1 == GetMoreSpecificHostname(h1, h2)
657-
})
658-
659-
for _, l := range attachableListeners {
660-
routeAllowed, routeAttached, routeHostnamesUnique := bindToListenerL4(
661-
l,
662-
route,
663-
gw,
664-
namespaces,
665-
portHostnamesMap,
666-
refStatus,
667-
)
668-
allowed = allowed || routeAllowed
669-
attached = attached || routeAttached
670-
hostnamesUnique = hostnamesUnique || routeHostnamesUnique
671-
attachedToAtLeastOneValidListener = attachedToAtLeastOneValidListener || (routeAttached && l.Valid)
672-
}
673-
674633
if !attached {
675634
if !allowed {
676635
return conditions.NewRouteNotAllowedByListeners(), false
@@ -681,7 +640,7 @@ func tryToAttachL4RouteToListeners(
681640
return conditions.NewRouteNoMatchingListenerHostname(), false
682641
}
683642

684-
if !attachedToAtLeastOneValidListener {
643+
if !attachableListener.Valid {
685644
return conditions.NewRouteInvalidListener(), true
686645
}
687646

@@ -754,7 +713,7 @@ func bindL7RouteToListeners(
754713
continue
755714
}
756715

757-
attachment, attachableListeners := validateParentRef(ref, gw)
716+
attachment, attachableListener := validateParentRef(ref, gw)
758717

759718
if route.RouteType == RouteTypeGRPC && isHTTP2Disabled(gw.EffectiveNginxProxy) {
760719
msg := "HTTP2 is disabled - cannot configure GRPCRoutes"
@@ -775,11 +734,10 @@ func bindL7RouteToListeners(
775734
}
776735
}
777736

778-
// Try to attach Route to all matching listeners
779-
780-
cond, attached := tryToAttachL7RouteToListeners(
737+
// Try to attach Route to the matching listener
738+
cond, attached := tryToAttachL7RouteToListener(
781739
ref.Attachment,
782-
attachableListeners,
740+
attachableListener,
783741
route,
784742
gw,
785743
namespaces,
@@ -808,19 +766,16 @@ func isHTTP2Disabled(npCfg *EffectiveNginxProxy) bool {
808766
return *npCfg.DisableHTTP2
809767
}
810768

811-
// tryToAttachRouteToListeners tries to attach the route to the listeners that match the parentRef and the hostnames.
812-
// There are two cases:
813-
// (1) If it succeeds in attaching at least one listener it will return true. The returned condition will be empty if
814-
// at least one of the listeners is valid. Otherwise, it will return the failure condition.
815-
// (2) If it fails to attach the route, it will return false and the failure condition.
816-
func tryToAttachL7RouteToListeners(
769+
// tryToAttachL7RouteToListener attempts to attach a L7 route to the given listener.
770+
// Returns a condition and whether the attachment was successful.
771+
func tryToAttachL7RouteToListener(
817772
refStatus *ParentRefAttachmentStatus,
818-
attachableListeners []*Listener,
773+
attachableListener*Listener,
819774
route *L7Route,
820775
gw *Gateway,
821776
namespaces map[types.NamespacedName]*apiv1.Namespace,
822777
) (conditions.Condition, bool) {
823-
if len(attachableListeners) == 0 {
778+
if attachableListener == nil {
824779
return conditions.NewRouteInvalidListener(), false
825780
}
826781

@@ -848,15 +803,7 @@ func tryToAttachL7RouteToListeners(
848803
return true, true
849804
}
850805

851-
var attachedToAtLeastOneValidListener bool
852-
853-
var allowed, attached bool
854-
for _, l := range attachableListeners {
855-
routeAllowed, routeAttached := bind(l)
856-
allowed = allowed || routeAllowed
857-
attached = attached || routeAttached
858-
attachedToAtLeastOneValidListener = attachedToAtLeastOneValidListener || (routeAttached && l.Valid)
859-
}
806+
allowed, attached := bind(attachableListener)
860807

861808
if !attached {
862809
if !allowed {
@@ -865,57 +812,31 @@ func tryToAttachL7RouteToListeners(
865812
return conditions.NewRouteNoMatchingListenerHostname(), false
866813
}
867814

868-
if !attachedToAtLeastOneValidListener {
815+
if !attachableListener.Valid {
869816
return conditions.NewRouteInvalidListener(), true
870817
}
871818

872819
return conditions.Condition{}, true
873820
}
874821

875-
// findAttachableListeners returns a list of attachable listeners and whether the listener exists for a non-empty
876-
// sectionName.
877-
func findAttachableListeners(ref *ParentRef, listeners []*Listener) ([]*Listener, bool) {
878-
sectionName := getSectionName(ref.SectionName)
879-
880-
// Case 1: sectionName is specified - look for that specific listener
881-
if sectionName != "" {
882-
for _, l := range listeners {
883-
if l.Name == sectionName {
884-
if l.Attachable && (ref.Port == nil || l.Source.Port == *ref.Port) {
885-
return []*Listener{l}, true
886-
}
887-
if ref.Port != nil && l.Source.Port != *ref.Port {
888-
return nil, false
889-
}
890-
return nil, true
891-
}
892-
}
893-
return nil, false
894-
}
895-
896-
// Case 2: Only port is specified - find all attachable listeners matching that port
897-
if ref.Port != nil {
898-
var attachableListeners []*Listener
899-
var foundListener bool
900-
for _, l := range listeners {
901-
if l.Source.Port == *ref.Port {
902-
foundListener = true
903-
if l.Attachable {
904-
attachableListeners = append(attachableListeners, l)
905-
}
906-
}
907-
}
908-
return attachableListeners, foundListener
909-
}
822+
// findAttachableListener returns the attachable listener that matches the sectionName and port.
823+
// Since buildSectionNameRefs always expands to concrete sectionName+port combinations,
824+
// we only need to handle the sectionName case.
825+
func findAttachableListener(ref *ParentRef, listeners []*Listener) (*Listener, bool) {
826+
sectionName := string(*ref.SectionName)
910827

911-
// Case 3: Neither sectionName nor port specified - return all attachable listeners
912-
var attachableListeners []*Listener
913828
for _, l := range listeners {
914-
if l.Attachable {
915-
attachableListeners = append(attachableListeners, l)
829+
if l.Name == sectionName {
830+
if l.Attachable && (ref.Port == nil || l.Source.Port == *ref.Port) {
831+
return l, true
832+
}
833+
if ref.Port != nil && l.Source.Port != *ref.Port {
834+
return nil, false
835+
}
836+
return nil, true
916837
}
917838
}
918-
return attachableListeners, len(listeners) >0
839+
return nil, false
919840
}
920841

921842
func findAcceptedHostnames(listenerHostname *v1.Hostname, routeHostnames []v1.Hostname) []string {
@@ -1057,13 +978,6 @@ func getHostname(h *v1.Hostname) string {
1057978
return string(*h)
1058979
}
1059980

1060-
func getSectionName(s *v1.SectionName) string {
1061-
if s == nil {
1062-
return ""
1063-
}
1064-
return string(*s)
1065-
}
1066-
1067981
func validateHostnames(hostnames []v1.Hostname, path *field.Path) error {
1068982
var allErrs field.ErrorList
1069983

0 commit comments

Comments
(0)

AltStyle によって変換されたページ (->オリジナル) /