api: split ClusterSettings into BackendSettings and ClusterSettings - #8899
api: split ClusterSettings into BackendSettings and ClusterSettings#8899muwaqar-cflt wants to merge 5 commits into
Conversation
✅ Deploy Preview for cerulean-figolla-1f9435 ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 3d5671b0a1
ℹ️ About Codex in GitHub
Codex has been enabled to automatically 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 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
|
Hi @muwaqar-cflt, the new API shape looks good to me. Thanks for the quick PR! A couple of questions:
cc @envoyproxy/gateway-maintainers , please chime in. |
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #8899 +/- ##
==========================================
+ Coverage 76.19% 76.20% +0.01%
==========================================
Files 261 261
Lines 43528 43528
==========================================
+ Hits 33164 33170 +6
+ Misses 8161 8158 -3
+ Partials 2203 2200 -3 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
74b8411 to
5e69afc
Compare
5e69afc to
159b4e2
Compare
a08ab35 to
9084c06
Compare
|
LGTM, thanks! |
…ields Extract CDS-only fields (LoadBalancer, ProxyProtocol, TCPKeepalive, HealthCheck, CircuitBreaker, Timeout, Connection, DNS, HTTP2) into BackendClusterSettings. ClusterSettings embeds it inline and adds Retry. JSON field paths are unchanged (non-breaking). Closes envoyproxy#8898 Signed-off-by: Muhammad Waqar <waqar.hameed08@gmail.com>
Address reviewer feedback to use a shorter, clearer name. Signed-off-by: Muhammad Waqar <waqar.hameed08@gmail.com>
… BackendSettings Swap type names per reviewer feedback: the CDS-only struct becomes ClusterSettings and the wrapper (with Retry) becomes BackendSettings. Also rename clustersettings.go to backendsettings.go. Signed-off-by: Muhammad Waqar <waqar.hameed08@gmail.com>
…s after rebase Rebasing onto main picked up test cases that construct BackendTrafficPolicySpec with a direct ClusterSettings field, which this branch's split moved one level deeper under the new BackendSettings field. git's auto-merge didn't flag these as conflicts since the surrounding lines didn't textually overlap. Signed-off-by: Muhammad Waqar <mwaqar@confluent.io>
The rebase's conflict resolution took a stale snapshot of this generated file; regenerate it properly from the current API types. Signed-off-by: Muhammad Waqar <mwaqar@confluent.io>
9084c06 to
e3a3351
Compare
|
LGTM. Deferring to @guydc I'm also OK with delaying this to EG API graduating to V1. |
Summary
Extract CDS-only fields from the existing
ClusterSettingsinto a dedicatedClusterSettingsstruct, and rename the wrapper (which addsRetry) toBackendSettings.BackendSettingsembedsClusterSettingsinline and addsRetry. JSON field paths are unchanged — this is a non-breaking refactor.Closes #8898
Fields in
ClusterSettings(CDS-only)loadBalancerproxyProtocoltcpKeepalivehealthCheckcircuitBreakertimeoutconnectiondnshttp2Fields in
BackendSettings(wrapper)ClusterSettingsinlineretry(RDS concern, not applicable to Backend targets)Test plan
go build ./...succeedsmake lintpassesmake formatpassesmake gen-checkpasses