Skip to content
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

Support cross-namespace routing with TLSRoutes #2379

Merged
merged 3 commits into from
Aug 14, 2024
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
502 changes: 429 additions & 73 deletions internal/mode/static/state/change_processor_test.go

Large diffs are not rendered by default.

1 change: 1 addition & 0 deletions internal/mode/static/state/graph/graph.go
Original file line number Diff line number Diff line change
Expand Up @@ -228,6 +228,7 @@ func BuildGraph(
processedGws.GetAllNsNames(),
state.Services,
npCfg,
refGrantResolver,
)

bindRoutesToListeners(routes, l4routes, gw, state.Namespaces)
Expand Down
8 changes: 8 additions & 0 deletions internal/mode/static/state/graph/reference_grant.go
Original file line number Diff line number Diff line change
Expand Up @@ -81,6 +81,14 @@ func fromGRPCRoute(namespace string) fromResource {
}
}

func fromTLSRoute(namespace string) fromResource {
return fromResource{
group: v1.GroupName,
kind: kinds.TLSRoute,
namespace: namespace,
}
}

// newReferenceGrantResolver creates a new referenceGrantResolver.
func newReferenceGrantResolver(refGrants map[types.NamespacedName]*v1beta1.ReferenceGrant) *referenceGrantResolver {
allowed := make(map[allowedReference]struct{})
Expand Down
68 changes: 68 additions & 0 deletions internal/mode/static/state/graph/reference_grant_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,8 @@ import (
)

func TestReferenceGrantResolver(t *testing.T) {
t.Parallel()

gwNs := "gw-ns"
secretNsName := types.NamespacedName{Namespace: "test", Name: "certificate"}

Expand Down Expand Up @@ -160,7 +162,10 @@ func TestReferenceGrantResolver(t *testing.T) {
resolver := newReferenceGrantResolver(refGrants)

for _, test := range tests {
test := test
t.Run(test.msg, func(t *testing.T) {
t.Parallel()

g := NewWithT(t)

g.Expect(resolver.refAllowed(test.to, test.from)).To(Equal(test.allowed))
Expand All @@ -169,6 +174,8 @@ func TestReferenceGrantResolver(t *testing.T) {
}

func TestToSecret(t *testing.T) {
t.Parallel()

ref := toSecret(types.NamespacedName{Namespace: "ns", Name: "secret"})

exp := toResource{
Expand All @@ -182,6 +189,8 @@ func TestToSecret(t *testing.T) {
}

func TestToService(t *testing.T) {
t.Parallel()

ref := toService(types.NamespacedName{Namespace: "ns", Name: "service"})

exp := toResource{
Expand All @@ -195,6 +204,8 @@ func TestToService(t *testing.T) {
}

func TestFromGateway(t *testing.T) {
t.Parallel()

ref := fromGateway("ns")

exp := fromResource{
Expand All @@ -208,6 +219,8 @@ func TestFromGateway(t *testing.T) {
}

func TestFromHTTPRoute(t *testing.T) {
t.Parallel()

ref := fromHTTPRoute("ns")

exp := fromResource{
Expand All @@ -221,6 +234,8 @@ func TestFromHTTPRoute(t *testing.T) {
}

func TestFromGRPCRoute(t *testing.T) {
t.Parallel()

ref := fromGRPCRoute("ns")

exp := fromResource{
Expand All @@ -233,17 +248,38 @@ func TestFromGRPCRoute(t *testing.T) {
g.Expect(ref).To(Equal(exp))
}

func TestFromTLSRoute(t *testing.T) {
t.Parallel()

ref := fromTLSRoute("ns")
kate-osborn marked this conversation as resolved.
Show resolved Hide resolved

exp := fromResource{
group: v1beta1.GroupName,
kind: kinds.TLSRoute,
namespace: "ns",
}

g := NewWithT(t)
g.Expect(ref).To(Equal(exp))
}

func TestRefAllowedFrom(t *testing.T) {
t.Parallel()

gwNs := "gw-ns"
hrNs := "hr-ns"
grNs := "gr-ns"
trNs := "tr-ns"

allowedHTTPRouteNs := "hr-allowed-ns"
allowedHTTPRouteNsName := types.NamespacedName{Namespace: allowedHTTPRouteNs, Name: "all-allowed-in-ns"}

allowedGRPCRouteNs := "gr-allowed-ns"
allowedGRPCRouteNsName := types.NamespacedName{Namespace: allowedGRPCRouteNs, Name: "all-allowed-in-ns"}

allowedTLSRouteNs := "tr-allowed-ns"
allowedTLSRouteNsName := types.NamespacedName{Namespace: allowedTLSRouteNs, Name: "all-allowed-in-ns"}

allowedGatewayNs := "gw-allowed-ns"
allowedGatewayNsName := types.NamespacedName{Namespace: allowedGatewayNs, Name: "all-allowed-in-ns"}

Expand Down Expand Up @@ -298,11 +334,28 @@ func TestRefAllowedFrom(t *testing.T) {
},
},
},
{Namespace: allowedTLSRouteNs, Name: "tr-2-svc"}: {
Spec: v1beta1.ReferenceGrantSpec{
From: []v1beta1.ReferenceGrantFrom{
{
Group: v1beta1.GroupName,
Kind: kinds.TLSRoute,
Namespace: v1beta1.Namespace(trNs),
},
},
To: []v1beta1.ReferenceGrantTo{
{
Kind: "Service",
},
},
},
},
}

resolver := newReferenceGrantResolver(refGrants)
refAllowedFromGRPCRoute := resolver.refAllowedFrom(fromGRPCRoute(grNs))
refAllowedFromHTTPRoute := resolver.refAllowedFrom(fromHTTPRoute(hrNs))
refAllowedFromTLSRoute := resolver.refAllowedFrom(fromTLSRoute(trNs))
refAllowedFromGateway := resolver.refAllowedFrom(fromGateway(gwNs))

tests := []struct {
Expand Down Expand Up @@ -347,10 +400,25 @@ func TestRefAllowedFrom(t *testing.T) {
toResource: toService(notAllowedNsName),
expAllowed: false,
},
{
name: "ref allowed from tlsroute to service",
refAllowedFrom: refAllowedFromTLSRoute,
toResource: toService(allowedTLSRouteNsName),
expAllowed: true,
},
{
name: "ref not allowed from tlsroute to service",
refAllowedFrom: refAllowedFromTLSRoute,
toResource: toService(notAllowedNsName),
expAllowed: false,
},
}

for _, test := range tests {
test := test
t.Run(test.name, func(t *testing.T) {
kate-osborn marked this conversation as resolved.
Show resolved Hide resolved
t.Parallel()

g := NewWithT(t)
g.Expect(test.refAllowedFrom(test.toResource)).To(Equal(test.expAllowed))
})
Expand Down
9 changes: 8 additions & 1 deletion internal/mode/static/state/graph/route_common.go
Original file line number Diff line number Diff line change
Expand Up @@ -178,14 +178,21 @@ func buildL4RoutesForGateways(
gatewayNsNames []types.NamespacedName,
services map[types.NamespacedName]*apiv1.Service,
npCfg *NginxProxy,
resolver *referenceGrantResolver,
) map[L4RouteKey]*L4Route {
if len(gatewayNsNames) == 0 {
return nil
}

routes := make(map[L4RouteKey]*L4Route)
for _, route := range tlsRoutes {
r := buildTLSRoute(route, gatewayNsNames, services, npCfg)
r := buildTLSRoute(
route,
gatewayNsNames,
services,
npCfg,
resolver.refAllowedFrom(fromTLSRoute(route.Namespace)),
)
if r != nil {
routes[CreateRouteKeyL4(route)] = r
}
Expand Down
3 changes: 3 additions & 0 deletions internal/mode/static/state/graph/route_common_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -2436,11 +2436,14 @@ func TestBuildL4RoutesForGateways_NoGateways(t *testing.T) {
},
}

refGrantResolver := newReferenceGrantResolver(nil)

g.Expect(buildL4RoutesForGateways(
tlsRoutes,
nil,
services,
nil,
refGrantResolver,
)).To(BeNil())
}

Expand Down
66 changes: 31 additions & 35 deletions internal/mode/static/state/graph/tlsroute.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ func buildTLSRoute(
gatewayNsNames []types.NamespacedName,
services map[types.NamespacedName]*apiv1.Service,
npCfg *NginxProxy,
refGrantResolver func(resource toResource) bool,
) *L4Route {
r := &L4Route{
Source: gtr,
Expand Down Expand Up @@ -53,15 +54,14 @@ func buildTLSRoute(
return r
}

cond, br := validateBackendRefTLSRoute(gtr, services, npCfg)
br, cond := validateBackendRefTLSRoute(gtr, services, npCfg, refGrantResolver)

r.Spec.BackendRef = br
r.Valid = true
r.Attachable = true

if cond != nil {
r.Conditions = append(r.Conditions, *cond)
r.Valid = false
}

return r
Expand All @@ -70,12 +70,26 @@ func buildTLSRoute(
func validateBackendRefTLSRoute(gtr *v1alpha2.TLSRoute,
services map[types.NamespacedName]*apiv1.Service,
npCfg *NginxProxy,
) (*conditions.Condition, BackendRef) {
refGrantResolver func(resource toResource) bool,
) (BackendRef, *conditions.Condition) {
// Length of BackendRefs and Rules is guaranteed to be one due to earlier check in buildTLSRoute
refPath := field.NewPath("spec").Child("rules").Index(0).Child("backendRefs").Index(0)

ref := gtr.Spec.Rules[0].BackendRefs[0]

if valid, cond := validateBackendRef(
ref,
gtr.Namespace,
refGrantResolver,
refPath,
); !valid {
backendRef := BackendRef{
Valid: false,
}

return backendRef, &cond
}

ns := gtr.Namespace
if ref.Namespace != nil {
ns = string(*ref.Namespace)
Expand All @@ -86,48 +100,30 @@ func validateBackendRefTLSRoute(gtr *v1alpha2.TLSRoute,
Name: string(gtr.Spec.Rules[0].BackendRefs[0].Name),
}

backendRef := BackendRef{
Valid: true,
}
var cond *conditions.Condition

if ref.Port == nil {
valErr := field.Required(refPath.Child("port"), "port cannot be nil")
backendRef.Valid = false
cond = helpers.GetPointer(staticConds.NewRouteBackendRefUnsupportedValue(valErr.Error()))

return cond, backendRef
}

svcIPFamily, svcPort, err := getIPFamilyAndPortFromRef(
ref,
svcNsName,
services,
refPath,
)

backendRef.ServicePort = svcPort
backendRef.SvcNsName = svcNsName
backendRef := BackendRef{
SvcNsName: svcNsName,
ServicePort: svcPort,
Valid: true,
}

if err != nil {
backendRef.Valid = false
cond = helpers.GetPointer(staticConds.NewRouteBackendRefRefBackendNotFound(err.Error()))
} else if err := verifyIPFamily(npCfg, svcIPFamily); err != nil {
bjee19 marked this conversation as resolved.
Show resolved Hide resolved
backendRef.Valid = false
cond = helpers.GetPointer(staticConds.NewRouteInvalidIPFamily(err.Error()))
} else if ref.Group != nil && !(*ref.Group == "core" || *ref.Group == "") {
valErr := field.NotSupported(refPath.Child("group"), *ref.Group, []string{"core", ""})
backendRef.Valid = false
cond = helpers.GetPointer(staticConds.NewRouteBackendRefInvalidKind(valErr.Error()))
} else if ref.Kind != nil && *ref.Kind != "Service" {
valErr := field.NotSupported(refPath.Child("kind"), *ref.Kind, []string{"Service"})
backendRef.Valid = false
cond = helpers.GetPointer(staticConds.NewRouteBackendRefInvalidKind(valErr.Error()))
} else if ref.Namespace != nil && string(*ref.Namespace) != gtr.Namespace {
msg := "Cross-namespace routing is not supported"

return backendRef, helpers.GetPointer(staticConds.NewRouteBackendRefRefBackendNotFound(err.Error()))
}

if err := verifyIPFamily(npCfg, svcIPFamily); err != nil {
backendRef.Valid = false
cond = helpers.GetPointer(staticConds.NewRouteBackendRefUnsupportedValue(msg))

return backendRef, helpers.GetPointer(staticConds.NewRouteInvalidIPFamily(err.Error()))
}
// FIXME(sarthyparty): Add check for invalid weights, we removed checks to pass the conformance test
return cond, backendRef

return backendRef, nil
}
Loading
Loading