Skip to content

AllowedRoutes support for Listeners #721

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.

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

Merged
merged 7 commits into from
Jun 9, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
1 change: 1 addition & 0 deletions deploy/manifests/rbac.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ rules:
- apiGroups:
- ""
resources:
- namespaces
- services
- secrets
verbs:
Expand Down
3 changes: 2 additions & 1 deletion docs/gateway-api-compatibility.md
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,7 @@ Fields:
* `mode` - partially supported. Allowed value: `Terminate`.
* `certificateRefs` - partially supported. The TLS certificate and key must be stored in a Secret resource of type `kubernetes.io/tls` in the same namespace as the Gateway resource. Only a single reference is supported. You must deploy the Secret before the Gateway resource. Secret rotation (watching for updates) is not supported.
* `options` - not supported.
* `allowedRoutes` - not supported.
* `allowedRoutes` - supported.
* `addresses` - not supported.
* `status`
* `addresses` - Pod IPAddress supported.
Expand Down Expand Up @@ -122,6 +122,7 @@ Fields:
* `Accepted/True/Accepted`
* `Accepted/False/NoMatchingListenerHostname`
* `Accepted/False/NoMatchingParent`
* `Accepted/False/NotAllowedByListeners`
* `Accepted/False/UnsupportedValue`: Custom reason for when the HTTPRoute includes an invalid or unsupported value.
* `Accepted/False/InvalidListener`: Custom reason for when the HTTPRoute references an invalid listener.
* `Accepted/False/GatewayNotProgrammed`: Custom reason for when the Gateway is not Programmed. HTTPRoute may be valid and configured, but will maintain this status as long as the Gateway is not Programmed.
Expand Down
4 changes: 4 additions & 0 deletions internal/events/handler.go
Original file line number Diff line number Diff line change
Expand Up @@ -128,6 +128,8 @@ func (h *EventHandlerImpl) propagateUpsert(e *UpsertEvent) {
h.cfg.Processor.CaptureUpsertChange(r)
case *apiv1.Service:
h.cfg.Processor.CaptureUpsertChange(r)
case *apiv1.Namespace:
h.cfg.Processor.CaptureUpsertChange(r)
case *apiv1.Secret:
// FIXME(kate-osborn): need to handle certificate rotation
// https://github.com/nginxinc/nginx-kubernetes-gateway/issues/553
Expand All @@ -149,6 +151,8 @@ func (h *EventHandlerImpl) propagateDelete(e *DeleteEvent) {
h.cfg.Processor.CaptureDeleteChange(e.Type, e.NamespacedName)
case *apiv1.Service:
h.cfg.Processor.CaptureDeleteChange(e.Type, e.NamespacedName)
case *apiv1.Namespace:
h.cfg.Processor.CaptureDeleteChange(e.Type, e.NamespacedName)
case *apiv1.Secret:
// FIXME(kate-osborn): make sure that affected servers are updated
// https://github.com/nginxinc/nginx-kubernetes-gateway/issues/553
Expand Down
7 changes: 7 additions & 0 deletions internal/manager/manager.go
Original file line number Diff line number Diff line change
Expand Up @@ -112,6 +112,12 @@ func Start(cfg config.Config) error {
controller.WithFieldIndices(index.CreateEndpointSliceFieldIndices()),
},
},
{
objectType: &apiv1.Namespace{},
options: []controller.Option{
controller.WithK8sPredicate(k8spredicate.LabelChangedPredicate{}),
},
},
}

ctx := ctlr.SetupSignalHandler()
Expand Down Expand Up @@ -195,6 +201,7 @@ func prepareFirstEventBatchPreparerArgs(
objectLists := []client.ObjectList{
&apiv1.ServiceList{},
&apiv1.SecretList{},
&apiv1.NamespaceList{},
&discoveryV1.EndpointSliceList{},
&gatewayv1beta1.HTTPRouteList{},
}
Expand Down
2 changes: 2 additions & 0 deletions internal/manager/manager_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@ func TestPrepareFirstEventBatchPreparerArgs(t *testing.T) {
expectedObjectLists: []client.ObjectList{
&apiv1.ServiceList{},
&apiv1.SecretList{},
&apiv1.NamespaceList{},
&discoveryV1.EndpointSliceList{},
&gatewayv1beta1.HTTPRouteList{},
&gatewayv1beta1.GatewayList{},
Expand All @@ -48,6 +49,7 @@ func TestPrepareFirstEventBatchPreparerArgs(t *testing.T) {
expectedObjectLists: []client.ObjectList{
&apiv1.ServiceList{},
&apiv1.SecretList{},
&apiv1.NamespaceList{},
&discoveryV1.EndpointSliceList{},
&gatewayv1beta1.HTTPRouteList{},
},
Expand Down
8 changes: 7 additions & 1 deletion internal/state/change_processor.go
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,7 @@ type ChangeProcessorConfig struct {
Logger logr.Logger
// EventRecorder records events for Kubernetes resources.
EventRecorder record.EventRecorder
// Scheme is the a Kubernetes scheme.
// Scheme is the Kubernetes scheme.
Scheme *runtime.Scheme
// GatewayCtlrName is the name of the Gateway controller.
GatewayCtlrName string
Expand Down Expand Up @@ -89,6 +89,7 @@ func NewChangeProcessorImpl(cfg ChangeProcessorConfig) *ChangeProcessorImpl {
Gateways: make(map[types.NamespacedName]*v1beta1.Gateway),
HTTPRoutes: make(map[types.NamespacedName]*v1beta1.HTTPRoute),
Services: make(map[types.NamespacedName]*apiv1.Service),
Namespaces: make(map[types.NamespacedName]*apiv1.Namespace),
}

extractGVK := func(obj client.Object) schema.GroupVersionKind {
Expand Down Expand Up @@ -118,6 +119,11 @@ func NewChangeProcessorImpl(cfg ChangeProcessorConfig) *ChangeProcessorImpl {
store: newObjectStoreMapAdapter(clusterStore.HTTPRoutes),
trackUpsertDelete: true,
},
{
gvk: extractGVK(&apiv1.Namespace{}),
store: newObjectStoreMapAdapter(clusterStore.Namespaces),
trackUpsertDelete: false,
},
{
gvk: extractGVK(&apiv1.Service{}),
store: newObjectStoreMapAdapter(clusterStore.Services),
Expand Down
48 changes: 48 additions & 0 deletions internal/state/change_processor_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -924,6 +924,54 @@ var _ = Describe("ChangeProcessor", func() {
})
})
})
Describe("namespace changes", func() {
When("namespace is linked via label selectors", func() {
It("triggers an update when labels are removed", func() {
ns := &apiv1.Namespace{
ObjectMeta: metav1.ObjectMeta{
Name: "ns",
Labels: map[string]string{
"app": "allowed",
},
},
}
gw := &v1beta1.Gateway{
ObjectMeta: metav1.ObjectMeta{
Name: "gw",
},
Spec: v1beta1.GatewaySpec{
Listeners: []v1beta1.Listener{
{
AllowedRoutes: &v1beta1.AllowedRoutes{
Namespaces: &v1beta1.RouteNamespaces{
From: helpers.GetPointer(v1beta1.NamespacesFromSelector),
Selector: &metav1.LabelSelector{
MatchLabels: map[string]string{
"app": "allowed",
},
},
},
},
},
},
},
}

processor.CaptureUpsertChange(gw)
processor.CaptureUpsertChange(ns)

changed, _ := processor.Process()
Expect(changed).To(BeTrue())

newNS := ns.DeepCopy()
newNS.Labels = nil
processor.CaptureUpsertChange(newNS)

changed, _ = processor.Process()
Expect(changed).To(BeTrue())
})
})
})
})

Describe("Ensuring non-changing changes don't override previously changing changes", func() {
Expand Down
11 changes: 11 additions & 0 deletions internal/state/conditions/conditions.go
Original file line number Diff line number Diff line change
Expand Up @@ -109,6 +109,17 @@ func NewDefaultRouteConditions() []Condition {
}
}

// NewRouteNotAllowedByListeners returns a Condition that indicates that the HTTPRoute is not allowed by
// any listener.
func NewRouteNotAllowedByListeners() Condition {
return Condition{
Type: string(v1beta1.RouteConditionAccepted),
Status: metav1.ConditionFalse,
Reason: string(v1beta1.RouteReasonNotAllowedByListeners),
Message: "HTTPRoute is not allowed by any listener",
}
}

// NewRouteNoMatchingListenerHostname returns a Condition that indicates that the hostname of the listener
// does not match the hostnames of the HTTPRoute.
func NewRouteNoMatchingListenerHostname() Condition {
Expand Down
75 changes: 72 additions & 3 deletions internal/state/graph/gateway_listener.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,8 @@ package graph
import (
"fmt"

metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/labels"
"k8s.io/apimachinery/pkg/types"
"k8s.io/apimachinery/pkg/util/validation/field"
"sigs.k8s.io/gateway-api/apis/v1beta1"
Expand All @@ -19,6 +21,8 @@ type Listener struct {
// Routes holds the routes attached to the Listener.
// Only valid routes are attached.
Routes map[types.NamespacedName]*Route
// AllowedRouteLabelSelector is the label selector for this Listener's allowed routes, if defined.
AllowedRouteLabelSelector labels.Selector
// SecretPath is the path to the secret on disk.
SecretPath string
// Conditions holds the conditions of the Listener.
Expand Down Expand Up @@ -78,6 +82,8 @@ func newListenerConfiguratorFactory(
},
http: &listenerConfigurator{
validators: []listenerValidator{
validateListenerAllowedRouteKind,
validateListenerLabelSelector,
validateListenerHostname,
validateHTTPListener,
},
Expand All @@ -87,6 +93,8 @@ func newListenerConfiguratorFactory(
},
https: &listenerConfigurator{
validators: []listenerValidator{
validateListenerAllowedRouteKind,
validateListenerLabelSelector,
validateListenerHostname,
createHTTPSListenerValidator(gw.Namespace),
},
Expand Down Expand Up @@ -135,6 +143,16 @@ func (c *listenerConfigurator) configure(listener v1beta1.Listener) *Listener {
conds = append(conds, validator(listener)...)
}

var allowedRouteSelector labels.Selector
if selector := GetAllowedRouteLabelSelector(listener); selector != nil {
var err error
allowedRouteSelector, err = metav1.LabelSelectorAsSelector(selector)
if err != nil {
msg := fmt.Sprintf("invalid label selector: %s", err.Error())
conds = append(conds, conditions.NewListenerUnsupportedValue(msg))
}
}

if len(conds) > 0 {
return &Listener{
Source: listener,
Expand All @@ -144,9 +162,10 @@ func (c *listenerConfigurator) configure(listener v1beta1.Listener) *Listener {
}

l := &Listener{
Source: listener,
Routes: make(map[types.NamespacedName]*Route),
Valid: true,
Source: listener,
AllowedRouteLabelSelector: allowedRouteSelector,
Routes: make(map[types.NamespacedName]*Route),
Valid: true,
}

// resolvers might add different conditions to the listener, so we run them all.
Expand Down Expand Up @@ -182,6 +201,45 @@ func validateListenerHostname(listener v1beta1.Listener) []conditions.Condition
return nil
}

func validateListenerAllowedRouteKind(listener v1beta1.Listener) []conditions.Condition {
validHTTPRouteKind := func(kind v1beta1.RouteGroupKind) bool {
if kind.Kind != v1beta1.Kind("HTTPRoute") {
return false
}
if kind.Group == nil || *kind.Group != v1beta1.GroupName {
return false
}
return true
}

switch listener.Protocol {
case v1beta1.HTTPProtocolType, v1beta1.HTTPSProtocolType:
if listener.AllowedRoutes != nil {
for _, kind := range listener.AllowedRoutes.Kinds {
if !validHTTPRouteKind(kind) {
msg := fmt.Sprintf("Unsupported route kind \"%s/%s\"", *kind.Group, kind.Kind)
return []conditions.Condition{conditions.NewListenerUnsupportedValue(msg)}
}
}
}
}

return nil
}

func validateListenerLabelSelector(listener v1beta1.Listener) []conditions.Condition {
if listener.AllowedRoutes != nil &&
listener.AllowedRoutes.Namespaces != nil &&
listener.AllowedRoutes.Namespaces.From != nil &&
*listener.AllowedRoutes.Namespaces.From == v1beta1.NamespacesFromSelector &&
listener.AllowedRoutes.Namespaces.Selector == nil {
msg := "Listener's AllowedRoutes Selector must be set when From is set to type Selector"
return []conditions.Condition{conditions.NewListenerUnsupportedValue(msg)}
}

return nil
}

func validateHTTPListener(listener v1beta1.Listener) []conditions.Condition {
if listener.Port != 80 {
path := field.NewPath("port")
Expand Down Expand Up @@ -314,3 +372,14 @@ func createExternalReferencesForTLSSecretsResolver(
}
}
}

// GetAllowedRouteLabelSelector returns a listener's AllowedRoutes label selector if it exists.
func GetAllowedRouteLabelSelector(l v1beta1.Listener) *metav1.LabelSelector {
if l.AllowedRoutes != nil && l.AllowedRoutes.Namespaces != nil {
if *l.AllowedRoutes.Namespaces.From == v1beta1.NamespacesFromSelector && l.AllowedRoutes.Namespaces.Selector != nil {
return l.AllowedRoutes.Namespaces.Selector
}
}

return nil
}
Loading