perf: store extension server resources centrally - #10071
Merged
Merged
Conversation
Signed-off-by: zirain <zirain2009@gmail.com>
✅ Deploy Preview for cerulean-figolla-1f9435 ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #10071 +/- ##
==========================================
+ Coverage 81.31% 81.34% +0.02%
==========================================
Files 264 265 +1
Lines 40994 41050 +56
==========================================
+ Hits 33336 33393 +57
+ Misses 7657 7656 -1
Partials 1 1 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
guydc
previously approved these changes
Sep 23, 2026
guydc
left a comment
guydc
left a comment
Contributor
There was a problem hiding this comment.
Overall LGTM, maybe tests for policy/filter mixing on the same route can be added.
Signed-off-by: zirain <zirain2009@gmail.com>
|
Thank you for this change. We run Envoy Gateway with On a copy of our objects (a few hundred merged Gateways, 766 listeners, about 100 Gateway-targeted policies), three runs each:
The peak after a policy change fell from about 1.9 GiB to 0.25 GiB (process RSS). Before this change, 90% of our heap at rest was copies of these policies in listener Could this be cherry-picked into release-v1.9 and release-v1.8? #10072 is already labelled for both. The attachment of these policies to every merged listener is a separate bug: #10097, fixed in #10099. |
Member
Author
|
@vvoytovych add this to 1.9.2 and 1.8.5 |
|
Thank you! One note for the v1.8.5 pick, from a trial on our side. On |
15 tasks
kkk777-7
added a commit
that referenced
this pull request
Sep 28, 2026
* fix(luavalidator): block getfenv/setfenv/newproxy/module in the Lua security sandbox (#9743) * fix(luavalidator): block getfenv/setfenv/newproxy/module in the Lua sandbox getfenv/setfenv/newproxy/module were not in the blocklist. getfenv(0) returns the global table regardless of `_G = nil`, so that line's comment is inaccurate. No escape is reachable today (io/os originals stay in upvalues, debug is nil), so this is defense-in-depth. Signed-off-by: kanywst <niwatakuma@icloud.com> * docs: add release note for Lua sandbox env hardening Signed-off-by: kanywst <niwatakuma@icloud.com> * docs: note the Lua sandbox hardening as a breaking change Signed-off-by: kanywst <niwatakuma@icloud.com> --------- Signed-off-by: kanywst <niwatakuma@icloud.com> Co-authored-by: Rudrakh Panigrahi <rudrakh97@gmail.com> * fix(ratelimit): apply priorityClassName to the rate limit pod (#9898) rateLimitDeployment.pod.priorityClassName was accepted in EnvoyGateway config but never copied onto the managed Deployment pod spec, so the rate limit pod ignored the configured PriorityClass. Set it the same way the proxy Deployment already does. Fixes #9897 Signed-off-by: Vijay Pal <vijay.pal@nutanix.com> * fix: apply maxDynamicDescriptors at route level (#9974) * fix: apply maxDynamicDescriptors at route level Envoy Gateway sets max_dynamic_descriptors to 10000 only on the HCM-level local_ratelimit filter, which carries no descriptors. The real configuration lives in each route's typed_per_filter_config, and Envoy resolves that per-route config as a whole without inheriting anything from the HCM filter, so every route-level limiter fell back to Envoy's default of 20. A Distinct rule (per client IP, per header value, per query parameter) therefore tracked only the 20 most recently seen values: the 21st evicted the oldest bucket, and an evicted client came back with a full bucket. That made per-client limits on a public listener a coarse layer rather than a bound. Set the same limit on the route-level config, where the wildcard descriptors actually live, and share the constant between both sites. Signed-off-by: Andreea Popescu <andreea.popescu@pidginhost.com> * test(ratelimit): cover and document dynamic bucket retention Signed-off-by: Andreea Popescu <andreea.popescu@pidginhost.com> * chore: number the local rate limit release note Signed-off-by: Andreea Popescu <andreea.popescu@pidginhost.com> * test(ratelimit): pin the limiter headers on 429 CompareRoundTrip compares response headers only for 200 and 204, so the retention subtest's final 429 was attributed by status alone. Assert x-ratelimit-limit and x-ratelimit-remaining on the captured response to tie the rejection to the original user's exhausted bucket. Also reword the constant's comment: the capacity applies per wildcard descriptor in each route configuration, and entries are allocated on demand. Signed-off-by: Andreea Popescu <andreea.popescu@pidginhost.com> --------- Signed-off-by: Andreea Popescu <andreea.popescu@pidginhost.com> Co-authored-by: zirain <zirain2009@gmail.com> * fix(translator): match SNI on every TCP filter chain (#10011) * Revert "fix(tcp): add SNI-based filter chain matching for TLS passthrough empty routes (#8521)" This reverts commit c11f0c2. emitting identical EmptyCluster filter chains -- by collapsing the placeholder chains into a single shared one, rather than by giving them distinct matchers. Collapsing only helps against another placeholder: it does nothing when the colliding chain belongs to a different listener, which is why #9341 and #9516 still reproduce. Its own commit message describes adding SNI matching, but the merged change carries none. Restore the per-listener empty route. The commits that follow add the SNI match the placeholder chains actually need, which keeps them unique on their own. Signed-off-by: Huabing (Robin) Zhao <zhaohuabing@gmail.com> Signed-off-by: Huabing (Robin) Zhao <huabing@tetrate.io> * fix(translator): match SNI on every TCP filter chain Filter chains on a shared xDS listener must each have a distinct match or Envoy rejects the listener and stops accepting updates to it. Placeholder chains for route-less listeners had no match at all, so one sharing a port with a wildcard HTTPS listener produced two `{}` matchers and a NACK. Carry the Gateway listener hostname on the TCP IR listener, for TLS protocol listeners only since TCP ignores hostnames, and resolve the SNI once in addXdsTCPFilterChain, falling back to that hostname whenever the route carries none of its own. Every TCP filter chain is then matched the same way and the placeholder needs no special case. Reinstate the #7866 regression test dropped by the revert, with each listener given a distinct hostname -- listeners sharing a port with identical hostnames are rejected with HostnameConflict before translation, so this is what the Gateway API layer actually emits. It now asserts three chains with three distinct serverNames rather than one shared chain. Co-authored-by: Florian Wiegand <36593900+wiegandf@users.noreply.github.com> Signed-off-by: Florian Wiegand <36593900+wiegandf@users.noreply.github.com> Signed-off-by: Huabing (Robin) Zhao <zhaohuabing@gmail.com> Signed-off-by: Huabing (Robin) Zhao <huabing@tetrate.io> * clean up Signed-off-by: Huabing (Robin) Zhao <huabing@tetrate.io> * fix(translator): name the placeholder filter chain after its listener The placeholder chain for a route-less TCP listener was named after the EmptyCluster it points at, which reads as a cluster in a config dump and goes stale if the placeholder ever targets something else. Name it after the listener that owns it instead, with a /no-routes suffix saying why the chain exists. Signed-off-by: Huabing (Robin) Zhao <zhaohuabing@gmail.com> Signed-off-by: Huabing (Robin) Zhao <huabing@tetrate.io> * test: include SNI hostname in merged gateway fixture Signed-off-by: Huabing (Robin) Zhao <zhaohuabing@gmail.com> --------- Signed-off-by: Huabing (Robin) Zhao <zhaohuabing@gmail.com> Signed-off-by: Huabing (Robin) Zhao <huabing@tetrate.io> Signed-off-by: Florian Wiegand <36593900+wiegandf@users.noreply.github.com> Co-authored-by: Florian Wiegand <36593900+wiegandf@users.noreply.github.com> * fix(gatewayapi): conflict HTTPS and TLS listeners sharing a hostname when gateways are merged (#10013) validateConflictedMergedListeners keys on the listener protocol verbatim, so an HTTPS listener and a TLS listener on the same port with the same hostname land in different buckets and are both accepted. They cannot both be served: listeners sharing a port are told apart by SNI, and here the SNI is the same. Envoy ends up with two filter chains carrying an identical server_names match and rejects the listener, along with every update to it that follows. Key on the protocol class instead, which puts HTTPS and TLS together, so the second listener is marked Conflicted with HostnameConflict. This is what validateConflictedHostnameListeners already does within a single Gateway; merged Gateways were the only path that skipped the check. Also route the conflict through setConflictedConditions rather than setting the condition directly. Every other conflict path already uses it, and it is what sets Accepted=False and Programmed=False on a ListenerSet listener -- without it a conflicted ListenerSet entry reports no Accepted condition at all and a generic ListenersNotValid reason for Programmed. Signed-off-by: Huabing (Robin) Zhao <zhaohuabing@gmail.com> Signed-off-by: Huabing (Robin) Zhao <huabing@tetrate.io> * fix: scope merged cluster settings by backend protocol (#10016) * fix: scope merged cluster settings by backend protocol Signed-off-by: kkk777-7 <kota.kimura0725@gmail.com> * rename release note Signed-off-by: kkk777-7 <kota.kimura0725@gmail.com> --------- Signed-off-by: kkk777-7 <kota.kimura0725@gmail.com> * fix: marshal xDS typed_config deterministically to avoid equivalent-resource churn (#10019) * fix: marshal xDS typed_config deterministically to avoid equivalent-resource churn ToAnyWithValidation wrapped every filter typed_config via anypb.New, which uses the default non-deterministic marshaler. Proto map fields (an access log's json_format, JWT providers, filter metadata) then serialize in a random key order on each translation, so semantically unchanged resources are emitted with different bytes and republished on every reconcile. Marshal deterministically via anypb.MarshalFrom so map keys serialize in a stable order. Add a regression test, since the golden xDS tests compare protojson output (which re-sorts map keys) and cannot catch this. Signed-off-by: nguyenptk <nguyenptk@gmail.com> * docs: add release note for deterministic xDS marshaling fix Signed-off-by: nguyenptk <nguyenptk@gmail.com> --------- Signed-off-by: nguyenptk <nguyenptk@gmail.com> * fix: accept equal-preference cipher groups in TLS settings (#10050) The cipher validation added in v1.8.0 compares each entry of the cipher list against the set of supported cipher names as a whole string. That rejects BoringSSL equal-preference groups such as "[ECDHE-ECDSA-AES128-GCM-SHA256|ECDHE-ECDSA-CHACHA20-POLY1305]", which Envoy accepts and which let the client's own preference break the tie between equally preferred ciphers. This includes the default cipher list documented on the ciphers field itself, so copying the documented defaults into a ClientTrafficPolicy left it not accepted. Unwrap group entries and validate each member, rejecting malformed groups (unclosed, empty, nested) and a separator used outside a group. Fixes #9679 Signed-off-by: Huabing (Robin) Zhao <zhaohuabing@gmail.com> Signed-off-by: Huabing (Robin) Zhao <huabing@tetrate.io> Co-authored-by: zirain <zirain2009@gmail.com> * fix(gatewayapi): allow consistent hash with merge backends (#10052) * feat(gatewayapi): allow consistent hash with merge backends Signed-off-by: Huabing (Robin) Zhao <huabing@tetrate.io> * fix: refresh consistent hash merge fixtures and cleanup Signed-off-by: Huabing (Robin) Zhao <huabing@tetrate.io> * fix: clarify consistent hash merge regression coverage and release note Signed-off-by: Huabing (Robin) Zhao <huabing@tetrate.io> * chore: remove unused BTPLoadBalancerIndex The gateway-level ConsistentHash check in weightedRuleBackendsMustBeInOneCluster was its only consumer, and that check is gone now that consistent hashing works with merged backends. Signed-off-by: Huabing (Robin) Zhao <zhaohuabing@gmail.com> Signed-off-by: Huabing (Robin) Zhao <huabing@tetrate.io> * test(xds): cover consistent hash with merged backend clusters Feed the IR from the gatewayapi mergebackends consistent-hash fixture through the xDS translator, checking that shared clusters get the Maglev policy and hash config and the weighted route keeps its hash policy with use_hash_policy. Signed-off-by: Huabing (Robin) Zhao <zhaohuabing@gmail.com> Signed-off-by: Huabing (Robin) Zhao <huabing@tetrate.io> --------- Signed-off-by: Huabing (Robin) Zhao <huabing@tetrate.io> Signed-off-by: Huabing (Robin) Zhao <zhaohuabing@gmail.com> * perf: store extension server resources centrally (#10071) * perf: store extension server resources centrally Signed-off-by: zirain <zirain2009@gmail.com> * nit Signed-off-by: zirain <zirain2009@gmail.com> * more tests Signed-off-by: zirain <zirain2009@gmail.com> --------- Signed-off-by: zirain <zirain2009@gmail.com> * perf: store upstream CA bundles once per gateway (#10072) * perf: share immutable TLS certificate bytes across IR copies A heap profile of a gateway whose backends validate against a shared CA had 4.75 GB of a 7.45 GB live heap in duplicated certificate bytes. Two multipliers stack: every DestinationSetting gets its own copy of the CA bundle, and the watchable map deep copies the whole ir.Xds once per store and once per subscriber, so each bundle was held once per destination per copy. The bytes are already immutable once translation has produced them. The translator aliases them straight out of the Secret, and the xDS translator only reads them into protobuf InlineBytes, so the copy in the generated deep-copy functions bought nothing. Hand-write DeepCopyInto for TLSCACertificate, TLSCertificate and TLSCrl so a copy shares those slices. controller-gen omits a generated DeepCopyInto when the type already has one, so the generated DeepCopy wrappers and every caller stay as they are. Intern the assembled CA bundle per translation so destinations referencing the same CA share one backing array as well. Deep copying an IR with 1000 destinations sharing a 22KB CA bundle drops from 25.6 MB and 2.7 ms to 1.1 MB and 0.27 ms. Translation allocation on the same shape drops about 29%. Signed-off-by: Huabing (Robin) Zhao <zhaohuabing@gmail.com> Signed-off-by: Huabing (Robin) Zhao <huabing@tetrate.io> * polish Signed-off-by: Huabing (Robin) Zhao <huabing@tetrate.io> * test: build the benchmark CA bundle instead of pasting it The bundle was one certificate repeated twenty times inline in the ConfigMap YAML, which cost about 400 unreadable lines and defined the bundle twice, so nothing kept the translation benchmark and the deep-copy benchmark measuring the same bytes. Build it from a single constant instead. Also name the release note after the PR rather than the issue. Signed-off-by: Huabing (Robin) Zhao <zhaohuabing@gmail.com> Signed-off-by: Huabing (Robin) Zhao <huabing@tetrate.io> * perf: store upstream CA bundles centrally instead of sharing their memory Sharing the certificate bytes between IR copies kept the deep copy at the watchable boundary from duplicating them, but it also meant a byte slice reachable from several snapshots at once, which is harder to reason about and leaves an append on one copy visible to another. Store the bundles centrally instead, the way extension server resources are stored, so each copy stays self-contained. Xds.CACertificates holds one CACertificateEntry per distinct bundle, addressed by a digest of its bytes, and TLSCACertificate carries that digest rather than its own copy. A reference keeps its own Name, so the xDS secret name is unchanged and Envoy sees exactly what it saw before. Where no gateway IR is in scope to register against, such as the ext service and telemetry paths, the bytes stay on the reference and resolution falls back to them. Addressing by content rather than by the owning policy matters because BackendTLSPolicy cannot reference across namespaces, so a cluster trusting one corporate CA is forced to copy it per namespace. Keying on the policy would leave every one of those copies in place. Deep copying an IR with 1000 destinations, each with its own policy trusting the same CA, drops from 25.6 MB and 2.1 ms to 1.1 MB and 0.28 ms. Signed-off-by: Huabing (Robin) Zhao <zhaohuabing@gmail.com> Signed-off-by: Huabing (Robin) Zhao <huabing@tetrate.io> * fix: use the full digest to content-address CA bundles The digest was cut to its first 64 bits, and a match reused the existing entry without comparing the bytes behind it. Two bundles on one gateway sharing that prefix would silently give the second destination the first bundle as its trusted CA. A gateway can carry bundles from tenants that do not trust each other, so the truncation is not worth the shorter name. Signed-off-by: Huabing (Robin) Zhao <zhaohuabing@gmail.com> Signed-off-by: Huabing (Robin) Zhao <huabing@tetrate.io> * fix: key the resolved CA cache by the resource, not its secret name A Backend and a BackendTLSPolicy both name their CA secret <name>/<namespace>-ca, so one of each sharing a name in a namespace produced the same cache key. The cache was also written from the merged config, which cannot tell which of the two the CA came from. A route to the Backend translated first therefore let the policy skip reading its own refs and adopt the Backend's CA. Key the cache by the resource's kind, namespace and name instead, and write it where the CA is resolved rather than after the merge. Cache the digest rather than the entry, so a destination rejected before its bundle is registered leaves nothing for a later lookup to reuse. The Backend path now takes the same shortcut the policy path already had, which is what makes the kind in the key meaningful. Signed-off-by: Huabing (Robin) Zhao <zhaohuabing@gmail.com> Signed-off-by: Huabing (Robin) Zhao <huabing@tetrate.io> --------- Signed-off-by: Huabing (Robin) Zhao <zhaohuabing@gmail.com> Signed-off-by: Huabing (Robin) Zhao <huabing@tetrate.io> Signed-off-by: zirain <zirain2009@gmail.com> Co-authored-by: zirain <zirain2009@gmail.com> * fix(translator): compute GeoIP header removals once per listener (#10107) * [envoy-gateway] Reuse listener GeoIP header removals [ci changed_files] Signed-off-by: Cooper Gamble <cooper@openai.com> * [envoy-gateway] Add GeoIP translation performance release note [ci changed_files] Signed-off-by: Cooper Gamble <cooper@openai.com> --------- Signed-off-by: Cooper Gamble <cooper@openai.com> Co-authored-by: zirain <zirain2009@gmail.com> * fix: count UDP proxy sessions in shutdown drain (#10110) fix: count UDP proxy sessions in shutdown drain (#10109) The shutdown manager only summed listener downstream_cx_active, which never includes UDP proxy sessions, so a UDP-only proxy exited as soon as the minimum drain period passed. Also count udp.*.downstream_sess_active. Signed-off-by: Huabing (Robin) Zhao <zhaohuabing@gmail.com> Signed-off-by: Huabing (Robin) Zhao <huabing@tetrate.io> * test: regenerate the merged consistent-hash cluster golden (#10115) Marking merged backend clusters as route clusters stopped them carrying a cluster-level hash policy, since a route cluster gets its hash policy from the route action instead. The golden was not regenerated at the time, so TestTranslateXds and gen-check fail on main. Consistent hashing is unaffected: the route still carries the source IP hash policy and both clusters keep their maglev load balancing policy. Signed-off-by: Huabing (Robin) Zhao <zhaohuabing@gmail.com> Signed-off-by: Huabing (Robin) Zhao <huabing@tetrate.io> * [cooper-09] fix(jsonpatch): batch JSONPath matches (#10118) * [envoy-gateway] Batch JSONPath matches in one apply [ci changed_files] Signed-off-by: Cooper Gamble <cooper@openai.com> * fix: generate testdata Signed-off-by: kkk777-7 <kota.kimura0725@gmail.com> * fix: security fixes (#10130) * security fix: GHSA-gmx8-fhfh-mgjx (#35) Signed-off-by: Guy Daich <guy.daich@sap.com> * fix: check for nil pod-name key in Token Review extra field (#41) Signed-off-by: Karol Szwaj <karol.szwaj@gmail.com> Co-authored-by: Karol Szwaj <karol.szwaj@gmail.com> * fix: allow to disable EnvoyProxy patch with RuntimeFlag (#37) Signed-off-by: zirain <zirain2009@gmail.com> * fix: a panic during shutdown (#33) Signed-off-by: zirain <zirain2009@gmail.com> * release notes Signed-off-by: zirain <zirain2009@gmail.com> * fix PR number Signed-off-by: zirain <zirain2009@gmail.com> --------- Signed-off-by: Guy Daich <guy.daich@sap.com> Signed-off-by: Karol Szwaj <karol.szwaj@gmail.com> Signed-off-by: zirain <zirain2009@gmail.com> Co-authored-by: Guy Daich <guy.daich@sap.com> Co-authored-by: Karol Szwaj <karol.szwaj@gmail.com> * add: envoy proxy warn condition Signed-off-by: kkk777-7 <kota.kimura0725@gmail.com> --------- Signed-off-by: kanywst <niwatakuma@icloud.com> Signed-off-by: Vijay Pal <vijay.pal@nutanix.com> Signed-off-by: Andreea Popescu <andreea.popescu@pidginhost.com> Signed-off-by: Huabing (Robin) Zhao <zhaohuabing@gmail.com> Signed-off-by: Huabing (Robin) Zhao <huabing@tetrate.io> Signed-off-by: Florian Wiegand <36593900+wiegandf@users.noreply.github.com> Signed-off-by: kkk777-7 <kota.kimura0725@gmail.com> Signed-off-by: nguyenptk <nguyenptk@gmail.com> Signed-off-by: zirain <zirain2009@gmail.com> Signed-off-by: Cooper Gamble <cooper@openai.com> Signed-off-by: Guy Daich <guy.daich@sap.com> Signed-off-by: Karol Szwaj <karol.szwaj@gmail.com> Co-authored-by: kt <kanywst12@gmail.com> Co-authored-by: Rudrakh Panigrahi <rudrakh97@gmail.com> Co-authored-by: Vijaypal(NAI) <vijay.pal@nutanix.com> Co-authored-by: andreeapid <andreea.popescu@pidginhost.com> Co-authored-by: zirain <zirain2009@gmail.com> Co-authored-by: Huabing (Robin) Zhao <zhaohuabing@gmail.com> Co-authored-by: Florian Wiegand <36593900+wiegandf@users.noreply.github.com> Co-authored-by: Nguyên (Harry) <nguyenptk@gmail.com> Co-authored-by: cooper-oai <cooper@openai.com> Co-authored-by: Guy Daich <guy.daich@sap.com> Co-authored-by: Karol Szwaj <karol.szwaj@gmail.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
fixes: #10065