From 7fb59de38f8313c23181b3bc78838733869adf34 Mon Sep 17 00:00:00 2001 From: Saloni Date: Tue, 23 Jul 2024 15:34:51 -0600 Subject: [PATCH] add proxy_protocol to server configuration --- apis/v1alpha1/nginxproxy_types.go | 6 ++++ charts/nginx-gateway-fabric/values.yaml | 1 + .../bases/gateway.nginx.org_nginxproxies.yaml | 5 ++++ deploy/crds.yaml | 5 ++++ .../mode/static/nginx/config/http/config.go | 5 ++-- internal/mode/static/nginx/config/servers.go | 5 ++-- .../static/nginx/config/servers_template.go | 17 +++++------ .../mode/static/nginx/config/servers_test.go | 26 ++++++++++++++++- .../static/state/dataplane/configuration.go | 6 ++++ .../state/dataplane/configuration_test.go | 28 +++++++++++++++++++ internal/mode/static/state/dataplane/types.go | 2 ++ .../how-to/monitoring/troubleshooting.md | 14 ++++++++++ site/content/reference/api.md | 26 +++++++++++++++++ 13 files changed, 133 insertions(+), 13 deletions(-) diff --git a/apis/v1alpha1/nginxproxy_types.go b/apis/v1alpha1/nginxproxy_types.go index 018911da7e..5d1d3199d9 100644 --- a/apis/v1alpha1/nginxproxy_types.go +++ b/apis/v1alpha1/nginxproxy_types.go @@ -58,6 +58,12 @@ type NginxProxySpec struct { // // +optional DisableHTTP2 bool `json:"disableHTTP2,omitempty"` + + // EnableProxyProtocol defines if the Proxy Protocol should be enabled for all servers. + // Default is false, meaning the Proxy Protocol will be disabled. + // + // +optional + EnableProxyProtocol bool `json:"enableProxyProtocol,omitempty"` } // Telemetry specifies the OpenTelemetry configuration. diff --git a/charts/nginx-gateway-fabric/values.yaml b/charts/nginx-gateway-fabric/values.yaml index 9cfc2064b2..e964bd4ab1 100644 --- a/charts/nginx-gateway-fabric/values.yaml +++ b/charts/nginx-gateway-fabric/values.yaml @@ -93,6 +93,7 @@ nginx: {} # disableHTTP2: false # ipFamily: dual + # enableProxyProtocol: 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 73acae5e84..8688b93d9f 100644 --- a/config/crd/bases/gateway.nginx.org_nginxproxies.yaml +++ b/config/crd/bases/gateway.nginx.org_nginxproxies.yaml @@ -52,6 +52,11 @@ spec: DisableHTTP2 defines if http2 should be disabled for all servers. Default is false, meaning http2 will be enabled for all servers. type: boolean + enableProxyProtocol: + description: |- + EnableProxyProtocol defines if the Proxy Protocol should be enabled for all servers. + Default is false, meaning the Proxy Protocol will be disabled. + type: boolean ipFamily: default: dual description: |- diff --git a/deploy/crds.yaml b/deploy/crds.yaml index 547c912748..1ea9da088a 100644 --- a/deploy/crds.yaml +++ b/deploy/crds.yaml @@ -697,6 +697,11 @@ spec: DisableHTTP2 defines if http2 should be disabled for all servers. Default is false, meaning http2 will be enabled for all servers. type: boolean + enableProxyProtocol: + description: |- + EnableProxyProtocol defines if the Proxy Protocol should be enabled for all servers. + Default is false, meaning the Proxy Protocol will be disabled. + type: boolean ipFamily: default: dual description: |- diff --git a/internal/mode/static/nginx/config/http/config.go b/internal/mode/static/nginx/config/http/config.go index 9326ebb439..0f32caf5c6 100644 --- a/internal/mode/static/nginx/config/http/config.go +++ b/internal/mode/static/nginx/config/http/config.go @@ -115,6 +115,7 @@ type ProxySSLVerify struct { // ServerConfig holds configuration for an HTTP server and IP family to be used by NGINX. type ServerConfig struct { - Servers []Server - IPFamily IPFamily + Servers []Server + IPFamily IPFamily + ProxyProtocol bool } diff --git a/internal/mode/static/nginx/config/servers.go b/internal/mode/static/nginx/config/servers.go index 3aeefa47c7..cb334db61e 100644 --- a/internal/mode/static/nginx/config/servers.go +++ b/internal/mode/static/nginx/config/servers.go @@ -61,8 +61,9 @@ func executeServers(conf dataplane.Configuration) []executeResult { servers, httpMatchPairs := createServers(conf.HTTPServers, conf.SSLServers) serverConfig := http.ServerConfig{ - Servers: servers, - IPFamily: getIPFamily(conf.BaseHTTPConfig), + Servers: servers, + IPFamily: getIPFamily(conf.BaseHTTPConfig), + ProxyProtocol: conf.BaseHTTPConfig.ProxyProtocol, } serverResult := executeResult{ diff --git a/internal/mode/static/nginx/config/servers_template.go b/internal/mode/static/nginx/config/servers_template.go index 4d8196b180..cec449b41c 100644 --- a/internal/mode/static/nginx/config/servers_template.go +++ b/internal/mode/static/nginx/config/servers_template.go @@ -2,14 +2,15 @@ package config const serversTemplateText = ` js_preload_object matches from /etc/nginx/conf.d/matches.json; +{{ $proxyProtocol := "" }}{{ if $.ProxyProtocol }}{{ $proxyProtocol = " proxy_protocol" }}{{ end }} {{- range $s := .Servers -}} {{ if $s.IsDefaultSSL -}} server { {{- if $.IPFamily.IPv4 }} - listen {{ $s.Port }} ssl default_server; + listen {{ $s.Port }} ssl default_server{{ $proxyProtocol }}; {{- end }} {{- if $.IPFamily.IPv6 }} - listen [::]:{{ $s.Port }} ssl default_server; + listen [::]:{{ $s.Port }} ssl default_server{{ $proxyProtocol }}; {{- end }} ssl_reject_handshake on; @@ -17,10 +18,10 @@ server { {{- else if $s.IsDefaultHTTP }} server { {{- if $.IPFamily.IPv4 }} - listen {{ $s.Port }} default_server; + listen {{ $s.Port }} default_server{{ $proxyProtocol }}; {{- end }} {{- if $.IPFamily.IPv6 }} - listen [::]:{{ $s.Port }} default_server; + listen [::]:{{ $s.Port }} default_server{{ $proxyProtocol }}; {{- end }} default_type text/html; @@ -30,10 +31,10 @@ server { server { {{- if $s.SSL }} {{- if $.IPFamily.IPv4 }} - listen {{ $s.Port }} ssl; + listen {{ $s.Port }} ssl{{ $proxyProtocol }}; {{- end }} {{- if $.IPFamily.IPv6 }} - listen [::]:{{ $s.Port }} ssl; + listen [::]:{{ $s.Port }} ssl{{ $proxyProtocol }}; {{- end }} ssl_certificate {{ $s.SSL.Certificate }}; ssl_certificate_key {{ $s.SSL.CertificateKey }}; @@ -43,10 +44,10 @@ server { } {{- else }} {{- if $.IPFamily.IPv4 }} - listen {{ $s.Port }}; + listen {{ $s.Port }}{{ $proxyProtocol }}; {{- end }} {{- if $.IPFamily.IPv6 }} - listen [::]:{{ $s.Port }}; + listen [::]:{{ $s.Port }}{{ $proxyProtocol }}; {{- end }} {{- end }} diff --git a/internal/mode/static/nginx/config/servers_test.go b/internal/mode/static/nginx/config/servers_test.go index effb0099ab..359e0b2a69 100644 --- a/internal/mode/static/nginx/config/servers_test.go +++ b/internal/mode/static/nginx/config/servers_test.go @@ -137,7 +137,7 @@ func TestExecuteServers(t *testing.T) { } } -func TestExecuteServersForIPFamily(t *testing.T) { +func TestExecuteServerConfig(t *testing.T) { httpServers := []dataplane.VirtualServer{ { IsDefault: true, @@ -230,6 +230,30 @@ func TestExecuteServersForIPFamily(t *testing.T) { "listen [::]:8443 ssl;": 1, }, }, + { + msg: "http and ssl servers with proxy protocol enabled", + config: dataplane.Configuration{ + HTTPServers: httpServers, + SSLServers: sslServers, + BaseHTTPConfig: dataplane.BaseHTTPConfig{ + ProxyProtocol: true, + }, + }, + expectedHTTPConfig: map[string]int{ + "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, + }, + }, } for _, test := range tests { diff --git a/internal/mode/static/state/dataplane/configuration.go b/internal/mode/static/state/dataplane/configuration.go index b81dbfe7a0..54e6b52f82 100644 --- a/internal/mode/static/state/dataplane/configuration.go +++ b/internal/mode/static/state/dataplane/configuration.go @@ -678,6 +678,8 @@ func buildBaseHTTPConfig(g *graph.Graph) BaseHTTPConfig { // HTTP2 should be enabled by default HTTP2: true, IPFamily: Dual, + // EnableProxyProtocol should be disabled by default + ProxyProtocol: false, } if g.NginxProxy == nil || !g.NginxProxy.Valid { return baseConfig @@ -696,6 +698,10 @@ func buildBaseHTTPConfig(g *graph.Graph) BaseHTTPConfig { } } + if g.NginxProxy.Source.Spec.EnableProxyProtocol { + baseConfig.ProxyProtocol = true + } + return baseConfig } diff --git a/internal/mode/static/state/dataplane/configuration_test.go b/internal/mode/static/state/dataplane/configuration_test.go index 83d5873ce0..5a994a6cfb 100644 --- a/internal/mode/static/state/dataplane/configuration_test.go +++ b/internal/mode/static/state/dataplane/configuration_test.go @@ -2089,6 +2089,34 @@ 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{EnableProxyProtocol: true}, + }, + } + return g + }), + expConf: getModifiedExpectedConfiguration(func(conf Configuration) Configuration { + conf.SSLServers = []VirtualServer{} + conf.SSLKeyPairs = map[SSLKeyPairID]SSLKeyPair{} + conf.BaseHTTPConfig = BaseHTTPConfig{HTTP2: true, IPFamily: Dual, ProxyProtocol: true} + return conf + }), + msg: "NginxProxy with proxy protocol enabled", + }, } for _, test := range tests { diff --git a/internal/mode/static/state/dataplane/types.go b/internal/mode/static/state/dataplane/types.go index d342ff3b0c..d0cc188aa8 100644 --- a/internal/mode/static/state/dataplane/types.go +++ b/internal/mode/static/state/dataplane/types.go @@ -300,6 +300,8 @@ type BaseHTTPConfig struct { IPFamily IPFamilyType // HTTP2 specifies whether http2 should be enabled for all servers. HTTP2 bool + // ProxyProtocol specifies whether the Proxy Protocol should be enabled for all servers. + ProxyProtocol bool } // IPFamilyType specifies the IP family to be used by NGINX. diff --git a/site/content/how-to/monitoring/troubleshooting.md b/site/content/how-to/monitoring/troubleshooting.md index e15bee6a1c..98ff2f8b05 100644 --- a/site/content/how-to/monitoring/troubleshooting.md +++ b/site/content/how-to/monitoring/troubleshooting.md @@ -439,6 +439,20 @@ To **resolve** this, you can do one of the following: - Adjust the IPFamily of your Service to match that of the NginxProxy configuration. +##### 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 b597e1a39e..522f03da6d 100644 --- a/site/content/reference/api.md +++ b/site/content/reference/api.md @@ -339,6 +339,19 @@ bool Default is false, meaning http2 will be enabled for all servers.

+ + +enableProxyProtocol
+ +bool + + + +(Optional) +

EnableProxyProtocol defines if the Proxy Protocol should be enabled for all servers. +Default is false, meaning the Proxy Protocol will be disabled.

+ + @@ -961,6 +974,19 @@ bool Default is false, meaning http2 will be enabled for all servers.

+ + +enableProxyProtocol
+ +bool + + + +(Optional) +

EnableProxyProtocol defines if the Proxy Protocol should be enabled for all servers. +Default is false, meaning the Proxy Protocol will be disabled.

+ +

ObservabilityPolicySpec