Skip to content

feat: add MergeBackends option for backend cluster deduplication#9477

Open
muwaqar-cflt wants to merge 127 commits into
envoyproxy:mainfrom
muwaqar:backend-cluster-dedup-rebase
Open

feat: add MergeBackends option for backend cluster deduplication#9477
muwaqar-cflt wants to merge 127 commits into
envoyproxy:mainfrom
muwaqar:backend-cluster-dedup-rebase

Conversation

@muwaqar-cflt

@muwaqar-cflt muwaqar-cflt commented Jul 11, 2026

Copy link
Copy Markdown

What this PR does / why we need it:

Adds an opt-in EnvoyProxy.spec.mergeBackends option: routes referencing the identical backend can share a single Envoy cluster instead of Envoy Gateway generating one cluster per route rule. This reduces xDS size, active health-check traffic, and stats cardinality on gateways with many routes fanning out to a small set of shared backends. Disabled by default and marked experimental.

Cluster sharing is skipped wherever it would be unsafe:

  • multi-backendRef rules using session persistence, ConsistentHash load balancing, or a fallback backend (all need the full backend pool kept together in one cluster, not split per identity)
  • dynamic-resolver/custom (extension-provided) backends
  • a backendRef carrying any per-backendRef filter (e.g. CredentialInjection, header modification) or a UseClientProtocol override, both of which are baked into the shared cluster rather than the per-route destination
  • a route-level BackendTrafficPolicy cluster setting, or a RoutingType diverging from the gateway's baseline
  • TCP/UDP/TLS rules with more than one backendRef, since those route types have no way to represent a rule split across more than one cluster

Approach: enabling this required restructuring how a route's backend data is stored. Previously, every RouteDestination embedded its own copy of the resolved DestinationSettings, so two routes sharing a backend could never share anything at the IR level - deduplication had nowhere to attach. This PR introduces a per-gateway backend cluster registry (ir.Xds.Backends, ir.BackendCluster): each distinct backend is resolved once, and every RouteDestination holds only a BackendClusterRef (a name) into that registry. RouteDestination.Settings is removed once BackendClusterRefs is the sole source.

Beyond enabling MergeBackends, this registry is what makes a future backend-level BackendTrafficPolicy straightforward: since a backend's settings now live in exactly one place instead of being copied into every route that references it, a policy targeting the backend directly can be applied once at the registry entry, rather than needing to be threaded through and reconciled across every route-level copy.

Reviewer notes: this is a large diff; start here:

  • api/v1alpha1/envoyproxy_types.go - the new user-facing MergeBackends/MergeBackendsConfig field
  • internal/ir/xds.go - the BackendCluster/BackendClusterRef types and the Xds.Backends registry
  • internal/gatewayapi/route.go - processBackendRef is the single entry point every route type (and the RequestMirror filter) uses to resolve a backendRef's destination and cluster assignment together. It wraps resolveBackendCluster (decides the cache key/name/merge outcome), shouldMergeBackend (owns the full eligibility decision), mergeIncompatibleForWeightedRule/mergeIncompatibleForSingleClusterRule (the per-rule-level exclusions, split by whether the route type supports weighted-cluster route actions), and getOrCreateBackendCluster (the find-or-create registry lookup)
  • internal/xds/translator/backend_registry.go - registry resolution (getBackendClusters, singleResolvedClusterName) consumed by the HTTP/GRPC route-action and TCP/UDP listener builders, and cluster.go's cluster-per-setting/weight logic

A good portion of the remaining diff is golden testdata: internal/gatewayapi/testdata/*.out.yaml fixtures re-derived to reflect destinations serializing as backendClusterRefs + a top-level backends: registry instead of inline settings:, plus new fixtures (mergebackends-* in internal/gatewayapi/testdata, merge-backends-* in internal/xds/translator/testdata) covering the merge-eligibility rules above at both the IR-building layer and the xDS translation layer - several of the latter are derived directly from the former's real output, tracing a single Gateway API resource all the way through to the generated Envoy config.

Which issue(s) this PR fixes:
Fixes #5307

@netlify

netlify Bot commented Jul 11, 2026

Copy link
Copy Markdown

Deploy Preview for cerulean-figolla-1f9435 ready!

Name Link
🔨 Latest commit 7c0a69e
🔍 Latest deploy log https://app.netlify.com/projects/cerulean-figolla-1f9435/deploys/6a603c1c857d940008231477
😎 Deploy Preview https://deploy-preview-9477--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.

muwaqar-cflt pushed a commit to muwaqar/gateway that referenced this pull request Jul 11, 2026
Signed-off-by: Muhammad Waqar <waqar.hameed08@gmail.com>
@muwaqar-cflt

Copy link
Copy Markdown
Author

/retest

@codecov

codecov Bot commented Jul 12, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 93.49112% with 44 lines in your changes missing coverage. Please review.
✅ Project coverage is 75.79%. Comparing base (265fdeb) to head (7c0a69e).
⚠️ Report is 11 commits behind head on main.

Files with missing lines Patch % Lines
internal/xds/translator/translator.go 81.81% 4 Missing and 6 partials ⚠️
internal/gatewayapi/backendtrafficpolicy.go 93.27% 5 Missing and 3 partials ⚠️
internal/gatewayapi/route.go 97.24% 5 Missing and 3 partials ⚠️
internal/xds/translator/route.go 82.97% 4 Missing and 4 partials ⚠️
internal/gatewayapi/clienttrafficpolicy.go 90.00% 2 Missing and 2 partials ⚠️
internal/ir/xds.go 87.87% 2 Missing and 2 partials ⚠️
internal/gatewayapi/runner/runner.go 0.00% 1 Missing ⚠️
internal/xds/translator/backend_registry.go 96.96% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #9477      +/-   ##
==========================================
+ Coverage   75.53%   75.79%   +0.26%     
==========================================
  Files         252      253       +1     
  Lines       41646    42174     +528     
==========================================
+ Hits        31457    31967     +510     
- Misses       8062     8067       +5     
- Partials     2127     2140      +13     

☔ 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.

muwaqar-cflt pushed a commit to muwaqar/gateway that referenced this pull request Jul 13, 2026
Signed-off-by: Muhammad Waqar <waqar.hameed08@gmail.com>
@muwaqar-cflt
muwaqar-cflt force-pushed the backend-cluster-dedup-rebase branch from d5cbd23 to 0f5efab Compare July 13, 2026 00:32
Comment thread internal/gatewayapi/route.go
@muwaqar-cflt
muwaqar-cflt marked this pull request as ready for review July 13, 2026 09:23
@muwaqar-cflt
muwaqar-cflt requested a review from a team as a code owner July 13, 2026 09:23

@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: 6011972ddb

ℹ️ 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 internal/xds/translator/route.go Outdated
Comment thread internal/gatewayapi/backendtrafficpolicy.go
Comment thread internal/gatewayapi/backendtrafficpolicy.go Outdated
@zhaohuabing

Copy link
Copy Markdown
Member

@codex review

@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: 31fbfa0fa5

ℹ️ 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".

Comment thread internal/gatewayapi/route.go Outdated
Comment thread api/v1alpha1/envoyproxy_types.go Outdated
Comment thread internal/gatewayapi/route.go Outdated
Comment thread internal/gatewayapi/backendtrafficpolicy.go Outdated
Comment thread api/v1alpha1/envoyproxy_types.go Outdated
//
// +optional
// +kubebuilder:default=false
Enabled *bool `json:"enabled,omitempty"`

@zhaohuabing zhaohuabing Jul 17, 2026

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.

We may want to default this to true eventually. The fallback approach won't break current user-facing routing behaviors, and it helps with the Envoy memory issue.

It may be too risky to turn this on by default in 1.9, as it does break EPPs and extension servers, but we could keep it opt-in for the first release and consider flipping the default in 1.10 after we have more coverage and user feedback.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

resolving because not seeing an action item for now.

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.

Let's keep it unresolved for now, so other reviewer can see and discuss on this. cc @arkodg @guydc

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

is this flag needed ? mergeBackends: {} should imply we are turning on the feature

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.

yeah, not necessarily. A bool flag is more explicit than mergeBackends: {}, I'm fine with both.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

i like an explicit flag, mergeBackends: {} looks odd

@zhaohuabing
zhaohuabing requested a review from a team July 17, 2026 08:59
Comment thread internal/gatewayapi/backendtrafficpolicy.go Outdated
@cnvergence

Copy link
Copy Markdown
Member

lint fails :(

Comment thread internal/gatewayapi/backendtrafficpolicy.go Outdated
Comment thread internal/gatewayapi/route.go
@zhaohuabing

Copy link
Copy Markdown
Member

This PR looks good. Great work!

I left a few minor comments.

@muwaqar-cflt Could you please fix the link and check whether all e2e tests pass? I’d like to get this merged before 1.9.0 if possible. Thanks!

muwaqar-cflt pushed a commit to muwaqar/gateway that referenced this pull request Jul 17, 2026
Per maintainer feedback on PR envoyproxy#9477, ship only the Enabled boolean for the
first release. Fallback (the only supported behavior) is now unconditional;
Force was already rejected by CEL validation and had no implementation, so
there was no user-visible behavior to preserve by keeping the enum. A
Mode/MergeBackendsMode field can be reintroduced later if a concrete Force
use case emerges.

Regenerated deepcopy, CRDs, and docs; dropped mode: Fallback from all
mergebackends-* input fixtures and their derived golden output; replaced the
CEL validation cases exercising Mode with Enabled-only equivalents.

Signed-off-by: Muhammad Waqar <waqar.hameed08@gmail.com>
muwaqar-cflt pushed a commit to muwaqar/gateway that referenced this pull request Jul 17, 2026
A merged cluster is already scoped to one gateway, so a gateway-level BTP
setting applies uniformly to every route sharing it and can never cause the
divergence this check exists to catch — worth a one-line note for future
readers, per review feedback on PR envoyproxy#9477.

Signed-off-by: Muhammad Waqar <waqar.hameed08@gmail.com>
…r_test.go

cluster.go: the three reverted asClusterArgs signatures (UDP/TCP/HTTP) had
collapsed from main's multi-line parameter formatting to single-line during
Task 9's revert, and HTTPRouteTranslator.asClusterArgs's struct literal had
statName/metadata swapped relative to main's field order. Restore both to
match main exactly. Also reformats the new (no main counterpart)
mergedHTTPRouteTranslator.asClusterArgs's signature the same way, for
consistency with its siblings.

cluster_test.go: four TestBuildClusterWithBackendUtilization* tests had
settings/endpointType field order swapped relative to main, left over from
Task 11 converting their xdsClusterArgs{backendCluster: ...} literals to flat
fields. Restore main's field order.

Signed-off-by: Muhammad Waqar <mwaqar@confluent.io>
Comment thread internal/gatewayapi/backendtrafficpolicy.go
…, not per-route

Merged cluster construction previously ran inside each HTTP/TCP/UDP route's own
cluster-building loop, relying on addXdsCluster's existing-name skip to
deduplicate across routes sharing one backend. Since a merged cluster has no
Traffic settings of its own, this meant whichever route happened to build it
first silently decided the cluster's circuitBreaker/healthCheck/timeout/etc,
non-deterministically (see envoyproxy#9477 (comment)).

Until BackendCluster gets a real Traffic field of its own (pending
clarification from EG maintainers on whether merged clusters should inherit
Gateway-level BackendTrafficPolicy settings), stop guessing: build every
merged cluster in a single new processBackendClusters pass over xdsIR.Backends,
using a stateless BackendClusterTranslator (renamed from
mergedHTTPRouteTranslator, and no longer route-scoped) that never applies any
route-specific Traffic. Removed the now-redundant per-route merged-cluster
loops from the HTTP, TCP, and UDP listener processing blocks, since
processBackendClusters already covers every merged backend regardless of
which route type registered it. Also moved merged-cluster client-cert
processing into the same pass, since it was doing the same per-route-instead-
of-per-cluster work (harmless before, since addXdsSecret dedupes by name, but
still redundant).

Extracted the shared "does this destination need its own route-scoped
cluster" guard (len(Settings) > 0 || len(BackendClusterRefs) == 0 - verified
by testing that dropping either half breaks a real scenario: the Settings
check for mixed merge/non-merge rules, the BackendClusterRefs check for the
placeholder-destination edge case) into needsRouteCluster, used identically
across HTTP/TCP/UDP instead of duplicated three times. Restructured the HTTP
cluster-building block so this check wraps the whole NeedsClusterPerSetting
if/else, rather than being nested only inside one branch.

Signed-off-by: Muhammad Waqar <mwaqar@confluent.io>
…y need backendClusterIndex

resolveBackendClusters (and everything that had to call it) was a method on
*Translator purely to reach t.backendIndex, forcing buildXdsRoute,
buildXdsRouteAction, buildXdsURLRewriteAction, and processSDSClusters to
either become Translator methods or take a whole *Translator parameter, even
though none of them use anything else on Translator. Convert
resolveBackendClusters to a plain function taking backendClusterIndex
directly (already an existing, immutable-per-Translate()-call type), and
change every one of those functions to take backendIndex backendClusterIndex
instead of t *Translator, declaring exactly what they need. Callers now pass
t.backendIndex explicitly at the one call site where t is genuinely in scope
for other reasons.

Restored processSDSClusters to main's original inline-loop structure (no
scanSettings closure) by extracting a routeDestinationSettings helper that
combines a RouteDestination's own Settings with its resolved backends'
Settings into one slice, so the HTTP/TCP loop bodies stay a single
range-and-check block instead of two separate scan calls per route.

Also: dropped the now-redundant "merged" qualifier throughout
backend_registry.go (everything reachable through backendClusterIndex is
inherently merged by construction, so calling it out repeatedly added no
information) and renamed a few functions for clarity:
mergedBackendClusterRef -> backendClusterRef, allDestinationSettings ->
routeDestinationSettings, mergedHTTPRouteTranslator -> BackendClusterTranslator
(from an earlier commit on this branch), tcpDestinationClusterName ->
singleClusterDestinationName (it's used for TCP, UDP, and TLS routes alike,
not just TCP). Trimmed several doc comments that had drifted into
caller-perspective phrasing ("so callers can...").

Signed-off-by: Muhammad Waqar <mwaqar@confluent.io>
…deltas

- cluster.go: remove BackendClusterTranslator's isRoute: true field. It's
  dead: ExtraArgs.timeout is never set for merged clusters, so the only
  reader (hasTimeoutArgs) can never see args.timeout != nil for this type.
- translator.go: restore main's multi-line processXdsCluster call
  formatting and the tSettings intermediate variable in the HTTP
  per-setting loop, restore the "1:1 between IR UDPRoute and xDS Cluster"
  comment, and drop the redundant "Skip a fully merged destination..."
  comments at the needsRouteCluster guards.
- translator_test.go: revert the tracing-invalid expected error back to
  "validation failed for xds resource", matching main. Translate()'s
  joined error contains both messages, so either substring passes the
  test's require.Contains check.
- route_test.go: restore the dropped Settings field on
  TestBuildXdsURLRewriteAction_AppendXForwardedHost's fixture, matching
  main.
- utils_test.go: remove TestCreateExtServiceXDSCluster, a new test for
  createExtServiceXDSCluster, a function this PR never modifies.

Signed-off-by: Muhammad Waqar <mwaqar@confluent.io>
… terminology

Rename mergedClusters -> backendClusters and replace the three-way weighted-
routing condition with a single clusterCount tally (backend clusters plus one
if Settings is non-empty), gated on clusterCount > 1. Share the single-cluster
return path between the one-backend-cluster and zero-backend-cluster cases via
a clusterName default instead of two near-identical return statements.

Move applyWeightedClusterFilters after its only caller, buildXdsWeightedRouteAction.

Comments no longer use "merged"/"non-merged" as adjectives; they describe the
two data sources (Settings, backend clusters) directly.

Signed-off-by: Muhammad Waqar <mwaqar@confluent.io>
… additions

- helpers.go: switch IsMergeBackendsEnabled to inline comments, matching its
  sibling IsMergeGatewaysEnabled's established style instead of a doc comment.
- backendtrafficpolicy_test.go: drop a cross-reference to
  mergeIncompatibleForWeightedRule from TestBTPLoadBalancerIndexIsConsistentHash's
  doc comment.

Reviewed contexts.go, backendtrafficpolicy.go, translator.go, translator_test.go,
runner/runner.go, helpers_test.go, and filters.go against main; all other
MergeBackends additions there are purely additive and already consistent with
house style. filters.go's xdsIR/gatewayCtx/btpRoutingType threading is a
mechanical consequence of route.go's processDestination signature and will be
revisited when route.go gets its own pass.

Signed-off-by: Muhammad Waqar <mwaqar@confluent.io>
…edback

Addresses several review threads from PR envoyproxy#9477 on the performance/naming of
the merge-eligibility machinery:

- BuildBTPIndexes replaces BuildBTPRoutingTypeIndex + the prior
  BuildBTPClusterSettingsIndex/BuildBTPLoadBalancerIndex pair, building all
  three BackendTrafficPolicy indexes (grouped in a new BTPIndexes struct) in
  a single pass over btps, resolving each policy's targets at most once
  instead of once per index. mergeBackendsEnabled gates ClusterSettings/
  LoadBalancer per-BTP internally (moot when no accepted gateway can enable
  merging), while RoutingType applies unconditionally - collapsing two
  separate outer `if` gates in Translate() into one direct call.
- Fixed a pre-existing bug in the LoadBalancer index: like RoutingType, it
  stores a real value (whether LoadBalancer.Type == ConsistentHash) that can
  legitimately differ between BTPs targeting the same gateway, but unlike
  RoutingType it had no first-write-wins guard, so the last-processed policy
  silently won instead of the highest-priority one.
- Added btpRoutingTypeIndexMaps/btpClusterSettingsIndexMaps/
  btpLoadBalancerIndexMaps constructors for consistency across all three
  index types.
- resolveRoutingContext -> resolveRuleBackendContext, with a doc comment
  reflecting everything it actually resolves (gateway, BTP routing type,
  and cluster-settings signal), not just "routing context".
- resolveBackendCluster now builds backendClusterIdentity itself, only on
  the merge-eligible path, instead of processBackendRef building it
  unconditionally before the merge decision is even made.
- Documented the gatewayCtx == nil branch in resolveBackendCluster.

Signed-off-by: Muhammad Waqar <mwaqar@confluent.io>
…x stale refs

- Remove backendClusterRefBuilder: its .add() method was a trivial append
  with no other logic, so all 5 call sites (HTTP/GRPC/TLS/UDP/TCP) now just
  append directly to a plain []*ir.BackendClusterRef.
- shouldMergeBackend: restructure the RoutingType-divergence check to match
  the "if X { return false }" shape used by every other check in the
  function, ending with an explicit return true.
- Fix two comments pointing at mergedBackendClusterRef/getOrCreateBackendCluster
  in ways that had gone stale or were unnecessary cross-references
  (mergedBackendClusterRef no longer exists anywhere - it was renamed to
  backendClusterRef in a different package earlier on this branch).
- Trim a comment in processBackendRef that named two sibling helper
  functions to explain a two-step name assignment; state the fact instead.
- Drop the "Kind is left empty here..." comment on resolveBackendCluster's
  non-merge branch - it raised a question (why doesn't this collide?) that
  it didn't answer.

Signed-off-by: Muhammad Waqar <mwaqar@confluent.io>
…work, address route.go review feedback

Implements the two agreed P0 items from the route.go performance/structure
review, plus a round of follow-up consistency fixes:

- Collapse resolveBackendCluster/ResolvedBackendCluster/getOrCreateBackendCluster's
  three-way split into a single resolveBackendClusterKey (nil = not eligible)
  folded into processBackendRef, which now returns *ir.BackendClusterRef
  directly (nil for the non-merge case) instead of an intermediate cluster
  struct the caller had to unpack at all 5 call sites. BackendClusterKey is
  now only ever constructed on the actual merge-eligible path - the non-merge
  case, the common one when MergeBackends is off, no longer allocates one
  and throws it away.
- getOrCreateBackendCluster no longer takes a separate clusterName param
  (computed internally from key); it also now overwrites the shared
  Setting's Name to match the BackendCluster's own name rather than
  whichever route-scoped name ds carried in with, fixing a real bug the
  reordering above introduced and caught via the full test suite (merged
  clusters were briefly getting route-scoped destination names).
- Exit earlier when merging is disabled: mergeIncompatibleForWeightedRule/
  mergeIncompatibleForSingleClusterRule's computation (and the
  hasRouteLevelClusterSettings BTP index lookup it depends on) is now
  skipped entirely unless isMergeBackendsEnabledForGateway is true, at all
  5 route-type call sites (HTTP/GRPC/TLS/UDP/TCP) for consistency, even
  though TLS/UDP/TCP's variant is cheap enough that this saves little on
  its own.
- resolveRuleBackendContext only resolves gatewayCtx/btpRoutingType now
  (needed unconditionally, regardless of MergeBackends); the merge-only
  hasRouteLevelClusterSettings lookup moved to its own method, computed
  lazily inside the isMergeBackendsEnabledForGateway guard alongside the
  BTPLoadBalancerIndex/ConsistentHash lookup it's parallel to.
- mergeIncompatibleForSingleClusterRule is now a method on *Translator
  (was a free function) and takes backendRefs directly rather than a
  precomputed count, matching mergeIncompatibleForWeightedRule's shape.
- Consistent argument order across mergeIncompatibleForWeightedRule/
  ForSingleClusterRule/hasRouteLevelClusterSettings/
  weightedRuleBackendsMustBeInOneCluster: gatewayCtx, routeCtx, parentRef,
  routeRuleName, backendRefs, then the rest. RouteContext params renamed
  route -> routeCtx throughout this call chain.
- Removed backendClusterRefBuilder: its only method was a trivial append
  with no other logic, so all 5 call sites now append directly to a plain
  []*ir.BackendClusterRef.
- Added toBackendObjectReferences, a small generic helper projecting
  HTTPBackendRef/GRPCBackendRef/BackendRef slices down to their common
  BackendObjectReference, replacing duplicated flattening loops at 5
  call sites.
- Various comment trims (dropped stale/dangling cross-references, redundant
  "merged" qualifiers now that BackendCluster is inherently merged-only)
  and structural cleanup (var-block grouping in the HTTP/GRPC preambles,
  consistent blank-line spacing) carried over from the earlier
  processHTTPRouteRules pass to processGRPCRouteRules and friends.

Signed-off-by: Muhammad Waqar <mwaqar@confluent.io>
The comment explained why udpRoute.Spec.Rules[0] is safe to index, but sat
next to a later, unrelated use of the already-computed routeRuleMetadata
value instead of near the Spec.Rules[0] indexing itself.

Signed-off-by: Muhammad Waqar <mwaqar@confluent.io>
…lvedMetricSinks

Found while running the full lint suite: the loop copied each 136-byte
ir.ResolvedMetricSink by value per iteration. Iterate by index and take a
pointer instead - unrelated to the MergeBackends work on this branch, but
was already failing `make lint` at HEAD.

Signed-off-by: Muhammad Waqar <mwaqar@confluent.io>
@muwaqar-cflt

muwaqar-cflt commented Jul 22, 2026

Copy link
Copy Markdown
Author

commenting on #9477 (comment) (cc @arkodg)

The two priority items are done, plus one more:

  1. Keep the default path local — done. Non-merged destinations use Settings again and never touch BackendCluster/BackendClusterMap (the design also changed since this comment: BackendCluster is now scoped to the merged case only, see the other thread). BackendClusterMap is lazily initialized; ResolvedBackendCluster/routeScopedAdded no longer exist.
  2. Exit earlier when merging is disabled — done. shouldMergeBackend checks gateway enablement first; the merge-incompatibility checks (and the BTP index lookups they depend on) are skipped entirely per-rule when merging is off for that gateway.
  3. Deduplicate earlier in xDS translation — done. Merged clusters are now built exactly once from xdsIR.Backends (already deduplicated by construction), no linear scan involved.

… rules' backends

A TLSRoute has no per-rule match criteria, so all rules' backendRefs pool
into a single Envoy cluster destination. Merge eligibility was previously
decided per rule using only that rule's own backendRefs, so two rules each
with a single distinct backend both looked safe to merge independently,
producing two separate merged clusters where only one could ever be
referenced downstream and silently dropping traffic to the other.

Compute the distinct backend identities across all of a TLSRoute's rules
once per parentRef and treat more than one distinct backend as merge-
incompatible for every rule, in a new distinctBackendObjectReferences
helper reusable by TCP/UDP if their single-rule restriction is ever
relaxed. Also extracts resolveBTPRoutingType out of the now-removed
resolveRuleBackendContext so gatewayCtx is resolved once per parentRef
instead of once per rule.

Signed-off-by: Muhammad Waqar <mwaqar@confluent.io>
@zhaohuabing

Copy link
Copy Markdown
Member

Nice! The number of changed files is much smaller now.

… overrides

Signed-off-by: Muhammad Waqar <mwaqar@confluent.io>
Signed-off-by: Muhammad Waqar <mwaqar@confluent.io>
Signed-off-by: Muhammad Waqar <mwaqar@confluent.io>
…icy HTTP1 override

Signed-off-by: Muhammad Waqar <mwaqar@confluent.io>
Signed-off-by: Muhammad Waqar <mwaqar@confluent.io>
…ge-eligibility fields

Signed-off-by: Muhammad Waqar <mwaqar@confluent.io>
Per review feedback (envoyproxy#9477 (comment)),
the MergeGateways+MergeBackends combination adds complexity around a mode
(MergeGateways) that may eventually be deprecated in favor of ListenerSet.
Rather than supporting the combination, disallow it via CEL validation
(separate commit).

Drops irBackendClusterName's mergeGateways-conditional suffixing of a merged
cluster's rendered name and the MergeGatewaysBackends e2e conformance test
and its testdata/fixtures, which only existed to cover this combination.

Signed-off-by: Muhammad Waqar <mwaqar@confluent.io>
…ia CEL

Per review feedback on PR envoyproxy#9477, MergeGateways and MergeBackends should
never be enabled together - add a CEL rule on EnvoyProxySpec rejecting
that combination, and cover it in the CEL validation test suite.

Signed-off-by: Muhammad Waqar <mwaqar@confluent.io>
@muwaqar-cflt

Copy link
Copy Markdown
Author

found a bug when investigating some changes for this PR: #9541

metadata:
namespace: default
name: httproute-1
name: httproute-2

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.

Why this changed?

metadata:
namespace: envoy-gateway
name: httproute-1
name: httproute-6

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.

Why this changed?

metadata:
namespace: default
name: httproute-3
name: httproute-4

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.

Why this changed?

metadata:
namespace: default
name: httproute-1
name: httproute-3

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.

Why this changed?

@zhaohuabing

Copy link
Copy Markdown
Member

found a bug when investigating some changes for this PR: #9541

Could we address this in a separate PR? That would make the change easier to review and allow us to backport the fix to an earlier release if necessary. We could merge that PR first, then proceed with this one.

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.

Revisit Kubernetes Services mapping to Envoy v3.Cluster

5 participants