Skip to content

Commit 7b5c6ab

Browse files
authored
Check for ReferenceGrants from GRPCRoutes to Services (#2337)
Problem: ReferenceGrants that permit GRPCRoutes to reference Services in different namespaces are ignored by NGF, effectively preventing all cross-namespace routing for GRPC traffic. Solution: Check for ReferenceGrants from GRPCRoutes to Services when validating GRPCRoutes. If a ReferenceGrant exists that permits the cross-namespace reference, allow it.
1 parent 9840a51 commit 7b5c6ab

File tree

5 files changed

+266
-65
lines changed

5 files changed

+266
-65
lines changed

internal/mode/static/state/graph/backend_refs.go

Lines changed: 18 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -84,11 +84,12 @@ func addBackendRefsToRules(
8484

8585
for refIdx, ref := range rule.RouteBackendRefs {
8686
refPath := field.NewPath("spec").Child("rules").Index(idx).Child("backendRefs").Index(refIdx)
87+
routeNs := route.Source.GetNamespace()
8788

8889
ref, cond := createBackendRef(
8990
ref,
90-
route.Source.GetNamespace(),
91-
refGrantResolver,
91+
routeNs,
92+
refGrantResolver.refAllowedFrom(getRefGrantFromResourceForRoute(route.RouteType, routeNs)),
9293
services,
9394
refPath,
9495
backendTLSPolicies,
@@ -118,7 +119,7 @@ func addBackendRefsToRules(
118119
func createBackendRef(
119120
ref RouteBackendRef,
120121
sourceNamespace string,
121-
refGrantResolver *referenceGrantResolver,
122+
refGrantResolver func(resource toResource) bool,
122123
services map[types.NamespacedName]*v1.Service,
123124
refPath *field.Path,
124125
backendTLSPolicies map[types.NamespacedName]*BackendTLSPolicy,
@@ -347,7 +348,7 @@ func verifyIPFamily(npCfg *NginxProxy, svcIPFamily []v1.IPFamily) error {
347348
func validateRouteBackendRef(
348349
ref RouteBackendRef,
349350
routeNs string,
350-
refGrantResolver *referenceGrantResolver,
351+
refGrantResolver func(resource toResource) bool,
351352
path *field.Path,
352353
) (valid bool, cond conditions.Condition) {
353354
// Because all errors cause the same condition but different reasons, we return as soon as we find an error
@@ -362,7 +363,7 @@ func validateRouteBackendRef(
362363
func validateBackendRef(
363364
ref gatewayv1.BackendRef,
364365
routeNs string,
365-
refGrantResolver *referenceGrantResolver,
366+
refGrantResolver func(toResource toResource) bool,
366367
path *field.Path,
367368
) (valid bool, cond conditions.Condition) {
368369
// Because all errors cause same condition but different reasons, we return as soon as we find an error
@@ -382,7 +383,7 @@ func validateBackendRef(
382383
if ref.Namespace != nil && string(*ref.Namespace) != routeNs {
383384
refNsName := types.NamespacedName{Namespace: string(*ref.Namespace), Name: string(ref.Name)}
384385

385-
if !refGrantResolver.refAllowed(toService(refNsName), fromHTTPRoute(routeNs)) {
386+
if !refGrantResolver(toService(refNsName)) {
386387
msg := fmt.Sprintf("Backend ref to Service %s not permitted by any ReferenceGrant", refNsName)
387388

388389
return false, staticConds.NewRouteBackendRefRefNotPermitted(msg)
@@ -428,3 +429,14 @@ func getServicePort(svc *v1.Service, port int32) (v1.ServicePort, error) {
428429

429430
return v1.ServicePort{}, fmt.Errorf("no matching port for Service %s and port %d", svc.Name, port)
430431
}
432+
433+
func getRefGrantFromResourceForRoute(routeType RouteType, routeNs string) fromResource {
434+
switch routeType {
435+
case RouteTypeHTTP:
436+
return fromHTTPRoute(routeNs)
437+
case RouteTypeGRPC:
438+
return fromGRPCRoute(routeNs)
439+
default:
440+
panic(fmt.Errorf("unknown route type %s", routeType))
441+
}
442+
}

internal/mode/static/state/graph/backend_refs_test.go

Lines changed: 69 additions & 55 deletions
Original file line numberDiff line numberDiff line change
@@ -13,12 +13,10 @@ import (
1313
gatewayv1 "sigs.k8s.io/gateway-api/apis/v1"
1414
"sigs.k8s.io/gateway-api/apis/v1alpha2"
1515
"sigs.k8s.io/gateway-api/apis/v1alpha3"
16-
"sigs.k8s.io/gateway-api/apis/v1beta1"
1716

1817
ngfAPI "github.com/nginxinc/nginx-gateway-fabric/apis/v1alpha1"
1918
"github.com/nginxinc/nginx-gateway-fabric/internal/framework/conditions"
2019
"github.com/nginxinc/nginx-gateway-fabric/internal/framework/helpers"
21-
"github.com/nginxinc/nginx-gateway-fabric/internal/framework/kinds"
2220
staticConds "github.com/nginxinc/nginx-gateway-fabric/internal/mode/static/state/conditions"
2321
)
2422

@@ -88,9 +86,9 @@ func TestValidateRouteBackendRef(t *testing.T) {
8886
for _, test := range tests {
8987
t.Run(test.name, func(t *testing.T) {
9088
g := NewWithT(t)
91-
resolver := newReferenceGrantResolver(nil)
89+
alwaysTrueRefGrantResolver := func(_ toResource) bool { return true }
9290

93-
valid, cond := validateRouteBackendRef(test.ref, "test", resolver, field.NewPath("test"))
91+
valid, cond := validateRouteBackendRef(test.ref, "test", alwaysTrueRefGrantResolver, field.NewPath("test"))
9492

9593
g.Expect(valid).To(Equal(test.expectedValid))
9694
g.Expect(cond).To(Equal(test.expectedCondition))
@@ -99,84 +97,57 @@ func TestValidateRouteBackendRef(t *testing.T) {
9997
}
10098

10199
func TestValidateBackendRef(t *testing.T) {
102-
specificRefGrant := &v1beta1.ReferenceGrant{
103-
Spec: v1beta1.ReferenceGrantSpec{
104-
To: []v1beta1.ReferenceGrantTo{
105-
{
106-
Kind: "Service",
107-
Name: helpers.GetPointer[gatewayv1.ObjectName]("service1"),
108-
},
109-
},
110-
From: []v1beta1.ReferenceGrantFrom{
111-
{
112-
Group: gatewayv1.GroupName,
113-
Kind: kinds.HTTPRoute,
114-
Namespace: "test",
115-
},
116-
},
117-
},
118-
}
119-
120-
allInNamespaceRefGrant := specificRefGrant.DeepCopy()
121-
allInNamespaceRefGrant.Spec.To[0].Name = nil
100+
alwaysFalseRefGrantResolver := func(_ toResource) bool { return false }
101+
alwaysTrueRefGrantResolver := func(_ toResource) bool { return true }
122102

123103
tests := []struct {
124104
ref gatewayv1.BackendRef
125-
refGrants map[types.NamespacedName]*v1beta1.ReferenceGrant
105+
refGrantResolver func(resource toResource) bool
126106
expectedCondition conditions.Condition
127107
name string
128108
expectedValid bool
129109
}{
130110
{
131-
name: "normal case",
132-
ref: getNormalRef(),
133-
expectedValid: true,
111+
name: "normal case",
112+
ref: getNormalRef(),
113+
refGrantResolver: alwaysTrueRefGrantResolver,
114+
expectedValid: true,
134115
},
135116
{
136117
name: "normal case with implicit namespace",
137118
ref: getModifiedRef(func(backend gatewayv1.BackendRef) gatewayv1.BackendRef {
138119
backend.Namespace = nil
139120
return backend
140121
}),
141-
expectedValid: true,
122+
refGrantResolver: alwaysTrueRefGrantResolver,
123+
expectedValid: true,
142124
},
143125
{
144126
name: "normal case with implicit kind Service",
145127
ref: getModifiedRef(func(backend gatewayv1.BackendRef) gatewayv1.BackendRef {
146128
backend.Kind = nil
147129
return backend
148130
}),
149-
expectedValid: true,
131+
refGrantResolver: alwaysTrueRefGrantResolver,
132+
expectedValid: true,
150133
},
151134
{
152-
name: "normal case with backend ref allowed by specific reference grant",
135+
name: "normal case with backend ref allowed by reference grant",
153136
ref: getModifiedRef(func(backend gatewayv1.BackendRef) gatewayv1.BackendRef {
154137
backend.Namespace = helpers.GetPointer[gatewayv1.Namespace]("cross-ns")
155138
return backend
156139
}),
157-
refGrants: map[types.NamespacedName]*v1beta1.ReferenceGrant{
158-
{Namespace: "cross-ns", Name: "rg"}: specificRefGrant,
159-
},
160-
expectedValid: true,
161-
},
162-
{
163-
name: "normal case with backend ref allowed by all-in-namespace reference grant",
164-
ref: getModifiedRef(func(backend gatewayv1.BackendRef) gatewayv1.BackendRef {
165-
backend.Namespace = helpers.GetPointer[gatewayv1.Namespace]("cross-ns")
166-
return backend
167-
}),
168-
refGrants: map[types.NamespacedName]*v1beta1.ReferenceGrant{
169-
{Namespace: "cross-ns", Name: "rg"}: allInNamespaceRefGrant,
170-
},
171-
expectedValid: true,
140+
refGrantResolver: alwaysTrueRefGrantResolver,
141+
expectedValid: true,
172142
},
173143
{
174144
name: "invalid group",
175145
ref: getModifiedRef(func(backend gatewayv1.BackendRef) gatewayv1.BackendRef {
176146
backend.Group = helpers.GetPointer[gatewayv1.Group]("invalid")
177147
return backend
178148
}),
179-
expectedValid: false,
149+
refGrantResolver: alwaysTrueRefGrantResolver,
150+
expectedValid: false,
180151
expectedCondition: staticConds.NewRouteBackendRefInvalidKind(
181152
`test.group: Unsupported value: "invalid": supported values: "core", ""`,
182153
),
@@ -187,7 +158,8 @@ func TestValidateBackendRef(t *testing.T) {
187158
backend.Kind = helpers.GetPointer[gatewayv1.Kind]("NotService")
188159
return backend
189160
}),
190-
expectedValid: false,
161+
refGrantResolver: alwaysTrueRefGrantResolver,
162+
expectedValid: false,
191163
expectedCondition: staticConds.NewRouteBackendRefInvalidKind(
192164
`test.kind: Unsupported value: "NotService": supported values: "Service"`,
193165
),
@@ -198,7 +170,8 @@ func TestValidateBackendRef(t *testing.T) {
198170
backend.Namespace = helpers.GetPointer[gatewayv1.Namespace]("invalid")
199171
return backend
200172
}),
201-
expectedValid: false,
173+
refGrantResolver: alwaysFalseRefGrantResolver,
174+
expectedValid: false,
202175
expectedCondition: staticConds.NewRouteBackendRefRefNotPermitted(
203176
"Backend ref to Service invalid/service1 not permitted by any ReferenceGrant",
204177
),
@@ -209,7 +182,8 @@ func TestValidateBackendRef(t *testing.T) {
209182
backend.Weight = helpers.GetPointer[int32](-1)
210183
return backend
211184
}),
212-
expectedValid: false,
185+
refGrantResolver: alwaysTrueRefGrantResolver,
186+
expectedValid: false,
213187
expectedCondition: staticConds.NewRouteBackendRefUnsupportedValue(
214188
"test.weight: Invalid value: -1: must be in the range [0, 1000000]",
215189
),
@@ -220,7 +194,8 @@ func TestValidateBackendRef(t *testing.T) {
220194
backend.Port = nil
221195
return backend
222196
}),
223-
expectedValid: false,
197+
refGrantResolver: alwaysTrueRefGrantResolver,
198+
expectedValid: false,
224199
expectedCondition: staticConds.NewRouteBackendRefUnsupportedValue(
225200
"test.port: Required value: port cannot be nil",
226201
),
@@ -231,8 +206,7 @@ func TestValidateBackendRef(t *testing.T) {
231206
t.Run(test.name, func(t *testing.T) {
232207
g := NewWithT(t)
233208

234-
resolver := newReferenceGrantResolver(test.refGrants)
235-
valid, cond := validateBackendRef(test.ref, "test", resolver, field.NewPath("test"))
209+
valid, cond := validateBackendRef(test.ref, "test", test.refGrantResolver, field.NewPath("test"))
236210

237211
g.Expect(valid).To(Equal(test.expectedValid))
238212
g.Expect(cond).To(Equal(test.expectedCondition))
@@ -437,6 +411,7 @@ func TestAddBackendRefsToRulesTest(t *testing.T) {
437411
Name: name,
438412
},
439413
},
414+
RouteType: RouteTypeHTTP,
440415
ParentRefs: sectionNameRefs,
441416
Valid: true,
442417
}
@@ -1034,7 +1009,7 @@ func TestCreateBackend(t *testing.T) {
10341009
t.Run(test.name, func(t *testing.T) {
10351010
g := NewWithT(t)
10361011

1037-
resolver := newReferenceGrantResolver(nil)
1012+
alwaysTrueRefGrantResolver := func(_ toResource) bool { return true }
10381013

10391014
rbr := RouteBackendRef{
10401015
test.ref.BackendRef,
@@ -1043,7 +1018,7 @@ func TestCreateBackend(t *testing.T) {
10431018
backend, cond := createBackendRef(
10441019
rbr,
10451020
sourceNamespace,
1046-
resolver,
1021+
alwaysTrueRefGrantResolver,
10471022
services,
10481023
refPath,
10491024
policies,
@@ -1265,3 +1240,42 @@ func TestFindBackendTLSPolicyForService(t *testing.T) {
12651240
})
12661241
}
12671242
}
1243+
1244+
func TestGetRefGrantFromResourceForRoute(t *testing.T) {
1245+
tests := []struct {
1246+
name string
1247+
routeType RouteType
1248+
ns string
1249+
expFromResource fromResource
1250+
}{
1251+
{
1252+
name: "HTTPRoute",
1253+
routeType: RouteTypeHTTP,
1254+
ns: "hr",
1255+
expFromResource: fromHTTPRoute("hr"),
1256+
},
1257+
{
1258+
name: "GRPCRoute",
1259+
routeType: RouteTypeGRPC,
1260+
ns: "gr",
1261+
expFromResource: fromGRPCRoute("gr"),
1262+
},
1263+
}
1264+
1265+
for _, test := range tests {
1266+
t.Run(test.name, func(t *testing.T) {
1267+
g := NewWithT(t)
1268+
g.Expect(getRefGrantFromResourceForRoute(test.routeType, test.ns)).To(Equal(test.expFromResource))
1269+
})
1270+
}
1271+
}
1272+
1273+
func TestGetRefGrantFromResourceForRoute_Panics(t *testing.T) {
1274+
g := NewWithT(t)
1275+
1276+
get := func() {
1277+
getRefGrantFromResourceForRoute("unknown", "ns")
1278+
}
1279+
1280+
g.Expect(get).To(Panic())
1281+
}

internal/mode/static/state/graph/graph_test.go

Lines changed: 26 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -306,9 +306,9 @@ func TestBuildGraph(t *testing.T) {
306306
},
307307
}
308308

309-
rgService := &v1beta1.ReferenceGrant{
309+
hrToServiceNsRefGrant := &v1beta1.ReferenceGrant{
310310
ObjectMeta: metav1.ObjectMeta{
311-
Name: "rg-service",
311+
Name: "hr-to-service",
312312
Namespace: "service",
313313
},
314314
Spec: v1beta1.ReferenceGrantSpec{
@@ -327,6 +327,27 @@ func TestBuildGraph(t *testing.T) {
327327
},
328328
}
329329

330+
grToServiceNsRefGrant := &v1beta1.ReferenceGrant{
331+
ObjectMeta: metav1.ObjectMeta{
332+
Name: "gr-to-service",
333+
Namespace: "service",
334+
},
335+
Spec: v1beta1.ReferenceGrantSpec{
336+
From: []v1beta1.ReferenceGrantFrom{
337+
{
338+
Group: gatewayv1.GroupName,
339+
Kind: kinds.GRPCRoute,
340+
Namespace: gatewayv1.Namespace(testNs),
341+
},
342+
},
343+
To: []v1beta1.ReferenceGrantTo{
344+
{
345+
Kind: "Service",
346+
},
347+
},
348+
},
349+
}
350+
330351
proxy := &ngfAPI.NginxProxy{
331352
ObjectMeta: metav1.ObjectMeta{
332353
Name: "nginx-proxy",
@@ -443,8 +464,9 @@ func TestBuildGraph(t *testing.T) {
443464
client.ObjectKeyFromObject(ns): ns,
444465
},
445466
ReferenceGrants: map[types.NamespacedName]*v1beta1.ReferenceGrant{
446-
client.ObjectKeyFromObject(rgSecret): rgSecret,
447-
client.ObjectKeyFromObject(rgService): rgService,
467+
client.ObjectKeyFromObject(rgSecret): rgSecret,
468+
client.ObjectKeyFromObject(hrToServiceNsRefGrant): hrToServiceNsRefGrant,
469+
client.ObjectKeyFromObject(grToServiceNsRefGrant): grToServiceNsRefGrant,
448470
},
449471
Secrets: map[types.NamespacedName]*v1.Secret{
450472
client.ObjectKeyFromObject(secret): secret,

internal/mode/static/state/graph/reference_grant.go

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -73,6 +73,14 @@ func fromHTTPRoute(namespace string) fromResource {
7373
}
7474
}
7575

76+
func fromGRPCRoute(namespace string) fromResource {
77+
return fromResource{
78+
group: v1.GroupName,
79+
kind: kinds.GRPCRoute,
80+
namespace: namespace,
81+
}
82+
}
83+
7684
// newReferenceGrantResolver creates a new referenceGrantResolver.
7785
func newReferenceGrantResolver(refGrants map[types.NamespacedName]*v1beta1.ReferenceGrant) *referenceGrantResolver {
7886
allowed := make(map[allowedReference]struct{})
@@ -137,3 +145,11 @@ func (r *referenceGrantResolver) refAllowed(to toResource, from fromResource) bo
137145

138146
return false
139147
}
148+
149+
// refAllowedFrom returns a closure function that takes a toResource parameter
150+
// and checks if a reference from the specified fromResource to the given toResource is allowed.
151+
func (r *referenceGrantResolver) refAllowedFrom(from fromResource) func(to toResource) bool {
152+
return func(to toResource) bool {
153+
return r.refAllowed(to, from)
154+
}
155+
}

0 commit comments

Comments
 (0)