Envoy: Upgrade websocket request when header present, otherwise do not reject

[shalako]

The current websocket support appears to offer two choices:

• When route is configured with use_websocket:true upgrade if header is present, otherwise reject
• The spec doesn't say what the behavior is when use_websocket:false; are requests with the upgrade header rejected or is the header ignored and the request forwarded over http?

The behavior of Cloud Foundry's current edge proxy is:

• If the upgrade header is present, we initiate a TCP connection to the backend
• If the upgrade header is not present, we forward an HTTP request
• There is no user control of whether upgrade header is rejected or accepted
• Apps are responsible for rejecting connections/requests based on whether they require TCP or HTTP requests

In order to transition from our current edge proxy to Envoy without downtime for upstream applications, we must continue supporting websockets on all routes without breaking apps that don't support them.

Could we change the semantics of this boolean to:

• when use_websocket:true upgrade if header is present, otherwise forward request over http
• when use_websocket:false keep current behavior

[alyssawilk]

@rshriram have thoughts on this?

[ggreenway]

I'll need this also, eventually. If eventually arrives and it isn't done yet, I can help on this, but I don't have time to do it right now.

[shalako]

@mattklein123 By adding the label help_wanted does that mean you approve of the proposed semantic change?

[mattklein123]

Sure fine with me.

[rshriram]

Hi, sorry for the delay (missed this in the millions of notifications that GH sends me every day). Will comment in detail on the rationale for choosing the connection rejection behavior in an hour.

[ggreenway]

@rshriram if you need the current behavior, I'd say we should move the setting to a tri-state: required, allowed, and prohibited (possibly with better names than that).

[rshriram]

Sadly, I cannot recall why we decided to have the strict mode. But I am in favor of removing the restriction as it simplifies configuration overall (in Istio) and perhaps elsewhere as well.

[shalako]

Hooray! Thank you, @rshriram!0

[yuvipanda]

Heya! The current docs for v1.5.0 or latest seem to say that this change is no longer valid (https://www.envoyproxy.io/docs/envoy/v1.5.0/api-v2/rds.proto.html#routeaction under 'use_websocket'). The doc changes in #1792 seem to be no longer present. Is that a miss during repo changes or has this change been reverted?

I do get an error upstream connect error or disconnect/reset before headers with use_websocket: true against a http endpoint.

[yuvipanda]

The error is PBKAC, but the doc issue seems real.

[mattklein123]

@yuvipanda please do a doc PR and fix!