feat: support section name for BackendTrafficPolicy by kkk777-7 · Pull Request #6888 · envoyproxy/gateway

kkk777-7 · 2025-09-02T14:49:23Z

What this PR does / why we need it:
Support section name policy attachment (Gateway Listener/xRoute Rule) for BackendTrafficPolicy.

For simplicity, the merge strategy of policy attachments (include section name) is as follows:

Merging is only performed between resources (gateway/route)
Not supported: merging within a single resource (e.g., gateway/gateway listener, route/route rule)

Which issue(s) this PR fixes:

Fixes #6607

Release Notes: Yes

codecov · 2025-09-02T14:58:25Z

Codecov Report

❌ Patch coverage is 88.97638% with 42 lines in your changes missing coverage. Please review.
✅ Project coverage is 72.13%. Comparing base (21cbec5) to head (68f635b).
⚠️ Report is 13 commits behind head on main.

Files with missing lines	Patch %	Lines
internal/gatewayapi/backendtrafficpolicy.go	88.05%	28 Missing and 10 partials ⚠️
internal/gatewayapi/helpers.go	93.65%	2 Missing and 2 partials ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #6888      +/-   ##
==========================================
+ Coverage   72.01%   72.13%   +0.12%     
==========================================
  Files         230      230              
  Lines       33420    33619     +199     
==========================================
+ Hits        24066    24252     +186     
- Misses       7601     7609       +8     
- Partials     1753     1758       +5

☔ View full report in Codecov by Sentry.
📢 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.

Signed-off-by: kkk777-7 <kota.kimura0725@gmail.com>

internal/gatewayapi/testdata/backendtrafficpolicy-strategic-merge-global-ratelimit.out.yaml

github-actions · 2025-10-07T20:02:13Z

This pull request has been automatically marked as stale because it has not had activity in the last 30 days. Please feel free to give a status update now, ping for review, when it's ready. Thank you for your contributions!

rtheobald · 2025-10-09T16:48:21Z

This is great! I was just trying to use section names to activate proxy protocol on one gateway listener today. Anything I can do to help get this merged?

arkodg · 2025-10-10T21:00:04Z

internal/gatewayapi/route.go

 						Settings: destSettings,
-						Metadata: buildResourceMetadata(udpRoute, nil),
+						// udpRoute Must have a single rule, so can use index 0.
+						Metadata: buildResourceMetadata(udpRoute, udpRoute.Spec.Rules[0].Name),


is this from a different PR ?

yes 🙏 I just rebased onto the latest main.

guydc · 2025-10-13T15:14:53Z

internal/gatewayapi/backendtrafficpolicy.go

 		}

+		// If specified the sectionName must match listenerName from ir listener metadata.
+		if target.SectionName != nil && string(*target.SectionName) != tcp.Metadata.SectionName {


Can we avoid reliance on metadata here? I think that it turns this feature from being user-facing to being more of a core EG feature. I'm concerned that in the long term, we may need to disable metadata for some use cases, so prefer to not depend on it in the translator.

thanks for the good insight.

For listener, it seems we can handle it based on the IR name.
https://github.com/envoyproxy/gateway/blob/main/internal/gatewayapi/testdata/httproute-with-metadata.out.yaml#L142

For xRoute rule, since the IR name uses an index-based format, it seems there’s currently no field other than metadata from which we can obtain the section name.
https://github.com/envoyproxy/gateway/blob/main/internal/gatewayapi/testdata/httproute-with-metadata.out.yaml#L182

It would mean having some redundant field, but what do you think about adding a sectionName field to the IR struct?

Alternatively, we cloud consider using the rule name instead of the index only when a rule name is set.
httproute/default/httproute-1/rule/<rule-name>/match/0/*
(I’m not exactly sure where this change would impact the codebase...)

it may seems that Istio has some code implementing this kind of behavior.
https://github.com/istio/istio/blob/master/pilot/pkg/config/kube/gateway/conversion.go#L105

guydc

Overall, seems structurally correct. Thanks, this is an important change.

guydc · 2025-10-13T16:11:02Z

internal/gatewayapi/backendtrafficpolicy.go

-				gateway, ok := gatewayMap[key]
-				if !ok {
-					continue
+			if currTarget.Kind != resource.KindGateway && currTarget.SectionName != nil {


nit: maybe a comment like "If the target is not a gateway, then it's an xRoute. If the section name is defined, then it's a route rule", or maybe a function like "isRouteRule(currTarget)". Here and in other places.

done : 84b2038

guydc · 2025-10-13T16:36:31Z

internal/gatewayapi/backendtrafficpolicy.go

-					status.SetConditionForPolicyAncestors(&policy.Status,
-						ancestorRefs,
+				parentPolicy := gwPolicy
+				if listenerPolicy != nil {


Maybe mention in the feature doc under current.yaml the order for merging logic, e.g. only one of "GW/Listener would be selected for merge, with listener taking precedence".

thanks! update docs : 0a649f7

guydc · 2025-10-13T16:41:16Z

internal/gatewayapi/backendtrafficpolicy.go

+	status.SetAcceptedForPolicyAncestors(&policy.Status, ancestorRefs, t.GatewayControllerName, policy.Generation)
+
+	// Check if this policy is overridden by other policies targeting at route rule levels
+	key := policyTargetRouteKey{


Can we skip this when the current target is a route rule?

done : 14c2a2b

guydc · 2025-10-13T18:16:10Z

internal/gatewayapi/backendtrafficpolicy.go

-	route RouteContext, xdsIR resource.XdsIRMap, resources *resource.Resources,
+	policy, parentPolicy *egv1a1.BackendTrafficPolicy,
+	target gwapiv1a2.LocalPolicyTargetReferenceWithSectionName,
+	gatewayNN types.NamespacedName, listenerName *gwapiv1a2.SectionName, route RouteContext,


Can we use more descriptive names here, like policyTargetGatewayNN, PolicyTargetListener? Do we need these additional parameters, or can they be taken from target?

I’ve updated the variable names. 14c2a2b

Since translateBackendTrafficPolicyForRouteWithMerge is called by processBTPolicyForRoute, the target at that time refers to xRoute, so my understanding is that we need additional parameters.

guydc · 2025-10-13T18:26:17Z

internal/gatewayapi/backendtrafficpolicy.go

-				r.DNS = tf.DNS
+				// only set attributes which weren't already set by a more
+				// specific policy
+				setIfNil(&r.LoadBalancer, tf.LoadBalancer)


Is this (and in other places) required? A lot of the test files have traffic: {} removed, and I'm not sure what the xds implication of that would be...
Do we expect the final resolved configuration to be ambiguous and need to let the first win?

thanks, good catch!

yeah, to prioritize more specific target policies, the setting is only applied when it hasn’t been set yet.
(process order : route rule -> route -> listener -> gateway)

In previous code, setIfNil was only used in the function corresponding to translateBackendTrafficPolicyForGateway.
but since we now need to consider the route rule level as well, setIfNil is also used in applyTrafficFeatureToRoute.

The issue this time was...
since was also skipping cases where r.DirectResponse was not nil, the behavior ended up differing from the previous output.

I’ve fixed it so that it now matches the previous behavior.

if r.Traffic != nil || r.UseClientProtocol != nil || r.DirectResponse != nil { continue }

Signed-off-by: kkk777-7 <kota.kimura0725@gmail.com>

kkk777-7 · 2025-10-18T19:32:29Z

@guydc
thanks for review !
addressed your suggestion and rebased latest main branch.
please take another look when you have a moment.

Signed-off-by: kkk777-7 <kota.kimura0725@gmail.com>

internal/gatewayapi/backendtrafficpolicy.go

Signed-off-by: kkk777-7 <kota.kimura0725@gmail.com>

guydc

LGTM!

zirain · 2025-10-29T02:16:13Z

internal/gatewayapi/backendtrafficpolicy.go

-						status.Error2ConditionMsg(err),
-					)
-				}
+func (t *Translator) processBTPolicyForRoute(


nit: let's use full name in function.

Suggested change

func (t *Translator) processBTPolicyForRoute(

func (t *Translator) processBackendTrafficPolicyForRoute(

zirain

LGTM, a small nit could be fixed in following PR.

zirain · 2025-10-29T07:40:01Z

/retest

kkk777-7 force-pushed the feat-section-for-btp branch from 63a7932 to 59a4758 Compare September 3, 2025 14:34

kkk777-7 added 3 commits September 6, 2025 15:13

feat: support ir metadata to tcproute/udproute

fde62d2

Signed-off-by: kkk777-7 <kota.kimura0725@gmail.com>

feat: support attaching policy to section name level

fcb6302

Signed-off-by: kkk777-7 <kota.kimura0725@gmail.com>

update crd validation of btp

9dbe1f4

Signed-off-by: kkk777-7 <kota.kimura0725@gmail.com>

kkk777-7 force-pushed the feat-section-for-btp branch from 543779a to c83d688 Compare September 6, 2025 06:58

add e2e test

baf5287

Signed-off-by: kkk777-7 <kota.kimura0725@gmail.com>

kkk777-7 force-pushed the feat-section-for-btp branch from c83d688 to baf5287 Compare September 6, 2025 08:36

kkk777-7 added 2 commits September 6, 2025 18:01

fix golint

b045df7

Signed-off-by: kkk777-7 <kota.kimura0725@gmail.com>

revert no need update func

7fdd859

Signed-off-by: kkk777-7 <kota.kimura0725@gmail.com>

kkk777-7 changed the title ~~Feat section for btp~~ feat: support section name for BackendTrafficPolicy Sep 7, 2025

kkk777-7 commented Sep 7, 2025

View reviewed changes

internal/gatewayapi/testdata/backendtrafficpolicy-strategic-merge-global-ratelimit.out.yaml Show resolved Hide resolved

kkk777-7 marked this pull request as ready for review September 7, 2025 17:03

kkk777-7 requested a review from a team as a code owner September 7, 2025 17:03

arkodg mentioned this pull request Oct 1, 2025

Nominate kkk777-7 as Maintainer #7112

Closed

github-actions bot added the stale label Oct 7, 2025

github-actions bot removed the stale label Oct 9, 2025

arkodg reviewed Oct 10, 2025

View reviewed changes

guydc reviewed Oct 13, 2025

View reviewed changes

kkk777-7 added 5 commits October 18, 2025 18:17

Merge branch 'main' into feat-section-for-btp

5b8a174

Signed-off-by: kkk777-7 <kota.kimura0725@gmail.com>

update gatewayapi test output

73e5fc1

Signed-off-by: kkk777-7 <kota.kimura0725@gmail.com>

add comment

84b2038

Signed-off-by: kkk777-7 <kota.kimura0725@gmail.com>

address review

14c2a2b

Signed-off-by: kkk777-7 <kota.kimura0725@gmail.com>

update btp merge section

0a649f7

Signed-off-by: kkk777-7 <kota.kimura0725@gmail.com>

kkk777-7 added 2 commits October 20, 2025 20:28

perf: make sets when needed in policy processing

6df1a85

Signed-off-by: kkk777-7 <kota.kimura0725@gmail.com>

perf: avoid non need deep equal

07ad036

Signed-off-by: kkk777-7 <kota.kimura0725@gmail.com>

zirain reviewed Oct 23, 2025

View reviewed changes

internal/gatewayapi/backendtrafficpolicy.go Show resolved Hide resolved

kkk777-7 added 2 commits October 25, 2025 15:35

Merge branch 'main' into feat-section-for-btp

d7c06e3

Signed-off-by: kkk777-7 <kota.kimura0725@gmail.com>

update: improve data structure

2e7f995

Signed-off-by: kkk777-7 <kota.kimura0725@gmail.com>

guydc approved these changes Oct 28, 2025

View reviewed changes

Merge branch 'main' into feat-section-for-btp

68f635b

zirain reviewed Oct 29, 2025

View reviewed changes

zirain approved these changes Oct 29, 2025

View reviewed changes

arkodg added this to the v1.6.0-rc.1 Release milestone Oct 30, 2025

arkodg merged commit 810e5a2 into envoyproxy:main Oct 30, 2025
56 of 58 checks passed

This was referenced Oct 30, 2025

fix: merged policy status #7376

Merged

style: update policy function name #7381

Merged

	func (t *Translator) processBTPolicyForRoute(
	func (t *Translator) processBackendTrafficPolicyForRoute(

Conversation

kkk777-7 commented Sep 2, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

codecov bot commented Sep 2, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

Uh oh!

github-actions bot commented Oct 7, 2025

Uh oh!

rtheobald commented Oct 9, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

kkk777-7 Oct 18, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

guydc left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

kkk777-7 Oct 18, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

kkk777-7 commented Oct 18, 2025

Uh oh!

Uh oh!

guydc left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

zirain left a comment

Choose a reason for hiding this comment

Uh oh!

zirain commented Oct 29, 2025

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

kkk777-7 commented Sep 2, 2025 •

edited

Loading

codecov bot commented Sep 2, 2025 •

edited

Loading

kkk777-7 Oct 18, 2025 •

edited

Loading

kkk777-7 Oct 18, 2025 •

edited

Loading