fix: route level policy should override gateway wide compression setting by sudiptob2 · Pull Request #6904 · envoyproxy/gateway

sudiptob2 · 2025-09-08T01:22:20Z

codecov · 2025-09-08T01:31:16Z

Codecov Report

❌ Patch coverage is 66.66667% with 3 lines in your changes missing coverage. Please review.
✅ Project coverage is 71.06%. Comparing base (4e33b31) to head (ac30731).
⚠️ Report is 187 commits behind head on main.

Files with missing lines	Patch %	Lines
internal/gatewayapi/backendtrafficpolicy.go	66.66%	2 Missing and 1 partial ⚠️

Additional details and impacted files

@@           Coverage Diff           @@
##             main    #6904   +/-   ##
=======================================
  Coverage   71.06%   71.06%           
=======================================
  Files         225      225           
  Lines       39871    39878    +7     
=======================================
+ Hits        28336    28341    +5     
  Misses       9863     9863           
- Partials     1672     1674    +2

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

zirain · 2025-09-08T02:53:25Z

will Compression []*Compression json:"compression,omitempty" patchMergeKey:"type" patchStrategy:"replace"`` help on this instead of a custom logic?

sudiptob2 · 2025-09-08T03:52:37Z

Just tried that approach, but it has a couple of issues:

Setting compression: [] is treated as empty so its omitted. As a result, the merge utility sees it as empty and doesn’t override the gateway-wide policy.
If omitempty is removed from the type, then it will always override the gateway-wide policy when nothing is set at the route level. This is also not expected.

Any thoughts on working around these? @zirain

zirain · 2025-09-08T04:14:01Z

Just tried that approach, but it has a couple of issues:

Setting compression: [] is treated as empty so its omitted. As a result, the merge utility sees it as empty and doesn’t override the gateway-wide policy.

If omitempty is removed from the type, then it will always override the gateway-wide policy when nothing is set at the route level. This is also not expected.

Any thoughts on working around these? @zirain

git it, then we need document it Note: to disable compression in route level, set compression to empty array i.e compression: [].

arkodg · 2025-09-08T16:17:48Z

internal/gatewayapi/backendtrafficpolicy_test.go

can we add a yaml test in testdata/

added yaml tests

arkodg · 2025-09-08T16:19:36Z

internal/gatewayapi/backendtrafficpolicy.go

can we avoid this ?

I am not sure if the custom logic is avoidable. Another approach suggested by Zirain to use annotations but it had a couple of isssues - discussed here.

Let me know if you have any other approach in mind.

Using custom logic is not uncommon; merging the ratelimit policy also uses something similar.

gateway/internal/gatewayapi/backendtrafficpolicy.go

Lines 444 to 465 in 4e33b31

// Since GlobalRateLimit merge relies on IR auto-generated key: (<policy-ns>/<policy-name>/rule/<rule-index>)

// We can't simply merge the BTP's using utils.Merge() we need to specifically merge the GlobalRateLimit.Rules using IR fields.

// Since ir.TrafficFeatures is not a built-in Kubernetes API object with defined merging strategies and it does not support a deep merge (for lists/maps).

// Handle rate limit merging cases:

// 1. Both policies have rate limits - merge them

// 2. Only gateway policy has rate limits - preserve gateway policy's rule names

// 3. Only route policy has rate limits - use route policy's rule names (default behavior)

if policy.Spec.RateLimit != nil && gwPolicy.Spec.RateLimit != nil {

// Case 1: Both policies have rate limits - merge them

tfGW, _ := t.buildTrafficFeatures(gwPolicy, resources)

tfRoute, _ := t.buildTrafficFeatures(policy, resources)

if tfGW != nil && tfRoute != nil &&

tfGW.RateLimit != nil && tfRoute.RateLimit != nil {

mergedRL, err := utils.Merge(tfGW.RateLimit, tfRoute.RateLimit, *policy.Spec.MergeType)

if err != nil {

return fmt.Errorf("error merging rate limits: %w", err)

}

// Replace the rate limit in the merged features if successful

tf.RateLimit = mergedRL

the plan is to rm this custom logic

Signed-off-by: sudipto baral <sudiptobaral.me@gmail.com>

… logic Signed-off-by: sudipto baral <sudiptobaral.me@gmail.com>

Signed-off-by: sudipto baral <sudiptobaral.me@gmail.com>

github-actions · 2025-10-10T00:08:00Z

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!

sudiptob2 marked this pull request as ready for review September 8, 2025 02:35

sudiptob2 requested a review from a team as a code owner September 8, 2025 02:35

sudiptob2 force-pushed the fix/6775/merge-policy branch from f64d6ad to 75e1fb4 Compare September 8, 2025 14:28

arkodg reviewed Sep 8, 2025

View reviewed changes

sudiptob2 added 3 commits September 8, 2025 13:15

fix: routepolicy should override compression while merging policies

7d51201

Signed-off-by: sudipto baral <sudiptobaral.me@gmail.com>

test: add yml testdata for overriding compression case in translation…

8b95521

… logic Signed-off-by: sudipto baral <sudiptobaral.me@gmail.com>

test: add docs for disabling compressiong

ac30731

Signed-off-by: sudipto baral <sudiptobaral.me@gmail.com>

sudiptob2 force-pushed the fix/6775/merge-policy branch from 55f29ce to ac30731 Compare September 8, 2025 17:15

sudiptob2 mentioned this pull request Sep 9, 2025

Require Both Type and Compressor Field for Compression in BackendTrafficPolicy config #6924

Closed

sudiptob2 marked this pull request as draft September 9, 2025 20:19

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

sudiptob2 closed this Oct 20, 2025

sudiptob2 deleted the fix/6775/merge-policy branch March 1, 2026 23:26

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix: route level policy should override gateway wide compression setting#6904

fix: route level policy should override gateway wide compression setting#6904
sudiptob2 wants to merge 3 commits intoenvoyproxy:mainfrom
sudiptob2:fix/6775/merge-policy

sudiptob2 commented Sep 8, 2025

Uh oh!

codecov bot commented Sep 8, 2025 •

edited

Loading

Uh oh!

zirain commented Sep 8, 2025

Uh oh!

sudiptob2 commented Sep 8, 2025

Uh oh!

zirain commented Sep 8, 2025

Uh oh!

arkodg Sep 8, 2025

Uh oh!

sudiptob2 Sep 8, 2025

Uh oh!

arkodg Sep 8, 2025

Uh oh!

sudiptob2 Sep 8, 2025

Uh oh!

sudiptob2 Sep 8, 2025

Uh oh!

arkodg Sep 8, 2025

Uh oh!

github-actions bot commented Oct 10, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

	// Since GlobalRateLimit merge relies on IR auto-generated key: (<policy-ns>/<policy-name>/rule/<rule-index>)
	// We can't simply merge the BTP's using utils.Merge() we need to specifically merge the GlobalRateLimit.Rules using IR fields.
	// Since ir.TrafficFeatures is not a built-in Kubernetes API object with defined merging strategies and it does not support a deep merge (for lists/maps).

	// Handle rate limit merging cases:
	// 1. Both policies have rate limits - merge them
	// 2. Only gateway policy has rate limits - preserve gateway policy's rule names
	// 3. Only route policy has rate limits - use route policy's rule names (default behavior)
	if policy.Spec.RateLimit != nil && gwPolicy.Spec.RateLimit != nil {
	// Case 1: Both policies have rate limits - merge them
	tfGW, _ := t.buildTrafficFeatures(gwPolicy, resources)
	tfRoute, _ := t.buildTrafficFeatures(policy, resources)

	if tfGW != nil && tfRoute != nil &&
	tfGW.RateLimit != nil && tfRoute.RateLimit != nil {

	mergedRL, err := utils.Merge(tfGW.RateLimit, tfRoute.RateLimit, *policy.Spec.MergeType)
	if err != nil {
	return fmt.Errorf("error merging rate limits: %w", err)
	}
	// Replace the rate limit in the merged features if successful
	tf.RateLimit = mergedRL

Conversation

sudiptob2 commented Sep 8, 2025

Uh oh!

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

Uh oh!

Codecov Report

Uh oh!

zirain commented Sep 8, 2025

Uh oh!

sudiptob2 commented Sep 8, 2025

Uh oh!

zirain commented Sep 8, 2025

Uh oh!

arkodg Sep 8, 2025

Choose a reason for hiding this comment

Uh oh!

sudiptob2 Sep 8, 2025

Choose a reason for hiding this comment

Uh oh!

arkodg Sep 8, 2025

Choose a reason for hiding this comment

Uh oh!

sudiptob2 Sep 8, 2025

Choose a reason for hiding this comment

Uh oh!

sudiptob2 Sep 8, 2025

Choose a reason for hiding this comment

Uh oh!

arkodg Sep 8, 2025

Choose a reason for hiding this comment

Uh oh!

github-actions bot commented Oct 10, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

codecov bot commented Sep 8, 2025 •

edited

Loading