Skip to content

Commit c9ae428

Browse files
committed
Address code review comments
1 parent a7e9047 commit c9ae428

File tree

4 files changed

+31
-8
lines changed

4 files changed

+31
-8
lines changed

internal/state/dataplane/configuration.go

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -232,7 +232,7 @@ func (hpr *hostPathRules) upsertListener(l *graph.Listener) {
232232
for _, h := range hostnames {
233233
if prevListener, exists := hpr.listenersForHost[h]; exists {
234234
// override the previous listener if the new one has a more specific hostname
235-
if hostnameMoreSpecific(l.Source.Hostname, prevListener.Source.Hostname) {
235+
if listenerHostnameMoreSpecific(l.Source.Hostname, prevListener.Source.Hostname) {
236236
hpr.listenersForHost[h] = l
237237
}
238238
} else {
@@ -443,7 +443,7 @@ func convertPathType(pathType v1beta1.PathMatchType) PathType {
443443
}
444444
}
445445

446-
// Returns true if host1 is more specific than host2 (using length).
446+
// listenerHostnameMoreSpecific returns true if host1 is more specific than host2 (using length).
447447
//
448448
// FIXME(sberman): Since the only caller of this function specifies listener hostnames that are both
449449
// bound to the same route hostname, this function assumes that host1 and host2 match, either
@@ -458,7 +458,7 @@ func convertPathType(pathType v1beta1.PathMatchType) PathType {
458458
// As we add regex support, we should put in the proper
459459
// validation and error handling for this function to ensure that the hostnames are actually matching,
460460
// to avoid the unintended inputs above for the invalid case.
461-
func hostnameMoreSpecific(host1, host2 *v1beta1.Hostname) bool {
461+
func listenerHostnameMoreSpecific(host1, host2 *v1beta1.Hostname) bool {
462462
var host1Str, host2Str string
463463
if host1 == nil {
464464
host1Str = ""

internal/state/dataplane/configuration_test.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1749,6 +1749,6 @@ func TestHostnameMoreSpecific(t *testing.T) {
17491749
}
17501750

17511751
for _, tc := range tests {
1752-
g.Expect(hostnameMoreSpecific(tc.host1, tc.host2)).To(Equal(tc.host1Wins), tc.msg)
1752+
g.Expect(listenerHostnameMoreSpecific(tc.host1, tc.host2)).To(Equal(tc.host1Wins), tc.msg)
17531753
}
17541754
}

internal/state/graph/httproute.go

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -340,10 +340,10 @@ func bindRouteToListeners(r *Route, gw *Gateway) {
340340
return true
341341
}
342342

343-
var valid bool
343+
var validListener bool
344344
if getSectionName(routeRef.SectionName) == "" {
345345
for _, l := range gw.Listeners {
346-
valid = bind(l) || valid
346+
validListener = bind(l) || validListener
347347
}
348348
} else {
349349
l, exists := gw.Listeners[string(*routeRef.SectionName)]
@@ -354,9 +354,9 @@ func bindRouteToListeners(r *Route, gw *Gateway) {
354354
continue
355355
}
356356

357-
valid = bind(l)
357+
validListener = bind(l)
358358
}
359-
if !valid {
359+
if !validListener {
360360
attachment.FailedCondition = conditions.NewRouteInvalidListener()
361361
continue
362362
}

internal/state/graph/httproute_test.go

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -872,6 +872,29 @@ func TestBindRouteToListeners(t *testing.T) {
872872
},
873873
name: "section name is empty",
874874
},
875+
{
876+
route: routeWithEmptySectionName,
877+
gateway: &Gateway{
878+
Source: gw,
879+
Listeners: map[string]*Listener{
880+
"listener-80-1": notValidListener,
881+
},
882+
},
883+
expectedSectionNameRefs: []ParentRef{
884+
{
885+
Idx: 0,
886+
Gateway: client.ObjectKeyFromObject(gw),
887+
Attachment: &ParentRefAttachmentStatus{
888+
Attached: false,
889+
FailedCondition: conditions.NewRouteInvalidListener(),
890+
},
891+
},
892+
},
893+
expectedGatewayListeners: map[string]*Listener{
894+
"listener-80-1": notValidListener,
895+
},
896+
name: "empty section name with no valid listeners",
897+
},
875898
{
876899
route: routeWithPort,
877900
gateway: &Gateway{

0 commit comments

Comments
 (0)