Restrict policies to non-duplicate routes

internal/mode/static/handler.go

@@ -24,7 +24,6 @@ import (
 	ngxConfig "github.com/nginxinc/nginx-gateway-fabric/internal/mode/static/nginx/config"
 	"github.com/nginxinc/nginx-gateway-fabric/internal/mode/static/nginx/file"
 	"github.com/nginxinc/nginx-gateway-fabric/internal/mode/static/nginx/runtime"
-	"github.com/nginxinc/nginx-gateway-fabric/internal/mode/static/policies"
 	"github.com/nginxinc/nginx-gateway-fabric/internal/mode/static/state"
 	"github.com/nginxinc/nginx-gateway-fabric/internal/mode/static/state/dataplane"
 	"github.com/nginxinc/nginx-gateway-fabric/internal/mode/static/state/graph"
@@ -65,8 +64,6 @@ type eventHandlerConfig struct {
 	serviceResolver resolver.ServiceResolver
 	// generator is the nginx config generator.
 	generator ngxConfig.Generator
-	// policyConfigGenerator is the config generator for NGF policies.
-	policyConfigGenerator policies.ConfigGenerator
 	// k8sClient is a Kubernetes API client
 	k8sClient client.Client
 	// logLevelSetter is used to update the logging level.
@@ -196,7 +193,7 @@ func (h *eventHandlerImpl) HandleEventBatch(ctx context.Context, logger logr.Log
 		return
 	case state.EndpointsOnlyChange:
 		h.version++
-		cfg := dataplane.BuildConfiguration(ctx, graph, h.cfg.serviceResolver, h.cfg.policyConfigGenerator, h.version)
+		cfg := dataplane.BuildConfiguration(ctx, graph, h.cfg.serviceResolver, h.version)
 
 		h.setLatestConfiguration(&cfg)
 
@@ -207,7 +204,7 @@ func (h *eventHandlerImpl) HandleEventBatch(ctx context.Context, logger logr.Log
 		)
 	case state.ClusterStateChange:
 		h.version++
-		cfg := dataplane.BuildConfiguration(ctx, graph, h.cfg.serviceResolver, h.cfg.policyConfigGenerator, h.version)
+		cfg := dataplane.BuildConfiguration(ctx, graph, h.cfg.serviceResolver, h.version)
 
 		h.setLatestConfiguration(&cfg)
 

internal/mode/static/manager.go

@@ -49,12 +49,12 @@
 	"github.com/nginxinc/nginx-gateway-fabric/internal/mode/static/config"
 	"github.com/nginxinc/nginx-gateway-fabric/internal/mode/static/metrics/collectors"
 	ngxcfg "github.com/nginxinc/nginx-gateway-fabric/internal/mode/static/nginx/config"
+	"github.com/nginxinc/nginx-gateway-fabric/internal/mode/static/nginx/config/policies"
+	"github.com/nginxinc/nginx-gateway-fabric/internal/mode/static/nginx/config/policies/clientsettings"
+	"github.com/nginxinc/nginx-gateway-fabric/internal/mode/static/nginx/config/policies/observability"
 	ngxvalidation "github.com/nginxinc/nginx-gateway-fabric/internal/mode/static/nginx/config/validation"
 	"github.com/nginxinc/nginx-gateway-fabric/internal/mode/static/nginx/file"
 	ngxruntime "github.com/nginxinc/nginx-gateway-fabric/internal/mode/static/nginx/runtime"
-	"github.com/nginxinc/nginx-gateway-fabric/internal/mode/static/policies"
-	"github.com/nginxinc/nginx-gateway-fabric/internal/mode/static/policies/clientsettings"
-	"github.com/nginxinc/nginx-gateway-fabric/internal/mode/static/policies/observability"
 	"github.com/nginxinc/nginx-gateway-fabric/internal/mode/static/state"
 	"github.com/nginxinc/nginx-gateway-fabric/internal/mode/static/state/resolver"
 	"github.com/nginxinc/nginx-gateway-fabric/internal/mode/static/state/validation"
@@ -232,7 +232,6 @@
 		usageSecret:                   usageSecret,
 		gatewayCtlrName:               cfg.GatewayCtlrName,
 		updateGatewayClassStatus:      cfg.UpdateGatewayClassStatus,
-		policyConfigGenerator:         policyManager,
 	})
 
 	objects, objectLists := prepareFirstEventBatchPreparerArgs(
@@ -287,17 +286,15 @@
 func createPolicyManager(
 	mustExtractGVK kinds.MustExtractGVK,
 	validator validation.GenericValidator,
-) *policies.Manager {
+) *policies.CompositeValidator {
 	cfgs := []policies.ManagerConfig{
 		{
 			GVK:       mustExtractGVK(&ngfAPI.ClientSettingsPolicy{}),
 			Validator: clientsettings.NewValidator(validator),
-			Generator: clientsettings.Generate,
 		},
 		{
 			GVK:       mustExtractGVK(&ngfAPI.ObservabilityPolicy{}),
 			Validator: observability.NewValidator(validator),
-			Generator: observability.Generate,
 		},
 	}
 

internal/mode/static/nginx/config/generator.go

@@ -3,6 +3,9 @@ package config
 import (
 	"path/filepath"
 
+	"github.com/nginxinc/nginx-gateway-fabric/internal/mode/static/nginx/config/policies"
+	"github.com/nginxinc/nginx-gateway-fabric/internal/mode/static/nginx/config/policies/clientsettings"
+	"github.com/nginxinc/nginx-gateway-fabric/internal/mode/static/nginx/config/policies/observability"
 	"github.com/nginxinc/nginx-gateway-fabric/internal/mode/static/nginx/file"
 	"github.com/nginxinc/nginx-gateway-fabric/internal/mode/static/state/dataplane"
 )
@@ -85,7 +88,12 @@ func (g GeneratorImpl) Generate(conf dataplane.Configuration) []file.File {
 		files = append(files, generatePEM(id, pair.Cert, pair.Key))
 	}
 
-	files = append(files, g.generateHTTPConfig(conf)...)
+	policyGenerator := policies.NewCompositeGenerator(
+		clientsettings.NewGenerator(),
+		observability.NewGenerator(conf.Telemetry),
+	)
+
+	files = append(files, g.generateHTTPConfig(conf, policyGenerator)...)
 
 	files = append(files, generateConfigVersion(conf.Version))
 
@@ -127,20 +135,23 @@ func generateCertBundleFileName(id dataplane.CertBundleID) string {
 	return filepath.Join(secretsFolder, string(id)+".crt")
 }
 
-func (g GeneratorImpl) generateHTTPConfig(conf dataplane.Configuration) []file.File {
+func (g GeneratorImpl) generateHTTPConfig(
+	conf dataplane.Configuration,
+	generator policies.Generator,
+) []file.File {
 	fileBytes := make(map[string][]byte)
 
-	for _, execute := range g.getExecuteFuncs() {
+	for _, execute := range g.getExecuteFuncs(generator) {
 		results := execute(conf)
 		for _, res := range results {
 			fileBytes[res.dest] = append(fileBytes[res.dest], res.data...)
 		}
 	}
 
 	files := make([]file.File, 0, len(fileBytes))
-	for filepath, bytes := range fileBytes {
+	for fp, bytes := range fileBytes {
 		files = append(files, file.File{
-			Path:    filepath,
+			Path:    fp,
 			Content: bytes,
 			Type:    file.TypeRegular,
 		})
@@ -149,10 +160,10 @@ func (g GeneratorImpl) generateHTTPConfig(conf dataplane.Configuration) []file.F
 	return files
 }
 
-func (g GeneratorImpl) getExecuteFuncs() []executeFunc {
+func (g GeneratorImpl) getExecuteFuncs(generator policies.Generator) []executeFunc {
 	return []executeFunc{
 		executeBaseHTTPConfig,
-		executeServers,
+		newExecuteServersFunc(generator),
 		g.executeUpstreams,
 		executeSplitClients,
 		executeMaps,

internal/mode/static/nginx/config/http/config.go

@@ -1,11 +1,13 @@
 package http
 
+const InternalRoutePathPrefix = "/_ngf-internal"
+
 // Server holds all configuration for an HTTP server.
 type Server struct {
 	SSL           *SSL
 	ServerName    string
 	Locations     []Location
-	Includes      []string
+	Includes      []Include
 	Port          int32
 	IsDefaultHTTP bool
 	IsDefaultSSL  bool
@@ -18,18 +20,26 @@ type IPFamily struct {
 	IPv6 bool
 }
 
+type LocationType string
+
+const (
+	InternalLocationType LocationType = "internal"
+	ExternalLocationType LocationType = "external"
+	RedirectLocationType LocationType = "redirect"
+)
+
 // Location holds all configuration for an HTTP location.
 type Location struct {
 	Path            string
 	ProxyPass       string
 	HTTPMatchKey    string
-	HTTPMatchVar    string
+	Type            LocationType
 	ProxySetHeaders []Header
 	ProxySSLVerify  *ProxySSLVerify
 	Return          *Return
 	ResponseHeaders ResponseHeaders
 	Rewrites        []string
-	Includes        []string
+	Includes        []Include
 	GRPC            bool
 }
 
@@ -101,7 +111,7 @@ type Map struct {
 	Parameters []MapParameter
 }
 
-// Parameter defines a Value and Result pair in a Map.
+// MapParameter defines a Value and Result pair in a Map.
 type MapParameter struct {
 	Value  string
 	Result string
@@ -118,3 +128,9 @@ type ServerConfig struct {
 	Servers  []Server
 	IPFamily IPFamily
 }
+
+// Include defines a file that's included via the include directive.
+type Include struct {
+	Name    string
+	Content []byte
+}

internal/mode/static/nginx/config/maps_template.go

@@ -24,4 +24,9 @@ map $http_upgrade $connection_upgrade {
     default upgrade;
     '' close;
 }
+
+## Returns just the path from the original request URI.
+map $request_uri $request_uri_path {
+  "~^(?P<path>[^?]*)(\?.*)?$"  $path;
+}
 `

internal/mode/static/nginx/config/maps_test.go

@@ -86,6 +86,7 @@ func TestExecuteMaps(t *testing.T) {
 		"map ${http_my_set_header} $my_set_header_header_var {":               0,
 		"map $http_host $gw_api_compliant_host {":                             1,
 		"map $http_upgrade $connection_upgrade {":                             1,
+		"map $request_uri $request_uri_path {":                                1,
 	}
 
 	mapResult := executeMaps(conf)

internal/mode/static/nginx/config/policies/clientsettings/generator.go

@@ -0,0 +1,80 @@
+package clientsettings
+
+import (
+	"fmt"
+	"text/template"
+
+	ngfAPI "github.com/nginxinc/nginx-gateway-fabric/apis/v1alpha1"
+	"github.com/nginxinc/nginx-gateway-fabric/internal/framework/helpers"
+	"github.com/nginxinc/nginx-gateway-fabric/internal/mode/static/nginx/config/http"
+	"github.com/nginxinc/nginx-gateway-fabric/internal/mode/static/nginx/config/policies"
+)
+
+var tmpl = template.Must(template.New("client settings policy").Parse(clientSettingsTemplate))
+
+const clientSettingsTemplate = `
+{{- if .Body }}
+	{{- if .Body.MaxSize }}
+client_max_body_size {{ .Body.MaxSize }};
+	{{- end }}
+	{{- if .Body.Timeout }}
+client_body_timeout {{ .Body.Timeout }};
+	{{- end }}
+{{- end }}
+{{- if .KeepAlive }}
+	{{- if .KeepAlive.Requests }}
+keepalive_requests {{ .KeepAlive.Requests }};
+	{{- end }}
+	{{- if .KeepAlive.Time }}
+keepalive_time {{ .KeepAlive.Time }};
+	{{- end }}
+    {{- if .KeepAlive.Timeout }}
+        {{- if and .KeepAlive.Timeout.Server .KeepAlive.Timeout.Header }}
+keepalive_timeout {{ .KeepAlive.Timeout.Server }} {{ .KeepAlive.Timeout.Header }};
+        {{- else if .KeepAlive.Timeout.Server }}
+keepalive_timeout {{ .KeepAlive.Timeout.Server }};
+        {{- end }}
+    {{- end }}
+{{- end }}
+`
+
+// Generator generates nginx configuration based on a clientsettings policy.
+type Generator struct{}
+
+// NewGenerator returns a new instance of Generator.
+func NewGenerator() *Generator {
+	return &Generator{}
+}
+
+// GenerateForServer generates policy configuration for the server block.
+func (g Generator) GenerateForServer(pols []policies.Policy, _ http.Server) policies.GenerateResultFiles {
+	return generate(pols)
+}
+
+// GenerateForLocation generates policy configuration for a normal location block.
+func (g Generator) GenerateForLocation(pols []policies.Policy, _ http.Location) policies.GenerateResultFiles {
+	return generate(pols)
+}
+
+// GenerateForInternalLocation generates policy configuration for an internal location block.
+func (g Generator) GenerateForInternalLocation(pols []policies.Policy) policies.GenerateResultFiles {
+	return generate(pols)
+}
+
+func generate(pols []policies.Policy) policies.GenerateResultFiles {
+	files := make(policies.GenerateResultFiles, 0, len(pols))
+
+	for _, pol := range pols {
+		csp, ok := pol.(*ngfAPI.ClientSettingsPolicy)
+		if !ok {
+			continue
+		}
+
+		files = append(files, policies.File{
+			Name:    fmt.Sprintf("ClientSettingsPolicy_%s_%s.conf", csp.Namespace, csp.Name),
+			Content: helpers.MustExecuteTemplate(tmpl, csp.Spec),
+		})
+	}
+
+	return files
+}

internal/mode/static/policies/clientsettings/generator_test.go → internal/mode/static/nginx/config/policies/clientsettings/generator_test.go

@@ -7,9 +7,9 @@ import (
 
 	ngfAPI "github.com/nginxinc/nginx-gateway-fabric/apis/v1alpha1"
 	"github.com/nginxinc/nginx-gateway-fabric/internal/framework/helpers"
-	"github.com/nginxinc/nginx-gateway-fabric/internal/mode/static/policies"
-	"github.com/nginxinc/nginx-gateway-fabric/internal/mode/static/policies/clientsettings"
-	"github.com/nginxinc/nginx-gateway-fabric/internal/mode/static/policies/policiesfakes"
+	"github.com/nginxinc/nginx-gateway-fabric/internal/mode/static/nginx/config/http"
+	"github.com/nginxinc/nginx-gateway-fabric/internal/mode/static/nginx/config/policies"
+	"github.com/nginxinc/nginx-gateway-fabric/internal/mode/static/nginx/config/policies/clientsettings"
 )
 
 func TestGenerate(t *testing.T) {
@@ -149,25 +149,52 @@ func TestGenerate(t *testing.T) {
 		},
 	}
 
+	g := NewWithT(t)
+
+	checkResults := func(resFiles policies.GenerateResultFiles, expStrings []string) {
+		g.Expect(resFiles).To(HaveLen(1))
+
+		for _, str := range expStrings {
+			g.Expect(string(resFiles[0].Content)).To(ContainSubstring(str))
+		}
+	}
+
 	for _, test := range tests {
-		t.Run(test.name, func(t *testing.T) {
-			g := NewWithT(t)
+		t.Run(test.name, func(_ *testing.T) {
+			generator := clientsettings.NewGenerator()
+
+			resFiles := generator.GenerateForServer([]policies.Policy{test.policy}, http.Server{})
+			checkResults(resFiles, test.expStrings)
 
-			cfgString := string(clientsettings.Generate(test.policy, nil))
+			resFiles = generator.GenerateForLocation([]policies.Policy{test.policy}, http.Location{})
+			checkResults(resFiles, test.expStrings)
 
-			for _, str := range test.expStrings {
-				g.Expect(cfgString).To(ContainSubstring(str))
-			}
+			resFiles = generator.GenerateForInternalLocation([]policies.Policy{test.policy})
+			checkResults(resFiles, test.expStrings)
 		})
 	}
 }
 
-func TestGeneratePanics(t *testing.T) {
+func TestGenerateNoPolicies(t *testing.T) {
 	g := NewWithT(t)
 
-	generate := func() {
-		clientsettings.Generate(&policiesfakes.FakePolicy{}, nil)
-	}
+	generator := clientsettings.NewGenerator()
+
+	resFiles := generator.GenerateForServer([]policies.Policy{}, http.Server{})
+	g.Expect(resFiles).To(BeEmpty())
+
+	resFiles = generator.GenerateForServer([]policies.Policy{&ngfAPI.ObservabilityPolicy{}}, http.Server{})
+	g.Expect(resFiles).To(BeEmpty())
+
+	resFiles = generator.GenerateForLocation([]policies.Policy{}, http.Location{})
+	g.Expect(resFiles).To(BeEmpty())
+
+	resFiles = generator.GenerateForLocation([]policies.Policy{&ngfAPI.ObservabilityPolicy{}}, http.Location{})
+	g.Expect(resFiles).To(BeEmpty())
+
+	resFiles = generator.GenerateForInternalLocation([]policies.Policy{})
+	g.Expect(resFiles).To(BeEmpty())
 
-	g.Expect(generate).To(Panic())
+	resFiles = generator.GenerateForInternalLocation([]policies.Policy{&ngfAPI.ObservabilityPolicy{}})
+	g.Expect(resFiles).To(BeEmpty())
 }

internal/mode/static/policies/clientsettings/validator.go → internal/mode/static/nginx/config/policies/clientsettings/validator.go

@@ -8,7 +8,7 @@ import (
 	"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"
-	"github.com/nginxinc/nginx-gateway-fabric/internal/mode/static/policies"
+	"github.com/nginxinc/nginx-gateway-fabric/internal/mode/static/nginx/config/policies"
 	staticConds "github.com/nginxinc/nginx-gateway-fabric/internal/mode/static/state/conditions"
 	"github.com/nginxinc/nginx-gateway-fabric/internal/mode/static/state/validation"
 )