refactor: only include cluster-scoped traffic features in BackendCluster - #9642
Open
zhaohuabing wants to merge 1 commit into
Open
refactor: only include cluster-scoped traffic features in BackendCluster#9642zhaohuabing wants to merge 1 commit into
zhaohuabing wants to merge 1 commit into
Conversation
✅ Deploy Preview for cerulean-figolla-1f9435 ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
zhaohuabing
marked this pull request as draft
August 3, 2026 03:52
zhaohuabing
force-pushed
the
fix-cluster-traffic-features
branch
from
August 3, 2026 03:53
e87562b to
30fbf39
Compare
zhaohuabing
marked this pull request as ready for review
August 3, 2026 03:57
zhaohuabing
force-pushed
the
fix-cluster-traffic-features
branch
from
August 3, 2026 03:58
30fbf39 to
9d6c368
Compare
Member
Author
|
@codex review |
zhaohuabing
force-pushed
the
fix-cluster-traffic-features
branch
from
August 3, 2026 04:05
9d6c368 to
3853fd0
Compare
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 9d6c36836e
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
zhaohuabing
force-pushed
the
fix-cluster-traffic-features
branch
4 times, most recently
from
August 3, 2026 08:04
ed25170 to
7fd7984
Compare
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #9642 +/- ##
==========================================
+ Coverage 76.06% 76.08% +0.01%
==========================================
Files 260 260
Lines 43449 43470 +21
==========================================
+ Hits 33048 33072 +24
Misses 8196 8196
+ Partials 2205 2202 -3 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
zhaohuabing
force-pushed
the
fix-cluster-traffic-features
branch
2 times, most recently
from
August 4, 2026 01:09
74416aa to
de6c525
Compare
Member
Author
|
/retest |
zhaohuabing
force-pushed
the
fix-cluster-traffic-features
branch
from
August 4, 2026 02:59
de6c525 to
a9bb9a1
Compare
Contributor
|
seems similar to #8899 |
muwaqar-cflt
approved these changes
Aug 4, 2026
Member
Author
This PR doesn't touch API, just the internal IR. |
BackendCluster.Traffic carried the full TrafficFeatures, but only the ten fields applyTraffic() consumes translate to an Envoy cluster (CDS). The remaining route/HCM-scoped features (rate limit, fault injection, retry, response override, ...) were dead data on a cluster shared across many routes, and nothing stopped a future reader from consuming them there, where they would be silently wrong. Split those ten fields into ClusterTrafficFeatures, inline-embedded into TrafficFeatures so serialization and promoted field access are unchanged, and narrow BackendCluster.Traffic to *ClusterTrafficFeatures. applyTraffic and ExtraArgs.traffic take the narrow type too, so a route-scoped TrafficFeatures field can no longer reach CDS. Timeout needed the same treatment one level down: RequestTimeout and StreamIdleTimeout are read only by route and filter translation, yet rode along on every merged cluster. Split the three CDS members into ClusterHTTPTimeout, inline-embedded into HTTPTimeout, and give xdsClusterArgs a ClusterTimeout so reading a route-scoped member during cluster translation is a compile error. Timeout.ClusterOnly() builds that value from ClusterHTTPTimeout rather than by clearing fields, so a member added to HTTPTimeout stays out of CDS unless added deliberately. No behavior change: xDS output is identical and no testdata was touched. Fixes envoyproxy#9579 Signed-off-by: Huabing (Robin) Zhao <zhaohuabing@gmail.com>
zhaohuabing
force-pushed
the
fix-cluster-traffic-features
branch
from
August 5, 2026 01:51
a9bb9a1 to
b2bdf57
Compare
Member
Author
|
/retest |
zirain
approved these changes
Aug 5, 2026
Member
Author
|
/retest |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
BackendCluster.Trafficheld the fullTrafficFeatures, but only the ten fieldsapplyTraffic()consumes translate to an Envoy cluster (CDS). The other nine are route/HCM-scoped — dead data on a cluster shared across many routes, and a trap for anyone who later reads them there.This splits those ten into
ClusterTrafficFeatures, inline-embedded intoTrafficFeaturesso serialization and promoted field access are unchanged, and narrowsBackendCluster.Trafficto*ClusterTrafficFeatures.applyTrafficandExtraArgs.traffictake the narrow type too, so a route-scopedTrafficFeaturesfield can no longer reach CDS.Timeoutneeded the same treatment one level down:requestTimeoutandstreamIdleTimeoutare read only by route and filter translation, yet rode along on every merged cluster. The three CDS members move intoClusterHTTPTimeout, inline-embedded intoHTTPTimeout, andxdsClusterArgsnow holds aClusterTimeout— so reading a route-scoped member during cluster translation is a compile error.Timeout.ClusterOnly()builds that value fromClusterHTTPTimeoutrather than by clearing fields, so a member added toHTTPTimeoutstays out of CDS unless it is added deliberately.No behavior change: xDS output is identical and no testdata was touched.
Fixes #9579