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

Gateway API with websocket #5241

Closed
s-bauer opened this issue Apr 3, 2023 · 7 comments · Fixed by #5524
Closed

Gateway API with websocket #5241

s-bauer opened this issue Apr 3, 2023 · 7 comments · Fixed by #5524
Assignees
Labels
area/gateway-api Issues or PRs related to the Gateway (Gateway API working group) API. kind/feature Categorizes issue or PR as related to a new feature.
Milestone

Comments

@s-bauer
Copy link

s-bauer commented Apr 3, 2023

What question do you have?:
When using the Kubernetes Gateway API, I don't see any obvious option to enable websocket support for contour. This is fairly annoying as I'd need to switch to the "traditional" deployment mode just for that. Is there any way to get websockets working while using the Gateway API?

@s-bauer s-bauer added kind/question Categorizes an issue as a user question. lifecycle/needs-triage Indicates that an issue needs to be triaged by a project contributor. labels Apr 3, 2023
@github-actions
Copy link

github-actions bot commented Apr 3, 2023

Hey @s-bauer! Thanks for opening your first issue. We appreciate your contribution and welcome you to our community! We are glad to have you here and to have your input on Contour. You can also join us on our mailing list and in our channel in the Kubernetes Slack Workspace

@skriss skriss added the area/gateway-api Issues or PRs related to the Gateway (Gateway API working group) API. label Apr 3, 2023
@skriss
Copy link
Member

skriss commented Apr 3, 2023

Hey @s-bauer, unfortunately right now we don't have a way to enable websockets via Gateway API. This is being discussed upstream in Gateway API in kubernetes-sigs/gateway-api#205 and https://gateway-api.sigs.k8s.io/geps/gep-1282/. In the interim, I can see a few possibilities:

  • we enable the websockets upgrade by default for Gateway API (some other implementations do this already)
  • we add a ContourConfiguration setting toggling whether to enable websockets by default for all routes on a Gateway, which can then be used as part of a GatewayClass's parameters.

The first is definitely tempting from an ease-of-implementation perspective, but I think the second would be a bit more robust to potential future changes to the API spec. Interested in others' thoughts here as well.

@skriss skriss added kind/feature Categorizes issue or PR as related to a new feature. and removed kind/question Categorizes an issue as a user question. labels Apr 3, 2023
@s-bauer
Copy link
Author

s-bauer commented Apr 3, 2023

Thanks for the quick reply! Would it be an option to add it as an annotation to the HTTPRoute as a workaround? That way users could still selectively turn it on for specific routes. At the same time, I don't see a reason for websockets to be disabled in the first place, so enabling them globally for gateway api sounds good as well.

Anyway, I myself migrated to the "classic" deployment model of Contour to be able to use websockets. Gateway API sounds great in the beginning, but it's simply not ready for actual usage yet. Hopefully this will change in the near future.

@skriss
Copy link
Member

skriss commented Apr 3, 2023

Would it be an option to add it as an annotation to the HTTPRoute as a workaround?

We, and Gateway API in general, have really tried to stay away from using annotations on Gateway API resources to avoid going down the rabbit-hole that Ingress ended up in with lots of custom annotations. I think we would really prefer not to do this here, despite its expediency.

At the same time, I don't see a reason for websockets to be disabled in the first place, so enabling them globally for gateway api sounds good as well.

So we'll just need to decide (a) do we add a knob to disable this; and (b) is it on by default or off by default

@evankanderson
Copy link

My $0.02 would also be to enable the websocket handshake by default, possibly with a flag to disable the handshake. I'll admit that I don't have a perfect mental model of how the handshake works, but my simple model suggests that even if envoy is configured with "upgrade_configs": [{"upgrade_type": "websocket"}], the upstream could still refuse the Upgrade: websocket header (e.g. not returning a 101 Switching Protocols, and instead returning something like a 400 error.

If so, it seems like the main risk of enabling websocket by default is that platform operators might want to enforce shorter HTTP request deadlines (and the issue that we're technically breaking an existing API). Maybe a config option to set the default if unset (and default the option to true going forward)... I hate to just add config options, but I also hate to break an existing API. On the gripping hand, platform operators who want to prevent websocket usage will already need to use some form of policy system, which could specifically set the websocket value to false, rather than simply disallowing true.

Historical notes

For those sleuthing, the discussion in envoyproxy/envoy#319 (comment) referenced by #600 is probably no longer relevant, as that version of websocket support in envoy was retired in envoyproxy/envoy#4745 in favor of the rework done in envoyproxy/envoy#3301. For those not interested in the minutae; the TL;DR is that the initial websocket implementation expected exactly the websocket handshake and then did a TCP forward to the upstream, bypassing all HTTP filters. Alyssa's rewrite incorporates the upgrade into the HTTP filter chain, allowing for things like timeouts to apply to websockets the same way they do to other requests.

Ironically, Alyssa's work appears to have occurred almost contemporaneously with the discussion in #600 (within about 45 days of each other).

@evankanderson
Copy link

evankanderson commented Apr 4, 2023

FWIW, defaulting to websockets and h2c enabled for Gateway API on Contour would resolve most of the net-gateway-api failures in Knative:

Tested Ingress

| Ingress | Unavailable features |
| ===|===|
| Istio | retry, httpoption, host-rewrite |
| Contour | httpoption, basics/http2, websocket, websocket/split, grpc, grpc/split, update, host-rewrite |

httpoption and host-rewrite are currently unimplemented, but could probably be implemented fairly soon if it would move net-gateway-api towards a supported GA bill of health.

@Jean-Daniel
Copy link
Contributor

Jean-Daniel commented Apr 6, 2023

Anyway, I myself migrated to the "classic" deployment model of Contour to be able to use websockets. Gateway API sounds great in the beginning, but it's simply not ready for actual usage yet. Hopefully this will change in the near future.

Using the provisioner and a Gateway to deploy Contour/Envoy does not prevent you to use HTTPProxy to define routes.

Contour merges definitions from both HTTPRoute and HTTPProxy to configure envoy.

@skriss skriss added this to the 1.26.0 milestone May 4, 2023
@skriss skriss removed the lifecycle/needs-triage Indicates that an issue needs to be triaged by a project contributor. label May 15, 2023
@skriss skriss self-assigned this Jun 23, 2023
skriss added a commit that referenced this issue Jun 23, 2023
Closes #5241. 

Signed-off-by: Steve Kriss <krisss@vmware.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/gateway-api Issues or PRs related to the Gateway (Gateway API working group) API. kind/feature Categorizes issue or PR as related to a new feature.
Projects
Status: Done
Development

Successfully merging a pull request may close this issue.

4 participants