Explorar el Código

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 <nicolas.duchon@gmail.com>
Richard Hansen hace 2 años
padre
commit
8fbc8514ef

+ 4 - 3
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 <nginx-proxy-instance> cat /etc/nginx/conf.d/default.conf
+docker exec <nginx-proxy-instance> 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

+ 2 - 7
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) "" }}

+ 1 - 3
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
+  - ./tmp_certs:/etc/nginx/certs:ro

+ 0 - 12
test/test_debug/test_proxy-debug-flag.py

@@ -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

+ 0 - 26
test/test_debug/test_proxy-debug-flag.yml

@@ -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"

+ 0 - 8
test/test_debug/test_server-debug-flag.py

@@ -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

+ 0 - 25
test/test_debug/test_server-debug-flag.yml

@@ -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