Skip to content

[xds] stabilize listener-level Lua XDS filters to avoid listener drain#8598

Merged
rudrakhp merged 2 commits intoenvoyproxy:mainfrom
arkodg:fix-drain-lua
Mar 26, 2026
Merged

[xds] stabilize listener-level Lua XDS filters to avoid listener drain#8598
rudrakhp merged 2 commits intoenvoyproxy:mainfrom
arkodg:fix-drain-lua

Conversation

@arkodg
Copy link
Copy Markdown
Contributor

@arkodg arkodg commented Mar 25, 2026

Use one disabled HCM Lua filter per Lua slot index instead of per EnvoyExtensionPolicy. The slot/index logic is needed because https://www.envoyproxy.io/docs/envoy/latest/api-v3/extensions/filters/http/lua/v3/lua.proto#envoy-v3-api-msg-extensions-filters-http-lua-v3-luaperroute only supports one sourceCode at a time

Route-specific scripts still use LuaPerRoute, so multi-Lua ordering is preserved while adding new single-Lua policies no longer churns the listener filter chain once slot 0 already exists.

Use one disabled HCM Lua filter per Lua slot index instead of per EnvoyExtensionPolicy.
The slot/index logic is needed because https://www.envoyproxy.io/docs/envoy/latest/api-v3/extensions/filters/http/lua/v3/lua.proto#envoy-v3-api-msg-extensions-filters-http-lua-v3-luaperroute only supports one sourceCode at a time

Route-specific scripts still use LuaPerRoute, so multi-Lua ordering is preserved while adding new single-Lua policies
no longer churns the listener filter chain once slot 0 already exists.

Co-authored-by: Codex <noreply@openai.com>
Signed-off-by: Arko Dasgupta <arkodg@gmail.com>
Copilot AI review requested due to automatic review settings March 25, 2026 22:50
@netlify
Copy link
Copy Markdown

netlify Bot commented Mar 25, 2026

Deploy Preview for cerulean-figolla-1f9435 canceled.

Name Link
🔨 Latest commit 3b16503
🔍 Latest deploy log https://app.netlify.com/projects/cerulean-figolla-1f9435/deploys/69c4a401317c8f0008fd891d

Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Stabilizes listener-level Lua HTTP filter naming in the XDS translator by allocating disabled HCM Lua filters per Lua slot index (rather than per policy-derived name), so adding/removing EnvoyExtensionPolicy objects does not churn the listener filter chain while preserving per-route multi-Lua ordering via LuaPerRoute.

Changes:

  • Generate one disabled HCM Lua filter per maximum Lua list index used across routes (envoy.filters.http.lua/<idx>), instead of per policy-derived Lua name.
  • Bind route-level LuaPerRoute configs to those stable slot-indexed filter names to preserve ordering.
  • Update IR/XDS golden test outputs to reflect the new stable filter naming.

Reviewed changes

Copilot reviewed 2 out of 4 changed files in this pull request and generated no comments.

File Description
internal/xds/translator/lua.go Switches Lua HCM filter generation and per-route binding to stable slot-index naming.
internal/ir/xds.go Updates Lua IR field comment to remove outdated translator behavior reference.
internal/xds/translator/testdata/out/xds-ir/lua.routes.yaml Updates per-route typedPerFilterConfig keys to envoy.filters.http.lua/<idx>.
internal/xds/translator/testdata/out/xds-ir/lua.listeners.yaml Updates listener httpFilters names to stable slot-indexed Lua filters and deduplicates filters.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@codecov
Copy link
Copy Markdown

codecov Bot commented Mar 25, 2026

Codecov Report

❌ Patch coverage is 73.68421% with 5 lines in your changes missing coverage. Please review.
✅ Project coverage is 74.32%. Comparing base (1d66589) to head (3b16503).
⚠️ Report is 54 commits behind head on main.

Files with missing lines Patch % Lines
internal/xds/translator/lua.go 73.68% 3 Missing and 2 partials ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #8598      +/-   ##
==========================================
+ Coverage   74.13%   74.32%   +0.18%     
==========================================
  Files         242      243       +1     
  Lines       37749    38147     +398     
==========================================
+ Hits        27987    28353     +366     
+ Misses       7807     7805       -2     
- Partials     1955     1989      +34     

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

Comment thread internal/xds/translator/lua.go Outdated
@zhaohuabing
Copy link
Copy Markdown
Member

Nice hack. It does make the Envoy config a bit harder to read, though. Curious how often policy renames/adds without maximum lua list length change happen in practice?

Also, I just realized @rudrakhp suggested a similar optimization in #8355, and I previously turned it down. Sorry about that.

zhaohuabing
zhaohuabing previously approved these changes Mar 26, 2026
Copy link
Copy Markdown
Member

@zhaohuabing zhaohuabing left a comment

Choose a reason for hiding this comment

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

LGTM. Thanks!

Co-authored-by: Huabing (Robin) Zhao <zhaohuabing@gmail.com>
Signed-off-by: Arko Dasgupta <arkodg@users.noreply.github.com>
@arkodg arkodg requested a review from a team as a code owner March 26, 2026 03:11
@arkodg arkodg requested a review from zhaohuabing March 26, 2026 03:12
@arkodg
Copy link
Copy Markdown
Contributor Author

arkodg commented Mar 26, 2026

Nice hack. It does make the Envoy config a bit harder to read, though. Curious how often policy renames/adds without maximum lua list length change happen in practice?

Also, I just realized @rudrakhp suggested a similar optimization in #8355, and I previously turned it down. Sorry about that.

ah I missed it too :) , codex helped here, reread your comment about the breaking change around xds naming, we should avoid releasing in a patch release, and make it part of v1.8

Copy link
Copy Markdown
Member

@rudrakhp rudrakhp left a comment

Choose a reason for hiding this comment

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

LGTM!
QQ: Do we not guarantee backward compatibility for filter names across minor versions?

@rudrakhp rudrakhp merged commit 80b2762 into envoyproxy:main Mar 26, 2026
57 of 59 checks passed
@arkodg
Copy link
Copy Markdown
Contributor Author

arkodg commented Mar 26, 2026

LGTM! QQ: Do we not guarantee backward compatibility for filter names across minor versions?

technically we dont, we do say in https://gateway.envoyproxy.io/docs/tasks/extensibility/envoy-patch-policy/ that patching xDS will always be unstable

we try to avoid it especially for common resources that get patched like the listener and route
patching a lua filter is uncommon, so imo this shouldnt negatively impact most users

skos-ninja pushed a commit to skos-ninja/envoy-gateway that referenced this pull request May 1, 2026
envoyproxy#8598)

* [xds] stabilize listener-level Lua XDS filters to avoid listener drain

Use one disabled HCM Lua filter per Lua slot index instead of per EnvoyExtensionPolicy.
The slot/index logic is needed because https://www.envoyproxy.io/docs/envoy/latest/api-v3/extensions/filters/http/lua/v3/lua.proto#envoy-v3-api-msg-extensions-filters-http-lua-v3-luaperroute only supports one sourceCode at a time

Route-specific scripts still use LuaPerRoute, so multi-Lua ordering is preserved while adding new single-Lua policies
no longer churns the listener filter chain once slot 0 already exists.

Co-authored-by: Codex <noreply@openai.com>
Signed-off-by: Arko Dasgupta <arkodg@gmail.com>

* Update internal/xds/translator/lua.go

Co-authored-by: Huabing (Robin) Zhao <zhaohuabing@gmail.com>
Signed-off-by: Arko Dasgupta <arkodg@users.noreply.github.com>

---------

Signed-off-by: Arko Dasgupta <arkodg@gmail.com>
Signed-off-by: Arko Dasgupta <arkodg@users.noreply.github.com>
Co-authored-by: Codex <noreply@openai.com>
Co-authored-by: Huabing (Robin) Zhao <zhaohuabing@gmail.com>
Signed-off-by: Jake Oliver <jake@truelayer.com>
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.

4 participants