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.

Sign up for GitHub

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

Check for ReferenceGrants from GRPCRoutes to Services #2337

Merged
merged 1 commit into from
Aug 7, 2024
Merged

Check for ReferenceGrants from GRPCRoutes to Services #2337

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
24 changes: 18 additions & 6 deletions internal/mode/static/state/graph/backend_refs.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -84,11 +84,12 @@ func addBackendRefsToRules(

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

ref, cond := createBackendRef(
ref,
route.Source.GetNamespace(),
refGrantResolver,
routeNs,
refGrantResolver.refAllowedFrom(getRefGrantFromResourceForRoute(route.RouteType, routeNs)),
services,
refPath,
backendTLSPolicies,
Expand Down Expand Up @@ -118,7 +119,7 @@ func addBackendRefsToRules(
func createBackendRef(
ref RouteBackendRef,
sourceNamespace string,
refGrantResolver *referenceGrantResolver,
refGrantResolver func(resource toResource) bool,
services map[types.NamespacedName]*v1.Service,
refPath *field.Path,
backendTLSPolicies map[types.NamespacedName]*BackendTLSPolicy,
Expand Down Expand Up @@ -347,7 +348,7 @@ func verifyIPFamily(npCfg *NginxProxy, svcIPFamily []v1.IPFamily) error {
func validateRouteBackendRef(
ref RouteBackendRef,
routeNs string,
refGrantResolver *referenceGrantResolver,
refGrantResolver func(resource toResource) bool,
path *field.Path,
) (valid bool, cond conditions.Condition) {
// Because all errors cause the same condition but different reasons, we return as soon as we find an error
Expand All @@ -362,7 +363,7 @@ func validateRouteBackendRef(
func validateBackendRef(
ref gatewayv1.BackendRef,
routeNs string,
refGrantResolver *referenceGrantResolver,
refGrantResolver func(toResource toResource) bool,
path *field.Path,
) (valid bool, cond conditions.Condition) {
// Because all errors cause same condition but different reasons, we return as soon as we find an error
Expand All @@ -382,7 +383,7 @@ func validateBackendRef(
if ref.Namespace != nil && string(*ref.Namespace) != routeNs {
refNsName := types.NamespacedName{Namespace: string(*ref.Namespace), Name: string(ref.Name)}

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

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

return v1.ServicePort{}, fmt.Errorf("no matching port for Service %s and port %d", svc.Name, port)
}

func getRefGrantFromResourceForRoute(routeType RouteType, routeNs string) fromResource {
switch routeType {
case RouteTypeHTTP:
return fromHTTPRoute(routeNs)
case RouteTypeGRPC:
return fromGRPCRoute(routeNs)
default:
panic(fmt.Errorf("unknown route type %s", routeType))
}
}
124 changes: 69 additions & 55 deletions internal/mode/static/state/graph/backend_refs_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,12 +13,10 @@ import (
gatewayv1 "sigs.k8s.io/gateway-api/apis/v1"
"sigs.k8s.io/gateway-api/apis/v1alpha2"
"sigs.k8s.io/gateway-api/apis/v1alpha3"
"sigs.k8s.io/gateway-api/apis/v1beta1"

ngfAPI "github.com/nginxinc/nginx-gateway-fabric/apis/v1alpha1"
"github.com/nginxinc/nginx-gateway-fabric/internal/framework/conditions"
"github.com/nginxinc/nginx-gateway-fabric/internal/framework/helpers"
"github.com/nginxinc/nginx-gateway-fabric/internal/framework/kinds"
staticConds "github.com/nginxinc/nginx-gateway-fabric/internal/mode/static/state/conditions"
)

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

valid, cond := validateRouteBackendRef(test.ref, "test", resolver, field.NewPath("test"))
valid, cond := validateRouteBackendRef(test.ref, "test", alwaysTrueRefGrantResolver, field.NewPath("test"))

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

func TestValidateBackendRef(t *testing.T) {
specificRefGrant := &v1beta1.ReferenceGrant{
Spec: v1beta1.ReferenceGrantSpec{
To: []v1beta1.ReferenceGrantTo{
{
Kind: "Service",
Name: helpers.GetPointer[gatewayv1.ObjectName]("service1"),
},
},
From: []v1beta1.ReferenceGrantFrom{
{
Group: gatewayv1.GroupName,
Kind: kinds.HTTPRoute,
Namespace: "test",
},
},
},
}

allInNamespaceRefGrant := specificRefGrant.DeepCopy()
allInNamespaceRefGrant.Spec.To[0].Name = nil
alwaysFalseRefGrantResolver := func(_ toResource) bool { return false }
alwaysTrueRefGrantResolver := func(_ toResource) bool { return true }

tests := []struct {
ref gatewayv1.BackendRef
refGrants map[types.NamespacedName]*v1beta1.ReferenceGrant
refGrantResolver func(resource toResource) bool
expectedCondition conditions.Condition
name string
expectedValid bool
}{
{
name: "normal case",
ref: getNormalRef(),
expectedValid: true,
name: "normal case",
ref: getNormalRef(),
refGrantResolver: alwaysTrueRefGrantResolver,
expectedValid: true,
},
{
name: "normal case with implicit namespace",
ref: getModifiedRef(func(backend gatewayv1.BackendRef) gatewayv1.BackendRef {
backend.Namespace = nil
return backend
}),
expectedValid: true,
refGrantResolver: alwaysTrueRefGrantResolver,
expectedValid: true,
},
{
name: "normal case with implicit kind Service",
ref: getModifiedRef(func(backend gatewayv1.BackendRef) gatewayv1.BackendRef {
backend.Kind = nil
return backend
}),
expectedValid: true,
refGrantResolver: alwaysTrueRefGrantResolver,
expectedValid: true,
},
{
name: "normal case with backend ref allowed by specific reference grant",
name: "normal case with backend ref allowed by reference grant",
ref: getModifiedRef(func(backend gatewayv1.BackendRef) gatewayv1.BackendRef {
backend.Namespace = helpers.GetPointer[gatewayv1.Namespace]("cross-ns")
return backend
}),
refGrants: map[types.NamespacedName]*v1beta1.ReferenceGrant{
{Namespace: "cross-ns", Name: "rg"}: specificRefGrant,
},
expectedValid: true,
},
{
name: "normal case with backend ref allowed by all-in-namespace reference grant",
ref: getModifiedRef(func(backend gatewayv1.BackendRef) gatewayv1.BackendRef {
backend.Namespace = helpers.GetPointer[gatewayv1.Namespace]("cross-ns")
return backend
}),
refGrants: map[types.NamespacedName]*v1beta1.ReferenceGrant{
{Namespace: "cross-ns", Name: "rg"}: allInNamespaceRefGrant,
},
expectedValid: true,
refGrantResolver: alwaysTrueRefGrantResolver,
expectedValid: true,
},
{
name: "invalid group",
ref: getModifiedRef(func(backend gatewayv1.BackendRef) gatewayv1.BackendRef {
backend.Group = helpers.GetPointer[gatewayv1.Group]("invalid")
return backend
}),
expectedValid: false,
refGrantResolver: alwaysTrueRefGrantResolver,
expectedValid: false,
expectedCondition: staticConds.NewRouteBackendRefInvalidKind(
`test.group: Unsupported value: "invalid": supported values: "core", ""`,
),
Expand All @@ -187,7 +158,8 @@ func TestValidateBackendRef(t *testing.T) {
backend.Kind = helpers.GetPointer[gatewayv1.Kind]("NotService")
return backend
}),
expectedValid: false,
refGrantResolver: alwaysTrueRefGrantResolver,
expectedValid: false,
expectedCondition: staticConds.NewRouteBackendRefInvalidKind(
`test.kind: Unsupported value: "NotService": supported values: "Service"`,
),
Expand All @@ -198,7 +170,8 @@ func TestValidateBackendRef(t *testing.T) {
backend.Namespace = helpers.GetPointer[gatewayv1.Namespace]("invalid")
return backend
}),
expectedValid: false,
refGrantResolver: alwaysFalseRefGrantResolver,
expectedValid: false,
expectedCondition: staticConds.NewRouteBackendRefRefNotPermitted(
"Backend ref to Service invalid/service1 not permitted by any ReferenceGrant",
),
Expand All @@ -209,7 +182,8 @@ func TestValidateBackendRef(t *testing.T) {
backend.Weight = helpers.GetPointer[int32](-1)
return backend
}),
expectedValid: false,
refGrantResolver: alwaysTrueRefGrantResolver,
expectedValid: false,
expectedCondition: staticConds.NewRouteBackendRefUnsupportedValue(
"test.weight: Invalid value: -1: must be in the range [0, 1000000]",
),
Expand All @@ -220,7 +194,8 @@ func TestValidateBackendRef(t *testing.T) {
backend.Port = nil
return backend
}),
expectedValid: false,
refGrantResolver: alwaysTrueRefGrantResolver,
expectedValid: false,
expectedCondition: staticConds.NewRouteBackendRefUnsupportedValue(
"test.port: Required value: port cannot be nil",
),
Expand All @@ -231,8 +206,7 @@ func TestValidateBackendRef(t *testing.T) {
t.Run(test.name, func(t *testing.T) {
g := NewWithT(t)

resolver := newReferenceGrantResolver(test.refGrants)
valid, cond := validateBackendRef(test.ref, "test", resolver, field.NewPath("test"))
valid, cond := validateBackendRef(test.ref, "test", test.refGrantResolver, field.NewPath("test"))

g.Expect(valid).To(Equal(test.expectedValid))
g.Expect(cond).To(Equal(test.expectedCondition))
Expand Down Expand Up @@ -437,6 +411,7 @@ func TestAddBackendRefsToRulesTest(t *testing.T) {
Name: name,
},
},
RouteType: RouteTypeHTTP,
ParentRefs: sectionNameRefs,
Valid: true,
}
Expand Down Expand Up @@ -1034,7 +1009,7 @@ func TestCreateBackend(t *testing.T) {
t.Run(test.name, func(t *testing.T) {
g := NewWithT(t)

resolver := newReferenceGrantResolver(nil)
alwaysTrueRefGrantResolver := func(_ toResource) bool { return true }

rbr := RouteBackendRef{
test.ref.BackendRef,
Expand All @@ -1043,7 +1018,7 @@ func TestCreateBackend(t *testing.T) {
backend, cond := createBackendRef(
rbr,
sourceNamespace,
resolver,
alwaysTrueRefGrantResolver,
services,
refPath,
policies,
Expand Down Expand Up @@ -1265,3 +1240,42 @@ func TestFindBackendTLSPolicyForService(t *testing.T) {
})
}
}

func TestGetRefGrantFromResourceForRoute(t *testing.T) {
tests := []struct {
name string
routeType RouteType
ns string
expFromResource fromResource
}{
{
name: "HTTPRoute",
routeType: RouteTypeHTTP,
ns: "hr",
expFromResource: fromHTTPRoute("hr"),
},
{
name: "GRPCRoute",
routeType: RouteTypeGRPC,
ns: "gr",
expFromResource: fromGRPCRoute("gr"),
},
}

for _, test := range tests {
t.Run(test.name, func(t *testing.T) {
g := NewWithT(t)
g.Expect(getRefGrantFromResourceForRoute(test.routeType, test.ns)).To(Equal(test.expFromResource))
})
}
}

func TestGetRefGrantFromResourceForRoute_Panics(t *testing.T) {
g := NewWithT(t)

get := func() {
getRefGrantFromResourceForRoute("unknown", "ns")
}

g.Expect(get).To(Panic())
}
30 changes: 26 additions & 4 deletions internal/mode/static/state/graph/graph_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -306,9 +306,9 @@ func TestBuildGraph(t *testing.T) {
},
}

rgService := &v1beta1.ReferenceGrant{
hrToServiceNsRefGrant := &v1beta1.ReferenceGrant{
ObjectMeta: metav1.ObjectMeta{
Name: "rg-service",
Name: "hr-to-service",
Namespace: "service",
},
Spec: v1beta1.ReferenceGrantSpec{
Expand All @@ -327,6 +327,27 @@ func TestBuildGraph(t *testing.T) {
},
}

grToServiceNsRefGrant := &v1beta1.ReferenceGrant{
ObjectMeta: metav1.ObjectMeta{
Name: "gr-to-service",
Namespace: "service",
},
Spec: v1beta1.ReferenceGrantSpec{
From: []v1beta1.ReferenceGrantFrom{
{
Group: gatewayv1.GroupName,
Kind: kinds.GRPCRoute,
Namespace: gatewayv1.Namespace(testNs),
},
},
To: []v1beta1.ReferenceGrantTo{
{
Kind: "Service",
},
},
},
}

proxy := &ngfAPI.NginxProxy{
ObjectMeta: metav1.ObjectMeta{
Name: "nginx-proxy",
Expand Down Expand Up @@ -443,8 +464,9 @@ func TestBuildGraph(t *testing.T) {
client.ObjectKeyFromObject(ns): ns,
},
ReferenceGrants: map[types.NamespacedName]*v1beta1.ReferenceGrant{
client.ObjectKeyFromObject(rgSecret): rgSecret,
client.ObjectKeyFromObject(rgService): rgService,
client.ObjectKeyFromObject(rgSecret): rgSecret,
client.ObjectKeyFromObject(hrToServiceNsRefGrant): hrToServiceNsRefGrant,
client.ObjectKeyFromObject(grToServiceNsRefGrant): grToServiceNsRefGrant,
},
Secrets: map[types.NamespacedName]*v1.Secret{
client.ObjectKeyFromObject(secret): secret,
Expand Down
16 changes: 16 additions & 0 deletions internal/mode/static/state/graph/reference_grant.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -73,6 +73,14 @@ func fromHTTPRoute(namespace string) fromResource {
}
}

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

// newReferenceGrantResolver creates a new referenceGrantResolver.
func newReferenceGrantResolver(refGrants map[types.NamespacedName]*v1beta1.ReferenceGrant) *referenceGrantResolver {
allowed := make(map[allowedReference]struct{})
Expand Down Expand Up @@ -137,3 +145,11 @@ func (r *referenceGrantResolver) refAllowed(to toResource, from fromResource) bo

return false
}

// refAllowedFrom returns a closure function that takes a toResource parameter
// and checks if a reference from the specified fromResource to the given toResource is allowed.
func (r *referenceGrantResolver) refAllowedFrom(from fromResource) func(to toResource) bool {
return func(to toResource) bool {
return r.refAllowed(to, from)
}
}
Loading
Loading