From 49bb37dfdb3e4222f1f303d8e2eca748d0edfcbd Mon Sep 17 00:00:00 2001 From: Richard Hansen Date: Mon, 28 Mar 2022 02:56:36 -0400 Subject: [PATCH] feat: Add support for HTTP keep-alive between the proxy and upstream --- README.md | 7 +++++++ nginx.tmpl | 39 +++++++++++++++++++++++++++++++++++---- test/test_keepalive.py | 31 +++++++++++++++++++++++++++++++ test/test_keepalive.yml | 25 +++++++++++++++++++++++++ 4 files changed, 98 insertions(+), 4 deletions(-) create mode 100644 test/test_keepalive.py create mode 100644 test/test_keepalive.yml diff --git a/README.md b/README.md index 1728467..1ff960b 100644 --- a/README.md +++ b/README.md @@ -373,6 +373,13 @@ docker run -d -p 80:80 -p 443:443 \ You'll need apache2-utils on the machine where you plan to create the htpasswd file. Follow these [instructions](http://httpd.apache.org/docs/2.2/programs/htpasswd.html) +### Upstream (Backend) Server HTTP Keep-Alive Support + +> **Warning** +> This feature is experimental. The behavior may change (or the feature may be removed entirely) without warning in a future release, even if the release is not a new major version. If you use this feature, or if you would like to use this feature but you require changes to it first, please [provide feedback in #2194](https://github.com/nginx-proxy/nginx-proxy/discussions/2194). Once we have collected enough feedback we will promote this feature to officially supported. + +To enable HTTP keep-alive between `nginx-proxy` and a backend server, set the `com.github.nginx-proxy.nginx-proxy.keepalive` label on the server's container to the desired maximum number of idle connections. See the [nginx keepalive documentation](https://nginx.org/en/docs/http/ngx_http_upstream_module.html#keepalive) and the [Docker label documentation](https://docs.docker.com/config/labels-custom-metadata/) for details. + ### Headers By default, `nginx-proxy` forwards all incoming request headers from the client to the backend server unmodified, with the following exceptions: diff --git a/nginx.tmpl b/nginx.tmpl index 6f84eb7..e35258f 100644 --- a/nginx.tmpl +++ b/nginx.tmpl @@ -176,6 +176,7 @@ {{- if exists $override }} include {{ $override }}; {{- else }} + {{- $keepalive := first (keys (groupByLabel .Containers "com.github.nginx-proxy.nginx-proxy.keepalive")) }} location {{ .Path }} { {{- if eq .NetworkTag "internal" }} # Only allow traffic from internal clients @@ -189,10 +190,14 @@ root {{ trim .VhostRoot }}; include fastcgi_params; fastcgi_pass {{ trim .Upstream }}; + {{- if $keepalive }} + fastcgi_keep_conn on; + {{- end }} {{- else if eq .Proto "grpc" }} grpc_pass {{ trim .Proto }}://{{ trim .Upstream }}; {{- else }} proxy_pass {{ trim .Proto }}://{{ trim .Upstream }}{{ trim .Dest }}; + set $upstream_keepalive {{ if $keepalive }}true{{ else }}false{{ end }}; {{- end }} {{- if (exists (printf "/etc/nginx/htpasswd/%s" .Host)) }} @@ -232,6 +237,10 @@ upstream {{ .Upstream }} { # Fallback entry server 127.0.0.1 down; {{- end }} + {{- $keepalive := first (keys (groupByLabel .Containers "com.github.nginx-proxy.nginx-proxy.keepalive")) }} + {{- if $keepalive }} + keepalive {{ $keepalive }}; + {{- end }} } {{- end }} @@ -254,11 +263,33 @@ map $http_x_forwarded_port $proxy_x_forwarded_port { '' $server_port; } -# If we receive Upgrade, set Connection to "upgrade"; otherwise, preserve -# NGINX's default behavior ("Connection: close"). +# If the request from the downstream client has an "Upgrade:" header (set to any +# non-empty value), pass "Connection: upgrade" to the upstream (backend) server. +# Otherwise, the value for the "Connection" header depends on whether the user +# has enabled keepalive to the upstream server. map $http_upgrade $proxy_connection { default upgrade; - '' close; + '' $proxy_connection_noupgrade; +} +map $upstream_keepalive $proxy_connection_noupgrade { + # Preserve nginx's default behavior (send "Connection: close"). + default close; + # Use an empty string to cancel nginx's default behavior. + true ''; +} +# Abuse the map directive (see ) to ensure +# that $upstream_keepalive is always defined. This is necessary because: +# - The $proxy_connection variable is indirectly derived from +# $upstream_keepalive, so $upstream_keepalive must be defined whenever +# $proxy_connection is resolved. +# - The $proxy_connection variable is used in a proxy_set_header directive in +# the http block, so it is always fully resolved for every request -- even +# those where proxy_pass is not used (e.g., unknown virtual host). +map "" $upstream_keepalive { + # The value here should not matter because it should always be overridden in + # a location block (see the "location" template) for all requests where the + # value actually matters. + default false; } # Apply fix for very long server names @@ -514,7 +545,7 @@ server { {{- $upstream = printf "%s-%s" $upstream $sum }} {{- $dest = (or (first (groupByKeys $containers "Env.VIRTUAL_DEST")) "") }} {{- end }} - {{- template "location" (dict "Path" $path "Proto" $proto "Upstream" $upstream "Host" $host "VhostRoot" $vhost_root "Dest" $dest "NetworkTag" $network_tag) }} + {{- template "location" (dict "Path" $path "Proto" $proto "Upstream" $upstream "Host" $host "VhostRoot" $vhost_root "Dest" $dest "NetworkTag" $network_tag "Containers" $containers) }} {{- end }} {{- if and (not (contains $paths "/")) (ne $globals.default_root_response "none")}} location / { diff --git a/test/test_keepalive.py b/test/test_keepalive.py new file mode 100644 index 0000000..b5b8353 --- /dev/null +++ b/test/test_keepalive.py @@ -0,0 +1,31 @@ +import re + + +def test_keepalive_disabled(docker_compose, nginxproxy): + r = nginxproxy.get("http://keepalive-disabled.nginx-proxy.test/headers") + assert r.status_code == 200 + assert re.search(fr'(?m)^(?i:Connection): close$', r.text) + +def test_keepalive_disabled_other_headers_ok(docker_compose, nginxproxy): + """Make sure the other proxy_set_header headers are still set. + + According to the nginx docs [1], any proxy_set_header directive in a block + disables inheritance of proxy_set_header directives in a parent block. Make + sure that doesn't happen. + + [1] https://nginx.org/en/docs/http/ngx_http_proxy_module.html#proxy_set_header + """ + r = nginxproxy.get("http://keepalive-disabled.nginx-proxy.test/headers") + assert r.status_code == 200 + assert re.search(fr'(?m)^(?i:X-Real-IP): ', r.text) + +def test_keepalive_enabled(docker_compose, nginxproxy): + r = nginxproxy.get("http://keepalive-enabled.nginx-proxy.test/headers") + assert r.status_code == 200 + assert not re.search(fr'(?m)^(?i:Connection):', r.text) + +def test_keepalive_enabled_other_headers_ok(docker_compose, nginxproxy): + """See the docstring for the disabled case above.""" + r = nginxproxy.get("http://keepalive-enabled.nginx-proxy.test/headers") + assert r.status_code == 200 + assert re.search(fr'(?m)^(?i:X-Real-IP): ', r.text) diff --git a/test/test_keepalive.yml b/test/test_keepalive.yml new file mode 100644 index 0000000..541b69d --- /dev/null +++ b/test/test_keepalive.yml @@ -0,0 +1,25 @@ +keepalive-disabled: + image: web + expose: + - "80" + environment: + WEB_PORTS: 80 + VIRTUAL_HOST: keepalive-disabled.nginx-proxy.test + +keepalive-enabled: + image: web + expose: + - "80" + environment: + WEB_PORTS: 80 + VIRTUAL_HOST: keepalive-enabled.nginx-proxy.test + labels: + com.github.nginx-proxy.nginx-proxy.keepalive: "64" + + +sut: + image: nginxproxy/nginx-proxy:test + volumes: + - /var/run/docker.sock:/tmp/docker.sock:ro + environment: + HTTPS_METHOD: nohttps