8000 gateway-api: appProtocol support (GEP-1911) by rauanmayemir · Pull Request #31310 · cilium/cilium · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

gateway-api: appProtocol support (GEP-1911) #31310

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

Merged
merged 4 commits into from
Apr 29, 2024

Conversation

rauanmayemir
Copy link
Contributor

No description provided.

@maintainer-s-little-helper maintainer-s-little-helper bot added the dont-merge/needs-release-note-label The author needs to describe the release impact of these changes. label Mar 11, 2024
@github-actions github-actions bot added the kind/community-contribution This was a contribution made by a community member. label Mar 11, 2024
@joestringer
Copy link
Member

/test

@rauanmayemir rauanmayemir force-pushed the feature/gwapi-app-protocol branch 2 times, most recently from d40683e to d1de40a Compare March 12, 2024 07:31
@rauanmayemir rauanmayemir force-pushed the feature/gwapi-app-protocol branch 3 times, most recently from fe729bf to 585f67e Compare March 30, 2024 12:46
@rauanmayemir rauanmayemir marked this pull request as ready for review March 30, 2024 14:52
@rauanmayemir rauanmayemir requested review from a team as code owners March 30, 2024 14:52
@rauanmayemir
Copy link
Contributor Author

/test

@rauanmayemir
Copy link
Contributor Author

/ci-gateway-api

Copy link
Contributor
@lambdanis lambdanis left a comment

Choose a reason for hiding this comment

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

docs good

@lambdanis lambdanis added feature/k8s-gateway-api release-note/minor This PR changes functionality that users may find relevant to operating Cilium. labels Apr 2, 2024
@maintainer-s-little-helper maintainer-s-little-helper bot removed the dont-merge/needs-release-note-label The author needs to describe the release impact of these changes. label Apr 2, 2024
@rauanmayemir
Copy link
Contributor Author

@sayboras 🥹

@sayboras sayboras self-requested a review April 12, 2024 13:55
@sayboras
Copy link
Member

Thanks for the ping, I have added myself as reviewer.

@sayboras
Copy link
Member

/ci-gateway-api

@sayboras
Copy link
Member

From a quick look, the changes are looking good to me. Will take a closer look later.

@rauanmayemir rauanmayemir force-pushed the feature/gwapi-app-protocol branch from 00ee5cd to 3b28de2 Compare April 13, 2024 13:36
@sayboras
Copy link
Member

/test

@rauanmayemir
Copy link
Contributor Author

@sayboras ci-ipsec-upgrade failure doesn't look like my doing, do I need to check it?

Copy link
Member
@sayboras sayboras left a comment

Choose a reason for hiding this comment

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

Thanks and LGTM ✅

@rauanmayemir rauanmayemir force-pushed the feature/gwapi-app-protocol branch from 3b28de2 to e9bffc6 Compare April 16, 2024 10:16
@sayboras
Copy link
Member

/test

@rauanmayemir rauanmayemir force-pushed the feature/gwapi-app-protocol branch from e9bffc6 to 8135fee Compare April 21, 2024 13:30
@sayboras
Copy link
Member

/test

@sayboras
Copy link
Member

Seems like we just need to wait for ack from other codeowner, so you don't need to rebase with main branch (just to avoid re-run the tests).

@rauanmayemir
Copy link
Contributor Author

@sayboras I occasionally rebase to make sure there are no conflicts, will wait for the ack from the codeowner.

@sayboras
Copy link
Member

I occasionally rebase to make sure there are no conflicts, will wait for the ack from the codeowner.

Understood and thanks, I normally just rely on github UI to flag if there is any conflict with main branch.

This feature is hidden behind `enable-gateway-api-app-protocol` flag on the operator gwapi cell. Cilium's internal Backend structure is changed to account for appProtocol which is populated from the service spec definitions.

Initial implementation sets envoy cluster `http_protocol_options` to use HTTP1.1 and switches to HTTP2 if backendRef service port specified appProtocol with "kubernetes.io/h2c".

Fixes: cilium#30452

Signed-off-by: Rauan Mayemir <rauan@mayemir.io>
`enable-gateway-api-app-protocol` is false by default, setting it explicitly will turn appProtocol support on. This will effectively change envoy agent config to use HTTP1.1 for upstreams by default which could potentially break existing users if they are sending ingress request as HTTP2 and also expecting their backends to reply in HTTP2/H2C. They will need to add appProtocol option on the service port to unbreak their workloads.

Signed-off-by: Rauan Mayemir <rauan@mayemir.io>
protocol mutator function reset HttpProtocolOptions, leaving only UpstreamProtocolOptions and thus negating WithIdleTimeout mutator changes.

Signed-off-by: Rauan Mayemir <rauan@mayemir.io>
This adds gwapi translator test coverage for H2C backend protocol selection  when appProtocol enabled or disabled.

Signed-off-by: Rauan Mayemir <rauan@mayemir.io>
@rauanmayemir rauanmayemir force-pushed the feature/gwapi-app-protocol branch from 8135fee to 0c1a4ea Compare April 28, 2024 09:33
@youngnick
Copy link
Contributor

/test

@sayboras sayboras enabled auto-merge April 29, 2024 11:31
@sayboras sayboras added this pull request to the merge queue Apr 29, 2024
@maintainer-s-little-helper maintainer-s-little-helper bot added the ready-to-merge This PR has passed all tests and received consensus from code owners to merge. label Apr 29, 2024
Merged via the queue into cilium:main with commit 3d7e8f7 Apr 29, 2024
64 checks passed
@rauanmayemir rauanmayemir deleted the feature/gwapi-app-protocol branch April 29, 2024 12:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature/k8s-gateway-api kind/community-contribution This was a contribution made by a community member. ready-to-merge This PR has passed all tests and received consensus from code owners to merge. release-note/minor This PR changes functionality that users may find relevant to operating Cilium.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants
0