Skip to content

Commit

Permalink
feat(gateway) confirm valid AllowedRoute
Browse files Browse the repository at this point in the history 
Check that AllowedRoutes from user-provided Listeners meet the criteria
necessary for Kong: Listeners for the same protocol must not specify
different filter requirements because Kong combines all routes into a
single proxy configuration.
  • Loading branch information
@rainest
rainest committed Apr 11, 2022
1 parent 8efcb6b commit ef2fb87
Show file tree
Hide file tree
Showing 2 changed files with 304 additions and 13 deletions.
51 changes: 39 additions & 12 deletions internal/controllers/gateway/gateway_controller.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -311,11 +311,12 @@ func (r *GatewayReconciler) reconcileUnmanagedGateway(ctx context.Context, log l
return ctrl.Result{}, err
}

gatewayListeners, err = mergeAllowedRoutes(gateway.Spec.Listeners, gatewayListeners)
if err != nil {
return ctrl.Result{}, err
if !areAllowedRoutesConsistentByProtocol(gateway.Spec.Listeners) {
return ctrl.Result{}, fmt.Errorf("all listeners for a protocol must use the same AllowedRoutes")
}

gatewayListeners = mergeAllowedRoutes(gateway.Spec.Listeners, gatewayListeners)

debug(log, gateway, "updating the gateway if any changes to listeners occurred")
isChanged, err := r.updateAddressesAndListenersSpec(ctx, gateway, gatewayAddresses, gatewayListeners)
if err != nil {
Expand Down Expand Up @@ -570,21 +571,47 @@ func (r *GatewayReconciler) updateAddressesAndListenersStatus(
return false, nil
}

func mergeAllowedRoutes(source, target []gatewayv1alpha2.Listener) ([]gatewayv1alpha2.Listener, error) {
// mergeAllowedRoutes takes two sets of listeners, scans the source for AllowedRoutes filters, and copies those into
// listeners in the target set that share the same protocol as the source listener. As implemented, it makes no attempt
// to account for same-protocol listeners in the source set having different AllowedRoutes: it assumes
// areAllowedRoutesConsistentByProtocol has been run first because Kong would not support multiple listeners with
// different protocols anyway
func mergeAllowedRoutes(source, target []gatewayv1alpha2.Listener) []gatewayv1alpha2.Listener {
var result []gatewayv1alpha2.Listener
mappings := make(map[string]gatewayv1alpha2.RouteNamespaces)
mappings := make(map[gatewayv1alpha2.ProtocolType]gatewayv1alpha2.RouteNamespaces)
for _, listener := range source {
if _, exists := mappings[fmt.Sprintf("%s:%d", listener.Protocol, listener.Port)]; exists {
return nil, fmt.Errorf("multiple listeners for a single protocol/port combination are not supported: %s:%d",
listener.Protocol, listener.Port)
}
mappings[fmt.Sprintf("%s:%d", listener.Protocol, listener.Port)] = *listener.AllowedRoutes.Namespaces
mappings[listener.Protocol] = *listener.AllowedRoutes.Namespaces
}
for _, listener := range target {
if namespaces, exists := mappings[fmt.Sprintf("%s:%d", listener.Protocol, listener.Port)]; exists {
if namespaces, exists := mappings[listener.Protocol]; exists {
if listener.AllowedRoutes == nil {
listener.AllowedRoutes = &gatewayv1alpha2.AllowedRoutes{}
}
listener.AllowedRoutes.Namespaces = &namespaces
}
result = append(result, listener)
}
return result, nil
return result
}

// areAllowedRoutesConsistentByProtocol returns an error if a set of listeners includes multiple listeners for the same
// protocol that do not use the same AllowedRoutes filters. Kong does not support limiting routes to a specific listen:
// all routes are always served on all listens compatible with their protocol. As such, while we can filter the routes
// we ingest, if we ingest routes from two incompatible filters, we will combine them into a single proxy configuration
// It may be possible to write a new Kong plugin that checks the inbound port/address to de facto apply listen-based
// filters in the future.
func areAllowedRoutesConsistentByProtocol(listeners []gatewayv1alpha2.Listener) bool {
allowedByProtocol := make(map[gatewayv1alpha2.ProtocolType]gatewayv1alpha2.AllowedRoutes)
for _, listener := range listeners {
var allowed gatewayv1alpha2.AllowedRoutes
var exists bool
if allowed, exists = allowedByProtocol[listener.Protocol]; !exists {
allowedByProtocol[listener.Protocol] = *listener.AllowedRoutes
} else {
if !reflect.DeepEqual(allowed, *listener.AllowedRoutes) {
return false
}
}
}
return true
}
266 changes: 265 additions & 1 deletion internal/controllers/gateway/gateway_controller_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -754,6 +754,270 @@ func Test_areListenersEqual(t *testing.T) {
}

for _, input := range inputs {
assert.Equal(t, input.expected, areListenersEqual(input.l1, input.l2), input.message, input.l1, input.l2)
assert.Equal(t, input.expected, areListenersEqual(input.l1, input.l2), input.message)
}
}

func Test_mergeAllowedRoutes(t *testing.T) {
same := gatewayv1alpha2.NamespacesFromSame
all := gatewayv1alpha2.NamespacesFromAll
inputs := []struct {
message string
expected []gatewayv1alpha2.Listener
l1 []gatewayv1alpha2.Listener
l2 []gatewayv1alpha2.Listener
}{
{
message: "empty",
expected: []gatewayv1alpha2.Listener(nil), // TODO what the go-isms here, shouldn't this be empty array, not whatever this is?
l1: []gatewayv1alpha2.Listener{},
l2: []gatewayv1alpha2.Listener{},
},
{
message: "single",
expected: []gatewayv1alpha2.Listener{
{
Protocol: gatewayv1alpha2.UDPProtocolType,
AllowedRoutes: &gatewayv1alpha2.AllowedRoutes{
Namespaces: &gatewayv1alpha2.RouteNamespaces{
From: &same,
},
},
},
},
l1: []gatewayv1alpha2.Listener{
{
Protocol: gatewayv1alpha2.UDPProtocolType,
AllowedRoutes: &gatewayv1alpha2.AllowedRoutes{
Namespaces: &gatewayv1alpha2.RouteNamespaces{
From: &same,
},
},
},
},
l2: []gatewayv1alpha2.Listener{
{
Protocol: gatewayv1alpha2.UDPProtocolType,
},
},
},
{
message: "multiple",
expected: []gatewayv1alpha2.Listener{
{
Name: "one",
Protocol: gatewayv1alpha2.UDPProtocolType,
AllowedRoutes: &gatewayv1alpha2.AllowedRoutes{
Namespaces: &gatewayv1alpha2.RouteNamespaces{
From: &same,
},
},
},
{
Name: "two",
Protocol: gatewayv1alpha2.UDPProtocolType,
AllowedRoutes: &gatewayv1alpha2.AllowedRoutes{
Namespaces: &gatewayv1alpha2.RouteNamespaces{
From: &same,
},
},
},
{
Name: "three",
Protocol: gatewayv1alpha2.TCPProtocolType,
AllowedRoutes: &gatewayv1alpha2.AllowedRoutes{
Namespaces: &gatewayv1alpha2.RouteNamespaces{
From: &all,
},
},
},
},
l1: []gatewayv1alpha2.Listener{
{
Protocol: gatewayv1alpha2.UDPProtocolType,
AllowedRoutes: &gatewayv1alpha2.AllowedRoutes{
Namespaces: &gatewayv1alpha2.RouteNamespaces{
From: &same,
},
},
},
{
Protocol: gatewayv1alpha2.TCPProtocolType,
AllowedRoutes: &gatewayv1alpha2.AllowedRoutes{
Namespaces: &gatewayv1alpha2.RouteNamespaces{
From: &all,
},
},
},
},
l2: []gatewayv1alpha2.Listener{
{
Name: "one",
Protocol: gatewayv1alpha2.UDPProtocolType,
},
{
Name: "two",
Protocol: gatewayv1alpha2.UDPProtocolType,
},
{
Name: "three",
Protocol: gatewayv1alpha2.TCPProtocolType,
},
},
},
}
for _, input := range inputs {
assert.Equal(t, input.expected, mergeAllowedRoutes(input.l1, input.l2), input.message)
}
}

func Test_areAllowedRoutesConsistentByProtocol(t *testing.T) {
same := gatewayv1alpha2.NamespacesFromSame
all := gatewayv1alpha2.NamespacesFromAll
selector := gatewayv1alpha2.NamespacesFromSelector

inputs := []struct {
expected bool
message string
l []gatewayv1alpha2.Listener
}{
{
expected: true,
message: "empty",
l: []gatewayv1alpha2.Listener{},
},
{
expected: true,
message: "no intersect",
l: []gatewayv1alpha2.Listener{
{
Protocol: gatewayv1alpha2.UDPProtocolType,
AllowedRoutes: &gatewayv1alpha2.AllowedRoutes{
Namespaces: &gatewayv1alpha2.RouteNamespaces{
From: &same,
},
},
},
{
Protocol: gatewayv1alpha2.TCPProtocolType,
AllowedRoutes: &gatewayv1alpha2.AllowedRoutes{
Namespaces: &gatewayv1alpha2.RouteNamespaces{
From: &all,
},
},
},
},
},
{
expected: true,
message: "same allowed for each listener with same protocol",
l: []gatewayv1alpha2.Listener{
{
Protocol: gatewayv1alpha2.UDPProtocolType,
AllowedRoutes: &gatewayv1alpha2.AllowedRoutes{
Namespaces: &gatewayv1alpha2.RouteNamespaces{
From: &same,
},
},
},
{
Protocol: gatewayv1alpha2.UDPProtocolType,
AllowedRoutes: &gatewayv1alpha2.AllowedRoutes{
Namespaces: &gatewayv1alpha2.RouteNamespaces{
From: &same,
},
},
},
{
Protocol: gatewayv1alpha2.TCPProtocolType,
AllowedRoutes: &gatewayv1alpha2.AllowedRoutes{
Namespaces: &gatewayv1alpha2.RouteNamespaces{
From: &all,
},
},
},
},
},
{
expected: false,
message: "different allowed for listeners with same protocol",
l: []gatewayv1alpha2.Listener{
{
Protocol: gatewayv1alpha2.UDPProtocolType,
AllowedRoutes: &gatewayv1alpha2.AllowedRoutes{
Namespaces: &gatewayv1alpha2.RouteNamespaces{
From: &same,
},
},
},
{
Protocol: gatewayv1alpha2.UDPProtocolType,
AllowedRoutes: &gatewayv1alpha2.AllowedRoutes{
Namespaces: &gatewayv1alpha2.RouteNamespaces{
From: &all,
},
},
},
},
},
{
expected: true,
message: "same selector",
l: []gatewayv1alpha2.Listener{
{
Protocol: gatewayv1alpha2.UDPProtocolType,
AllowedRoutes: &gatewayv1alpha2.AllowedRoutes{
Namespaces: &gatewayv1alpha2.RouteNamespaces{
From: &selector,
Selector: &metav1.LabelSelector{
MatchLabels: map[string]string{"key": "value"},
},
},
},
},
{
Protocol: gatewayv1alpha2.UDPProtocolType,
AllowedRoutes: &gatewayv1alpha2.AllowedRoutes{
Namespaces: &gatewayv1alpha2.RouteNamespaces{
From: &selector,
Selector: &metav1.LabelSelector{
MatchLabels: map[string]string{"key": "value"},
},
},
},
},
},
},
{
expected: false,
message: "different selector",
l: []gatewayv1alpha2.Listener{
{
Protocol: gatewayv1alpha2.UDPProtocolType,
AllowedRoutes: &gatewayv1alpha2.AllowedRoutes{
Namespaces: &gatewayv1alpha2.RouteNamespaces{
From: &selector,
Selector: &metav1.LabelSelector{
MatchLabels: map[string]string{"key": "value"},
},
},
},
},
{
Protocol: gatewayv1alpha2.UDPProtocolType,
AllowedRoutes: &gatewayv1alpha2.AllowedRoutes{
Namespaces: &gatewayv1alpha2.RouteNamespaces{
From: &selector,
Selector: &metav1.LabelSelector{
MatchLabels: map[string]string{"key": "notvalue"},
},
},
},
},
},
},
}
for _, input := range inputs {
assert.Equal(t, input.expected, areAllowedRoutesConsistentByProtocol(input.l), input.message)
}
}

0 comments on commit ef2fb87

Please sign in to comment.