feat(conformance): support GatewayPort8080 - #79
Merged
Conversation
Declare features.SupportGatewayPort8080. The multi-listener architecture already maps listener ports straight through to Service and container ports, so no operator change is needed. Enabling the feature also un-skips HTTPRouteRedirectPortAndScheme, which surfaced a redirect bug: a RequestRedirect filter with no scheme and no port built the Location port from the client's Host header rather than the Gateway listener port. A portless Host against a listener on 8080 redirected to port 80. The listener socket name is authoritative, per spec; fall back to the Host header only for sockets that carry no port (e.g. ghost-reload). Closes varnish#30
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #79 +/- ##
==========================================
+ Coverage 71.87% 71.91% +0.04%
==========================================
Files 41 41
Lines 6716 6716
==========================================
+ Hits 4827 4830 +3
+ Misses 1545 1543 -2
+ Partials 344 343 -1 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
The pool was built from the subnet's first two octets with a hardcoded .255.x range, assuming the /16 kind usually creates. OrbStack hands kind a /24, so the pool landed off-network and every LoadBalancer IP was unroutable from the host — all traffic-based conformance tests timed out locally. Derive the third octet from the prefix length instead: .255 on a /16, so the pool stays clear of Docker IPAM (which allocates upward from the bottom), and the subnet's own third octet on anything smaller.
KealanAU
force-pushed
the
feat/gateway-port-8080
branch
from
July 28, 2026 09:16
28ea8eb to
d8040d3
Compare
The redirect path added a second private strip_port alongside the bracket-aware one in director.rs, and the two disagreed: on a malformed `[2001:db8::1]:foo` Host the new one returned the whole string, which would have landed a stray `:foo` inside the Location header. Make director::strip_port pub(crate) and import it, so the tested implementation is the only one. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
test_listener_port covers the helper, but nothing exercised the redirect itself: varnishtest names its socket "a0", so every assertion in test_request_redirect.vtc went through the 80/443 fallback and the new code path only ran under the out-of-tree conformance suite. Name the VTC sockets the way the chaperone does and connect to each explicitly. The ports live in the socket names only — varnishtest binds each to a random port, which is the point: nothing reads the real bound port. Each case sends a Host with a conflicting port to prove it is ignored. Verified as a regression guard: hardcoding the port back to the Host value fails c_http8080. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
KealanAU
force-pushed
the
feat/gateway-port-8080
branch
from
July 28, 2026 10:18
232a322 to
9f13b14
Compare
The VTC in 9f13b14 is the end-to-end guard, but VTC tests only run in release mode, so a debug 'cargo test --lib' had no coverage of a redirect on a listener that isn't on 80/443. Add the build_location case, plus the two that must not change with it: an explicit filter port still wins, and a scheme change still resets to that scheme's well-known port. Also note the behaviour change in the changelog — deployments that port-translate in front of the Gateway now get the listener port in a portless redirect, and must set requestRedirect.port to pin the old value.
# Conflicts: # CHANGELOG.md # ghost/src/redirect_backend.rs
The merge of varnish#80 brought test_build_location_same_scheme_uses_well_known_port, whose no-scheme case asserts exactly what test_build_location_keeps_listener_port did; its other sub-cases repeated test_build_location_basic and test_build_location_default_ports. The vtc keeps http-8080 and https-8443 as the end-to-end proof that scheme and port derive from the socket name; the default-port-omission and portless-socket clients re-ran pure functions already unit-tested (test_should_omit_port, test_listener_port).
KealanAU
marked this pull request as ready for review
July 28, 2026 11:40
Contributor
|
Thanks! |
Contributor
Author
|
Tack, Appreciate the review and merge! |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #30.
The operator change matched the issue's prediction—a one-line feature declaration—but enabling it exposed an unrelated redirect bug.
features.SupportGatewayPort8080. No operator changes were required because listener ports already propagate to the Service and container ports.HTTPRouteRedirectPortAndScheme, which is gated on both this feature andHTTPRoutePortRedirect.RequestRedirectwith no scheme or port derived the redirect port from the client'sHostheader. For example,curl -H 'Host: example.org' http://gw:8080/redirected tohttp://example.org/.http-8080), matching the existing scheme behavior.RedirectConfig.original_scheme/original_porttolistener_scheme/listener_portto reflect their actual purpose: the values come from the listener, not the request.mainafter fix(ghost): use well-known redirect port when scheme is unchanged #80 landed the complementary spec fix (a non-empty redirect scheme always takes its well-known port). The resolution keeps fix(ghost): use well-known redirect port when scheme is unchanged #80's semantics verbatim on the renamed fields:filter.port→well_known_port(scheme)→listener_port, in the spec's own order.kind-metallb.shto work on OrbStack. It assumed Kind's usual/16network, but OrbStack uses/24, causing the address pool to be off-network and all traffic conformance tests to time out.Behavior change: Deployments that perform port translation in front of the Gateway now use the listener port for redirects when no explicit redirect port is configured. To preserve the previous behavior, set
requestRedirect.port.Tests:
test_redirect_listener_port.vtcproves scheme and port derive from the socket name end-to-end (http-8080,https-8443). Unit coverage for the port rules is #80'stest_build_location_same_scheme_uses_well_known_portplus the pre-existingbuild_locationtests.cargo test --release: 97 passed, 0 failed. Full conformance suite re-run post-merge: PASS (make test-conformance).