diff --git a/internal/mode/static/state/graph/backend_refs.go b/internal/mode/static/state/graph/backend_refs.go index 2b79569dc..b90fa2ebc 100644 --- a/internal/mode/static/state/graph/backend_refs.go +++ b/internal/mode/static/state/graph/backend_refs.go @@ -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, @@ -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, @@ -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 @@ -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 @@ -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) @@ -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)) + } +} diff --git a/internal/mode/static/state/graph/backend_refs_test.go b/internal/mode/static/state/graph/backend_refs_test.go index c260408c4..9f32b552e 100644 --- a/internal/mode/static/state/graph/backend_refs_test.go +++ b/internal/mode/static/state/graph/backend_refs_test.go @@ -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" ) @@ -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)) @@ -99,38 +97,21 @@ 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", @@ -138,7 +119,8 @@ func TestValidateBackendRef(t *testing.T) { backend.Namespace = nil return backend }), - expectedValid: true, + refGrantResolver: alwaysTrueRefGrantResolver, + expectedValid: true, }, { name: "normal case with implicit kind Service", @@ -146,29 +128,17 @@ func TestValidateBackendRef(t *testing.T) { 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", @@ -176,7 +146,8 @@ func TestValidateBackendRef(t *testing.T) { 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", ""`, ), @@ -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"`, ), @@ -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", ), @@ -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]", ), @@ -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", ), @@ -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)) @@ -437,6 +411,7 @@ func TestAddBackendRefsToRulesTest(t *testing.T) { Name: name, }, }, + RouteType: RouteTypeHTTP, ParentRefs: sectionNameRefs, Valid: true, } @@ -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, @@ -1043,7 +1018,7 @@ func TestCreateBackend(t *testing.T) { backend, cond := createBackendRef( rbr, sourceNamespace, - resolver, + alwaysTrueRefGrantResolver, services, refPath, policies, @@ -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()) +} diff --git a/internal/mode/static/state/graph/graph_test.go b/internal/mode/static/state/graph/graph_test.go index 15d1afde1..ea8de6c55 100644 --- a/internal/mode/static/state/graph/graph_test.go +++ b/internal/mode/static/state/graph/graph_test.go @@ -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{ @@ -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", @@ -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, diff --git a/internal/mode/static/state/graph/reference_grant.go b/internal/mode/static/state/graph/reference_grant.go index de04211ba..bdda74aae 100644 --- a/internal/mode/static/state/graph/reference_grant.go +++ b/internal/mode/static/state/graph/reference_grant.go @@ -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{}) @@ -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) + } +} diff --git a/internal/mode/static/state/graph/reference_grant_test.go b/internal/mode/static/state/graph/reference_grant_test.go index 3b61e6f1c..455149989 100644 --- a/internal/mode/static/state/graph/reference_grant_test.go +++ b/internal/mode/static/state/graph/reference_grant_test.go @@ -219,3 +219,140 @@ func TestFromHTTPRoute(t *testing.T) { g := NewWithT(t) g.Expect(ref).To(Equal(exp)) } + +func TestFromGRPCRoute(t *testing.T) { + ref := fromGRPCRoute("ns") + + exp := fromResource{ + group: v1beta1.GroupName, + kind: kinds.GRPCRoute, + namespace: "ns", + } + + g := NewWithT(t) + g.Expect(ref).To(Equal(exp)) +} + +func TestRefAllowedFrom(t *testing.T) { + gwNs := "gw-ns" + hrNs := "hr-ns" + grNs := "gr-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"} + + allowedGatewayNs := "gw-allowed-ns" + allowedGatewayNsName := types.NamespacedName{Namespace: allowedGatewayNs, Name: "all-allowed-in-ns"} + + notAllowedNsName := types.NamespacedName{Namespace: "not-allowed-ns", Name: "not-allowed-in-ns"} + + refGrants := map[types.NamespacedName]*v1beta1.ReferenceGrant{ + {Namespace: allowedGatewayNs, Name: "gw-2-secret"}: { + Spec: v1beta1.ReferenceGrantSpec{ + From: []v1beta1.ReferenceGrantFrom{ + { + Group: v1beta1.GroupName, + Kind: kinds.Gateway, + Namespace: v1beta1.Namespace(gwNs), + }, + }, + To: []v1beta1.ReferenceGrantTo{ + { + Kind: "Secret", + }, + }, + }, + }, + {Namespace: allowedHTTPRouteNs, Name: "hr-2-svc"}: { + Spec: v1beta1.ReferenceGrantSpec{ + From: []v1beta1.ReferenceGrantFrom{ + { + Group: v1beta1.GroupName, + Kind: kinds.HTTPRoute, + Namespace: v1beta1.Namespace(hrNs), + }, + }, + To: []v1beta1.ReferenceGrantTo{ + { + Kind: "Service", + }, + }, + }, + }, + {Namespace: allowedGRPCRouteNs, Name: "gr-2-svc"}: { + Spec: v1beta1.ReferenceGrantSpec{ + From: []v1beta1.ReferenceGrantFrom{ + { + Group: v1beta1.GroupName, + Kind: kinds.GRPCRoute, + Namespace: v1beta1.Namespace(grNs), + }, + }, + To: []v1beta1.ReferenceGrantTo{ + { + Kind: "Service", + }, + }, + }, + }, + } + + resolver := newReferenceGrantResolver(refGrants) + refAllowedFromGRPCRoute := resolver.refAllowedFrom(fromGRPCRoute(grNs)) + refAllowedFromHTTPRoute := resolver.refAllowedFrom(fromHTTPRoute(hrNs)) + refAllowedFromGateway := resolver.refAllowedFrom(fromGateway(gwNs)) + + tests := []struct { + name string + refAllowedFrom func(resource toResource) bool + toResource toResource + expAllowed bool + }{ + { + name: "ref allowed from gateway to secret", + refAllowedFrom: refAllowedFromGateway, + toResource: toSecret(allowedGatewayNsName), + expAllowed: true, + }, + { + name: "ref not allowed from gateway to secret", + refAllowedFrom: refAllowedFromGateway, + toResource: toSecret(notAllowedNsName), + expAllowed: false, + }, + { + name: "ref allowed from httproute to service", + refAllowedFrom: refAllowedFromHTTPRoute, + toResource: toService(allowedHTTPRouteNsName), + expAllowed: true, + }, + { + name: "ref not allowed from httproute to service", + refAllowedFrom: refAllowedFromHTTPRoute, + toResource: toService(notAllowedNsName), + expAllowed: false, + }, + { + name: "ref allowed from grpcroute to service", + refAllowedFrom: refAllowedFromGRPCRoute, + toResource: toService(allowedGRPCRouteNsName), + expAllowed: true, + }, + { + name: "ref not allowed from grpcroute to service", + refAllowedFrom: refAllowedFromGRPCRoute, + toResource: toService(notAllowedNsName), + expAllowed: false, + }, + } + + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + g := NewWithT(t) + g.Expect(test.refAllowedFrom(test.toResource)).To(Equal(test.expAllowed)) + }) + } +}