Send a stable host id to the relay on host connections - #679
Send a stable host id to the relay on host connections#679Juan Pablo Acosta (jpablo2002) wants to merge 3 commits into
Conversation
… tell a host reconnecting to its own tunnel from a different host taking it over.
Matches the C# and TypeScript SDKs so the relay can identify a reconnecting host process.
|
Azure Pipelines: There may be pipelines that require an authorized user to comment /azp run to run. |
|
/azp run |
|
Azure Pipelines will not run the associated pipelines, because the pull request was updated after the run command was issued. Review the pull request again and issue a new run command. |
| if (isHostConnection && !string.IsNullOrEmpty(MultiModeTunnelHost.HostId)) | ||
| { | ||
| options.SetRequestHeader( | ||
| TunnelRelayConnection.HostIdHeaderName, MultiModeTunnelHost.HostId); |
There was a problem hiding this comment.
The endpoint and the WebSocket can end up using different host IDs here. TunnelRelayTunnelHost captures MultiModeTunnelHost.HostId in its constructor and publishes that captured value as TunnelEndpoint.HostId, while this factory rereads the publicly settable static on every connection and reconnect. If an application changes the supported HostId setting after constructing a host, the endpoint can advertise ID A while this header sends ID B; existing hosts can then be mistaken for another process, or distinct hosts can be treated as the same process, undermining the arbitration this change adds. The TypeScript implementation has the same split between its captured this.hostId and the static read in DefaultTunnelRelayStreamFactory. Can we pass the host instance’s captured ID into stream creation, or otherwise make the process ID immutable before hosts are constructed?
2 participants
Host connections now send an
X-Tunnels-Host-Process-Idrequest header identifying the host process. The relay uses it to tell a host reconnecting to a tunnel it already holds from a genuinely different host taking that tunnel over.Why
When two hosts race for the same tunnel, each connect evicts the incumbent with
TooManyConnections; the evicted host reconnects, evicts the challenger, and the loop sustains itself, tearing down every live client bridge each time.We need an identity that is stable across a host's own reconnects. Host connections authenticate as
ClientNone, so there is no SSH key on the wire, and the request correlation id is per-connection — a host redialling after sleep/wake or a dropped network would fail to recognize itself. The SDK is the only place that knows the answer, so it sends it explicitly.MultiModeTunnelHost.HostIdis minted once per process and already reported onTunnelEndpoint.HostId, so it's a proven-stable identity.What changed
TunnelRelayStreamFactory.cs) — sets the header fromMultiModeTunnelHost.HostIdwhen the sub-protocol list contains a host sub-protocol. The header name is a new public const,TunnelRelayConnection.HostIdHeaderName.defaultTunnelRelayStreamFactory.ts) — same, fromMultiModeTunnelHost.hostId, on the Node path.relay_tunnel_host.rs) — adds the header to the relay host's WebSocket request, from thehost_idthe host already generates and reports on its endpoint.Only host connections send it; clients are untouched.
Notes
TunnelHeaderNames. That file is the generated contract surface, and generating this into Go/Java would produce a constant nothing can use. It lives next to the three host implementations that actually send it.RelayTunnelHost, not per-process. It's stable across the reconnects the host performs internally, which is the case that matters. A caller that constructs a freshRelayTunnelHoston reconnect gets a new id and is arbitrated as a new host — the same as today's behavior, never worse.Compatibility
Fully backward compatible in both directions. The relay ignores the header when it's absent and treats an unidentified host exactly as it does today, so old SDKs against a new service are unchanged and a new SDK against an older service is a no-op — the header is simply dropped. It's also stripped at web forwarding, so it never reaches a customer's app.
Testing
C# builds clean and the existing suite passes; TypeScript compiles and lints clean. No new unit tests — both suites substitute a mock stream factory, so the default factory this change touches isn't reachable from them.