Skip to content

Commit 156f9d8

Browse files
salonichf5salonichf5
authored
Disallow route to attach to listener if not present in allowed routes. (#2314)
Problem: NGF allows all route kinds to attach to a listener regardless of the kinds specified in the listener AllowedRoutes.Kinds field Solution: Add check to reject a route trying to attach to a listener which doesn't allow its kind.
1 parent 13ea272 commit 156f9d8

File tree

7 files changed

+404
-208
lines changed

7 files changed

+404
-208
lines changed

‎internal/mode/static/state/change_processor_test.go

Lines changed: 13 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -519,12 +519,15 @@ var _ = Describe("ChangeProcessor", func() {
519519
Source: gw1,
520520
Listeners: []*graph.Listener{
521521
{
522-
Name: "listener-80-1",
523-
Source: gw1.Spec.Listeners[0],
524-
Valid: true,
525-
Attachable: true,
526-
Routes: map[graph.RouteKey]*graph.L7Route{routeKey1: expRouteHR1},
527-
SupportedKinds: []v1.RouteGroupKind{{Kind: kinds.HTTPRoute}},
522+
Name: "listener-80-1",
523+
Source: gw1.Spec.Listeners[0],
524+
Valid: true,
525+
Attachable: true,
526+
Routes: map[graph.RouteKey]*graph.L7Route{routeKey1: expRouteHR1},
527+
SupportedKinds: []v1.RouteGroupKind{
528+
{Kind: v1.Kind(kinds.HTTPRoute), Group: helpers.GetPointer[v1.Group](v1.GroupName)},
529+
{Kind: v1.Kind(kinds.GRPCRoute), Group: helpers.GetPointer[v1.Group](v1.GroupName)},
530+
},
528531
},
529532
{
530533
Name: "listener-443-1",
@@ -533,7 +536,10 @@ var _ = Describe("ChangeProcessor", func() {
533536
Attachable: true,
534537
Routes: map[graph.RouteKey]*graph.L7Route{routeKey1: expRouteHR1},
535538
ResolvedSecret: helpers.GetPointer(client.ObjectKeyFromObject(diffNsTLSSecret)),
536-
SupportedKinds: []v1.RouteGroupKind{{Kind: kinds.HTTPRoute}},
539+
SupportedKinds: []v1.RouteGroupKind{
540+
{Kind: v1.Kind(kinds.HTTPRoute), Group: helpers.GetPointer[v1.Group](v1.GroupName)},
541+
{Kind: v1.Kind(kinds.GRPCRoute), Group: helpers.GetPointer[v1.Group](v1.GroupName)},
542+
},
537543
},
538544
},
539545
Valid: true,

‎internal/mode/static/state/graph/gateway_listener.go

Lines changed: 18 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@ import (
1111
v1 "sigs.k8s.io/gateway-api/apis/v1"
1212

1313
"github.com/nginxinc/nginx-gateway-fabric/internal/framework/conditions"
14+
"github.com/nginxinc/nginx-gateway-fabric/internal/framework/helpers"
1415
"github.com/nginxinc/nginx-gateway-fabric/internal/framework/kinds"
1516
staticConds "github.com/nginxinc/nginx-gateway-fabric/internal/mode/static/state/conditions"
1617
)
@@ -224,22 +225,17 @@ func validateListenerHostname(listener v1.Listener) (conds []conditions.Conditio
224225
return nil, true
225226
}
226227

228+
// getAndValidateListenerSupportedKinds validates the route kind and returns the supported kinds for the listener.
229+
// The supported kinds are determined based on the listener's allowedRoutes field.
230+
// If the listener does not specify allowedRoutes, listener determines allowed routes based on its protocol.
227231
func getAndValidateListenerSupportedKinds(listener v1.Listener) (
228232
[]conditions.Condition,
229233
[]v1.RouteGroupKind,
230234
) {
231-
if listener.AllowedRoutes == nil || listener.AllowedRoutes.Kinds == nil {
232-
return nil, []v1.RouteGroupKind{
233-
{
234-
Kind: kinds.HTTPRoute,
235-
},
236-
}
237-
}
238235
var conds []conditions.Condition
236+
var supportedKinds []v1.RouteGroupKind
239237

240-
supportedKinds := make([]v1.RouteGroupKind, 0, len(listener.AllowedRoutes.Kinds))
241-
242-
validHTTPProtocolRouteKind := func(kind v1.RouteGroupKind) bool {
238+
validRouteKind := func(kind v1.RouteGroupKind) bool {
243239
if kind.Kind != v1.Kind(kinds.HTTPRoute) && kind.Kind != v1.Kind(kinds.GRPCRoute) {
244240
return false
245241
}
@@ -249,17 +245,26 @@ func getAndValidateListenerSupportedKinds(listener v1.Listener) (
249245
return true
250246
}
251247

252-
switch listener.Protocol {
253-
case v1.HTTPProtocolType, v1.HTTPSProtocolType:
248+
if listener.AllowedRoutes != nil && listener.AllowedRoutes.Kinds != nil {
249+
supportedKinds = make([]v1.RouteGroupKind, 0, len(listener.AllowedRoutes.Kinds))
254250
for _, kind := range listener.AllowedRoutes.Kinds {
255-
if !validHTTPProtocolRouteKind(kind) {
251+
if !validRouteKind(kind) {
256252
msg := fmt.Sprintf("Unsupported route kind \"%s/%s\"", *kind.Group, kind.Kind)
257253
conds = append(conds, staticConds.NewListenerInvalidRouteKinds(msg)...)
258254
continue
259255
}
260256
supportedKinds = append(supportedKinds, kind)
261257
}
258+
} else {
259+
switch listener.Protocol {
260+
case v1.HTTPProtocolType, v1.HTTPSProtocolType:
261+
supportedKinds = []v1.RouteGroupKind{
262+
{Kind: v1.Kind(kinds.HTTPRoute), Group: helpers.GetPointer[v1.Group](v1.GroupName)},
263+
{Kind: v1.Kind(kinds.GRPCRoute), Group: helpers.GetPointer[v1.Group](v1.GroupName)},
264+
}
265+
}
262266
}
267+
263268
return conds, supportedKinds
264269
}
265270

‎internal/mode/static/state/graph/gateway_listener_test.go

Lines changed: 15 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -289,11 +289,13 @@ func TestValidateListenerHostname(t *testing.T) {
289289
}
290290

291291
func TestGetAndValidateListenerSupportedKinds(t *testing.T) {
292-
HTTPRouteGroupKind := []v1.RouteGroupKind{
293-
{
294-
Kind: kinds.HTTPRoute,
295-
Group: helpers.GetPointer[v1.Group](v1.GroupName),
296-
},
292+
HTTPRouteGroupKind := v1.RouteGroupKind{
293+
Kind: kinds.HTTPRoute,
294+
Group: helpers.GetPointer[v1.Group](v1.GroupName),
295+
}
296+
GRPCRouteGroupKind := v1.RouteGroupKind{
297+
Kind: kinds.GRPCRoute,
298+
Group: helpers.GetPointer[v1.Group](v1.GroupName),
297299
}
298300
TCPRouteGroupKind := []v1.RouteGroupKind{
299301
{
@@ -312,8 +314,7 @@ func TestGetAndValidateListenerSupportedKinds(t *testing.T) {
312314
protocol: v1.TCPProtocolType,
313315
expectErr: false,
314316
name: "unsupported protocol is ignored",
315-
kind: TCPRouteGroupKind,
316-
expected: []v1.RouteGroupKind{},
317+
expected: nil,
317318
},
318319
{
319320
protocol: v1.HTTPProtocolType,
@@ -336,43 +337,38 @@ func TestGetAndValidateListenerSupportedKinds(t *testing.T) {
336337
},
337338
{
338339
protocol: v1.HTTPProtocolType,
339-
kind: HTTPRouteGroupKind,
340+
kind: []v1.RouteGroupKind{HTTPRouteGroupKind},
340341
expectErr: false,
341342
name: "valid HTTP",
342-
expected: HTTPRouteGroupKind,
343+
expected: []v1.RouteGroupKind{HTTPRouteGroupKind},
343344
},
344345
{
345346
protocol: v1.HTTPSProtocolType,
346-
kind: HTTPRouteGroupKind,
347+
kind: []v1.RouteGroupKind{HTTPRouteGroupKind},
347348
expectErr: false,
348349
name: "valid HTTPS",
349-
expected: HTTPRouteGroupKind,
350+
expected: []v1.RouteGroupKind{HTTPRouteGroupKind},
350351
},
351352
{
352353
protocol: v1.HTTPSProtocolType,
353354
expectErr: false,
354355
name: "valid HTTPS no kind specified",
355356
expected: []v1.RouteGroupKind{
356-
{
357-
Kind: kinds.HTTPRoute,
358-
},
357+
HTTPRouteGroupKind, GRPCRouteGroupKind,
359358
},
360359
},
361360
{
362361
protocol: v1.HTTPProtocolType,
363362
kind: []v1.RouteGroupKind{
364-
{
365-
Kind: kinds.HTTPRoute,
366-
Group: helpers.GetPointer[v1.Group](v1.GroupName),
367-
},
363+
HTTPRouteGroupKind,
368364
{
369365
Kind: "bad-kind",
370366
Group: helpers.GetPointer[v1.Group](v1.GroupName),
371367
},
372368
},
373369
expectErr: true,
374370
name: "valid and invalid kinds",
375-
expected: HTTPRouteGroupKind,
371+
expected: []v1.RouteGroupKind{HTTPRouteGroupKind},
376372
},
377373
}
378374

0 commit comments

Comments
 (0)