From 72d840bb86dfa0e63c2eeebcf632a50e3f98a641 Mon Sep 17 00:00:00 2001 From: Saloni Date: Wed, 21 Aug 2024 13:13:12 -0600 Subject: [PATCH] fix CRD and unit tests --- apis/v1alpha1/nginxproxy_types.go | 25 ++- charts/nginx-gateway-fabric/README.md | 8 +- charts/nginx-gateway-fabric/values.yaml | 11 +- .../bases/gateway.nginx.org_nginxproxies.yaml | 25 ++- config/tests/static-deployment.yaml | 4 +- deploy/aws-nlb/deploy.yaml | 4 +- deploy/azure/deploy.yaml | 4 +- deploy/crds.yaml | 25 ++- deploy/default/deploy.yaml | 4 +- deploy/experimental-nginx-plus/deploy.yaml | 4 +- deploy/experimental/deploy.yaml | 4 +- deploy/nginx-plus/deploy.yaml | 4 +- deploy/nodeport/deploy.yaml | 4 +- deploy/openshift/deploy.yaml | 4 +- internal/mode/static/nginx/config/servers.go | 39 ++++- .../mode/static/nginx/config/servers_test.go | 151 +++++++++++++++++- .../state/dataplane/configuration_test.go | 22 +++ internal/mode/static/state/dataplane/types.go | 2 +- .../mode/static/state/graph/nginxproxy.go | 12 +- .../static/state/graph/nginxproxy_test.go | 46 ++++-- .../how-to/monitoring/troubleshooting.md | 6 +- site/content/reference/api.md | 23 ++- 22 files changed, 349 insertions(+), 82 deletions(-) diff --git a/apis/v1alpha1/nginxproxy_types.go b/apis/v1alpha1/nginxproxy_types.go index 5142f3a250..a05a73d32f 100644 --- a/apis/v1alpha1/nginxproxy_types.go +++ b/apis/v1alpha1/nginxproxy_types.go @@ -124,15 +124,23 @@ type TelemetryExporter struct { // RewriteClientIP specifies the configuration for rewriting the client's IP address. type RewriteClientIP struct { // Mode defines how NGINX will rewrite the client's IP address. - // Possible modes: ProxyProtocol, XForwardedFor. + // There are two possible modes: + // - ProxyProtocol: NGINX will rewrite the client's IP using the PROXY protocol header. + // - XForwardedFor: NGINX will rewrite the client's IP using the X-Forwarded-For header. + // Sets NGINX directive real_ip_header: https://nginx.org/en/docs/http/ngx_http_realip_module.html#real_ip_header // // +optional Mode *RewriteClientIPModeType `json:"mode,omitempty"` - // SetIPRecursively configures whether recursive search is used for selecting client's - // address from the X-Forwarded-For header and used in conjunction with TrustedAddresses. - // If enabled, NGINX will recurse on the values in X-Forwarded-Header from the end of - // array to start of array and select the first untrusted IP. + // SetIPRecursively configures whether recursive search is used when selecting the client's address from. + // the X-Forwarded-For header. It is used in conjunction with TrustedAddresses. + // If enabled, NGINX will recurse on the values in X-Forwarded-Header from the end of array + // to start of array and select the first untrusted IP. + // For example, if X-Forwarded-For is [11.11.11.11, 22.22.22.22, 55.55.55.1], + // and TrustedAddresses is set to 55.55.55.1/0, NGINX will rewrite the client IP to 22.22.22.22. + // If disabled, NGINX will select the IP at the end of the array. + // In the previous example, 55.55.55.1 would be selected. + // Sets NGINX directive real_ip_recursive: https://nginx.org/en/docs/http/ngx_http_realip_module.html#real_ip_recursive // // +optional SetIPRecursively *bool `json:"setIPRecursively,omitempty"` @@ -140,9 +148,14 @@ type RewriteClientIP struct { // TrustedAddresses specifies the addresses that are trusted to send correct client IP information. // If a request comes from a trusted address, NGINX will rewrite the client IP information, // and forward it to the backend in the X-Forwarded-For* and X-Real-IP headers. + // If the request does not come from a trusted address, NGINX will not rewrite the client IP information. + // Addresses must be provided as CIDR blocks: 10.0.0.0/32, 192.33.21/0. + // To trust all addresses (not recommended), set to 0.0.0.0/0. + // If no addresses are provided, NGINX will not rewrite the client IP information. + // Sets NGINX directive set_real_ip_from: https://nginx.org/en/docs/http/ngx_http_realip_module.html#set_real_ip_from // This field is required if mode is set. // +kubebuilder:validation:MaxItems=16 - // +listType=atomic + // +listType=set // // // +optional diff --git a/charts/nginx-gateway-fabric/README.md b/charts/nginx-gateway-fabric/README.md index 3f67c3e2e2..9ca6cb9073 100644 --- a/charts/nginx-gateway-fabric/README.md +++ b/charts/nginx-gateway-fabric/README.md @@ -263,8 +263,8 @@ The following table lists the configurable parameters of the NGINX Gateway Fabri | `nginx.config` | The configuration for the data plane that is contained in the NginxProxy resource. | object | `{}` | | `nginx.extraVolumeMounts` | extraVolumeMounts are the additional volume mounts for the nginx container. | list | `[]` | | `nginx.image.pullPolicy` | | string | `"Always"` | -| `nginx.image.repository` | The NGINX image to use. | string | `"gcr.io/f5-gcs-7899-ptg-ingrss-ctlr/nginx-gateway-fabric/nginx"` | -| `nginx.image.tag` | | string | `"sa.choudhary"` | +| `nginx.image.repository` | The NGINX image to use. | string | `"ghcr.io/nginxinc/nginx-gateway-fabric/nginx"` | +| `nginx.image.tag` | | string | `"edge"` | | `nginx.lifecycle` | The lifecycle of the nginx container. | object | `{}` | | `nginx.plus` | Is NGINX Plus image being used | bool | `false` | | `nginx.usage.clusterName` | The display name of the Kubernetes cluster in the NGINX Plus usage reporting server. | string | `""` | @@ -279,8 +279,8 @@ The following table lists the configurable parameters of the NGINX Gateway Fabri | `nginxGateway.gatewayControllerName` | The name of the Gateway controller. The controller name must be of the form: DOMAIN/PATH. The controller's domain is gateway.nginx.org. | string | `"gateway.nginx.org/nginx-gateway-controller"` | | `nginxGateway.gwAPIExperimentalFeatures.enable` | Enable the experimental features of Gateway API which are supported by NGINX Gateway Fabric. Requires the Gateway APIs installed from the experimental channel. | bool | `false` | | `nginxGateway.image.pullPolicy` | | string | `"Always"` | -| `nginxGateway.image.repository` | The NGINX Gateway Fabric image to use | string | `"gcr.io/f5-gcs-7899-ptg-ingrss-ctlr/nginx-gateway-fabric"` | -| `nginxGateway.image.tag` | | string | `"sa.choudhary"` | +| `nginxGateway.image.repository` | The NGINX Gateway Fabric image to use | string | `"ghcr.io/nginxinc/nginx-gateway-fabric"` | +| `nginxGateway.image.tag` | | string | `"edge"` | | `nginxGateway.kind` | The kind of the NGINX Gateway Fabric installation - currently, only deployment is supported. | string | `"deployment"` | | `nginxGateway.leaderElection.enable` | Enable leader election. Leader election is used to avoid multiple replicas of the NGINX Gateway Fabric reporting the status of the Gateway API resources. If not enabled, all replicas of NGINX Gateway Fabric will update the statuses of the Gateway API resources. | bool | `true` | | `nginxGateway.leaderElection.lockName` | The name of the leader election lock. A Lease object with this name will be created in the same Namespace as the controller. | string | Autogenerated if not set or set to "". | diff --git a/charts/nginx-gateway-fabric/values.yaml b/charts/nginx-gateway-fabric/values.yaml index ef061515f9..9c14104dd3 100644 --- a/charts/nginx-gateway-fabric/values.yaml +++ b/charts/nginx-gateway-fabric/values.yaml @@ -52,8 +52,8 @@ nginxGateway: image: # -- The NGINX Gateway Fabric image to use - repository: gcr.io/f5-gcs-7899-ptg-ingrss-ctlr/nginx-gateway-fabric - tag: sa.choudhary + repository: ghcr.io/nginxinc/nginx-gateway-fabric + tag: edge pullPolicy: Always securityContext: @@ -81,8 +81,8 @@ nginxGateway: nginx: image: # -- The NGINX image to use. - repository: gcr.io/f5-gcs-7899-ptg-ingrss-ctlr/nginx-gateway-fabric/nginx - tag: sa.choudhary + repository: ghcr.io/nginxinc/nginx-gateway-fabric/nginx + tag: edge pullPolicy: Always # -- Is NGINX Plus image being used @@ -95,7 +95,8 @@ nginx: # ipFamily: dual # rewriteClientIP: # mode: "ProxyProtocol" - # trustedAddresses: ["0.0.0.0/0"] + # # -- The trusted addresses field needs to be replaced with the load balancer's IP address. + # trustedAddresses: [] # setIPRecursively: true # telemetry: # exporter: diff --git a/config/crd/bases/gateway.nginx.org_nginxproxies.yaml b/config/crd/bases/gateway.nginx.org_nginxproxies.yaml index a8f706b244..1656baee93 100644 --- a/config/crd/bases/gateway.nginx.org_nginxproxies.yaml +++ b/config/crd/bases/gateway.nginx.org_nginxproxies.yaml @@ -69,23 +69,36 @@ spec: mode: description: |- Mode defines how NGINX will rewrite the client's IP address. - Possible modes: ProxyProtocol, XForwardedFor. + There are two possible modes: + - ProxyProtocol: NGINX will rewrite the client's IP using the PROXY protocol header. + - XForwardedFor: NGINX will rewrite the client's IP using the X-Forwarded-For header. + Sets NGINX directive real_ip_header: https://nginx.org/en/docs/http/ngx_http_realip_module.html#real_ip_header enum: - ProxyProtocol - XForwardedFor type: string setIPRecursively: description: |- - SetIPRecursively configures whether recursive search is used for selecting client's - address from the X-Forwarded-For header and used in conjunction with TrustedAddresses. - If enabled, NGINX will recurse on the values in X-Forwarded-Header from the end of - array to start of array and select the first untrusted IP. + SetIPRecursively configures whether recursive search is used when selecting the client's address from. + the X-Forwarded-For header. It is used in conjunction with TrustedAddresses. + If enabled, NGINX will recurse on the values in X-Forwarded-Header from the end of array + to start of array and select the first untrusted IP. + For example, if X-Forwarded-For is [11.11.11.11, 22.22.22.22, 55.55.55.1], + and TrustedAddresses is set to 55.55.55.1/0, NGINX will rewrite the client IP to 22.22.22.22. + If disabled, NGINX will select the IP at the end of the array. + In the previous example, 55.55.55.1 would be selected. + Sets NGINX directive real_ip_recursive: https://nginx.org/en/docs/http/ngx_http_realip_module.html#real_ip_recursive type: boolean trustedAddresses: description: |- TrustedAddresses specifies the addresses that are trusted to send correct client IP information. If a request comes from a trusted address, NGINX will rewrite the client IP information, and forward it to the backend in the X-Forwarded-For* and X-Real-IP headers. + If the request does not come from a trusted address, NGINX will not rewrite the client IP information. + Addresses must be provided as CIDR blocks: 10.0.0.0/32, 192.33.21/0. + To trust all addresses (not recommended), set to 0.0.0.0/0. + If no addresses are provided, NGINX will not rewrite the client IP information. + Sets NGINX directive set_real_ip_from: https://nginx.org/en/docs/http/ngx_http_realip_module.html#set_real_ip_from This field is required if mode is set. items: description: |- @@ -95,7 +108,7 @@ spec: type: string maxItems: 16 type: array - x-kubernetes-list-type: atomic + x-kubernetes-list-type: set type: object x-kubernetes-validations: - message: if mode is set, trustedAddresses is a required field diff --git a/config/tests/static-deployment.yaml b/config/tests/static-deployment.yaml index bab7329dd2..bb2fb62765 100644 --- a/config/tests/static-deployment.yaml +++ b/config/tests/static-deployment.yaml @@ -45,7 +45,7 @@ spec: valueFrom: fieldRef: fieldPath: metadata.name - image: gcr.io/f5-gcs-7899-ptg-ingrss-ctlr/nginx-gateway-fabric:sa.choudhary + image: ghcr.io/nginxinc/nginx-gateway-fabric:edge imagePullPolicy: Always name: nginx-gateway ports: @@ -82,7 +82,7 @@ spec: mountPath: /var/run/nginx - name: nginx-includes mountPath: /etc/nginx/includes - - image: gcr.io/f5-gcs-7899-ptg-ingrss-ctlr/nginx-gateway-fabric/nginx:sa.choudhary + - image: ghcr.io/nginxinc/nginx-gateway-fabric/nginx:edge imagePullPolicy: Always name: nginx ports: diff --git a/deploy/aws-nlb/deploy.yaml b/deploy/aws-nlb/deploy.yaml index 588c5c5671..49b29bf988 100644 --- a/deploy/aws-nlb/deploy.yaml +++ b/deploy/aws-nlb/deploy.yaml @@ -217,7 +217,7 @@ spec: valueFrom: fieldRef: fieldPath: metadata.name - image: gcr.io/f5-gcs-7899-ptg-ingrss-ctlr/nginx-gateway-fabric:sa.choudhary + image: ghcr.io/nginxinc/nginx-gateway-fabric:edge imagePullPolicy: Always name: nginx-gateway ports: @@ -256,7 +256,7 @@ spec: name: nginx-run - mountPath: /etc/nginx/includes name: nginx-includes - - image: gcr.io/f5-gcs-7899-ptg-ingrss-ctlr/nginx-gateway-fabric/nginx:sa.choudhary + - image: ghcr.io/nginxinc/nginx-gateway-fabric/nginx:edge imagePullPolicy: Always name: nginx ports: diff --git a/deploy/azure/deploy.yaml b/deploy/azure/deploy.yaml index b61a1363d2..968c1a2926 100644 --- a/deploy/azure/deploy.yaml +++ b/deploy/azure/deploy.yaml @@ -214,7 +214,7 @@ spec: valueFrom: fieldRef: fieldPath: metadata.name - image: gcr.io/f5-gcs-7899-ptg-ingrss-ctlr/nginx-gateway-fabric:sa.choudhary + image: ghcr.io/nginxinc/nginx-gateway-fabric:edge imagePullPolicy: Always name: nginx-gateway ports: @@ -253,7 +253,7 @@ spec: name: nginx-run - mountPath: /etc/nginx/includes name: nginx-includes - - image: gcr.io/f5-gcs-7899-ptg-ingrss-ctlr/nginx-gateway-fabric/nginx:sa.choudhary + - image: ghcr.io/nginxinc/nginx-gateway-fabric/nginx:edge imagePullPolicy: Always name: nginx ports: diff --git a/deploy/crds.yaml b/deploy/crds.yaml index f150c34d06..ff60b15b55 100644 --- a/deploy/crds.yaml +++ b/deploy/crds.yaml @@ -654,23 +654,36 @@ spec: mode: description: |- Mode defines how NGINX will rewrite the client's IP address. - Possible modes: ProxyProtocol, XForwardedFor. + There are two possible modes: + - ProxyProtocol: NGINX will rewrite the client's IP using the PROXY protocol header. + - XForwardedFor: NGINX will rewrite the client's IP using the X-Forwarded-For header. + Sets NGINX directive real_ip_header: https://nginx.org/en/docs/http/ngx_http_realip_module.html#real_ip_header enum: - ProxyProtocol - XForwardedFor type: string setIPRecursively: description: |- - SetIPRecursively configures whether recursive search is used for selecting client's - address from the X-Forwarded-For header and used in conjunction with TrustedAddresses. - If enabled, NGINX will recurse on the values in X-Forwarded-Header from the end of - array to start of array and select the first untrusted IP. + SetIPRecursively configures whether recursive search is used when selecting the client's address from. + the X-Forwarded-For header. It is used in conjunction with TrustedAddresses. + If enabled, NGINX will recurse on the values in X-Forwarded-Header from the end of array + to start of array and select the first untrusted IP. + For example, if X-Forwarded-For is [11.11.11.11, 22.22.22.22, 55.55.55.1], + and TrustedAddresses is set to 55.55.55.1/0, NGINX will rewrite the client IP to 22.22.22.22. + If disabled, NGINX will select the IP at the end of the array. + In the previous example, 55.55.55.1 would be selected. + Sets NGINX directive real_ip_recursive: https://nginx.org/en/docs/http/ngx_http_realip_module.html#real_ip_recursive type: boolean trustedAddresses: description: |- TrustedAddresses specifies the addresses that are trusted to send correct client IP information. If a request comes from a trusted address, NGINX will rewrite the client IP information, and forward it to the backend in the X-Forwarded-For* and X-Real-IP headers. + If the request does not come from a trusted address, NGINX will not rewrite the client IP information. + Addresses must be provided as CIDR blocks: 10.0.0.0/32, 192.33.21/0. + To trust all addresses (not recommended), set to 0.0.0.0/0. + If no addresses are provided, NGINX will not rewrite the client IP information. + Sets NGINX directive set_real_ip_from: https://nginx.org/en/docs/http/ngx_http_realip_module.html#set_real_ip_from This field is required if mode is set. items: description: |- @@ -680,7 +693,7 @@ spec: type: string maxItems: 16 type: array - x-kubernetes-list-type: atomic + x-kubernetes-list-type: set type: object x-kubernetes-validations: - message: if mode is set, trustedAddresses is a required field diff --git a/deploy/default/deploy.yaml b/deploy/default/deploy.yaml index 3e38b84f62..6245a2bbc7 100644 --- a/deploy/default/deploy.yaml +++ b/deploy/default/deploy.yaml @@ -214,7 +214,7 @@ spec: valueFrom: fieldRef: fieldPath: metadata.name - image: gcr.io/f5-gcs-7899-ptg-ingrss-ctlr/nginx-gateway-fabric:sa.choudhary + image: ghcr.io/nginxinc/nginx-gateway-fabric:edge imagePullPolicy: Always name: nginx-gateway ports: @@ -253,7 +253,7 @@ spec: name: nginx-run - mountPath: /etc/nginx/includes name: nginx-includes - - image: gcr.io/f5-gcs-7899-ptg-ingrss-ctlr/nginx-gateway-fabric/nginx:sa.choudhary + - image: ghcr.io/nginxinc/nginx-gateway-fabric/nginx:edge imagePullPolicy: Always name: nginx ports: diff --git a/deploy/experimental-nginx-plus/deploy.yaml b/deploy/experimental-nginx-plus/deploy.yaml index ae6b2c6191..ed9c748e1a 100644 --- a/deploy/experimental-nginx-plus/deploy.yaml +++ b/deploy/experimental-nginx-plus/deploy.yaml @@ -229,7 +229,7 @@ spec: valueFrom: fieldRef: fieldPath: metadata.name - image: gcr.io/f5-gcs-7899-ptg-ingrss-ctlr/nginx-gateway-fabric:sa.choudhary + image: ghcr.io/nginxinc/nginx-gateway-fabric:edge imagePullPolicy: Always name: nginx-gateway ports: @@ -268,7 +268,7 @@ spec: name: nginx-run - mountPath: /etc/nginx/includes name: nginx-includes - - image: private-registry.nginx.com/nginx-gateway-fabric/nginx-plus:sa.choudhary + - image: private-registry.nginx.com/nginx-gateway-fabric/nginx-plus:edge imagePullPolicy: Always name: nginx ports: diff --git a/deploy/experimental/deploy.yaml b/deploy/experimental/deploy.yaml index f67a7a0ab6..28cc7b6d19 100644 --- a/deploy/experimental/deploy.yaml +++ b/deploy/experimental/deploy.yaml @@ -220,7 +220,7 @@ spec: valueFrom: fieldRef: fieldPath: metadata.name - image: gcr.io/f5-gcs-7899-ptg-ingrss-ctlr/nginx-gateway-fabric:sa.choudhary + image: ghcr.io/nginxinc/nginx-gateway-fabric:edge imagePullPolicy: Always name: nginx-gateway ports: @@ -259,7 +259,7 @@ spec: name: nginx-run - mountPath: /etc/nginx/includes name: nginx-includes - - image: gcr.io/f5-gcs-7899-ptg-ingrss-ctlr/nginx-gateway-fabric/nginx:sa.choudhary + - image: ghcr.io/nginxinc/nginx-gateway-fabric/nginx:edge imagePullPolicy: Always name: nginx ports: diff --git a/deploy/nginx-plus/deploy.yaml b/deploy/nginx-plus/deploy.yaml index 6652561096..76249e80c2 100644 --- a/deploy/nginx-plus/deploy.yaml +++ b/deploy/nginx-plus/deploy.yaml @@ -225,7 +225,7 @@ spec: valueFrom: fieldRef: fieldPath: metadata.name - image: gcr.io/f5-gcs-7899-ptg-ingrss-ctlr/nginx-gateway-fabric:sa.choudhary + image: ghcr.io/nginxinc/nginx-gateway-fabric:edge imagePullPolicy: Always name: nginx-gateway ports: @@ -264,7 +264,7 @@ spec: name: nginx-run - mountPath: /etc/nginx/includes name: nginx-includes - - image: private-registry.nginx.com/nginx-gateway-fabric/nginx-plus:sa.choudhary + - image: private-registry.nginx.com/nginx-gateway-fabric/nginx-plus:edge imagePullPolicy: Always name: nginx ports: diff --git a/deploy/nodeport/deploy.yaml b/deploy/nodeport/deploy.yaml index 132b273db7..db81fdf259 100644 --- a/deploy/nodeport/deploy.yaml +++ b/deploy/nodeport/deploy.yaml @@ -214,7 +214,7 @@ spec: valueFrom: fieldRef: fieldPath: metadata.name - image: gcr.io/f5-gcs-7899-ptg-ingrss-ctlr/nginx-gateway-fabric:sa.choudhary + image: ghcr.io/nginxinc/nginx-gateway-fabric:edge imagePullPolicy: Always name: nginx-gateway ports: @@ -253,7 +253,7 @@ spec: name: nginx-run - mountPath: /etc/nginx/includes name: nginx-includes - - image: gcr.io/f5-gcs-7899-ptg-ingrss-ctlr/nginx-gateway-fabric/nginx:sa.choudhary + - image: ghcr.io/nginxinc/nginx-gateway-fabric/nginx:edge imagePullPolicy: Always name: nginx ports: diff --git a/deploy/openshift/deploy.yaml b/deploy/openshift/deploy.yaml index be969cadc8..cb78ce0f39 100644 --- a/deploy/openshift/deploy.yaml +++ b/deploy/openshift/deploy.yaml @@ -222,7 +222,7 @@ spec: valueFrom: fieldRef: fieldPath: metadata.name - image: gcr.io/f5-gcs-7899-ptg-ingrss-ctlr/nginx-gateway-fabric:sa.choudhary + image: ghcr.io/nginxinc/nginx-gateway-fabric:edge imagePullPolicy: Always name: nginx-gateway ports: @@ -261,7 +261,7 @@ spec: name: nginx-run - mountPath: /etc/nginx/includes name: nginx-includes - - image: gcr.io/f5-gcs-7899-ptg-ingrss-ctlr/nginx-gateway-fabric/nginx:sa.choudhary + - image: ghcr.io/nginxinc/nginx-gateway-fabric/nginx:edge imagePullPolicy: Always name: nginx ports: diff --git a/internal/mode/static/nginx/config/servers.go b/internal/mode/static/nginx/config/servers.go index c3b6e6e299..d0171f4d70 100644 --- a/internal/mode/static/nginx/config/servers.go +++ b/internal/mode/static/nginx/config/servers.go @@ -41,6 +41,22 @@ var httpBaseHeaders = []http.Header{ Name: "Connection", Value: "$connection_upgrade", }, + { + Name: "X-Real-IP", + Value: "$remote_addr", + }, + { + Name: "X-Forwarded-Proto", + Value: "$scheme", + }, + { + Name: "X-Forwarded-Host", + Value: "$host", + }, + { + Name: "X-Forwarded-Port", + Value: "$server_port", + }, } // grpcBaseHeaders contains the constant headers set in each gRPC server block. @@ -57,6 +73,22 @@ var grpcBaseHeaders = []http.Header{ Name: "Authority", Value: "$gw_api_compliant_host", }, + { + Name: "X-Real-IP", + Value: "$remote_addr", + }, + { + Name: "X-Forwarded-Proto", + Value: "$scheme", + }, + { + Name: "X-Forwarded-Host", + Value: "$host", + }, + { + Name: "X-Forwarded-Port", + Value: "$server_port", + }, } func (g GeneratorImpl) newExecuteServersFunc(generator policies.Generator) executeFunc { @@ -878,14 +910,9 @@ func isNonSlashedPrefixPath(pathType dataplane.PathType, path string) bool { // getRewriteClientIPSettings returns the configuration for the rewriting client IP settings. func getRewriteClientIPSettings(rewriteIP dataplane.RewriteClientIPSettings) http.RewriteClientIPSettings { - var proxyProtocol bool - if rewriteIP.Mode == dataplane.RewriteIPModeProxyProtocol { - proxyProtocol = true - } - return http.RewriteClientIPSettings{ Recursive: rewriteIP.IPRecursive, - ProxyProtocol: proxyProtocol, + ProxyProtocol: rewriteIP.Mode == dataplane.RewriteIPModeProxyProtocol, RealIPFrom: rewriteIP.TrustedCIDRs, RealIPHeader: string(rewriteIP.Mode), } diff --git a/internal/mode/static/nginx/config/servers_test.go b/internal/mode/static/nginx/config/servers_test.go index d3ab248f75..ece16a8c03 100644 --- a/internal/mode/static/nginx/config/servers_test.go +++ b/internal/mode/static/nginx/config/servers_test.go @@ -144,7 +144,7 @@ func TestExecuteServers(t *testing.T) { } } -func TestExecuteServerConfig(t *testing.T) { +func TestExecuteServers_IPFamily(t *testing.T) { httpServers := []dataplane.VirtualServer{ { IsDefault: true, @@ -263,11 +263,58 @@ func TestExecuteServerConfig(t *testing.T) { "real_ip_recursive on;": 0, }, }, + } + + for _, test := range tests { + t.Run(test.msg, func(t *testing.T) { + g := NewWithT(t) + + gen := GeneratorImpl{} + results := gen.executeServers(test.config, &policiesfakes.FakeGenerator{}) + g.Expect(results).To(HaveLen(2)) + serverConf := string(results[0].data) + httpMatchConf := string(results[1].data) + g.Expect(httpMatchConf).To(Equal("{}")) + + for expSubStr, expCount := range test.expectedHTTPConfig { + g.Expect(strings.Count(serverConf, expSubStr)).To(Equal(expCount)) + } + }) + } +} + +func TestExecuteServers_RewriteClientIP(t *testing.T) { + tests := []struct { + msg string + expectedHTTPConfig map[string]int + config dataplane.Configuration + }{ { msg: "http and ssl servers with rewrite client IP settings", config: dataplane.Configuration{ - HTTPServers: httpServers, - SSLServers: sslServers, + HTTPServers: []dataplane.VirtualServer{ + { + IsDefault: true, + Port: 8080, + }, + { + Hostname: "example.com", + Port: 8080, + }, + }, + SSLServers: []dataplane.VirtualServer{ + { + IsDefault: true, + Port: 8443, + }, + { + Hostname: "example.com", + SSL: &dataplane.SSL{ + KeyPairID: "test-keypair", + }, + Port: 8443, + }, + }, BaseHTTPConfig: dataplane.BaseHTTPConfig{ IPFamily: dataplane.Dual, RewriteClientIPSettings: dataplane.RewriteClientIPSettings{ @@ -279,7 +326,7 @@ func TestExecuteServerConfig(t *testing.T) { }, expectedHTTPConfig: map[string]int{ "set_real_ip_from 0.0.0.0/0;": 4, - "real_ip_header proxy-protocol;": 4, + "real_ip_header proxy_protocol;": 4, "real_ip_recursive on;": 4, "listen 8080 default_server proxy_protocol;": 1, "listen 8080 proxy_protocol;": 1, @@ -939,6 +986,22 @@ func TestCreateServers(t *testing.T) { Name: "Connection", Value: "$connection_upgrade", }, + { + Name: "X-Real-IP", + Value: "$remote_addr", + }, + { + Name: "X-Forwarded-Proto", + Value: "$scheme", + }, + { + Name: "X-Forwarded-Host", + Value: "$host", + }, + { + Name: "X-Forwarded-Port", + Value: "$server_port", + }, } externalIncludes := []http.Include{ @@ -1206,6 +1269,22 @@ func TestCreateServers(t *testing.T) { Name: "Connection", Value: "$connection_upgrade", }, + { + Name: "X-Real-IP", + Value: "$remote_addr", + }, + { + Name: "X-Forwarded-Proto", + Value: "$scheme", + }, + { + Name: "X-Forwarded-Host", + Value: "$host", + }, + { + Name: "X-Forwarded-Port", + Value: "$server_port", + }, }, ResponseHeaders: http.ResponseHeaders{ Add: []http.Header{ @@ -1244,6 +1323,22 @@ func TestCreateServers(t *testing.T) { Name: "Connection", Value: "$connection_upgrade", }, + { + Name: "X-Real-IP", + Value: "$remote_addr", + }, + { + Name: "X-Forwarded-Proto", + Value: "$scheme", + }, + { + Name: "X-Forwarded-Host", + Value: "$host", + }, + { + Name: "X-Forwarded-Port", + Value: "$server_port", + }, }, ResponseHeaders: http.ResponseHeaders{ Add: []http.Header{ @@ -2419,6 +2514,22 @@ func TestGenerateProxySetHeaders(t *testing.T) { Name: "Connection", Value: "$connection_upgrade", }, + { + Name: "X-Real-IP", + Value: "$remote_addr", + }, + { + Name: "X-Forwarded-Proto", + Value: "$scheme", + }, + { + Name: "X-Forwarded-Host", + Value: "$host", + }, + { + Name: "X-Forwarded-Port", + Value: "$server_port", + }, }, }, { @@ -2457,6 +2568,22 @@ func TestGenerateProxySetHeaders(t *testing.T) { Name: "Connection", Value: "$connection_upgrade", }, + { + Name: "X-Real-IP", + Value: "$remote_addr", + }, + { + Name: "X-Forwarded-Proto", + Value: "$scheme", + }, + { + Name: "X-Forwarded-Host", + Value: "$host", + }, + { + Name: "X-Forwarded-Port", + Value: "$server_port", + }, }, }, { @@ -2504,6 +2631,22 @@ func TestGenerateProxySetHeaders(t *testing.T) { Name: "Authority", Value: "$gw_api_compliant_host", }, + { + Name: "X-Real-IP", + Value: "$remote_addr", + }, + { + Name: "X-Forwarded-Proto", + Value: "$scheme", + }, + { + Name: "X-Forwarded-Host", + Value: "$host", + }, + { + Name: "X-Forwarded-Port", + Value: "$server_port", + }, }, }, } diff --git a/internal/mode/static/state/dataplane/configuration_test.go b/internal/mode/static/state/dataplane/configuration_test.go index ec5122ade1..b5144b03d4 100644 --- a/internal/mode/static/state/dataplane/configuration_test.go +++ b/internal/mode/static/state/dataplane/configuration_test.go @@ -3655,6 +3655,28 @@ func TestBuildRewriteIPSettings(t *testing.T) { IPRecursive: true, }, }, + { + msg: "rewrite IP settings configured with recursive set to false and multiple trusted addresses", + g: &graph.Graph{ + NginxProxy: &graph.NginxProxy{ + Valid: true, + Source: &ngfAPI.NginxProxy{ + Spec: ngfAPI.NginxProxySpec{ + RewriteClientIP: &ngfAPI.RewriteClientIP{ + Mode: helpers.GetPointer(ngfAPI.RewriteClientIPModeXForwardedFor), + TrustedAddresses: []ngfAPI.TrustedAddress{"0.0.0.0/0", "1.1.1.1/32", "2.2.2.2/32", "3.3.3.3/24"}, + SetIPRecursively: helpers.GetPointer(false), + }, + }, + }, + }, + }, + expRewriteIPSettings: RewriteClientIPSettings{ + Mode: RewriteIPModeXForwardedFor, + TrustedCIDRs: []string{"0.0.0.0/0", "1.1.1.1/32", "2.2.2.2/32", "3.3.3.3/24"}, + IPRecursive: false, + }, + }, } for _, tc := range tests { diff --git a/internal/mode/static/state/dataplane/types.go b/internal/mode/static/state/dataplane/types.go index 185c18c7f2..0ae4b3b2a2 100644 --- a/internal/mode/static/state/dataplane/types.go +++ b/internal/mode/static/state/dataplane/types.go @@ -330,7 +330,7 @@ type RewriteIPModeType string const ( // RewriteIPModeProxyProtocol specifies that client IP will be rewrritten using the Proxy-Protocol header. - RewriteIPModeProxyProtocol RewriteIPModeType = "proxy-protocol" + RewriteIPModeProxyProtocol RewriteIPModeType = "proxy_protocol" // RewriteIPModeXForwardedFor specifies that client IP will be rewrritten using the X-Forwarded-For header. RewriteIPModeXForwardedFor RewriteIPModeType = "X-Forwarded-For" ) diff --git a/internal/mode/static/state/graph/nginxproxy.go b/internal/mode/static/state/graph/nginxproxy.go index bd3b824b53..67b36c87a6 100644 --- a/internal/mode/static/state/graph/nginxproxy.go +++ b/internal/mode/static/state/graph/nginxproxy.go @@ -144,8 +144,10 @@ func validateRewriteClientIP(npCfg *ngfAPI.NginxProxy) field.ErrorList { if rewriteClientIP.Mode != nil { mode := *rewriteClientIP.Mode if len(rewriteClientIP.TrustedAddresses) == 0 { - allErrs = append(allErrs, - field.Required(rewriteClientIPPath, "trustedAddresses field required when mode is set")) + allErrs = append( + allErrs, + field.Required(rewriteClientIPPath, "trustedAddresses field required when mode is set"), + ) } switch mode { @@ -156,7 +158,8 @@ func validateRewriteClientIP(npCfg *ngfAPI.NginxProxy) field.ErrorList { field.NotSupported( rewriteClientIPPath.Child("mode"), mode, - []string{string(ngfAPI.RewriteClientIPModeProxyProtocol), string(ngfAPI.RewriteClientIPModeXForwardedFor)}), + []string{string(ngfAPI.RewriteClientIPModeProxyProtocol), string(ngfAPI.RewriteClientIPModeXForwardedFor)}, + ), ) } } @@ -164,7 +167,8 @@ func validateRewriteClientIP(npCfg *ngfAPI.NginxProxy) field.ErrorList { if len(rewriteClientIP.TrustedAddresses) > 16 { allErrs = append( allErrs, - field.TooLongMaxLength(trustedAddressesPath, rewriteClientIP.TrustedAddresses, 16)) + field.TooLongMaxLength(trustedAddressesPath, rewriteClientIP.TrustedAddresses, 16), + ) } for _, addr := range rewriteClientIP.TrustedAddresses { diff --git a/internal/mode/static/state/graph/nginxproxy_test.go b/internal/mode/static/state/graph/nginxproxy_test.go index c7d01b4fcc..816e21086c 100644 --- a/internal/mode/static/state/graph/nginxproxy_test.go +++ b/internal/mode/static/state/graph/nginxproxy_test.go @@ -364,11 +364,11 @@ func TestValidateNginxProxy(t *testing.T) { func TestValidateRewriteClientIP(t *testing.T) { tests := []struct { - np *ngfAPI.NginxProxy - validator *validationfakes.FakeGenericValidator - name string - expErrSubstring string - expectErrCount int + np *ngfAPI.NginxProxy + validator *validationfakes.FakeGenericValidator + name string + errorString string + expectErrCount int }{ { name: "valid rewriteClientIP", @@ -396,8 +396,10 @@ func TestValidateRewriteClientIP(t *testing.T) { }, }, }, - expectErrCount: 1, - expErrSubstring: "spec.rewriteClientIP.trustedAddresses", + expectErrCount: 1, + errorString: "spec.rewriteClientIP.trustedAddresses.2001:db8:a0b:12f0::1: " + + "Invalid value: \"2001:db8:a0b:12f0::1\": spec.rewriteClientIP.trustedAddresses: " + + "Invalid value: \"2001:db8:a0b:12f0::1\": must be a valid CIDR value, (e.g. 10.9.8.0/24 or 2001:db8::/64)", }, { name: "invalid when mode is set and trustedAddresses is empty", @@ -409,8 +411,8 @@ func TestValidateRewriteClientIP(t *testing.T) { }, }, }, - expectErrCount: 1, - expErrSubstring: "trustedAddresses field required when mode is set", + expectErrCount: 1, + errorString: "spec.rewriteClientIP: Required value: trustedAddresses field required when mode is set", }, { name: "invalid when trustedAddresses is greater in length than 16", @@ -430,8 +432,8 @@ func TestValidateRewriteClientIP(t *testing.T) { }, }, }, - expectErrCount: 1, - expErrSubstring: "Too long: may not be longer than 16", + expectErrCount: 1, + errorString: "spec.rewriteClientIP.trustedAddresses: Too long: may not be longer than 16", }, { name: "invalid when mode is not proxyProtocol or XForwardedFor", @@ -444,8 +446,24 @@ func TestValidateRewriteClientIP(t *testing.T) { }, }, }, - expectErrCount: 1, - expErrSubstring: "supported values: \"ProxyProtocol\", \"XForwardedFor\"", + expectErrCount: 1, + errorString: "spec.rewriteClientIP.mode: Unsupported value: \"invalid\": " + + "supported values: \"ProxyProtocol\", \"XForwardedFor\"", + }, + { + name: "invalid when mode is not proxyProtocol or XForwardedFor and trustedAddresses is empty", + validator: createInvalidValidator(), + np: &ngfAPI.NginxProxy{ + Spec: ngfAPI.NginxProxySpec{ + RewriteClientIP: &ngfAPI.RewriteClientIP{ + Mode: helpers.GetPointer(ngfAPI.RewriteClientIPModeType("invalid")), + }, + }, + }, + expectErrCount: 2, + errorString: "[spec.rewriteClientIP: Required value: trustedAddresses field " + + "required when mode is set, spec.rewriteClientIP.mode: " + + "Unsupported value: \"invalid\": supported values: \"ProxyProtocol\", \"XForwardedFor\"]", }, } @@ -456,7 +474,7 @@ func TestValidateRewriteClientIP(t *testing.T) { allErrs := validateRewriteClientIP(test.np) g.Expect(allErrs).To(HaveLen(test.expectErrCount)) if len(allErrs) > 0 { - g.Expect(allErrs.ToAggregate().Error()).To(ContainSubstring(test.expErrSubstring)) + g.Expect(allErrs.ToAggregate().Error()).To(Equal(test.errorString)) } }) } diff --git a/site/content/how-to/monitoring/troubleshooting.md b/site/content/how-to/monitoring/troubleshooting.md index 2bd08825c7..30d97fa568 100644 --- a/site/content/how-to/monitoring/troubleshooting.md +++ b/site/content/how-to/monitoring/troubleshooting.md @@ -455,7 +455,7 @@ This means you are attempting to attach a Policy to a Route that has an overlapp - Combine the Route rules for the overlapping path into a single Route. - If the Policy allows it, specify both Routes in the `targetRefs` list. -##### Broken Header Error +##### Broken Header error If you check your _nginx_ container logs and see the following error: @@ -463,9 +463,9 @@ If you check your _nginx_ container logs and see the following error: 2024/07/25 00:50:45 [error] 211#211: *22 broken header: "GET /coffee HTTP/1.1" while reading PROXY protocol, client: 127.0.0.1, server: 0.0.0.0:80 ``` -It indicates that `proxy_protocol` is enabled for the gateway listeners but the request sent to the application endpoint does not contain proxy information.To **resolve** this, you can do one of the following: +It indicates that `proxy_protocol` is enabled for the gateway listeners, but the request sent to the application endpoint does not contain proxy information. To **resolve** this, you can do one of the following: -- Update the NginxProxy configuration with [`enableProxyProtocol`](({{< relref "reference/api.md" >}})) disabled. +- Update field [`rewriteClientIP.mode`](({{< relref "reference/api.md" >}})) to `ProxyProtocol` in the NginxProxy configuration. - Send valid proxy information with requests being handled by your application. diff --git a/site/content/reference/api.md b/site/content/reference/api.md index e7105983bd..3518381a4a 100644 --- a/site/content/reference/api.md +++ b/site/content/reference/api.md @@ -1071,7 +1071,10 @@ RewriteClientIPModeType (Optional)

Mode defines how NGINX will rewrite the client’s IP address. -Possible modes: ProxyProtocol, XForwardedFor.

+There are two possible modes: +- ProxyProtocol: NGINX will rewrite the client’s IP using the PROXY protocol header. +- XForwardedFor: NGINX will rewrite the client’s IP using the X-Forwarded-For header. +Sets NGINX directive real_ip_header: https://nginx.org/en/docs/http/ngx_http_realip_module.html#real_ip_header

@@ -1083,10 +1086,15 @@ bool (Optional) -

SetIPRecursively configures whether recursive search is used for selecting client’s -address from the X-Forwarded-For header and used in conjunction with TrustedAddresses. -If enabled, NGINX will recurse on the values in X-Forwarded-Header from the end of -array to start of array and select the first untrusted IP.

+

SetIPRecursively configures whether recursive search is used when selecting the client’s address from. +the X-Forwarded-For header. It is used in conjunction with TrustedAddresses. +If enabled, NGINX will recurse on the values in X-Forwarded-Header from the end of array +to start of array and select the first untrusted IP. +For example, if X-Forwarded-For is [11.11.11.11, 22.22.22.22, 55.55.55.1], +and TrustedAddresses is set to 55.55.55.1⁄0, NGINX will rewrite the client IP to 22.22.22.22. +If disabled, NGINX will select the IP at the end of the array. +In the previous example, 55.55.55.1 would be selected. +Sets NGINX directive real_ip_recursive: https://nginx.org/en/docs/http/ngx_http_realip_module.html#real_ip_recursive

@@ -1103,6 +1111,11 @@ array to start of array and select the first untrusted IP.

TrustedAddresses specifies the addresses that are trusted to send correct client IP information. If a request comes from a trusted address, NGINX will rewrite the client IP information, and forward it to the backend in the X-Forwarded-For* and X-Real-IP headers. +If the request does not come from a trusted address, NGINX will not rewrite the client IP information. +Addresses must be provided as CIDR blocks: 10.0.0.0/32, 192.33.21⁄0. +To trust all addresses (not recommended), set to 0.0.0.0/0. +If no addresses are provided, NGINX will not rewrite the client IP information. +Sets NGINX directive set_real_ip_from: https://nginx.org/en/docs/http/ngx_http_realip_module.html#set_real_ip_from This field is required if mode is set.