Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Disallow route to attach to listener if not present in allowed routes. #2314

Merged
merged 7 commits into from
Aug 5, 2024
+404 −208
Merged

Disallow route to attach to listener if not present in allowed routes. #2314

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
b16e0cb
reject route if not allowed by listener
salonichf5 Jul 30, 2024
d2224bb
allowed routes rules updated
salonichf5 Aug 1, 2024
aea34cd
update tests
salonichf5 Aug 1, 2024
d4ba0cf
fix logic
salonichf5 Aug 1, 2024
175ac09
add conversion function
salonichf5 Aug 1, 2024
be047a9
modify getAndValidateSupportedKind
salonichf5 Aug 1, 2024
38538dc
fix test description
salonichf5 Aug 2, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
20 changes: 13 additions & 7 deletions internal/mode/static/state/change_processor_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -519,12 +519,15 @@ var _ = Describe("ChangeProcessor", func() {
Source: gw1,
Listeners: []*graph.Listener{
{
Name: "listener-80-1",
Source: gw1.Spec.Listeners[0],
Valid: true,
Attachable: true,
Routes: map[graph.RouteKey]*graph.L7Route{routeKey1: expRouteHR1},
SupportedKinds: []v1.RouteGroupKind{{Kind: kinds.HTTPRoute}},
Name: "listener-80-1",
Source: gw1.Spec.Listeners[0],
Valid: true,
Attachable: true,
Routes: map[graph.RouteKey]*graph.L7Route{routeKey1: expRouteHR1},
SupportedKinds: []v1.RouteGroupKind{
{Kind: v1.Kind(kinds.HTTPRoute), Group: helpers.GetPointer[v1.Group](v1.GroupName)},
{Kind: v1.Kind(kinds.GRPCRoute), Group: helpers.GetPointer[v1.Group](v1.GroupName)},
},
},
{
Name: "listener-443-1",
Expand All @@ -533,7 +536,10 @@ var _ = Describe("ChangeProcessor", func() {
Attachable: true,
Routes: map[graph.RouteKey]*graph.L7Route{routeKey1: expRouteHR1},
ResolvedSecret: helpers.GetPointer(client.ObjectKeyFromObject(diffNsTLSSecret)),
SupportedKinds: []v1.RouteGroupKind{{Kind: kinds.HTTPRoute}},
SupportedKinds: []v1.RouteGroupKind{
{Kind: v1.Kind(kinds.HTTPRoute), Group: helpers.GetPointer[v1.Group](v1.GroupName)},
{Kind: v1.Kind(kinds.GRPCRoute), Group: helpers.GetPointer[v1.Group](v1.GroupName)},
},
},
},
Valid: true,
Expand Down
31 changes: 18 additions & 13 deletions internal/mode/static/state/graph/gateway_listener.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import (
v1 "sigs.k8s.io/gateway-api/apis/v1"

"github.com/nginxinc/nginx-gateway-fabric/internal/framework/conditions"
"github.com/nginxinc/nginx-gateway-fabric/internal/framework/helpers"
"github.com/nginxinc/nginx-gateway-fabric/internal/framework/kinds"
staticConds "github.com/nginxinc/nginx-gateway-fabric/internal/mode/static/state/conditions"
)
Expand Down Expand Up @@ -224,22 +225,17 @@ func validateListenerHostname(listener v1.Listener) (conds []conditions.Conditio
return nil, true
}

// getAndValidateListenerSupportedKinds validates the route kind and returns the supported kinds for the listener.
// The supported kinds are determined based on the listener's allowedRoutes field.
// If the listener does not specify allowedRoutes, listener determines allowed routes based on its protocol.
func getAndValidateListenerSupportedKinds(listener v1.Listener) (
[]conditions.Condition,
[]v1.RouteGroupKind,
) {
if listener.AllowedRoutes == nil || listener.AllowedRoutes.Kinds == nil {
return nil, []v1.RouteGroupKind{
{
Kind: kinds.HTTPRoute,
},
}
}
var conds []conditions.Condition
var supportedKinds []v1.RouteGroupKind

supportedKinds := make([]v1.RouteGroupKind, 0, len(listener.AllowedRoutes.Kinds))

validHTTPProtocolRouteKind := func(kind v1.RouteGroupKind) bool {
validRouteKind := func(kind v1.RouteGroupKind) bool {
if kind.Kind != v1.Kind(kinds.HTTPRoute) && kind.Kind != v1.Kind(kinds.GRPCRoute) {
return false
}
Expand All @@ -249,17 +245,26 @@ func getAndValidateListenerSupportedKinds(listener v1.Listener) (
return true
}

switch listener.Protocol {
case v1.HTTPProtocolType, v1.HTTPSProtocolType:
if listener.AllowedRoutes != nil && listener.AllowedRoutes.Kinds != nil {
supportedKinds = make([]v1.RouteGroupKind, 0, len(listener.AllowedRoutes.Kinds))
for _, kind := range listener.AllowedRoutes.Kinds {
if !validHTTPProtocolRouteKind(kind) {
if !validRouteKind(kind) {
msg := fmt.Sprintf("Unsupported route kind \"%s/%s\"", *kind.Group, kind.Kind)
conds = append(conds, staticConds.NewListenerInvalidRouteKinds(msg)...)
continue
}
supportedKinds = append(supportedKinds, kind)
}
} else {
switch listener.Protocol {
case v1.HTTPProtocolType, v1.HTTPSProtocolType:
supportedKinds = []v1.RouteGroupKind{
{Kind: v1.Kind(kinds.HTTPRoute), Group: helpers.GetPointer[v1.Group](v1.GroupName)},
{Kind: v1.Kind(kinds.GRPCRoute), Group: helpers.GetPointer[v1.Group](v1.GroupName)},
}
}
}

return conds, supportedKinds
}

Expand Down
34 changes: 15 additions & 19 deletions internal/mode/static/state/graph/gateway_listener_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -289,11 +289,13 @@ func TestValidateListenerHostname(t *testing.T) {
}

func TestGetAndValidateListenerSupportedKinds(t *testing.T) {
HTTPRouteGroupKind := []v1.RouteGroupKind{
{
Kind: kinds.HTTPRoute,
Group: helpers.GetPointer[v1.Group](v1.GroupName),
},
HTTPRouteGroupKind := v1.RouteGroupKind{
Kind: kinds.HTTPRoute,
Group: helpers.GetPointer[v1.Group](v1.GroupName),
}
GRPCRouteGroupKind := v1.RouteGroupKind{
Kind: kinds.GRPCRoute,
Group: helpers.GetPointer[v1.Group](v1.GroupName),
}
TCPRouteGroupKind := []v1.RouteGroupKind{
{
Expand All @@ -312,8 +314,7 @@ func TestGetAndValidateListenerSupportedKinds(t *testing.T) {
protocol: v1.TCPProtocolType,
expectErr: false,
name: "unsupported protocol is ignored",
kind: TCPRouteGroupKind,
expected: []v1.RouteGroupKind{},
expected: nil,
},
{
protocol: v1.HTTPProtocolType,
Expand All @@ -336,43 +337,38 @@ func TestGetAndValidateListenerSupportedKinds(t *testing.T) {
},
{
protocol: v1.HTTPProtocolType,
kind: HTTPRouteGroupKind,
kind: []v1.RouteGroupKind{HTTPRouteGroupKind},
expectErr: false,
name: "valid HTTP",
expected: HTTPRouteGroupKind,
expected: []v1.RouteGroupKind{HTTPRouteGroupKind},
},
{
protocol: v1.HTTPSProtocolType,
kind: HTTPRouteGroupKind,
kind: []v1.RouteGroupKind{HTTPRouteGroupKind},
expectErr: false,
name: "valid HTTPS",
expected: HTTPRouteGroupKind,
expected: []v1.RouteGroupKind{HTTPRouteGroupKind},
},
{
protocol: v1.HTTPSProtocolType,
expectErr: false,
name: "valid HTTPS no kind specified",
expected: []v1.RouteGroupKind{
{
Kind: kinds.HTTPRoute,
},
HTTPRouteGroupKind, GRPCRouteGroupKind,
},
},
{
protocol: v1.HTTPProtocolType,
kind: []v1.RouteGroupKind{
{
Kind: kinds.HTTPRoute,
Group: helpers.GetPointer[v1.Group](v1.GroupName),
},
HTTPRouteGroupKind,
{
Kind: "bad-kind",
Group: helpers.GetPointer[v1.Group](v1.GroupName),
},
},
expectErr: true,
name: "valid and invalid kinds",
expected: HTTPRouteGroupKind,
expected: []v1.RouteGroupKind{HTTPRouteGroupKind},
},
}

Expand Down
Loading
Loading