Skip to content

Allow X-Stream-Protocol-Version so SPDY streaming works - #38

Open
Wouter0100 wants to merge 1 commit into
netbirdio:mainfrom
Wouter0100:fix/allow-stream-protocol-version-header
Open

Allow X-Stream-Protocol-Version so SPDY streaming works#38
Wouter0100 wants to merge 1 commit into
netbirdio:mainfrom
Wouter0100:fix/allow-stream-protocol-version-header

Conversation

@Wouter0100

@Wouter0100 Wouter0100 commented Jul 29, 2026

Copy link
Copy Markdown

The header allowlist permits the Sec-Websocket-* handshake headers but not X-Stream-Protocol-Version, so every request on the SPDY streaming transport had its protocol negotiation header stripped and was rejected by the API server with:

unable to upgrade connection: unable to upgrade:
X-Stream-Protocol-Version is required

This breaks kubectl exec, attach and cp whenever the WebSocket transport is not used: clients older than 1.30, clients with KUBECTL_REMOTE_COMMAND_WEBSOCKETS=false, and clients that fall back to SPDY because a proxy-url is configured. It also breaks kubectl port-forward against API servers without WebSocket port-forward support, since SPDY is the only transport left there.

The failure is easy to misread, because kubectl attempts WebSocket first and falls back to SPDY: the real error from the first attempt is discarded and the API server answers the SPDY request with a 400 whose body is not a Status object, which kubectl renders as a bare "Error from server:" with no message.


View with [code]smith Autofix with [code]smith
Need help on this PR? Tag @codesmith-bot with what you need. Autofix is disabled.

Summary by CodeRabbit

  • Bug Fixes
    • Preserved the stream protocol version header when proxying requests.
    • Added coverage to verify that valid stream protocol version information reaches the destination service.

The header allowlist permits the Sec-Websocket-* handshake headers but not
X-Stream-Protocol-Version, so every request on the SPDY streaming transport
had its protocol negotiation header stripped and was rejected by the API
server with:

  unable to upgrade connection: unable to upgrade:
  X-Stream-Protocol-Version is required

This breaks kubectl exec, attach and cp whenever the WebSocket transport is
not used: clients older than 1.30, clients with
KUBECTL_REMOTE_COMMAND_WEBSOCKETS=false, and clients that fall back to SPDY
because a proxy-url is configured. It also breaks kubectl port-forward
against API servers without WebSocket port-forward support, since SPDY is
the only transport left there.

The failure is easy to misread, because kubectl attempts WebSocket first and
falls back to SPDY: the real error from the first attempt is discarded and
the API server answers the SPDY request with a 400 whose body is not a
Status object, which kubectl renders as a bare "Error from server:" with no
message.
@coderabbitai

coderabbitai Bot commented Jul 29, 2026

Copy link
Copy Markdown

Review Change Stack

📝 Walkthrough

Walkthrough

The proxy now defines and preserves the X-Stream-Protocol-Version header, with tests verifying forwarding and response formatting.

Changes

Stream protocol header forwarding

Layer / File(s) Summary
Allow and test stream protocol header
internal/proxy/proxy.go, internal/proxy/proxy_test.go
Defines the stream protocol header constant, adds it to the proxy allowlist, and tests its forwarded value in the response.

Estimated code review effort: 1 (Trivial) | ~5 minutes

Suggested reviewers: phillebaba

Poem

A bunny hops through headers bright,
Keeping stream versions in sight.
The proxy lets the value through,
Tests confirm the path stays true.
One little field, no loss tonight!

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly matches the main change: allowing X-Stream-Protocol-Version for streaming negotiation.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@Wouter0100

Wouter0100 commented Aug 6, 2026

Copy link
Copy Markdown
Author

We're running this on production, works flawless. Improves compatibility with older clusters.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant