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

support websocket #836

Closed
qicz opened this issue Dec 27, 2022 · 9 comments · Fixed by #867
Closed

support websocket #836

qicz opened this issue Dec 27, 2022 · 9 comments · Fixed by #867
Assignees
Labels
kind/enhancement New feature or request
Milestone

Comments

@qicz
Copy link
Member

qicz commented Dec 27, 2022

Description:

Describe the desired behavior, what scenario it enables and how it
would be used.

[optional Relevant Links:]

Any extra documentation required to understand the issue.

@qicz qicz added the kind/enhancement New feature or request label Dec 27, 2022
@Xunzhuo
Copy link
Member

Xunzhuo commented Dec 29, 2022

Plz provide more infos, then we can have a deep understanding on the background of this issue.

@qicz
Copy link
Member Author

qicz commented Dec 31, 2022

the current eg version cannot support WebSocket requests that are missing upgrade configs for this. just this. @Xunzhuo

@danehans
Copy link
Contributor

danehans commented Jan 3, 2023

Moving to the backlog since kubernetes-sigs/gateway-api#205 must be resolved before this feature can be implemented.

@danehans danehans added this to the Backlog milestone Jan 3, 2023
@zhshw
Copy link

zhshw commented Jan 4, 2023

@danehans HTTPRoute add annations ,like this can imp websocket、timeout、retry......
HTTPRoute.Annotations["envoy-gateway/upgrade-websocket"]

routeAction setting UpgradeConfig
if isWebsocket { routeAction.UpgradeConfigs = []*route.RouteAction_UpgradeConfig{{ UpgradeType: "websocket", }} }

@arkodg
Copy link
Contributor

arkodg commented Jan 4, 2023

@zhshw annotations lack structure needed to correctly validate intent, we prefer adding support using explicit fields instead

@zhshw
Copy link

zhshw commented Jan 5, 2023

@arkodg
annotations are the simplest and most practical way to quickly implement useful functions. Annotation content can be yaml or json,Disagree with annotations lack structure

Backendref serivce(single OR multi?)can be an associated Envoy route configuration . Use the Envoy xds route field as a richer configuration

The intermediate field definition will lose many useful configurations. Moreover, there is a lack of validation, design documents and usage documents and the development cycle is long

Use xds route configuration fragments (annotations) or associated complete route configuration (configmap, crd), they are all better implementation

Summary:

  • No lose many useful configurations
  • No risk of design failure ( intermediate field)
  • Quickly implement
  • Configure it by using the envoy document (helpful)

@luofish
Copy link

luofish commented Jan 5, 2023

@arkodg it's a feature request, maybe we can use anno instead before explicit fields be designed

@crain-cn
Copy link

crain-cn commented Jan 5, 2023

@arkodg it's a feature request, maybe we can use anno instead before explicit fields be designed

@arkodg
Copy link
Contributor

arkodg commented Jan 6, 2023

it looks like envoy allows web socket upgrade https://www.envoyproxy.io/docs/envoy/latest/start/sandboxes/websocket which should be easy to achieve using this config in HCM

upgrade_configs:
          - upgrade_type: websocket

and it looks like istio enables this by default, so leaning towards enabling this by default for HTTP 1.1 (HTTPRoute), upgrades are disallowed for HTTP 2.0 (GRPCRoute)

@arkodg arkodg self-assigned this Jan 6, 2023
arkodg added a commit to arkodg/gateway that referenced this issue Jan 6, 2023
Fixes: envoyproxy#836

Signed-off-by: Arko Dasgupta <arko@tetrate.io>
arkodg added a commit to arkodg/gateway that referenced this issue Jan 10, 2023
Fixes: envoyproxy#836

Signed-off-by: Arko Dasgupta <arko@tetrate.io>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/enhancement New feature or request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

7 participants