From 40e13b5f134a342a02b34f3876761e410ec11051 Mon Sep 17 00:00:00 2001 From: Jose Diaz-Gonzalez Date: Mon, 1 Jun 2026 18:44:34 -0400 Subject: [PATCH] fix: address PR review feedback on per-backend route tests T1 of every per-backend test file asserted 200 with the api Procfile body for a request that lacks --strip-prefix, but without the strip the upstream sees /api/v0/Procfile and python's http.server returns 404; the retry loop polled on status only and accepted web's catch-all 200 first. The assertion now expects 404, which is the right signal that the route reaches the api process. T3 was rewritten to follow T2's two-step pattern (404 without strip, then 200 with strip) so the route is proven applied before checking content. T4 of proxy-route-nginx.bats relaxes the WebSocket header assertions to allow more than one occurrence because the headers correctly appear in both the route location block and the existing location / block. Backticks in fn-traefik-host-rule and fn-traefik-route-labels are now triple-escaped so they survive the docker-args eval path that turns the labels into a docker run invocation, matching the existing convention in docker-args-process-deploy. The label-based backend test files also echo the ps:rebuild output now so future failures of that step surface in the CI logs instead of silently exiting non-zero. --- plugins/traefik-vhosts/internal-functions | 12 +++++++--- tests/unit/proxy-route-caddy.bats | 24 +++++++++++++++++++- tests/unit/proxy-route-k3s-nginx.bats | 17 +++++++++++++- tests/unit/proxy-route-k3s-traefik.bats | 17 +++++++++++++- tests/unit/proxy-route-nginx.bats | 27 ++++++++++++++++++----- tests/unit/proxy-route-openresty.bats | 24 +++++++++++++++++++- tests/unit/proxy-route-traefik.bats | 26 +++++++++++++++++++++- 7 files changed, 133 insertions(+), 14 deletions(-) diff --git a/plugins/traefik-vhosts/internal-functions b/plugins/traefik-vhosts/internal-functions index 1112e605e..8d4fa5229 100755 --- a/plugins/traefik-vhosts/internal-functions +++ b/plugins/traefik-vhosts/internal-functions @@ -23,7 +23,8 @@ fn-traefik-route-labels() { service_name="${APP}-${route_proc}-${route_slug}" path_len="${#route_path}" - local rule="PathPrefix(\`${route_path}\`)" + # Triple-backslash for the same eval-survival reason as fn-traefik-host-rule. + local rule="PathPrefix(\\\`${route_path}\\\`)" if [[ -n "$host_rule" ]]; then rule="(${host_rule}) && ${rule}" fi @@ -75,12 +76,17 @@ fn-traefik-host-rule() { return fi + # Backticks must be double-escaped so they survive the docker-args eval + # path. A single backslash leaves a bare backtick in the final string, + # which then runs as a command substitution at eval time and breaks the + # docker run invocation. Triple-backslash matches the existing emission + # convention used by docker-args-process-deploy. for domain in $(echo "$app_domains" | xargs); do if [[ -z "$result" ]]; then - result="Host(\`${domain}\`)" + result="Host(\\\`${domain}\\\`)" continue fi - result="${result} || Host(\`${domain}\`)" + result="${result} || Host(\\\`${domain}\\\`)" done echo "$result" } diff --git a/tests/unit/proxy-route-caddy.bats b/tests/unit/proxy-route-caddy.bats index a5fad8eec..609914f6a 100644 --- a/tests/unit/proxy-route-caddy.bats +++ b/tests/unit/proxy-route-caddy.bats @@ -33,9 +33,14 @@ teardown() { assert_success run /bin/bash -c "dokku ps:rebuild $TEST_APP" + echo "output: $output" + echo "status: $status" assert_success - assert_http_localhost_response_contains "http" "${TEST_APP}.${DOKKU_DOMAIN}" "80" "/api/v0/Procfile" "python3 -m http.server" + # Without --strip-prefix, /api/v0/Procfile reaches api as /api/v0/Procfile + # which python's http.server returns 404 for. A 404 here proves the route + # reaches api - web's catch-all would have returned 200. + assert_http_localhost_response_contains "http" "${TEST_APP}.${DOKKU_DOMAIN}" "80" "/api/v0/Procfile" "" "404" } @test "(proxy-route:caddy) --strip-prefix toggles upstream path visibility" { @@ -48,12 +53,16 @@ teardown() { run /bin/bash -c "dokku proxy:route:set $TEST_APP api /api/v0 --port 5001" assert_success run /bin/bash -c "dokku ps:rebuild $TEST_APP" + echo "output: $output" + echo "status: $status" assert_success assert_http_localhost_response_contains "http" "${TEST_APP}.${DOKKU_DOMAIN}" "80" "/api/v0/Procfile" "" "404" run /bin/bash -c "dokku proxy:route:set $TEST_APP api /api/v0 --port 5001 --strip-prefix" assert_success run /bin/bash -c "dokku ps:rebuild $TEST_APP" + echo "output: $output" + echo "status: $status" assert_success assert_http_localhost_response_contains "http" "${TEST_APP}.${DOKKU_DOMAIN}" "80" "/api/v0/Procfile" "python3 -m http.server" } @@ -65,15 +74,28 @@ teardown() { run /bin/bash -c "dokku ps:scale $TEST_APP web=1 api=1" assert_success + # Set without --strip-prefix first; 404 proves the labels attached. + run /bin/bash -c "dokku proxy:route:set $TEST_APP api /api/v0 --port 5001" + assert_success + run /bin/bash -c "dokku ps:rebuild $TEST_APP" + echo "output: $output" + echo "status: $status" + assert_success + assert_http_localhost_response_contains "http" "${TEST_APP}.${DOKKU_DOMAIN}" "80" "/api/v0/Procfile" "" "404" + run /bin/bash -c "dokku proxy:route:set $TEST_APP api /api/v0 --port 5001 --strip-prefix" assert_success run /bin/bash -c "dokku ps:rebuild $TEST_APP" + echo "output: $output" + echo "status: $status" assert_success assert_http_localhost_response_contains "http" "${TEST_APP}.${DOKKU_DOMAIN}" "80" "/api/v0/Procfile" "python3 -m http.server" run /bin/bash -c "dokku proxy:route:remove $TEST_APP /api/v0" assert_success run /bin/bash -c "dokku ps:rebuild $TEST_APP" + echo "output: $output" + echo "status: $status" assert_success run curl --connect-to "${TEST_APP}.${DOKKU_DOMAIN}:80:localhost:80" -kSso /tmp/route-removed-body "http://${TEST_APP}.${DOKKU_DOMAIN}:80/api/v0/Procfile" diff --git a/tests/unit/proxy-route-k3s-nginx.bats b/tests/unit/proxy-route-k3s-nginx.bats index f6200babb..6dc750c88 100644 --- a/tests/unit/proxy-route-k3s-nginx.bats +++ b/tests/unit/proxy-route-k3s-nginx.bats @@ -40,7 +40,10 @@ teardown() { echo "status: $status" assert_success - assert_http_localhost_response_contains "http" "${TEST_APP}.${DOKKU_DOMAIN}" "80" "/api/v0/Procfile" "python3 -m http.server" + # Without --strip-prefix, /api/v0/Procfile reaches api as /api/v0/Procfile + # which python's http.server returns 404 for. 404 proves the route reaches + # api (web's catch-all would have returned 200). + assert_http_localhost_response_contains "http" "${TEST_APP}.${DOKKU_DOMAIN}" "80" "/api/v0/Procfile" "" "404" } @test "(proxy-route:k3s-nginx) --strip-prefix renders rewrite-target Ingress and strips upstream path" { @@ -59,6 +62,12 @@ teardown() { run /bin/bash -c "dokku ps:scale $TEST_APP web=1 api=1" assert_success + # Set without strip first; the 404 proves the route reached api before we + # assert the strip-prefix HTTP behavior. + run /bin/bash -c "dokku proxy:route:set $TEST_APP api /api/v0 --port 5001" + assert_success + assert_http_localhost_response_contains "http" "${TEST_APP}.${DOKKU_DOMAIN}" "80" "/api/v0/Procfile" "" "404" + run /bin/bash -c "dokku proxy:route:set $TEST_APP api /api/v0 --port 5001 --strip-prefix" assert_success @@ -89,6 +98,12 @@ teardown() { run /bin/bash -c "dokku ps:scale $TEST_APP web=1 api=1" assert_success + # Verify the route reaches api first (404 proves routing) before checking + # that strip applied (200 with Procfile content). + run /bin/bash -c "dokku proxy:route:set $TEST_APP api /api/v0 --port 5001" + assert_success + assert_http_localhost_response_contains "http" "${TEST_APP}.${DOKKU_DOMAIN}" "80" "/api/v0/Procfile" "" "404" + run /bin/bash -c "dokku proxy:route:set $TEST_APP api /api/v0 --port 5001 --strip-prefix" assert_success assert_http_localhost_response_contains "http" "${TEST_APP}.${DOKKU_DOMAIN}" "80" "/api/v0/Procfile" "python3 -m http.server" diff --git a/tests/unit/proxy-route-k3s-traefik.bats b/tests/unit/proxy-route-k3s-traefik.bats index 3b20847c3..76188dbbe 100644 --- a/tests/unit/proxy-route-k3s-traefik.bats +++ b/tests/unit/proxy-route-k3s-traefik.bats @@ -40,7 +40,10 @@ teardown() { echo "status: $status" assert_success - assert_http_localhost_response_contains "http" "${TEST_APP}.${DOKKU_DOMAIN}" "80" "/api/v0/Procfile" "python3 -m http.server" + # Without --strip-prefix, /api/v0/Procfile reaches api as /api/v0/Procfile + # which python's http.server returns 404 for. 404 proves the route reaches + # api (web's catch-all would have returned 200). + assert_http_localhost_response_contains "http" "${TEST_APP}.${DOKKU_DOMAIN}" "80" "/api/v0/Procfile" "" "404" } @test "(proxy-route:k3s-traefik) --strip-prefix renders stripPrefix Middleware and strips upstream path" { @@ -59,6 +62,12 @@ teardown() { run /bin/bash -c "dokku ps:scale $TEST_APP web=1 api=1" assert_success + # Set without strip first; the 404 proves the route reached api before we + # assert the strip-prefix HTTP behavior. + run /bin/bash -c "dokku proxy:route:set $TEST_APP api /api/v0 --port 5001" + assert_success + assert_http_localhost_response_contains "http" "${TEST_APP}.${DOKKU_DOMAIN}" "80" "/api/v0/Procfile" "" "404" + run /bin/bash -c "dokku proxy:route:set $TEST_APP api /api/v0 --port 5001 --strip-prefix" assert_success @@ -89,6 +98,12 @@ teardown() { run /bin/bash -c "dokku ps:scale $TEST_APP web=1 api=1" assert_success + # Verify the route reaches api first (404 proves routing) before checking + # that strip applied (200 with Procfile content). + run /bin/bash -c "dokku proxy:route:set $TEST_APP api /api/v0 --port 5001" + assert_success + assert_http_localhost_response_contains "http" "${TEST_APP}.${DOKKU_DOMAIN}" "80" "/api/v0/Procfile" "" "404" + run /bin/bash -c "dokku proxy:route:set $TEST_APP api /api/v0 --port 5001 --strip-prefix" assert_success assert_http_localhost_response_contains "http" "${TEST_APP}.${DOKKU_DOMAIN}" "80" "/api/v0/Procfile" "python3 -m http.server" diff --git a/tests/unit/proxy-route-nginx.bats b/tests/unit/proxy-route-nginx.bats index 1c9846a3e..b8961358e 100644 --- a/tests/unit/proxy-route-nginx.bats +++ b/tests/unit/proxy-route-nginx.bats @@ -29,7 +29,11 @@ teardown() { echo "status: $status" assert_success - assert_http_localhost_response_contains "http" "${TEST_APP}.${DOKKU_DOMAIN}" "80" "/api/v0/Procfile" "python3 -m http.server" + # Without --strip-prefix, /api/v0/Procfile reaches api as /api/v0/Procfile. + # api (python's http.server) does not have that file on disk and returns + # 404. A 404 here proves the route reaches api - web's catch-all would + # have returned 200 instead. + assert_http_localhost_response_contains "http" "${TEST_APP}.${DOKKU_DOMAIN}" "80" "/api/v0/Procfile" "" "404" } @test "(proxy-route:nginx) --strip-prefix toggles upstream path visibility" { @@ -65,6 +69,14 @@ teardown() { run /bin/bash -c "dokku ps:scale $TEST_APP web=1 api=1" assert_success + # First set the route without --strip-prefix so a 404 from api uniquely + # identifies that the route reached api (avoids racing with web's catch-all + # 200 during the brief window before nginx fully reloads). + run /bin/bash -c "dokku proxy:route:set $TEST_APP api /api/v0 --port 5001" + assert_success + assert_http_localhost_response_contains "http" "${TEST_APP}.${DOKKU_DOMAIN}" "80" "/api/v0/Procfile" "" "404" + + # Now switch the route to --strip-prefix and assert api serves the Procfile. run /bin/bash -c "dokku proxy:route:set $TEST_APP api /api/v0 --port 5001 --strip-prefix" assert_success assert_http_localhost_response_contains "http" "${TEST_APP}.${DOKKU_DOMAIN}" "80" "/api/v0/Procfile" "python3 -m http.server" @@ -72,8 +84,8 @@ teardown() { run /bin/bash -c "dokku proxy:route:remove $TEST_APP /api/v0" assert_success - # Web does not serve /api/v0/Procfile; we just assert it is no longer the - # api-served Procfile content. + # After removal, /api/v0/Procfile no longer reaches api - it falls through + # to web's catch-all. Assert the response is no longer api's Procfile body. run curl --connect-to "${TEST_APP}.${DOKKU_DOMAIN}:80:localhost:80" -kSso /tmp/route-removed-body "http://${TEST_APP}.${DOKKU_DOMAIN}:80/api/v0/Procfile" run /bin/bash -c "grep -c 'python3 -m http.server' /tmp/route-removed-body || true" assert_output "0" @@ -92,9 +104,12 @@ teardown() { echo "status: $status" assert_success assert_output_contains "location /api/v0" - assert_output_contains "proxy_http_version 1.1" - assert_output_contains "proxy_set_header Upgrade \$http_upgrade" - assert_output_contains "proxy_set_header Connection \$http_connection" + # These headers appear in both the route's location block AND the existing + # location / block, so allow >= 1 occurrence (count = -1) rather than the + # default exact-count-of-1. + assert_output_contains "proxy_http_version 1.1" -1 + assert_output_contains "proxy_set_header Upgrade \$http_upgrade" -1 + assert_output_contains "proxy_set_header Connection \$http_connection" -1 } add_api_process_callback() { diff --git a/tests/unit/proxy-route-openresty.bats b/tests/unit/proxy-route-openresty.bats index b55ba58cc..03ab980cb 100644 --- a/tests/unit/proxy-route-openresty.bats +++ b/tests/unit/proxy-route-openresty.bats @@ -33,9 +33,14 @@ teardown() { assert_success run /bin/bash -c "dokku ps:rebuild $TEST_APP" + echo "output: $output" + echo "status: $status" assert_success - assert_http_localhost_response_contains "http" "${TEST_APP}.${DOKKU_DOMAIN}" "80" "/api/v0/Procfile" "python3 -m http.server" + # Without --strip-prefix, /api/v0/Procfile reaches api as /api/v0/Procfile + # which python's http.server returns 404 for. A 404 here proves the route + # reaches api - web's catch-all would have returned 200. + assert_http_localhost_response_contains "http" "${TEST_APP}.${DOKKU_DOMAIN}" "80" "/api/v0/Procfile" "" "404" } @test "(proxy-route:openresty) --strip-prefix toggles upstream path visibility" { @@ -48,12 +53,16 @@ teardown() { run /bin/bash -c "dokku proxy:route:set $TEST_APP api /api/v0 --port 5001" assert_success run /bin/bash -c "dokku ps:rebuild $TEST_APP" + echo "output: $output" + echo "status: $status" assert_success assert_http_localhost_response_contains "http" "${TEST_APP}.${DOKKU_DOMAIN}" "80" "/api/v0/Procfile" "" "404" run /bin/bash -c "dokku proxy:route:set $TEST_APP api /api/v0 --port 5001 --strip-prefix" assert_success run /bin/bash -c "dokku ps:rebuild $TEST_APP" + echo "output: $output" + echo "status: $status" assert_success assert_http_localhost_response_contains "http" "${TEST_APP}.${DOKKU_DOMAIN}" "80" "/api/v0/Procfile" "python3 -m http.server" } @@ -65,15 +74,28 @@ teardown() { run /bin/bash -c "dokku ps:scale $TEST_APP web=1 api=1" assert_success + # Set without --strip-prefix first; 404 proves the labels attached. + run /bin/bash -c "dokku proxy:route:set $TEST_APP api /api/v0 --port 5001" + assert_success + run /bin/bash -c "dokku ps:rebuild $TEST_APP" + echo "output: $output" + echo "status: $status" + assert_success + assert_http_localhost_response_contains "http" "${TEST_APP}.${DOKKU_DOMAIN}" "80" "/api/v0/Procfile" "" "404" + run /bin/bash -c "dokku proxy:route:set $TEST_APP api /api/v0 --port 5001 --strip-prefix" assert_success run /bin/bash -c "dokku ps:rebuild $TEST_APP" + echo "output: $output" + echo "status: $status" assert_success assert_http_localhost_response_contains "http" "${TEST_APP}.${DOKKU_DOMAIN}" "80" "/api/v0/Procfile" "python3 -m http.server" run /bin/bash -c "dokku proxy:route:remove $TEST_APP /api/v0" assert_success run /bin/bash -c "dokku ps:rebuild $TEST_APP" + echo "output: $output" + echo "status: $status" assert_success run curl --connect-to "${TEST_APP}.${DOKKU_DOMAIN}:80:localhost:80" -kSso /tmp/route-removed-body "http://${TEST_APP}.${DOKKU_DOMAIN}:80/api/v0/Procfile" diff --git a/tests/unit/proxy-route-traefik.bats b/tests/unit/proxy-route-traefik.bats index 9007e28a3..25abd2fdd 100644 --- a/tests/unit/proxy-route-traefik.bats +++ b/tests/unit/proxy-route-traefik.bats @@ -35,9 +35,14 @@ teardown() { # Routes are applied via container labels - rebuild so the api container # restarts with the new labels attached. run /bin/bash -c "dokku ps:rebuild $TEST_APP" + echo "output: $output" + echo "status: $status" assert_success - assert_http_localhost_response_contains "http" "${TEST_APP}.${DOKKU_DOMAIN}" "80" "/api/v0/Procfile" "python3 -m http.server" + # Without --strip-prefix, /api/v0/Procfile reaches api as /api/v0/Procfile + # which python's http.server returns 404 for. A 404 here proves the route + # reaches api - web's catch-all would have returned 200. + assert_http_localhost_response_contains "http" "${TEST_APP}.${DOKKU_DOMAIN}" "80" "/api/v0/Procfile" "" "404" } @test "(proxy-route:traefik) --strip-prefix toggles upstream path visibility" { @@ -50,12 +55,16 @@ teardown() { run /bin/bash -c "dokku proxy:route:set $TEST_APP api /api/v0 --port 5001" assert_success run /bin/bash -c "dokku ps:rebuild $TEST_APP" + echo "output: $output" + echo "status: $status" assert_success assert_http_localhost_response_contains "http" "${TEST_APP}.${DOKKU_DOMAIN}" "80" "/api/v0/Procfile" "" "404" run /bin/bash -c "dokku proxy:route:set $TEST_APP api /api/v0 --port 5001 --strip-prefix" assert_success run /bin/bash -c "dokku ps:rebuild $TEST_APP" + echo "output: $output" + echo "status: $status" assert_success assert_http_localhost_response_contains "http" "${TEST_APP}.${DOKKU_DOMAIN}" "80" "/api/v0/Procfile" "python3 -m http.server" } @@ -67,15 +76,30 @@ teardown() { run /bin/bash -c "dokku ps:scale $TEST_APP web=1 api=1" assert_success + # Set the route without --strip-prefix first; a 404 from api proves the + # labels attached and traefik is routing correctly. + run /bin/bash -c "dokku proxy:route:set $TEST_APP api /api/v0 --port 5001" + assert_success + run /bin/bash -c "dokku ps:rebuild $TEST_APP" + echo "output: $output" + echo "status: $status" + assert_success + assert_http_localhost_response_contains "http" "${TEST_APP}.${DOKKU_DOMAIN}" "80" "/api/v0/Procfile" "" "404" + + # Switch to --strip-prefix and verify api now serves the Procfile. run /bin/bash -c "dokku proxy:route:set $TEST_APP api /api/v0 --port 5001 --strip-prefix" assert_success run /bin/bash -c "dokku ps:rebuild $TEST_APP" + echo "output: $output" + echo "status: $status" assert_success assert_http_localhost_response_contains "http" "${TEST_APP}.${DOKKU_DOMAIN}" "80" "/api/v0/Procfile" "python3 -m http.server" run /bin/bash -c "dokku proxy:route:remove $TEST_APP /api/v0" assert_success run /bin/bash -c "dokku ps:rebuild $TEST_APP" + echo "output: $output" + echo "status: $status" assert_success run curl --connect-to "${TEST_APP}.${DOKKU_DOMAIN}:80:localhost:80" -kSso /tmp/route-removed-body "http://${TEST_APP}.${DOKKU_DOMAIN}:80/api/v0/Procfile"