Skip to content

api: split ClusterSettings into BackendSettings and ClusterSettings - #8899

Open
muwaqar-cflt wants to merge 5 commits into
envoyproxy:mainfrom
muwaqar:backend-cluster-settings
Open

api: split ClusterSettings into BackendSettings and ClusterSettings#8899
muwaqar-cflt wants to merge 5 commits into
envoyproxy:mainfrom
muwaqar:backend-cluster-settings

Conversation

@muwaqar-cflt

@muwaqar-cflt muwaqar-cflt commented Apr 30, 2026

Copy link
Copy Markdown
Contributor

Summary

Extract CDS-only fields from the existing ClusterSettings into a dedicated ClusterSettings struct, and rename the wrapper (which adds Retry) to BackendSettings.

BackendSettings embeds ClusterSettings inline and adds Retry. JSON field paths are unchanged — this is a non-breaking refactor.

Closes #8898

Fields in ClusterSettings (CDS-only)

  • loadBalancer
  • proxyProtocol
  • tcpKeepalive
  • healthCheck
  • circuitBreaker
  • timeout
  • connection
  • dns
  • http2

Fields in BackendSettings (wrapper)

  • Embeds ClusterSettings inline
  • retry (RDS concern, not applicable to Backend targets)

Test plan

  • go build ./... succeeds
  • CRD YAML unchanged (no field path changes)
  • make lint passes
  • make format passes
  • make gen-check passes
  • CEL validation tests pass

@muwaqar-cflt
muwaqar-cflt requested a review from a team as a code owner April 30, 2026 18:59
@netlify

netlify Bot commented Apr 30, 2026

Copy link
Copy Markdown

Deploy Preview for cerulean-figolla-1f9435 ready!

Name Link
🔨 Latest commit e3a3351
🔍 Latest deploy log https://app.netlify.com/projects/cerulean-figolla-1f9435/deploys/6a73550c65da2e00099a8cf8
😎 Deploy Preview https://deploy-preview-8899--cerulean-figolla-1f9435.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify project configuration.

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 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".

Comment thread api/v1alpha1/shared_types.go Outdated
@zhaohuabing

zhaohuabing commented May 1, 2026

Copy link
Copy Markdown
Member

Hi @muwaqar-cflt, the new API shape looks good to me. Thanks for the quick PR!

A couple of questions:

  • The YAML CRDs are still compatible, but downstream projects would need to update their code if they construct BackendTrafficPolicy in Go using the old type. Is that acceptable?
  • Do we have better names to distinguish “CDS-only settings” from “non-route backend traffic settings”?

cc @envoyproxy/gateway-maintainers , please chime in.

@codecov

codecov Bot commented May 1, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 90.47619% with 2 lines in your changes missing coverage. Please review.
✅ Project coverage is 76.20%. Comparing base (aaa5569) to head (e3a3351).

Files with missing lines Patch % Lines
internal/gatewayapi/backendsettings.go 90.00% 0 Missing and 1 partial ⚠️
internal/gatewayapi/backendtrafficpolicy.go 90.00% 0 Missing and 1 partial ⚠️
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.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

Comment thread api/v1alpha1/shared_types.go Outdated
zhaohuabing
zhaohuabing previously approved these changes May 25, 2026
Comment thread api/v1alpha1/shared_types.go Outdated
@zhaohuabing
zhaohuabing self-requested a review May 25, 2026 02:48
@muwaqar-cflt muwaqar-cflt changed the title api: split ClusterSettings into BackendClusterSettings api: split ClusterSettings into BackendSettings Jun 10, 2026
@muwaqar-cflt
muwaqar-cflt force-pushed the backend-cluster-settings branch from 74b8411 to 5e69afc Compare June 11, 2026 19:49
@zhaohuabing
zhaohuabing requested a review from arkodg June 16, 2026 13:12
@muwaqar-cflt
muwaqar-cflt force-pushed the backend-cluster-settings branch from 5e69afc to 159b4e2 Compare June 16, 2026 15:44
@muwaqar-cflt muwaqar-cflt changed the title api: split ClusterSettings into BackendSettings api: split ClusterSettings into BackendSettings and ClusterSettings Jun 19, 2026

@zhaohuabing zhaohuabing left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

LGTM thanks!

@zhaohuabing zhaohuabing added this to the v1.9.0-rc.1 Release milestone Jun 22, 2026
@zhaohuabing
zhaohuabing requested review from a team and zhaohuabing June 22, 2026 09:10
@zhaohuabing
zhaohuabing force-pushed the backend-cluster-settings branch from a08ab35 to 9084c06 Compare June 25, 2026 13:48
zhaohuabing
zhaohuabing previously approved these changes Jun 25, 2026
@zhaohuabing
zhaohuabing requested a review from guydc June 25, 2026 14:11
kkk777-7
kkk777-7 previously approved these changes Jul 22, 2026
@kkk777-7

Copy link
Copy Markdown
Member

LGTM, thanks!

muwaqar and others added 5 commits August 5, 2026 10:59
…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>
@muwaqar-cflt
muwaqar-cflt dismissed stale reviews from kkk777-7 and zhaohuabing via e3a3351 August 5, 2026 15:21
@muwaqar-cflt
muwaqar-cflt force-pushed the backend-cluster-settings branch from 9084c06 to e3a3351 Compare August 5, 2026 15:21
@zhaohuabing

Copy link
Copy Markdown
Member

LGTM. Deferring to @guydc

I'm also OK with delaying this to EG API graduating to V1.

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.

api: split ClusterSettings into BackendClusterSettings for CDS-only fields

6 participants