From 200fa1fe914de18cc714a11e17feec1c21e215fb Mon Sep 17 00:00:00 2001 From: Ciara Stacke <18287516+ciarams87@users.noreply.github.com> Date: Thu, 13 Nov 2025 20:32:19 +0000 Subject: [PATCH 1/3] Implement ProxySettingsPolicy --- .yamllint.yaml | 1 + .../templates/clusterrole.yaml | 2 + deploy/azure/deploy.yaml | 2 + deploy/default/deploy.yaml | 2 + deploy/experimental-nginx-plus/deploy.yaml | 2 + deploy/experimental/deploy.yaml | 2 + deploy/inference-nginx-plus/deploy.yaml | 2 + deploy/inference/deploy.yaml | 2 + deploy/nginx-plus/deploy.yaml | 2 + deploy/nodeport/deploy.yaml | 2 + deploy/openshift/deploy.yaml | 2 + .../snippets-filters-nginx-plus/deploy.yaml | 2 + deploy/snippets-filters/deploy.yaml | 2 + examples/proxy-settings-policy/README.md | 3 + examples/proxy-settings-policy/app.yaml | 154 +++++++++ .../coffee-proxy-settings.yaml | 19 ++ .../gateway-proxy-settings.yaml | 15 + examples/proxy-settings-policy/gateway.yaml | 11 + .../proxy-settings-policy/httproutes.yaml | 37 +++ internal/controller/manager.go | 12 + internal/controller/manager_test.go | 5 + .../nginx/config/base_http_config.go | 18 +- .../nginx/config/base_http_config_test.go | 71 +++- internal/controller/nginx/config/generator.go | 4 +- .../policies/clientsettings/generator.go | 4 +- .../nginx/config/policies/generator.go | 17 + .../policies/policiesfakes/fake_generator.go | 77 +++++ .../policies/proxysettings/generator.go | 108 ++++++ .../policies/proxysettings/generator_test.go | 166 ++++++++++ .../policies/proxysettings/validator.go | 204 ++++++++++++ .../policies/proxysettings/validator_test.go | 312 ++++++++++++++++++ internal/controller/state/change_processor.go | 5 + .../controller/state/change_processor_test.go | 53 ++- .../state/dataplane/configuration.go | 1 + .../state/dataplane/configuration_test.go | 2 + internal/controller/state/dataplane/types.go | 2 + internal/framework/kinds/kinds.go | 2 + 37 files changed, 1310 insertions(+), 17 deletions(-) create mode 100644 examples/proxy-settings-policy/README.md create mode 100644 examples/proxy-settings-policy/app.yaml create mode 100644 examples/proxy-settings-policy/coffee-proxy-settings.yaml create mode 100644 examples/proxy-settings-policy/gateway-proxy-settings.yaml create mode 100644 examples/proxy-settings-policy/gateway.yaml create mode 100644 examples/proxy-settings-policy/httproutes.yaml create mode 100644 internal/controller/nginx/config/policies/proxysettings/generator.go create mode 100644 internal/controller/nginx/config/policies/proxysettings/generator_test.go create mode 100644 internal/controller/nginx/config/policies/proxysettings/validator.go create mode 100644 internal/controller/nginx/config/policies/proxysettings/validator_test.go diff --git a/.yamllint.yaml b/.yamllint.yaml index 6b5af8a6ec..e195fa7831 100644 --- a/.yamllint.yaml +++ b/.yamllint.yaml @@ -29,6 +29,7 @@ rules: check-multi-line-strings: true ignore: | operators/**/* + examples/proxy-settings-policy/app.yaml key-duplicates: enable key-ordering: disable line-length: diff --git a/charts/nginx-gateway-fabric/templates/clusterrole.yaml b/charts/nginx-gateway-fabric/templates/clusterrole.yaml index 5d9ea4a5be..97cb309ad1 100644 --- a/charts/nginx-gateway-fabric/templates/clusterrole.yaml +++ b/charts/nginx-gateway-fabric/templates/clusterrole.yaml @@ -129,6 +129,7 @@ rules: - clientsettingspolicies - observabilitypolicies - upstreamsettingspolicies + - proxysettingspolicies {{- if .Values.nginxGateway.snippetsFilters.enable }} - snippetsfilters {{- end }} @@ -142,6 +143,7 @@ rules: - clientsettingspolicies/status - observabilitypolicies/status - upstreamsettingspolicies/status + - proxysettingspolicies/status {{- if .Values.nginxGateway.snippetsFilters.enable }} - snippetsfilters/status {{- end }} diff --git a/deploy/azure/deploy.yaml b/deploy/azure/deploy.yaml index ed1699b656..a92adddd99 100644 --- a/deploy/azure/deploy.yaml +++ b/deploy/azure/deploy.yaml @@ -169,6 +169,7 @@ rules: - clientsettingspolicies - observabilitypolicies - upstreamsettingspolicies + - proxysettingspolicies verbs: - list - watch @@ -179,6 +180,7 @@ rules: - clientsettingspolicies/status - observabilitypolicies/status - upstreamsettingspolicies/status + - proxysettingspolicies/status verbs: - update - apiGroups: diff --git a/deploy/default/deploy.yaml b/deploy/default/deploy.yaml index 2651f0bc3c..f849a66918 100644 --- a/deploy/default/deploy.yaml +++ b/deploy/default/deploy.yaml @@ -169,6 +169,7 @@ rules: - clientsettingspolicies - observabilitypolicies - upstreamsettingspolicies + - proxysettingspolicies verbs: - list - watch @@ -179,6 +180,7 @@ rules: - clientsettingspolicies/status - observabilitypolicies/status - upstreamsettingspolicies/status + - proxysettingspolicies/status verbs: - update - apiGroups: diff --git a/deploy/experimental-nginx-plus/deploy.yaml b/deploy/experimental-nginx-plus/deploy.yaml index 4ea4257289..63bd3a575a 100644 --- a/deploy/experimental-nginx-plus/deploy.yaml +++ b/deploy/experimental-nginx-plus/deploy.yaml @@ -171,6 +171,7 @@ rules: - clientsettingspolicies - observabilitypolicies - upstreamsettingspolicies + - proxysettingspolicies verbs: - list - watch @@ -181,6 +182,7 @@ rules: - clientsettingspolicies/status - observabilitypolicies/status - upstreamsettingspolicies/status + - proxysettingspolicies/status verbs: - update - apiGroups: diff --git a/deploy/experimental/deploy.yaml b/deploy/experimental/deploy.yaml index 6b9ff2594b..a7fbb94351 100644 --- a/deploy/experimental/deploy.yaml +++ b/deploy/experimental/deploy.yaml @@ -171,6 +171,7 @@ rules: - clientsettingspolicies - observabilitypolicies - upstreamsettingspolicies + - proxysettingspolicies verbs: - list - watch @@ -181,6 +182,7 @@ rules: - clientsettingspolicies/status - observabilitypolicies/status - upstreamsettingspolicies/status + - proxysettingspolicies/status verbs: - update - apiGroups: diff --git a/deploy/inference-nginx-plus/deploy.yaml b/deploy/inference-nginx-plus/deploy.yaml index f2be0e2902..aa64974440 100644 --- a/deploy/inference-nginx-plus/deploy.yaml +++ b/deploy/inference-nginx-plus/deploy.yaml @@ -169,6 +169,7 @@ rules: - clientsettingspolicies - observabilitypolicies - upstreamsettingspolicies + - proxysettingspolicies verbs: - list - watch @@ -179,6 +180,7 @@ rules: - clientsettingspolicies/status - observabilitypolicies/status - upstreamsettingspolicies/status + - proxysettingspolicies/status verbs: - update - apiGroups: diff --git a/deploy/inference/deploy.yaml b/deploy/inference/deploy.yaml index 129573c45c..080a031708 100644 --- a/deploy/inference/deploy.yaml +++ b/deploy/inference/deploy.yaml @@ -169,6 +169,7 @@ rules: - clientsettingspolicies - observabilitypolicies - upstreamsettingspolicies + - proxysettingspolicies verbs: - list - watch @@ -179,6 +180,7 @@ rules: - clientsettingspolicies/status - observabilitypolicies/status - upstreamsettingspolicies/status + - proxysettingspolicies/status verbs: - update - apiGroups: diff --git a/deploy/nginx-plus/deploy.yaml b/deploy/nginx-plus/deploy.yaml index 6610a43166..a6761376a0 100644 --- a/deploy/nginx-plus/deploy.yaml +++ b/deploy/nginx-plus/deploy.yaml @@ -169,6 +169,7 @@ rules: - clientsettingspolicies - observabilitypolicies - upstreamsettingspolicies + - proxysettingspolicies verbs: - list - watch @@ -179,6 +180,7 @@ rules: - clientsettingspolicies/status - observabilitypolicies/status - upstreamsettingspolicies/status + - proxysettingspolicies/status verbs: - update - apiGroups: diff --git a/deploy/nodeport/deploy.yaml b/deploy/nodeport/deploy.yaml index f2538143ed..e8ba0e2bdf 100644 --- a/deploy/nodeport/deploy.yaml +++ b/deploy/nodeport/deploy.yaml @@ -169,6 +169,7 @@ rules: - clientsettingspolicies - observabilitypolicies - upstreamsettingspolicies + - proxysettingspolicies verbs: - list - watch @@ -179,6 +180,7 @@ rules: - clientsettingspolicies/status - observabilitypolicies/status - upstreamsettingspolicies/status + - proxysettingspolicies/status verbs: - update - apiGroups: diff --git a/deploy/openshift/deploy.yaml b/deploy/openshift/deploy.yaml index 09b25596fc..a7a77f4815 100644 --- a/deploy/openshift/deploy.yaml +++ b/deploy/openshift/deploy.yaml @@ -169,6 +169,7 @@ rules: - clientsettingspolicies - observabilitypolicies - upstreamsettingspolicies + - proxysettingspolicies verbs: - list - watch @@ -179,6 +180,7 @@ rules: - clientsettingspolicies/status - observabilitypolicies/status - upstreamsettingspolicies/status + - proxysettingspolicies/status verbs: - update - apiGroups: diff --git a/deploy/snippets-filters-nginx-plus/deploy.yaml b/deploy/snippets-filters-nginx-plus/deploy.yaml index b62bbb40df..cb748334a3 100644 --- a/deploy/snippets-filters-nginx-plus/deploy.yaml +++ b/deploy/snippets-filters-nginx-plus/deploy.yaml @@ -169,6 +169,7 @@ rules: - clientsettingspolicies - observabilitypolicies - upstreamsettingspolicies + - proxysettingspolicies - snippetsfilters verbs: - list @@ -180,6 +181,7 @@ rules: - clientsettingspolicies/status - observabilitypolicies/status - upstreamsettingspolicies/status + - proxysettingspolicies/status - snippetsfilters/status verbs: - update diff --git a/deploy/snippets-filters/deploy.yaml b/deploy/snippets-filters/deploy.yaml index 0ef73c141b..d0b733e2fe 100644 --- a/deploy/snippets-filters/deploy.yaml +++ b/deploy/snippets-filters/deploy.yaml @@ -169,6 +169,7 @@ rules: - clientsettingspolicies - observabilitypolicies - upstreamsettingspolicies + - proxysettingspolicies - snippetsfilters verbs: - list @@ -180,6 +181,7 @@ rules: - clientsettingspolicies/status - observabilitypolicies/status - upstreamsettingspolicies/status + - proxysettingspolicies/status - snippetsfilters/status verbs: - update diff --git a/examples/proxy-settings-policy/README.md b/examples/proxy-settings-policy/README.md new file mode 100644 index 0000000000..961cadfa31 --- /dev/null +++ b/examples/proxy-settings-policy/README.md @@ -0,0 +1,3 @@ +# Proxy Settings Policy + +This directory contains the YAML files used in the [ProxySettingsPolicy](https://docs.nginx.com/nginx-gateway-fabric/traffic-management/proxy-settings/) guide. diff --git a/examples/proxy-settings-policy/app.yaml b/examples/proxy-settings-policy/app.yaml new file mode 100644 index 0000000000..99a4465e2a --- /dev/null +++ b/examples/proxy-settings-policy/app.yaml @@ -0,0 +1,154 @@ +apiVersion: v1 +kind: ConfigMap +metadata: + name: response-generator +data: + main.go: | + package main + + import ( + "fmt" + "log" + "net/http" + "strings" + "time" + ) + + func coffeeHandler(w http.ResponseWriter, r *http.Request) { + // Returns a large response to demonstrate buffering requirements + // This generates a response larger than NGINX's default proxy_buffer_size (4k/8k) + // Without proper buffering configuration, this may cause errors + w.Header().Set("Content-Type", "text/plain") + w.Header().Set("X-Content-Type-Options", "nosniff") + + // Generate a large header that exceeds default proxy_buffer_size + // Default is typically 4k-8k depending on platform + largeHeader := strings.Repeat("X", 10000) + w.Header().Set("X-Large-Header", largeHeader) + + flusher, ok := w.(http.Flusher) + if !ok { + http.Error(w, "Streaming unsupported", http.StatusInternalServerError) + return + } + + // Generate a large response body (5MB in chunks) + // This tests both proxy_buffer_size and proxy_buffers settings + chunkSize := 1024 // 1KB chunks + totalChunks := 5120 // 5MB total + + for i := 0; i < totalChunks; i++ { + chunk := fmt.Sprintf("Coffee chunk %d: %s\n", i, strings.Repeat("x", chunkSize-20)) + fmt.Fprint(w, chunk) + if i%10 == 0 { + flusher.Flush() + } + time.Sleep(100 * time.Microsecond) // Small delay to simulate slow backend + } + } + + func healthHandler(w http.ResponseWriter, r *http.Request) { + w.WriteHeader(http.StatusOK) + fmt.Fprint(w, "OK") + } + + func main() { + http.HandleFunc("/coffee", coffeeHandler) + http.HandleFunc("/health", healthHandler) + + log.Println("Server starting on :8080") + if err := http.ListenAndServe(":8080", nil); err != nil { + log.Fatal(err) + } + } +--- +apiVersion: apps/v1 +kind: Deployment +metadata: + name: coffee +spec: + replicas: 1 + selector: + matchLabels: + app: coffee + template: + metadata: + labels: + app: coffee + spec: + containers: + - name: coffee + image: golang:1.25-alpine + command: ["/bin/sh"] + args: + - -c + - | + cd /app + go run main.go + ports: + - containerPort: 8080 + volumeMounts: + - name: app + mountPath: /app + livenessProbe: + httpGet: + path: /health + port: 8080 + initialDelaySeconds: 5 + periodSeconds: 10 + readinessProbe: + httpGet: + path: /health + port: 8080 + initialDelaySeconds: 5 + periodSeconds: 5 + volumes: + - name: app + configMap: + name: response-generator +--- +apiVersion: v1 +kind: Service +metadata: + name: coffee +spec: + ports: + - port: 80 + targetPort: 8080 + protocol: TCP + name: http + selector: + app: coffee +--- +apiVersion: apps/v1 +kind: Deployment +metadata: + name: tea +spec: + replicas: 1 + selector: + matchLabels: + app: tea + template: + metadata: + labels: + app: tea + spec: + containers: + - name: tea + image: nginxdemos/nginx-hello:plain-text + ports: + - containerPort: 8080 +--- +apiVersion: v1 +kind: Service +metadata: + name: tea +spec: + ports: + - port: 80 + targetPort: 8080 + protocol: TCP + name: http + selector: + app: tea diff --git a/examples/proxy-settings-policy/coffee-proxy-settings.yaml b/examples/proxy-settings-policy/coffee-proxy-settings.yaml new file mode 100644 index 0000000000..0349a438eb --- /dev/null +++ b/examples/proxy-settings-policy/coffee-proxy-settings.yaml @@ -0,0 +1,19 @@ +apiVersion: gateway.nginx.org/v1alpha1 +kind: ProxySettingsPolicy +metadata: + name: coffee-proxy-settings +spec: + targetRefs: + - group: gateway.networking.k8s.io + kind: HTTPRoute + name: coffee + buffering: + # Increase buffer size to handle large response headers (>10KB) + bufferSize: "16k" + # Configure more and larger buffers to handle 5MB response body + buffers: + number: 16 + size: "64k" + # Set busy buffers size to allow more data to be sent to client + # while still receiving from upstream + busyBuffersSize: "128k" diff --git a/examples/proxy-settings-policy/gateway-proxy-settings.yaml b/examples/proxy-settings-policy/gateway-proxy-settings.yaml new file mode 100644 index 0000000000..150edde7b1 --- /dev/null +++ b/examples/proxy-settings-policy/gateway-proxy-settings.yaml @@ -0,0 +1,15 @@ +apiVersion: gateway.nginx.org/v1alpha1 +kind: ProxySettingsPolicy +metadata: + name: gateway-proxy-settings +spec: + targetRefs: + - group: gateway.networking.k8s.io + kind: Gateway + name: gateway + buffering: + bufferSize: "4k" + buffers: + number: 8 + size: "4k" + busyBuffersSize: "16k" diff --git a/examples/proxy-settings-policy/gateway.yaml b/examples/proxy-settings-policy/gateway.yaml new file mode 100644 index 0000000000..e6507f613b --- /dev/null +++ b/examples/proxy-settings-policy/gateway.yaml @@ -0,0 +1,11 @@ +apiVersion: gateway.networking.k8s.io/v1 +kind: Gateway +metadata: + name: gateway +spec: + gatewayClassName: nginx + listeners: + - name: http + port: 80 + protocol: HTTP + hostname: "*.example.com" diff --git a/examples/proxy-settings-policy/httproutes.yaml b/examples/proxy-settings-policy/httproutes.yaml new file mode 100644 index 0000000000..67927335cb --- /dev/null +++ b/examples/proxy-settings-policy/httproutes.yaml @@ -0,0 +1,37 @@ +apiVersion: gateway.networking.k8s.io/v1 +kind: HTTPRoute +metadata: + name: coffee +spec: + parentRefs: + - name: gateway + sectionName: http + hostnames: + - "cafe.example.com" + rules: + - matches: + - path: + type: PathPrefix + value: /coffee + backendRefs: + - name: coffee + port: 80 +--- +apiVersion: gateway.networking.k8s.io/v1 +kind: HTTPRoute +metadata: + name: tea +spec: + parentRefs: + - name: gateway + sectionName: http + hostnames: + - "cafe.example.com" + rules: + - matches: + - path: + type: Exact + value: /tea + backendRefs: + - name: tea + port: 80 diff --git a/internal/controller/manager.go b/internal/controller/manager.go index d4e2114e8a..257511595f 100644 --- a/internal/controller/manager.go +++ b/internal/controller/manager.go @@ -49,6 +49,7 @@ import ( "github.com/nginx/nginx-gateway-fabric/v2/internal/controller/nginx/config/policies" "github.com/nginx/nginx-gateway-fabric/v2/internal/controller/nginx/config/policies/clientsettings" "github.com/nginx/nginx-gateway-fabric/v2/internal/controller/nginx/config/policies/observability" + "github.com/nginx/nginx-gateway-fabric/v2/internal/controller/nginx/config/policies/proxysettings" "github.com/nginx/nginx-gateway-fabric/v2/internal/controller/nginx/config/policies/upstreamsettings" ngxvalidation "github.com/nginx/nginx-gateway-fabric/v2/internal/controller/nginx/config/validation" "github.com/nginx/nginx-gateway-fabric/v2/internal/controller/provisioner" @@ -333,6 +334,10 @@ func createPolicyManager( GVK: mustExtractGVK(&ngfAPIv1alpha2.ObservabilityPolicy{}), Validator: observability.NewValidator(validator), }, + { + GVK: mustExtractGVK(&ngfAPIv1alpha1.ProxySettingsPolicy{}), + Validator: proxysettings.NewValidator(validator), + }, { GVK: mustExtractGVK(&ngfAPIv1alpha1.UpstreamSettingsPolicy{}), Validator: upstreamsettings.NewValidator(validator), @@ -523,6 +528,12 @@ func registerControllers( controller.WithK8sPredicate(k8spredicate.GenerationChangedPredicate{}), }, }, + { + objectType: &ngfAPIv1alpha1.ProxySettingsPolicy{}, + options: []controller.Option{ + controller.WithK8sPredicate(k8spredicate.GenerationChangedPredicate{}), + }, + }, { objectType: &ngfAPIv1alpha1.UpstreamSettingsPolicy{}, options: []controller.Option{ @@ -769,6 +780,7 @@ func prepareFirstEventBatchPreparerArgs(cfg config.Config) ([]client.Object, []c &gatewayv1.GRPCRouteList{}, &ngfAPIv1alpha1.ClientSettingsPolicyList{}, &ngfAPIv1alpha2.ObservabilityPolicyList{}, + &ngfAPIv1alpha1.ProxySettingsPolicyList{}, &ngfAPIv1alpha1.UpstreamSettingsPolicyList{}, partialObjectMetadataList, } diff --git a/internal/controller/manager_test.go b/internal/controller/manager_test.go index 88aee02ac8..0e871e047b 100644 --- a/internal/controller/manager_test.go +++ b/internal/controller/manager_test.go @@ -67,6 +67,7 @@ func TestPrepareFirstEventBatchPreparerArgs(t *testing.T) { partialObjectMetadataList, &ngfAPIv1alpha1.ClientSettingsPolicyList{}, &ngfAPIv1alpha2.ObservabilityPolicyList{}, + &ngfAPIv1alpha1.ProxySettingsPolicyList{}, &ngfAPIv1alpha1.UpstreamSettingsPolicyList{}, }, }, @@ -95,6 +96,7 @@ func TestPrepareFirstEventBatchPreparerArgs(t *testing.T) { &gatewayv1.GRPCRouteList{}, &ngfAPIv1alpha1.ClientSettingsPolicyList{}, &ngfAPIv1alpha2.ObservabilityPolicyList{}, + &ngfAPIv1alpha1.ProxySettingsPolicyList{}, &ngfAPIv1alpha1.UpstreamSettingsPolicyList{}, }, }, @@ -120,6 +122,7 @@ func TestPrepareFirstEventBatchPreparerArgs(t *testing.T) { &gatewayv1.GRPCRouteList{}, &ngfAPIv1alpha1.ClientSettingsPolicyList{}, &ngfAPIv1alpha2.ObservabilityPolicyList{}, + &ngfAPIv1alpha1.ProxySettingsPolicyList{}, &ngfAPIv1alpha1.UpstreamSettingsPolicyList{}, partialObjectMetadataList, &inference.InferencePoolList{}, @@ -151,6 +154,7 @@ func TestPrepareFirstEventBatchPreparerArgs(t *testing.T) { &ngfAPIv1alpha1.ClientSettingsPolicyList{}, &ngfAPIv1alpha2.ObservabilityPolicyList{}, &ngfAPIv1alpha1.SnippetsFilterList{}, + &ngfAPIv1alpha1.ProxySettingsPolicyList{}, &ngfAPIv1alpha1.UpstreamSettingsPolicyList{}, }, }, @@ -183,6 +187,7 @@ func TestPrepareFirstEventBatchPreparerArgs(t *testing.T) { &ngfAPIv1alpha1.ClientSettingsPolicyList{}, &ngfAPIv1alpha2.ObservabilityPolicyList{}, &ngfAPIv1alpha1.SnippetsFilterList{}, + &ngfAPIv1alpha1.ProxySettingsPolicyList{}, &ngfAPIv1alpha1.UpstreamSettingsPolicyList{}, }, }, diff --git a/internal/controller/nginx/config/base_http_config.go b/internal/controller/nginx/config/base_http_config.go index fcceaed0f2..2f641bc659 100644 --- a/internal/controller/nginx/config/base_http_config.go +++ b/internal/controller/nginx/config/base_http_config.go @@ -3,6 +3,7 @@ package config import ( gotemplate "text/template" + "github.com/nginx/nginx-gateway-fabric/v2/internal/controller/nginx/config/policies" "github.com/nginx/nginx-gateway-fabric/v2/internal/controller/nginx/config/shared" "github.com/nginx/nginx-gateway-fabric/v2/internal/controller/state/dataplane" "github.com/nginx/nginx-gateway-fabric/v2/internal/framework/helpers" @@ -26,8 +27,21 @@ type httpConfig struct { HTTP2 bool } -func executeBaseHTTPConfig(conf dataplane.Configuration) []executeResult { - includes := createIncludesFromSnippets(conf.BaseHTTPConfig.Snippets) +func newExecuteBaseHTTPConfigFunc(generator policies.Generator) executeFunc { + return func(conf dataplane.Configuration) []executeResult { + return executeBaseHTTPConfig(conf, generator) + } +} + +func executeBaseHTTPConfig(conf dataplane.Configuration, generator policies.Generator) []executeResult { + snippetIncludes := createIncludesFromSnippets(conf.BaseHTTPConfig.Snippets) + policyIncludes := createIncludesFromPolicyGenerateResult( + generator.GenerateForHTTP(conf.BaseHTTPConfig.Policies), + ) + + includes := make([]shared.Include, 0, len(snippetIncludes)+len(policyIncludes)) + includes = append(includes, snippetIncludes...) + includes = append(includes, policyIncludes...) hc := httpConfig{ HTTP2: conf.BaseHTTPConfig.HTTP2, diff --git a/internal/controller/nginx/config/base_http_config_test.go b/internal/controller/nginx/config/base_http_config_test.go index 50ac43232f..bf498d95e0 100644 --- a/internal/controller/nginx/config/base_http_config_test.go +++ b/internal/controller/nginx/config/base_http_config_test.go @@ -8,6 +8,8 @@ import ( . "github.com/onsi/gomega" + "github.com/nginx/nginx-gateway-fabric/v2/internal/controller/nginx/config/policies" + "github.com/nginx/nginx-gateway-fabric/v2/internal/controller/nginx/config/policies/policiesfakes" "github.com/nginx/nginx-gateway-fabric/v2/internal/controller/state/dataplane" ) @@ -69,7 +71,7 @@ func TestLoggingSettingsTemplate(t *testing.T) { Logging: dataplane.Logging{AccessLog: tt.accessLog}, } - res := executeBaseHTTPConfig(conf) + res := executeBaseHTTPConfig(conf, policies.UnimplementedGenerator{}) g.Expect(res).To(HaveLen(1)) httpConfig := string(res[0].data) for _, expectedOutput := range tt.expectedOutputs { @@ -120,7 +122,7 @@ func TestExecuteBaseHttp_HTTP2(t *testing.T) { t.Parallel() g := NewWithT(t) - res := executeBaseHTTPConfig(test.conf) + res := executeBaseHTTPConfig(test.conf, policies.UnimplementedGenerator{}) g.Expect(res).To(HaveLen(1)) g.Expect(test.expCount).To(Equal(strings.Count(string(res[0].data), expSubStr))) g.Expect(strings.Count(string(res[0].data), "map $http_host $gw_api_compliant_host {")).To(Equal(1)) @@ -150,7 +152,7 @@ func TestExecuteBaseHttp_Snippets(t *testing.T) { g := NewWithT(t) - res := executeBaseHTTPConfig(conf) + res := executeBaseHTTPConfig(conf, policies.UnimplementedGenerator{}) g.Expect(res).To(HaveLen(3)) sort.Slice( @@ -261,7 +263,7 @@ func TestExecuteBaseHttp_NginxReadinessProbePort(t *testing.T) { t.Parallel() g := NewWithT(t) - res := executeBaseHTTPConfig(test.conf) + res := executeBaseHTTPConfig(test.conf, policies.UnimplementedGenerator{}) g.Expect(res).To(HaveLen(1)) httpConfig := string(res[0].data) @@ -333,7 +335,7 @@ func TestExecuteBaseHttp_DNSResolver(t *testing.T) { t.Parallel() g := NewWithT(t) - res := executeBaseHTTPConfig(test.conf) + res := executeBaseHTTPConfig(test.conf, policies.UnimplementedGenerator{}) g.Expect(res).To(HaveLen(1)) httpConfig := string(res[0].data) @@ -391,7 +393,7 @@ func TestExecuteBaseHttp_GatewaySecretID(t *testing.T) { t.Parallel() g := NewWithT(t) - res := executeBaseHTTPConfig(test.conf) + res := executeBaseHTTPConfig(test.conf, policies.UnimplementedGenerator{}) g.Expect(res).To(HaveLen(1)) httpConfig := string(res[0].data) @@ -402,3 +404,60 @@ func TestExecuteBaseHttp_GatewaySecretID(t *testing.T) { }) } } + +func TestExecuteBaseHttp_Policies(t *testing.T) { + t.Parallel() + + g := NewWithT(t) + + fakeGen := &policiesfakes.FakeGenerator{} + fakeGen.GenerateForHTTPReturns(policies.GenerateResultFiles{ + { + Name: "policy1.conf", + Content: []byte("policy1 content"), + }, + { + Name: "policy2.conf", + Content: []byte("policy2 content"), + }, + }) + + conf := dataplane.Configuration{ + BaseHTTPConfig: dataplane.BaseHTTPConfig{ + Policies: []policies.Policy{ + &policiesfakes.FakePolicy{}, + &policiesfakes.FakePolicy{}, + }, + }, + } + + res := executeBaseHTTPConfig(conf, fakeGen) + g.Expect(res).To(HaveLen(3)) // 1 http.conf + 2 policy files + + sort.Slice(res, func(i, j int) bool { + return res[i].dest < res[j].dest + }) + + /* + Order of files: + /etc/nginx/conf.d/http.conf + /etc/nginx/includes/policy1.conf + /etc/nginx/includes/policy2.conf + */ + + httpRes := string(res[0].data) + g.Expect(httpRes).To(ContainSubstring("map $http_host $gw_api_compliant_host {")) + g.Expect(httpRes).To(ContainSubstring("include /etc/nginx/includes/policy1.conf;")) + g.Expect(httpRes).To(ContainSubstring("include /etc/nginx/includes/policy2.conf;")) + + policy1Res := string(res[1].data) + g.Expect(policy1Res).To(Equal("policy1 content")) + + policy2Res := string(res[2].data) + g.Expect(policy2Res).To(Equal("policy2 content")) + + // Verify GenerateForHTTP was called with the correct policies + g.Expect(fakeGen.GenerateForHTTPCallCount()).To(Equal(1)) + calledPolicies := fakeGen.GenerateForHTTPArgsForCall(0) + g.Expect(calledPolicies).To(HaveLen(2)) +} diff --git a/internal/controller/nginx/config/generator.go b/internal/controller/nginx/config/generator.go index 6fa7602c04..6d47fb9d89 100644 --- a/internal/controller/nginx/config/generator.go +++ b/internal/controller/nginx/config/generator.go @@ -15,6 +15,7 @@ import ( "github.com/nginx/nginx-gateway-fabric/v2/internal/controller/nginx/config/policies" "github.com/nginx/nginx-gateway-fabric/v2/internal/controller/nginx/config/policies/clientsettings" "github.com/nginx/nginx-gateway-fabric/v2/internal/controller/nginx/config/policies/observability" + "github.com/nginx/nginx-gateway-fabric/v2/internal/controller/nginx/config/policies/proxysettings" "github.com/nginx/nginx-gateway-fabric/v2/internal/controller/nginx/config/policies/upstreamsettings" "github.com/nginx/nginx-gateway-fabric/v2/internal/controller/state/dataplane" "github.com/nginx/nginx-gateway-fabric/v2/internal/framework/file" @@ -125,6 +126,7 @@ func (g GeneratorImpl) Generate(conf dataplane.Configuration) []agent.File { policyGenerator := policies.NewCompositeGenerator( clientsettings.NewGenerator(), observability.NewGenerator(conf.Telemetry), + proxysettings.NewGenerator(), ) files = append(files, g.executeConfigTemplates(conf, policyGenerator)...) @@ -203,7 +205,7 @@ func (g GeneratorImpl) getExecuteFuncs( return []executeFunc{ executeMainConfig, executeEventsConfig, - executeBaseHTTPConfig, + newExecuteBaseHTTPConfigFunc(generator), g.newExecuteServersFunc(generator, keepAliveCheck), newExecuteUpstreamsFunc(upstreams), executeSplitClients, diff --git a/internal/controller/nginx/config/policies/clientsettings/generator.go b/internal/controller/nginx/config/policies/clientsettings/generator.go index b895f3104a..dcfe05c59e 100644 --- a/internal/controller/nginx/config/policies/clientsettings/generator.go +++ b/internal/controller/nginx/config/policies/clientsettings/generator.go @@ -39,7 +39,9 @@ keepalive_timeout {{ .KeepAlive.Timeout.Server }}; ` // Generator generates nginx configuration based on a clientsettings policy. -type Generator struct{} +type Generator struct { + policies.UnimplementedGenerator +} // NewGenerator returns a new instance of Generator. func NewGenerator() *Generator { diff --git a/internal/controller/nginx/config/policies/generator.go b/internal/controller/nginx/config/policies/generator.go index 5a25df37dc..e3e46ac7e7 100644 --- a/internal/controller/nginx/config/policies/generator.go +++ b/internal/controller/nginx/config/policies/generator.go @@ -8,6 +8,8 @@ import ( // //counterfeiter:generate . Generator type Generator interface { + // GenerateForHTTP generates policy configuration for the http block. + GenerateForHTTP(policies []Policy) GenerateResultFiles // GenerateForServer generates policy configuration for the server block. GenerateForServer(policies []Policy, server http.Server) GenerateResultFiles // GenerateForLocation generates policy configuration for a normal location block. @@ -35,6 +37,17 @@ func NewCompositeGenerator(generators ...Generator) *CompositeGenerator { return &CompositeGenerator{generators: generators} } +// GenerateForHTTP calls all policy generators for the http block. +func (g *CompositeGenerator) GenerateForHTTP(policies []Policy) GenerateResultFiles { + var compositeResult GenerateResultFiles + + for _, generator := range g.generators { + compositeResult = append(compositeResult, generator.GenerateForHTTP(policies)...) + } + + return compositeResult +} + // GenerateForServer calls all policy generators for the server block. func (g *CompositeGenerator) GenerateForServer(policies []Policy, server http.Server) GenerateResultFiles { var compositeResult GenerateResultFiles @@ -72,6 +85,10 @@ func (g *CompositeGenerator) GenerateForInternalLocation(policies []Policy) Gene // possible generations, in order to satisfy the Generator interface. type UnimplementedGenerator struct{} +func (u UnimplementedGenerator) GenerateForHTTP(_ []Policy) GenerateResultFiles { + return nil +} + func (u UnimplementedGenerator) GenerateForServer(_ []Policy, _ http.Server) GenerateResultFiles { return nil } diff --git a/internal/controller/nginx/config/policies/policiesfakes/fake_generator.go b/internal/controller/nginx/config/policies/policiesfakes/fake_generator.go index e9e1689f3d..c80b0281a5 100644 --- a/internal/controller/nginx/config/policies/policiesfakes/fake_generator.go +++ b/internal/controller/nginx/config/policies/policiesfakes/fake_generator.go @@ -9,6 +9,17 @@ import ( ) type FakeGenerator struct { + GenerateForHTTPStub func([]policies.Policy) policies.GenerateResultFiles + generateForHTTPMutex sync.RWMutex + generateForHTTPArgsForCall []struct { + arg1 []policies.Policy + } + generateForHTTPReturns struct { + result1 policies.GenerateResultFiles + } + generateForHTTPReturnsOnCall map[int]struct { + result1 policies.GenerateResultFiles + } GenerateForInternalLocationStub func([]policies.Policy) policies.GenerateResultFiles generateForInternalLocationMutex sync.RWMutex generateForInternalLocationArgsForCall []struct { @@ -48,6 +59,72 @@ type FakeGenerator struct { invocationsMutex sync.RWMutex } +func (fake *FakeGenerator) GenerateForHTTP(arg1 []policies.Policy) policies.GenerateResultFiles { + var arg1Copy []policies.Policy + if arg1 != nil { + arg1Copy = make([]policies.Policy, len(arg1)) + copy(arg1Copy, arg1) + } + fake.generateForHTTPMutex.Lock() + ret, specificReturn := fake.generateForHTTPReturnsOnCall[len(fake.generateForHTTPArgsForCall)] + fake.generateForHTTPArgsForCall = append(fake.generateForHTTPArgsForCall, struct { + arg1 []policies.Policy + }{arg1Copy}) + stub := fake.GenerateForHTTPStub + fakeReturns := fake.generateForHTTPReturns + fake.recordInvocation("GenerateForHTTP", []interface{}{arg1Copy}) + fake.generateForHTTPMutex.Unlock() + if stub != nil { + return stub(arg1) + } + if specificReturn { + return ret.result1 + } + return fakeReturns.result1 +} + +func (fake *FakeGenerator) GenerateForHTTPCallCount() int { + fake.generateForHTTPMutex.RLock() + defer fake.generateForHTTPMutex.RUnlock() + return len(fake.generateForHTTPArgsForCall) +} + +func (fake *FakeGenerator) GenerateForHTTPCalls(stub func([]policies.Policy) policies.GenerateResultFiles) { + fake.generateForHTTPMutex.Lock() + defer fake.generateForHTTPMutex.Unlock() + fake.GenerateForHTTPStub = stub +} + +func (fake *FakeGenerator) GenerateForHTTPArgsForCall(i int) []policies.Policy { + fake.generateForHTTPMutex.RLock() + defer fake.generateForHTTPMutex.RUnlock() + argsForCall := fake.generateForHTTPArgsForCall[i] + return argsForCall.arg1 +} + +func (fake *FakeGenerator) GenerateForHTTPReturns(result1 policies.GenerateResultFiles) { + fake.generateForHTTPMutex.Lock() + defer fake.generateForHTTPMutex.Unlock() + fake.GenerateForHTTPStub = nil + fake.generateForHTTPReturns = struct { + result1 policies.GenerateResultFiles + }{result1} +} + +func (fake *FakeGenerator) GenerateForHTTPReturnsOnCall(i int, result1 policies.GenerateResultFiles) { + fake.generateForHTTPMutex.Lock() + defer fake.generateForHTTPMutex.Unlock() + fake.GenerateForHTTPStub = nil + if fake.generateForHTTPReturnsOnCall == nil { + fake.generateForHTTPReturnsOnCall = make(map[int]struct { + result1 policies.GenerateResultFiles + }) + } + fake.generateForHTTPReturnsOnCall[i] = struct { + result1 policies.GenerateResultFiles + }{result1} +} + func (fake *FakeGenerator) GenerateForInternalLocation(arg1 []policies.Policy) policies.GenerateResultFiles { var arg1Copy []policies.Policy if arg1 != nil { diff --git a/internal/controller/nginx/config/policies/proxysettings/generator.go b/internal/controller/nginx/config/policies/proxysettings/generator.go new file mode 100644 index 0000000000..243ef038cb --- /dev/null +++ b/internal/controller/nginx/config/policies/proxysettings/generator.go @@ -0,0 +1,108 @@ +package proxysettings + +import ( + "fmt" + "text/template" + + ngfAPI "github.com/nginx/nginx-gateway-fabric/v2/apis/v1alpha1" + "github.com/nginx/nginx-gateway-fabric/v2/internal/controller/nginx/config/http" + "github.com/nginx/nginx-gateway-fabric/v2/internal/controller/nginx/config/policies" + "github.com/nginx/nginx-gateway-fabric/v2/internal/framework/helpers" +) + +var tmpl = template.Must(template.New("proxy settings policy").Parse(proxySettingsTemplate)) + +const proxySettingsTemplate = ` +{{- if .ProxyBufferingDirective }} +proxy_buffering {{ .ProxyBufferingDirective }}; +{{- end }} +{{- if .ProxyBufferSize }} +proxy_buffer_size {{ .ProxyBufferSize }}; +{{- end }} +{{- if .ProxyBuffers }} +proxy_buffers {{ .ProxyBuffers }}; +{{- end }} +{{- if .ProxyBusyBuffersSize }} +proxy_busy_buffers_size {{ .ProxyBusyBuffersSize }}; +{{- end }} +` + +type proxySettings struct { + ProxyBufferingDirective string + ProxyBufferSize string + ProxyBuffers string + ProxyBusyBuffersSize string +} + +func getProxySettings(spec ngfAPI.ProxySettingsPolicySpec) proxySettings { + settings := proxySettings{} + + if spec.Buffering != nil { + if spec.Buffering.Disable != nil { + if *spec.Buffering.Disable { + settings.ProxyBufferingDirective = "off" + } else { + settings.ProxyBufferingDirective = "on" + } + } + + if spec.Buffering.BufferSize != nil { + settings.ProxyBufferSize = string(*spec.Buffering.BufferSize) + } + + if spec.Buffering.Buffers != nil { + settings.ProxyBuffers = fmt.Sprintf("%d %s", spec.Buffering.Buffers.Number, spec.Buffering.Buffers.Size) + } + + if spec.Buffering.BusyBuffersSize != nil { + settings.ProxyBusyBuffersSize = string(*spec.Buffering.BusyBuffersSize) + } + } + + return settings +} + +// Generator generates nginx configuration based on a ProxySettingsPolicy. +type Generator struct { + policies.UnimplementedGenerator +} + +// NewGenerator returns a new instance of Generator. +func NewGenerator() *Generator { + return &Generator{} +} + +// GenerateForHTTP generates policy configuration for the http block. +func (g Generator) GenerateForHTTP(pols []policies.Policy) 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 { + psp, ok := pol.(*ngfAPI.ProxySettingsPolicy) + if !ok { + continue + } + + settings := getProxySettings(psp.Spec) + + files = append(files, policies.File{ + Name: fmt.Sprintf("ProxySettingsPolicy_%s_%s.conf", psp.Namespace, psp.Name), + Content: helpers.MustExecuteTemplate(tmpl, settings), + }) + } + + return files +} diff --git a/internal/controller/nginx/config/policies/proxysettings/generator_test.go b/internal/controller/nginx/config/policies/proxysettings/generator_test.go new file mode 100644 index 0000000000..cd8a4a8313 --- /dev/null +++ b/internal/controller/nginx/config/policies/proxysettings/generator_test.go @@ -0,0 +1,166 @@ +package proxysettings_test + +import ( + "testing" + + . "github.com/onsi/gomega" + + ngfAPIv1alpha1 "github.com/nginx/nginx-gateway-fabric/v2/apis/v1alpha1" + ngfAPIv1alpha2 "github.com/nginx/nginx-gateway-fabric/v2/apis/v1alpha2" + "github.com/nginx/nginx-gateway-fabric/v2/internal/controller/nginx/config/http" + "github.com/nginx/nginx-gateway-fabric/v2/internal/controller/nginx/config/policies" + "github.com/nginx/nginx-gateway-fabric/v2/internal/controller/nginx/config/policies/proxysettings" + "github.com/nginx/nginx-gateway-fabric/v2/internal/framework/helpers" +) + +func TestGenerate(t *testing.T) { + t.Parallel() + + tests := []struct { + name string + policy policies.Policy + expStrings []string + }{ + { + name: "buffering disabled", + policy: &ngfAPIv1alpha1.ProxySettingsPolicy{ + Spec: ngfAPIv1alpha1.ProxySettingsPolicySpec{ + Buffering: &ngfAPIv1alpha1.ProxyBuffering{ + Disable: helpers.GetPointer(true), + }, + }, + }, + expStrings: []string{ + "proxy_buffering off;", + }, + }, + { + name: "buffering enabled", + policy: &ngfAPIv1alpha1.ProxySettingsPolicy{ + Spec: ngfAPIv1alpha1.ProxySettingsPolicySpec{ + Buffering: &ngfAPIv1alpha1.ProxyBuffering{ + Disable: helpers.GetPointer(false), + }, + }, + }, + expStrings: []string{ + "proxy_buffering on;", + }, + }, + { + name: "buffer size populated", + policy: &ngfAPIv1alpha1.ProxySettingsPolicy{ + Spec: ngfAPIv1alpha1.ProxySettingsPolicySpec{ + Buffering: &ngfAPIv1alpha1.ProxyBuffering{ + BufferSize: helpers.GetPointer[ngfAPIv1alpha1.Size]("16k"), + }, + }, + }, + expStrings: []string{ + "proxy_buffer_size 16k;", + }, + }, + { + name: "buffers populated", + policy: &ngfAPIv1alpha1.ProxySettingsPolicy{ + Spec: ngfAPIv1alpha1.ProxySettingsPolicySpec{ + Buffering: &ngfAPIv1alpha1.ProxyBuffering{ + Buffers: &ngfAPIv1alpha1.ProxyBuffers{ + Number: 8, + Size: "4k", + }, + }, + }, + }, + expStrings: []string{ + "proxy_buffers 8 4k;", + }, + }, + { + name: "busy buffers size populated", + policy: &ngfAPIv1alpha1.ProxySettingsPolicy{ + Spec: ngfAPIv1alpha1.ProxySettingsPolicySpec{ + Buffering: &ngfAPIv1alpha1.ProxyBuffering{ + BusyBuffersSize: helpers.GetPointer[ngfAPIv1alpha1.Size]("32k"), + }, + }, + }, + expStrings: []string{ + "proxy_busy_buffers_size 32k;", + }, + }, + { + name: "all buffering fields populated", + policy: &ngfAPIv1alpha1.ProxySettingsPolicy{ + Spec: ngfAPIv1alpha1.ProxySettingsPolicySpec{ + Buffering: &ngfAPIv1alpha1.ProxyBuffering{ + Disable: helpers.GetPointer(false), + BufferSize: helpers.GetPointer[ngfAPIv1alpha1.Size]("16k"), + Buffers: &ngfAPIv1alpha1.ProxyBuffers{ + Number: 8, + Size: "4k", + }, + BusyBuffersSize: helpers.GetPointer[ngfAPIv1alpha1.Size]("32k"), + }, + }, + }, + expStrings: []string{ + "proxy_buffering on;", + "proxy_buffer_size 16k;", + "proxy_buffers 8 4k;", + "proxy_busy_buffers_size 32k;", + }, + }, + } + + checkResults := func(t *testing.T, resFiles policies.GenerateResultFiles, expStrings []string) { + t.Helper() + g := NewWithT(t) + 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) { + t.Parallel() + generator := proxysettings.NewGenerator() + + resFiles := generator.GenerateForHTTP([]policies.Policy{test.policy}) + checkResults(t, resFiles, test.expStrings) + + resFiles = generator.GenerateForLocation([]policies.Policy{test.policy}, http.Location{}) + checkResults(t, resFiles, test.expStrings) + + resFiles = generator.GenerateForInternalLocation([]policies.Policy{test.policy}) + checkResults(t, resFiles, test.expStrings) + }) + } +} + +func TestGenerateNoPolicies(t *testing.T) { + t.Parallel() + g := NewWithT(t) + + generator := proxysettings.NewGenerator() + + resFiles := generator.GenerateForHTTP([]policies.Policy{}) + g.Expect(resFiles).To(BeEmpty()) + + resFiles = generator.GenerateForHTTP([]policies.Policy{&ngfAPIv1alpha2.ObservabilityPolicy{}}) + g.Expect(resFiles).To(BeEmpty()) + + resFiles = generator.GenerateForLocation([]policies.Policy{}, http.Location{}) + g.Expect(resFiles).To(BeEmpty()) + + resFiles = generator.GenerateForLocation([]policies.Policy{&ngfAPIv1alpha2.ObservabilityPolicy{}}, http.Location{}) + g.Expect(resFiles).To(BeEmpty()) + + resFiles = generator.GenerateForInternalLocation([]policies.Policy{}) + g.Expect(resFiles).To(BeEmpty()) + + resFiles = generator.GenerateForInternalLocation([]policies.Policy{&ngfAPIv1alpha2.ObservabilityPolicy{}}) + g.Expect(resFiles).To(BeEmpty()) +} diff --git a/internal/controller/nginx/config/policies/proxysettings/validator.go b/internal/controller/nginx/config/policies/proxysettings/validator.go new file mode 100644 index 0000000000..8d3615b77b --- /dev/null +++ b/internal/controller/nginx/config/policies/proxysettings/validator.go @@ -0,0 +1,204 @@ +package proxysettings + +import ( + "strconv" + "strings" + + "k8s.io/apimachinery/pkg/util/validation/field" + + ngfAPI "github.com/nginx/nginx-gateway-fabric/v2/apis/v1alpha1" + "github.com/nginx/nginx-gateway-fabric/v2/internal/controller/nginx/config/policies" + "github.com/nginx/nginx-gateway-fabric/v2/internal/controller/state/conditions" + "github.com/nginx/nginx-gateway-fabric/v2/internal/controller/state/validation" + "github.com/nginx/nginx-gateway-fabric/v2/internal/framework/helpers" +) + +// Validator validates a ProxySettingsPolicy. +// Implements policies.Validator interface. +type Validator struct { + genericValidator validation.GenericValidator +} + +// NewValidator returns a new instance of Validator. +func NewValidator(genericValidator validation.GenericValidator) *Validator { + return &Validator{genericValidator: genericValidator} +} + +// Validate validates the spec of a ProxySettingsPolicy. +func (v *Validator) Validate(policy policies.Policy) []conditions.Condition { + psp := helpers.MustCastObject[*ngfAPI.ProxySettingsPolicy](policy) + + if err := v.validateSettings(psp.Spec); err != nil { + return []conditions.Condition{conditions.NewPolicyInvalid(err.Error())} + } + + return nil +} + +// ValidateGlobalSettings validates a ProxySettingsPolicy with respect to the NginxProxy global settings. +func (v *Validator) ValidateGlobalSettings( + _ policies.Policy, + _ *policies.GlobalSettings, +) []conditions.Condition { + return nil +} + +// Conflicts returns true if the two ProxySettingsPolicies conflict. +func (v *Validator) Conflicts(polA, polB policies.Policy) bool { + pspA := helpers.MustCastObject[*ngfAPI.ProxySettingsPolicy](polA) + pspB := helpers.MustCastObject[*ngfAPI.ProxySettingsPolicy](polB) + + return conflicts(pspA.Spec, pspB.Spec) +} + +func conflicts(a, b ngfAPI.ProxySettingsPolicySpec) bool { + if a.Buffering != nil && b.Buffering != nil { + if a.Buffering.Disable != nil && b.Buffering.Disable != nil { + return true + } + + if a.Buffering.BufferSize != nil && b.Buffering.BufferSize != nil { + return true + } + + if a.Buffering.Buffers != nil && b.Buffering.Buffers != nil { + return true + } + + if a.Buffering.BusyBuffersSize != nil && b.Buffering.BusyBuffersSize != nil { + return true + } + } + + return false +} + +// validateSettings performs validation on fields in the spec that are vulnerable to code injection. +// For all other fields, we rely on the CRD validation. +func (v *Validator) validateSettings(spec ngfAPI.ProxySettingsPolicySpec) error { + var allErrs field.ErrorList + fieldPath := field.NewPath("spec") + + if spec.Buffering != nil { + allErrs = append(allErrs, v.validateBufferSizes(*spec.Buffering, fieldPath.Child("buffering"))...) + allErrs = append(allErrs, validateBusyBufferSizeRelationships(*spec.Buffering, fieldPath.Child("buffering"))...) + } + + return allErrs.ToAggregate() +} + +func (v *Validator) validateBufferSizes(buffering ngfAPI.ProxyBuffering, fieldPath *field.Path) field.ErrorList { + var allErrs field.ErrorList + + if buffering.BufferSize != nil { + if err := v.genericValidator.ValidateNginxSize(string(*buffering.BufferSize)); err != nil { + path := fieldPath.Child("bufferSize") + allErrs = append(allErrs, field.Invalid(path, buffering.BufferSize, err.Error())) + } + } + + if buffering.Buffers != nil { + if err := v.genericValidator.ValidateNginxSize(string(buffering.Buffers.Size)); err != nil { + path := fieldPath.Child("buffers").Child("size") + allErrs = append(allErrs, field.Invalid(path, buffering.Buffers.Size, err.Error())) + } + } + + if buffering.BusyBuffersSize != nil { + if err := v.genericValidator.ValidateNginxSize(string(*buffering.BusyBuffersSize)); err != nil { + path := fieldPath.Child("busyBuffersSize") + allErrs = append(allErrs, field.Invalid(path, buffering.BusyBuffersSize, err.Error())) + } + } + + return allErrs +} + +// validateBusyBufferSizeRelationships validates NGINX constraints for busyBuffersSize. +// CEL cannot validate these constraints because it requires parsing NGINX size strings with units. +// +// NGINX constraints validated: +// 1. proxy_busy_buffers_size > proxy_buffer_size (when both are set) +// 2. proxy_busy_buffers_size < (proxy_buffers.number * proxy_buffers.size) - proxy_buffers.size (when buffers is set) +// +// Note: We only validate when fields are set in the same merged policy (same NGINX context level). +// We do not validate cross-level inheritance because NGINX handles that automatically. +func validateBusyBufferSizeRelationships(buffering ngfAPI.ProxyBuffering, fieldPath *field.Path) field.ErrorList { + var allErrs field.ErrorList + + if buffering.BusyBuffersSize == nil { + return nil + } + + busyBuffersSize, err := parseNginxSize(string(*buffering.BusyBuffersSize)) + if err != nil { + return nil // Skip validation if size is invalid (will be caught by other validation) + } + + // Validate: busyBuffersSize > bufferSize + if buffering.BufferSize != nil { + bufferSize, err := parseNginxSize(string(*buffering.BufferSize)) + if err == nil && busyBuffersSize <= bufferSize { + path := fieldPath.Child("busyBuffersSize") + allErrs = append(allErrs, field.Invalid( + path, + buffering.BusyBuffersSize, + "must be larger than bufferSize", + )) + } + } + + // Validate: busyBuffersSize < (buffers.number * buffers.size) - buffers.size + if buffering.Buffers != nil { + buffersSize, err := parseNginxSize(string(buffering.Buffers.Size)) + if err == nil { + totalBufferSpace := buffersSize * int64(buffering.Buffers.Number) + maxBusyBuffersSize := totalBufferSpace - buffersSize + if busyBuffersSize >= maxBusyBuffersSize { + path := fieldPath.Child("busyBuffersSize") + allErrs = append(allErrs, field.Invalid( + path, + buffering.BusyBuffersSize, + "must be less than the size of all proxy_buffers minus one buffer", + )) + } + } + } + + return allErrs +} + +// parseNginxSize parses an NGINX size string (e.g., "8k", "16m", "1024") and returns the size in bytes. +// Returns an error if the size string is invalid. +func parseNginxSize(size string) (int64, error) { + size = strings.TrimSpace(strings.ToLower(size)) + if size == "" { + return 0, strconv.ErrSyntax + } + + var multiplier int64 + var numberPart string + + // Check for unit suffix + switch { + case strings.HasSuffix(size, "k"): + multiplier = 1024 + numberPart = strings.TrimSuffix(size, "k") + case strings.HasSuffix(size, "m"): + multiplier = 1024 * 1024 + numberPart = strings.TrimSuffix(size, "m") + case strings.HasSuffix(size, "g"): + multiplier = 1024 * 1024 * 1024 + numberPart = strings.TrimSuffix(size, "g") + default: + multiplier = 1 + numberPart = size + } + + num, err := strconv.ParseInt(numberPart, 10, 64) + if err != nil { + return 0, err + } + + return num * multiplier, nil +} diff --git a/internal/controller/nginx/config/policies/proxysettings/validator_test.go b/internal/controller/nginx/config/policies/proxysettings/validator_test.go new file mode 100644 index 0000000000..4fc79a3fff --- /dev/null +++ b/internal/controller/nginx/config/policies/proxysettings/validator_test.go @@ -0,0 +1,312 @@ +package proxysettings_test + +import ( + "testing" + + . "github.com/onsi/gomega" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + v1 "sigs.k8s.io/gateway-api/apis/v1" + + ngfAPI "github.com/nginx/nginx-gateway-fabric/v2/apis/v1alpha1" + "github.com/nginx/nginx-gateway-fabric/v2/internal/controller/nginx/config/policies/policiesfakes" + "github.com/nginx/nginx-gateway-fabric/v2/internal/controller/nginx/config/policies/proxysettings" + "github.com/nginx/nginx-gateway-fabric/v2/internal/controller/nginx/config/validation" + "github.com/nginx/nginx-gateway-fabric/v2/internal/controller/state/conditions" + "github.com/nginx/nginx-gateway-fabric/v2/internal/framework/helpers" + "github.com/nginx/nginx-gateway-fabric/v2/internal/framework/kinds" +) + +type policyModFunc func(policy *ngfAPI.ProxySettingsPolicy) *ngfAPI.ProxySettingsPolicy + +func createValidPolicy() *ngfAPI.ProxySettingsPolicy { + return &ngfAPI.ProxySettingsPolicy{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: "default", + }, + Spec: ngfAPI.ProxySettingsPolicySpec{ + TargetRefs: []v1.LocalPolicyTargetReference{ + { + Group: v1.GroupName, + Kind: kinds.Gateway, + Name: "gateway", + }, + }, + Buffering: &ngfAPI.ProxyBuffering{ + Disable: helpers.GetPointer(false), + BufferSize: helpers.GetPointer[ngfAPI.Size]("8k"), + Buffers: &ngfAPI.ProxyBuffers{Number: 8, Size: "8k"}, // Total: 64k, Max busy: 56k + BusyBuffersSize: helpers.GetPointer[ngfAPI.Size]("32k"), // 32k < 56k, valid + }, + }, + Status: v1.PolicyStatus{}, + } +} + +func createModifiedPolicy(mod policyModFunc) *ngfAPI.ProxySettingsPolicy { + return mod(createValidPolicy()) +} + +func TestValidator_Validate(t *testing.T) { + t.Parallel() + tests := []struct { + name string + policy *ngfAPI.ProxySettingsPolicy + expConditions []conditions.Condition + }{ + { + name: "invalid proxy buffer size", + policy: createModifiedPolicy(func(p *ngfAPI.ProxySettingsPolicy) *ngfAPI.ProxySettingsPolicy { + p.Spec.Buffering.BufferSize = helpers.GetPointer[ngfAPI.Size]("invalid") + p.Spec.Buffering.BusyBuffersSize = nil // Remove to avoid secondary validation error + return p + }), + expConditions: []conditions.Condition{ + conditions.NewPolicyInvalid("spec.buffering.bufferSize: Invalid value: \"invalid\": ^\\d{1,4}(k|m|g)?$ " + + "(e.g. '1024', or '8k', or '20m', or '1g', regex used for validation is 'must contain a number. " + + "May be followed by 'k', 'm', or 'g', otherwise bytes are assumed')"), + }, + }, + { + name: "invalid proxy buffers size", + policy: createModifiedPolicy(func(p *ngfAPI.ProxySettingsPolicy) *ngfAPI.ProxySettingsPolicy { + p.Spec.Buffering.Buffers.Size = "invalid" + return p + }), + expConditions: []conditions.Condition{ + conditions.NewPolicyInvalid("spec.buffering.buffers.size: Invalid value: \"invalid\": ^\\d{1,4}(k|m|g)?$ " + + "(e.g. '1024', or '8k', or '20m', or '1g', regex used for validation is 'must contain a number. " + + "May be followed by 'k', 'm', or 'g', otherwise bytes are assumed')"), + }, + }, + { + name: "invalid proxy busy buffers size", + policy: createModifiedPolicy(func(p *ngfAPI.ProxySettingsPolicy) *ngfAPI.ProxySettingsPolicy { + p.Spec.Buffering.BusyBuffersSize = helpers.GetPointer[ngfAPI.Size]("invalid") + return p + }), + expConditions: []conditions.Condition{ + conditions.NewPolicyInvalid("spec.buffering.busyBuffersSize: Invalid value: \"invalid\": ^\\d{1,4}(k|m|g)?$ " + + "(e.g. '1024', or '8k', or '20m', or '1g', regex used for validation is 'must contain a number. " + + "May be followed by 'k', 'm', or 'g', otherwise bytes are assumed')"), + }, + }, + { + name: "busyBuffersSize smaller than bufferSize", + policy: createModifiedPolicy(func(p *ngfAPI.ProxySettingsPolicy) *ngfAPI.ProxySettingsPolicy { + p.Spec.Buffering.BufferSize = helpers.GetPointer[ngfAPI.Size]("16k") + p.Spec.Buffering.BusyBuffersSize = helpers.GetPointer[ngfAPI.Size]("8k") + return p + }), + expConditions: []conditions.Condition{ + conditions.NewPolicyInvalid("spec.buffering.busyBuffersSize: Invalid value: \"8k\": " + + "must be larger than bufferSize"), + }, + }, + { + name: "busyBuffersSize equal to bufferSize", + policy: createModifiedPolicy(func(p *ngfAPI.ProxySettingsPolicy) *ngfAPI.ProxySettingsPolicy { + p.Spec.Buffering.BufferSize = helpers.GetPointer[ngfAPI.Size]("16k") + p.Spec.Buffering.BusyBuffersSize = helpers.GetPointer[ngfAPI.Size]("16k") + return p + }), + expConditions: []conditions.Condition{ + conditions.NewPolicyInvalid("spec.buffering.busyBuffersSize: Invalid value: \"16k\": " + + "must be larger than bufferSize"), + }, + }, + { + name: "busyBuffersSize larger than bufferSize with different units", + policy: createModifiedPolicy(func(p *ngfAPI.ProxySettingsPolicy) *ngfAPI.ProxySettingsPolicy { + p.Spec.Buffering.BufferSize = helpers.GetPointer[ngfAPI.Size]("8k") + p.Spec.Buffering.Buffers = &ngfAPI.ProxyBuffers{Number: 16, Size: "128k"} // Total: 2MB, Max busy: 2MB-128k + p.Spec.Buffering.BusyBuffersSize = helpers.GetPointer[ngfAPI.Size]("1m") // 1MB < 2MB-128k, valid + return p + }), + expConditions: nil, + }, + { + name: "busyBuffersSize larger than bufferSize with bytes", + policy: createModifiedPolicy(func(p *ngfAPI.ProxySettingsPolicy) *ngfAPI.ProxySettingsPolicy { + p.Spec.Buffering.BufferSize = helpers.GetPointer[ngfAPI.Size]("4096") + p.Spec.Buffering.Buffers = &ngfAPI.ProxyBuffers{Number: 8, Size: "16k"} // Total: 128k, Max busy: 112k + p.Spec.Buffering.BusyBuffersSize = helpers.GetPointer[ngfAPI.Size]("8192") // 8k < 112k, valid + return p + }), + expConditions: nil, + }, + { + name: "busyBuffersSize not less than total buffers minus one", + policy: createModifiedPolicy(func(p *ngfAPI.ProxySettingsPolicy) *ngfAPI.ProxySettingsPolicy { + p.Spec.Buffering.Buffers = &ngfAPI.ProxyBuffers{Number: 8, Size: "4k"} + p.Spec.Buffering.BusyBuffersSize = helpers.GetPointer[ngfAPI.Size]("32k") // 32k >= (8*4k - 4k) = 28k + return p + }), + expConditions: []conditions.Condition{ + conditions.NewPolicyInvalid("spec.buffering.busyBuffersSize: Invalid value: \"32k\": " + + "must be less than the size of all proxy_buffers minus one buffer"), + }, + }, + { + name: "busyBuffersSize equal to total buffers minus one", + policy: createModifiedPolicy(func(p *ngfAPI.ProxySettingsPolicy) *ngfAPI.ProxySettingsPolicy { + p.Spec.Buffering.Buffers = &ngfAPI.ProxyBuffers{Number: 8, Size: "4k"} + p.Spec.Buffering.BusyBuffersSize = helpers.GetPointer[ngfAPI.Size]("28k") // 28k = (8*4k - 4k) + return p + }), + expConditions: []conditions.Condition{ + conditions.NewPolicyInvalid("spec.buffering.busyBuffersSize: Invalid value: \"28k\": " + + "must be less than the size of all proxy_buffers minus one buffer"), + }, + }, + { + name: "busyBuffersSize valid less than total buffers minus one", + policy: createModifiedPolicy(func(p *ngfAPI.ProxySettingsPolicy) *ngfAPI.ProxySettingsPolicy { + p.Spec.Buffering.BufferSize = helpers.GetPointer[ngfAPI.Size]("8k") // Make sure busyBuffersSize > bufferSize + p.Spec.Buffering.Buffers = &ngfAPI.ProxyBuffers{Number: 8, Size: "4k"} + p.Spec.Buffering.BusyBuffersSize = helpers.GetPointer[ngfAPI.Size]("16k") // 16k > 8k AND 16k < 28k, valid + return p + }), + expConditions: nil, + }, + { + name: "valid", + policy: createValidPolicy(), + expConditions: nil, + }, + } + + v := proxysettings.NewValidator(validation.GenericValidator{}) + + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + t.Parallel() + g := NewWithT(t) + + conds := v.Validate(test.policy) + g.Expect(conds).To(Equal(test.expConditions)) + }) + } +} + +func TestValidator_ValidatePanics(t *testing.T) { + t.Parallel() + v := proxysettings.NewValidator(nil) + + validate := func() { + _ = v.Validate(&policiesfakes.FakePolicy{}) + } + + g := NewWithT(t) + + g.Expect(validate).To(Panic()) +} + +func TestValidator_ValidateGlobalSettings(t *testing.T) { + t.Parallel() + g := NewWithT(t) + + v := proxysettings.NewValidator(validation.GenericValidator{}) + + g.Expect(v.ValidateGlobalSettings(nil, nil)).To(BeNil()) +} + +func TestValidator_Conflicts(t *testing.T) { + t.Parallel() + tests := []struct { + polA *ngfAPI.ProxySettingsPolicy + polB *ngfAPI.ProxySettingsPolicy + name string + conflicts bool + }{ + { + name: "no conflicts", + polA: &ngfAPI.ProxySettingsPolicy{ + Spec: ngfAPI.ProxySettingsPolicySpec{ + Buffering: &ngfAPI.ProxyBuffering{ + BufferSize: helpers.GetPointer[ngfAPI.Size]("16k"), + }, + }, + }, + polB: &ngfAPI.ProxySettingsPolicy{ + Spec: ngfAPI.ProxySettingsPolicySpec{ + Buffering: &ngfAPI.ProxyBuffering{ + Buffers: &ngfAPI.ProxyBuffers{Number: 8, Size: "4k"}, + }, + }, + }, + conflicts: false, + }, + { + name: "buffering disable conflicts", + polA: createValidPolicy(), + polB: &ngfAPI.ProxySettingsPolicy{ + Spec: ngfAPI.ProxySettingsPolicySpec{ + Buffering: &ngfAPI.ProxyBuffering{ + Disable: helpers.GetPointer(true), + }, + }, + }, + conflicts: true, + }, + { + name: "buffer size conflicts", + polA: createValidPolicy(), + polB: &ngfAPI.ProxySettingsPolicy{ + Spec: ngfAPI.ProxySettingsPolicySpec{ + Buffering: &ngfAPI.ProxyBuffering{ + BufferSize: helpers.GetPointer[ngfAPI.Size]("8k"), + }, + }, + }, + conflicts: true, + }, + { + name: "buffers conflicts", + polA: createValidPolicy(), + polB: &ngfAPI.ProxySettingsPolicy{ + Spec: ngfAPI.ProxySettingsPolicySpec{ + Buffering: &ngfAPI.ProxyBuffering{ + Buffers: &ngfAPI.ProxyBuffers{Number: 16, Size: "8k"}, + }, + }, + }, + conflicts: true, + }, + { + name: "busy buffers size conflicts", + polA: createValidPolicy(), + polB: &ngfAPI.ProxySettingsPolicy{ + Spec: ngfAPI.ProxySettingsPolicySpec{ + Buffering: &ngfAPI.ProxyBuffering{ + BusyBuffersSize: helpers.GetPointer[ngfAPI.Size]("64k"), + }, + }, + }, + conflicts: true, + }, + } + + v := proxysettings.NewValidator(nil) + + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + t.Parallel() + g := NewWithT(t) + + g.Expect(v.Conflicts(test.polA, test.polB)).To(Equal(test.conflicts)) + }) + } +} + +func TestValidator_ConflictsPanics(t *testing.T) { + t.Parallel() + v := proxysettings.NewValidator(nil) + + conflicts := func() { + _ = v.Conflicts(&policiesfakes.FakePolicy{}, &policiesfakes.FakePolicy{}) + } + + g := NewWithT(t) + + g.Expect(conflicts).To(Panic()) +} diff --git a/internal/controller/state/change_processor.go b/internal/controller/state/change_processor.go index d661903b8c..6d38cba175 100644 --- a/internal/controller/state/change_processor.go +++ b/internal/controller/state/change_processor.go @@ -225,6 +225,11 @@ func NewChangeProcessorImpl(cfg ChangeProcessorConfig) *ChangeProcessorImpl { store: newObjectStoreMapAdapter(clusterStore.SnippetsFilters), predicate: nil, // we always want to write status to SnippetsFilters so we don't filter them out }, + { + gvk: cfg.MustExtractGVK(&ngfAPIv1alpha1.ProxySettingsPolicy{}), + store: commonPolicyObjectStore, + predicate: funcPredicate{stateChanged: isNGFPolicyRelevant}, + }, }, ) diff --git a/internal/controller/state/change_processor_test.go b/internal/controller/state/change_processor_test.go index 17af9a4a84..3d7fea3d1f 100644 --- a/internal/controller/state/change_processor_test.go +++ b/internal/controller/state/change_processor_test.go @@ -2941,13 +2941,14 @@ var _ = Describe("ChangeProcessor", func() { Describe("NGF Policy resource changes", Ordered, func() { var ( - gw *v1.Gateway - route *v1.HTTPRoute - svc *apiv1.Service - csp, cspUpdated *ngfAPIv1alpha1.ClientSettingsPolicy - obs, obsUpdated *ngfAPIv1alpha2.ObservabilityPolicy - usp, uspUpdated *ngfAPIv1alpha1.UpstreamSettingsPolicy - cspKey, obsKey, uspKey graph.PolicyKey + gw *v1.Gateway + route *v1.HTTPRoute + svc *apiv1.Service + csp, cspUpdated *ngfAPIv1alpha1.ClientSettingsPolicy + obs, obsUpdated *ngfAPIv1alpha2.ObservabilityPolicy + usp, uspUpdated *ngfAPIv1alpha1.UpstreamSettingsPolicy + psp, pspUpdated *ngfAPIv1alpha1.ProxySettingsPolicy + cspKey, obsKey, uspKey, pspKey graph.PolicyKey ) BeforeAll(func() { @@ -3069,6 +3070,37 @@ var _ = Describe("ChangeProcessor", func() { Version: "v1alpha1", }, } + + psp = &ngfAPIv1alpha1.ProxySettingsPolicy{ + ObjectMeta: metav1.ObjectMeta{ + Name: "psp", + Namespace: "test", + }, + Spec: ngfAPIv1alpha1.ProxySettingsPolicySpec{ + TargetRefs: []v1.LocalPolicyTargetReference{ + { + Group: v1.GroupName, + Kind: kinds.Gateway, + Name: "gw", + }, + }, + Buffering: &ngfAPIv1alpha1.ProxyBuffering{ + BufferSize: helpers.GetPointer[ngfAPIv1alpha1.Size]("8k"), + }, + }, + } + + pspUpdated = psp.DeepCopy() + pspUpdated.Spec.Buffering.BufferSize = helpers.GetPointer[ngfAPIv1alpha1.Size]("16k") + + pspKey = graph.PolicyKey{ + NsName: types.NamespacedName{Name: "psp", Namespace: "test"}, + GVK: schema.GroupVersionKind{ + Group: ngfAPIv1alpha1.GroupName, + Kind: kinds.ProxySettingsPolicy, + Version: "v1alpha1", + }, + } }) /* @@ -3082,6 +3114,7 @@ var _ = Describe("ChangeProcessor", func() { processor.CaptureUpsertChange(csp) processor.CaptureUpsertChange(obs) processor.CaptureUpsertChange(usp) + processor.CaptureUpsertChange(psp) Expect(processor.Process()).To(BeNil()) }) @@ -3094,6 +3127,8 @@ var _ = Describe("ChangeProcessor", func() { Expect(graph).ToNot(BeNil()) Expect(graph.NGFPolicies).To(HaveKey(cspKey)) Expect(graph.NGFPolicies[cspKey].Source).To(Equal(csp)) + Expect(graph.NGFPolicies).To(HaveKey(pspKey)) + Expect(graph.NGFPolicies[pspKey].Source).To(Equal(psp)) Expect(graph.NGFPolicies).ToNot(HaveKey(obsKey)) processor.CaptureUpsertChange(route) @@ -3114,6 +3149,7 @@ var _ = Describe("ChangeProcessor", func() { processor.CaptureUpsertChange(cspUpdated) processor.CaptureUpsertChange(obsUpdated) processor.CaptureUpsertChange(uspUpdated) + processor.CaptureUpsertChange(pspUpdated) graph := processor.Process() Expect(graph).ToNot(BeNil()) @@ -3123,6 +3159,8 @@ var _ = Describe("ChangeProcessor", func() { Expect(graph.NGFPolicies[obsKey].Source).To(Equal(obsUpdated)) Expect(graph.NGFPolicies).To(HaveKey(uspKey)) Expect(graph.NGFPolicies[uspKey].Source).To(Equal(uspUpdated)) + Expect(graph.NGFPolicies).To(HaveKey(pspKey)) + Expect(graph.NGFPolicies[pspKey].Source).To(Equal(pspUpdated)) }) }) When("the policy is deleted", func() { @@ -3130,6 +3168,7 @@ var _ = Describe("ChangeProcessor", func() { processor.CaptureDeleteChange(&ngfAPIv1alpha1.ClientSettingsPolicy{}, client.ObjectKeyFromObject(csp)) processor.CaptureDeleteChange(&ngfAPIv1alpha2.ObservabilityPolicy{}, client.ObjectKeyFromObject(obs)) processor.CaptureDeleteChange(&ngfAPIv1alpha1.UpstreamSettingsPolicy{}, client.ObjectKeyFromObject(usp)) + processor.CaptureDeleteChange(&ngfAPIv1alpha1.ProxySettingsPolicy{}, client.ObjectKeyFromObject(psp)) graph := processor.Process() Expect(graph).ToNot(BeNil()) diff --git a/internal/controller/state/dataplane/configuration.go b/internal/controller/state/dataplane/configuration.go index 26bc3c58ee..3b15e8a1b1 100644 --- a/internal/controller/state/dataplane/configuration.go +++ b/internal/controller/state/dataplane/configuration.go @@ -1064,6 +1064,7 @@ func buildBaseHTTPConfig( // HTTP2 should be enabled by default HTTP2: true, IPFamily: Dual, + Policies: buildPolicies(gateway, gateway.Policies), Snippets: buildSnippetsForContext(gatewaySnippetsFilters, ngfAPIv1alpha1.NginxContextHTTP), NginxReadinessProbePort: DefaultNginxReadinessProbePort, } diff --git a/internal/controller/state/dataplane/configuration_test.go b/internal/controller/state/dataplane/configuration_test.go index 330a0cef79..55305baec8 100644 --- a/internal/controller/state/dataplane/configuration_test.go +++ b/internal/controller/state/dataplane/configuration_test.go @@ -2123,6 +2123,7 @@ func TestBuildConfiguration(t *testing.T) { return g }), expConf: getModifiedExpectedConfiguration(func(conf Configuration) Configuration { + conf.BaseHTTPConfig.Policies = []policies.Policy{gwPolicy1.Source, gwPolicy2.Source} conf.SSLServers = []VirtualServer{ { IsDefault: true, @@ -2208,6 +2209,7 @@ func TestBuildConfiguration(t *testing.T) { return g }), expConf: getModifiedExpectedConfiguration(func(conf Configuration) Configuration { + conf.BaseHTTPConfig.Policies = []policies.Policy{gwPolicy1.Source, gwPolicy2.Source} conf.SSLServers = []VirtualServer{} conf.SSLKeyPairs = map[SSLKeyPairID]SSLKeyPair{} conf.HTTPServers = []VirtualServer{ diff --git a/internal/controller/state/dataplane/types.go b/internal/controller/state/dataplane/types.go index 1b0ccbde6a..d82580a131 100644 --- a/internal/controller/state/dataplane/types.go +++ b/internal/controller/state/dataplane/types.go @@ -395,6 +395,8 @@ type BaseHTTPConfig struct { IPFamily IPFamilyType // GatewaySecretID is the ID of the secret that contains the gateway backend TLS certificate. GatewaySecretID SSLKeyPairID + // Policies holds the policies attached to the Gateway for the http context. + Policies []policies.Policy // Snippets contain the snippets that apply to the http context. Snippets []Snippet // RewriteIPSettings defines configuration for rewriting the client IP to the original client's IP. diff --git a/internal/framework/kinds/kinds.go b/internal/framework/kinds/kinds.go index b59b06df96..dccbceaf76 100644 --- a/internal/framework/kinds/kinds.go +++ b/internal/framework/kinds/kinds.go @@ -45,6 +45,8 @@ const ( ObservabilityPolicy = "ObservabilityPolicy" // NginxProxy is the NginxProxy kind. NginxProxy = "NginxProxy" + // ProxySettingsPolicy is the ProxySettingsPolicy kind. + ProxySettingsPolicy = "ProxySettingsPolicy" // SnippetsFilter is the SnippetsFilter kind. SnippetsFilter = "SnippetsFilter" // UpstreamSettingsPolicy is the UpstreamSettingsPolicy kind. From 87a033cfd87036ee04ec572002e645e01c68143f Mon Sep 17 00:00:00 2001 From: Ciara Stacke <18287516+ciarams87@users.noreply.github.com> Date: Fri, 14 Nov 2025 11:21:08 +0000 Subject: [PATCH 2/3] Implement policy affected status --- .../controller/state/conditions/conditions.go | 17 ++++++++++++++++- internal/controller/state/graph/policies.go | 5 +++++ 2 files changed, 21 insertions(+), 1 deletion(-) diff --git a/internal/controller/state/conditions/conditions.go b/internal/controller/state/conditions/conditions.go index 8e186fe1a4..897e11ff78 100644 --- a/internal/controller/state/conditions/conditions.go +++ b/internal/controller/state/conditions/conditions.go @@ -127,8 +127,12 @@ const ( // ObservabilityPolicy is applied to a HTTPRoute, or GRPCRoute. ObservabilityPolicyAffected v1.PolicyConditionType = "ObservabilityPolicyAffected" + // ProxySettingsPolicyAffected is used with the "PolicyAffected" condition when a + // ProxySettingsPolicy is applied to a Gateway, HTTPRoute, or GRPCRoute. + ProxySettingsPolicyAffected v1.PolicyConditionType = "ProxySettingsPolicyAffected" + // PolicyAffectedReason is used with the "PolicyAffected" condition when a - // ObservabilityPolicy or ClientSettingsPolicy is applied to Gateways or Routes. + // ObservabilityPolicy, ClientSettingsPolicy, or ProxySettingsPolicy is applied to Gateways or Routes. PolicyAffectedReason v1.PolicyConditionReason = "PolicyAffected" // GatewayResolvedRefs condition indicates whether the controller was able to resolve the @@ -1145,6 +1149,17 @@ func NewClientSettingsPolicyAffected() Condition { } } +// NewProxySettingsPolicyAffected returns a Condition that indicates that a ProxySettingsPolicy +// is applied to the resource. +func NewProxySettingsPolicyAffected() Condition { + return Condition{ + Type: string(ProxySettingsPolicyAffected), + Status: metav1.ConditionTrue, + Reason: string(PolicyAffectedReason), + Message: "The ProxySettingsPolicy is applied to the resource", + } +} + // NewBackendTLSPolicyResolvedRefs returns a Condition that indicates that all CACertificateRefs // in the BackendTLSPolicy are resolved. func NewBackendTLSPolicyResolvedRefs() Condition { diff --git a/internal/controller/state/graph/policies.go b/internal/controller/state/graph/policies.go index 30e44fb014..6c6171e618 100644 --- a/internal/controller/state/graph/policies.go +++ b/internal/controller/state/graph/policies.go @@ -637,5 +637,10 @@ func addStatusToTargetRefs(policyKind string, conditionsList *[]conditions.Condi return } *conditionsList = append(*conditionsList, conditions.NewClientSettingsPolicyAffected()) + case kinds.ProxySettingsPolicy: + if conditions.HasMatchingCondition(*conditionsList, conditions.NewProxySettingsPolicyAffected()) { + return + } + *conditionsList = append(*conditionsList, conditions.NewProxySettingsPolicyAffected()) } } From 469d8e9f0329ab185a72b29c33093168b4110c25 Mon Sep 17 00:00:00 2001 From: Ciara Stacke <18287516+ciarams87@users.noreply.github.com> Date: Wed, 19 Nov 2025 14:01:59 +0000 Subject: [PATCH 3/3] Add unit test for ParseNginxSize --- .../policies/proxysettings/validator.go | 10 +-- .../policies/proxysettings/validator_test.go | 74 +++++++++++++++++++ 2 files changed, 79 insertions(+), 5 deletions(-) diff --git a/internal/controller/nginx/config/policies/proxysettings/validator.go b/internal/controller/nginx/config/policies/proxysettings/validator.go index 8d3615b77b..9387c331a4 100644 --- a/internal/controller/nginx/config/policies/proxysettings/validator.go +++ b/internal/controller/nginx/config/policies/proxysettings/validator.go @@ -130,14 +130,14 @@ func validateBusyBufferSizeRelationships(buffering ngfAPI.ProxyBuffering, fieldP return nil } - busyBuffersSize, err := parseNginxSize(string(*buffering.BusyBuffersSize)) + busyBuffersSize, err := ParseNginxSize(string(*buffering.BusyBuffersSize)) if err != nil { return nil // Skip validation if size is invalid (will be caught by other validation) } // Validate: busyBuffersSize > bufferSize if buffering.BufferSize != nil { - bufferSize, err := parseNginxSize(string(*buffering.BufferSize)) + bufferSize, err := ParseNginxSize(string(*buffering.BufferSize)) if err == nil && busyBuffersSize <= bufferSize { path := fieldPath.Child("busyBuffersSize") allErrs = append(allErrs, field.Invalid( @@ -150,7 +150,7 @@ func validateBusyBufferSizeRelationships(buffering ngfAPI.ProxyBuffering, fieldP // Validate: busyBuffersSize < (buffers.number * buffers.size) - buffers.size if buffering.Buffers != nil { - buffersSize, err := parseNginxSize(string(buffering.Buffers.Size)) + buffersSize, err := ParseNginxSize(string(buffering.Buffers.Size)) if err == nil { totalBufferSpace := buffersSize * int64(buffering.Buffers.Number) maxBusyBuffersSize := totalBufferSpace - buffersSize @@ -168,9 +168,9 @@ func validateBusyBufferSizeRelationships(buffering ngfAPI.ProxyBuffering, fieldP return allErrs } -// parseNginxSize parses an NGINX size string (e.g., "8k", "16m", "1024") and returns the size in bytes. +// ParseNginxSize parses an NGINX size string (e.g., "8k", "16m", "1024") and returns the size in bytes. // Returns an error if the size string is invalid. -func parseNginxSize(size string) (int64, error) { +func ParseNginxSize(size string) (int64, error) { size = strings.TrimSpace(strings.ToLower(size)) if size == "" { return 0, strconv.ErrSyntax diff --git a/internal/controller/nginx/config/policies/proxysettings/validator_test.go b/internal/controller/nginx/config/policies/proxysettings/validator_test.go index 4fc79a3fff..6a8cfdb4f0 100644 --- a/internal/controller/nginx/config/policies/proxysettings/validator_test.go +++ b/internal/controller/nginx/config/policies/proxysettings/validator_test.go @@ -310,3 +310,77 @@ func TestValidator_ConflictsPanics(t *testing.T) { g.Expect(conflicts).To(Panic()) } + +func TestParseNginxSize(t *testing.T) { + t.Parallel() + tests := []struct { + name string + input string + expectedBytes int64 + expectError bool + }{ + { + name: "bytes without unit", + input: "1024", + expectedBytes: 1024, + expectError: false, + }, + { + name: "kilobytes", + input: "8k", + expectedBytes: 8 * 1024, + expectError: false, + }, + { + name: "megabytes", + input: "16m", + expectedBytes: 16 * 1024 * 1024, + expectError: false, + }, + { + name: "gigabytes", + input: "2g", + expectedBytes: 2 * 1024 * 1024 * 1024, + expectError: false, + }, + { + name: "single digit", + input: "4", + expectedBytes: 4, + expectError: false, + }, + { + name: "four digits maximum", + input: "9999", + expectedBytes: 9999, + expectError: false, + }, + { + name: "four digits with unit", + input: "1024k", + expectedBytes: 1024 * 1024, + expectError: false, + }, + { + name: "invalid input - non-numeric", + input: "abc", + expectError: true, + }, + } + + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + t.Parallel() + g := NewWithT(t) + + result, err := proxysettings.ParseNginxSize(test.input) + + if test.expectError { + g.Expect(err).To(HaveOccurred()) + } else { + g.Expect(err).ToNot(HaveOccurred()) + g.Expect(result).To(Equal(test.expectedBytes)) + } + }) + } +}