Merge pull request #2214 from rhansen/default_server

fix: Remove `default_server` listen option from fallback server

nginx.tmpl

@@ -372,6 +372,15 @@ proxy_set_header Proxy "";
 {{- /*
      * If needed, create a catch-all fallback server to send an error code to
      * clients that request something from an unknown vhost.
+     *
+     * This server must appear first in the generated config because nginx uses
+     * the first `server` directive to handle requests that don't match any of
+     * the other `server` directives.  An alternative approach would be to add
+     * the `default_server` option to the `listen` directives inside this
+     * `server`, but some users inject a custom `server` directive that uses
+     * `default_server`.  Using `default_server` here would cause nginx to fail
+     * to start for those users.  See
+     * <https://github.com/nginx-proxy/nginx-proxy/issues/2212>.
      */}}
 {{- block "fallback_server" $globals }}
     {{- $globals := . }}
@@ -403,15 +412,15 @@ server {
     server_name _; # This is just an invalid value which will never trigger on a real hostname.
     server_tokens off;
         {{- if $fallback_http }}
-    listen {{ $globals.external_http_port }} default_server;
+    listen {{ $globals.external_http_port }}; {{- /* Do not add `default_server` (see comment above). */}}
                 {{- if $globals.enable_ipv6 }}
-    listen [::]:{{ $globals.external_http_port }} default_server;
+    listen [::]:{{ $globals.external_http_port }}; {{- /* Do not add `default_server` (see comment above). */}}
                 {{- end }}
         {{- end }}
         {{- if $fallback_https }}
-    listen {{ $globals.external_https_port }} ssl http2 default_server;
+    listen {{ $globals.external_https_port }} ssl http2; {{- /* Do not add `default_server` (see comment above). */}}
             {{- if $globals.enable_ipv6 }}
-    listen [::]:{{ $globals.external_https_port }} ssl http2 default_server;
+    listen [::]:{{ $globals.external_https_port }} ssl http2; {{- /* Do not add `default_server` (see comment above). */}}
             {{- end }}
         {{- end }}
     {{ $globals.access_log }}

test/test_fallback.data/custom-fallback.conf

@@ -0,0 +1,5 @@
+server {
+    server_name __;
+    listen 80 default_server;
+    return 418;
+}

test/test_fallback.data/custom-fallback.yml

@@ -0,0 +1,14 @@
+services:
+  sut:
+    image: nginxproxy/nginx-proxy:test
+    volumes:
+      - /var/run/docker.sock:/tmp/docker.sock:ro
+      - ./custom-fallback.conf:/etc/nginx/conf.d/zzz-custom-fallback.conf:ro
+  http-only:
+    image: web
+    expose:
+      - "83"
+    environment:
+      WEB_PORTS: "83"
+      VIRTUAL_HOST: http-only.nginx-proxy.test
+      HTTPS_METHOD: nohttps

test/test_fallback.py

@@ -89,6 +89,11 @@ CONNECTION_REFUSED_RE = re.compile("Connection refused")
     ("nohttps-on-app.yml", "https://http-only.nginx-proxy.test/", None, CONNECTION_REFUSED_RE),
     ("nohttps-on-app.yml", "http://unknown.nginx-proxy.test/", 503, None),
     ("nohttps-on-app.yml", "https://unknown.nginx-proxy.test/", None, CONNECTION_REFUSED_RE),
+    # Custom nginx config that has a `server` directive that uses `default_server` and simply
+    # returns 418.  Nginx should successfully start (in particular, the `default_server` in the
+    # custom config should not conflict with the fallback server generated by nginx-proxy) and nginx
+    # should prefer that server for handling requests for unknown vhosts.
+    ("custom-fallback.yml", "http://unknown.nginx-proxy.test/", 418, None),
 ])
 def test_fallback(get, url, want_code, want_err_re):
     if want_err_re is None: