Repository navigation
Conversation
Contributor
Go Test Coverage: 60.5%Patch coverage: 83.3% (10/12 changed lines covered) See the workflow run for the full per-package breakdown and downloadable HTML report. |
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #24044 +/- ##
==========================================
- Coverage 62.12% 56.77% -5.35%
==========================================
Files 1068 1069 +1
Lines 129929 129997 +68
==========================================
- Hits 80716 73808 -6908
- Misses 41710 49345 +7635
+ Partials 7503 6844 -659 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
Contributor
There was a problem hiding this comment.
🟡 Changes recommended
The peering regression case needs an actual peered intention to verify preservation of XFCC-based authorization.
1 open finding
What changed in this PR
Adds an opt-in sidecar setting to reduce XFCC metadata extraction overhead without changing default behavior.
Changes:
- Adds configuration parsing and conditional metadata-filter insertion.
- Adds parsing tests and resource goldens with and without peer trust bundles.
- Documents usage, metadata limitations, and override behavior.
| File | Description |
|---|---|
| agent/xds/testdata/secrets/http-public-listener-disable-xfcc-source-metadata.latest.golden | Records unchanged secrets. |
| agent/xds/testdata/secrets/http-public-listener-disable-xfcc-source-metadata-with-peer-trust-bundles.latest.golden | Records secrets for peering case. |
| agent/xds/testdata/routes/http-public-listener-disable-xfcc-source-metadata.latest.golden | Records route response. |
| agent/xds/testdata/routes/http-public-listener-disable-xfcc-source-metadata-with-peer-trust-bundles.latest.golden | Records peering route response. |
| agent/xds/testdata/listeners/http-public-listener-disable-xfcc-source-metadata.latest.golden | Shows metadata filter omitted. |
| agent/xds/testdata/listeners/http-public-listener-disable-xfcc-source-metadata-with-peer-trust-bundles.latest.golden | Shows opt-out with peer trust bundles. |
| agent/xds/testdata/endpoints/http-public-listener-disable-xfcc-source-metadata.latest.golden | Records unchanged endpoints. |
| agent/xds/testdata/endpoints/http-public-listener-disable-xfcc-source-metadata-with-peer-trust-bundles.latest.golden | Records peering-case endpoints. |
| agent/xds/testdata/clusters/http-public-listener-disable-xfcc-source-metadata.latest.golden | Records unchanged clusters. |
| agent/xds/testdata/clusters/http-public-listener-disable-xfcc-source-metadata-with-peer-trust-bundles.latest.golden | Records peering-case clusters. |
| agent/xds/resources_test.go | Adds two opt-out scenarios. |
| agent/xds/listeners.go | Conditionally adds metadata filter. |
| agent/xds/config/config.go | Defines and interprets the option. |
| agent/xds/config/config_test.go | Tests parsing and protocol preservation. |
| .changelog/24044.txt | Documents option and caveats. |
🧠 Review effort: Balanced
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Contributor
There was a problem hiding this comment.
🟢 Approval recommended
The narrowly scoped opt-in change preserves default behavior and peered authorization, with supporting regression coverage and no unresolved blocking issues.
No open issues 🎉
1 resolved since last review
Review effort: Balanced
…ader parsing at sidecar for the service.
mrgupta7
force-pushed
the
mridul/xfcc-skip-metadata
branch
from
October 27, 2026 09:27
f2106e3 to
6e8d472
Compare
This branch has not been deployed
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
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.
Add
envoy_disable_xfcc_source_metadatato skip the XFCC metadata filter on sidecarsWhy
On every request to an HTTP service, the sidecar's public listener runs the
header_to_metadatafilter. It applies five regexes to the wholex-forwarded-client-cert(XFCC) header (about 2 KB) to fill theconsul.*source metadata (trust domain,partition,namespace,datacenter,service). In our profiling this filter was about 55% of sidecar CPU per request.Turning off XFCC for the whole mesh (
sanitizeXForwardedClientCert: true) is not an option for many setups. It breaks peered L7 intentions, ext_authz/Lua/Wasm/access logs that readconsul.*, and XFCC forwarding from terminating gateways. This PR adds an opt-in switch that removes only the metadata filter, per sidecar.What changed
agent/xds/config/config.goenvoy_disable_xfcc_source_metadata(default: unset, so no change in behaviour).ProxyConfig.XFCCSourceMetadataDisabled()(strconv.ParseBool; anything it does not accept counts as off).agent/xds/listeners.go(makeInboundListener)parseXFCCToDynamicMetaHTTPFilter()is only called when the key is not set to true.config_test.go: parsing cases (bool, string, junk) and a table test that a bad value never dropsprotocol.resources_test.go: two golden cases, flag on and flag on with peer trust bundles (10 new files underagent/xds/testdata/).Behaviour
When the key is
truefor an HTTP service:header_to_metadatafilter is not added to the public listener.APPEND_FORWARD). RBAC for peered callers matches the header directly and is unaffected.consul.*metadata is empty on that sidecar, so ext_authz, Lua, Wasm and%DYNAMIC_METADATA(consul:...)%access logs see no values. Do not set it on services that use them.Where it can be set:
proxy-defaultsConfig(ProxyDefaults CRDspec.config)consul.hashicorp.com/proxy-config-map: '{"envoy_disable_xfcc_source_metadata": true}'proxy { config { ... } }in the service registrationCSL-17779