Skip to content

Commit

Permalink
feat: Unconditionally produce debug comments
Browse files Browse the repository at this point in the history
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
  • Loading branch information
rhansen committed Jan 18, 2023
1 parent 1775420 commit 4d33efc
Show file tree
Hide file tree
Showing 7 changed files with 6 additions and 83 deletions.
5 changes: 3 additions & 2 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -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 `/etc/nginx/conf.d/default.conf` file:

```console
docker exec <nginx-proxy-instance> cat /etc/nginx/conf.d/default.conf
```
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
Expand Down
9 changes: 2 additions & 7 deletions nginx.tmpl
Original file line number Diff line number Diff line change
Expand Up @@ -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")) }}
Expand Down Expand Up @@ -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" }}
Expand Down Expand Up @@ -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) "" }}
Expand Down
4 changes: 1 addition & 3 deletions test/stress_tests/test_deleted_cert/docker-compose.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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
12 changes: 0 additions & 12 deletions test/test_debug/test_proxy-debug-flag.py

This file was deleted.

26 changes: 0 additions & 26 deletions test/test_debug/test_proxy-debug-flag.yml

This file was deleted.

8 changes: 0 additions & 8 deletions test/test_debug/test_server-debug-flag.py

This file was deleted.

25 changes: 0 additions & 25 deletions test/test_debug/test_server-debug-flag.yml

This file was deleted.

0 comments on commit 4d33efc

Please sign in to comment.