-
Notifications
You must be signed in to change notification settings - Fork 647
fix(tcp): avoid duplicate EmptyCluster routes for listeners without routes #7912
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
base: main
Are you sure you want to change the base?
fix(tcp): avoid duplicate EmptyCluster routes for listeners without routes #7912
Conversation
✅ Deploy Preview for cerulean-figolla-1f9435 ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
Codecov Report❌ Patch coverage is
❌ Your patch check has failed because the patch coverage (50.00%) is below the target coverage (60.00%). You can increase the patch coverage or adjust the target coverage. Additional details and impacted files@@ Coverage Diff @@
## main #7912 +/- ##
==========================================
+ Coverage 72.36% 72.86% +0.49%
==========================================
Files 234 237 +3
Lines 34566 35538 +972
==========================================
+ Hits 25014 25895 +881
- Misses 7761 7802 +41
- Partials 1791 1841 +50 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
thanks @Aditya7880900936 , instead of the standalone test, can you rm that and add a test case in https://github.com/envoyproxy/gateway/tree/main/internal/xds/translator/testdata that mimics the issue |
|
Thanks for the suggestion @arkodg , |
|
can you revert the standalone test file and leave it as is |
|
Hi @arkodg , |
|
The new diff still doesn’t implement the requested changes and introduces the same issues as before. We’ve already provided detailed guidance, so further updates need to be reviewed carefully before being pushed. Please do not submit another diff until you have verified that it actually addresses each review comment. |
aeed89b to
407f10a
Compare
|
Hi @arkodg, Thanks for the feedback. I’ve cleaned up the diff and removed the unrelated dependabot change. I’ve re-verified the behavior locally and all translator tests are passing. |
407f10a to
d6a9025
Compare
…outes Signed-off-by: Aditya7880900936 <adityasanskarsrivastav788@gmail.com>
d6a9025 to
378b464
Compare
|
Hi @arkodg, I fixed the test compilation issue and re-ran the translator tests locally. I’ve force-pushed the updated commit — please let me know if anything else |
This PR fixes an issue where Envoy Gateway generated invalid Envoy configuration
when multiple TCP/TLS passthrough listeners had no attached routes.
In such cases, the translator was creating identical dummy EmptyCluster routes
per listener, which resulted in duplicate routes and Envoy rejecting the config.
What this PR does
Tests
with multiple TCP/TLS passthrough listeners that have no attached routes
Why this is needed
This scenario is valid per Gateway API, but previously caused Envoy startup
failures due to duplicate dummy routes.
Fixes #7866