Skip to content
This repository has been archived by the owner on Mar 19, 2024. It is now read-only.

Apply namespace selector to namespace instead of route #119

Merged
merged 7 commits into from
Mar 9, 2022
Merged

Apply namespace selector to namespace instead of route #119

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
ec31402
Add GetNamespace to gatewayclient.Client
nathancoleman Mar 8, 2022
4816c94
Apply namespace selector to namespace instead of route
nathancoleman Mar 8, 2022
4c7afbc
Pipe newly-required context through call stack
nathancoleman Mar 8, 2022
c803312
Update unit test coverage
nathancoleman Mar 8, 2022
0990561
Add entry to the CHANGELOG
nathancoleman Mar 8, 2022
4de765b
Use context from args instead of TODO()
nathancoleman Mar 9, 2022
fc15724
Remove redundant unit test
nathancoleman Mar 9, 2022
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
6 changes: 6 additions & 0 deletions CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,3 +1,9 @@
## UNRELEASED

BUG FIXES:

* Apply namespace selector for allowed routes to the route's namespace instead of the route itself [[GH-119](https://github.com/hashicorp/consul-api-gateway/pull/119)]

## 0.1.0 (February 23, 2022)

* Initial release of Consul API Gateway
Expand Down
12 changes: 12 additions & 0 deletions internal/k8s/gatewayclient/gatewayclient.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@ type Client interface {
GetHTTPRoute(ctx context.Context, key types.NamespacedName) (*gateway.HTTPRoute, error)
GetTCPRoute(ctx context.Context, key types.NamespacedName) (*gateway.TCPRoute, error)
GetMeshService(ctx context.Context, key types.NamespacedName) (*apigwv1alpha1.MeshService, error)
GetNamespace(ctx context.Context, key types.NamespacedName) (*core.Namespace, error)

// finalizer helpers

Expand Down Expand Up @@ -268,6 +269,17 @@ func (g *gatewayClient) GetMeshService(ctx context.Context, key types.Namespaced
return service, nil
}

func (g *gatewayClient) GetNamespace(ctx context.Context, key types.NamespacedName) (*core.Namespace, error) {
namespace := &core.Namespace{}
if err := g.Client.Get(ctx, key, namespace); err != nil {
if k8serrors.IsNotFound(err) {
return nil, nil
}
return nil, NewK8sError(err)
}
return namespace, nil
}

func (g *gatewayClient) EnsureFinalizer(ctx context.Context, object client.Object, finalizer string) (bool, error) {
finalizers := object.GetFinalizers()
for _, f := range finalizers {
Expand Down
15 changes: 15 additions & 0 deletions internal/k8s/gatewayclient/mocks/gatewayclient.go
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

16 changes: 9 additions & 7 deletions internal/k8s/reconciler/listener.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,15 +7,16 @@ import (
"strings"
"sync/atomic"

corev1 "k8s.io/api/core/v1"
"k8s.io/apimachinery/pkg/types"
gw "sigs.k8s.io/gateway-api/apis/v1alpha2"

"github.com/hashicorp/consul-api-gateway/internal/common"
"github.com/hashicorp/consul-api-gateway/internal/core"
"github.com/hashicorp/consul-api-gateway/internal/k8s/gatewayclient"
"github.com/hashicorp/consul-api-gateway/internal/k8s/utils"
"github.com/hashicorp/consul-api-gateway/internal/store"
"github.com/hashicorp/go-hclog"
corev1 "k8s.io/api/core/v1"
"k8s.io/apimachinery/pkg/types"
gw "sigs.k8s.io/gateway-api/apis/v1alpha2"
)

var (
Expand Down Expand Up @@ -309,7 +310,7 @@ func (l *K8sListener) Config() store.ListenerConfig {
// on the gateway the return value is nil, if not,
// an error specifying why the route cannot bind
// is returned.
func (l *K8sListener) CanBind(route store.Route) (bool, error) {
func (l *K8sListener) CanBind(ctx context.Context, route store.Route) (bool, error) {
k8sRoute, ok := route.(*K8sRoute)
if !ok {
l.logger.Error("route is not a known type")
Expand All @@ -322,7 +323,7 @@ func (l *K8sListener) CanBind(route store.Route) (bool, error) {
expected := utils.NamespacedName(l.gateway)
l.logger.Trace("checking gateway match", "expected", expected.String(), "found", namespacedName.String())
if expected == namespacedName {
canBind, err := l.canBind(ref, k8sRoute)
canBind, err := l.canBind(ctx, ref, k8sRoute)
if err != nil {
return false, err
}
Expand All @@ -335,7 +336,7 @@ func (l *K8sListener) CanBind(route store.Route) (bool, error) {
return false, nil
}

func (l *K8sListener) canBind(ref gw.ParentRef, route *K8sRoute) (bool, error) {
func (l *K8sListener) canBind(ctx context.Context, ref gw.ParentRef, route *K8sRoute) (bool, error) {
if l.status.Ready.HasError() {
l.logger.Trace("listener not ready, unable to bind", "route", route.ID())
return false, nil
Expand All @@ -354,7 +355,8 @@ func (l *K8sListener) canBind(ref gw.ParentRef, route *K8sRoute) (bool, error) {
}
return false, nil
}
allowed, err := routeAllowedForListenerNamespaces(l.gateway.Namespace, l.listener.AllowedRoutes, route)

allowed, err := routeAllowedForListenerNamespaces(ctx, l.gateway.Namespace, l.listener.AllowedRoutes, route, l.client)
if err != nil {
return false, fmt.Errorf("error checking listener namespaces: %w", err)
}
Expand Down
37 changes: 19 additions & 18 deletions internal/k8s/reconciler/listener_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,16 +6,17 @@ import (
"testing"

"github.com/golang/mock/gomock"
"github.com/hashicorp/consul-api-gateway/internal/core"
"github.com/hashicorp/consul-api-gateway/internal/k8s/gatewayclient/mocks"
"github.com/hashicorp/consul-api-gateway/internal/store"
storeMocks "github.com/hashicorp/consul-api-gateway/internal/store/mocks"
"github.com/hashicorp/go-hclog"
"github.com/stretchr/testify/require"
k8s "k8s.io/api/core/v1"
meta "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/runtime/schema"
gw "sigs.k8s.io/gateway-api/apis/v1alpha2"

"github.com/hashicorp/consul-api-gateway/internal/core"
"github.com/hashicorp/consul-api-gateway/internal/k8s/gatewayclient/mocks"
"github.com/hashicorp/consul-api-gateway/internal/store"
storeMocks "github.com/hashicorp/consul-api-gateway/internal/store/mocks"
"github.com/hashicorp/go-hclog"
)

func TestListenerID(t *testing.T) {
Expand Down Expand Up @@ -378,15 +379,15 @@ func TestListenerCanBind(t *testing.T) {
listener := NewK8sListener(&gw.Gateway{}, gw.Listener{}, K8sListenerConfig{
Logger: hclog.NewNullLogger(),
})
canBind, err := listener.CanBind(storeMocks.NewMockRoute(nil))
canBind, err := listener.CanBind(context.Background(), storeMocks.NewMockRoute(nil))
require.NoError(t, err)
require.False(t, canBind)

// no match
listener = NewK8sListener(&gw.Gateway{}, gw.Listener{}, K8sListenerConfig{
Logger: hclog.NewNullLogger(),
})
canBind, err = listener.CanBind(NewK8sRoute(&gw.HTTPRoute{}, K8sRouteConfig{
canBind, err = listener.CanBind(context.Background(), NewK8sRoute(&gw.HTTPRoute{}, K8sRouteConfig{
Logger: hclog.NewNullLogger(),
}))
require.NoError(t, err)
Expand All @@ -400,7 +401,7 @@ func TestListenerCanBind(t *testing.T) {
}, gw.Listener{}, K8sListenerConfig{
Logger: hclog.NewNullLogger(),
})
canBind, err = listener.CanBind(NewK8sRoute(&gw.HTTPRoute{
canBind, err = listener.CanBind(context.Background(), NewK8sRoute(&gw.HTTPRoute{
Spec: gw.HTTPRouteSpec{
CommonRouteSpec: gw.CommonRouteSpec{
ParentRefs: []gw.ParentRef{{
Expand All @@ -423,7 +424,7 @@ func TestListenerCanBind(t *testing.T) {
Logger: hclog.NewNullLogger(),
})
listener.status.Ready.Invalid = errors.New("invalid")
canBind, err = listener.CanBind(NewK8sRoute(&gw.HTTPRoute{
canBind, err = listener.CanBind(context.Background(), NewK8sRoute(&gw.HTTPRoute{
Spec: gw.HTTPRouteSpec{
CommonRouteSpec: gw.CommonRouteSpec{
ParentRefs: []gw.ParentRef{{
Expand Down Expand Up @@ -459,7 +460,7 @@ func TestListenerCanBind_RouteKind(t *testing.T) {
Logger: hclog.NewNullLogger(),
})
require.NoError(t, listener.Validate(context.Background()))
canBind, err := listener.CanBind(NewK8sRoute(&gw.UDPRoute{
canBind, err := listener.CanBind(context.Background(), NewK8sRoute(&gw.UDPRoute{
TypeMeta: routeMeta,
Spec: gw.UDPRouteSpec{
CommonRouteSpec: gw.CommonRouteSpec{
Expand All @@ -485,7 +486,7 @@ func TestListenerCanBind_RouteKind(t *testing.T) {
Logger: hclog.NewNullLogger(),
})
listener.supportedKinds = supportedProtocols[gw.HTTPProtocolType]
_, err = listener.CanBind(NewK8sRoute(&gw.UDPRoute{
_, err = listener.CanBind(context.Background(), NewK8sRoute(&gw.UDPRoute{
TypeMeta: routeMeta,
Spec: gw.UDPRouteSpec{
CommonRouteSpec: gw.CommonRouteSpec{
Expand Down Expand Up @@ -532,7 +533,7 @@ func TestListenerCanBind_AllowedNamespaces(t *testing.T) {
Logger: hclog.NewNullLogger(),
})
listener.supportedKinds = supportedProtocols[gw.HTTPProtocolType]
_, err := listener.CanBind(NewK8sRoute(&gw.HTTPRoute{
_, err := listener.CanBind(context.Background(), NewK8sRoute(&gw.HTTPRoute{
TypeMeta: routeMeta,
Spec: gw.HTTPRouteSpec{
CommonRouteSpec: gw.CommonRouteSpec{
Expand All @@ -547,7 +548,7 @@ func TestListenerCanBind_AllowedNamespaces(t *testing.T) {
Logger: hclog.NewNullLogger(),
}))
require.Error(t, err)
canBind, err := listener.CanBind(NewK8sRoute(&gw.HTTPRoute{
canBind, err := listener.CanBind(context.Background(), NewK8sRoute(&gw.HTTPRoute{
TypeMeta: routeMeta,
Spec: gw.HTTPRouteSpec{
CommonRouteSpec: gw.CommonRouteSpec{
Expand Down Expand Up @@ -586,7 +587,7 @@ func TestListenerCanBind_AllowedNamespaces(t *testing.T) {
Logger: hclog.NewNullLogger(),
})
listener.supportedKinds = supportedProtocols[gw.HTTPProtocolType]
_, err = listener.CanBind(NewK8sRoute(&gw.HTTPRoute{
_, err = listener.CanBind(context.Background(), NewK8sRoute(&gw.HTTPRoute{
TypeMeta: routeMeta,
Spec: gw.HTTPRouteSpec{
CommonRouteSpec: gw.CommonRouteSpec{
Expand All @@ -601,7 +602,7 @@ func TestListenerCanBind_AllowedNamespaces(t *testing.T) {
Logger: hclog.NewNullLogger(),
}))
require.Error(t, err)
_, err = listener.CanBind(NewK8sRoute(&gw.HTTPRoute{
_, err = listener.CanBind(context.Background(), NewK8sRoute(&gw.HTTPRoute{
TypeMeta: routeMeta,
Spec: gw.HTTPRouteSpec{
CommonRouteSpec: gw.CommonRouteSpec{
Expand Down Expand Up @@ -640,7 +641,7 @@ func TestListenerCanBind_HostnameMatch(t *testing.T) {
Logger: hclog.NewNullLogger(),
})
listener.supportedKinds = supportedProtocols[gw.HTTPProtocolType]
_, err := listener.CanBind(NewK8sRoute(&gw.HTTPRoute{
_, err := listener.CanBind(context.Background(), NewK8sRoute(&gw.HTTPRoute{
TypeMeta: routeMeta,
Spec: gw.HTTPRouteSpec{
CommonRouteSpec: gw.CommonRouteSpec{
Expand All @@ -656,7 +657,7 @@ func TestListenerCanBind_HostnameMatch(t *testing.T) {
}))
require.Error(t, err)

canBind, err := listener.CanBind(NewK8sRoute(&gw.HTTPRoute{
canBind, err := listener.CanBind(context.Background(), NewK8sRoute(&gw.HTTPRoute{
TypeMeta: routeMeta,
Spec: gw.HTTPRouteSpec{
CommonRouteSpec: gw.CommonRouteSpec{
Expand Down Expand Up @@ -688,7 +689,7 @@ func TestListenerCanBind_NameMatch(t *testing.T) {
}, K8sListenerConfig{
Logger: hclog.NewNullLogger(),
})
canBind, err := listener.CanBind(NewK8sRoute(&gw.HTTPRoute{
canBind, err := listener.CanBind(context.Background(), NewK8sRoute(&gw.HTTPRoute{
Spec: gw.HTTPRouteSpec{
CommonRouteSpec: gw.CommonRouteSpec{
ParentRefs: []gw.ParentRef{{
Expand Down
22 changes: 17 additions & 5 deletions internal/k8s/reconciler/utils.go
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package reconciler

import (
"context"
"encoding/json"
"fmt"
"reflect"
Expand All @@ -9,7 +10,10 @@ import (

metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
klabels "k8s.io/apimachinery/pkg/labels"
"k8s.io/apimachinery/pkg/types"
gw "sigs.k8s.io/gateway-api/apis/v1alpha2"

"github.com/hashicorp/consul-api-gateway/internal/k8s/gatewayclient"
)

const (
Expand Down Expand Up @@ -79,14 +83,16 @@ func routeKindIsAllowedForListener(kinds []gw.RouteGroupKind, route *K8sRoute) b
return false
}

func routeAllowedForListenerNamespaces(gatewayNS string, allowedRoutes *gw.AllowedRoutes, route *K8sRoute) (bool, error) {
// routeAllowedForListenerNamespaces determines whether the route is allowed
// to bind to the Gateway based on the AllowedRoutes namespace selectors.
func routeAllowedForListenerNamespaces(ctx context.Context, gatewayNS string, allowedRoutes *gw.AllowedRoutes, route *K8sRoute, c gatewayclient.Client) (bool, error) {
var namespaceSelector *gw.RouteNamespaces
if allowedRoutes != nil {
// check gateway namespace
namespaceSelector = allowedRoutes.Namespaces
}

// set default is namespace selector is nil
// set default if namespace selector is nil
from := gw.NamespacesFromSame
if namespaceSelector != nil && namespaceSelector.From != nil && *namespaceSelector.From != "" {
from = *namespaceSelector.From
Expand All @@ -97,12 +103,18 @@ func routeAllowedForListenerNamespaces(gatewayNS string, allowedRoutes *gw.Allow
case gw.NamespacesFromSame:
return gatewayNS == route.GetNamespace(), nil
case gw.NamespacesFromSelector:
ns, err := metav1.LabelSelectorAsSelector(namespaceSelector.Selector)
namespaceSelector, err := metav1.LabelSelectorAsSelector(namespaceSelector.Selector)
if err != nil {
return false, fmt.Errorf("error parsing label selector: %w", err)
}

// retrieve the route's namespace and determine whether selector matches
namespace, err := c.GetNamespace(ctx, types.NamespacedName{Name: route.GetNamespace()})
if err != nil {
return false, fmt.Errorf("error parsing label selector: %v", err)
return false, fmt.Errorf("error retrieving namespace for route: %w", err)
}

return ns.Matches(toNamespaceSet(route.GetNamespace(), route.GetLabels())), nil
return namespaceSelector.Matches(toNamespaceSet(namespace.GetName(), namespace.GetLabels())), nil
}
return false, nil
}
Expand Down
Loading