Skip to content
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
66 changes: 66 additions & 0 deletions gateway/perlistener_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,66 @@
package gateway_test

import (
"strings"
"testing"

"github.com/bitcomplete/tsjwt/gateway"
corev1 "k8s.io/api/core/v1"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
gwapi "sigs.k8s.io/gateway-api/apis/v1"
)

// allowedRoutes must be evaluated per matching listener, not per Gateway. On a
// Gateway with an open listener (From=All) and a strict one (From=Same) that own
// different hostnames, a route from another namespace is admitted only by the
// open listener and so may serve only the open listener's hostname — it must not
// borrow the strict listener's hostname.
//
// Regression for adversarial finding B (per-listener allowedRoutes).
func TestAllowedRoutesEvaluatedPerListener(t *testing.T) {
t.Parallel()
all := gwapi.NamespacesFromAll
same := gwapi.NamespacesFromSame
g := &gwapi.Gateway{ObjectMeta: metav1.ObjectMeta{Name: "edge", Namespace: "infra"}}
g.Spec.Listeners = []gwapi.Listener{
{
Name: "open", Port: 443, Protocol: gwapi.HTTPSProtocolType,
Hostname: ptr(gwapi.Hostname("open.example")),
AllowedRoutes: &gwapi.AllowedRoutes{Namespaces: &gwapi.RouteNamespaces{From: &all}},
},
{
Name: "strict", Port: 443, Protocol: gwapi.HTTPSProtocolType,
Hostname: ptr(gwapi.Hostname("strict.example")),
AllowedRoutes: &gwapi.AllowedRoutes{Namespaces: &gwapi.RouteNamespaces{From: &same}},
},
}

// A foreign-namespace route may attach (the open listener is From=All), but
// requesting the strict listener's hostname must not be served.
foreignStrict := route("gstrict", "other", "edge", []string{"strict.example"}, rule("guest-svc", 80, "/"))
foreignStrict.Spec.ParentRefs[0].Namespace = ptr(gwapi.Namespace("infra"))
foreignOpen := route("gopen", "other", "edge", []string{"open.example"}, rule("guest-svc", 80, "/"))
foreignOpen.Spec.ParentRefs[0].Namespace = ptr(gwapi.Namespace("infra"))
localStrict := route("lstrict", "infra", "edge", []string{"strict.example"}, rule("app-svc", 80, "/"))

svcs := []corev1.Service{svc("guest-svc", "other", 80), svc("app-svc", "infra", 80)}
got, _ := gateway.Translate(g, []gwapi.HTTPRoute{foreignStrict, foreignOpen, localStrict}, svcs)

var openServed, localServed bool
for _, r := range got {
switch {
case strings.HasPrefix(r.Name, "other/gstrict"):
t.Errorf("foreign-namespace route served strict.example, which only a From=Same listener owns: %s", r.Name)
case strings.HasPrefix(r.Name, "other/gopen"):
openServed = true
case strings.HasPrefix(r.Name, "infra/lstrict"):
localServed = true
}
}
if !openServed {
t.Error("foreign route on the From=All listener's hostname should be served")
}
if !localServed {
t.Error("same-namespace route on the From=Same listener's hostname should be served")
}
}
60 changes: 60 additions & 0 deletions gateway/reconcile.go
Original file line number Diff line number Diff line change
Expand Up @@ -179,6 +179,9 @@ func (r *GatewayReconciler) render(ctx context.Context, gw *gwapi.Gateway) (stri
}

table, errs := Translate(gw, routes.Items, services.Items)
// Report acceptance back on each attached route's status before rendering
// the table, so a rejected route is visible to its author, not only logged.
r.applyRouteStatus(ctx, gw, routes.Items, errs)
// The generation is a usable revision: it changes when the Gateway
// does, and the data plane only uses it for logs and readiness.
file := routetable.Render(gw.Namespace+"/"+gw.Name, gw.Generation, table)
Expand Down Expand Up @@ -314,4 +317,61 @@ func (conditionHelpers) setCondition(conds *[]metav1.Condition, c metav1.Conditi
*conds = append(*conds, c)
}

// applyRouteStatus writes this controller's Accepted condition onto every route
// attached to gw, so an author learns from the route's own status whether it
// was accepted or why it was rejected — not only from the controller log. A
// status write that fails is logged, never fatal to the reconcile.
func (r *GatewayReconciler) applyRouteStatus(ctx context.Context, gw *gwapi.Gateway, routes []gwapi.HTTPRoute, errs []RouteError) {
l := log.FromContext(ctx)
ref := parentRefToGateway(gw)
for i := range routes {
route := &routes[i]
if !attachedTo(gw, route) {
continue
}
if !upsertRouteParentCondition(route, ref, routeAcceptedCondition(route, errs)) {
continue // unchanged; do not churn the status
}
if err := r.Status().Update(ctx, route); err != nil {
l.Error(err, "update HTTPRoute status", "route", route.Namespace+"/"+route.Name)
}
}
}

// upsertRouteParentCondition merges cond into the RouteParentStatus for this
// controller and ref, and reports whether anything changed.
func upsertRouteParentCondition(route *gwapi.HTTPRoute, ref gwapi.ParentReference, cond metav1.Condition) bool {
for i := range route.Status.Parents {
p := &route.Status.Parents[i]
if string(p.ControllerName) != ControllerName || !parentRefEqual(p.ParentRef, ref) {
continue
}
for j := range p.Conditions {
c := p.Conditions[j]
if c.Type == cond.Type {
if c.Status == cond.Status && c.Reason == cond.Reason &&
c.Message == cond.Message && c.ObservedGeneration == cond.ObservedGeneration {
return false // identical; leave it, and its transition time, alone
}
break
}
}
meta.setCondition(&p.Conditions, cond)
return true
}
route.Status.Parents = append(route.Status.Parents, gwapi.RouteParentStatus{
ParentRef: ref,
ControllerName: gwapi.GatewayController(ControllerName),
Conditions: []metav1.Condition{cond},
})
return true
}

func parentRefEqual(a, b gwapi.ParentReference) bool {
name := a.Name == b.Name
ns := (a.Namespace == nil && b.Namespace == nil) ||
(a.Namespace != nil && b.Namespace != nil && *a.Namespace == *b.Namespace)
return name && ns
}

var _ = apierrors.IsNotFound
2 changes: 1 addition & 1 deletion gateway/reconcile_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,7 @@ func newReconciler(t *testing.T, objs ...client.Object) (*gateway.GatewayReconci
c := fake.NewClientBuilder().
WithScheme(reconcileScheme(t)).
WithObjects(objs...).
WithStatusSubresource(&gwapi.Gateway{}, &gwapi.GatewayClass{}).
WithStatusSubresource(&gwapi.Gateway{}, &gwapi.GatewayClass{}, &gwapi.HTTPRoute{}).
Build()
return &gateway.GatewayReconciler{
Client: c,
Expand Down
84 changes: 61 additions & 23 deletions gateway/routes.go
Original file line number Diff line number Diff line change
Expand Up @@ -275,12 +275,11 @@ func attachedTo(gw *gwapi.Gateway, r *gwapi.HTTPRoute) bool {
ns = string(*p.Namespace)
}
if string(p.Name) == gw.Name && ns == gw.Namespace {
// Naming the Gateway is necessary but not sufficient: the
// Gateway's listeners decide which namespaces may attach, via
// allowedRoutes.namespaces (Gateway API, default From=Same).
// Without this check a route in any namespace could bind to a
// Gateway it does not own and shadow another tenant's routes.
return listenerAllowsNamespace(gw, r.Namespace)
// Naming the Gateway is necessary but not sufficient: at least one
// listener's allowedRoutes.namespaces must admit the route's
// namespace (Gateway API, default From=Same). hostnamesFor then
// restricts what the route may serve to those same listeners.
return len(admittingListeners(gw, r.Namespace)) > 0
}
}
return false
Expand All @@ -291,25 +290,37 @@ func attachedTo(gw *gwapi.Gateway, r *gwapi.HTTPRoute) bool {
// when unset is From=Same (only the Gateway's own namespace). From=Selector
// needs the route namespace's labels, which this controller does not read, so
// it fails closed until that lookup is wired in.
func listenerAllowsNamespace(gw *gwapi.Gateway, routeNS string) bool {
func listenerAdmits(l gwapi.Listener, gwNamespace, routeNS string) bool {
from := gwapi.NamespacesFromSame
if l.AllowedRoutes != nil && l.AllowedRoutes.Namespaces != nil && l.AllowedRoutes.Namespaces.From != nil {
from = *l.AllowedRoutes.Namespaces.From
}
switch from {
case gwapi.NamespacesFromAll:
return true
case gwapi.NamespacesFromSame:
return routeNS == gwNamespace
default:
// Selector needs the route namespace's labels, which this controller
// does not list; None admits nothing. Fail closed rather than guess.
return false
}
}

// admittingListeners returns the Gateway's listeners whose allowedRoutes admit a
// route from routeNS. A route may attach through, and — crucially — may serve
// the hostnames of, only these listeners. Evaluating this per listener rather
// than per Gateway is what stops a route admitted by a permissive listener
// (From=All) from serving a hostname that only a stricter listener (From=Same)
// owns.
func admittingListeners(gw *gwapi.Gateway, routeNS string) []gwapi.Listener {
var out []gwapi.Listener
for _, l := range gw.Spec.Listeners {
from := gwapi.NamespacesFromSame
if l.AllowedRoutes != nil && l.AllowedRoutes.Namespaces != nil && l.AllowedRoutes.Namespaces.From != nil {
from = *l.AllowedRoutes.Namespaces.From
}
switch from {
case gwapi.NamespacesFromAll:
return true
case gwapi.NamespacesFromSame:
if routeNS == gw.Namespace {
return true
}
case gwapi.NamespacesFromSelector:
// Selector needs the route namespace's labels, which this
// controller does not list; fail closed rather than guess.
if listenerAdmits(l, gw.Namespace, routeNS) {
out = append(out, l)
}
}
return false
return out
}

// hostnamesFor intersects the route's hostnames with the Gateway's listeners,
Expand All @@ -321,8 +332,11 @@ func listenerAllowsNamespace(gw *gwapi.Gateway, routeNS string) bool {
// True with an empty list means neither side named any, so the route answers
// for whatever reaches the Gateway.
func hostnamesFor(gw *gwapi.Gateway, r *gwapi.HTTPRoute) ([]string, bool) {
// Only listeners that admit this route's namespace count. A route admitted
// through a From=All listener must not borrow the hostname of a From=Same
// listener that would not have admitted it.
var listener []string
for _, l := range gw.Spec.Listeners {
for _, l := range admittingListeners(gw, r.Namespace) {
if l.Hostname != nil && *l.Hostname != "" {
listener = append(listener, string(*l.Hostname))
}
Expand Down Expand Up @@ -400,6 +414,30 @@ func dedupe(in []string) []string {
return out
}

// parentRefToGateway builds a ParentReference identifying gw, for a route's
// parent status.
func parentRefToGateway(gw *gwapi.Gateway) gwapi.ParentReference {
group := gwapi.Group("gateway.networking.k8s.io")
kind := gwapi.Kind("Gateway")
ns := gwapi.Namespace(gw.Namespace)
return gwapi.ParentReference{Group: &group, Kind: &kind, Namespace: &ns, Name: gwapi.ObjectName(gw.Name)}
}

// routeAcceptedCondition reports this controller's Accepted condition for a
// route attached to gw, given the translation errors for the whole table: True
// when every rule translated, or False with the first rejection's reason. It
// lets a route author see acceptance (or why not) on the route's own status,
// as Gateway API expects, rather than only in the controller log.
func routeAcceptedCondition(route *gwapi.HTTPRoute, errs []RouteError) metav1.Condition {
key := route.Namespace + "/" + route.Name
for _, e := range errs {
if e.Route == key || strings.HasPrefix(e.Route, key+"#") {
return routeStatusCondition(route.Generation, false, e.Reason, e.Detail)
}
}
return routeStatusCondition(route.Generation, true, gwapi.RouteReasonAccepted, "accepted by tsjwt")
}

// routeStatusCondition builds the Accepted condition for a route.
func routeStatusCondition(gen int64, accepted bool, reason gwapi.RouteConditionReason, msg string) metav1.Condition {
status := metav1.ConditionTrue
Expand Down
64 changes: 64 additions & 0 deletions gateway/routestatus_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,64 @@
package gateway_test

import (
"context"
"testing"

"github.com/bitcomplete/tsjwt/gateway"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/types"
ctrl "sigs.k8s.io/controller-runtime"
gwapi "sigs.k8s.io/gateway-api/apis/v1"
)

// The controller must report acceptance on each attached route's own status —
// Accepted=True for a route it serves, Accepted=False with the reason for one
// it rejects — so an author sees it without reading controller logs.
//
// Regression for the missing per-HTTPRoute status conditions.
func TestReconcileWritesRouteStatus(t *testing.T) {
t.Parallel()
gc := gatewayClass("tsjwt", gateway.ControllerName)
g := gatewayWithClass("edge", "infra", "tsjwt", "app.ts.net")

good := route("good", "infra", "edge", []string{"app.ts.net"}, rule("app-svc", 80, "/"))
// A cross-namespace backendRef is rejected with RefNotPermitted.
bad := route("bad", "infra", "edge", []string{"app.ts.net"}, rule("payments", 8080, "/x"))
bad.Spec.Rules[0].BackendRefs[0].Namespace = ptr(gwapi.Namespace("other"))
s := svc("app-svc", "infra", 80)

r, c := newReconciler(t, gc, g, &good, &bad, &s)
ctx := context.Background()
if _, err := r.Reconcile(ctx, ctrl.Request{
NamespacedName: types.NamespacedName{Namespace: "infra", Name: "edge"}}); err != nil {
t.Fatalf("reconcile: %v", err)
}

accepted := func(name string) metav1.Condition {
t.Helper()
var hr gwapi.HTTPRoute
if err := c.Get(ctx, types.NamespacedName{Namespace: "infra", Name: name}, &hr); err != nil {
t.Fatalf("get route %s: %v", name, err)
}
for _, p := range hr.Status.Parents {
if string(p.ControllerName) != gateway.ControllerName {
continue
}
for _, cnd := range p.Conditions {
if cnd.Type == string(gwapi.RouteConditionAccepted) {
return cnd
}
}
}
t.Fatalf("route %s has no Accepted condition from this controller", name)
return metav1.Condition{}
}

if g := accepted("good"); g.Status != metav1.ConditionTrue {
t.Errorf("good route: Accepted=%s, want True", g.Status)
}
if b := accepted("bad"); b.Status != metav1.ConditionFalse ||
b.Reason != string(gwapi.RouteReasonRefNotPermitted) {
t.Errorf("bad route: Accepted=%s reason=%q, want False/RefNotPermitted", b.Status, b.Reason)
}
}
Loading