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

Add UpgradeConfig to HTTP Connection Manager #9639

Closed

Conversation

Legogris
Copy link

@Legogris Legogris commented Jan 26, 2021

This should solve #8283

Credit goes to @blake , I simply rebased his commit on master with small changes to get CI passing to get this PR considered for merging.

@vercel vercel bot temporarily deployed to Preview – consul January 26, 2021 11:16 Inactive
@vercel vercel bot temporarily deployed to Preview – consul-ui-staging January 26, 2021 11:16 Inactive
@Legogris Legogris force-pushed the blake/v1.8.0-ingress-websocket-fix branch from cff44ec to 41a5928 Compare January 26, 2021 11:56
@vercel vercel bot temporarily deployed to Preview – consul January 26, 2021 11:56 Inactive
@vercel vercel bot temporarily deployed to Preview – consul-ui-staging January 26, 2021 11:56 Inactive
@hashicorp-cla
Copy link

hashicorp-cla commented Jan 27, 2021

CLA assistant check
All committers have signed the CLA.

@vercel vercel bot temporarily deployed to Preview – consul-ui-staging January 27, 2021 02:42 Inactive
@vercel vercel bot temporarily deployed to Preview – consul January 27, 2021 02:42 Inactive
@dnephin dnephin added theme/connect Anything related to Consul Connect, Service Mesh, Side Car Proxies type/enhancement Proposed improvement or new feature labels Jan 28, 2021
@evandam
Copy link

evandam commented Feb 17, 2021

Hey @Legogris, looking forward to websocket support out of the box! I just wanted to confirm this 100% works right? I stumbled across envoyproxy/envoy#5595 which seems like there's some extra config in http2_protocol_options needed but not sure if that's applicable here.

@blake
Copy link
Contributor

blake commented Feb 17, 2021

@evandam I was able to get WebSocket upgrades to function by simply adding the UpgradeConfigs to the listener configuration, and did not need to add any additional config under http2_protocol_options. The description from the issue #8283 shows how WebSocket upgrades currently fail in Consul, and then succeed after applying this config patch fd993cc.

We are looking to submit a separate PR which will provide a configuration parameter to enable WebSocket upgrades on a per-service basis rather than explicitly enabling it for all services as is the case with this PR.

@neliseev
Copy link

neliseev commented Apr 5, 2021

Guys any news regards this feature requests? Maybe I can help you somehow for make it quickly, we really need support web sockets for our infrastructure. Thank you!

@markan
Copy link
Contributor

markan commented Jun 8, 2021

Thank you for the PR.

We're closing this because making it on by default isn't quite the behavior we need; it must be configurable on a per service basis.

Supporting websocket upgrades in a configurable fashion is something we are interested in doing but haven't fully figured out the implementation details on. We'll track that work in the original issue (#8283)

@Legogris
Copy link
Author

Legogris commented Apr 2, 2022

@markan I see - did that get anywhere?

@porterctrlz
Copy link

@markan @blake Looks like #8283 has stalled, any hope of getting momentum there?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
theme/connect Anything related to Consul Connect, Service Mesh, Side Car Proxies type/enhancement Proposed improvement or new feature
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants