From 10b80b09aa0c08efd231ed92b04c42d230ff8d36 Mon Sep 17 00:00:00 2001 From: Richard Hansen Date: Mon, 9 Jan 2023 17:56:41 -0500 Subject: [PATCH] feat: Use container names for upstream names to avoid repeats --- README.md | 68 +++++++++++- app/docker-entrypoint.sh | 5 + nginx.tmpl | 102 +++++++++++++++--- .../test_upstream-name/test_container-name.py | 21 ++++ .../test_container-name.yml | 42 ++++++++ 5 files changed, 222 insertions(+), 16 deletions(-) create mode 100644 test/test_upstream-name/test_container-name.py create mode 100644 test/test_upstream-name/test_container-name.yml diff --git a/README.md b/README.md index 2757c33..44a3369 100644 --- a/README.md +++ b/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 diff --git a/app/docker-entrypoint.sh b/app/docker-entrypoint.sh index 0477dd2..6674022 100755 --- a/app/docker-entrypoint.sh +++ b/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 "$@" diff --git a/nginx.tmpl b/nginx.tmpl index 5733351..2642e9a 100644 --- a/nginx.tmpl +++ b/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 + * ), 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 + * ). + * 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 + * character set of RFC 3986 (see + * ) would + * probably also work without any problems. Those characters are + * permitted in the and 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 }} - {{- if gt $nPaths 0 }} - {{- $sum := sha1 $path }} - {{- $upstream = printf "%s-%s" $upstream $sum }} + {{- $args := dict "globals" $globals "containers" $containers "host" $host }} + {{- if gt $nPaths 0 }}{{- $_ := set $args "path" $path }}{{- end }} + {{- $upstream := eval "upstream_name" $args }} + {{- 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) }} diff --git a/test/test_upstream-name/test_container-name.py b/test/test_upstream-name/test_container-name.py new file mode 100644 index 0000000..ce50b4d --- /dev/null +++ b/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" diff --git a/test/test_upstream-name/test_container-name.yml b/test/test_upstream-name/test_container-name.yml new file mode 100644 index 0000000..00461eb --- /dev/null +++ b/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