Skip to content

Commit

Permalink
allowed routes rules updated
Browse files Browse the repository at this point in the history
  • Loading branch information
salonichf5 committed Aug 1, 2024
1 parent eaf136a commit 6f86442
Show file tree
Hide file tree
Showing 2 changed files with 98 additions and 56 deletions.
27 changes: 14 additions & 13 deletions internal/mode/static/state/graph/route_common.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@ import (

apiv1 "k8s.io/api/core/v1"
"k8s.io/apimachinery/pkg/labels"
"k8s.io/apimachinery/pkg/runtime/schema"
"k8s.io/apimachinery/pkg/types"
"k8s.io/apimachinery/pkg/util/validation/field"
"sigs.k8s.io/controller-runtime/pkg/client"
Expand Down Expand Up @@ -359,7 +358,7 @@ func tryToAttachRouteToListeners(
return false, false
}

if !isRouteKindAllowedByListener(l, route.Source.GetObjectKind()) {
if !isRouteTypeAllowedByListener(l, route.RouteType) {
return false, false
}

Expand Down Expand Up @@ -535,24 +534,26 @@ func isRouteNamespaceAllowedByListener(
return true
}

// isRouteKindAllowedByListener checks if the route kind is allowed by the listener.
// If the listener does not specify allowed kinds, all kinds can attach to it.
// isRouteKindAllowedByListener checks if the route is allowed to attach to the listener.
// If the listener specifies allowed kinds, the route kind must be in the list.
// If the listener specifies HTTPRoute, a GRPCRoute can be attached to it.
func isRouteKindAllowedByListener(listener *Listener, routeKind schema.ObjectKind) bool {
// If the listener does not specify allowedRoutes, allowed routes are determined using the listener protocol.
func isRouteTypeAllowedByListener(listener *Listener, routeType RouteType) bool {
if listener.Source.AllowedRoutes != nil && listener.Source.AllowedRoutes.Kinds != nil {
for _, kind := range listener.Source.AllowedRoutes.Kinds {
routeKind := v1.Kind(routeKind.GroupVersionKind().Kind)
if kind.Kind == routeKind {
return true
}
if kind.Kind == kinds.HTTPRoute && routeKind == kinds.GRPCRoute {
return true
switch kind.Kind {
case kinds.HTTPRoute:
return routeType == RouteTypeHTTP
case kinds.GRPCRoute:
return routeType == RouteTypeGRPC
}
}
return false

Check warning on line 550 in internal/mode/static/state/graph/route_common.go

View check run for this annotation

Codecov / codecov/patch

internal/mode/static/state/graph/route_common.go#L550

Added line #L550 was not covered by tests
}
return true

if listener.Source.Protocol == v1.HTTPProtocolType || listener.Source.Protocol == v1.HTTPSProtocolType {
return routeType == RouteTypeHTTP || routeType == RouteTypeGRPC
}
return false

Check warning on line 556 in internal/mode/static/state/graph/route_common.go

View check run for this annotation

Codecov / codecov/patch

internal/mode/static/state/graph/route_common.go#L556

Added line #L556 was not covered by tests
}

func getHostname(h *v1.Hostname) string {
Expand Down
127 changes: 84 additions & 43 deletions internal/mode/static/state/graph/route_common_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -228,6 +228,7 @@ func TestBindRouteToListeners(t *testing.T) {
Source: gatewayv1.Listener{
Name: gatewayv1.SectionName(name),
Hostname: (*gatewayv1.Hostname)(helpers.GetPointer("foo.example.com")),
Protocol: gatewayv1.HTTPProtocolType,
},
Valid: true,
Attachable: true,
Expand Down Expand Up @@ -1116,44 +1117,6 @@ func TestBindRouteToListeners(t *testing.T) {
},
name: "route allowed via all namespaces",
},
{
route: createNormalHTTPRoute(gw),
gateway: &Gateway{
Source: gw,
Valid: true,
Listeners: []*Listener{
createModifiedListener("listener-80-1", func(l *Listener) {
l.Source.AllowedRoutes = &gatewayv1.AllowedRoutes{
Kinds: []gatewayv1.RouteGroupKind{
{Kind: "GRPCRoute"},
},
}
}),
},
},
expectedSectionNameRefs: []ParentRef{
{
Idx: 0,
Gateway: client.ObjectKeyFromObject(gw),
SectionName: hr.Spec.ParentRefs[0].SectionName,
Attachment: &ParentRefAttachmentStatus{
Attached: false,
FailedCondition: staticConds.NewRouteNotAllowedByListeners(),
AcceptedHostnames: map[string][]string{},
},
},
},
expectedGatewayListeners: []*Listener{
createModifiedListener("listener-80-1", func(l *Listener) {
l.Source.AllowedRoutes = &gatewayv1.AllowedRoutes{
Kinds: []gatewayv1.RouteGroupKind{
{Kind: "GRPCRoute"},
},
}
}),
},
name: "http route not allowed when listener allows only grpc routes",
},
{
route: createNormalGRPCRoute(gw),
gateway: &Gateway{
Expand All @@ -1178,10 +1141,9 @@ func TestBindRouteToListeners(t *testing.T) {
Gateway: client.ObjectKeyFromObject(gw),
SectionName: gr.Spec.ParentRefs[0].SectionName,
Attachment: &ParentRefAttachmentStatus{
Attached: true,
AcceptedHostnames: map[string][]string{
"listener-80-1": {"foo.example.com"},
},
Attached: false,
FailedCondition: staticConds.NewRouteNotAllowedByListeners(),
AcceptedHostnames: map[string][]string{},
},
},
},
Expand All @@ -1197,7 +1159,7 @@ func TestBindRouteToListeners(t *testing.T) {
}
}),
},
name: "grpc route allowed when listener kind is HTTPRoute",
name: "grpc route not allowed when listener kind is HTTPRoute",
},
{
route: createNormalHTTPRoute(gw),
Expand Down Expand Up @@ -1772,3 +1734,82 @@ func TestRouteKeyForKind(t *testing.T) {

g.Expect(rk).To(Panic())
}

func TestAllowedRouteType(t *testing.T) {
test := []struct {
listener *Listener
name string
routeType RouteType
expResult bool
}{
{
name: "httpRoute with listener protocol http",
routeType: RouteTypeHTTP,
listener: &Listener{
Source: gatewayv1.Listener{
Protocol: gatewayv1.HTTPProtocolType,
},
},
expResult: true,
},
{
name: "grpcRoute with listener protocol https",
routeType: RouteTypeGRPC,
listener: &Listener{
Source: gatewayv1.Listener{
Protocol: gatewayv1.HTTPSProtocolType,
},
},
expResult: true,
},
{
name: "grpcRoute with listener allowedRoutes set to httpRoute is not allowed",
routeType: RouteTypeGRPC,
listener: &Listener{
Source: gatewayv1.Listener{
AllowedRoutes: &gatewayv1.AllowedRoutes{
Kinds: []gatewayv1.RouteGroupKind{
{Kind: kinds.HTTPRoute},
},
},
},
},
expResult: false,
},
{
name: "httpRoute with listener allowedRoutes set to grpcRoute is not allowed",
routeType: RouteTypeHTTP,
listener: &Listener{
Source: gatewayv1.Listener{
AllowedRoutes: &gatewayv1.AllowedRoutes{
Kinds: []gatewayv1.RouteGroupKind{
{Kind: kinds.GRPCRoute},
},
},
},
},
expResult: false,
},
{
name: "grpcRoute with listener allowedRoutes set to grpcRoute is allowed",
routeType: RouteTypeGRPC,
listener: &Listener{
Source: gatewayv1.Listener{
AllowedRoutes: &gatewayv1.AllowedRoutes{
Kinds: []gatewayv1.RouteGroupKind{
{Kind: kinds.GRPCRoute},
},
},
},
},
expResult: true,
},
}

for _, test := range test {
t.Run(test.name, func(t *testing.T) {
g := NewWithT(t)
g.Expect(isRouteTypeAllowedByListener(test.listener, test.routeType)).To(Equal(test.expResult))
})
}
}

0 comments on commit 6f86442

Please sign in to comment.