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

ingress/gateway-api: sort virtual hosts in CEC #31493

Conversation

mhofstetter
Copy link
Member

@mhofstetter mhofstetter commented Mar 19, 2024

Currently, while translating K8s Ingress or Gateway API resources into Envoy resources, the virtualhosts aren't sorted. This leads to situations (especially in combination with Shared Ingress) where the order of the virtual hosts isn't guaranteed (depends on the order of the Shared Ingresses) - resulting in unnecessary reconciliations.

Therefore, this commit orders the virtualhosts within a Envoy RouteConfiguration by their name. This makes the translation deterministic.

It influences the Envoy route matching process - but the order of the virtual hosts should only be relevant if a request host header matches multiple virtual hosts on the same "level".

Domain search order:
- Exact domain names: www.foo.com.
- Suffix domain wildcards: *.foo.com or *-bar.foo.com.
- Prefix domain wildcards: foo.* or foo-*.
- Special wildcard * matching any domain.

https://www.envoyproxy.io/docs/envoy/latest/api-v3/config/route/v3/route_components.proto#envoy-v3-api-msg-config-route-v3-virtualhost

Currently, while translating K8s Ingress or Gateway API resources into
Envoy resources, the virtualhosts aren't sorted. This leads to situations
(especially in combination with Shared Ingress) where the order of the virtual
hosts isn't guaranteed.

Therefore, this commit orders the virtualhosts within a Envoy RouteConfiguration
by their name. This influences the Envoy route matching process
(https://www.envoyproxy.io/docs/envoy/latest/configuration/http/http_conn_man/route_matching),
but only by making it constant and not random.

Signed-off-by: Marco Hofstetter <marco.hofstetter@isovalent.com>
@mhofstetter mhofstetter added kind/bug This is a bug in the Cilium logic. release-note/bug This PR fixes an issue in a previous release of Cilium. area/servicemesh GH issues or PRs regarding servicemesh feature/k8s-gateway-api feature/k8s-ingress labels Mar 19, 2024
@mhofstetter
Copy link
Member Author

/test

@mhofstetter mhofstetter marked this pull request as ready for review March 19, 2024 14:17
@mhofstetter mhofstetter requested a review from a team as a code owner March 19, 2024 14:17
@mhofstetter mhofstetter changed the title ingress/gateway-api: sorted virtual hosts ingress/gateway-api: sort virtual hosts in CEC Mar 19, 2024
@mhofstetter mhofstetter added the needs-backport/1.15 This PR / issue needs backporting to the v1.15 branch label Mar 20, 2024
@sayboras sayboras added this pull request to the merge queue Mar 26, 2024
Merged via the queue into cilium:main with commit 7992f75 Mar 26, 2024
63 checks passed
@mhofstetter mhofstetter deleted the pr/mhofstetter/ingress-cec-virtualhost-sorted branch March 26, 2024 07:38
@joamaki joamaki mentioned this pull request Apr 2, 2024
13 tasks
@mhofstetter mhofstetter added backport/author The backport will be carried out by the author of the PR. backport-pending/1.15 The backport for Cilium 1.15.x for this PR is in progress. and removed needs-backport/1.15 This PR / issue needs backporting to the v1.15 branch labels Apr 3, 2024
@github-actions github-actions bot added backport-done/1.15 The backport for Cilium 1.15.x for this PR is done. and removed backport-pending/1.15 The backport for Cilium 1.15.x for this PR is in progress. labels Apr 3, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/servicemesh GH issues or PRs regarding servicemesh backport/author The backport will be carried out by the author of the PR. backport-done/1.15 The backport for Cilium 1.15.x for this PR is done. feature/k8s-gateway-api feature/k8s-ingress kind/bug This is a bug in the Cilium logic. release-note/bug This PR fixes an issue in a previous release of Cilium.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants