Skip to content

Commit 012cc3f

Browse files
committed
Refactor build section name refs
1 parent c7b0836 commit 012cc3f

File tree

3 files changed

+24
-118
lines changed

3 files changed

+24
-118
lines changed

internal/controller/state/graph/graph_test.go

Lines changed: 2 additions & 80 deletions
Original file line numberDiff line numberDiff line change
@@ -776,32 +776,6 @@ func TestBuildGraph(t *testing.T) {
776776
Attachable: true,
777777
Source: tr,
778778
ParentRefs: []ParentRef{
779-
{
780-
Idx: 0,
781-
Gateway: &ParentRefGateway{
782-
NamespacedName: client.ObjectKeyFromObject(gw1.Source),
783-
EffectiveNginxProxy: np1Effective,
784-
},
785-
Attachment: &ParentRefAttachmentStatus{
786-
AcceptedHostnames: map[string][]string{},
787-
Attached: false,
788-
FailedConditions: []conditions.Condition{conditions.NewRouteNotAllowedByListeners()},
789-
},
790-
SectionName: &gw1.Source.Spec.Listeners[0].Name,
791-
},
792-
{
793-
Idx: 0,
794-
Gateway: &ParentRefGateway{
795-
NamespacedName: client.ObjectKeyFromObject(gw1.Source),
796-
EffectiveNginxProxy: np1Effective,
797-
},
798-
Attachment: &ParentRefAttachmentStatus{
799-
AcceptedHostnames: map[string][]string{},
800-
Attached: false,
801-
FailedConditions: []conditions.Condition{conditions.NewRouteNotAllowedByListeners()},
802-
},
803-
SectionName: &gw1.Source.Spec.Listeners[1].Name,
804-
},
805779
{
806780
Idx: 0,
807781
Gateway: &ParentRefGateway{
@@ -815,26 +789,13 @@ func TestBuildGraph(t *testing.T) {
815789
client.ObjectKeyFromObject(gw1.Source),
816790
"listener-443-2",
817791
): {"fizz.example.org"},
818-
},
819-
},
820-
SectionName: &gw1.Source.Spec.Listeners[2].Name,
821-
},
822-
{
823-
Idx: 0,
824-
Gateway: &ParentRefGateway{
825-
NamespacedName: client.ObjectKeyFromObject(gw1.Source),
826-
EffectiveNginxProxy: np1Effective,
827-
},
828-
Attachment: &ParentRefAttachmentStatus{
829-
Attached: true,
830-
AcceptedHostnames: map[string][]string{
831792
CreateGatewayListenerKey(
832793
client.ObjectKeyFromObject(gw1.Source),
833794
"listener-8443",
834795
): {"fizz.example.org"},
835796
},
836797
},
837-
SectionName: &gw1.Source.Spec.Listeners[3].Name,
798+
SectionName: nil, // No expansion - represents attachment to all listeners
838799
},
839800
},
840801
Spec: L4RouteSpec{
@@ -858,45 +819,6 @@ func TestBuildGraph(t *testing.T) {
858819
Attachable: true,
859820
Source: tr2,
860821
ParentRefs: []ParentRef{
861-
{
862-
Idx: 0,
863-
Gateway: &ParentRefGateway{
864-
NamespacedName: client.ObjectKeyFromObject(gw1.Source),
865-
EffectiveNginxProxy: np1Effective,
866-
},
867-
Attachment: &ParentRefAttachmentStatus{
868-
Attached: false,
869-
AcceptedHostnames: map[string][]string{},
870-
FailedConditions: []conditions.Condition{conditions.NewRouteNotAllowedByListeners()},
871-
},
872-
SectionName: &gw1.Source.Spec.Listeners[0].Name,
873-
},
874-
{
875-
Idx: 0,
876-
Gateway: &ParentRefGateway{
877-
NamespacedName: client.ObjectKeyFromObject(gw1.Source),
878-
EffectiveNginxProxy: np1Effective,
879-
},
880-
Attachment: &ParentRefAttachmentStatus{
881-
AcceptedHostnames: map[string][]string{},
882-
Attached: false,
883-
FailedConditions: []conditions.Condition{conditions.NewRouteNotAllowedByListeners()},
884-
},
885-
SectionName: &gw1.Source.Spec.Listeners[1].Name,
886-
},
887-
{
888-
Idx: 0,
889-
Gateway: &ParentRefGateway{
890-
NamespacedName: client.ObjectKeyFromObject(gw1.Source),
891-
EffectiveNginxProxy: np1Effective,
892-
},
893-
Attachment: &ParentRefAttachmentStatus{
894-
Attached: false,
895-
AcceptedHostnames: map[string][]string{},
896-
FailedConditions: []conditions.Condition{conditions.NewRouteHostnameConflict()},
897-
},
898-
SectionName: &gw1.Source.Spec.Listeners[2].Name,
899-
},
900822
{
901823
Idx: 0,
902824
Gateway: &ParentRefGateway{
@@ -908,7 +830,7 @@ func TestBuildGraph(t *testing.T) {
908830
AcceptedHostnames: map[string][]string{},
909831
FailedConditions: []conditions.Condition{conditions.NewRouteHostnameConflict()},
910832
},
911-
SectionName: &gw1.Source.Spec.Listeners[3].Name,
833+
SectionName: nil, // No expansion - represents attachment to all listeners
912834
},
913835
},
914836
Spec: L4RouteSpec{

internal/controller/state/graph/route_common.go

Lines changed: 21 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -303,47 +303,36 @@ func buildSectionNameRefs(
303303
}
304304

305305
gwNsName := client.ObjectKeyFromObject(gw.Source)
306-
k := key{
307-
gwNsName: gwNsName,
308-
}
309306

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
307+
// If there is no section name, validate uniqueness by expanding in the uniqueness map only
313308
if p.SectionName == nil {
314-
// If port is specified, preserve the port-only nature for proper validation
315-
if p.Port != nil {
316-
sectionNameRefs = append(sectionNameRefs, ParentRef{
317-
Idx: i,
318-
Gateway: CreateParentRefGateway(gw),
319-
SectionName: nil, // Keep as nil to preserve port-only semantics
320-
Port: p.Port,
321-
})
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-
}
309+
// Check uniqueness for all listener names in this gateway
310+
for _, l := range gw.Listeners {
311+
k := key{
312+
gwNsName: gwNsName,
313+
sectionName: string(l.Source.Name),
314+
}
330315

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-
})
316+
if err := checkUniqueSections(k); err != nil {
317+
return nil, err
340318
}
341319
}
342320

321+
// Create a single parentRef without expansion
322+
sectionNameRefs = append(sectionNameRefs, ParentRef{
323+
Idx: i,
324+
Gateway: CreateParentRefGateway(gw),
325+
SectionName: nil, // Keep as nil to indicate attachment to multiple listeners
326+
Port: p.Port,
327+
})
328+
343329
continue
344330
}
345331

346-
k.sectionName = string(*p.SectionName)
332+
k := key{
333+
gwNsName: gwNsName,
334+
sectionName: string(*p.SectionName),
335+
}
347336
if err := checkUniqueSections(k); err != nil {
348337
return nil, err
349338
}

internal/controller/state/graph/route_common_test.go

Lines changed: 1 addition & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -121,12 +121,7 @@ func TestBuildSectionNameRefs(t *testing.T) {
121121
{
122122
Idx: 6,
123123
Gateway: CreateParentRefGateway(gws[gwNsName3]),
124-
SectionName: helpers.GetPointer[gatewayv1.SectionName]("http"),
125-
},
126-
{
127-
Idx: 6,
128-
Gateway: CreateParentRefGateway(gws[gwNsName3]),
129-
SectionName: helpers.GetPointer[gatewayv1.SectionName]("https"),
124+
SectionName: nil, // No expansion - keep as nil to represent attachment to all listeners
130125
},
131126
}
132127

0 commit comments

Comments
 (0)