-
Notifications
You must be signed in to change notification settings - Fork 108
WebSocket Support for Apps with Route-Services #474
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
WebSocket Support for Apps with Route-Services #474
Conversation
28e375e
to
7bbb6e0
Compare
Performed a test of this Routing-release Gorouter change on a dev landscape with a sample websocket-app to show that the apps with bound route services support web sockets:
wscat -c wss://websocket-app.cfapps.xxx.sapcloud.io/ws
|
ca97cc6
to
e88e6b1
Compare
src/code.cloudfoundry.org/gorouter/integration/route_services_test.go
Outdated
Show resolved
Hide resolved
src/code.cloudfoundry.org/gorouter/integration/route_services_test.go
Outdated
Show resolved
Hide resolved
src/code.cloudfoundry.org/gorouter/integration/route_services_test.go
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Requested default change
GitHub seems confused about workflows being there while also not being there on this branch. @Dariquest can you rebase once more? |
Everything looks good to me. Once the rebasing + GH actions are sorted out feel free to approve + merge.n |
0ebd0ac
to
6918d99
Compare
The workflow uses secrets, e.g. We will ignore the validation for now but can have a discussion on how to fix this for future contributions. /fyi @geofffranks @kart2bc |
Add new spec for gorouter.route_services.enable_websockets Default value is true Unit & Integration tests with a websocket client
6918d99
to
be0152d
Compare
Summary
Route services should be able to handle web socket requests.
The Gorouter flag "route_services.enable_websockets" is introduced to enable web socket communication for apps with route services. The default value is true.
Web socket upgrades on routes with an attached route service were prevented previously.
This issue has been brought up by one of our customers, who needed a route service to rate-limit their app.
GitHub Issue
Backward Compatibility
Breaking Change? No
Test
Scenario: A web socket app needs rate limiting via a route service.
Given
When I connect to wss://websocket-app.cfapps.xxx.sapcloud.io using a web socket client or simply curl it with the corresponding websocket request headers
Then The connection will work through the route service as expected