Merge pull request #1934 from rhansen/keepalive

feat: Add support for HTTP keep-alive between the proxy and upstream

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:

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 <https://stackoverflow.com/q/14433309>) 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 / {

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)

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