From 8fbc8514ef3cd4a464c0ed7315590f8dee8b972c Mon Sep 17 00:00:00 2001 From: Richard Hansen Date: Tue, 17 Jan 2023 21:23:08 -0500 Subject: [PATCH] feat: Unconditionally produce debug comments Rationale for eliminating the check to see if the `DEBUG` environment variable holds a true value: * The `DEBUG` environment variable might be set on a container (for purposes specific to that container, not `nginx-proxy`) to a value that cannot be parsed as a bool, which would break `nginx-proxy`. * It simplifies the template. * It eliminates a cold code path. * It avoids heisenbugs. * It makes debugging easier for users. Also delete the debug info tests, as they are fragile and they provide limited value. Alternatively, we could avoid collision with the container's use of the `DEBUG` environment variable by using a container label [1] such as `com.google.nginx-proxy.nginx-proxy.debug`. I think doing so has dubious value, especially if we want to attempt backwards compatibility with the `DEBUG` environment variable. Fixes #2139 [1] https://docs.docker.com/engine/reference/commandline/run/#-set-metadata-on-container--l---label---label-file Co-authored-by: Nicolas Duchon --- README.md | 7 ++--- nginx.tmpl | 9 ++----- .../test_deleted_cert/docker-compose.yml | 4 +-- test/test_debug/test_proxy-debug-flag.py | 12 --------- test/test_debug/test_proxy-debug-flag.yml | 26 ------------------- test/test_debug/test_server-debug-flag.py | 8 ------ test/test_debug/test_server-debug-flag.yml | 25 ------------------ 7 files changed, 7 insertions(+), 84 deletions(-) delete mode 100644 test/test_debug/test_proxy-debug-flag.py delete mode 100644 test/test_debug/test_proxy-debug-flag.yml delete mode 100644 test/test_debug/test_server-debug-flag.py delete mode 100644 test/test_debug/test_server-debug-flag.yml diff --git a/README.md b/README.md index 0ca08d7..82b2dcf 100644 --- a/README.md +++ b/README.md @@ -490,12 +490,13 @@ Please note that using regular expressions in `VIRTUAL_HOST` will always result ### Troubleshooting -In case you can't access your VIRTUAL_HOST, set `DEBUG=true` in the client container's environment and have a look at the generated nginx configuration file `/etc/nginx/conf.d/default.conf`: +If you can't access your `VIRTUAL_HOST`, inspect the generated nginx configuration: ```console -docker exec cat /etc/nginx/conf.d/default.conf +docker exec nginx -T ``` -Especially at `upstream` definition blocks which should look like: + +Pay attention to the `upstream` definition blocks, which should look like this: ```Nginx # foo.example.com diff --git a/nginx.tmpl b/nginx.tmpl index fb4f76f..2abfb7c 100644 --- a/nginx.tmpl +++ b/nginx.tmpl @@ -12,7 +12,6 @@ {{- $_ := set $globals "nginx_proxy_version" (coalesce $globals.Env.NGINX_PROXY_VERSION "") }} {{- $_ := set $globals "external_http_port" (coalesce $globals.Env.HTTP_PORT "80") }} {{- $_ := set $globals "external_https_port" (coalesce $globals.Env.HTTPS_PORT "443") }} -{{- $_ := set $globals "debug_all" $globals.Env.DEBUG }} {{- $_ := 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")) }} @@ -106,22 +105,18 @@ {{- define "upstream" }} {{- $networks := .Networks }} - {{- $debug_all := .Debug }} upstream {{ .Upstream }} { {{- $server_found := false }} {{- range $container := .Containers }} - {{- $debug := parseBool (coalesce $container.Env.DEBUG $debug_all "false") }} {{- /* If only 1 port exposed, use that as a default, else 80 */}} {{- $defaultPort := (when (eq (len $container.Addresses) 1) (first $container.Addresses) (dict "Port" "80")).Port }} {{- $port := (coalesce $container.Env.VIRTUAL_PORT $defaultPort) }} {{- $address := where $container.Addresses "Port" $port | first }} - {{- if $debug }} # Exposed ports: {{ $container.Addresses }} # Default virtual port: {{ $defaultPort }} # VIRTUAL_PORT: {{ $container.Env.VIRTUAL_PORT }} - {{- if not $address }} + {{- if not $address }} # /!\ Virtual port not exposed - {{- end }} {{- end }} {{- range $knownNetwork := $networks }} {{- range $containerNetwork := sortObjectsByKeysAsc $container.Networks "Name" }} @@ -292,7 +287,7 @@ server { {{- $upstream = printf "%s-%s" $upstream $sum }} {{- end }} # {{ $host }}{{ $path }} -{{ template "upstream" (dict "Upstream" $upstream "Containers" $containers "Networks" $globals.CurrentContainer.Networks "Debug" $globals.debug_all) }} +{{ template "upstream" (dict "Upstream" $upstream "Containers" $containers "Networks" $globals.CurrentContainer.Networks) }} {{- end }} {{- $default_host := or ($globals.Env.DEFAULT_HOST) "" }} diff --git a/test/stress_tests/test_deleted_cert/docker-compose.yml b/test/stress_tests/test_deleted_cert/docker-compose.yml index 33c92a7..a42abac 100644 --- a/test/stress_tests/test_deleted_cert/docker-compose.yml +++ b/test/stress_tests/test_deleted_cert/docker-compose.yml @@ -10,8 +10,6 @@ web: reverseproxy: image: nginxproxy/nginx-proxy:test container_name: reverseproxy - environment: - DEBUG: "true" volumes: - /var/run/docker.sock:/tmp/docker.sock:ro - - ./tmp_certs:/etc/nginx/certs:ro \ No newline at end of file + - ./tmp_certs:/etc/nginx/certs:ro diff --git a/test/test_debug/test_proxy-debug-flag.py b/test/test_debug/test_proxy-debug-flag.py deleted file mode 100644 index 6430d84..0000000 --- a/test/test_debug/test_proxy-debug-flag.py +++ /dev/null @@ -1,12 +0,0 @@ -import pytest -import re - -def test_debug_info_is_present_in_nginx_generated_conf(docker_compose, nginxproxy): - conf = nginxproxy.get_conf().decode('ASCII') - assert re.search(r"# Exposed ports: \[\{[^}]+\s+80\s+tcp \} \{[^}]+\s+81\s+tcp \}\]", conf) or \ - re.search(r"# Exposed ports: \[\{[^}]+\s+81\s+tcp \} \{[^}]+\s+80\s+tcp \}\]", conf) - assert re.search(r"# Exposed ports: \[\{[^}]+\s+82\s+tcp \} \{[^}]+\s+83\s+tcp \}\]", conf) or \ - re.search(r"# Exposed ports: \[\{[^}]+\s+83\s+tcp \} \{[^}]+\s+82\s+tcp \}\]", conf) - assert "# Default virtual port: 80" in conf - assert "# VIRTUAL_PORT: 82" in conf - assert conf.count("# /!\\ Virtual port not exposed") == 1 diff --git a/test/test_debug/test_proxy-debug-flag.yml b/test/test_debug/test_proxy-debug-flag.yml deleted file mode 100644 index f930da3..0000000 --- a/test/test_debug/test_proxy-debug-flag.yml +++ /dev/null @@ -1,26 +0,0 @@ -web1: - image: web - expose: - - "80" - - "81" - environment: - WEB_PORTS: "80 81" - VIRTUAL_HOST: "web1.nginx-proxy.tld" - VIRTUAL_PORT: "82" - -web2: - image: web - expose: - - "82" - - "83" - environment: - WEB_PORTS: "82 83" - VIRTUAL_HOST: "web2.nginx-proxy.tld" - VIRTUAL_PORT: "82" - -sut: - image: nginxproxy/nginx-proxy:test - volumes: - - /var/run/docker.sock:/tmp/docker.sock:ro - environment: - DEBUG: "true" diff --git a/test/test_debug/test_server-debug-flag.py b/test/test_debug/test_server-debug-flag.py deleted file mode 100644 index c635175..0000000 --- a/test/test_debug/test_server-debug-flag.py +++ /dev/null @@ -1,8 +0,0 @@ -import pytest -import re - -def test_debug_info_is_present_in_nginx_generated_conf(docker_compose, nginxproxy): - conf = nginxproxy.get_conf().decode('ASCII') - assert re.search(r"# Exposed ports: \[\{[^}]+\s+80\s+tcp \} \{[^}]+\s+81\s+tcp \}\]", conf) or \ - re.search(r"# Exposed ports: \[\{[^}]+\s+81\s+tcp \} \{[^}]+\s+80\s+tcp \}\]", conf) - assert conf.count("# Exposed ports: [{") == 1 diff --git a/test/test_debug/test_server-debug-flag.yml b/test/test_debug/test_server-debug-flag.yml deleted file mode 100644 index 89bb6b5..0000000 --- a/test/test_debug/test_server-debug-flag.yml +++ /dev/null @@ -1,25 +0,0 @@ -web1: - image: web - expose: - - "80" - - "81" - environment: - WEB_PORTS: "80 81" - VIRTUAL_HOST: "web1.nginx-proxy.tld" - VIRTUAL_PORT: "82" - DEBUG: "true" - -web2: - image: web - expose: - - "82" - - "83" - environment: - WEB_PORTS: "82 83" - VIRTUAL_HOST: "web2.nginx-proxy.tld" - VIRTUAL_PORT: "82" - -sut: - image: nginxproxy/nginx-proxy:test - volumes: - - /var/run/docker.sock:/tmp/docker.sock:ro