From 392f697ce5129784a96f569c5f3a0a1219b72c41 Mon Sep 17 00:00:00 2001 From: Saloni Date: Tue, 23 Jul 2024 15:34:51 -0600 Subject: [PATCH 01/19] add support for preserving clientIP --- apis/v1alpha1/nginxproxy_types.go | 59 ++++++++ apis/v1alpha1/zz_generated.deepcopy.go | 35 +++++ charts/nginx-gateway-fabric/README.md | 8 +- charts/nginx-gateway-fabric/values.yaml | 12 +- .../bases/gateway.nginx.org_nginxproxies.yaml | 38 +++++ config/tests/static-deployment.yaml | 4 +- deploy/aws-nlb/deploy.yaml | 4 +- deploy/azure/deploy.yaml | 4 +- deploy/crds.yaml | 38 +++++ 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 +- .../mode/static/nginx/config/http/config.go | 15 +- internal/mode/static/nginx/config/servers.go | 22 ++- .../static/nginx/config/servers_template.go | 53 +++++-- .../mode/static/nginx/config/servers_test.go | 36 ++++- .../static/state/dataplane/configuration.go | 28 ++++ .../state/dataplane/configuration_test.go | 113 ++++++++++++++ internal/mode/static/state/dataplane/types.go | 26 +++- .../mode/static/state/graph/nginxproxy.go | 48 ++++++ .../static/state/graph/nginxproxy_test.go | 139 +++++++++++++++--- .../how-to/monitoring/troubleshooting.md | 14 ++ site/content/reference/api.md | 136 +++++++++++++++++ 26 files changed, 792 insertions(+), 64 deletions(-) diff --git a/apis/v1alpha1/nginxproxy_types.go b/apis/v1alpha1/nginxproxy_types.go index 018911da7e..5142f3a250 100644 --- a/apis/v1alpha1/nginxproxy_types.go +++ b/apis/v1alpha1/nginxproxy_types.go @@ -53,6 +53,12 @@ type NginxProxySpec struct { // // +optional Telemetry *Telemetry `json:"telemetry,omitempty"` + // RewriteClientIP defines configuration for rewriting the client IP to the original client's IP. + // +kubebuilder:validation:XValidation:message="if mode is set, trustedAddresses is a required field",rule="!(has(self.mode) && !has(self.trustedAddresses))" + // + // +optional + //nolint:lll + RewriteClientIP *RewriteClientIP `json:"rewriteClientIP,omitempty"` // DisableHTTP2 defines if http2 should be disabled for all servers. // Default is false, meaning http2 will be enabled for all servers. // @@ -114,3 +120,56 @@ type TelemetryExporter struct { // +kubebuilder:validation:Pattern=`^(?:http?:\/\/)?[a-z0-9]([a-z0-9-]{0,61}[a-z0-9])?(?:\.[a-z0-9]([a-z0-9-]{0,61}[a-z0-9])?)*(?::\d{1,5})?$` Endpoint string `json:"endpoint"` } + +// 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. + // + // +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. + // + // +optional + SetIPRecursively *bool `json:"setIPRecursively,omitempty"` + + // 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. + // This field is required if mode is set. + // +kubebuilder:validation:MaxItems=16 + // +listType=atomic + // + // + // +optional + TrustedAddresses []TrustedAddress `json:"trustedAddresses,omitempty"` +} + +// RewriteClientIPModeType defines how NGINX Gateway Fabric will determine the client's original IP address. +// +kubebuilder:validation:Enum=ProxyProtocol;XForwardedFor +type RewriteClientIPModeType string + +const ( + // RewriteClientIPModeProxyProtocol configures NGINX to accept PROXY protocol and, + // set the client's IP address to the IP address in the PROXY protocol header. + // Sets the proxy_protocol parameter to the listen directive on all servers, and sets real_ip_header + // to proxy_protocol: https://nginx.org/en/docs/http/ngx_http_realip_module.html#real_ip_header. + RewriteClientIPModeProxyProtocol RewriteClientIPModeType = "ProxyProtocol" + + // RewriteClientIPModeXForwardedFor configures NGINX to set the client's IP address to the + // IP address in the X-Forwarded-For HTTP header. + // https://nginx.org/en/docs/http/ngx_http_realip_module.html#real_ip_header. + RewriteClientIPModeXForwardedFor RewriteClientIPModeType = "XForwardedFor" +) + +// TrustedAddress is a string value representing a CIDR block. +// Examples: 0.0.0.0/0 +// +// +kubebuilder:validation:Pattern=`^(?:(?:[0-9]{1,3}\.){3}[0-9]{1,3}(?:\/(?:[0-9]|[12][0-9]|3[0-2]))?|(?:[0-9a-fA-F]{1,4}:){7}[0-9a-fA-F]{1,4}(?:\/(?:[0-9]|[1-9][0-9]|1[0-1][0-9]|12[0-8]))?)$` +// +//nolint:lll +type TrustedAddress string diff --git a/apis/v1alpha1/zz_generated.deepcopy.go b/apis/v1alpha1/zz_generated.deepcopy.go index 90cdca7a41..42410e127b 100644 --- a/apis/v1alpha1/zz_generated.deepcopy.go +++ b/apis/v1alpha1/zz_generated.deepcopy.go @@ -367,6 +367,11 @@ func (in *NginxProxySpec) DeepCopyInto(out *NginxProxySpec) { *out = new(Telemetry) (*in).DeepCopyInto(*out) } + if in.RewriteClientIP != nil { + in, out := &in.RewriteClientIP, &out.RewriteClientIP + *out = new(RewriteClientIP) + (*in).DeepCopyInto(*out) + } } // DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new NginxProxySpec. @@ -463,6 +468,36 @@ func (in *ObservabilityPolicySpec) DeepCopy() *ObservabilityPolicySpec { return out } +// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. +func (in *RewriteClientIP) DeepCopyInto(out *RewriteClientIP) { + *out = *in + if in.Mode != nil { + in, out := &in.Mode, &out.Mode + *out = new(RewriteClientIPModeType) + **out = **in + } + if in.SetIPRecursively != nil { + in, out := &in.SetIPRecursively, &out.SetIPRecursively + *out = new(bool) + **out = **in + } + if in.TrustedAddresses != nil { + in, out := &in.TrustedAddresses, &out.TrustedAddresses + *out = make([]TrustedAddress, len(*in)) + copy(*out, *in) + } +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new RewriteClientIP. +func (in *RewriteClientIP) DeepCopy() *RewriteClientIP { + if in == nil { + return nil + } + out := new(RewriteClientIP) + in.DeepCopyInto(out) + return out +} + // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *SpanAttribute) DeepCopyInto(out *SpanAttribute) { *out = *in diff --git a/charts/nginx-gateway-fabric/README.md b/charts/nginx-gateway-fabric/README.md index 9ca6cb9073..3f67c3e2e2 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 | `"ghcr.io/nginxinc/nginx-gateway-fabric/nginx"` | -| `nginx.image.tag` | | string | `"edge"` | +| `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.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 | `"ghcr.io/nginxinc/nginx-gateway-fabric"` | -| `nginxGateway.image.tag` | | string | `"edge"` | +| `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.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 9cfc2064b2..ef061515f9 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: ghcr.io/nginxinc/nginx-gateway-fabric - tag: edge + repository: gcr.io/f5-gcs-7899-ptg-ingrss-ctlr/nginx-gateway-fabric + tag: sa.choudhary pullPolicy: Always securityContext: @@ -81,8 +81,8 @@ nginxGateway: nginx: image: # -- The NGINX image to use. - repository: ghcr.io/nginxinc/nginx-gateway-fabric/nginx - tag: edge + repository: gcr.io/f5-gcs-7899-ptg-ingrss-ctlr/nginx-gateway-fabric/nginx + tag: sa.choudhary pullPolicy: Always # -- Is NGINX Plus image being used @@ -93,6 +93,10 @@ nginx: {} # disableHTTP2: false # ipFamily: dual + # rewriteClientIP: + # mode: "ProxyProtocol" + # trustedAddresses: ["0.0.0.0/0"] + # setIPRecursively: true # telemetry: # exporter: # endpoint: otel-collector.default.svc:4317 diff --git a/config/crd/bases/gateway.nginx.org_nginxproxies.yaml b/config/crd/bases/gateway.nginx.org_nginxproxies.yaml index d0c8ba3330..e193cc3480 100644 --- a/config/crd/bases/gateway.nginx.org_nginxproxies.yaml +++ b/config/crd/bases/gateway.nginx.org_nginxproxies.yaml @@ -62,6 +62,44 @@ spec: - ipv4 - ipv6 type: string + rewriteClientIP: + description: RewriteClientIP defines configuration for rewriting the + client IP to the original client's IP. + properties: + mode: + description: |- + Mode defines how NGINX will rewrite the client's IP address. + Possible modes: ProxyProtocol, XForwardedFor. + 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. + 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. + This field is required if mode is set. + items: + description: |- + TrustedAddress is a string value representing a CIDR block. + Examples: 0.0.0.0/0 + pattern: ^(?:(?:[0-9]{1,3}\.){3}[0-9]{1,3}(?:\/(?:[0-9]|[12][0-9]|3[0-2]))?|(?:[0-9a-fA-F]{1,4}:){7}[0-9a-fA-F]{1,4}(?:\/(?:[0-9]|[1-9][0-9]|1[0-1][0-9]|12[0-8]))?)$ + type: string + maxItems: 16 + type: array + x-kubernetes-list-type: atomic + type: object + x-kubernetes-validations: + - message: if mode is set, trustedAddresses is a required field + rule: '!(has(self.mode) && !has(self.trustedAddresses))' telemetry: description: Telemetry specifies the OpenTelemetry configuration. properties: diff --git a/config/tests/static-deployment.yaml b/config/tests/static-deployment.yaml index bb2fb62765..bab7329dd2 100644 --- a/config/tests/static-deployment.yaml +++ b/config/tests/static-deployment.yaml @@ -45,7 +45,7 @@ spec: valueFrom: fieldRef: fieldPath: metadata.name - image: ghcr.io/nginxinc/nginx-gateway-fabric:edge + image: gcr.io/f5-gcs-7899-ptg-ingrss-ctlr/nginx-gateway-fabric:sa.choudhary imagePullPolicy: Always name: nginx-gateway ports: @@ -82,7 +82,7 @@ spec: mountPath: /var/run/nginx - name: nginx-includes mountPath: /etc/nginx/includes - - image: ghcr.io/nginxinc/nginx-gateway-fabric/nginx:edge + - image: gcr.io/f5-gcs-7899-ptg-ingrss-ctlr/nginx-gateway-fabric/nginx:sa.choudhary imagePullPolicy: Always name: nginx ports: diff --git a/deploy/aws-nlb/deploy.yaml b/deploy/aws-nlb/deploy.yaml index 49b29bf988..588c5c5671 100644 --- a/deploy/aws-nlb/deploy.yaml +++ b/deploy/aws-nlb/deploy.yaml @@ -217,7 +217,7 @@ spec: valueFrom: fieldRef: fieldPath: metadata.name - image: ghcr.io/nginxinc/nginx-gateway-fabric:edge + image: gcr.io/f5-gcs-7899-ptg-ingrss-ctlr/nginx-gateway-fabric:sa.choudhary imagePullPolicy: Always name: nginx-gateway ports: @@ -256,7 +256,7 @@ spec: name: nginx-run - mountPath: /etc/nginx/includes name: nginx-includes - - image: ghcr.io/nginxinc/nginx-gateway-fabric/nginx:edge + - image: gcr.io/f5-gcs-7899-ptg-ingrss-ctlr/nginx-gateway-fabric/nginx:sa.choudhary imagePullPolicy: Always name: nginx ports: diff --git a/deploy/azure/deploy.yaml b/deploy/azure/deploy.yaml index 968c1a2926..b61a1363d2 100644 --- a/deploy/azure/deploy.yaml +++ b/deploy/azure/deploy.yaml @@ -214,7 +214,7 @@ spec: valueFrom: fieldRef: fieldPath: metadata.name - image: ghcr.io/nginxinc/nginx-gateway-fabric:edge + image: gcr.io/f5-gcs-7899-ptg-ingrss-ctlr/nginx-gateway-fabric:sa.choudhary imagePullPolicy: Always name: nginx-gateway ports: @@ -253,7 +253,7 @@ spec: name: nginx-run - mountPath: /etc/nginx/includes name: nginx-includes - - image: ghcr.io/nginxinc/nginx-gateway-fabric/nginx:edge + - image: gcr.io/f5-gcs-7899-ptg-ingrss-ctlr/nginx-gateway-fabric/nginx:sa.choudhary imagePullPolicy: Always name: nginx ports: diff --git a/deploy/crds.yaml b/deploy/crds.yaml index 90a4c5d113..928ccc2476 100644 --- a/deploy/crds.yaml +++ b/deploy/crds.yaml @@ -647,6 +647,44 @@ spec: - ipv4 - ipv6 type: string + rewriteClientIP: + description: RewriteClientIP defines configuration for rewriting the + client IP to the original client's IP. + properties: + mode: + description: |- + Mode defines how NGINX will rewrite the client's IP address. + Possible modes: ProxyProtocol, XForwardedFor. + 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. + 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. + This field is required if mode is set. + items: + description: |- + TrustedAddress is a string value representing a CIDR block. + Examples: 0.0.0.0/0 + pattern: ^(?:(?:[0-9]{1,3}\.){3}[0-9]{1,3}(?:\/(?:[0-9]|[12][0-9]|3[0-2]))?|(?:[0-9a-fA-F]{1,4}:){7}[0-9a-fA-F]{1,4}(?:\/(?:[0-9]|[1-9][0-9]|1[0-1][0-9]|12[0-8]))?)$ + type: string + maxItems: 16 + type: array + x-kubernetes-list-type: atomic + type: object + x-kubernetes-validations: + - message: if mode is set, trustedAddresses is a required field + rule: '!(has(self.mode) && !has(self.trustedAddresses))' telemetry: description: Telemetry specifies the OpenTelemetry configuration. properties: diff --git a/deploy/default/deploy.yaml b/deploy/default/deploy.yaml index 6245a2bbc7..3e38b84f62 100644 --- a/deploy/default/deploy.yaml +++ b/deploy/default/deploy.yaml @@ -214,7 +214,7 @@ spec: valueFrom: fieldRef: fieldPath: metadata.name - image: ghcr.io/nginxinc/nginx-gateway-fabric:edge + image: gcr.io/f5-gcs-7899-ptg-ingrss-ctlr/nginx-gateway-fabric:sa.choudhary imagePullPolicy: Always name: nginx-gateway ports: @@ -253,7 +253,7 @@ spec: name: nginx-run - mountPath: /etc/nginx/includes name: nginx-includes - - image: ghcr.io/nginxinc/nginx-gateway-fabric/nginx:edge + - image: gcr.io/f5-gcs-7899-ptg-ingrss-ctlr/nginx-gateway-fabric/nginx:sa.choudhary imagePullPolicy: Always name: nginx ports: diff --git a/deploy/experimental-nginx-plus/deploy.yaml b/deploy/experimental-nginx-plus/deploy.yaml index ed9c748e1a..ae6b2c6191 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: ghcr.io/nginxinc/nginx-gateway-fabric:edge + image: gcr.io/f5-gcs-7899-ptg-ingrss-ctlr/nginx-gateway-fabric:sa.choudhary 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:edge + - image: private-registry.nginx.com/nginx-gateway-fabric/nginx-plus:sa.choudhary imagePullPolicy: Always name: nginx ports: diff --git a/deploy/experimental/deploy.yaml b/deploy/experimental/deploy.yaml index 28cc7b6d19..f67a7a0ab6 100644 --- a/deploy/experimental/deploy.yaml +++ b/deploy/experimental/deploy.yaml @@ -220,7 +220,7 @@ spec: valueFrom: fieldRef: fieldPath: metadata.name - image: ghcr.io/nginxinc/nginx-gateway-fabric:edge + image: gcr.io/f5-gcs-7899-ptg-ingrss-ctlr/nginx-gateway-fabric:sa.choudhary imagePullPolicy: Always name: nginx-gateway ports: @@ -259,7 +259,7 @@ spec: name: nginx-run - mountPath: /etc/nginx/includes name: nginx-includes - - image: ghcr.io/nginxinc/nginx-gateway-fabric/nginx:edge + - image: gcr.io/f5-gcs-7899-ptg-ingrss-ctlr/nginx-gateway-fabric/nginx:sa.choudhary imagePullPolicy: Always name: nginx ports: diff --git a/deploy/nginx-plus/deploy.yaml b/deploy/nginx-plus/deploy.yaml index 76249e80c2..6652561096 100644 --- a/deploy/nginx-plus/deploy.yaml +++ b/deploy/nginx-plus/deploy.yaml @@ -225,7 +225,7 @@ spec: valueFrom: fieldRef: fieldPath: metadata.name - image: ghcr.io/nginxinc/nginx-gateway-fabric:edge + image: gcr.io/f5-gcs-7899-ptg-ingrss-ctlr/nginx-gateway-fabric:sa.choudhary 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:edge + - image: private-registry.nginx.com/nginx-gateway-fabric/nginx-plus:sa.choudhary imagePullPolicy: Always name: nginx ports: diff --git a/deploy/nodeport/deploy.yaml b/deploy/nodeport/deploy.yaml index db81fdf259..132b273db7 100644 --- a/deploy/nodeport/deploy.yaml +++ b/deploy/nodeport/deploy.yaml @@ -214,7 +214,7 @@ spec: valueFrom: fieldRef: fieldPath: metadata.name - image: ghcr.io/nginxinc/nginx-gateway-fabric:edge + image: gcr.io/f5-gcs-7899-ptg-ingrss-ctlr/nginx-gateway-fabric:sa.choudhary imagePullPolicy: Always name: nginx-gateway ports: @@ -253,7 +253,7 @@ spec: name: nginx-run - mountPath: /etc/nginx/includes name: nginx-includes - - image: ghcr.io/nginxinc/nginx-gateway-fabric/nginx:edge + - image: gcr.io/f5-gcs-7899-ptg-ingrss-ctlr/nginx-gateway-fabric/nginx:sa.choudhary imagePullPolicy: Always name: nginx ports: diff --git a/deploy/openshift/deploy.yaml b/deploy/openshift/deploy.yaml index cb78ce0f39..be969cadc8 100644 --- a/deploy/openshift/deploy.yaml +++ b/deploy/openshift/deploy.yaml @@ -222,7 +222,7 @@ spec: valueFrom: fieldRef: fieldPath: metadata.name - image: ghcr.io/nginxinc/nginx-gateway-fabric:edge + image: gcr.io/f5-gcs-7899-ptg-ingrss-ctlr/nginx-gateway-fabric:sa.choudhary imagePullPolicy: Always name: nginx-gateway ports: @@ -261,7 +261,7 @@ spec: name: nginx-run - mountPath: /etc/nginx/includes name: nginx-includes - - image: ghcr.io/nginxinc/nginx-gateway-fabric/nginx:edge + - image: gcr.io/f5-gcs-7899-ptg-ingrss-ctlr/nginx-gateway-fabric/nginx:sa.choudhary imagePullPolicy: Always name: nginx ports: diff --git a/internal/mode/static/nginx/config/http/config.go b/internal/mode/static/nginx/config/http/config.go index 4e26604196..854ac109a2 100644 --- a/internal/mode/static/nginx/config/http/config.go +++ b/internal/mode/static/nginx/config/http/config.go @@ -109,9 +109,10 @@ type ProxySSLVerify struct { // ServerConfig holds configuration for an HTTP server and IP family to be used by NGINX. type ServerConfig struct { - Servers []Server - IPFamily shared.IPFamily - Plus bool + Servers []Server + RewriteClientIP RewriteClientIPSettings + IPFamily shared.IPFamily + Plus bool } // Include defines a file that's included via the include directive. @@ -119,3 +120,11 @@ type Include struct { Name string Content []byte } + +// RewriteClientIP holds the configuration for the rewrite client IP settings. +type RewriteClientIPSettings struct { + RealIPHeader string + RealIPFrom []string + Recursive bool + ProxyProtocol bool +} diff --git a/internal/mode/static/nginx/config/servers.go b/internal/mode/static/nginx/config/servers.go index 4ced814bd0..c3b6e6e299 100644 --- a/internal/mode/static/nginx/config/servers.go +++ b/internal/mode/static/nginx/config/servers.go @@ -69,9 +69,10 @@ func (g GeneratorImpl) executeServers(conf dataplane.Configuration, generator po servers, httpMatchPairs := createServers(conf.HTTPServers, conf.SSLServers, conf.TLSPassthroughServers, generator) serverConfig := http.ServerConfig{ - Servers: servers, - IPFamily: getIPFamily(conf.BaseHTTPConfig), - Plus: g.plus, + Servers: servers, + IPFamily: getIPFamily(conf.BaseHTTPConfig), + Plus: g.plus, + RewriteClientIP: getRewriteClientIPSettings(conf.BaseHTTPConfig.RewriteClientIPSettings), } serverResult := executeResult{ @@ -874,3 +875,18 @@ func createDefaultRootLocation() http.Location { func isNonSlashedPrefixPath(pathType dataplane.PathType, path string) bool { return pathType == dataplane.PathTypePrefix && !strings.HasSuffix(path, "/") } + +// 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, + RealIPFrom: rewriteIP.TrustedCIDRs, + RealIPHeader: string(rewriteIP.Mode), + } +} diff --git a/internal/mode/static/nginx/config/servers_template.go b/internal/mode/static/nginx/config/servers_template.go index 02b8fae97f..bc8df2af3f 100644 --- a/internal/mode/static/nginx/config/servers_template.go +++ b/internal/mode/static/nginx/config/servers_template.go @@ -2,27 +2,46 @@ package config const serversTemplateText = ` js_preload_object matches from /etc/nginx/conf.d/matches.json; -{{ range $s := .Servers -}} +{{ $proxyProtocol := "" }} +{{ if $.RewriteClientIP.ProxyProtocol }}{{ $proxyProtocol = " proxy_protocol" }}{{ end }} + +{{- range $s := .Servers -}} {{ if $s.IsDefaultSSL -}} server { - {{- if or ($.IPFamily.IPv4) ($s.IsSocket) }} - listen {{ $s.Listen }} ssl default_server; + {{- if or ($.IPFamily.IPv4) ($s.IsSocket) }} + listen {{ $s.Listen }} ssl default_server{{ $proxyProtocol }}; {{- end }} {{- if and ($.IPFamily.IPv6) (not $s.IsSocket) }} - listen [::]:{{ $s.Listen }} ssl default_server; + listen [::]:{{ $s.Listen }} ssl default_server{{ $proxyProtocol }}; {{- end }} - ssl_reject_handshake on; + {{- range $cidr := $.RewriteClientIP.RealIPFrom }} + set_real_ip_from {{ $cidr }}; + {{- end}} + {{- if $.RewriteClientIP.RealIPHeader}} + real_ip_header {{ $.RewriteClientIP.RealIPHeader }}; + {{- end}} + {{- if $.RewriteClientIP.Recursive }} + real_ip_recursive on; + {{ end }} } {{- else if $s.IsDefaultHTTP }} server { {{- if $.IPFamily.IPv4 }} - listen {{ $s.Listen }} default_server; + listen {{ $s.Listen }} default_server{{ $proxyProtocol }}; {{- end }} {{- if $.IPFamily.IPv6 }} - listen [::]:{{ $s.Listen }} default_server; + listen [::]:{{ $s.Listen }} default_server{{ $proxyProtocol }}; {{- end }} - + {{- range $cidr := $.RewriteClientIP.RealIPFrom }} + set_real_ip_from {{ $cidr }}; + {{- end}} + {{- if $.RewriteClientIP.RealIPHeader}} + real_ip_header {{ $.RewriteClientIP.RealIPHeader }}; + {{- end}} + {{- if $.RewriteClientIP.Recursive }} + real_ip_recursive on; + {{ end }} default_type text/html; return 404; } @@ -30,10 +49,10 @@ server { server { {{- if $s.SSL }} {{- if or ($.IPFamily.IPv4) ($s.IsSocket) }} - listen {{ $s.Listen }} ssl; + listen {{ $s.Listen }} ssl{{ $proxyProtocol }}; {{- end }} {{- if and ($.IPFamily.IPv6) (not $s.IsSocket) }} - listen [::]:{{ $s.Listen }} ssl; + listen [::]:{{ $s.Listen }} ssl{{ $proxyProtocol }}; {{- end }} ssl_certificate {{ $s.SSL.Certificate }}; ssl_certificate_key {{ $s.SSL.CertificateKey }}; @@ -43,10 +62,10 @@ server { } {{- else }} {{- if $.IPFamily.IPv4 }} - listen {{ $s.Listen }}; + listen {{ $s.Listen }}{{ $proxyProtocol }}; {{- end }} {{- if $.IPFamily.IPv6 }} - listen [::]:{{ $s.Listen }}; + listen [::]:{{ $s.Listen }}{{ $proxyProtocol }}; {{- end }} {{- end }} @@ -60,6 +79,16 @@ server { include {{ $i.Name }}; {{- end }} + {{- range $cidr := $.RewriteClientIP.RealIPFrom }} + set_real_ip_from {{ $cidr }}; + {{- end}} + {{- if $.RewriteClientIP.RealIPHeader}} + real_ip_header {{ $.RewriteClientIP.RealIPHeader }}; + {{- end}} + {{- if $.RewriteClientIP.Recursive }} + real_ip_recursive on; + {{ end }} + {{ range $l := $s.Locations }} location {{ $l.Path }} { {{ if eq $l.Type "internal" -}} diff --git a/internal/mode/static/nginx/config/servers_test.go b/internal/mode/static/nginx/config/servers_test.go index a912e6f2bc..d3ab248f75 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 TestExecuteServers_IPFamily(t *testing.T) { +func TestExecuteServerConfig(t *testing.T) { httpServers := []dataplane.VirtualServer{ { IsDefault: true, @@ -259,6 +259,40 @@ func TestExecuteServers_IPFamily(t *testing.T) { "listen [::]:8443 ssl default_server;": 1, "listen [::]:8443 ssl;": 1, "status_zone": 0, + "real_ip_header proxy-protocol;": 0, + "real_ip_recursive on;": 0, + }, + }, + { + msg: "http and ssl servers with rewrite client IP settings", + config: dataplane.Configuration{ + HTTPServers: httpServers, + SSLServers: sslServers, + BaseHTTPConfig: dataplane.BaseHTTPConfig{ + IPFamily: dataplane.Dual, + RewriteClientIPSettings: dataplane.RewriteClientIPSettings{ + Mode: dataplane.RewriteIPModeProxyProtocol, + TrustedCIDRs: []string{"0.0.0.0/0"}, + IPRecursive: true, + }, + }, + }, + expectedHTTPConfig: map[string]int{ + "set_real_ip_from 0.0.0.0/0;": 4, + "real_ip_header proxy-protocol;": 4, + "real_ip_recursive on;": 4, + "listen 8080 default_server proxy_protocol;": 1, + "listen 8080 proxy_protocol;": 1, + "listen 8443 ssl default_server proxy_protocol;": 1, + "listen 8443 ssl proxy_protocol;": 1, + "server_name example.com;": 2, + "ssl_certificate /etc/nginx/secrets/test-keypair.pem;": 1, + "ssl_certificate_key /etc/nginx/secrets/test-keypair.pem;": 1, + "ssl_reject_handshake on;": 1, + "listen [::]:8080 default_server proxy_protocol;": 1, + "listen [::]:8080 proxy_protocol;": 1, + "listen [::]:8443 ssl default_server proxy_protocol;": 1, + "listen [::]:8443 ssl proxy_protocol;": 1, }, }, } diff --git a/internal/mode/static/state/dataplane/configuration.go b/internal/mode/static/state/dataplane/configuration.go index ebaf15bf0b..54e40232ee 100644 --- a/internal/mode/static/state/dataplane/configuration.go +++ b/internal/mode/static/state/dataplane/configuration.go @@ -852,6 +852,26 @@ func buildBaseHTTPConfig(g *graph.Graph) BaseHTTPConfig { } } + if g.NginxProxy.Source.Spec.RewriteClientIP != nil { + if g.NginxProxy.Source.Spec.RewriteClientIP.Mode != nil { + switch *g.NginxProxy.Source.Spec.RewriteClientIP.Mode { + case ngfAPI.RewriteClientIPModeProxyProtocol: + baseConfig.RewriteClientIPSettings.Mode = RewriteIPModeProxyProtocol + case ngfAPI.RewriteClientIPModeXForwardedFor: + baseConfig.RewriteClientIPSettings.Mode = RewriteIPModeXForwardedFor + } + } + + if len(g.NginxProxy.Source.Spec.RewriteClientIP.TrustedAddresses) > 0 { + trustedAddresses := convertTrustedCIDRs(g) + baseConfig.RewriteClientIPSettings.TrustedCIDRs = trustedAddresses + } + + if g.NginxProxy.Source.Spec.RewriteClientIP.SetIPRecursively != nil { + baseConfig.RewriteClientIPSettings.IPRecursive = *g.NginxProxy.Source.Spec.RewriteClientIP.SetIPRecursively + } + } + return baseConfig } @@ -872,3 +892,11 @@ func buildPolicies(graphPolicies []*graph.Policy) []policies.Policy { return finalPolicies } + +func convertTrustedCIDRs(g *graph.Graph) []string { + trustedAddresses := make([]string, len(g.NginxProxy.Source.Spec.RewriteClientIP.TrustedAddresses)) + for i, addr := range g.NginxProxy.Source.Spec.RewriteClientIP.TrustedAddresses { + trustedAddresses[i] = string(addr) + } + return trustedAddresses +} diff --git a/internal/mode/static/state/dataplane/configuration_test.go b/internal/mode/static/state/dataplane/configuration_test.go index 0510a3719b..ec5122ade1 100644 --- a/internal/mode/static/state/dataplane/configuration_test.go +++ b/internal/mode/static/state/dataplane/configuration_test.go @@ -2180,6 +2180,48 @@ func TestBuildConfiguration(t *testing.T) { }), msg: "NginxProxy with IPv6 IPFamily and no routes", }, + { + graph: getModifiedGraph(func(g *graph.Graph) *graph.Graph { + g.Gateway.Source.ObjectMeta = metav1.ObjectMeta{ + Name: "gw", + Namespace: "ns", + } + g.Gateway.Listeners = append(g.Gateway.Listeners, &graph.Listener{ + Name: "listener-80-1", + Source: listener80, + Valid: true, + Routes: map[graph.RouteKey]*graph.L7Route{}, + }) + g.NginxProxy = &graph.NginxProxy{ + Valid: true, + Source: &ngfAPI.NginxProxy{ + Spec: ngfAPI.NginxProxySpec{ + RewriteClientIP: &ngfAPI.RewriteClientIP{ + SetIPRecursively: helpers.GetPointer(true), + TrustedAddresses: []ngfAPI.TrustedAddress{"0.0.0.0/0"}, + Mode: helpers.GetPointer(ngfAPI.RewriteClientIPModeProxyProtocol), + }, + }, + }, + } + return g + }), + expConf: getModifiedExpectedConfiguration(func(conf Configuration) Configuration { + conf.SSLServers = []VirtualServer{} + conf.SSLKeyPairs = map[SSLKeyPairID]SSLKeyPair{} + conf.BaseHTTPConfig = BaseHTTPConfig{ + HTTP2: true, + IPFamily: Dual, + RewriteClientIPSettings: RewriteClientIPSettings{ + IPRecursive: true, + TrustedCIDRs: []string{"0.0.0.0/0"}, + Mode: RewriteIPModeProxyProtocol, + }, + } + return conf + }), + msg: "NginxProxy with rewriteClientIP details set", + }, } for _, test := range tests { @@ -3552,3 +3594,74 @@ func TestBuildStreamUpstreams(t *testing.T) { g.Expect(streamUpstreams).To(ConsistOf(expectedStreamUpstreams)) } + +func TestBuildRewriteIPSettings(t *testing.T) { + tests := []struct { + msg string + g *graph.Graph + expRewriteIPSettings RewriteClientIPSettings + }{ + { + msg: "no rewrite IP settings configured", + g: &graph.Graph{ + NginxProxy: &graph.NginxProxy{ + Valid: true, + Source: &ngfAPI.NginxProxy{}, + }, + }, + expRewriteIPSettings: RewriteClientIPSettings{}, + }, + { + msg: "rewrite IP settings configured with proxyProtocol", + g: &graph.Graph{ + NginxProxy: &graph.NginxProxy{ + Valid: true, + Source: &ngfAPI.NginxProxy{ + Spec: ngfAPI.NginxProxySpec{ + RewriteClientIP: &ngfAPI.RewriteClientIP{ + Mode: helpers.GetPointer(ngfAPI.RewriteClientIPModeProxyProtocol), + TrustedAddresses: []ngfAPI.TrustedAddress{"0.0.0.0/0"}, + SetIPRecursively: helpers.GetPointer(true), + }, + }, + }, + }, + }, + expRewriteIPSettings: RewriteClientIPSettings{ + Mode: RewriteIPModeProxyProtocol, + TrustedCIDRs: []string{"0.0.0.0/0"}, + IPRecursive: true, + }, + }, + { + msg: "rewrite IP settings configured with xForwardedFor", + 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"}, + SetIPRecursively: helpers.GetPointer(true), + }, + }, + }, + }, + }, + expRewriteIPSettings: RewriteClientIPSettings{ + Mode: RewriteIPModeXForwardedFor, + TrustedCIDRs: []string{"0.0.0.0/0"}, + IPRecursive: true, + }, + }, + } + + for _, tc := range tests { + t.Run(tc.msg, func(t *testing.T) { + g := NewWithT(t) + baseConfig := buildBaseHTTPConfig(tc.g) + g.Expect(baseConfig.RewriteClientIPSettings).To(Equal(tc.expRewriteIPSettings)) + }) + } +} diff --git a/internal/mode/static/state/dataplane/types.go b/internal/mode/static/state/dataplane/types.go index 3741f131c8..185c18c7f2 100644 --- a/internal/mode/static/state/dataplane/types.go +++ b/internal/mode/static/state/dataplane/types.go @@ -38,10 +38,10 @@ type Configuration struct { StreamUpstreams []Upstream // BackendGroups holds all unique BackendGroups. BackendGroups []BackendGroup - // BaseHTTPConfig holds the configuration options at the http context. - BaseHTTPConfig BaseHTTPConfig // Telemetry holds the Otel configuration. Telemetry Telemetry + // BaseHTTPConfig holds the configuration options at the http context. + BaseHTTPConfig BaseHTTPConfig // Version represents the version of the generated configuration. Version int } @@ -309,10 +309,32 @@ type SpanAttribute struct { type BaseHTTPConfig struct { // IPFamily specifies the IP family for all servers. IPFamily IPFamilyType + // RewriteIPSettings defines configuration for rewriting the client IP to the original client's IP. + RewriteClientIPSettings RewriteClientIPSettings // HTTP2 specifies whether http2 should be enabled for all servers. HTTP2 bool } +// RewriteIPSettings defines configuration for rewriting the client IP to the original client's IP. +type RewriteClientIPSettings struct { + // Mode specifies the mode for rewriting the client IP. + Mode RewriteIPModeType + // TrustedCIDRs specifies the CIDRs that are trusted to provide the client IP. + TrustedCIDRs []string + // IPRecursive specifies whether a recursive search is used when selecting the client IP. + IPRecursive bool +} + +// RewriteIPModeType specifies the mode for rewriting the client IP. +type RewriteIPModeType string + +const ( + // RewriteIPModeProxyProtocol specifies that client IP will be rewrritten using the Proxy-Protocol header. + RewriteIPModeProxyProtocol RewriteIPModeType = "proxy-protocol" + // RewriteIPModeXForwardedFor specifies that client IP will be rewrritten using the X-Forwarded-For header. + RewriteIPModeXForwardedFor RewriteIPModeType = "X-Forwarded-For" +) + // IPFamilyType specifies the IP family to be used by NGINX. type IPFamilyType string diff --git a/internal/mode/static/state/graph/nginxproxy.go b/internal/mode/static/state/graph/nginxproxy.go index d36133308b..bd3b824b53 100644 --- a/internal/mode/static/state/graph/nginxproxy.go +++ b/internal/mode/static/state/graph/nginxproxy.go @@ -2,6 +2,7 @@ package graph import ( "k8s.io/apimachinery/pkg/types" + k8svalidation "k8s.io/apimachinery/pkg/util/validation" "k8s.io/apimachinery/pkg/util/validation/field" v1 "sigs.k8s.io/gateway-api/apis/v1" @@ -126,5 +127,52 @@ func validateNginxProxy( npCfg.Spec.IPFamily = helpers.GetPointer[ngfAPI.IPFamilyType](ngfAPI.Dual) } + allErrs = append(allErrs, validateRewriteClientIP(npCfg)...) + + return allErrs +} + +func validateRewriteClientIP(npCfg *ngfAPI.NginxProxy) field.ErrorList { + var allErrs field.ErrorList + spec := field.NewPath("spec") + + if npCfg.Spec.RewriteClientIP != nil { + rewriteClientIP := npCfg.Spec.RewriteClientIP + rewriteClientIPPath := spec.Child("rewriteClientIP") + trustedAddressesPath := rewriteClientIPPath.Child("trustedAddresses") + + 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")) + } + + switch mode { + case ngfAPI.RewriteClientIPModeProxyProtocol, ngfAPI.RewriteClientIPModeXForwardedFor: + default: + allErrs = append( + allErrs, + field.NotSupported( + rewriteClientIPPath.Child("mode"), + mode, + []string{string(ngfAPI.RewriteClientIPModeProxyProtocol), string(ngfAPI.RewriteClientIPModeXForwardedFor)}), + ) + } + } + + if len(rewriteClientIP.TrustedAddresses) > 16 { + allErrs = append( + allErrs, + field.TooLongMaxLength(trustedAddressesPath, rewriteClientIP.TrustedAddresses, 16)) + } + + for _, addr := range rewriteClientIP.TrustedAddresses { + if err := k8svalidation.IsValidCIDR(trustedAddressesPath, string(addr)); err != nil { + allErrs = append(allErrs, field.Invalid(trustedAddressesPath.Child(string(addr)), addr, err.ToAggregate().Error())) + } + } + } + return allErrs } diff --git a/internal/mode/static/state/graph/nginxproxy_test.go b/internal/mode/static/state/graph/nginxproxy_test.go index efec06e865..c7d01b4fcc 100644 --- a/internal/mode/static/state/graph/nginxproxy_test.go +++ b/internal/mode/static/state/graph/nginxproxy_test.go @@ -222,27 +222,27 @@ func TestGCReferencesAnyNginxProxy(t *testing.T) { } } -func TestValidateNginxProxy(t *testing.T) { - createValidValidator := func() *validationfakes.FakeGenericValidator { - v := &validationfakes.FakeGenericValidator{} - v.ValidateEscapedStringNoVarExpansionReturns(nil) - v.ValidateEndpointReturns(nil) - v.ValidateServiceNameReturns(nil) - v.ValidateNginxDurationReturns(nil) +func createValidValidator() *validationfakes.FakeGenericValidator { + v := &validationfakes.FakeGenericValidator{} + v.ValidateEscapedStringNoVarExpansionReturns(nil) + v.ValidateEndpointReturns(nil) + v.ValidateServiceNameReturns(nil) + v.ValidateNginxDurationReturns(nil) - return v - } + return v +} - createInvalidValidator := func() *validationfakes.FakeGenericValidator { - v := &validationfakes.FakeGenericValidator{} - v.ValidateEscapedStringNoVarExpansionReturns(errors.New("error")) - v.ValidateEndpointReturns(errors.New("error")) - v.ValidateServiceNameReturns(errors.New("error")) - v.ValidateNginxDurationReturns(errors.New("error")) +func createInvalidValidator() *validationfakes.FakeGenericValidator { + v := &validationfakes.FakeGenericValidator{} + v.ValidateEscapedStringNoVarExpansionReturns(errors.New("error")) + v.ValidateEndpointReturns(errors.New("error")) + v.ValidateServiceNameReturns(errors.New("error")) + v.ValidateNginxDurationReturns(errors.New("error")) - return v - } + return v +} +func TestValidateNginxProxy(t *testing.T) { tests := []struct { np *ngfAPI.NginxProxy validator *validationfakes.FakeGenericValidator @@ -266,6 +266,11 @@ func TestValidateNginxProxy(t *testing.T) { }, }, IPFamily: helpers.GetPointer[ngfAPI.IPFamilyType](ngfAPI.Dual), + RewriteClientIP: &ngfAPI.RewriteClientIP{ + SetIPRecursively: helpers.GetPointer(true), + TrustedAddresses: []ngfAPI.TrustedAddress{"2001:db8:a0b:12f0::1/32", "0.0.0.0/0"}, + Mode: helpers.GetPointer(ngfAPI.RewriteClientIPModeProxyProtocol), + }, }, }, expectErrCount: 0, @@ -356,3 +361,103 @@ func TestValidateNginxProxy(t *testing.T) { }) } } + +func TestValidateRewriteClientIP(t *testing.T) { + tests := []struct { + np *ngfAPI.NginxProxy + validator *validationfakes.FakeGenericValidator + name string + expErrSubstring string + expectErrCount int + }{ + { + name: "valid rewriteClientIP", + validator: createValidValidator(), + np: &ngfAPI.NginxProxy{ + Spec: ngfAPI.NginxProxySpec{ + RewriteClientIP: &ngfAPI.RewriteClientIP{ + SetIPRecursively: helpers.GetPointer(true), + TrustedAddresses: []ngfAPI.TrustedAddress{"2001:db8:a0b:12f0::1/32", "0.0.0.0/0"}, + Mode: helpers.GetPointer(ngfAPI.RewriteClientIPModeProxyProtocol), + }, + }, + }, + expectErrCount: 0, + }, + { + name: "invalid CIDR in trustedAddresses", + validator: createInvalidValidator(), + np: &ngfAPI.NginxProxy{ + Spec: ngfAPI.NginxProxySpec{ + RewriteClientIP: &ngfAPI.RewriteClientIP{ + SetIPRecursively: helpers.GetPointer(true), + TrustedAddresses: []ngfAPI.TrustedAddress{"2001:db8:a0b:12f0::1"}, + Mode: helpers.GetPointer(ngfAPI.RewriteClientIPModeProxyProtocol), + }, + }, + }, + expectErrCount: 1, + expErrSubstring: "spec.rewriteClientIP.trustedAddresses", + }, + { + name: "invalid when mode is set and trustedAddresses is empty", + validator: createInvalidValidator(), + np: &ngfAPI.NginxProxy{ + Spec: ngfAPI.NginxProxySpec{ + RewriteClientIP: &ngfAPI.RewriteClientIP{ + Mode: helpers.GetPointer(ngfAPI.RewriteClientIPModeProxyProtocol), + }, + }, + }, + expectErrCount: 1, + expErrSubstring: "trustedAddresses field required when mode is set", + }, + { + name: "invalid when trustedAddresses is greater in length than 16", + validator: createInvalidValidator(), + np: &ngfAPI.NginxProxy{ + Spec: ngfAPI.NginxProxySpec{ + RewriteClientIP: &ngfAPI.RewriteClientIP{ + Mode: helpers.GetPointer(ngfAPI.RewriteClientIPModeProxyProtocol), + TrustedAddresses: []ngfAPI.TrustedAddress{ + "2001:db8:a0b:12f0::1/32", "2001:db8:a0b:12f0::1/32", "2001:db8:a0b:12f0::1/32", + "2001:db8:a0b:12f0::1/32", "2001:db8:a0b:12f0::1/32", "2001:db8:a0b:12f0::1/32", + "2001:db8:a0b:12f0::1/32", "2001:db8:a0b:12f0::1/32", "2001:db8:a0b:12f0::1/32", + "2001:db8:a0b:12f0::1/32", "2001:db8:a0b:12f0::1/32", "2001:db8:a0b:12f0::1/32", + "2001:db8:a0b:12f0::1/32", "2001:db8:a0b:12f0::1/32", + "2001:db8:a0b:12f0::1/32", "2001:db8:a0b:12f0::1/32", "2001:db8:a0b:12f0::1/32", + }, + }, + }, + }, + expectErrCount: 1, + expErrSubstring: "Too long: may not be longer than 16", + }, + { + name: "invalid when mode is not proxyProtocol or XForwardedFor", + validator: createInvalidValidator(), + np: &ngfAPI.NginxProxy{ + Spec: ngfAPI.NginxProxySpec{ + RewriteClientIP: &ngfAPI.RewriteClientIP{ + Mode: helpers.GetPointer(ngfAPI.RewriteClientIPModeType("invalid")), + TrustedAddresses: []ngfAPI.TrustedAddress{"2001:db8:a0b:12f0::1/32", "0.0.0.0/0"}, + }, + }, + }, + expectErrCount: 1, + expErrSubstring: "supported values: \"ProxyProtocol\", \"XForwardedFor\"", + }, + } + + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + g := NewWithT(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)) + } + }) + } +} diff --git a/site/content/how-to/monitoring/troubleshooting.md b/site/content/how-to/monitoring/troubleshooting.md index 4d4b266ad8..9fb5c24876 100644 --- a/site/content/how-to/monitoring/troubleshooting.md +++ b/site/content/how-to/monitoring/troubleshooting.md @@ -455,6 +455,20 @@ 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 + +If you check your _nginx_ container logs and see the following error: + + ```text + 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: + +- Update the NginxProxy configuration with [`enableProxyProtocol`](({{< relref "reference/api.md" >}})) disabled. + +- Send valid proxy information with requests being handled by your application. + ### Further reading You can view the [Kubernetes Troubleshooting Guide](https://kubernetes.io/docs/tasks/debug/debug-application/) for more debugging guidance. diff --git a/site/content/reference/api.md b/site/content/reference/api.md index d9dd3d2de2..e7105983bd 100644 --- a/site/content/reference/api.md +++ b/site/content/reference/api.md @@ -329,6 +329,20 @@ Telemetry +rewriteClientIP
+ + +RewriteClientIP + + + + +(Optional) +

RewriteClientIP defines configuration for rewriting the client IP to the original client’s IP.

+ + + + disableHTTP2
bool @@ -951,6 +965,20 @@ Telemetry +rewriteClientIP
+ + +RewriteClientIP + + + + +(Optional) +

RewriteClientIP defines configuration for rewriting the client IP to the original client’s IP.

+ + + + disableHTTP2
bool @@ -1013,6 +1041,103 @@ Support: HTTPRoute, GRPCRoute.

+

RewriteClientIP + +

+

+(Appears on: +NginxProxySpec) +

+

+

RewriteClientIP specifies the configuration for rewriting the client’s IP address.

+

+ + + + + + + + + + + + + + + + + + + + + +
FieldDescription
+mode
+ + +RewriteClientIPModeType + + +		 +(Optional) +

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

+
+setIPRecursively
+ +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.

+
+trustedAddresses
+ + +[]TrustedAddress + + +		 +(Optional) +

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. +This field is required if mode is set.

+
+

RewriteClientIPModeType +(string alias)

+

+

+(Appears on: +RewriteClientIP) +

+

+

RewriteClientIPModeType defines how NGINX Gateway Fabric will determine the client’s original IP address.

+

+ + + + + + + + + + + + +
ValueDescription

"ProxyProtocol"

RewriteClientIPModeProxyProtocol configures NGINX to accept PROXY protocol and, +set the client’s IP address to the IP address in the PROXY protocol header. +Sets the proxy_protocol parameter to the listen directive on all servers, and sets real_ip_header +to proxy_protocol: https://nginx.org/en/docs/http/ngx_http_realip_module.html#real_ip_header.

+

"XForwardedFor"

RewriteClientIPModeXForwardedFor configures NGINX to set the client’s IP address to the +IP address in the X-Forwarded-For HTTP header. +https://nginx.org/en/docs/http/ngx_http_realip_module.html#real_ip_header.

+

Size (string alias)

@@ -1353,6 +1478,17 @@ Examples of invalid names: some-$value, quoted-“value”-name, unescap +

TrustedAddress +(string alias)

+

+

+(Appears on: +RewriteClientIP) +

+

+

TrustedAddress is a string value representing a CIDR block. +Examples: 0.0.0.0/0

+

Generated with gen-crd-api-reference-docs From fffaaf993ad9d0449eae727e512ea435aa7c877b Mon Sep 17 00:00:00 2001 From: Saloni Date: Wed, 21 Aug 2024 13:13:12 -0600 Subject: [PATCH 02/19] 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 e193cc3480..81ea4c1bd9 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 928ccc2476..bd19100a3f 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 9fb5c24876..54da2f2128 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.

From 575edb64d062dca85048afecf02f2ed7e4b9865e Mon Sep 17 00:00:00 2001 From: Saloni Date: Thu, 22 Aug 2024 11:21:14 -0600 Subject: [PATCH 03/19] update stream servers config --- .../mode/static/nginx/config/http/config.go | 10 +- internal/mode/static/nginx/config/servers.go | 4 +- .../static/nginx/config/servers_template.go | 34 ++++--- .../mode/static/nginx/config/servers_test.go | 85 ++++++++++++----- .../mode/static/nginx/config/shared/config.go | 8 ++ .../mode/static/nginx/config/stream/config.go | 7 +- .../static/nginx/config/stream_servers.go | 7 +- .../nginx/config/stream_servers_template.go | 11 ++- .../nginx/config/stream_servers_test.go | 95 +++++++++++++++++++ 9 files changed, 208 insertions(+), 53 deletions(-) diff --git a/internal/mode/static/nginx/config/http/config.go b/internal/mode/static/nginx/config/http/config.go index 854ac109a2..ee45daf98c 100644 --- a/internal/mode/static/nginx/config/http/config.go +++ b/internal/mode/static/nginx/config/http/config.go @@ -110,7 +110,7 @@ type ProxySSLVerify struct { // ServerConfig holds configuration for an HTTP server and IP family to be used by NGINX. type ServerConfig struct { Servers []Server - RewriteClientIP RewriteClientIPSettings + RewriteClientIP shared.RewriteClientIPSettings IPFamily shared.IPFamily Plus bool } @@ -120,11 +120,3 @@ type Include struct { Name string Content []byte } - -// RewriteClientIP holds the configuration for the rewrite client IP settings. -type RewriteClientIPSettings struct { - RealIPHeader string - RealIPFrom []string - Recursive bool - ProxyProtocol bool -} diff --git a/internal/mode/static/nginx/config/servers.go b/internal/mode/static/nginx/config/servers.go index d0171f4d70..1d19ddae97 100644 --- a/internal/mode/static/nginx/config/servers.go +++ b/internal/mode/static/nginx/config/servers.go @@ -909,8 +909,8 @@ 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 { - return http.RewriteClientIPSettings{ +func getRewriteClientIPSettings(rewriteIP dataplane.RewriteClientIPSettings) shared.RewriteClientIPSettings { + return shared.RewriteClientIPSettings{ Recursive: rewriteIP.IPRecursive, ProxyProtocol: rewriteIP.Mode == dataplane.RewriteIPModeProxyProtocol, RealIPFrom: rewriteIP.TrustedCIDRs, diff --git a/internal/mode/static/nginx/config/servers_template.go b/internal/mode/static/nginx/config/servers_template.go index bc8df2af3f..cb16dfc62e 100644 --- a/internal/mode/static/nginx/config/servers_template.go +++ b/internal/mode/static/nginx/config/servers_template.go @@ -15,13 +15,17 @@ server { listen [::]:{{ $s.Listen }} ssl default_server{{ $proxyProtocol }}; {{- end }} ssl_reject_handshake on; - {{- range $cidr := $.RewriteClientIP.RealIPFrom }} + {{- if and ($.RewriteClientIP.ProxyProtocol) ($s.IsSocket)}} + set_real_ip_from unix:; + {{- else if (not $s.IsSocket)}} + {{- range $cidr := $.RewriteClientIP.RealIPFrom }} set_real_ip_from {{ $cidr }}; - {{- end}} - {{- if $.RewriteClientIP.RealIPHeader}} + {{- end}} + {{ end }} + {{- if and ($.RewriteClientIP.RealIPHeader) (not $s.IsSocket)}} real_ip_header {{ $.RewriteClientIP.RealIPHeader }}; {{- end}} - {{- if $.RewriteClientIP.Recursive }} + {{- if and ($.RewriteClientIP.Recursive) (not $s.IsSocket)}} real_ip_recursive on; {{ end }} } @@ -33,13 +37,17 @@ server { {{- if $.IPFamily.IPv6 }} listen [::]:{{ $s.Listen }} default_server{{ $proxyProtocol }}; {{- end }} - {{- range $cidr := $.RewriteClientIP.RealIPFrom }} + {{- if and ($.RewriteClientIP.ProxyProtocol) ($s.IsSocket)}} + set_real_ip_from unix:; + {{- else if (not $s.IsSocket)}} + {{- range $cidr := $.RewriteClientIP.RealIPFrom }} set_real_ip_from {{ $cidr }}; - {{- end}} - {{- if $.RewriteClientIP.RealIPHeader}} + {{- end}} + {{ end }} + {{- if and ($.RewriteClientIP.RealIPHeader) (not $s.IsSocket)}} real_ip_header {{ $.RewriteClientIP.RealIPHeader }}; {{- end}} - {{- if $.RewriteClientIP.Recursive }} + {{- if and ($.RewriteClientIP.Recursive) (not $s.IsSocket)}} real_ip_recursive on; {{ end }} default_type text/html; @@ -79,13 +87,17 @@ server { include {{ $i.Name }}; {{- end }} - {{- range $cidr := $.RewriteClientIP.RealIPFrom }} + {{- if and ($.RewriteClientIP.ProxyProtocol) ($s.IsSocket)}} + set_real_ip_from unix:; + {{- else if (not $s.IsSocket)}} + {{- range $cidr := $.RewriteClientIP.RealIPFrom }} set_real_ip_from {{ $cidr }}; {{- end}} - {{- if $.RewriteClientIP.RealIPHeader}} + {{ end }} + {{- if and ($.RewriteClientIP.RealIPHeader) (not $s.IsSocket)}} real_ip_header {{ $.RewriteClientIP.RealIPHeader }}; {{- end}} - {{- if $.RewriteClientIP.Recursive }} + {{- if and ($.RewriteClientIP.Recursive) (not $s.IsSocket)}} real_ip_recursive on; {{ end }} diff --git a/internal/mode/static/nginx/config/servers_test.go b/internal/mode/static/nginx/config/servers_test.go index ece16a8c03..5946106a56 100644 --- a/internal/mode/static/nginx/config/servers_test.go +++ b/internal/mode/static/nginx/config/servers_test.go @@ -284,37 +284,40 @@ func TestExecuteServers_IPFamily(t *testing.T) { } func TestExecuteServers_RewriteClientIP(t *testing.T) { + 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, + }, + } tests := []struct { msg string expectedHTTPConfig map[string]int config dataplane.Configuration }{ { - msg: "http and ssl servers with rewrite client IP settings", + msg: "rewrite client IP settings configured with proxy protocol", config: dataplane.Configuration{ - 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, - }, - }, + HTTPServers: httpServers, + SSLServers: sslServers, BaseHTTPConfig: dataplane.BaseHTTPConfig{ IPFamily: dataplane.Dual, RewriteClientIPSettings: dataplane.RewriteClientIPSettings{ @@ -328,6 +331,7 @@ func TestExecuteServers_RewriteClientIP(t *testing.T) { "set_real_ip_from 0.0.0.0/0;": 4, "real_ip_header proxy_protocol;": 4, "real_ip_recursive on;": 4, + "proxy_protocol on;": 0, "listen 8080 default_server proxy_protocol;": 1, "listen 8080 proxy_protocol;": 1, "listen 8443 ssl default_server proxy_protocol;": 1, @@ -342,6 +346,39 @@ func TestExecuteServers_RewriteClientIP(t *testing.T) { "listen [::]:8443 ssl proxy_protocol;": 1, }, }, + { + msg: "rewrite client IP settings configured with x-forwarded-for", + config: dataplane.Configuration{ + HTTPServers: httpServers, + SSLServers: sslServers, + BaseHTTPConfig: dataplane.BaseHTTPConfig{ + IPFamily: dataplane.Dual, + RewriteClientIPSettings: dataplane.RewriteClientIPSettings{ + Mode: dataplane.RewriteIPModeXForwardedFor, + TrustedCIDRs: []string{"0.0.0.0/0"}, + IPRecursive: true, + }, + }, + }, + expectedHTTPConfig: map[string]int{ + "set_real_ip_from 0.0.0.0/0;": 4, + "real_ip_header X-Forwarded-For;": 4, + "real_ip_recursive on;": 4, + "proxy_protocol on;": 0, + "listen 8080 default_server;": 1, + "listen 8080;": 1, + "listen 8443 ssl default_server;": 1, + "listen 8443 ssl;": 1, + "server_name example.com;": 2, + "ssl_certificate /etc/nginx/secrets/test-keypair.pem;": 1, + "ssl_certificate_key /etc/nginx/secrets/test-keypair.pem;": 1, + "ssl_reject_handshake on;": 1, + "listen [::]:8080 default_server;": 1, + "listen [::]:8080;": 1, + "listen [::]:8443 ssl default_server;": 1, + "listen [::]:8443 ssl;": 1, + }, + }, } for _, test := range tests { diff --git a/internal/mode/static/nginx/config/shared/config.go b/internal/mode/static/nginx/config/shared/config.go index 65c0c873f5..eada321715 100644 --- a/internal/mode/static/nginx/config/shared/config.go +++ b/internal/mode/static/nginx/config/shared/config.go @@ -19,3 +19,11 @@ type IPFamily struct { IPv4 bool IPv6 bool } + +// RewriteClientIP holds the configuration for the rewrite client IP settings. +type RewriteClientIPSettings struct { + RealIPHeader string + RealIPFrom []string + Recursive bool + ProxyProtocol bool +} diff --git a/internal/mode/static/nginx/config/stream/config.go b/internal/mode/static/nginx/config/stream/config.go index 6a3687306c..0bbebadc20 100644 --- a/internal/mode/static/nginx/config/stream/config.go +++ b/internal/mode/static/nginx/config/stream/config.go @@ -26,7 +26,8 @@ type UpstreamServer struct { // ServerConfig holds configuration for a stream server and IP family to be used by NGINX. type ServerConfig struct { - Servers []Server - IPFamily shared.IPFamily - Plus bool + Servers []Server + RewriteClientIP shared.RewriteClientIPSettings + IPFamily shared.IPFamily + Plus bool } diff --git a/internal/mode/static/nginx/config/stream_servers.go b/internal/mode/static/nginx/config/stream_servers.go index b655818698..73d90a7de7 100644 --- a/internal/mode/static/nginx/config/stream_servers.go +++ b/internal/mode/static/nginx/config/stream_servers.go @@ -15,9 +15,10 @@ func (g GeneratorImpl) executeStreamServers(conf dataplane.Configuration) []exec streamServers := createStreamServers(conf) streamServerConfig := stream.ServerConfig{ - Servers: streamServers, - IPFamily: getIPFamily(conf.BaseHTTPConfig), - Plus: g.plus, + Servers: streamServers, + IPFamily: getIPFamily(conf.BaseHTTPConfig), + Plus: g.plus, + RewriteClientIP: getRewriteClientIPSettings(conf.BaseHTTPConfig.RewriteClientIPSettings), } streamServerResult := executeResult{ diff --git a/internal/mode/static/nginx/config/stream_servers_template.go b/internal/mode/static/nginx/config/stream_servers_template.go index 4b619a9e0e..7320770b68 100644 --- a/internal/mode/static/nginx/config/stream_servers_template.go +++ b/internal/mode/static/nginx/config/stream_servers_template.go @@ -1,15 +1,24 @@ package config const streamServersTemplateText = ` +{{ $proxyProtocol := "" }} +{{ if $.RewriteClientIP.ProxyProtocol }}{{ $proxyProtocol = " proxy_protocol" }}{{ end }} {{- range $s := .Servers }} server { - {{- if or ($.IPFamily.IPv4) ($s.IsSocket) }} + {{- if and ($.IPFamily.IPv4) (not $s.IsSocket) }} listen {{ $s.Listen }}; + {{- else if and ($.IPFamily.IPv4) ( $s.IsSocket)}} + listen {{ $s.Listen }}{{ $proxyProtocol }}; {{- end }} {{- if and ($.IPFamily.IPv6) (not $s.IsSocket) }} listen [::]:{{ $s.Listen }}; {{- end }} + {{- if and ($s.IsSocket) ($.RewriteClientIP.ProxyProtocol) }} + {{- range $cidr := $.RewriteClientIP.RealIPFrom }} + set_real_ip_from {{ $cidr }}; + {{- end}} + {{- end }} {{- if $.Plus }} status_zone {{ $s.StatusZone }}; {{- end }} diff --git a/internal/mode/static/nginx/config/stream_servers_test.go b/internal/mode/static/nginx/config/stream_servers_test.go index 1e08874c8f..9d7e0d9165 100644 --- a/internal/mode/static/nginx/config/stream_servers_test.go +++ b/internal/mode/static/nginx/config/stream_servers_test.go @@ -297,6 +297,101 @@ func TestExecuteStreamServersForIPFamily(t *testing.T) { } } +func TestExecuteStreamServers_RewriteClientIP(t *testing.T) { + passThroughServers := []dataplane.Layer4VirtualServer{ + { + UpstreamName: "backend1", + Hostname: "cafe.example.com", + Port: 8443, + }, + } + streamUpstreams := []dataplane.Upstream{ + { + Name: "backend1", + Endpoints: []resolver.Endpoint{ + { + Address: "1.1.1.1", + }, + }, + }, + } + tests := []struct { + msg string + expectedStreamConfig map[string]int + config dataplane.Configuration + }{ + { + msg: "rewrite client IP not configured", + config: dataplane.Configuration{ + TLSPassthroughServers: passThroughServers, + StreamUpstreams: streamUpstreams, + }, + expectedStreamConfig: map[string]int{ + "listen 8443;": 1, + "listen [::]:8443;": 1, + "listen unix:/var/run/nginx/cafe.example.com-8443.sock;": 1, + }, + }, + { + msg: "rewrite client IP configured with proxy protocol", + config: dataplane.Configuration{ + BaseHTTPConfig: dataplane.BaseHTTPConfig{ + RewriteClientIPSettings: dataplane.RewriteClientIPSettings{ + Mode: dataplane.RewriteIPModeProxyProtocol, + TrustedCIDRs: []string{"1.1.1.1/32"}, + IPRecursive: true, + }, + }, + TLSPassthroughServers: passThroughServers, + StreamUpstreams: streamUpstreams, + }, + expectedStreamConfig: map[string]int{ + "listen 8443;": 1, + "listen [::]:8443;": 1, + "listen unix:/var/run/nginx/cafe.example.com-8443.sock proxy_protocol;": 1, + "set_real_ip_from 1.1.1.1/32;": 1, + "real_ip_recursive on;": 0, + }, + }, + { + msg: "rewrite client IP configured with xforwardedfor", + config: dataplane.Configuration{ + BaseHTTPConfig: dataplane.BaseHTTPConfig{ + RewriteClientIPSettings: dataplane.RewriteClientIPSettings{ + Mode: dataplane.RewriteIPModeXForwardedFor, + TrustedCIDRs: []string{"1.1.1.1/32"}, + IPRecursive: true, + }, + }, + TLSPassthroughServers: passThroughServers, + StreamUpstreams: streamUpstreams, + }, + expectedStreamConfig: map[string]int{ + "listen 8443;": 1, + "listen [::]:8443;": 1, + "listen unix:/var/run/nginx/cafe.example.com-8443.sock;": 1, + "set_real_ip_from 1.1.1.1/32;": 0, + "real_ip_recursive on;": 0, + }, + }, + } + + for _, test := range tests { + t.Run(test.msg, func(t *testing.T) { + g := NewWithT(t) + + gen := GeneratorImpl{} + results := gen.executeStreamServers(test.config) + g.Expect(results).To(HaveLen(1)) + serverConf := string(results[0].data) + + for expSubStr, expCount := range test.expectedStreamConfig { + g.Expect(strings.Count(serverConf, expSubStr)).To(Equal(expCount)) + } + }) + } +} + func TestCreateStreamServersWithNone(t *testing.T) { conf := dataplane.Configuration{ TLSPassthroughServers: nil, From 2b956f3b805830a88b03556872028dd41be3dd9b Mon Sep 17 00:00:00 2001 From: Saloni Date: Tue, 27 Aug 2024 13:12:08 -0600 Subject: [PATCH 04/19] fix unit tests --- internal/mode/static/nginx/config/servers_template.go | 2 +- internal/mode/static/nginx/config/stream_servers_template.go | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/internal/mode/static/nginx/config/servers_template.go b/internal/mode/static/nginx/config/servers_template.go index cb16dfc62e..7a6dded313 100644 --- a/internal/mode/static/nginx/config/servers_template.go +++ b/internal/mode/static/nginx/config/servers_template.go @@ -8,7 +8,7 @@ js_preload_object matches from /etc/nginx/conf.d/matches.json; {{- range $s := .Servers -}} {{ if $s.IsDefaultSSL -}} server { - {{- if or ($.IPFamily.IPv4) ($s.IsSocket) }} + {{- if or ($.IPFamily.IPv4) ($s.IsSocket) }} listen {{ $s.Listen }} ssl default_server{{ $proxyProtocol }}; {{- end }} {{- if and ($.IPFamily.IPv6) (not $s.IsSocket) }} diff --git a/internal/mode/static/nginx/config/stream_servers_template.go b/internal/mode/static/nginx/config/stream_servers_template.go index 7320770b68..2b98944df8 100644 --- a/internal/mode/static/nginx/config/stream_servers_template.go +++ b/internal/mode/static/nginx/config/stream_servers_template.go @@ -7,7 +7,7 @@ const streamServersTemplateText = ` server { {{- if and ($.IPFamily.IPv4) (not $s.IsSocket) }} listen {{ $s.Listen }}; - {{- else if and ($.IPFamily.IPv4) ( $s.IsSocket)}} + {{- else if $s.IsSocket }} listen {{ $s.Listen }}{{ $proxyProtocol }}; {{- end }} {{- if and ($.IPFamily.IPv6) (not $s.IsSocket) }} From f0f276262e80f6351a3b3c3506ba52975159ae64 Mon Sep 17 00:00:00 2001 From: Saloni Date: Wed, 28 Aug 2024 10:02:11 -0600 Subject: [PATCH 05/19] simplify config templates --- apis/v1alpha1/nginxproxy_types.go | 2 +- charts/nginx-gateway-fabric/values.yaml | 2 +- internal/mode/static/nginx/config/servers.go | 32 ++++---- .../static/nginx/config/servers_template.go | 56 +++++-------- .../mode/static/nginx/config/servers_test.go | 78 +++++++++---------- .../mode/static/nginx/config/shared/config.go | 6 +- .../mode/static/nginx/config/stream/config.go | 20 ++--- .../static/nginx/config/stream_servers.go | 43 ++++++++-- .../nginx/config/stream_servers_template.go | 10 +-- .../nginx/config/stream_servers_test.go | 7 +- site/content/reference/api.md | 5 +- 11 files changed, 137 insertions(+), 124 deletions(-) diff --git a/apis/v1alpha1/nginxproxy_types.go b/apis/v1alpha1/nginxproxy_types.go index a05a73d32f..867e8113dd 100644 --- a/apis/v1alpha1/nginxproxy_types.go +++ b/apis/v1alpha1/nginxproxy_types.go @@ -167,7 +167,7 @@ type RewriteClientIP struct { type RewriteClientIPModeType string const ( - // RewriteClientIPModeProxyProtocol configures NGINX to accept PROXY protocol and, + // RewriteClientIPModeProxyProtocol configures NGINX to accept PROXY protocol and // set the client's IP address to the IP address in the PROXY protocol header. // Sets the proxy_protocol parameter to the listen directive on all servers, and sets real_ip_header // to proxy_protocol: https://nginx.org/en/docs/http/ngx_http_realip_module.html#real_ip_header. diff --git a/charts/nginx-gateway-fabric/values.yaml b/charts/nginx-gateway-fabric/values.yaml index 9c14104dd3..758b12fc35 100644 --- a/charts/nginx-gateway-fabric/values.yaml +++ b/charts/nginx-gateway-fabric/values.yaml @@ -94,7 +94,7 @@ nginx: # disableHTTP2: false # ipFamily: dual # rewriteClientIP: - # mode: "ProxyProtocol" + # mode: "XForwadedFor" # # -- The trusted addresses field needs to be replaced with the load balancer's IP address. # trustedAddresses: [] # setIPRecursively: true diff --git a/internal/mode/static/nginx/config/servers.go b/internal/mode/static/nginx/config/servers.go index 1d19ddae97..92a3dcb015 100644 --- a/internal/mode/static/nginx/config/servers.go +++ b/internal/mode/static/nginx/config/servers.go @@ -98,7 +98,7 @@ func (g GeneratorImpl) newExecuteServersFunc(generator policies.Generator) execu } func (g GeneratorImpl) executeServers(conf dataplane.Configuration, generator policies.Generator) []executeResult { - servers, httpMatchPairs := createServers(conf.HTTPServers, conf.SSLServers, conf.TLSPassthroughServers, generator) + servers, httpMatchPairs := createServers(conf, generator) serverConfig := http.ServerConfig{ Servers: servers, @@ -172,28 +172,23 @@ func createIncludeFileResults(servers []http.Server) []executeResult { return results } -func createServers( - httpServers, - sslServers []dataplane.VirtualServer, - tlsPassthroughServers []dataplane.Layer4VirtualServer, - generator policies.Generator, -) ([]http.Server, httpMatchPairs) { - servers := make([]http.Server, 0, len(httpServers)+len(sslServers)) +func createServers(conf dataplane.Configuration, generator policies.Generator) ([]http.Server, httpMatchPairs) { + servers := make([]http.Server, 0, len(conf.HTTPServers)+len(conf.SSLServers)) finalMatchPairs := make(httpMatchPairs) sharedTLSPorts := make(map[int32]struct{}) - for _, passthroughServer := range tlsPassthroughServers { + for _, passthroughServer := range conf.TLSPassthroughServers { sharedTLSPorts[passthroughServer.Port] = struct{}{} } - for idx, s := range httpServers { + for idx, s := range conf.HTTPServers { serverID := fmt.Sprintf("%d", idx) httpServer, matchPairs := createServer(s, serverID, generator) servers = append(servers, httpServer) maps.Copy(finalMatchPairs, matchPairs) } - for idx, s := range sslServers { + for idx, s := range conf.SSLServers { serverID := fmt.Sprintf("SSL_%d", idx) sslServer, matchPairs := createSSLServer(s, serverID, generator) @@ -909,11 +904,16 @@ func isNonSlashedPrefixPath(pathType dataplane.PathType, path string) bool { } // getRewriteClientIPSettings returns the configuration for the rewriting client IP settings. -func getRewriteClientIPSettings(rewriteIP dataplane.RewriteClientIPSettings) shared.RewriteClientIPSettings { +func getRewriteClientIPSettings(rewriteIPConfig dataplane.RewriteClientIPSettings) shared.RewriteClientIPSettings { + var proxyProtocol string + if rewriteIPConfig.Mode == dataplane.RewriteIPModeProxyProtocol { + proxyProtocol = shared.ProxyProtocolDirective + } + return shared.RewriteClientIPSettings{ - Recursive: rewriteIP.IPRecursive, - ProxyProtocol: rewriteIP.Mode == dataplane.RewriteIPModeProxyProtocol, - RealIPFrom: rewriteIP.TrustedCIDRs, - RealIPHeader: string(rewriteIP.Mode), + RealIPHeader: string(rewriteIPConfig.Mode), + RealIPFrom: rewriteIPConfig.TrustedCIDRs, + Recursive: rewriteIPConfig.IPRecursive, + ProxyProtocol: proxyProtocol, } } diff --git a/internal/mode/static/nginx/config/servers_template.go b/internal/mode/static/nginx/config/servers_template.go index 7a6dded313..7437c62917 100644 --- a/internal/mode/static/nginx/config/servers_template.go +++ b/internal/mode/static/nginx/config/servers_template.go @@ -2,54 +2,44 @@ package config const serversTemplateText = ` js_preload_object matches from /etc/nginx/conf.d/matches.json; -{{ $proxyProtocol := "" }} -{{ if $.RewriteClientIP.ProxyProtocol }}{{ $proxyProtocol = " proxy_protocol" }}{{ end }} {{- range $s := .Servers -}} {{ if $s.IsDefaultSSL -}} server { {{- if or ($.IPFamily.IPv4) ($s.IsSocket) }} - listen {{ $s.Listen }} ssl default_server{{ $proxyProtocol }}; + listen {{ $s.Listen }} ssl default_server{{ $.RewriteClientIP.ProxyProtocol }}; {{- end }} {{- if and ($.IPFamily.IPv6) (not $s.IsSocket) }} - listen [::]:{{ $s.Listen }} ssl default_server{{ $proxyProtocol }}; + listen [::]:{{ $s.Listen }} ssl default_server{{ $.RewriteClientIP.ProxyProtocol }}; {{- end }} ssl_reject_handshake on; - {{- if and ($.RewriteClientIP.ProxyProtocol) ($s.IsSocket)}} - set_real_ip_from unix:; - {{- else if (not $s.IsSocket)}} - {{- range $cidr := $.RewriteClientIP.RealIPFrom }} + {{- range $cidr := $.RewriteClientIP.RealIPFrom }} set_real_ip_from {{ $cidr }}; - {{- end}} - {{ end }} - {{- if and ($.RewriteClientIP.RealIPHeader) (not $s.IsSocket)}} + {{- end}} + {{- if $.RewriteClientIP.RealIPHeader}} real_ip_header {{ $.RewriteClientIP.RealIPHeader }}; {{- end}} - {{- if and ($.RewriteClientIP.Recursive) (not $s.IsSocket)}} + {{- if $.RewriteClientIP.Recursive}} real_ip_recursive on; - {{ end }} + {{- end }} } {{- else if $s.IsDefaultHTTP }} server { {{- if $.IPFamily.IPv4 }} - listen {{ $s.Listen }} default_server{{ $proxyProtocol }}; + listen {{ $s.Listen }} default_server{{ $.RewriteClientIP.ProxyProtocol }}; {{- end }} {{- if $.IPFamily.IPv6 }} - listen [::]:{{ $s.Listen }} default_server{{ $proxyProtocol }}; + listen [::]:{{ $s.Listen }} default_server{{ $.RewriteClientIP.ProxyProtocol }}; {{- end }} - {{- if and ($.RewriteClientIP.ProxyProtocol) ($s.IsSocket)}} - set_real_ip_from unix:; - {{- else if (not $s.IsSocket)}} - {{- range $cidr := $.RewriteClientIP.RealIPFrom }} + {{- range $cidr := $.RewriteClientIP.RealIPFrom }} set_real_ip_from {{ $cidr }}; - {{- end}} - {{ end }} - {{- if and ($.RewriteClientIP.RealIPHeader) (not $s.IsSocket)}} + {{- end}} + {{- if $.RewriteClientIP.RealIPHeader}} real_ip_header {{ $.RewriteClientIP.RealIPHeader }}; {{- end}} - {{- if and ($.RewriteClientIP.Recursive) (not $s.IsSocket)}} + {{- if $.RewriteClientIP.Recursive}} real_ip_recursive on; - {{ end }} + {{- end }} default_type text/html; return 404; } @@ -57,10 +47,10 @@ server { server { {{- if $s.SSL }} {{- if or ($.IPFamily.IPv4) ($s.IsSocket) }} - listen {{ $s.Listen }} ssl{{ $proxyProtocol }}; + listen {{ $s.Listen }} ssl{{ $.RewriteClientIP.ProxyProtocol }}; {{- end }} {{- if and ($.IPFamily.IPv6) (not $s.IsSocket) }} - listen [::]:{{ $s.Listen }} ssl{{ $proxyProtocol }}; + listen [::]:{{ $s.Listen }} ssl{{ $.RewriteClientIP.ProxyProtocol }}; {{- end }} ssl_certificate {{ $s.SSL.Certificate }}; ssl_certificate_key {{ $s.SSL.CertificateKey }}; @@ -70,10 +60,10 @@ server { } {{- else }} {{- if $.IPFamily.IPv4 }} - listen {{ $s.Listen }}{{ $proxyProtocol }}; + listen {{ $s.Listen }}{{ $.RewriteClientIP.ProxyProtocol }}; {{- end }} {{- if $.IPFamily.IPv6 }} - listen [::]:{{ $s.Listen }}{{ $proxyProtocol }}; + listen [::]:{{ $s.Listen }}{{ $.RewriteClientIP.ProxyProtocol }}; {{- end }} {{- end }} @@ -87,19 +77,15 @@ server { include {{ $i.Name }}; {{- end }} - {{- if and ($.RewriteClientIP.ProxyProtocol) ($s.IsSocket)}} - set_real_ip_from unix:; - {{- else if (not $s.IsSocket)}} {{- range $cidr := $.RewriteClientIP.RealIPFrom }} set_real_ip_from {{ $cidr }}; {{- end}} - {{ end }} - {{- if and ($.RewriteClientIP.RealIPHeader) (not $s.IsSocket)}} + {{- if $.RewriteClientIP.RealIPHeader}} real_ip_header {{ $.RewriteClientIP.RealIPHeader }}; {{- end}} - {{- if and ($.RewriteClientIP.Recursive) (not $s.IsSocket)}} + {{- if $.RewriteClientIP.Recursive}} real_ip_recursive on; - {{ end }} + {{- end }} {{ range $l := $s.Locations }} location {{ $l.Path }} { diff --git a/internal/mode/static/nginx/config/servers_test.go b/internal/mode/static/nginx/config/servers_test.go index 5946106a56..b969ce7141 100644 --- a/internal/mode/static/nginx/config/servers_test.go +++ b/internal/mode/static/nginx/config/servers_test.go @@ -332,6 +332,7 @@ func TestExecuteServers_RewriteClientIP(t *testing.T) { "real_ip_header proxy_protocol;": 4, "real_ip_recursive on;": 4, "proxy_protocol on;": 0, + "set_real_ip_from unix:;": 0, "listen 8080 default_server proxy_protocol;": 1, "listen 8080 proxy_protocol;": 1, "listen 8443 ssl default_server proxy_protocol;": 1, @@ -914,44 +915,44 @@ func TestCreateServers(t *testing.T) { }, } - httpServers := []dataplane.VirtualServer{ - { - IsDefault: true, - Port: 8080, - }, - { - Hostname: "cafe.example.com", - PathRules: cafePathRules, - Port: 8080, - Policies: []policies.Policy{ - &policiesfakes.FakePolicy{}, - &policiesfakes.FakePolicy{}, + conf := dataplane.Configuration{ + HTTPServers: []dataplane.VirtualServer{ + { + IsDefault: true, + Port: 8080, + }, + { + Hostname: "cafe.example.com", + PathRules: cafePathRules, + Port: 8080, + Policies: []policies.Policy{ + &policiesfakes.FakePolicy{}, + &policiesfakes.FakePolicy{}, + }, }, }, - } - - sslServers := []dataplane.VirtualServer{ - { - IsDefault: true, - Port: 8443, - }, - { - Hostname: "cafe.example.com", - SSL: &dataplane.SSL{KeyPairID: sslKeyPairID}, - PathRules: cafePathRules, - Port: 8443, - Policies: []policies.Policy{ - &policiesfakes.FakePolicy{}, - &policiesfakes.FakePolicy{}, + SSLServers: []dataplane.VirtualServer{ + { + IsDefault: true, + Port: 8443, + }, + { + Hostname: "cafe.example.com", + SSL: &dataplane.SSL{KeyPairID: sslKeyPairID}, + PathRules: cafePathRules, + Port: 8443, + Policies: []policies.Policy{ + &policiesfakes.FakePolicy{}, + &policiesfakes.FakePolicy{}, + }, }, }, - } - - tlsPassthroughServers := []dataplane.Layer4VirtualServer{ - { - Hostname: "app.example.com", - Port: 8443, - UpstreamName: "sup", + TLSPassthroughServers: []dataplane.Layer4VirtualServer{ + { + Hostname: "app.example.com", + Port: 8443, + UpstreamName: "sup", + }, }, } @@ -1481,7 +1482,7 @@ func TestCreateServers(t *testing.T) { }, }) - result, httpMatchPair := createServers(httpServers, sslServers, tlsPassthroughServers, fakeGenerator) + result, httpMatchPair := createServers(conf, fakeGenerator) g.Expect(httpMatchPair).To(Equal(allExpMatchPair)) g.Expect(helpers.Diff(expectedServers, result)).To(BeEmpty()) @@ -1696,12 +1697,7 @@ func TestCreateServersConflicts(t *testing.T) { g := NewWithT(t) - result, _ := createServers( - httpServers, - []dataplane.VirtualServer{}, - []dataplane.Layer4VirtualServer{}, - &policiesfakes.FakeGenerator{}, - ) + result, _ := createServers(dataplane.Configuration{HTTPServers: httpServers}, &policiesfakes.FakeGenerator{}) g.Expect(helpers.Diff(expectedServers, result)).To(BeEmpty()) }) } diff --git a/internal/mode/static/nginx/config/shared/config.go b/internal/mode/static/nginx/config/shared/config.go index eada321715..62ea4bec82 100644 --- a/internal/mode/static/nginx/config/shared/config.go +++ b/internal/mode/static/nginx/config/shared/config.go @@ -23,7 +23,11 @@ type IPFamily struct { // RewriteClientIP holds the configuration for the rewrite client IP settings. type RewriteClientIPSettings struct { RealIPHeader string + ProxyProtocol string RealIPFrom []string Recursive bool - ProxyProtocol bool } + +const ( + ProxyProtocolDirective = " proxy_protocol" +) diff --git a/internal/mode/static/nginx/config/stream/config.go b/internal/mode/static/nginx/config/stream/config.go index 0bbebadc20..ddc215eea7 100644 --- a/internal/mode/static/nginx/config/stream/config.go +++ b/internal/mode/static/nginx/config/stream/config.go @@ -4,12 +4,13 @@ import "github.com/nginxinc/nginx-gateway-fabric/internal/mode/static/nginx/conf // Server holds all configuration for a stream server. type Server struct { - Listen string - StatusZone string - ProxyPass string - Pass string - SSLPreread bool - IsSocket bool + Listen string + StatusZone string + ProxyPass string + Pass string + RewriteClientIP shared.RewriteClientIPSettings + SSLPreread bool + IsSocket bool } // Upstream holds all configuration for a stream upstream. @@ -26,8 +27,7 @@ type UpstreamServer struct { // ServerConfig holds configuration for a stream server and IP family to be used by NGINX. type ServerConfig struct { - Servers []Server - RewriteClientIP shared.RewriteClientIPSettings - IPFamily shared.IPFamily - Plus bool + Servers []Server + IPFamily shared.IPFamily + Plus bool } diff --git a/internal/mode/static/nginx/config/stream_servers.go b/internal/mode/static/nginx/config/stream_servers.go index 73d90a7de7..c6bad43002 100644 --- a/internal/mode/static/nginx/config/stream_servers.go +++ b/internal/mode/static/nginx/config/stream_servers.go @@ -5,6 +5,7 @@ import ( gotemplate "text/template" "github.com/nginxinc/nginx-gateway-fabric/internal/framework/helpers" + "github.com/nginxinc/nginx-gateway-fabric/internal/mode/static/nginx/config/shared" "github.com/nginxinc/nginx-gateway-fabric/internal/mode/static/nginx/config/stream" "github.com/nginxinc/nginx-gateway-fabric/internal/mode/static/state/dataplane" ) @@ -15,10 +16,9 @@ func (g GeneratorImpl) executeStreamServers(conf dataplane.Configuration) []exec streamServers := createStreamServers(conf) streamServerConfig := stream.ServerConfig{ - Servers: streamServers, - IPFamily: getIPFamily(conf.BaseHTTPConfig), - Plus: g.plus, - RewriteClientIP: getRewriteClientIPSettings(conf.BaseHTTPConfig.RewriteClientIPSettings), + Servers: streamServers, + IPFamily: getIPFamily(conf.BaseHTTPConfig), + Plus: g.plus, } streamServerResult := executeResult{ @@ -37,6 +37,7 @@ func createStreamServers(conf dataplane.Configuration) []stream.Server { } streamServers := make([]stream.Server, 0, len(conf.TLSPassthroughServers)*2) + var streamServer stream.Server portSet := make(map[int32]struct{}) upstreams := make(map[string]dataplane.Upstream) @@ -47,12 +48,17 @@ func createStreamServers(conf dataplane.Configuration) []stream.Server { for _, server := range conf.TLSPassthroughServers { if u, ok := upstreams[server.UpstreamName]; ok && server.UpstreamName != "" { if server.Hostname != "" && len(u.Endpoints) > 0 { - streamServers = append(streamServers, stream.Server{ + streamServer = stream.Server{ Listen: getSocketNameTLS(server.Port, server.Hostname), StatusZone: server.Hostname, ProxyPass: server.UpstreamName, IsSocket: true, - }) + } + streamServer.RewriteClientIP = getRewriteClientIPSettingsForStream( + conf.BaseHTTPConfig.RewriteClientIPSettings, + streamServer.IsSocket, + ) + streamServers = append(streamServers, streamServer) } } @@ -61,13 +67,34 @@ func createStreamServers(conf dataplane.Configuration) []stream.Server { } portSet[server.Port] = struct{}{} - streamServers = append(streamServers, stream.Server{ + + streamServer = stream.Server{ Listen: fmt.Sprint(server.Port), StatusZone: server.Hostname, Pass: getTLSPassthroughVarName(server.Port), SSLPreread: true, - }) + } + streamServer.RewriteClientIP = getRewriteClientIPSettingsForStream( + conf.BaseHTTPConfig.RewriteClientIPSettings, + streamServer.IsSocket, + ) + streamServers = append(streamServers, streamServer) } return streamServers } + +func getRewriteClientIPSettingsForStream( + rewriteConfig dataplane.RewriteClientIPSettings, + isSocket bool, +) shared.RewriteClientIPSettings { + proxyEnabled := rewriteConfig.Mode == dataplane.RewriteIPModeProxyProtocol + if isSocket && proxyEnabled { + return shared.RewriteClientIPSettings{ + ProxyProtocol: shared.ProxyProtocolDirective, + RealIPFrom: rewriteConfig.TrustedCIDRs, + } + } + + return shared.RewriteClientIPSettings{} +} diff --git a/internal/mode/static/nginx/config/stream_servers_template.go b/internal/mode/static/nginx/config/stream_servers_template.go index 2b98944df8..2944a28837 100644 --- a/internal/mode/static/nginx/config/stream_servers_template.go +++ b/internal/mode/static/nginx/config/stream_servers_template.go @@ -1,24 +1,20 @@ package config const streamServersTemplateText = ` -{{ $proxyProtocol := "" }} -{{ if $.RewriteClientIP.ProxyProtocol }}{{ $proxyProtocol = " proxy_protocol" }}{{ end }} {{- range $s := .Servers }} server { {{- if and ($.IPFamily.IPv4) (not $s.IsSocket) }} listen {{ $s.Listen }}; {{- else if $s.IsSocket }} - listen {{ $s.Listen }}{{ $proxyProtocol }}; + listen {{ $s.Listen }}{{ $s.RewriteClientIP.ProxyProtocol }}; {{- end }} {{- if and ($.IPFamily.IPv6) (not $s.IsSocket) }} listen [::]:{{ $s.Listen }}; {{- end }} - {{- if and ($s.IsSocket) ($.RewriteClientIP.ProxyProtocol) }} - {{- range $cidr := $.RewriteClientIP.RealIPFrom }} + {{- range $cidr := $s.RewriteClientIP.RealIPFrom }} set_real_ip_from {{ $cidr }}; - {{- end}} - {{- end }} + {{- end}} {{- if $.Plus }} status_zone {{ $s.StatusZone }}; {{- end }} diff --git a/internal/mode/static/nginx/config/stream_servers_test.go b/internal/mode/static/nginx/config/stream_servers_test.go index 9d7e0d9165..45bf3aa715 100644 --- a/internal/mode/static/nginx/config/stream_servers_test.go +++ b/internal/mode/static/nginx/config/stream_servers_test.go @@ -369,9 +369,10 @@ func TestExecuteStreamServers_RewriteClientIP(t *testing.T) { expectedStreamConfig: map[string]int{ "listen 8443;": 1, "listen [::]:8443;": 1, - "listen unix:/var/run/nginx/cafe.example.com-8443.sock;": 1, - "set_real_ip_from 1.1.1.1/32;": 0, - "real_ip_recursive on;": 0, + "listen unix:/var/run/nginx/cafe.example.com-8443.sock;": 1, + "set_real_ip_from 1.1.1.1/32;": 0, + "real_ip_recursive on;": 0, + "listen unix:/var/run/nginx/cafe.example.com-8443.sock proxy_protocol;": 0, }, }, } diff --git a/site/content/reference/api.md b/site/content/reference/api.md index 3518381a4a..e8e546ef5f 100644 --- a/site/content/reference/api.md +++ b/site/content/reference/api.md @@ -3,8 +3,11 @@ title: "API reference" weight: 100 toc: false --- + ## Overview + NGINX Gateway API Reference +

Packages:

  • @@ -1139,7 +1142,7 @@ This field is required if mode is set.

    "ProxyProtocol"

    -

    RewriteClientIPModeProxyProtocol configures NGINX to accept PROXY protocol and, +

    RewriteClientIPModeProxyProtocol configures NGINX to accept PROXY protocol and set the client’s IP address to the IP address in the PROXY protocol header. Sets the proxy_protocol parameter to the listen directive on all servers, and sets real_ip_header to proxy_protocol: https://nginx.org/en/docs/http/ngx_http_realip_module.html#real_ip_header.

    From 7f9af09f793a9ce6bdf406615de181280806efc7 Mon Sep 17 00:00:00 2001 From: Saloni Date: Thu, 29 Aug 2024 15:22:38 -0600 Subject: [PATCH 06/19] updates based on reviews --- apis/v1alpha1/nginxproxy_types.go | 12 +- charts/nginx-gateway-fabric/values.yaml | 4 +- .../bases/gateway.nginx.org_nginxproxies.yaml | 13 ++- deploy/crds.yaml | 13 ++- .../mode/static/nginx/config/http/config.go | 5 +- internal/mode/static/nginx/config/servers.go | 34 ++++-- .../static/nginx/config/servers_template.go | 4 +- .../mode/static/nginx/config/servers_test.go | 64 +++++++++-- .../static/nginx/config/stream_servers.go | 17 +-- .../nginx/config/stream_servers_template.go | 6 +- .../nginx/config/stream_servers_test.go | 21 ++-- .../static/state/dataplane/configuration.go | 6 +- .../state/dataplane/configuration_test.go | 24 ++-- internal/mode/static/state/dataplane/types.go | 2 +- .../mode/static/state/graph/nginxproxy.go | 13 ++- .../static/state/graph/nginxproxy_test.go | 31 +++-- .../how-to/monitoring/troubleshooting.md | 106 +++++++++--------- site/content/reference/api.md | 13 +-- 18 files changed, 231 insertions(+), 157 deletions(-) diff --git a/apis/v1alpha1/nginxproxy_types.go b/apis/v1alpha1/nginxproxy_types.go index 867e8113dd..f1d03b9e54 100644 --- a/apis/v1alpha1/nginxproxy_types.go +++ b/apis/v1alpha1/nginxproxy_types.go @@ -54,7 +54,7 @@ type NginxProxySpec struct { // +optional Telemetry *Telemetry `json:"telemetry,omitempty"` // RewriteClientIP defines configuration for rewriting the client IP to the original client's IP. - // +kubebuilder:validation:XValidation:message="if mode is set, trustedAddresses is a required field",rule="!(has(self.mode) && !has(self.trustedAddresses))" + // +kubebuilder:validation:XValidation:message="if mode is set, trustedAddresses is a required field",rule="!(has(self.mode) && (!has(self.trustedAddresses) || size(self.trustedAddresses) == 0))" // // +optional //nolint:lll @@ -132,12 +132,12 @@ type RewriteClientIP struct { // +optional Mode *RewriteClientIPModeType `json:"mode,omitempty"` - // SetIPRecursively configures whether recursive search is used when selecting the client's address from. + // 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. + // and TrustedAddresses is set to 55.55.55.1, 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 @@ -149,7 +149,7 @@ type RewriteClientIP struct { // 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. + // Addresses must be provided as CIDR blocks or IP address: 10.0.0.0, 192.33.21/24, fe80::1/128. // 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 @@ -179,8 +179,8 @@ const ( RewriteClientIPModeXForwardedFor RewriteClientIPModeType = "XForwardedFor" ) -// TrustedAddress is a string value representing a CIDR block. -// Examples: 0.0.0.0/0 +// TrustedAddress is a string value representing a CIDR block or an IP address. +// Examples: 10.0.0.2/32, 10.0.0.1, fe80::1/128, ::1/24. // // +kubebuilder:validation:Pattern=`^(?:(?:[0-9]{1,3}\.){3}[0-9]{1,3}(?:\/(?:[0-9]|[12][0-9]|3[0-2]))?|(?:[0-9a-fA-F]{1,4}:){7}[0-9a-fA-F]{1,4}(?:\/(?:[0-9]|[1-9][0-9]|1[0-1][0-9]|12[0-8]))?)$` // diff --git a/charts/nginx-gateway-fabric/values.yaml b/charts/nginx-gateway-fabric/values.yaml index 758b12fc35..322776125d 100644 --- a/charts/nginx-gateway-fabric/values.yaml +++ b/charts/nginx-gateway-fabric/values.yaml @@ -94,8 +94,8 @@ nginx: # disableHTTP2: false # ipFamily: dual # rewriteClientIP: - # mode: "XForwadedFor" - # # -- The trusted addresses field needs to be replaced with the load balancer's IP address. + # mode: "XForwardedFor" + # # -- Set to the CIDR range or IP of the proxy that sits in front of NGINX Gateway Fabric. # trustedAddresses: [] # setIPRecursively: true # telemetry: diff --git a/config/crd/bases/gateway.nginx.org_nginxproxies.yaml b/config/crd/bases/gateway.nginx.org_nginxproxies.yaml index 81ea4c1bd9..3f9fd62e9f 100644 --- a/config/crd/bases/gateway.nginx.org_nginxproxies.yaml +++ b/config/crd/bases/gateway.nginx.org_nginxproxies.yaml @@ -79,12 +79,12 @@ spec: type: string setIPRecursively: description: |- - SetIPRecursively configures whether recursive search is used when selecting the client's address from. + 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. + and TrustedAddresses is set to 55.55.55.1, 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 @@ -95,15 +95,15 @@ spec: 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. + Addresses must be provided as CIDR blocks or IP address: 10.0.0.0, 192.33.21/24, fe80::1/128. 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: |- - TrustedAddress is a string value representing a CIDR block. - Examples: 0.0.0.0/0 + TrustedAddress is a string value representing a CIDR block or an IP address. + Examples: 10.0.0.2/32, 10.0.0.1, fe80::1/128, ::1/24. pattern: ^(?:(?:[0-9]{1,3}\.){3}[0-9]{1,3}(?:\/(?:[0-9]|[12][0-9]|3[0-2]))?|(?:[0-9a-fA-F]{1,4}:){7}[0-9a-fA-F]{1,4}(?:\/(?:[0-9]|[1-9][0-9]|1[0-1][0-9]|12[0-8]))?)$ type: string maxItems: 16 @@ -112,7 +112,8 @@ spec: type: object x-kubernetes-validations: - message: if mode is set, trustedAddresses is a required field - rule: '!(has(self.mode) && !has(self.trustedAddresses))' + rule: '!(has(self.mode) && (!has(self.trustedAddresses) || size(self.trustedAddresses) + == 0))' telemetry: description: Telemetry specifies the OpenTelemetry configuration. properties: diff --git a/deploy/crds.yaml b/deploy/crds.yaml index bd19100a3f..1c6163fb1c 100644 --- a/deploy/crds.yaml +++ b/deploy/crds.yaml @@ -664,12 +664,12 @@ spec: type: string setIPRecursively: description: |- - SetIPRecursively configures whether recursive search is used when selecting the client's address from. + 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. + and TrustedAddresses is set to 55.55.55.1, 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 @@ -680,15 +680,15 @@ spec: 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. + Addresses must be provided as CIDR blocks or IP address: 10.0.0.0, 192.33.21/24, fe80::1/128. 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: |- - TrustedAddress is a string value representing a CIDR block. - Examples: 0.0.0.0/0 + TrustedAddress is a string value representing a CIDR block or an IP address. + Examples: 10.0.0.2/32, 10.0.0.1, fe80::1/128, ::1/24. pattern: ^(?:(?:[0-9]{1,3}\.){3}[0-9]{1,3}(?:\/(?:[0-9]|[12][0-9]|3[0-2]))?|(?:[0-9a-fA-F]{1,4}:){7}[0-9a-fA-F]{1,4}(?:\/(?:[0-9]|[1-9][0-9]|1[0-1][0-9]|12[0-8]))?)$ type: string maxItems: 16 @@ -697,7 +697,8 @@ spec: type: object x-kubernetes-validations: - message: if mode is set, trustedAddresses is a required field - rule: '!(has(self.mode) && !has(self.trustedAddresses))' + rule: '!(has(self.mode) && (!has(self.trustedAddresses) || size(self.trustedAddresses) + == 0))' telemetry: description: Telemetry specifies the OpenTelemetry configuration. properties: diff --git a/internal/mode/static/nginx/config/http/config.go b/internal/mode/static/nginx/config/http/config.go index ee45daf98c..f17d51e5d5 100644 --- a/internal/mode/static/nginx/config/http/config.go +++ b/internal/mode/static/nginx/config/http/config.go @@ -2,7 +2,10 @@ package http import "github.com/nginxinc/nginx-gateway-fabric/internal/mode/static/nginx/config/shared" -const InternalRoutePathPrefix = "/_ngf-internal" +const ( + InternalRoutePathPrefix = "/_ngf-internal" + HTTPSScheme = "https" +) // Server holds all configuration for an HTTP server. type Server struct { diff --git a/internal/mode/static/nginx/config/servers.go b/internal/mode/static/nginx/config/servers.go index 92a3dcb015..eeac39944f 100644 --- a/internal/mode/static/nginx/config/servers.go +++ b/internal/mode/static/nginx/config/servers.go @@ -800,14 +800,7 @@ func generateProxySetHeaders(filters *dataplane.HTTPFilters, grpc bool) []http.H copy(headers, grpcBaseHeaders) } - if filters != nil && filters.RequestURLRewrite != nil && filters.RequestURLRewrite.Hostname != nil { - for i, header := range headers { - if header.Name == "Host" { - headers[i].Value = *filters.RequestURLRewrite.Hostname - break - } - } - } + setHeaderForHTTPSRedirect(filters, headers) if filters == nil || filters.RequestHeaderModifiers == nil { return headers @@ -836,6 +829,29 @@ func generateProxySetHeaders(filters *dataplane.HTTPFilters, grpc bool) []http.H return append(proxySetHeaders, headers...) } +func setHeaderForHTTPSRedirect(filters *dataplane.HTTPFilters, headers []http.Header) { + if filters != nil { + if filters.RequestURLRewrite != nil && filters.RequestURLRewrite.Hostname != nil { + for i, header := range headers { + if header.Name == "Host" { + headers[i].Value = *filters.RequestURLRewrite.Hostname + break + } + } + } + if filters.RequestRedirect != nil && + filters.RequestRedirect.Scheme != nil && + *filters.RequestRedirect.Scheme == http.HTTPSScheme { + for i, header := range headers { + if header.Name == "X-Forwarded-Proto" { + headers[i].Value = http.HTTPSScheme + return + } + } + } + } +} + func generateResponseHeaders(filters *dataplane.HTTPFilters) http.ResponseHeaders { if filters == nil || filters.ResponseHeaderModifiers == nil { return http.ResponseHeaders{} @@ -912,7 +928,7 @@ func getRewriteClientIPSettings(rewriteIPConfig dataplane.RewriteClientIPSetting return shared.RewriteClientIPSettings{ RealIPHeader: string(rewriteIPConfig.Mode), - RealIPFrom: rewriteIPConfig.TrustedCIDRs, + RealIPFrom: rewriteIPConfig.TrustedAddresses, Recursive: rewriteIPConfig.IPRecursive, ProxyProtocol: proxyProtocol, } diff --git a/internal/mode/static/nginx/config/servers_template.go b/internal/mode/static/nginx/config/servers_template.go index 7437c62917..3c851457d2 100644 --- a/internal/mode/static/nginx/config/servers_template.go +++ b/internal/mode/static/nginx/config/servers_template.go @@ -13,7 +13,7 @@ server { listen [::]:{{ $s.Listen }} ssl default_server{{ $.RewriteClientIP.ProxyProtocol }}; {{- end }} ssl_reject_handshake on; - {{- range $cidr := $.RewriteClientIP.RealIPFrom }} + {{- range $cidr := $.RewriteClientIP.RealIPFrom }} set_real_ip_from {{ $cidr }}; {{- end}} {{- if $.RewriteClientIP.RealIPHeader}} @@ -77,7 +77,7 @@ server { include {{ $i.Name }}; {{- end }} - {{- range $cidr := $.RewriteClientIP.RealIPFrom }} + {{- range $cidr := $.RewriteClientIP.RealIPFrom }} set_real_ip_from {{ $cidr }}; {{- end}} {{- if $.RewriteClientIP.RealIPHeader}} diff --git a/internal/mode/static/nginx/config/servers_test.go b/internal/mode/static/nginx/config/servers_test.go index b969ce7141..1c21ec05d0 100644 --- a/internal/mode/static/nginx/config/servers_test.go +++ b/internal/mode/static/nginx/config/servers_test.go @@ -321,18 +321,15 @@ func TestExecuteServers_RewriteClientIP(t *testing.T) { BaseHTTPConfig: dataplane.BaseHTTPConfig{ IPFamily: dataplane.Dual, RewriteClientIPSettings: dataplane.RewriteClientIPSettings{ - Mode: dataplane.RewriteIPModeProxyProtocol, - TrustedCIDRs: []string{"0.0.0.0/0"}, - IPRecursive: true, + Mode: dataplane.RewriteIPModeProxyProtocol, + TrustedAddresses: []string{"10.56.73.51/32"}, + IPRecursive: false, }, }, }, expectedHTTPConfig: map[string]int{ - "set_real_ip_from 0.0.0.0/0;": 4, + "set_real_ip_from 10.56.73.51/32;": 4, "real_ip_header proxy_protocol;": 4, - "real_ip_recursive on;": 4, - "proxy_protocol on;": 0, - "set_real_ip_from unix:;": 0, "listen 8080 default_server proxy_protocol;": 1, "listen 8080 proxy_protocol;": 1, "listen 8443 ssl default_server proxy_protocol;": 1, @@ -355,17 +352,18 @@ func TestExecuteServers_RewriteClientIP(t *testing.T) { BaseHTTPConfig: dataplane.BaseHTTPConfig{ IPFamily: dataplane.Dual, RewriteClientIPSettings: dataplane.RewriteClientIPSettings{ - Mode: dataplane.RewriteIPModeXForwardedFor, - TrustedCIDRs: []string{"0.0.0.0/0"}, - IPRecursive: true, + Mode: dataplane.RewriteIPModeXForwardedFor, + TrustedAddresses: []string{"10.1.1.3/32", "2.2.2.2", "2001:db8::/32"}, + IPRecursive: true, }, }, }, expectedHTTPConfig: map[string]int{ - "set_real_ip_from 0.0.0.0/0;": 4, + "set_real_ip_from 10.1.1.3/32;": 4, + "set_real_ip_from 2.2.2.2;": 4, + "set_real_ip_from 2001:db8::/32;": 4, "real_ip_header X-Forwarded-For;": 4, "real_ip_recursive on;": 4, - "proxy_protocol on;": 0, "listen 8080 default_server;": 1, "listen 8080;": 1, "listen 8443 ssl default_server;": 1, @@ -2682,6 +2680,48 @@ func TestGenerateProxySetHeaders(t *testing.T) { }, }, }, + { + msg: "request redirect filter with https scheme", + filters: &dataplane.HTTPFilters{ + RequestRedirect: &dataplane.HTTPRequestRedirectFilter{ + Scheme: helpers.GetPointer("https"), + }, + }, + expectedHeaders: []http.Header{ + { + Name: "Host", + Value: "$gw_api_compliant_host", + }, + { + Name: "X-Forwarded-For", + Value: "$proxy_add_x_forwarded_for", + }, + { + Name: "Upgrade", + Value: "$http_upgrade", + }, + { + Name: "Connection", + Value: "$connection_upgrade", + }, + { + Name: "X-Real-IP", + Value: "$remote_addr", + }, + { + Name: "X-Forwarded-Proto", + Value: "https", + }, + { + Name: "X-Forwarded-Host", + Value: "$host", + }, + { + Name: "X-Forwarded-Port", + Value: "$server_port", + }, + }, + }, } for _, tc := range tests { diff --git a/internal/mode/static/nginx/config/stream_servers.go b/internal/mode/static/nginx/config/stream_servers.go index c6bad43002..4cef5de2f1 100644 --- a/internal/mode/static/nginx/config/stream_servers.go +++ b/internal/mode/static/nginx/config/stream_servers.go @@ -37,7 +37,6 @@ func createStreamServers(conf dataplane.Configuration) []stream.Server { } streamServers := make([]stream.Server, 0, len(conf.TLSPassthroughServers)*2) - var streamServer stream.Server portSet := make(map[int32]struct{}) upstreams := make(map[string]dataplane.Upstream) @@ -48,15 +47,15 @@ func createStreamServers(conf dataplane.Configuration) []stream.Server { for _, server := range conf.TLSPassthroughServers { if u, ok := upstreams[server.UpstreamName]; ok && server.UpstreamName != "" { if server.Hostname != "" && len(u.Endpoints) > 0 { - streamServer = stream.Server{ + streamServer := stream.Server{ Listen: getSocketNameTLS(server.Port, server.Hostname), StatusZone: server.Hostname, ProxyPass: server.UpstreamName, IsSocket: true, } + // set rewriteClientIP settings as this is a socket stream server streamServer.RewriteClientIP = getRewriteClientIPSettingsForStream( conf.BaseHTTPConfig.RewriteClientIPSettings, - streamServer.IsSocket, ) streamServers = append(streamServers, streamServer) } @@ -68,31 +67,25 @@ func createStreamServers(conf dataplane.Configuration) []stream.Server { portSet[server.Port] = struct{}{} - streamServer = stream.Server{ + streamServer := stream.Server{ Listen: fmt.Sprint(server.Port), StatusZone: server.Hostname, Pass: getTLSPassthroughVarName(server.Port), SSLPreread: true, } - streamServer.RewriteClientIP = getRewriteClientIPSettingsForStream( - conf.BaseHTTPConfig.RewriteClientIPSettings, - streamServer.IsSocket, - ) streamServers = append(streamServers, streamServer) } - return streamServers } func getRewriteClientIPSettingsForStream( rewriteConfig dataplane.RewriteClientIPSettings, - isSocket bool, ) shared.RewriteClientIPSettings { proxyEnabled := rewriteConfig.Mode == dataplane.RewriteIPModeProxyProtocol - if isSocket && proxyEnabled { + if proxyEnabled { return shared.RewriteClientIPSettings{ ProxyProtocol: shared.ProxyProtocolDirective, - RealIPFrom: rewriteConfig.TrustedCIDRs, + RealIPFrom: rewriteConfig.TrustedAddresses, } } diff --git a/internal/mode/static/nginx/config/stream_servers_template.go b/internal/mode/static/nginx/config/stream_servers_template.go index 2944a28837..0a84e02338 100644 --- a/internal/mode/static/nginx/config/stream_servers_template.go +++ b/internal/mode/static/nginx/config/stream_servers_template.go @@ -3,10 +3,8 @@ package config const streamServersTemplateText = ` {{- range $s := .Servers }} server { - {{- if and ($.IPFamily.IPv4) (not $s.IsSocket) }} - listen {{ $s.Listen }}; - {{- else if $s.IsSocket }} - listen {{ $s.Listen }}{{ $s.RewriteClientIP.ProxyProtocol }}; + {{- if or ($.IPFamily.IPv4) ($s.IsSocket) }} + listen {{ $s.Listen }}{{ $s.RewriteClientIP.ProxyProtocol }}; {{- end }} {{- if and ($.IPFamily.IPv6) (not $s.IsSocket) }} listen [::]:{{ $s.Listen }}; diff --git a/internal/mode/static/nginx/config/stream_servers_test.go b/internal/mode/static/nginx/config/stream_servers_test.go index 45bf3aa715..322e474e2f 100644 --- a/internal/mode/static/nginx/config/stream_servers_test.go +++ b/internal/mode/static/nginx/config/stream_servers_test.go @@ -337,9 +337,9 @@ func TestExecuteStreamServers_RewriteClientIP(t *testing.T) { config: dataplane.Configuration{ BaseHTTPConfig: dataplane.BaseHTTPConfig{ RewriteClientIPSettings: dataplane.RewriteClientIPSettings{ - Mode: dataplane.RewriteIPModeProxyProtocol, - TrustedCIDRs: []string{"1.1.1.1/32"}, - IPRecursive: true, + Mode: dataplane.RewriteIPModeProxyProtocol, + TrustedAddresses: []string{"10.1.1.22/32", "::1/128", "3.4.5.6"}, + IPRecursive: false, }, }, TLSPassthroughServers: passThroughServers, @@ -349,7 +349,9 @@ func TestExecuteStreamServers_RewriteClientIP(t *testing.T) { "listen 8443;": 1, "listen [::]:8443;": 1, "listen unix:/var/run/nginx/cafe.example.com-8443.sock proxy_protocol;": 1, - "set_real_ip_from 1.1.1.1/32;": 1, + "set_real_ip_from 10.1.1.22/32;": 1, + "set_real_ip_from ::1/128;": 1, + "set_real_ip_from 3.4.5.6;": 1, "real_ip_recursive on;": 0, }, }, @@ -358,9 +360,9 @@ func TestExecuteStreamServers_RewriteClientIP(t *testing.T) { config: dataplane.Configuration{ BaseHTTPConfig: dataplane.BaseHTTPConfig{ RewriteClientIPSettings: dataplane.RewriteClientIPSettings{ - Mode: dataplane.RewriteIPModeXForwardedFor, - TrustedCIDRs: []string{"1.1.1.1/32"}, - IPRecursive: true, + Mode: dataplane.RewriteIPModeXForwardedFor, + TrustedAddresses: []string{"1.1.1.1/32"}, + IPRecursive: true, }, }, TLSPassthroughServers: passThroughServers, @@ -369,10 +371,7 @@ func TestExecuteStreamServers_RewriteClientIP(t *testing.T) { expectedStreamConfig: map[string]int{ "listen 8443;": 1, "listen [::]:8443;": 1, - "listen unix:/var/run/nginx/cafe.example.com-8443.sock;": 1, - "set_real_ip_from 1.1.1.1/32;": 0, - "real_ip_recursive on;": 0, - "listen unix:/var/run/nginx/cafe.example.com-8443.sock proxy_protocol;": 0, + "listen unix:/var/run/nginx/cafe.example.com-8443.sock;": 1, }, }, } diff --git a/internal/mode/static/state/dataplane/configuration.go b/internal/mode/static/state/dataplane/configuration.go index 54e40232ee..44479f87dd 100644 --- a/internal/mode/static/state/dataplane/configuration.go +++ b/internal/mode/static/state/dataplane/configuration.go @@ -863,8 +863,8 @@ func buildBaseHTTPConfig(g *graph.Graph) BaseHTTPConfig { } if len(g.NginxProxy.Source.Spec.RewriteClientIP.TrustedAddresses) > 0 { - trustedAddresses := convertTrustedCIDRs(g) - baseConfig.RewriteClientIPSettings.TrustedCIDRs = trustedAddresses + trustedAddresses := convertTrustedAddresses(g) + baseConfig.RewriteClientIPSettings.TrustedAddresses = trustedAddresses } if g.NginxProxy.Source.Spec.RewriteClientIP.SetIPRecursively != nil { @@ -893,7 +893,7 @@ func buildPolicies(graphPolicies []*graph.Policy) []policies.Policy { return finalPolicies } -func convertTrustedCIDRs(g *graph.Graph) []string { +func convertTrustedAddresses(g *graph.Graph) []string { trustedAddresses := make([]string, len(g.NginxProxy.Source.Spec.RewriteClientIP.TrustedAddresses)) for i, addr := range g.NginxProxy.Source.Spec.RewriteClientIP.TrustedAddresses { trustedAddresses[i] = string(addr) diff --git a/internal/mode/static/state/dataplane/configuration_test.go b/internal/mode/static/state/dataplane/configuration_test.go index b5144b03d4..170e18eb9f 100644 --- a/internal/mode/static/state/dataplane/configuration_test.go +++ b/internal/mode/static/state/dataplane/configuration_test.go @@ -2213,9 +2213,9 @@ func TestBuildConfiguration(t *testing.T) { HTTP2: true, IPFamily: Dual, RewriteClientIPSettings: RewriteClientIPSettings{ - IPRecursive: true, - TrustedCIDRs: []string{"0.0.0.0/0"}, - Mode: RewriteIPModeProxyProtocol, + IPRecursive: true, + TrustedAddresses: []string{"0.0.0.0/0"}, + Mode: RewriteIPModeProxyProtocol, }, } return conf @@ -3628,9 +3628,9 @@ func TestBuildRewriteIPSettings(t *testing.T) { }, }, expRewriteIPSettings: RewriteClientIPSettings{ - Mode: RewriteIPModeProxyProtocol, - TrustedCIDRs: []string{"0.0.0.0/0"}, - IPRecursive: true, + Mode: RewriteIPModeProxyProtocol, + TrustedAddresses: []string{"0.0.0.0/0"}, + IPRecursive: true, }, }, { @@ -3650,9 +3650,9 @@ func TestBuildRewriteIPSettings(t *testing.T) { }, }, expRewriteIPSettings: RewriteClientIPSettings{ - Mode: RewriteIPModeXForwardedFor, - TrustedCIDRs: []string{"0.0.0.0/0"}, - IPRecursive: true, + Mode: RewriteIPModeXForwardedFor, + TrustedAddresses: []string{"0.0.0.0/0"}, + IPRecursive: true, }, }, { @@ -3672,9 +3672,9 @@ func TestBuildRewriteIPSettings(t *testing.T) { }, }, 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, + Mode: RewriteIPModeXForwardedFor, + TrustedAddresses: []string{"0.0.0.0/0", "1.1.1.1/32", "2.2.2.2/32", "3.3.3.3/24"}, + IPRecursive: false, }, }, } diff --git a/internal/mode/static/state/dataplane/types.go b/internal/mode/static/state/dataplane/types.go index 0ae4b3b2a2..2b18fc004b 100644 --- a/internal/mode/static/state/dataplane/types.go +++ b/internal/mode/static/state/dataplane/types.go @@ -320,7 +320,7 @@ type RewriteClientIPSettings struct { // Mode specifies the mode for rewriting the client IP. Mode RewriteIPModeType // TrustedCIDRs specifies the CIDRs that are trusted to provide the client IP. - TrustedCIDRs []string + TrustedAddresses []string // IPRecursive specifies whether a recursive search is used when selecting the client IP. IPRecursive bool } diff --git a/internal/mode/static/state/graph/nginxproxy.go b/internal/mode/static/state/graph/nginxproxy.go index 67b36c87a6..42993c1347 100644 --- a/internal/mode/static/state/graph/nginxproxy.go +++ b/internal/mode/static/state/graph/nginxproxy.go @@ -172,8 +172,17 @@ func validateRewriteClientIP(npCfg *ngfAPI.NginxProxy) field.ErrorList { } for _, addr := range rewriteClientIP.TrustedAddresses { - if err := k8svalidation.IsValidCIDR(trustedAddressesPath, string(addr)); err != nil { - allErrs = append(allErrs, field.Invalid(trustedAddressesPath.Child(string(addr)), addr, err.ToAggregate().Error())) + cidrError := k8svalidation.IsValidCIDR(trustedAddressesPath, string(addr)) + ipError := k8svalidation.IsValidIP(trustedAddressesPath, string(addr)) + + if cidrError != nil && ipError != nil { + allErrs = append( + allErrs, + field.Invalid(trustedAddressesPath.Child(string(addr)), + addr, + "must be a valid IP address or CIDR range", + ), + ) } } } diff --git a/internal/mode/static/state/graph/nginxproxy_test.go b/internal/mode/static/state/graph/nginxproxy_test.go index 816e21086c..1ef38076ee 100644 --- a/internal/mode/static/state/graph/nginxproxy_test.go +++ b/internal/mode/static/state/graph/nginxproxy_test.go @@ -2,6 +2,7 @@ package graph import ( "errors" + "fmt" "testing" . "github.com/onsi/gomega" @@ -268,7 +269,7 @@ func TestValidateNginxProxy(t *testing.T) { IPFamily: helpers.GetPointer[ngfAPI.IPFamilyType](ngfAPI.Dual), RewriteClientIP: &ngfAPI.RewriteClientIP{ SetIPRecursively: helpers.GetPointer(true), - TrustedAddresses: []ngfAPI.TrustedAddress{"2001:db8:a0b:12f0::1/32", "0.0.0.0/0"}, + TrustedAddresses: []ngfAPI.TrustedAddress{"2001:db8:a0b:12f0::1/32", "1.1.1.1"}, Mode: helpers.GetPointer(ngfAPI.RewriteClientIPModeProxyProtocol), }, }, @@ -377,7 +378,7 @@ func TestValidateRewriteClientIP(t *testing.T) { Spec: ngfAPI.NginxProxySpec{ RewriteClientIP: &ngfAPI.RewriteClientIP{ SetIPRecursively: helpers.GetPointer(true), - TrustedAddresses: []ngfAPI.TrustedAddress{"2001:db8:a0b:12f0::1/32", "0.0.0.0/0"}, + TrustedAddresses: []ngfAPI.TrustedAddress{"2001:db8:a0b:12f0::1/32", "10.56.32.11/32"}, Mode: helpers.GetPointer(ngfAPI.RewriteClientIPModeProxyProtocol), }, }, @@ -391,15 +392,30 @@ func TestValidateRewriteClientIP(t *testing.T) { Spec: ngfAPI.NginxProxySpec{ RewriteClientIP: &ngfAPI.RewriteClientIP{ SetIPRecursively: helpers.GetPointer(true), - TrustedAddresses: []ngfAPI.TrustedAddress{"2001:db8:a0b:12f0::1"}, + TrustedAddresses: []ngfAPI.TrustedAddress{"2001:db8::/129", "10.0.0.1"}, Mode: helpers.GetPointer(ngfAPI.RewriteClientIPModeProxyProtocol), }, }, }, 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)", + errorString: "spec.rewriteClientIP.trustedAddresses.2001:db8::/129: " + + "Invalid value: \"2001:db8::/129\": must be a valid IP address or CIDR range", + }, + { + name: "invalid IP and CIDR in trustedAddresses", + validator: createInvalidValidator(), + np: &ngfAPI.NginxProxy{ + Spec: ngfAPI.NginxProxySpec{ + RewriteClientIP: &ngfAPI.RewriteClientIP{ + SetIPRecursively: helpers.GetPointer(true), + TrustedAddresses: []ngfAPI.TrustedAddress{"2001:db8::1/48", "256.100.50.25"}, + Mode: helpers.GetPointer(ngfAPI.RewriteClientIPModeProxyProtocol), + }, + }, + }, + expectErrCount: 1, + errorString: "spec.rewriteClientIP.trustedAddresses.256.100.50.25: " + + "Invalid value: \"256.100.50.25\": must be a valid IP address or CIDR range", }, { name: "invalid when mode is set and trustedAddresses is empty", @@ -442,7 +458,7 @@ func TestValidateRewriteClientIP(t *testing.T) { Spec: ngfAPI.NginxProxySpec{ RewriteClientIP: &ngfAPI.RewriteClientIP{ Mode: helpers.GetPointer(ngfAPI.RewriteClientIPModeType("invalid")), - TrustedAddresses: []ngfAPI.TrustedAddress{"2001:db8:a0b:12f0::1/32", "0.0.0.0/0"}, + TrustedAddresses: []ngfAPI.TrustedAddress{"2001:db8:a0b:12f0::1/32", "10.0.0.1/32"}, }, }, }, @@ -474,6 +490,7 @@ func TestValidateRewriteClientIP(t *testing.T) { allErrs := validateRewriteClientIP(test.np) g.Expect(allErrs).To(HaveLen(test.expectErrCount)) if len(allErrs) > 0 { + fmt.Println(allErrs.ToAggregate().Error()) 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 54da2f2128..1783832432 100644 --- a/site/content/how-to/monitoring/troubleshooting.md +++ b/site/content/how-to/monitoring/troubleshooting.md @@ -83,56 +83,56 @@ You can see logs for a crashed or killed container by adding the `-p` flag to th 1. Container Logs - To see logs for the control plane container: + To see logs for the control plane container: - ```shell - kubectl -n nginx-gateway logs -c nginx-gateway - ``` + ```shell + kubectl -n nginx-gateway logs -c nginx-gateway + ``` - To see logs for the data plane container: + To see logs for the data plane container: - ```shell - kubectl -n nginx-gateway logs -c nginx - ``` + ```shell + kubectl -n nginx-gateway logs -c nginx + ``` 1. Error Logs - For the _nginx-gateway_ container, you can `grep` the logs for the word `error`: + For the _nginx-gateway_ container, you can `grep` the logs for the word `error`: - ```shell - kubectl -n nginx-gateway logs -c nginx-gateway | grep error - ``` + ```shell + kubectl -n nginx-gateway logs -c nginx-gateway | grep error + ``` - For example, an error message when telemetry is not enabled for NGINX Plus installations: + For example, an error message when telemetry is not enabled for NGINX Plus installations: - ```text - kubectl logs -n nginx-gateway nginx-gateway-nginx-gateway-fabric-77f8746996-j6z6v | grep error - Defaulted container "nginx-gateway" out of: nginx-gateway, nginx - {"level":"error","ts":"2024-06-13T18:22:16Z","logger":"usageReporter","msg":"Usage reporting must be enabled when using NGINX Plus; redeploy with usage reporting enabled","error":"usage reporting not enabled","stacktrace":"github.com/nginxinc/nginx-gateway-fabric/internal/mode/static.createUsageWarningJob.func1\n\tgithub.com/nginxinc/nginx-gateway-fabric/internal/mode/static/manager.go:616\nk8s.io/apimachinery/pkg/util/wait.JitterUntilWithContext.func1\n\tk8s.io/apimachinery@v0.30.1/pkg/util/wait/backoff.go:259\nk8s.io/apimachinery/pkg/util/wait.BackoffUntil.func1\n\tk8s.io/apimachinery@v0.30.1/pkg/util/wait/backoff.go:226\nk8s.io/apimachinery/pkg/util/wait.BackoffUntil\n\tk8s.io/apimachinery@v0.30.1/pkg/util/wait/backoff.go:227\nk8s.io/apimachinery/pkg/util/wait.JitterUntil\n\tk8s.io/apimachinery@v0.30.1/pkg/util/wait/backoff.go:204\nk8s.io/apimachinery/pkg/util/wait.JitterUntilWithContext\n\tk8s.io/apimachinery@v0.30.1/pkg/util/wait/backoff.go:259\ngithub.com/nginxinc/nginx-gateway-fabric/internal/framework/runnables.(*CronJob).Start\n\tgithub.com/nginxinc/nginx-gateway-fabric/internal/framework/runnables/cronjob.go:53\nsigs.k8s.io/controller-runtime/pkg/manager.(*runnableGroup).reconcile.func1\n\tsigs.k8s.io/controller-runtime@v0.18.4/pkg/manager/runnable_group.go:226"} - ``` + ```text + kubectl logs -n nginx-gateway nginx-gateway-nginx-gateway-fabric-77f8746996-j6z6v | grep error + Defaulted container "nginx-gateway" out of: nginx-gateway, nginx + {"level":"error","ts":"2024-06-13T18:22:16Z","logger":"usageReporter","msg":"Usage reporting must be enabled when using NGINX Plus; redeploy with usage reporting enabled","error":"usage reporting not enabled","stacktrace":"github.com/nginxinc/nginx-gateway-fabric/internal/mode/static.createUsageWarningJob.func1\n\tgithub.com/nginxinc/nginx-gateway-fabric/internal/mode/static/manager.go:616\nk8s.io/apimachinery/pkg/util/wait.JitterUntilWithContext.func1\n\tk8s.io/apimachinery@v0.30.1/pkg/util/wait/backoff.go:259\nk8s.io/apimachinery/pkg/util/wait.BackoffUntil.func1\n\tk8s.io/apimachinery@v0.30.1/pkg/util/wait/backoff.go:226\nk8s.io/apimachinery/pkg/util/wait.BackoffUntil\n\tk8s.io/apimachinery@v0.30.1/pkg/util/wait/backoff.go:227\nk8s.io/apimachinery/pkg/util/wait.JitterUntil\n\tk8s.io/apimachinery@v0.30.1/pkg/util/wait/backoff.go:204\nk8s.io/apimachinery/pkg/util/wait.JitterUntilWithContext\n\tk8s.io/apimachinery@v0.30.1/pkg/util/wait/backoff.go:259\ngithub.com/nginxinc/nginx-gateway-fabric/internal/framework/runnables.(*CronJob).Start\n\tgithub.com/nginxinc/nginx-gateway-fabric/internal/framework/runnables/cronjob.go:53\nsigs.k8s.io/controller-runtime/pkg/manager.(*runnableGroup).reconcile.func1\n\tsigs.k8s.io/controller-runtime@v0.18.4/pkg/manager/runnable_group.go:226"} + ``` - For the _nginx_ container you can `grep` for various [error](https://nginx.org/en/docs/ngx_core_module.html#error_log) logs. For example, to search for all logs logged at the `emerg` level: + For the _nginx_ container you can `grep` for various [error](https://nginx.org/en/docs/ngx_core_module.html#error_log) logs. For example, to search for all logs logged at the `emerg` level: - ```shell - kubectl -n nginx-gateway logs -c nginx | grep emerg - ``` + ```shell + kubectl -n nginx-gateway logs -c nginx | grep emerg + ``` - For example, if a variable is too long, NGINX may display such an error message: + For example, if a variable is too long, NGINX may display such an error message: - ```text - kubectl logs -n nginx-gateway ngf-nginx-gateway-fabric-bb8598998-jwk2m -c nginx | grep emerg - 2024/06/13 20:04:17 [emerg] 27#27: too long parameter, probably missing terminating """ character in /etc/nginx/conf.d/http.conf:78 - ``` + ```text + kubectl logs -n nginx-gateway ngf-nginx-gateway-fabric-bb8598998-jwk2m -c nginx | grep emerg + 2024/06/13 20:04:17 [emerg] 27#27: too long parameter, probably missing terminating """ character in /etc/nginx/conf.d/http.conf:78 + ``` 1. Access Logs - NGINX access logs record all requests processed by the NGINX server. These logs provide detailed information about each request, which can be useful for troubleshooting and analyzing web traffic. - Access logs can be viewed with the above method of using `kubectl logs`, or by viewing the access log file directly. To do that, get shell access to your NGINX container using these [steps](#get-shell-access-to-nginx-container). The access logs are located in the file `/var/log/nginx/access.log` in the NGINX container. + NGINX access logs record all requests processed by the NGINX server. These logs provide detailed information about each request, which can be useful for troubleshooting and analyzing web traffic. + Access logs can be viewed with the above method of using `kubectl logs`, or by viewing the access log file directly. To do that, get shell access to your NGINX container using these [steps](#get-shell-access-to-nginx-container). The access logs are located in the file `/var/log/nginx/access.log` in the NGINX container. 1. Modify Log Levels - To modify log levels for the control plane in NGINX Gateway Fabric, edit the `NginxGateway` configuration. This can be done either before or after deploying NGINX Gateway Fabric. Refer to this [guide](https://docs.nginx.com/nginx-gateway-fabric/how-to/configuration/control-plane-configuration) to do so. - To check error logs, modify the log level to `error` to view error logs. Similarly, change the log level to `debug` and `grep` for the word `debug` to view debug logs. + To modify log levels for the control plane in NGINX Gateway Fabric, edit the `NginxGateway` configuration. This can be done either before or after deploying NGINX Gateway Fabric. Refer to this [guide](https://docs.nginx.com/nginx-gateway-fabric/how-to/configuration/control-plane-configuration) to do so. + To check error logs, modify the log level to `error` to view error logs. Similarly, change the log level to `debug` and `grep` for the word `debug` to view debug logs. #### Understanding the generated NGINX configuration @@ -167,18 +167,18 @@ metadata: name: coffee spec: parentRefs: - - name: gateway - sectionName: http + - name: gateway + sectionName: http hostnames: - - "cafe.example.com" + - "cafe.example.com" rules: - - matches: - - path: - type: PathPrefix - value: /coffee - backendRefs: - - name: coffee - port: 80 + - matches: + - path: + type: PathPrefix + value: /coffee + backendRefs: + - name: coffee + port: 80 ``` The modified `nginx.conf`: @@ -231,7 +231,7 @@ upstream default_coffee_80 { Key information to note is: 1. A new `server` block is created with the hostname of the HTTPRoute. When a request is sent to this hostname, it will be handled by this `server` block. -2. Within the `server` block, three new `location` blocks are added for *coffee*, each with distinct prefix and exact paths. Requests directed to the *coffee* application with a path prefix `/coffee/hello` will be managed by the first location block, while those with an exact path `/coffee` will be handled by the second location block. Any other requests not recognized by the server block for this hostname will default to the third location block, returning a 404 Not Found status. +2. Within the `server` block, three new `location` blocks are added for _coffee_, each with distinct prefix and exact paths. Requests directed to the _coffee_ application with a path prefix `/coffee/hello` will be managed by the first location block, while those with an exact path `/coffee` will be handled by the second location block. Any other requests not recognized by the server block for this hostname will default to the third location block, returning a 404 Not Found status. 3. Each `location` block has headers and directives that configure the NGINX proxy to forward requests to the `/coffee` path correctly, preserving important client information and ensuring compatibility with the upstream server. 4. The `upstream` block in the given NGINX configuration defines a group of backend servers and configures how NGINX should load balance requests among them. @@ -294,19 +294,19 @@ Verify that the port number (for example, `8080`) matches the port number you ha ### Common errors {{< bootstrap-table "table table-striped table-bordered" >}} -| Problem Area | Symptom | Troubleshooting Method | Common Cause | +| Problem Area | Symptom | Troubleshooting Method | Common Cause | |------------------------------|----------------------------------------|---------------------------------------------------------------------------------------------------------------------|-----------------------------------------------------------------------------------------------------------------------------------------| -| Startup | NGINX Gateway Fabric fails to start. | Check logs for _nginx_ and _nginx-gateway_ containers. | Readiness probe failed. | -| Resources not configured | Status missing on resources. | Check referenced resources. | Referenced resources do not belong to NGINX Gateway Fabric. | -| NGINX errors | Reload failures on NGINX | Fix permissions for control plane. | Security context not configured. | -| Usage reporting | Errors logs related to usage reporting | Enable usage reporting. Refer to [Usage Reporting]({{< relref "installation/usage-reporting.md" >}}) | Usage reporting disabled. | -| Client Settings | Request entity too large error | Adjust client settings. Refer to [Client Settings Policy]({{< relref "../traffic-management/client-settings.md" >}}) | Payload is greater than the [`client_max_body_size`](https://nginx.org/en/docs/http/ngx_http_core_module.html#client_max_body_size) value.| +| Startup | NGINX Gateway Fabric fails to start. | Check logs for _nginx_ and _nginx-gateway_ containers. | Readiness probe failed. | +| Resources not configured | Status missing on resources. | Check referenced resources. | Referenced resources do not belong to NGINX Gateway Fabric. | +| NGINX errors | Reload failures on NGINX | Fix permissions for control plane. | Security context not configured. | +| Usage reporting | Errors logs related to usage reporting | Enable usage reporting. Refer to [Usage Reporting]({{< relref "installation/usage-reporting.md" >}}) | Usage reporting disabled. | +| Client Settings | Request entity too large error | Adjust client settings. Refer to [Client Settings Policy]({{< relref "../traffic-management/client-settings.md" >}}) | Payload is greater than the [`client_max_body_size`](https://nginx.org/en/docs/http/ngx_http_core_module.html#client_max_body_size) value.| {{< /bootstrap-table >}} ##### NGINX fails to reload NGINX reload errors can occur for various reasons, including syntax errors in configuration files, permission issues, and more. To determine if NGINX has failed to reload, check logs for your _nginx-gateway_ and _nginx_ containers. -You will see the following error in the _nginx-gateway_ logs: `failed to reload NGINX:`, followed by the reason for the failure. Similarly, error logs in _nginx_ container start with `emerg`. For example, `2024/06/12 14:25:11 [emerg] 12345#0: open() "/var/run/nginx.pid" failed (13: Permission denied)` shows a critical error, such as a permission problem preventing NGINX from accessing necessary files. +You will see the following error in the _nginx-gateway_ logs: `failed to reload NGINX:`, followed by the reason for the failure. Similarly, error logs in _nginx_ container start with `emerg`. For example, `2024/06/12 14:25:11 [emerg] 12345#0: open() "/var/run/nginx.pid" failed (13: Permission denied)` shows a critical error, such as a permission problem preventing NGINX from accessing necessary files. To debug why your reload has failed, start with verifying the syntax of your configuration files by opening a shell in the NGINX container following these [steps](#get-shell-access-to-nginx-container) and running `nginx -T`. If there are errors in your configuration file, the reload will fail and specify the reason for it. @@ -459,13 +459,13 @@ This means you are attempting to attach a Policy to a Route that has an overlapp If you check your _nginx_ container logs and see the following error: - ```text - 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 - ``` +```text + 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: -- Update field [`rewriteClientIP.mode`](({{< relref "reference/api.md" >}})) to `ProxyProtocol` in the NginxProxy configuration. +- Disable field [`rewriteClientIP.mode`](({{< relref "reference/api.md" >}})) 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 e8e546ef5f..53a27d1926 100644 --- a/site/content/reference/api.md +++ b/site/content/reference/api.md @@ -3,11 +3,8 @@ title: "API reference" weight: 100 toc: false --- - ## Overview - NGINX Gateway API Reference -

    Packages:

    • @@ -1089,12 +1086,12 @@ bool (Optional) -

      SetIPRecursively configures whether recursive search is used when selecting the client’s address from. +

      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. +and TrustedAddresses is set to 55.55.55.1, 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

      @@ -1115,7 +1112,7 @@ Sets NGINX directive real_ip_recursive: https://nginx.org/en/docs/http/ngx_http_realip_module.html#set_real_ip_from @@ -1502,8 +1499,8 @@ Examples of invalid names: some-$value, quoted-“value”-name, unescap RewriteClientIP)

      -

      TrustedAddress is a string value representing a CIDR block. -Examples: 0.0.0.0/0

      +

      TrustedAddress is a string value representing a CIDR block or an IP address. +Examples: 10.0.0.2⁄32, 10.0.0.1, fe80::1⁄128, ::1⁄24.

      From 80622e307afff775c9f4f882119c05c8a47cc514 Mon Sep 17 00:00:00 2001 From: salonichf5 <146118978+salonichf5@users.noreply.github.com> Date: Tue, 3 Sep 2024 08:34:21 -0600 Subject: [PATCH 07/19] Update apis/v1alpha1/nginxproxy_types.go Co-authored-by: Kate Osborn <50597707+kate-osborn@users.noreply.github.com> --- apis/v1alpha1/nginxproxy_types.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/apis/v1alpha1/nginxproxy_types.go b/apis/v1alpha1/nginxproxy_types.go index f1d03b9e54..901abc10e6 100644 --- a/apis/v1alpha1/nginxproxy_types.go +++ b/apis/v1alpha1/nginxproxy_types.go @@ -149,7 +149,7 @@ type RewriteClientIP struct { // 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 or IP address: 10.0.0.0, 192.33.21/24, fe80::1/128. + // Addresses must be provided as CIDR blocks or IP addresses: 10.0.0.0, 192.33.21/24, fe80::1/128. // 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 From aa6c70262e5294f043993ba907afbb13bb3c1f42 Mon Sep 17 00:00:00 2001 From: salonichf5 <146118978+salonichf5@users.noreply.github.com> Date: Tue, 3 Sep 2024 08:34:29 -0600 Subject: [PATCH 08/19] Update apis/v1alpha1/nginxproxy_types.go Co-authored-by: Kate Osborn <50597707+kate-osborn@users.noreply.github.com> --- apis/v1alpha1/nginxproxy_types.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/apis/v1alpha1/nginxproxy_types.go b/apis/v1alpha1/nginxproxy_types.go index 901abc10e6..71dec0e910 100644 --- a/apis/v1alpha1/nginxproxy_types.go +++ b/apis/v1alpha1/nginxproxy_types.go @@ -150,7 +150,7 @@ type RewriteClientIP struct { // 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 or IP addresses: 10.0.0.0, 192.33.21/24, fe80::1/128. - // To trust all addresses (not recommended), set to 0.0.0.0/0. + // To trust all addresses (not recommended for production), 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. From cde1371518ff6f1c0e21472bd130ae3cce7ab625 Mon Sep 17 00:00:00 2001 From: salonichf5 <146118978+salonichf5@users.noreply.github.com> Date: Tue, 3 Sep 2024 08:34:37 -0600 Subject: [PATCH 09/19] Update apis/v1alpha1/nginxproxy_types.go Co-authored-by: Kate Osborn <50597707+kate-osborn@users.noreply.github.com> --- apis/v1alpha1/nginxproxy_types.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/apis/v1alpha1/nginxproxy_types.go b/apis/v1alpha1/nginxproxy_types.go index 71dec0e910..79f274dcd6 100644 --- a/apis/v1alpha1/nginxproxy_types.go +++ b/apis/v1alpha1/nginxproxy_types.go @@ -169,7 +169,7 @@ type RewriteClientIPModeType string const ( // RewriteClientIPModeProxyProtocol configures NGINX to accept PROXY protocol and // set the client's IP address to the IP address in the PROXY protocol header. - // Sets the proxy_protocol parameter to the listen directive on all servers, and sets real_ip_header + // Sets the proxy_protocol parameter on the listen directive of all servers and sets real_ip_header // to proxy_protocol: https://nginx.org/en/docs/http/ngx_http_realip_module.html#real_ip_header. RewriteClientIPModeProxyProtocol RewriteClientIPModeType = "ProxyProtocol" From d04c794f443109d83dcb498dec4c2fbcb2288a60 Mon Sep 17 00:00:00 2001 From: salonichf5 <146118978+salonichf5@users.noreply.github.com> Date: Tue, 3 Sep 2024 08:35:49 -0600 Subject: [PATCH 10/19] Update internal/mode/static/state/dataplane/configuration.go Co-authored-by: Kate Osborn <50597707+kate-osborn@users.noreply.github.com> --- internal/mode/static/state/dataplane/configuration.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/internal/mode/static/state/dataplane/configuration.go b/internal/mode/static/state/dataplane/configuration.go index 44479f87dd..cace312981 100644 --- a/internal/mode/static/state/dataplane/configuration.go +++ b/internal/mode/static/state/dataplane/configuration.go @@ -864,7 +864,7 @@ func buildBaseHTTPConfig(g *graph.Graph) BaseHTTPConfig { if len(g.NginxProxy.Source.Spec.RewriteClientIP.TrustedAddresses) > 0 { trustedAddresses := convertTrustedAddresses(g) - baseConfig.RewriteClientIPSettings.TrustedAddresses = trustedAddresses + baseConfig.RewriteClientIPSettings.TrustedAddresses = convertTrustedAddresses(g) } if g.NginxProxy.Source.Spec.RewriteClientIP.SetIPRecursively != nil { From 3190e74d2a60107bca2238257ab32292ccb052e2 Mon Sep 17 00:00:00 2001 From: salonichf5 <146118978+salonichf5@users.noreply.github.com> Date: Tue, 3 Sep 2024 08:35:57 -0600 Subject: [PATCH 11/19] Update internal/mode/static/state/dataplane/types.go Co-authored-by: Kate Osborn <50597707+kate-osborn@users.noreply.github.com> --- internal/mode/static/state/dataplane/types.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/internal/mode/static/state/dataplane/types.go b/internal/mode/static/state/dataplane/types.go index 2b18fc004b..59110f8cbb 100644 --- a/internal/mode/static/state/dataplane/types.go +++ b/internal/mode/static/state/dataplane/types.go @@ -319,7 +319,7 @@ type BaseHTTPConfig struct { type RewriteClientIPSettings struct { // Mode specifies the mode for rewriting the client IP. Mode RewriteIPModeType - // TrustedCIDRs specifies the CIDRs that are trusted to provide the client IP. + // TrustedAddresses specifies the addresses that are trusted to provide the client IP. TrustedAddresses []string // IPRecursive specifies whether a recursive search is used when selecting the client IP. IPRecursive bool From abb87600ccbff1ee521d87b783796a56ea9423a4 Mon Sep 17 00:00:00 2001 From: Saloni Date: Tue, 3 Sep 2024 09:45:53 -0600 Subject: [PATCH 12/19] updates based on reviews --- .../bases/gateway.nginx.org_nginxproxies.yaml | 4 +-- deploy/crds.yaml | 4 +-- internal/mode/static/nginx/config/servers.go | 32 ++++++++++--------- .../mode/static/nginx/config/servers_test.go | 1 + .../static/nginx/config/stream_servers.go | 1 + .../static/state/dataplane/configuration.go | 10 +++--- .../how-to/monitoring/troubleshooting.md | 2 +- site/content/reference/api.md | 6 ++-- 8 files changed, 32 insertions(+), 28 deletions(-) diff --git a/config/crd/bases/gateway.nginx.org_nginxproxies.yaml b/config/crd/bases/gateway.nginx.org_nginxproxies.yaml index 3f9fd62e9f..904d7bb421 100644 --- a/config/crd/bases/gateway.nginx.org_nginxproxies.yaml +++ b/config/crd/bases/gateway.nginx.org_nginxproxies.yaml @@ -95,8 +95,8 @@ spec: 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 or IP address: 10.0.0.0, 192.33.21/24, fe80::1/128. - To trust all addresses (not recommended), set to 0.0.0.0/0. + Addresses must be provided as CIDR blocks or IP addresses: 10.0.0.0, 192.33.21/24, fe80::1/128. + To trust all addresses (not recommended for production), 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. diff --git a/deploy/crds.yaml b/deploy/crds.yaml index 1c6163fb1c..c3631cd7c0 100644 --- a/deploy/crds.yaml +++ b/deploy/crds.yaml @@ -680,8 +680,8 @@ spec: 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 or IP address: 10.0.0.0, 192.33.21/24, fe80::1/128. - To trust all addresses (not recommended), set to 0.0.0.0/0. + Addresses must be provided as CIDR blocks or IP addresses: 10.0.0.0, 192.33.21/24, fe80::1/128. + To trust all addresses (not recommended for production), 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. diff --git a/internal/mode/static/nginx/config/servers.go b/internal/mode/static/nginx/config/servers.go index eeac39944f..8ae4218db0 100644 --- a/internal/mode/static/nginx/config/servers.go +++ b/internal/mode/static/nginx/config/servers.go @@ -830,23 +830,25 @@ func generateProxySetHeaders(filters *dataplane.HTTPFilters, grpc bool) []http.H } func setHeaderForHTTPSRedirect(filters *dataplane.HTTPFilters, headers []http.Header) { - if filters != nil { - if filters.RequestURLRewrite != nil && filters.RequestURLRewrite.Hostname != nil { - for i, header := range headers { - if header.Name == "Host" { - headers[i].Value = *filters.RequestURLRewrite.Hostname - break - } + if filters == nil { + return + } + + if filters.RequestURLRewrite != nil && filters.RequestURLRewrite.Hostname != nil { + for i, header := range headers { + if header.Name == "Host" { + headers[i].Value = *filters.RequestURLRewrite.Hostname + break } } - if filters.RequestRedirect != nil && - filters.RequestRedirect.Scheme != nil && - *filters.RequestRedirect.Scheme == http.HTTPSScheme { - for i, header := range headers { - if header.Name == "X-Forwarded-Proto" { - headers[i].Value = http.HTTPSScheme - return - } + } + if filters.RequestRedirect != nil && + filters.RequestRedirect.Scheme != nil && + *filters.RequestRedirect.Scheme == http.HTTPSScheme { + for i, header := range headers { + if header.Name == "X-Forwarded-Proto" { + headers[i].Value = http.HTTPSScheme + return } } } diff --git a/internal/mode/static/nginx/config/servers_test.go b/internal/mode/static/nginx/config/servers_test.go index 1c21ec05d0..3475c959a7 100644 --- a/internal/mode/static/nginx/config/servers_test.go +++ b/internal/mode/static/nginx/config/servers_test.go @@ -342,6 +342,7 @@ func TestExecuteServers_RewriteClientIP(t *testing.T) { "listen [::]:8080 proxy_protocol;": 1, "listen [::]:8443 ssl default_server proxy_protocol;": 1, "listen [::]:8443 ssl proxy_protocol;": 1, + "real_ip_recursive on;": 0, }, }, { diff --git a/internal/mode/static/nginx/config/stream_servers.go b/internal/mode/static/nginx/config/stream_servers.go index 4cef5de2f1..b6cb763cb5 100644 --- a/internal/mode/static/nginx/config/stream_servers.go +++ b/internal/mode/static/nginx/config/stream_servers.go @@ -67,6 +67,7 @@ func createStreamServers(conf dataplane.Configuration) []stream.Server { portSet[server.Port] = struct{}{} + // we do not evaluate rewriteClientIP settings for non-socket stream servers streamServer := stream.Server{ Listen: fmt.Sprint(server.Port), StatusZone: server.Hostname, diff --git a/internal/mode/static/state/dataplane/configuration.go b/internal/mode/static/state/dataplane/configuration.go index cace312981..197cc73202 100644 --- a/internal/mode/static/state/dataplane/configuration.go +++ b/internal/mode/static/state/dataplane/configuration.go @@ -863,8 +863,8 @@ func buildBaseHTTPConfig(g *graph.Graph) BaseHTTPConfig { } if len(g.NginxProxy.Source.Spec.RewriteClientIP.TrustedAddresses) > 0 { - trustedAddresses := convertTrustedAddresses(g) - baseConfig.RewriteClientIPSettings.TrustedAddresses = convertTrustedAddresses(g) + trustedAddresses := convertTrustedAddresses(g.NginxProxy.Source.Spec.RewriteClientIP.TrustedAddresses) + baseConfig.RewriteClientIPSettings.TrustedAddresses = trustedAddresses } if g.NginxProxy.Source.Spec.RewriteClientIP.SetIPRecursively != nil { @@ -893,9 +893,9 @@ func buildPolicies(graphPolicies []*graph.Policy) []policies.Policy { return finalPolicies } -func convertTrustedAddresses(g *graph.Graph) []string { - trustedAddresses := make([]string, len(g.NginxProxy.Source.Spec.RewriteClientIP.TrustedAddresses)) - for i, addr := range g.NginxProxy.Source.Spec.RewriteClientIP.TrustedAddresses { +func convertTrustedAddresses(addresses []ngfAPI.TrustedAddress) []string { + trustedAddresses := make([]string, len(addresses)) + for i, addr := range addresses { trustedAddresses[i] = string(addr) } return trustedAddresses diff --git a/site/content/how-to/monitoring/troubleshooting.md b/site/content/how-to/monitoring/troubleshooting.md index 1783832432..b1fc476890 100644 --- a/site/content/how-to/monitoring/troubleshooting.md +++ b/site/content/how-to/monitoring/troubleshooting.md @@ -465,7 +465,7 @@ If you check your _nginx_ container logs and see the following error: 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: -- Disable field [`rewriteClientIP.mode`](({{< relref "reference/api.md" >}})) in the NginxProxy configuration. +- Unassign the field [`rewriteClientIP.mode`](({{< relref "reference/api.md" >}})) 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 53a27d1926..deb2eee011 100644 --- a/site/content/reference/api.md +++ b/site/content/reference/api.md @@ -1112,8 +1112,8 @@ Sets NGINX directive real_ip_recursive: https://nginx.org/en/docs/http/ngx_http_realip_module.html#set_real_ip_from This field is required if mode is set.

      @@ -1141,7 +1141,7 @@ This field is required if mode is set.

      "ProxyProtocol"

      RewriteClientIPModeProxyProtocol configures NGINX to accept PROXY protocol and set the client’s IP address to the IP address in the PROXY protocol header. -Sets the proxy_protocol parameter to the listen directive on all servers, and sets real_ip_header +Sets the proxy_protocol parameter on the listen directive of all servers and sets real_ip_header to proxy_protocol: https://nginx.org/en/docs/http/ngx_http_realip_module.html#real_ip_header.

      "XForwardedFor"

      From 86fc671a80bceed9f4ce84de20437cd2a2aaae53 Mon Sep 17 00:00:00 2001 From: Saloni Date: Tue, 3 Sep 2024 11:20:47 -0600 Subject: [PATCH 13/19] address nits --- .../static/state/dataplane/configuration_test.go | 16 ++++++++-------- .../mode/static/state/graph/nginxproxy_test.go | 2 +- 2 files changed, 9 insertions(+), 9 deletions(-) diff --git a/internal/mode/static/state/dataplane/configuration_test.go b/internal/mode/static/state/dataplane/configuration_test.go index 170e18eb9f..b1d6d98c54 100644 --- a/internal/mode/static/state/dataplane/configuration_test.go +++ b/internal/mode/static/state/dataplane/configuration_test.go @@ -2198,7 +2198,7 @@ func TestBuildConfiguration(t *testing.T) { Spec: ngfAPI.NginxProxySpec{ RewriteClientIP: &ngfAPI.RewriteClientIP{ SetIPRecursively: helpers.GetPointer(true), - TrustedAddresses: []ngfAPI.TrustedAddress{"0.0.0.0/0"}, + TrustedAddresses: []ngfAPI.TrustedAddress{"1.1.1.1/32"}, Mode: helpers.GetPointer(ngfAPI.RewriteClientIPModeProxyProtocol), }, }, @@ -2214,7 +2214,7 @@ func TestBuildConfiguration(t *testing.T) { IPFamily: Dual, RewriteClientIPSettings: RewriteClientIPSettings{ IPRecursive: true, - TrustedAddresses: []string{"0.0.0.0/0"}, + TrustedAddresses: []string{"1.1.1.1/32"}, Mode: RewriteIPModeProxyProtocol, }, } @@ -3620,7 +3620,7 @@ func TestBuildRewriteIPSettings(t *testing.T) { Spec: ngfAPI.NginxProxySpec{ RewriteClientIP: &ngfAPI.RewriteClientIP{ Mode: helpers.GetPointer(ngfAPI.RewriteClientIPModeProxyProtocol), - TrustedAddresses: []ngfAPI.TrustedAddress{"0.0.0.0/0"}, + TrustedAddresses: []ngfAPI.TrustedAddress{"10.9.9.4"}, SetIPRecursively: helpers.GetPointer(true), }, }, @@ -3629,7 +3629,7 @@ func TestBuildRewriteIPSettings(t *testing.T) { }, expRewriteIPSettings: RewriteClientIPSettings{ Mode: RewriteIPModeProxyProtocol, - TrustedAddresses: []string{"0.0.0.0/0"}, + TrustedAddresses: []string{"10.9.9.4"}, IPRecursive: true, }, }, @@ -3642,7 +3642,7 @@ func TestBuildRewriteIPSettings(t *testing.T) { Spec: ngfAPI.NginxProxySpec{ RewriteClientIP: &ngfAPI.RewriteClientIP{ Mode: helpers.GetPointer(ngfAPI.RewriteClientIPModeXForwardedFor), - TrustedAddresses: []ngfAPI.TrustedAddress{"0.0.0.0/0"}, + TrustedAddresses: []ngfAPI.TrustedAddress{"76.89.90.11"}, SetIPRecursively: helpers.GetPointer(true), }, }, @@ -3651,7 +3651,7 @@ func TestBuildRewriteIPSettings(t *testing.T) { }, expRewriteIPSettings: RewriteClientIPSettings{ Mode: RewriteIPModeXForwardedFor, - TrustedAddresses: []string{"0.0.0.0/0"}, + TrustedAddresses: []string{"76.89.90.11"}, IPRecursive: true, }, }, @@ -3664,7 +3664,7 @@ func TestBuildRewriteIPSettings(t *testing.T) { 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"}, + TrustedAddresses: []ngfAPI.TrustedAddress{"5.5.5.5", "1.1.1.1/32", "2.2.2.2/32", "3.3.3.3/24"}, SetIPRecursively: helpers.GetPointer(false), }, }, @@ -3673,7 +3673,7 @@ func TestBuildRewriteIPSettings(t *testing.T) { }, expRewriteIPSettings: RewriteClientIPSettings{ Mode: RewriteIPModeXForwardedFor, - TrustedAddresses: []string{"0.0.0.0/0", "1.1.1.1/32", "2.2.2.2/32", "3.3.3.3/24"}, + TrustedAddresses: []string{"5.5.5.5", "1.1.1.1/32", "2.2.2.2/32", "3.3.3.3/24"}, IPRecursive: false, }, }, diff --git a/internal/mode/static/state/graph/nginxproxy_test.go b/internal/mode/static/state/graph/nginxproxy_test.go index 1ef38076ee..3df1347981 100644 --- a/internal/mode/static/state/graph/nginxproxy_test.go +++ b/internal/mode/static/state/graph/nginxproxy_test.go @@ -402,7 +402,7 @@ func TestValidateRewriteClientIP(t *testing.T) { "Invalid value: \"2001:db8::/129\": must be a valid IP address or CIDR range", }, { - name: "invalid IP and CIDR in trustedAddresses", + name: "invalid IP and valid CIDR in trustedAddresses", validator: createInvalidValidator(), np: &ngfAPI.NginxProxy{ Spec: ngfAPI.NginxProxySpec{ From e31fa4da671cfbe680d98305e184a772461640bd Mon Sep 17 00:00:00 2001 From: Saloni Date: Tue, 3 Sep 2024 13:50:34 -0600 Subject: [PATCH 14/19] fix test --- internal/mode/static/nginx/config/servers_test.go | 5 +++++ internal/mode/static/state/dataplane/configuration.go | 5 +++-- 2 files changed, 8 insertions(+), 2 deletions(-) diff --git a/internal/mode/static/nginx/config/servers_test.go b/internal/mode/static/nginx/config/servers_test.go index 3475c959a7..0a05149182 100644 --- a/internal/mode/static/nginx/config/servers_test.go +++ b/internal/mode/static/nginx/config/servers_test.go @@ -2723,6 +2723,11 @@ func TestGenerateProxySetHeaders(t *testing.T) { }, }, }, + { + msg: "no filter set", + expectedHeaders: httpBaseHeaders, + GRPC: false, + }, } for _, tc := range tests { diff --git a/internal/mode/static/state/dataplane/configuration.go b/internal/mode/static/state/dataplane/configuration.go index 197cc73202..ce06c94b1e 100644 --- a/internal/mode/static/state/dataplane/configuration.go +++ b/internal/mode/static/state/dataplane/configuration.go @@ -863,8 +863,9 @@ func buildBaseHTTPConfig(g *graph.Graph) BaseHTTPConfig { } if len(g.NginxProxy.Source.Spec.RewriteClientIP.TrustedAddresses) > 0 { - trustedAddresses := convertTrustedAddresses(g.NginxProxy.Source.Spec.RewriteClientIP.TrustedAddresses) - baseConfig.RewriteClientIPSettings.TrustedAddresses = trustedAddresses + baseConfig.RewriteClientIPSettings.TrustedAddresses = convertTrustedAddresses( + g.NginxProxy.Source.Spec.RewriteClientIP.TrustedAddresses, + ) } if g.NginxProxy.Source.Spec.RewriteClientIP.SetIPRecursively != nil { From 71f3a1b764143bd8d332497aa25baadb09a73759 Mon Sep 17 00:00:00 2001 From: Saloni Date: Wed, 4 Sep 2024 15:54:40 -0600 Subject: [PATCH 15/19] change trustedAddress type to struct --- apis/v1alpha1/nginxproxy_types.go | 39 +++++-- apis/v1alpha1/zz_generated.deepcopy.go | 17 ++- charts/nginx-gateway-fabric/values.yaml | 13 ++- .../bases/gateway.nginx.org_nginxproxies.yaml | 26 +++-- deploy/crds.yaml | 26 +++-- .../static/nginx/config/servers_template.go | 12 +- .../nginx/config/stream_servers_template.go | 4 +- .../static/state/dataplane/configuration.go | 6 +- .../state/dataplane/configuration_test.go | 54 +++++++-- .../mode/static/state/graph/nginxproxy.go | 9 +- .../static/state/graph/nginxproxy_test.go | 103 ++++++++++++------ site/content/reference/api.md | 89 ++++++++++++--- 12 files changed, 289 insertions(+), 109 deletions(-) diff --git a/apis/v1alpha1/nginxproxy_types.go b/apis/v1alpha1/nginxproxy_types.go index 79f274dcd6..fac34f0504 100644 --- a/apis/v1alpha1/nginxproxy_types.go +++ b/apis/v1alpha1/nginxproxy_types.go @@ -137,7 +137,7 @@ type RewriteClientIP struct { // 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, NGINX will rewrite the client IP to 22.22.22.22. + // and TrustedAddresses is set to 55.55.55.1/32, 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 @@ -149,17 +149,17 @@ type RewriteClientIP struct { // 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 or IP addresses: 10.0.0.0, 192.33.21/24, fe80::1/128. + // TrustedAddresses only supports CIDR blocks: 192.33.21.1/24, fe80::1/64. // To trust all addresses (not recommended for production), 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=set + // +listType=atomic // // // +optional - TrustedAddresses []TrustedAddress `json:"trustedAddresses,omitempty"` + TrustedAddresses []Address `json:"trustedAddresses,omitempty"` } // RewriteClientIPModeType defines how NGINX Gateway Fabric will determine the client's original IP address. @@ -179,10 +179,27 @@ const ( RewriteClientIPModeXForwardedFor RewriteClientIPModeType = "XForwardedFor" ) -// TrustedAddress is a string value representing a CIDR block or an IP address. -// Examples: 10.0.0.2/32, 10.0.0.1, fe80::1/128, ::1/24. -// -// +kubebuilder:validation:Pattern=`^(?:(?:[0-9]{1,3}\.){3}[0-9]{1,3}(?:\/(?:[0-9]|[12][0-9]|3[0-2]))?|(?:[0-9a-fA-F]{1,4}:){7}[0-9a-fA-F]{1,4}(?:\/(?:[0-9]|[1-9][0-9]|1[0-1][0-9]|12[0-8]))?)$` -// -//nolint:lll -type TrustedAddress string +// Address is a struct that specifies address type and value. +type Address struct { + // Type specifies the type of address. + // Default is "cidr" which specifies that the address is a CIDR block. + // + // +optional + // +kubebuilder:default:=cidr + Type AddressType `json:"type,omitempty"` + + // Value specifies the address value. + // + // +optional + Value string `json:"value,omitempty"` +} + +// AddressType specifies the type of address. +// +kubebuilder:validation:Enum=cidr +type AddressType string + +const ( + // AddressTypeCIDR specifies that the address is a CIDR block. + // kubebuilder:validation:Pattern=`(\/([0-9]?[0-9]?[0-8]))$` + AddressTypeCIDR AddressType = "cidr" +) diff --git a/apis/v1alpha1/zz_generated.deepcopy.go b/apis/v1alpha1/zz_generated.deepcopy.go index 42410e127b..bffbb7dfdb 100644 --- a/apis/v1alpha1/zz_generated.deepcopy.go +++ b/apis/v1alpha1/zz_generated.deepcopy.go @@ -10,6 +10,21 @@ import ( "sigs.k8s.io/gateway-api/apis/v1alpha2" ) +// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. +func (in *Address) DeepCopyInto(out *Address) { + *out = *in +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new Address. +func (in *Address) DeepCopy() *Address { + if in == nil { + return nil + } + out := new(Address) + in.DeepCopyInto(out) + return out +} + // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *ClientBody) DeepCopyInto(out *ClientBody) { *out = *in @@ -483,7 +498,7 @@ func (in *RewriteClientIP) DeepCopyInto(out *RewriteClientIP) { } if in.TrustedAddresses != nil { in, out := &in.TrustedAddresses, &out.TrustedAddresses - *out = make([]TrustedAddress, len(*in)) + *out = make([]Address, len(*in)) copy(*out, *in) } } diff --git a/charts/nginx-gateway-fabric/values.yaml b/charts/nginx-gateway-fabric/values.yaml index 322776125d..1bf3de338e 100644 --- a/charts/nginx-gateway-fabric/values.yaml +++ b/charts/nginx-gateway-fabric/values.yaml @@ -94,10 +94,15 @@ nginx: # disableHTTP2: false # ipFamily: dual # rewriteClientIP: - # mode: "XForwardedFor" - # # -- Set to the CIDR range or IP of the proxy that sits in front of NGINX Gateway Fabric. - # trustedAddresses: [] - # setIPRecursively: true + # mode: "ProxyProtocol" + # # -- The trusted addresses field needs to be replaced with the load balancer's IP address and type. + # trustedAddresses: [ + # { + # # -- The IP address of the load balancer. + # value: "", + # type: "cidr", + # } + # ] # telemetry: # exporter: # endpoint: otel-collector.default.svc:4317 diff --git a/config/crd/bases/gateway.nginx.org_nginxproxies.yaml b/config/crd/bases/gateway.nginx.org_nginxproxies.yaml index 904d7bb421..e1644b852d 100644 --- a/config/crd/bases/gateway.nginx.org_nginxproxies.yaml +++ b/config/crd/bases/gateway.nginx.org_nginxproxies.yaml @@ -84,7 +84,7 @@ spec: 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, NGINX will rewrite the client IP to 22.22.22.22. + and TrustedAddresses is set to 55.55.55.1/32, 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 @@ -95,20 +95,30 @@ spec: 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 or IP addresses: 10.0.0.0, 192.33.21/24, fe80::1/128. + TrustedAddresses only supports CIDR blocks: 192.33.21.1/24, fe80::1/64. To trust all addresses (not recommended for production), 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: |- - TrustedAddress is a string value representing a CIDR block or an IP address. - Examples: 10.0.0.2/32, 10.0.0.1, fe80::1/128, ::1/24. - pattern: ^(?:(?:[0-9]{1,3}\.){3}[0-9]{1,3}(?:\/(?:[0-9]|[12][0-9]|3[0-2]))?|(?:[0-9a-fA-F]{1,4}:){7}[0-9a-fA-F]{1,4}(?:\/(?:[0-9]|[1-9][0-9]|1[0-1][0-9]|12[0-8]))?)$ - type: string + description: Address is a struct that specifies address type + and value. + properties: + type: + default: cidr + description: |- + Type specifies the type of address. + Default is "cidr" which specifies that the address is a CIDR block. + enum: + - cidr + type: string + value: + description: Value specifies the address value. + type: string + type: object maxItems: 16 type: array - x-kubernetes-list-type: set + x-kubernetes-list-type: atomic type: object x-kubernetes-validations: - message: if mode is set, trustedAddresses is a required field diff --git a/deploy/crds.yaml b/deploy/crds.yaml index c3631cd7c0..2b0ea4f133 100644 --- a/deploy/crds.yaml +++ b/deploy/crds.yaml @@ -669,7 +669,7 @@ spec: 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, NGINX will rewrite the client IP to 22.22.22.22. + and TrustedAddresses is set to 55.55.55.1/32, 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 @@ -680,20 +680,30 @@ spec: 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 or IP addresses: 10.0.0.0, 192.33.21/24, fe80::1/128. + TrustedAddresses only supports CIDR blocks: 192.33.21.1/24, fe80::1/64. To trust all addresses (not recommended for production), 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: |- - TrustedAddress is a string value representing a CIDR block or an IP address. - Examples: 10.0.0.2/32, 10.0.0.1, fe80::1/128, ::1/24. - pattern: ^(?:(?:[0-9]{1,3}\.){3}[0-9]{1,3}(?:\/(?:[0-9]|[12][0-9]|3[0-2]))?|(?:[0-9a-fA-F]{1,4}:){7}[0-9a-fA-F]{1,4}(?:\/(?:[0-9]|[1-9][0-9]|1[0-1][0-9]|12[0-8]))?)$ - type: string + description: Address is a struct that specifies address type + and value. + properties: + type: + default: cidr + description: |- + Type specifies the type of address. + Default is "cidr" which specifies that the address is a CIDR block. + enum: + - cidr + type: string + value: + description: Value specifies the address value. + type: string + type: object maxItems: 16 type: array - x-kubernetes-list-type: set + x-kubernetes-list-type: atomic type: object x-kubernetes-validations: - message: if mode is set, trustedAddresses is a required field diff --git a/internal/mode/static/nginx/config/servers_template.go b/internal/mode/static/nginx/config/servers_template.go index 3c851457d2..80b0847e2e 100644 --- a/internal/mode/static/nginx/config/servers_template.go +++ b/internal/mode/static/nginx/config/servers_template.go @@ -13,8 +13,8 @@ server { listen [::]:{{ $s.Listen }} ssl default_server{{ $.RewriteClientIP.ProxyProtocol }}; {{- end }} ssl_reject_handshake on; - {{- range $cidr := $.RewriteClientIP.RealIPFrom }} - set_real_ip_from {{ $cidr }}; + {{- range $address := $.RewriteClientIP.RealIPFrom }} + set_real_ip_from {{ $address }}; {{- end}} {{- if $.RewriteClientIP.RealIPHeader}} real_ip_header {{ $.RewriteClientIP.RealIPHeader }}; @@ -31,8 +31,8 @@ server { {{- if $.IPFamily.IPv6 }} listen [::]:{{ $s.Listen }} default_server{{ $.RewriteClientIP.ProxyProtocol }}; {{- end }} - {{- range $cidr := $.RewriteClientIP.RealIPFrom }} - set_real_ip_from {{ $cidr }}; + {{- range $address := $.RewriteClientIP.RealIPFrom }} + set_real_ip_from {{ $address }}; {{- end}} {{- if $.RewriteClientIP.RealIPHeader}} real_ip_header {{ $.RewriteClientIP.RealIPHeader }}; @@ -77,8 +77,8 @@ server { include {{ $i.Name }}; {{- end }} - {{- range $cidr := $.RewriteClientIP.RealIPFrom }} - set_real_ip_from {{ $cidr }}; + {{- range $address := $.RewriteClientIP.RealIPFrom }} + set_real_ip_from {{ $address }}; {{- end}} {{- if $.RewriteClientIP.RealIPHeader}} real_ip_header {{ $.RewriteClientIP.RealIPHeader }}; diff --git a/internal/mode/static/nginx/config/stream_servers_template.go b/internal/mode/static/nginx/config/stream_servers_template.go index 0a84e02338..58a95a70b0 100644 --- a/internal/mode/static/nginx/config/stream_servers_template.go +++ b/internal/mode/static/nginx/config/stream_servers_template.go @@ -10,8 +10,8 @@ server { listen [::]:{{ $s.Listen }}; {{- end }} - {{- range $cidr := $s.RewriteClientIP.RealIPFrom }} - set_real_ip_from {{ $cidr }}; + {{- range $address := $s.RewriteClientIP.RealIPFrom }} + set_real_ip_from {{ $address }}; {{- end}} {{- if $.Plus }} status_zone {{ $s.StatusZone }}; diff --git a/internal/mode/static/state/dataplane/configuration.go b/internal/mode/static/state/dataplane/configuration.go index ce06c94b1e..eefe5e4bb4 100644 --- a/internal/mode/static/state/dataplane/configuration.go +++ b/internal/mode/static/state/dataplane/configuration.go @@ -863,7 +863,7 @@ func buildBaseHTTPConfig(g *graph.Graph) BaseHTTPConfig { } if len(g.NginxProxy.Source.Spec.RewriteClientIP.TrustedAddresses) > 0 { - baseConfig.RewriteClientIPSettings.TrustedAddresses = convertTrustedAddresses( + baseConfig.RewriteClientIPSettings.TrustedAddresses = convertAddresses( g.NginxProxy.Source.Spec.RewriteClientIP.TrustedAddresses, ) } @@ -894,10 +894,10 @@ func buildPolicies(graphPolicies []*graph.Policy) []policies.Policy { return finalPolicies } -func convertTrustedAddresses(addresses []ngfAPI.TrustedAddress) []string { +func convertAddresses(addresses []ngfAPI.Address) []string { trustedAddresses := make([]string, len(addresses)) for i, addr := range addresses { - trustedAddresses[i] = string(addr) + trustedAddresses[i] = addr.Value } return trustedAddresses } diff --git a/internal/mode/static/state/dataplane/configuration_test.go b/internal/mode/static/state/dataplane/configuration_test.go index b1d6d98c54..a2ae5dc910 100644 --- a/internal/mode/static/state/dataplane/configuration_test.go +++ b/internal/mode/static/state/dataplane/configuration_test.go @@ -2198,8 +2198,13 @@ func TestBuildConfiguration(t *testing.T) { Spec: ngfAPI.NginxProxySpec{ RewriteClientIP: &ngfAPI.RewriteClientIP{ SetIPRecursively: helpers.GetPointer(true), - TrustedAddresses: []ngfAPI.TrustedAddress{"1.1.1.1/32"}, - Mode: helpers.GetPointer(ngfAPI.RewriteClientIPModeProxyProtocol), + TrustedAddresses: []ngfAPI.Address{ + { + Type: ngfAPI.AddressTypeCIDR, + Value: "1.1.1.1/32", + }, + }, + Mode: helpers.GetPointer(ngfAPI.RewriteClientIPModeProxyProtocol), }, }, }, @@ -3619,8 +3624,13 @@ func TestBuildRewriteIPSettings(t *testing.T) { Source: &ngfAPI.NginxProxy{ Spec: ngfAPI.NginxProxySpec{ RewriteClientIP: &ngfAPI.RewriteClientIP{ - Mode: helpers.GetPointer(ngfAPI.RewriteClientIPModeProxyProtocol), - TrustedAddresses: []ngfAPI.TrustedAddress{"10.9.9.4"}, + Mode: helpers.GetPointer(ngfAPI.RewriteClientIPModeProxyProtocol), + TrustedAddresses: []ngfAPI.Address{ + { + Type: ngfAPI.AddressTypeCIDR, + Value: "10.9.9.4/32", + }, + }, SetIPRecursively: helpers.GetPointer(true), }, }, @@ -3629,7 +3639,7 @@ func TestBuildRewriteIPSettings(t *testing.T) { }, expRewriteIPSettings: RewriteClientIPSettings{ Mode: RewriteIPModeProxyProtocol, - TrustedAddresses: []string{"10.9.9.4"}, + TrustedAddresses: []string{"10.9.9.4/32"}, IPRecursive: true, }, }, @@ -3641,8 +3651,13 @@ func TestBuildRewriteIPSettings(t *testing.T) { Source: &ngfAPI.NginxProxy{ Spec: ngfAPI.NginxProxySpec{ RewriteClientIP: &ngfAPI.RewriteClientIP{ - Mode: helpers.GetPointer(ngfAPI.RewriteClientIPModeXForwardedFor), - TrustedAddresses: []ngfAPI.TrustedAddress{"76.89.90.11"}, + Mode: helpers.GetPointer(ngfAPI.RewriteClientIPModeXForwardedFor), + TrustedAddresses: []ngfAPI.Address{ + { + Type: ngfAPI.AddressTypeCIDR, + Value: "76.89.90.11/24", + }, + }, SetIPRecursively: helpers.GetPointer(true), }, }, @@ -3651,7 +3666,7 @@ func TestBuildRewriteIPSettings(t *testing.T) { }, expRewriteIPSettings: RewriteClientIPSettings{ Mode: RewriteIPModeXForwardedFor, - TrustedAddresses: []string{"76.89.90.11"}, + TrustedAddresses: []string{"76.89.90.11/24"}, IPRecursive: true, }, }, @@ -3663,8 +3678,25 @@ func TestBuildRewriteIPSettings(t *testing.T) { Source: &ngfAPI.NginxProxy{ Spec: ngfAPI.NginxProxySpec{ RewriteClientIP: &ngfAPI.RewriteClientIP{ - Mode: helpers.GetPointer(ngfAPI.RewriteClientIPModeXForwardedFor), - TrustedAddresses: []ngfAPI.TrustedAddress{"5.5.5.5", "1.1.1.1/32", "2.2.2.2/32", "3.3.3.3/24"}, + Mode: helpers.GetPointer(ngfAPI.RewriteClientIPModeXForwardedFor), + TrustedAddresses: []ngfAPI.Address{ + { + Type: ngfAPI.AddressTypeCIDR, + Value: "5.5.5.5/12", + }, + { + Type: ngfAPI.AddressTypeCIDR, + Value: "1.1.1.1/26", + }, + { + Type: ngfAPI.AddressTypeCIDR, + Value: "2.2.2.2/32", + }, + { + Type: ngfAPI.AddressTypeCIDR, + Value: "3.3.3.3/24", + }, + }, SetIPRecursively: helpers.GetPointer(false), }, }, @@ -3673,7 +3705,7 @@ func TestBuildRewriteIPSettings(t *testing.T) { }, expRewriteIPSettings: RewriteClientIPSettings{ Mode: RewriteIPModeXForwardedFor, - TrustedAddresses: []string{"5.5.5.5", "1.1.1.1/32", "2.2.2.2/32", "3.3.3.3/24"}, + TrustedAddresses: []string{"5.5.5.5/12", "1.1.1.1/26", "2.2.2.2/32", "3.3.3.3/24"}, IPRecursive: false, }, }, diff --git a/internal/mode/static/state/graph/nginxproxy.go b/internal/mode/static/state/graph/nginxproxy.go index 42993c1347..67d156791a 100644 --- a/internal/mode/static/state/graph/nginxproxy.go +++ b/internal/mode/static/state/graph/nginxproxy.go @@ -172,15 +172,12 @@ func validateRewriteClientIP(npCfg *ngfAPI.NginxProxy) field.ErrorList { } for _, addr := range rewriteClientIP.TrustedAddresses { - cidrError := k8svalidation.IsValidCIDR(trustedAddressesPath, string(addr)) - ipError := k8svalidation.IsValidIP(trustedAddressesPath, string(addr)) - - if cidrError != nil && ipError != nil { + if err := k8svalidation.IsValidCIDR(trustedAddressesPath, addr.Value); err != nil { allErrs = append( allErrs, - field.Invalid(trustedAddressesPath.Child(string(addr)), + field.Invalid(trustedAddressesPath.Child(addr.Value), addr, - "must be a valid IP address or CIDR range", + err.ToAggregate().Error(), ), ) } diff --git a/internal/mode/static/state/graph/nginxproxy_test.go b/internal/mode/static/state/graph/nginxproxy_test.go index 3df1347981..e5ecab47b8 100644 --- a/internal/mode/static/state/graph/nginxproxy_test.go +++ b/internal/mode/static/state/graph/nginxproxy_test.go @@ -2,7 +2,6 @@ package graph import ( "errors" - "fmt" "testing" . "github.com/onsi/gomega" @@ -269,8 +268,17 @@ func TestValidateNginxProxy(t *testing.T) { IPFamily: helpers.GetPointer[ngfAPI.IPFamilyType](ngfAPI.Dual), RewriteClientIP: &ngfAPI.RewriteClientIP{ SetIPRecursively: helpers.GetPointer(true), - TrustedAddresses: []ngfAPI.TrustedAddress{"2001:db8:a0b:12f0::1/32", "1.1.1.1"}, - Mode: helpers.GetPointer(ngfAPI.RewriteClientIPModeProxyProtocol), + TrustedAddresses: []ngfAPI.Address{ + { + Type: ngfAPI.AddressTypeCIDR, + Value: "2001:db8:a0b:12f0::1/32", + }, + { + Type: ngfAPI.AddressTypeCIDR, + Value: "1.1.1.1/24", + }, + }, + Mode: helpers.GetPointer(ngfAPI.RewriteClientIPModeProxyProtocol), }, }, }, @@ -378,8 +386,17 @@ func TestValidateRewriteClientIP(t *testing.T) { Spec: ngfAPI.NginxProxySpec{ RewriteClientIP: &ngfAPI.RewriteClientIP{ SetIPRecursively: helpers.GetPointer(true), - TrustedAddresses: []ngfAPI.TrustedAddress{"2001:db8:a0b:12f0::1/32", "10.56.32.11/32"}, - Mode: helpers.GetPointer(ngfAPI.RewriteClientIPModeProxyProtocol), + TrustedAddresses: []ngfAPI.Address{ + { + Type: ngfAPI.AddressTypeCIDR, + Value: "2001:db8:a0b:12f0::1/32", + }, + { + Type: ngfAPI.AddressTypeCIDR, + Value: "10.56.32.11/32", + }, + }, + Mode: helpers.GetPointer(ngfAPI.RewriteClientIPModeProxyProtocol), }, }, }, @@ -392,30 +409,25 @@ func TestValidateRewriteClientIP(t *testing.T) { Spec: ngfAPI.NginxProxySpec{ RewriteClientIP: &ngfAPI.RewriteClientIP{ SetIPRecursively: helpers.GetPointer(true), - TrustedAddresses: []ngfAPI.TrustedAddress{"2001:db8::/129", "10.0.0.1"}, - Mode: helpers.GetPointer(ngfAPI.RewriteClientIPModeProxyProtocol), + TrustedAddresses: []ngfAPI.Address{ + { + Type: ngfAPI.AddressTypeCIDR, + Value: "2001:db8::/129", + }, + { + Type: ngfAPI.AddressTypeCIDR, + Value: "10.0.0.1/32", + }, + }, + Mode: helpers.GetPointer(ngfAPI.RewriteClientIPModeProxyProtocol), }, }, }, expectErrCount: 1, errorString: "spec.rewriteClientIP.trustedAddresses.2001:db8::/129: " + - "Invalid value: \"2001:db8::/129\": must be a valid IP address or CIDR range", - }, - { - name: "invalid IP and valid CIDR in trustedAddresses", - validator: createInvalidValidator(), - np: &ngfAPI.NginxProxy{ - Spec: ngfAPI.NginxProxySpec{ - RewriteClientIP: &ngfAPI.RewriteClientIP{ - SetIPRecursively: helpers.GetPointer(true), - TrustedAddresses: []ngfAPI.TrustedAddress{"2001:db8::1/48", "256.100.50.25"}, - Mode: helpers.GetPointer(ngfAPI.RewriteClientIPModeProxyProtocol), - }, - }, - }, - expectErrCount: 1, - errorString: "spec.rewriteClientIP.trustedAddresses.256.100.50.25: " + - "Invalid value: \"256.100.50.25\": must be a valid IP address or CIDR range", + "Invalid value: v1alpha1.Address{Type:\"cidr\", Value:\"2001:db8::/129\"}: " + + "spec.rewriteClientIP.trustedAddresses: Invalid value: " + + "\"2001:db8::/129\": 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", @@ -437,13 +449,28 @@ func TestValidateRewriteClientIP(t *testing.T) { Spec: ngfAPI.NginxProxySpec{ RewriteClientIP: &ngfAPI.RewriteClientIP{ Mode: helpers.GetPointer(ngfAPI.RewriteClientIPModeProxyProtocol), - TrustedAddresses: []ngfAPI.TrustedAddress{ - "2001:db8:a0b:12f0::1/32", "2001:db8:a0b:12f0::1/32", "2001:db8:a0b:12f0::1/32", - "2001:db8:a0b:12f0::1/32", "2001:db8:a0b:12f0::1/32", "2001:db8:a0b:12f0::1/32", - "2001:db8:a0b:12f0::1/32", "2001:db8:a0b:12f0::1/32", "2001:db8:a0b:12f0::1/32", - "2001:db8:a0b:12f0::1/32", "2001:db8:a0b:12f0::1/32", "2001:db8:a0b:12f0::1/32", - "2001:db8:a0b:12f0::1/32", "2001:db8:a0b:12f0::1/32", - "2001:db8:a0b:12f0::1/32", "2001:db8:a0b:12f0::1/32", "2001:db8:a0b:12f0::1/32", + TrustedAddresses: []ngfAPI.Address{ + {Type: ngfAPI.AddressTypeCIDR, Value: "2001:db8:a0b:12f0::1/32"}, + {Type: ngfAPI.AddressTypeCIDR, Value: "2001:db8:a0b:12f0::1/32"}, + {Type: ngfAPI.AddressTypeCIDR, Value: "2001:db8:a0b:12f0::1/32"}, + {Type: ngfAPI.AddressTypeCIDR, Value: "2001:db8:a0b:12f0::1/32"}, + {Type: ngfAPI.AddressTypeCIDR, Value: "2001:db8:a0b:12f0::1/32"}, + {Type: ngfAPI.AddressTypeCIDR, Value: "2001:db8:a0b:12f0::1/32"}, + {Type: ngfAPI.AddressTypeCIDR, Value: "2001:db8:a0b:12f0::1/32"}, + {Type: ngfAPI.AddressTypeCIDR, Value: "2001:db8:a0b:12f0::1/32"}, + {Type: ngfAPI.AddressTypeCIDR, Value: "2001:db8:a0b:12f0::1/32"}, + {Type: ngfAPI.AddressTypeCIDR, Value: "2001:db8:a0b:12f0::1/32"}, + {Type: ngfAPI.AddressTypeCIDR, Value: "2001:db8:a0b:12f0::1/32"}, + {Type: ngfAPI.AddressTypeCIDR, Value: "2001:db8:a0b:12f0::1/32"}, + {Type: ngfAPI.AddressTypeCIDR, Value: "2001:db8:a0b:12f0::1/32"}, + {Type: ngfAPI.AddressTypeCIDR, Value: "2001:db8:a0b:12f0::1/32"}, + {Type: ngfAPI.AddressTypeCIDR, Value: "2001:db8:a0b:12f0::1/32"}, + {Type: ngfAPI.AddressTypeCIDR, Value: "2001:db8:a0b:12f0::1/32"}, + {Type: ngfAPI.AddressTypeCIDR, Value: "2001:db8:a0b:12f0::1/32"}, + {Type: ngfAPI.AddressTypeCIDR, Value: "2001:db8:a0b:12f0::1/32"}, + {Type: ngfAPI.AddressTypeCIDR, Value: "2001:db8:a0b:12f0::1/32"}, + {Type: ngfAPI.AddressTypeCIDR, Value: "2001:db8:a0b:12f0::1/32"}, + {Type: ngfAPI.AddressTypeCIDR, Value: "2001:db8:a0b:12f0::1/32"}, }, }, }, @@ -457,8 +484,17 @@ func TestValidateRewriteClientIP(t *testing.T) { np: &ngfAPI.NginxProxy{ Spec: ngfAPI.NginxProxySpec{ RewriteClientIP: &ngfAPI.RewriteClientIP{ - Mode: helpers.GetPointer(ngfAPI.RewriteClientIPModeType("invalid")), - TrustedAddresses: []ngfAPI.TrustedAddress{"2001:db8:a0b:12f0::1/32", "10.0.0.1/32"}, + Mode: helpers.GetPointer(ngfAPI.RewriteClientIPModeType("invalid")), + TrustedAddresses: []ngfAPI.Address{ + { + Type: ngfAPI.AddressTypeCIDR, + Value: "2001:db8:a0b:12f0::1/32", + }, + { + Type: ngfAPI.AddressTypeCIDR, + Value: "10.0.0.1/32", + }, + }, }, }, }, @@ -490,7 +526,6 @@ func TestValidateRewriteClientIP(t *testing.T) { allErrs := validateRewriteClientIP(test.np) g.Expect(allErrs).To(HaveLen(test.expectErrCount)) if len(allErrs) > 0 { - fmt.Println(allErrs.ToAggregate().Error()) g.Expect(allErrs.ToAggregate().Error()).To(Equal(test.errorString)) } }) diff --git a/site/content/reference/api.md b/site/content/reference/api.md index deb2eee011..b7e49011af 100644 --- a/site/content/reference/api.md +++ b/site/content/reference/api.md @@ -467,6 +467,76 @@ sigs.k8s.io/gateway-api/apis/v1alpha2.PolicyStatus +

      Address + +

      +

      +(Appears on: +RewriteClientIP) +

      +

      +

      Address is a struct that specifies address type and value.

      +

      + + + + + + + + + + + + + + + + + +
      FieldDescription
      +type
      + + +AddressType + + +      		 +(Optional) +

      Type specifies the type of address. +Default is “cidr” which specifies that the address is a CIDR block.

      +
      +value
      + +string + +      		 +(Optional) +

      Value specifies the address value.

      +
      +

      AddressType +(string alias)

      +

      +

      +(Appears on: +Address) +

      +

      +

      AddressType specifies the type of address.

      +

      + + + + + + + + + + +
      ValueDescription

      "cidr"

      AddressTypeCIDR specifies that the address is a CIDR block. +kubebuilder:validation:Pattern=(\/([0-9]?[0-9]?[0-8]))$

      +

      ClientBody

      @@ -1091,7 +1161,7 @@ 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, NGINX will rewrite the client IP to 22.22.22.22. +and TrustedAddresses is set to 55.55.55.1⁄32, 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

      @@ -1101,8 +1171,8 @@ Sets NGINX directive real_ip_recursive: -[]TrustedAddress + +[]Address       @@ -1112,7 +1182,7 @@ Sets NGINX directive real_ip_recursive: https://nginx.org/en/docs/http/ngx_http_realip_module.html#set_real_ip_from @@ -1491,17 +1561,6 @@ Examples of invalid names: some-$value, quoted-“value”-name, unescap -

      TrustedAddress -(string alias)

      -

      -

      -(Appears on: -RewriteClientIP) -

      -

      -

      TrustedAddress is a string value representing a CIDR block or an IP address. -Examples: 10.0.0.2⁄32, 10.0.0.1, fe80::1⁄128, ::1⁄24.

      -

      Generated with gen-crd-api-reference-docs From 20878e704ab1bec01a83c7c7623b9dceb6a080b8 Mon Sep 17 00:00:00 2001 From: Saloni Date: Thu, 5 Sep 2024 12:28:24 -0600 Subject: [PATCH 16/19] improve regex for cidr --- apis/v1alpha1/nginxproxy_types.go | 5 +++-- .../bases/gateway.nginx.org_nginxproxies.yaml | 4 +++- deploy/crds.yaml | 4 +++- .../mode/static/state/graph/nginxproxy.go | 19 +++++++++++++---- .../static/state/graph/nginxproxy_test.go | 21 +++++++++++++++++++ site/content/reference/api.md | 2 +- 6 files changed, 46 insertions(+), 9 deletions(-) diff --git a/apis/v1alpha1/nginxproxy_types.go b/apis/v1alpha1/nginxproxy_types.go index fac34f0504..431adeeab5 100644 --- a/apis/v1alpha1/nginxproxy_types.go +++ b/apis/v1alpha1/nginxproxy_types.go @@ -155,7 +155,8 @@ type RewriteClientIP struct { // 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=map + // +listMapKey=type // // // +optional @@ -200,6 +201,6 @@ type AddressType string const ( // AddressTypeCIDR specifies that the address is a CIDR block. - // kubebuilder:validation:Pattern=`(\/([0-9]?[0-9]?[0-8]))$` + // kubebuilder:validation:Pattern=`^[\.a-zA-Z0-9::]*(\/([0-9]?[0-9]?[0-8]))$` AddressTypeCIDR AddressType = "cidr" ) diff --git a/config/crd/bases/gateway.nginx.org_nginxproxies.yaml b/config/crd/bases/gateway.nginx.org_nginxproxies.yaml index e1644b852d..19ed93c64b 100644 --- a/config/crd/bases/gateway.nginx.org_nginxproxies.yaml +++ b/config/crd/bases/gateway.nginx.org_nginxproxies.yaml @@ -118,7 +118,9 @@ spec: type: object maxItems: 16 type: array - x-kubernetes-list-type: atomic + x-kubernetes-list-map-keys: + - type + x-kubernetes-list-type: map type: object x-kubernetes-validations: - message: if mode is set, trustedAddresses is a required field diff --git a/deploy/crds.yaml b/deploy/crds.yaml index 2b0ea4f133..ef8ffea772 100644 --- a/deploy/crds.yaml +++ b/deploy/crds.yaml @@ -703,7 +703,9 @@ spec: type: object maxItems: 16 type: array - x-kubernetes-list-type: atomic + x-kubernetes-list-map-keys: + - type + x-kubernetes-list-type: map type: object x-kubernetes-validations: - message: if mode is set, trustedAddresses is a required field diff --git a/internal/mode/static/state/graph/nginxproxy.go b/internal/mode/static/state/graph/nginxproxy.go index 67d156791a..2c1cd3ffb9 100644 --- a/internal/mode/static/state/graph/nginxproxy.go +++ b/internal/mode/static/state/graph/nginxproxy.go @@ -172,12 +172,23 @@ func validateRewriteClientIP(npCfg *ngfAPI.NginxProxy) field.ErrorList { } for _, addr := range rewriteClientIP.TrustedAddresses { - if err := k8svalidation.IsValidCIDR(trustedAddressesPath, addr.Value); err != nil { + switch addr.Type { + case ngfAPI.AddressTypeCIDR: + if err := k8svalidation.IsValidCIDR(trustedAddressesPath, addr.Value); err != nil { + allErrs = append( + allErrs, + field.Invalid(trustedAddressesPath.Child(addr.Value), + addr, + err.ToAggregate().Error(), + ), + ) + } + default: allErrs = append( allErrs, - field.Invalid(trustedAddressesPath.Child(addr.Value), - addr, - err.ToAggregate().Error(), + field.NotSupported(trustedAddressesPath.Child(addr.Value), + addr.Type, + []string{string(ngfAPI.AddressTypeCIDR)}, ), ) } diff --git a/internal/mode/static/state/graph/nginxproxy_test.go b/internal/mode/static/state/graph/nginxproxy_test.go index e5ecab47b8..8603598f5c 100644 --- a/internal/mode/static/state/graph/nginxproxy_test.go +++ b/internal/mode/static/state/graph/nginxproxy_test.go @@ -517,6 +517,27 @@ func TestValidateRewriteClientIP(t *testing.T) { "required when mode is set, spec.rewriteClientIP.mode: " + "Unsupported value: \"invalid\": supported values: \"ProxyProtocol\", \"XForwardedFor\"]", }, + { + name: "invalid address type in trustedAddresses", + validator: createInvalidValidator(), + np: &ngfAPI.NginxProxy{ + Spec: ngfAPI.NginxProxySpec{ + RewriteClientIP: &ngfAPI.RewriteClientIP{ + SetIPRecursively: helpers.GetPointer(true), + TrustedAddresses: []ngfAPI.Address{ + { + Type: ngfAPI.AddressType("invalid"), + Value: "2001:db8::/129", + }, + }, + Mode: helpers.GetPointer(ngfAPI.RewriteClientIPModeProxyProtocol), + }, + }, + }, + expectErrCount: 1, + errorString: "spec.rewriteClientIP.trustedAddresses.2001:db8::/129: " + + "Unsupported value: \"invalid\": supported values: \"cidr\"", + }, } for _, test := range tests { diff --git a/site/content/reference/api.md b/site/content/reference/api.md index b7e49011af..702e758b2e 100644 --- a/site/content/reference/api.md +++ b/site/content/reference/api.md @@ -533,7 +533,7 @@ string

      "cidr"

      AddressTypeCIDR specifies that the address is a CIDR block. -kubebuilder:validation:Pattern=(\/([0-9]?[0-9]?[0-8]))$

      +kubebuilder:validation:Pattern=^[\.a-zA-Z0-9::]*(\/([0-9]?[0-9]?[0-8]))$

      From ef2170badcafe874447c4a16c7a6d491abf0996c Mon Sep 17 00:00:00 2001 From: Saloni Date: Thu, 5 Sep 2024 13:16:56 -0600 Subject: [PATCH 17/19] updates based on reviews --- apis/v1alpha1/nginxproxy_types.go | 5 ++--- charts/nginx-gateway-fabric/values.yaml | 1 + internal/mode/static/state/graph/nginxproxy.go | 2 +- internal/mode/static/state/graph/nginxproxy_test.go | 2 +- site/content/reference/api.md | 2 +- 5 files changed, 6 insertions(+), 6 deletions(-) diff --git a/apis/v1alpha1/nginxproxy_types.go b/apis/v1alpha1/nginxproxy_types.go index 431adeeab5..acb42e4f5e 100644 --- a/apis/v1alpha1/nginxproxy_types.go +++ b/apis/v1alpha1/nginxproxy_types.go @@ -155,10 +155,9 @@ type RewriteClientIP struct { // 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=map + // +listType=map // +listMapKey=type // - // // +optional TrustedAddresses []Address `json:"trustedAddresses,omitempty"` } @@ -201,6 +200,6 @@ type AddressType string const ( // AddressTypeCIDR specifies that the address is a CIDR block. - // kubebuilder:validation:Pattern=`^[\.a-zA-Z0-9::]*(\/([0-9]?[0-9]?[0-8]))$` + // kubebuilder:validation:Pattern=`^[\.a-zA-Z0-9:]*(\/([0-9]?[0-9]?[0-9]))$` AddressTypeCIDR AddressType = "cidr" ) diff --git a/charts/nginx-gateway-fabric/values.yaml b/charts/nginx-gateway-fabric/values.yaml index 1bf3de338e..ccde62f569 100644 --- a/charts/nginx-gateway-fabric/values.yaml +++ b/charts/nginx-gateway-fabric/values.yaml @@ -103,6 +103,7 @@ nginx: # type: "cidr", # } # ] + # setIPRecursively: true # telemetry: # exporter: # endpoint: otel-collector.default.svc:4317 diff --git a/internal/mode/static/state/graph/nginxproxy.go b/internal/mode/static/state/graph/nginxproxy.go index 2c1cd3ffb9..cf6dc70990 100644 --- a/internal/mode/static/state/graph/nginxproxy.go +++ b/internal/mode/static/state/graph/nginxproxy.go @@ -186,7 +186,7 @@ func validateRewriteClientIP(npCfg *ngfAPI.NginxProxy) field.ErrorList { default: allErrs = append( allErrs, - field.NotSupported(trustedAddressesPath.Child(addr.Value), + field.NotSupported(trustedAddressesPath.Child("type"), addr.Type, []string{string(ngfAPI.AddressTypeCIDR)}, ), diff --git a/internal/mode/static/state/graph/nginxproxy_test.go b/internal/mode/static/state/graph/nginxproxy_test.go index 8603598f5c..8c9f236237 100644 --- a/internal/mode/static/state/graph/nginxproxy_test.go +++ b/internal/mode/static/state/graph/nginxproxy_test.go @@ -535,7 +535,7 @@ func TestValidateRewriteClientIP(t *testing.T) { }, }, expectErrCount: 1, - errorString: "spec.rewriteClientIP.trustedAddresses.2001:db8::/129: " + + errorString: "spec.rewriteClientIP.trustedAddresses.type: " + "Unsupported value: \"invalid\": supported values: \"cidr\"", }, } diff --git a/site/content/reference/api.md b/site/content/reference/api.md index 702e758b2e..d5b191193b 100644 --- a/site/content/reference/api.md +++ b/site/content/reference/api.md @@ -533,7 +533,7 @@ string

      "cidr"

      AddressTypeCIDR specifies that the address is a CIDR block. -kubebuilder:validation:Pattern=^[\.a-zA-Z0-9::]*(\/([0-9]?[0-9]?[0-8]))$

      +kubebuilder:validation:Pattern=^[\.a-zA-Z0-9:]*(\/([0-9]?[0-9]?[0-9]))$

      From 5c3b6b7be64a18d3b7f6f23461886d3f99852908 Mon Sep 17 00:00:00 2001 From: Saloni Date: Thu, 5 Sep 2024 15:11:00 -0600 Subject: [PATCH 18/19] address nits --- charts/nginx-gateway-fabric/values.yaml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/charts/nginx-gateway-fabric/values.yaml b/charts/nginx-gateway-fabric/values.yaml index ccde62f569..c6e44cbae7 100644 --- a/charts/nginx-gateway-fabric/values.yaml +++ b/charts/nginx-gateway-fabric/values.yaml @@ -95,10 +95,10 @@ nginx: # ipFamily: dual # rewriteClientIP: # mode: "ProxyProtocol" - # # -- The trusted addresses field needs to be replaced with the load balancer's IP address and type. + # # -- The trusted addresses field needs to be replaced with the load balancer's address and type. # trustedAddresses: [ # { - # # -- The IP address of the load balancer. + # # -- The CIDR block of the load balancer(s). # value: "", # type: "cidr", # } From 8f670389240c8ae9b833b0e21abedd18e584a4d1 Mon Sep 17 00:00:00 2001 From: Saloni Date: Fri, 6 Sep 2024 13:09:46 -0600 Subject: [PATCH 19/19] remove redirect filter header modification --- charts/nginx-gateway-fabric/values.yaml | 2 +- internal/mode/static/nginx/config/servers.go | 34 ++++---------- .../mode/static/nginx/config/servers_test.go | 47 ------------------- 3 files changed, 9 insertions(+), 74 deletions(-) diff --git a/charts/nginx-gateway-fabric/values.yaml b/charts/nginx-gateway-fabric/values.yaml index c6e44cbae7..b81fb9063d 100644 --- a/charts/nginx-gateway-fabric/values.yaml +++ b/charts/nginx-gateway-fabric/values.yaml @@ -103,7 +103,7 @@ nginx: # type: "cidr", # } # ] - # setIPRecursively: true + # setIPRecursively: true # telemetry: # exporter: # endpoint: otel-collector.default.svc:4317 diff --git a/internal/mode/static/nginx/config/servers.go b/internal/mode/static/nginx/config/servers.go index 8ae4218db0..0fd1930f5e 100644 --- a/internal/mode/static/nginx/config/servers.go +++ b/internal/mode/static/nginx/config/servers.go @@ -800,7 +800,14 @@ func generateProxySetHeaders(filters *dataplane.HTTPFilters, grpc bool) []http.H copy(headers, grpcBaseHeaders) } - setHeaderForHTTPSRedirect(filters, headers) + if filters != nil && filters.RequestURLRewrite != nil && filters.RequestURLRewrite.Hostname != nil { + for i, header := range headers { + if header.Name == "Host" { + headers[i].Value = *filters.RequestURLRewrite.Hostname + break + } + } + } if filters == nil || filters.RequestHeaderModifiers == nil { return headers @@ -829,31 +836,6 @@ func generateProxySetHeaders(filters *dataplane.HTTPFilters, grpc bool) []http.H return append(proxySetHeaders, headers...) } -func setHeaderForHTTPSRedirect(filters *dataplane.HTTPFilters, headers []http.Header) { - if filters == nil { - return - } - - if filters.RequestURLRewrite != nil && filters.RequestURLRewrite.Hostname != nil { - for i, header := range headers { - if header.Name == "Host" { - headers[i].Value = *filters.RequestURLRewrite.Hostname - break - } - } - } - if filters.RequestRedirect != nil && - filters.RequestRedirect.Scheme != nil && - *filters.RequestRedirect.Scheme == http.HTTPSScheme { - for i, header := range headers { - if header.Name == "X-Forwarded-Proto" { - headers[i].Value = http.HTTPSScheme - return - } - } - } -} - func generateResponseHeaders(filters *dataplane.HTTPFilters) http.ResponseHeaders { if filters == nil || filters.ResponseHeaderModifiers == nil { return http.ResponseHeaders{} diff --git a/internal/mode/static/nginx/config/servers_test.go b/internal/mode/static/nginx/config/servers_test.go index 0a05149182..e3a9122b23 100644 --- a/internal/mode/static/nginx/config/servers_test.go +++ b/internal/mode/static/nginx/config/servers_test.go @@ -2681,53 +2681,6 @@ func TestGenerateProxySetHeaders(t *testing.T) { }, }, }, - { - msg: "request redirect filter with https scheme", - filters: &dataplane.HTTPFilters{ - RequestRedirect: &dataplane.HTTPRequestRedirectFilter{ - Scheme: helpers.GetPointer("https"), - }, - }, - expectedHeaders: []http.Header{ - { - Name: "Host", - Value: "$gw_api_compliant_host", - }, - { - Name: "X-Forwarded-For", - Value: "$proxy_add_x_forwarded_for", - }, - { - Name: "Upgrade", - Value: "$http_upgrade", - }, - { - Name: "Connection", - Value: "$connection_upgrade", - }, - { - Name: "X-Real-IP", - Value: "$remote_addr", - }, - { - Name: "X-Forwarded-Proto", - Value: "https", - }, - { - Name: "X-Forwarded-Host", - Value: "$host", - }, - { - Name: "X-Forwarded-Port", - Value: "$server_port", - }, - }, - }, - { - msg: "no filter set", - expectedHeaders: httpBaseHeaders, - GRPC: false, - }, } for _, tc := range tests {