diff --git a/internal/controller/state/dataplane/configuration.go b/internal/controller/state/dataplane/configuration.go index 6bded6cebb..6943c4a2ad 100644 --- a/internal/controller/state/dataplane/configuration.go +++ b/internal/controller/state/dataplane/configuration.go @@ -44,7 +44,10 @@ func BuildConfiguration( return config } - baseHTTPConfig := buildBaseHTTPConfig(g, gateway) + // Get SnippetsFilters that are specifically referenced by routes attached to this gateway + gatewaySnippetsFilters := gateway.GetReferencedSnippetsFilters(g.Routes, g.SnippetsFilters) + + baseHTTPConfig := buildBaseHTTPConfig(gateway, gatewaySnippetsFilters) httpServers, sslServers := buildServers(gateway) backendGroups := buildBackendGroups(append(httpServers, sslServers...)) @@ -77,7 +80,7 @@ func BuildConfiguration( BaseHTTPConfig: baseHTTPConfig, Logging: buildLogging(gateway), NginxPlus: nginxPlus, - MainSnippets: buildSnippetsForContext(g.SnippetsFilters, ngfAPIv1alpha1.NginxContextMain), + MainSnippets: buildSnippetsForContext(gatewaySnippetsFilters, ngfAPIv1alpha1.NginxContextMain), AuxiliarySecrets: buildAuxiliarySecrets(g.PlusSecrets), } @@ -963,12 +966,15 @@ func CreateRatioVarName(ratio int32) string { } // buildBaseHTTPConfig generates the base http context config that should be applied to all servers. -func buildBaseHTTPConfig(g *graph.Graph, gateway *graph.Gateway) BaseHTTPConfig { +func buildBaseHTTPConfig( + gateway *graph.Gateway, + gatewaySnippetsFilters map[types.NamespacedName]*graph.SnippetsFilter, +) BaseHTTPConfig { baseConfig := BaseHTTPConfig{ // HTTP2 should be enabled by default HTTP2: true, IPFamily: Dual, - Snippets: buildSnippetsForContext(g.SnippetsFilters, ngfAPIv1alpha1.NginxContextHTTP), + Snippets: buildSnippetsForContext(gatewaySnippetsFilters, ngfAPIv1alpha1.NginxContextHTTP), } // safe to access EffectiveNginxProxy since we only call this function when the Gateway is not nil. diff --git a/internal/controller/state/dataplane/configuration_test.go b/internal/controller/state/dataplane/configuration_test.go index 95a4806e5a..12fc3d7eb3 100644 --- a/internal/controller/state/dataplane/configuration_test.go +++ b/internal/controller/state/dataplane/configuration_test.go @@ -2462,31 +2462,17 @@ func TestBuildConfiguration(t *testing.T) { return g }), expConf: getModifiedExpectedConfiguration(func(conf Configuration) Configuration { - conf.MainSnippets = []Snippet{ - { - Name: createSnippetName( - ngfAPIv1alpha1.NginxContextMain, - client.ObjectKeyFromObject(sf1.Source), - ), - Contents: "main snippet", - }, - } - conf.BaseHTTPConfig.Snippets = []Snippet{ - { - Name: createSnippetName( - ngfAPIv1alpha1.NginxContextHTTP, - client.ObjectKeyFromObject(sf1.Source), - ), - Contents: "http snippet", - }, - } + // With proper scoping, no snippets should be included since no routes + // attached to this gateway reference the SnippetsFilters + conf.MainSnippets = nil // nil - no snippets should be included + conf.BaseHTTPConfig.Snippets = nil // nil - no snippets should be included conf.HTTPServers = []VirtualServer{} conf.SSLServers = []VirtualServer{} conf.SSLKeyPairs = map[SSLKeyPairID]SSLKeyPair{} return conf }), - msg: "SnippetsFilters with main and http snippet", + msg: "SnippetsFilters scoped per gateway - no routes reference SnippetsFilters", }, { graph: getModifiedGraph(func(g *graph.Graph) *graph.Graph { @@ -4488,7 +4474,10 @@ func TestBuildRewriteIPSettings(t *testing.T) { t.Run(tc.msg, func(t *testing.T) { t.Parallel() g := NewWithT(t) - baseConfig := buildBaseHTTPConfig(tc.g, tc.g.Gateways[types.NamespacedName{}]) + baseConfig := buildBaseHTTPConfig( + tc.g.Gateways[types.NamespacedName{}], + make(map[types.NamespacedName]*graph.SnippetsFilter), + ) g.Expect(baseConfig.RewriteClientIPSettings).To(Equal(tc.expRewriteIPSettings)) }) } diff --git a/internal/controller/state/graph/gateway.go b/internal/controller/state/graph/gateway.go index ba174db032..e7129da55c 100644 --- a/internal/controller/state/graph/gateway.go +++ b/internal/controller/state/graph/gateway.go @@ -3,6 +3,7 @@ package graph import ( "k8s.io/apimachinery/pkg/types" "k8s.io/apimachinery/pkg/util/validation/field" + "sigs.k8s.io/controller-runtime/pkg/client" v1 "sigs.k8s.io/gateway-api/apis/v1" "github.com/nginx/nginx-gateway-fabric/internal/controller/config" @@ -196,3 +197,70 @@ func validateGateway(gw *v1.Gateway, gc *GatewayClass, npCfg *NginxProxy) ([]con return conds, valid } + +// GetReferencedSnippetsFilters returns all SnippetsFilters that are referenced by routes attached to this Gateway. +func (g *Gateway) GetReferencedSnippetsFilters( + routes map[RouteKey]*L7Route, + allSnippetsFilters map[types.NamespacedName]*SnippetsFilter, +) map[types.NamespacedName]*SnippetsFilter { + if len(routes) == 0 || len(allSnippetsFilters) == 0 { + return nil + } + + gatewayNsName := client.ObjectKeyFromObject(g.Source) + referencedSnippetsFilters := make(map[types.NamespacedName]*SnippetsFilter) + + for _, route := range routes { + if !route.Valid || !g.isRouteAttachedToGateway(route, gatewayNsName) { + continue + } + + g.collectSnippetsFiltersFromRoute(route, allSnippetsFilters, referencedSnippetsFilters) + } + + if len(referencedSnippetsFilters) == 0 { + return nil + } + + return referencedSnippetsFilters +} + +// isRouteAttachedToGateway checks if the given route is attached to this gateway. +func (g *Gateway) isRouteAttachedToGateway(route *L7Route, gatewayNsName types.NamespacedName) bool { + for _, parentRef := range route.ParentRefs { + if parentRef.Gateway != nil && parentRef.Gateway.NamespacedName == gatewayNsName { + return true + } + } + return false +} + +// collectSnippetsFiltersFromRoute extracts SnippetsFilters from a single route's rules. +func (g *Gateway) collectSnippetsFiltersFromRoute( + route *L7Route, + allSnippetsFilters map[types.NamespacedName]*SnippetsFilter, + referencedFilters map[types.NamespacedName]*SnippetsFilter, +) { + for _, rule := range route.Spec.Rules { + if !rule.Filters.Valid { + continue + } + + for _, filter := range rule.Filters.Filters { + if filter.FilterType != FilterExtensionRef || + filter.ResolvedExtensionRef == nil || + filter.ResolvedExtensionRef.SnippetsFilter == nil { + continue + } + + sf := filter.ResolvedExtensionRef.SnippetsFilter + nsName := client.ObjectKeyFromObject(sf.Source) + + // Only include if it exists in the cluster-wide map and is valid + // Using the cluster-wide version ensures consistency and avoids duplicates + if clusterSF, exists := allSnippetsFilters[nsName]; exists && clusterSF.Valid { + referencedFilters[nsName] = clusterSF + } + } + } +} diff --git a/internal/controller/state/graph/gateway_test.go b/internal/controller/state/graph/gateway_test.go index 26dbd375ed..f37349bee1 100644 --- a/internal/controller/state/graph/gateway_test.go +++ b/internal/controller/state/graph/gateway_test.go @@ -13,6 +13,7 @@ import ( v1 "sigs.k8s.io/gateway-api/apis/v1" "sigs.k8s.io/gateway-api/apis/v1beta1" + ngfAPIv1alpha1 "github.com/nginx/nginx-gateway-fabric/apis/v1alpha1" ngfAPIv1alpha2 "github.com/nginx/nginx-gateway-fabric/apis/v1alpha2" "github.com/nginx/nginx-gateway-fabric/internal/controller/state/conditions" "github.com/nginx/nginx-gateway-fabric/internal/framework/controller" @@ -1582,3 +1583,200 @@ func TestValidateGatewayParametersRef(t *testing.T) { }) } } + +func TestGetReferencedSnippetsFilters(t *testing.T) { + t.Parallel() + + gw := &Gateway{ + Source: &v1.Gateway{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: "gateway-ns", + Name: "test-gateway", + }, + }, + } + + sf1 := &SnippetsFilter{ + Source: &ngfAPIv1alpha1.SnippetsFilter{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: "app1", + Name: "app1-logging", + }, + }, + Valid: true, + } + + sf2 := &SnippetsFilter{ + Source: &ngfAPIv1alpha1.SnippetsFilter{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: "app2", + Name: "app2-logging", + }, + }, + Valid: true, + } + + sf3Invalid := &SnippetsFilter{ + Source: &ngfAPIv1alpha1.SnippetsFilter{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: "app3", + Name: "invalid-filter", + }, + }, + Valid: false, + } + + routeAttachedToGateway := &L7Route{ + Source: &v1.HTTPRoute{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: "app1", + Name: "attached-route", + }, + }, + Valid: true, + ParentRefs: []ParentRef{ + { + Gateway: &ParentRefGateway{ + NamespacedName: types.NamespacedName{ + Namespace: "gateway-ns", + Name: "test-gateway", + }, + }, + }, + }, + Spec: L7RouteSpec{ + Rules: []RouteRule{ + { + Filters: RouteRuleFilters{ + Valid: true, + Filters: []Filter{ + { + FilterType: FilterExtensionRef, + ResolvedExtensionRef: &ExtensionRefFilter{ + SnippetsFilter: sf1, + Valid: true, + }, + }, + }, + }, + }, + }, + }, + } + + routeNotAttachedToGateway := &L7Route{ + Source: &v1.HTTPRoute{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: "app2", + Name: "not-attached-route", + }, + }, + Valid: true, + ParentRefs: []ParentRef{ + { + Gateway: &ParentRefGateway{ + NamespacedName: types.NamespacedName{ + Namespace: "other-gateway-ns", + Name: "other-gateway", + }, + }, + }, + }, + Spec: L7RouteSpec{ + Rules: []RouteRule{ + { + Filters: RouteRuleFilters{ + Valid: true, + Filters: []Filter{ + { + FilterType: FilterExtensionRef, + ResolvedExtensionRef: &ExtensionRefFilter{ + SnippetsFilter: sf2, + Valid: true, + }, + }, + }, + }, + }, + }, + }, + } + + routeWithInvalidFilter := &L7Route{ + Source: &v1.HTTPRoute{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: "app3", + Name: "route-with-invalid-filter", + }, + }, + Valid: true, + ParentRefs: []ParentRef{ + { + Gateway: &ParentRefGateway{ + NamespacedName: types.NamespacedName{ + Namespace: "gateway-ns", + Name: "test-gateway", + }, + }, + }, + }, + Spec: L7RouteSpec{ + Rules: []RouteRule{ + { + Filters: RouteRuleFilters{ + Valid: true, + Filters: []Filter{ + { + FilterType: FilterExtensionRef, + ResolvedExtensionRef: &ExtensionRefFilter{ + SnippetsFilter: sf3Invalid, + Valid: false, + }, + }, + }, + }, + }, + }, + }, + } + + routes := map[RouteKey]*L7Route{ + { + NamespacedName: types.NamespacedName{Namespace: "app1", Name: "attached-route"}, + RouteType: RouteTypeHTTP, + }: routeAttachedToGateway, + { + NamespacedName: types.NamespacedName{Namespace: "app2", Name: "not-attached-route"}, + RouteType: RouteTypeHTTP, + }: routeNotAttachedToGateway, + { + NamespacedName: types.NamespacedName{Namespace: "app3", Name: "route-with-invalid-filter"}, + RouteType: RouteTypeHTTP, + }: routeWithInvalidFilter, + } + + allSnippetsFilters := map[types.NamespacedName]*SnippetsFilter{ + {Namespace: "app1", Name: "app1-logging"}: sf1, + {Namespace: "app2", Name: "app2-logging"}: sf2, + {Namespace: "app3", Name: "invalid-filter"}: sf3Invalid, + } + + g := NewWithT(t) + + result := gw.GetReferencedSnippetsFilters(routes, allSnippetsFilters) + + // Should only include sf1 (valid filter from route attached to this gateway) + expectedResult := map[types.NamespacedName]*SnippetsFilter{ + {Namespace: "app1", Name: "app1-logging"}: sf1, + } + + g.Expect(result).To(Equal(expectedResult)) + + // Test with no routes + emptyResult := gw.GetReferencedSnippetsFilters(map[RouteKey]*L7Route{}, allSnippetsFilters) + g.Expect(emptyResult).To(BeEmpty()) + + // Test with routes but no snippets filters + emptyFilterResult := gw.GetReferencedSnippetsFilters(routes, map[types.NamespacedName]*SnippetsFilter{}) + g.Expect(emptyFilterResult).To(BeEmpty()) +}