Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Option to send websockets upstream using HTTP/1.1 #261

Closed
wants to merge 2 commits into from
Closed

Option to send websockets upstream using HTTP/1.1 #261

wants to merge 2 commits into from

Conversation

46bit
Copy link
Contributor

@46bit 46bit commented Oct 8, 2021

Gorouter recently added support for HTTP/2. However it turns out to be broken for HTTP/2 Websockets (RFC 8441): cloudfoundry/routing-release#230. It can't immediately be fixed because the bug arises from a problem in Go's net/http: golang/go#32763.

This PR implements a suggestion by @Gerg in #259: forward Websocket traffic to Gorouter over HTTP/1.1, even if HTTP2 is enabled.

Downside of this approach is that we're duplicating a lot of config between the two backend blocks. To stop them drifting from each other, I've added a test that their config matches. If anyone only changes one of the blocks then the tests will fail, pushing them to change the other block as well.

@46bit 46bit marked this pull request as ready for review October 15, 2021 10:14
@46bit 46bit changed the title [WIP] Option to send websockets upstream using HTTP/1.1 Option to send websockets upstream using HTTP/1.1 Oct 15, 2021
@peterellisjones peterellisjones added the run-ci Allow this PR to be tested on Concourse label Oct 18, 2021
@plowin
Copy link
Contributor

plowin commented Oct 22, 2021

Closing in favor of #263

@plowin plowin closed this Oct 22, 2021
@46bit 46bit deleted the send-websockets-upstream-using-http1.1 branch October 22, 2021 20:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
run-ci Allow this PR to be tested on Concourse
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants