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

Allow korifi apps to use websockets #1436

Merged
merged 1 commit into from Jul 27, 2022
Merged

Allow korifi apps to use websockets #1436

merged 1 commit into from Jul 27, 2022

Conversation

georgethebeatle
Copy link
Member

Is there a related GitHub Issue?

#1250

What is this change about?

It turns out that Contour is disallowing websockets by default. This
breaks apps like postfacto. This commit allows all apps to use
websockets.

Does this PR introduce a breaking change?

No

Acceptance Steps

Deploy an app that uses websockets (e.g postfacto) and see it wokring. With postfacto working means that different browsers see live updates on the retro board.

Tag your pair, your PM, and/or team

@kieron-dev

Things to remember

It turns out that Contour is disallowing websockets by default. This
breaks apps like postfacto. This commit allows all apps to use
websockets.

Co-authored-by: Georgi Sabev <georgethebeatle@gmail.com>
@tcdowney
Copy link
Member

Re: my questions on Gateway API support for websockets from the WG meeting. I did some quick digging, doesn't look like backend protocol tweaks like this are supported yet, but it is on their radar.

Relevant issues:

I don't think uncertainty there should block us from enabling websockets on Contour because it's a pretty big CF app compatibility gap not to support them, but this will delay us from adopting Gateway API in any non-experimental capacity I think.

@tcdowney
Copy link
Member

Other question I had: turning this config on broadly doesn't break apps that don't support websockets, right?

Copy link
Member

@Birdrock Birdrock left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As discussed at the bi-weekly Cloud Foundry on Kubernetes Working Group Forum meeting 7/26, this seems reasonable. The default of Envoy is to disable Websockets, so Contour follows in this regard. Enabling Websockets does increase surface area, but is a widespread and fairly well known area.

@kieron-dev
Copy link
Contributor

Other question I had: turning this config on broadly doesn't break apps that don't support websockets, right?

Postfacto is using websockets and http on the same port, and it works ok. I don't see any reason why it might break other apps not using websockets. I think we should just apply this and keep an eye on it.

@kieron-dev kieron-dev merged commit 260ba16 into main Jul 27, 2022
@kieron-dev kieron-dev deleted the pr-allow-websockets branch July 27, 2022 09:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants