Only include relevant snippets (#3586)

Problem: SnippetsFilters are being included in NGINX configuration for all gateways regardless of whether routes attached to those specific gateways actually referenced the filters.

Solution: Implemented gateway-specific scoping by adding GetReferencedSnippetsFilters() method that returns only SnippetsFilters referenced by routes attached to the current gateway.

Author: ciarams87

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.

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))
 		})
 	}

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
+			}
+		}
+	}
+}

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())
+}