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

fix: duplicated xroutes are added to gatewayapi.Resources #3282

Merged
merged 5 commits into from May 16, 2024

Conversation

aoledk
Copy link
Contributor

@aoledk aoledk commented Apr 26, 2024

What type of PR is this?

Fix duplicated TLSRoute/HTTPRoute/GRPCRoute/TCPRoute/UDPRoute are added to gatewayapi.Resources when single xRoute is attaching to multiple Gateways.

What this PR does / why we need it:

When single xRoute is attaching to multiple Gateways, this case is normal, different listeners with distinct certificates or transport sockets should route traffic to same backend. Duplicated xRoutes will be processed in gatewayapi module, then listener's attachedRoutes will be incorrect.

Which issue(s) this PR fixes:

Fixes #3165

Signed-off-by: Dingkang Li <dingkang1743@gmail.com>
@aoledk aoledk requested a review from a team as a code owner April 26, 2024 07:38
Copy link

codecov bot commented May 16, 2024

Codecov Report

Attention: Patch coverage is 40.00000% with 15 lines in your changes are missing coverage. Please review.

Project coverage is 67.01%. Comparing base (c4b0216) to head (2bf50d6).
Report is 7 commits behind head on main.

Files Patch % Lines
internal/provider/kubernetes/routes.go 25.00% 10 Missing and 5 partials ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #3282      +/-   ##
==========================================
+ Coverage   66.97%   67.01%   +0.03%     
==========================================
  Files         164      166       +2     
  Lines       23882    23956      +74     
==========================================
+ Hits        15994    16053      +59     
- Misses       6964     6973       +9     
- Partials      924      930       +6     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Copy link
Contributor

@arkodg arkodg left a comment

Choose a reason for hiding this comment

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

LGTM thanks !

@arkodg arkodg added this to the v1.1.0-rc1 milestone May 16, 2024
@arkodg arkodg requested review from a team May 16, 2024 22:55
@zirain
Copy link
Contributor

zirain commented May 16, 2024

/retest

Comment on lines 118 to +128
allAssociatedNamespaces map[string]struct{}
// Map for storing TLSRoutes' NamespacedNames attaching to various Gateway objects.
allAssociatedTLSRoutes map[string]struct{}
// Map for storing HTTPRoutes' NamespacedNames attaching to various Gateway objects.
allAssociatedHTTPRoutes map[string]struct{}
// Map for storing GRPCRoutes' NamespacedNames attaching to various Gateway objects.
allAssociatedGRPCRoutes map[string]struct{}
// Map for storing TCPRoutes' NamespacedNames attaching to various Gateway objects.
allAssociatedTCPRoutes map[string]struct{}
// Map for storing UDPRoutes' NamespacedNames attaching to various Gateway objects.
allAssociatedUDPRoutes map[string]struct{}
Copy link
Contributor

Choose a reason for hiding this comment

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

replace this with sets.set[string]

Copy link
Contributor

Choose a reason for hiding this comment

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

tracked with #3411

Copy link
Contributor

@zirain zirain left a comment

Choose a reason for hiding this comment

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

LGTM, with a followup

Copy link
Contributor

@zirain zirain left a comment

Choose a reason for hiding this comment

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

LGTM, with a followup

@arkodg arkodg merged commit 32c6876 into envoyproxy:main May 16, 2024
21 of 22 checks passed
@aoledk aoledk deleted the fix-duplicated-xroutes branch May 17, 2024 01:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Duplicated xRoutes in gatewayapi.Resources when attaching xRoute to multiple Gateways
3 participants