Merge 10b80b09aa into 0d9cd2b45c

README.md

@@ -587,11 +587,73 @@ location / {
 #### Per-VIRTUAL_HOST `server_tokens` configuration
 Per virtual-host `servers_tokens` directive can be configured by passing appropriate value to the `SERVER_TOKENS` environment variable. Please see the [nginx http_core module configuration](https://nginx.org/en/docs/http/ngx_http_core_module.html#server_tokens) for more details.
 
-### Unhashed vs SHA1 upstream names
+### Upstream name style
 
-By default the nginx configuration `upstream` blocks will use this block's corresponding hostname as a predictable name. However, this can cause issues in some setups (see [this issue](https://github.com/nginx-proxy/nginx-proxy/issues/1162)). In those cases you might want to switch to SHA1 names for the `upstream` blocks by setting the `SHA1_UPSTREAM_NAME` environment variable to `true` on the nginx-proxy container.
+The generated nginx config will have one or more `upstream` blocks, each of which defines a [server group](https://nginx.org/en/docs/http/ngx_http_upstream_module.html) for the applicable containers. The name of the upstream is computed in one of three different ways (or "styles"), described below. The desired style is selected by setting the `UPSTREAM_NAME_STYLE` environment variable on the nginx-proxy container.
 
-Please note that using regular expressions in `VIRTUAL_HOST` will always result in a corresponding `upstream` block with an SHA1 name.
+#### `UPSTREAM_NAME_STYLE: container-name`
+
+> **Warning**
+> This feature is experimental.  The behavior may change (or the feature may be removed) without warning in a future release, even if the release is not a new major version.  If you use this feature, please provide feedback so we know whether it is working properly and can be promoted to officially supported.
+
+This is the recommended style, but it is not the default for legacy compatibility reasons.
+
+When this style is selected, the upstream name is computed by sorting the names of the applicable Docker containers, appending each one with a tilde character (`~`), and concatenating the results. For example, if containers named `foo` and `bar` are used for the upstream, the upstream name is `bar~foo~`.
+
+#### `UPSTREAM_NAME_STYLE: virtual-host`
+
+This style is deprecated; you are encouraged to use `container-name` instead. This is the default style for legacy compatibility reasons.
+
+When this style is selected, the upstream name is the name of the virtual host. If a container is used in multiple virtual hosts (e.g., `VIRTUAL_HOST: foo.example,bar.example`) then multiple identical upstreams are defined for the same container.
+
+If `VIRTUAL_PATH` is used for the virtual host, the virtual path in effect is hashed via SHA1 and appended to the upstream name.
+
+If the virtual host name is a regular expression (begins with `~`), the style is automatically switched to `virtual-host-sha1`.
+
+#### `UPSTREAM_NAME_STYLE: virtual-host-sha1`
+
+This style is deprecated; you are encouraged to use `container-name` instead.
+
+This style can also be selected by leaving `UPSTREAM_NAME_STYLE` unset and setting `SHA1_UPSTREAM_NAME` to `true`.
+
+This style is identical to `virtual-host`, except the hostname is first hashed via SHA1.
+
+#### Upstream name style example
+
+```yaml
+version: "3.8"
+services:
+  nginx-proxy:
+    image: nginxproxy/nginx-proxy
+    volumes:
+      - /var/run/docker.sock:/tmp/docker.sock:ro
+    environment:
+      UPSTREAM_NAME_STYLE: container-name
+  whoami1:
+    container_name: whoami1
+    image: jwilder/whoami
+    environment:
+      VIRTUAL_HOST: whoami.example.net,whoami.example.com
+      VIRTUAL_PATH: /whoami
+      VIRTUAL_DEST: /
+  whoami2:
+    container_name: whoami2
+    image: jwilder/whoami
+    environment:
+      VIRTUAL_HOST: whoami.example.net,whoami.example.com
+      VIRTUAL_PATH: /whoami
+      VIRTUAL_DEST: /
+```
+
+With `UPSTREAM_NAME_STYLE` set to `container-name` as shown in the example above, one upstream is defined named `whoami1~whoami2~` for serving both `whoami.example.net/whoami` and `whoami.example.com/whoami`.
+
+With `UPSTREAM_NAME_STYLE` set to `virtual-host`, two upstreams are defined:
+  * An upstream named `whoami.example.net-e36977100088be78b338862f2a84ebc1ce959fef` that is used for serving `whoami.example.net/whoami`.
+  * An upstream named `whoami.example.com-e36977100088be78b338862f2a84ebc1ce959fef` that is used for serving `whoami.example.com/whoami`. (This upstream is identical to the other upstream.)
+
+With `UPSTREAM_NAME_STYLE` set to `virtual-host-sha1` two upstreams are defined:
+  * An upstream named `41d3ec6bf8437c9bb49fd0a1fe07275a6d1d714c-e36977100088be78b338862f2a84ebc1ce959fef` that is used for serving `whoami.example.net/whoami`.
+  * An upstream named `19cc2f29cfd8c0c5fda00d8312f24aad3c729c61-e36977100088be78b338862f2a84ebc1ce959fef` that is used for serving `whoami.example.com/whoami`. (This upstream is identical to the other upstream.)
 
 ### Troubleshooting
 

app/docker-entrypoint.sh

@@ -116,6 +116,11 @@ if [[ $* == 'forego start -r' ]]; then
 			Warning: The default value of TRUST_DOWNSTREAM_PROXY might change to "false" in a future version of nginx-proxy. If you require TRUST_DOWNSTREAM_PROXY to be enabled, explicitly set it to "true".
 		EOT
 	fi
+	if [ "${UPSTREAM_NAME_STYLE}" != "container-name" ]; then
+		cat >&2 <<-EOT
+			Warning: UPSTREAM_NAME_STYLE values other than container-name are deprecated.
+		EOT
+	fi
 fi
 
 exec "$@"

nginx.tmpl

@@ -14,12 +14,13 @@
 {{- $_ := set $globals "default_cert_ok" (and (exists "/etc/nginx/certs/default.crt") (exists "/etc/nginx/certs/default.key")) }}
 {{- $_ := set $globals "external_http_port" (coalesce $globals.Env.HTTP_PORT "80") }}
 {{- $_ := set $globals "external_https_port" (coalesce $globals.Env.HTTPS_PORT "443") }}
-{{- $_ := set $globals "sha1_upstream_name" (parseBool (coalesce $globals.Env.SHA1_UPSTREAM_NAME "false")) }}
 {{- $_ := set $globals "default_root_response" (coalesce $globals.Env.DEFAULT_ROOT "404") }}
 {{- $_ := set $globals "trust_downstream_proxy" (parseBool (coalesce $globals.Env.TRUST_DOWNSTREAM_PROXY "true")) }}
 {{- $_ := set $globals "access_log" (or (and (not $globals.Env.DISABLE_ACCESS_LOGS) "access_log /var/log/nginx/access.log vhost;") "") }}
 {{- $_ := set $globals "enable_ipv6" (parseBool (coalesce $globals.Env.ENABLE_IPV6 "false")) }}
 {{- $_ := set $globals "ssl_policy" (or ($globals.Env.SSL_POLICY) "Mozilla-Intermediate") }}
+{{- $_ := set $globals "upstream_name_style" (coalesce $globals.Env.UPSTREAM_NAME_STYLE (when (parseBool (coalesce $globals.Env.SHA1_UPSTREAM_NAME "false")) "virtual-host-sha1" "virtual-host")) }}
+{{- $_ := set $globals "upstream_names" (dict) }}
 {{- $_ := set $globals "vhosts" (dict) }}
 {{- $_ := set $globals "networks" (dict) }}
 # Networks available to the container running docker-gen (which are assumed to
@@ -121,6 +122,82 @@
     {{- $_ := set $ "port" $port }}
 {{- end }}
 
+{{- /*
+     * Template used as a function to compute an upstream name.  This template
+     * outputs the name of the upstream, so this template should be used with
+     * `eval`.
+     *
+     * The dot dict is expected to have the following entries:
+     *   - "globals": Global values.
+     *   - "containers": An array or slice of RuntimeContainer structs.
+     *   - "host": The virtual hostname the upstream will be served from.
+     *   - "path": The virtual path the upstream will be served from, or unset
+     *     if the VIRTUAL_PATH environment variable is not used in any of the
+     *     containers.
+     */}}
+{{- define "upstream_name" }}
+    {{- $style := $.globals.upstream_name_style }}
+    {{- if and (eq $style "virtual-host") (hasPrefix "~" $.host) }}
+        {{- $style = "virtual-host-sha1" }}
+    {{- end }}
+    {{- $name := "" }}
+    {{- if eq $style "container-name" }}
+        {{- /*
+             * Create the upstream name by joining the sorted container names.
+             *
+             * The container names are sorted to avoid redundant upstreams and
+             * for consistency in case the user provides a `*_location_override`
+             * file or similar.
+             *
+             * A tilde ("~") character is inserted after each container name to
+             * avoid name collisions.  Apparently nginx supports a wide range of
+             * characters in an upstream name (see
+             * <https://stackoverflow.com/q/36485834>), but as far as I can
+             * tell, the exact set of allowed characters is not officially
+             * specified in any of nginx's documentation.  To be safe, the tilde
+             * character is used because it is in the URI unreserved character
+             * set (percent-encoding is never required), and it is not permitted
+             * in Docker container names (according to
+             * <https://github.com/moby/moby/blob/v20.10.14/daemon/names/names.go>).
+             * Thus, joining multiple container names with "~" should yield a
+             * name that is both a valid nginx upstream name and a name that can
+             * never collide with another upstream name.  Futhermore, the name
+             * is unlikely to collide with a DNS hostname.  (Characters in the
+             * <sub-delims> character set of RFC 3986 (see
+             * <https://www.rfc-editor.org/rfc/rfc3986.html#section-2.2>) would
+             * probably also work without any problems.  Those characters are
+             * permitted in the <userinfo> and <host> parts of a URL without
+             * percent-encoding, and they are not permitted to be in Docker
+             * container names.)
+             *
+             * nginx-proxy currently does not support more than one upstream
+             * server per container, so the set of container names are
+             * sufficient to completely identify an upstream.  If support for
+             * multiple proxied servers in a single container is added,
+             * additional disambiguating information such as protocol name and
+             * port number will need to be included in the generated upstream
+             * name.  (To remain backwards compatible with the current scheme,
+             * the additional information must not be included if nginx-proxy
+             * is only proxying one of the container's servers.)
+             */}}
+        {{- range sortAlpha (groupByKeys $.containers "Name") }}
+            {{- $name = printf "%s%s~" $name . }}
+        {{- end }}
+    {{- else }}
+        {{- if eq $style "virtual-host" }}
+            {{- $name = $.host }}
+        {{- else if eq $style "virtual-host-sha1" }}
+            {{- $name = sha1 $.host }}
+        {{- else }}
+            {{- fail (printf "unknown or unsupported UPSTREAM_NAME_STYLE: %s" $style) }}
+        {{- end }}
+        {{- if $.path }}
+            {{- $name = printf "%s-%s" $name (sha1 $.path) }}
+        {{- end }}
+    {{- end }}
+    {{- $name }}
+{{- end }}
+
 {{- define "ssl_policy" }}
     {{- if eq .ssl_policy "Mozilla-Modern" }}
     ssl_protocols TLSv1.3;
@@ -444,9 +521,6 @@ server {
     {{- $default_server := when $vhost.default "default_server" "" }}
     {{- $https_method := $vhost.https_method }}
 
-    {{- $is_regexp := hasPrefix "~" $host }}
-    {{- $upstream_name := when (or $is_regexp $globals.sha1_upstream_name) (sha1 $host) $host }}
-
     {{- $paths := groupBy $containers "Env.VIRTUAL_PATH" }}
     {{- $nPaths := len $paths }}
     {{- if eq $nPaths 0 }}
@@ -454,13 +528,13 @@ server {
     {{- end }}
 
     {{- range $path, $containers := $paths }}
-        {{- $upstream := $upstream_name }}
+        {{- $args := dict "globals" $globals "containers" $containers "host" $host }}
-        {{- if gt $nPaths 0 }}
+        {{- if gt $nPaths 0 }}{{- $_ := set $args "path" $path }}{{- end }}
-            {{- $sum := sha1 $path }}
+        {{- $upstream := eval "upstream_name" $args }}
-            {{- $upstream = printf "%s-%s" $upstream $sum }}
+        {{- if not (index $globals.upstream_names $upstream) }}
+            {{- template "upstream" (dict "globals" $globals "Upstream" $upstream "Containers" $containers) }}
+            {{- $_ := set $globals.upstream_names $upstream true }}
         {{- end }}
-# {{ $host }}{{ $path }}
-{{ template "upstream" (dict "globals" $globals "Upstream" $upstream "Containers" $containers) }}
     {{- end }}
 
     {{- /*
@@ -609,11 +683,13 @@ server {
              * falling back to "external".
              */}}
         {{- $network_tag := or (first (groupByKeys $containers "Env.NETWORK_ACCESS")) "external" }}
-        {{- $upstream := $upstream_name }}
+
+        {{- $args := dict "globals" $globals "containers" $containers "host" $host }}
+        {{- if gt $nPaths 0 }}{{- $_ := set $args "path" $path }}{{- end }}
+        {{- $upstream := eval "upstream_name" $args }}
+
         {{- $dest := "" }}
         {{- if gt $nPaths 0 }}
-            {{- $sum := sha1 $path }}
-            {{- $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 "Containers" $containers) }}

test/test_upstream-name/test_container-name.py

@@ -0,0 +1,21 @@
+import pytest
+import re
+
+
+def test_single_container_in_upstream(docker_compose, nginxproxy):
+    conf = nginxproxy.get_conf().decode()
+    assert re.search(r"upstream web1~ \{", conf)
+
+def test_multiple_containers_in_upstream(docker_compose, nginxproxy):
+    conf = nginxproxy.get_conf().decode()
+    assert re.search(r"upstream web2~web3~ \{", conf)
+
+def test_no_redundant_upstreams(docker_compose, nginxproxy):
+    conf = nginxproxy.get_conf().decode()
+    assert len(re.findall(r"upstream web4~ \{", conf)) == 1
+
+def test_valid_upstream_name(docker_compose, nginxproxy):
+    """nginx should not choke on the tilde character."""
+    r = nginxproxy.get("http://web1.nginx-proxy.test/port")
+    assert r.status_code == 200
+    assert r.text == "answer from port 80\n"

test/test_upstream-name/test_container-name.yml

@@ -0,0 +1,42 @@
+version: '2'
+
+services:
+  web1:
+    container_name: web1
+    image: web
+    expose:
+      - "80"
+    environment:
+      WEB_PORTS: 80
+      VIRTUAL_HOST: web1.nginx-proxy.test
+  web2:
+    container_name: web2
+    image: web
+    expose:
+      - "80"
+    environment:
+      WEB_PORTS: 80
+      VIRTUAL_HOST: web23.nginx-proxy.test
+  web3:
+    container_name: web3
+    image: web
+    expose:
+      - "80"
+    environment:
+      WEB_PORTS: 80
+      VIRTUAL_HOST: web23.nginx-proxy.test
+  web4:
+    container_name: web4
+    image: web
+    expose:
+      - "80"
+    environment:
+      WEB_PORTS: 80
+      VIRTUAL_HOST: web4a.nginx-proxy.test,web4b.nginx-proxy.test
+
+  sut:
+    image: nginxproxy/nginx-proxy:test
+    volumes:
+      - /var/run/docker.sock:/tmp/docker.sock:ro
+    environment:
+      UPSTREAM_NAME_STYLE: container-name