feat(api): add ClientTrafficPolicy.clientIPDetection.downstreamRemoteAddress for L4-transparent geoip by sboulkour · Pull Request #8956 · envoyproxy/gateway

sboulkour · 2026-05-10T14:03:43Z

What type of PR is this?
feat

What this PR does / why we need it:

Adds ClientTrafficPolicy.spec.clientIPDetection.downstreamRemoteAddress as an explicit third client-IP source for GeoIP authorization.

When this field is set, Envoy Gateway emits the GeoIP filter without xff_config and without custom_header_config, so Envoy uses the immediate downstream connection source address.

This enables SecurityPolicy.authorization.rules[].principal.clientIPGeoLocations in L4-transparent topologies (for example AWS NLB target-type: instance + externalTrafficPolicy: Local, Azure Standard Load Balancer).

Per maintainer feedback on #8955, behavior is explicit opt-in (new field), not implicit fallback from missing clientIPDetection.

Changes

API: add DownstreamRemoteAddressSettings and clientIPDetection.downstreamRemoteAddress.
API validation: enforce strict one-of (xForwardedFor / customHeader / downstreamRemoteAddress) via CEL size() == 1.
Validator: keep requiring clientIPDetection for GeoIP auth and add a defensive runtime exactly-one check.
Translator: add explicit DownstreamRemoteAddress branch in GeoIP filter generation.
Tests: unit + CEL + gatewayapi/xDS golden coverage, including nil/empty/multi-mode rejection and downstream-remote-address acceptance.
Docs and release notes updated.

Which issue(s) this PR fixes:
Fixes #8955

Release Notes: Yes

Today, SecurityPolicy authorization.rules[].principal.clientIPGeoLocations requires ClientTrafficPolicy.spec.clientIPDetection to be set. This makes GeoIP authorization unreachable in L4-transparent topologies where the downstream TCP peer IS the real client IP (e.g. AWS NLB with target-type=instance + externalTrafficPolicy=Local, Azure Standard LB), because there is no XFF/custom header to trust. Envoy's HTTP geoip filter (envoy.extensions.filters.http.geoip.v3.Geoip) supports three IP-source modes: xff_config, custom_header_config, and "neither set" -- in which case the filter uses "the immediate downstream connection source address" (per the proto). EG already exposes the first two modes; this PR exposes the third. Changes: * internal/gatewayapi/securitypolicy.go: validateAuthorizationGeoIP no longer rejects nil clientIPDetection. The TrustedCIDRs check is kept and guarded. * internal/xds/translator/geoip.go: comment-only update; the existing code already gated xff_config/custom_header_config on a non-nil ClientIPDetection, so no behavioral change is needed there. * Tests: unit + golden coverage for the nil-clientIPDetection case (gatewayapi translator and xDS translator). The xDS golden confirms the geoip filter is emitted with neither xff_config nor custom_header_config, and that no original_ip_detection_extensions are added to the HCM. * Docs and release notes updated. Fixes envoyproxy#8955 Signed-off-by: Salim Boulkour <salim.boulkour@algolia.com>

netlify · 2026-05-10T14:03:48Z

✅ Deploy Preview for cerulean-figolla-1f9435 ready!

Name	Link
🔨 Latest commit	`a0a50a5`
🔍 Latest deploy log	https://app.netlify.com/projects/cerulean-figolla-1f9435/deploys/6a04323774534d0008ef7e95
😎 Deploy Preview	https://deploy-preview-8956--cerulean-figolla-1f9435.netlify.app
📱 Preview on mobile	Toggle QR Code... Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify project configuration.

Per maintainer feedback on envoyproxy#8955, replace the implicit "nil clientIPDetection ⇒ use downstream remote address" behavior introduced in the previous commit with an explicit new field: ClientTrafficPolicy.spec.clientIPDetection.downstreamRemoteAddress. Triggering the new behavior on absence of clientIPDetection was a fail-open footgun: an operator forgetting to set XFF behind a NATing proxy would silently trust the wrong IP for geolocation. The explicit field makes the operator's intent unambiguous. API: * Add DownstreamRemoteAddressSettings (empty struct, room to grow) and the DownstreamRemoteAddress field on ClientIPDetectionSettings. * Update the CEL XValidation rule to enforce mutual exclusion across all three siblings (xForwardedFor, customHeader, downstreamRemoteAddress). Validator: * Restore the rejection of nil clientIPDetection in validateAuthorizationGeoIP. The new mode requires an explicit non-nil ClientIPDetection with DownstreamRemoteAddress set. Translator: * Add an explicit switch arm in buildHCMGeoIPFilter that emits the geoip filter with neither xff_config nor custom_header_config when DownstreamRemoteAddress is set, so Envoy uses its documented default (the immediate downstream connection source address). HCM-side wiring needs no change: useRemoteAddress remains true since no original_ip_detection_extensions are produced. Tests: * Restore the gatewayapi golden testdata for the rejection case (byte-identical to upstream/main). * Add positive gatewayapi + xDS golden coverage for the new mode. * Update CEL validation tests for the new mutual-exclusion message and add positive/negative cases for downstreamRemoteAddress. Docs and release notes updated to describe the explicit field. Signed-off-by: Salim Boulkour <salim.boulkour@algolia.com>

…moteAddress field These golden files render the ClientTrafficPolicy CRD inside the helm chart and need to reflect the new downstreamRemoteAddress field plus the updated CEL mutual-exclusion message. Caught by 'make gen-check'. Signed-off-by: Salim Boulkour <salim.boulkour@algolia.com>

Tighten ClientTrafficPolicy.clientIPDetection validation from "at most one" to "exactly one" of {xForwardedFor, customHeader, downstreamRemoteAddress}. This rejects an empty object (`{}`) and aligns behavior with the explicit opt-in requirement for GeoIP source selection. Also add a defensive runtime validation in authorization GeoIP translation so stale resources (or bypassed admission validation) are rejected unless exactly one mode is configured. Tests updated: * gatewayapi unit tests: reject empty and multi-mode clientIPDetection. * CEL validation tests: updated error message and added empty-object rejection case. * regenerated CRD/docs/helm golden outputs for the new CEL rule and message. Signed-off-by: Salim Boulkour <salim.boulkour@algolia.com>

sboulkour · 2026-05-11T12:18:37Z

Follow-up pushed in f2a990233: clientIPDetection is now strict one-of.

CEL rule changed from <= 1 to == 1 with message: exactly one of xForwardedFor, customHeader, or downstreamRemoteAddress must be set.
This rejects clientIPDetection: {} (empty object) and any multi-mode combination.
Added defensive runtime check in validateAuthorizationGeoIP to require exactly one mode as a safeguard for stale/bypassed objects.
Updated tests: gatewayapi unit (empty + multi-mode rejected), CEL validation (message + empty-object case), and regenerated CRD/helm/docs artifacts accordingly.

sboulkour · 2026-05-11T12:26:26Z

@zhaohuabing could you please enable CI for this first-time contributor PR and review when you have a moment?

I updated the PR to the explicit opt-in design with strict one-of clientIPDetection (xForwardedFor / customHeader / downstreamRemoteAddress), added nil/empty/multi-mode rejection tests, and regenerated artifacts.

Thank you!

codecov · 2026-05-11T12:37:24Z

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 74.75%. Comparing base (fa5d4dd) to head (f2a9902).
⚠️ Report is 1 commits behind head on main.

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #8956      +/-   ##
==========================================
+ Coverage   74.73%   74.75%   +0.02%     
==========================================
  Files         251      251              
  Lines       40394    40404      +10     
==========================================
+ Hits        30188    30205      +17     
+ Misses       8138     8132       -6     
+ Partials     2068     2067       -1

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

sboulkour · 2026-05-11T13:41:39Z

@zhaohuabing thx for the CI run :)

About the failed test, seems like the the gateway didn't get an IP within the 3min deadline.
~~Would you re-run the test ? Could be flacky, otherwise I don't know why this one failed specifically.~~

sboulkour · 2026-05-11T13:42:41Z

/retest

Per reviewer feedback, DownstreamRemoteAddress is too Envoy-internal. DirectRemoteAddress better describes the user-facing intent: use the direct TCP peer address as the client IP. Also add breaking change release note entry for the strict one-of CEL validation on clientIPDetection. Signed-off-by: Salim Boulkour <salim.boulkour@algolia.com>

Signed-off-by: Salim Boulkour <salim.boulkour@algolia.com>

sboulkour · 2026-05-11T14:31:23Z

@zhaohuabing ok pushed required changes. ready for a new CI run :)

…dress Signed-off-by: Salim Boulkour <salim.boulkour@algolia.com>

sboulkour · 2026-05-11T14:50:59Z

/cc @arkodg @rudrakhp — you both reviewed the original GeoIP implementation PRs (#8002, #8453). Would appreciate your review on this follow-up.

The sed-based rename accidentally changed downstreamRemoteAddressWithoutPortCelFormatter in ratelimit.go, which is a rate-limiting internal constant unrelated to the directRemoteAddress GeoIP feature. Revert to original name. Signed-off-by: Salim Boulkour <salim.boulkour@algolia.com>

zhaohuabing · 2026-05-13T08:11:48Z


 # Changes that are expected to cause an incompatibility with previous versions, such as deletions or modifications to existing APIs.
 breaking changes: |
+  `ClientTrafficPolicy.spec.clientIPDetection` now requires exactly one of `xForwardedFor`, `customHeader`, or `directRemoteAddress` to be set. Previously an empty `clientIPDetection: {}` was accepted; it is now rejected by CEL validation.


This is an API-breaking change, but I think it should be acceptable since an empty clientIPDetection is not meaningful and is unlikely to be used in existing setups. cc @envoyproxy/gateway-maintainers

sboulkour requested a review from a team as a code owner May 10, 2026 14:03

sboulkour changed the title ~~feat(gatewayapi): allow clientIPGeoLocations without clientIPDetection~~ feat(api): add ClientTrafficPolicy.clientIPDetection.downstreamRemoteAddress for L4-transparent geoip May 11, 2026

sboulkour mentioned this pull request May 11, 2026

Allow clientIPGeoLocations without clientIPDetection.xForwardedFor #8955

Open

sboulkour added 2 commits May 11, 2026 09:47

zhaohuabing reviewed May 11, 2026

View reviewed changes

Comment thread api/v1alpha1/clienttrafficpolicy_types.go Outdated

zhaohuabing reviewed May 11, 2026

View reviewed changes

Comment thread api/v1alpha1/clienttrafficpolicy_types.go Outdated

sboulkour force-pushed the geoip-allow-no-clientipdetection branch from bedc422 to c59c988 Compare May 11, 2026 14:24

fix(docs): fix broken anchor link for DirectRemoteAddressSettings

a3b3b6b

Signed-off-by: Salim Boulkour <salim.boulkour@algolia.com>

fix(test): rename stale test case name downstream -> direct remote ad…

d9257fe

…dress Signed-off-by: Salim Boulkour <salim.boulkour@algolia.com>

github-actions Bot requested a review from rudrakhp May 11, 2026 14:51

sboulkour mentioned this pull request May 12, 2026

[release/v1.8] v1.8.0 release docs and announcement #8942

Merged

zhaohuabing reviewed May 13, 2026

View reviewed changes

Comment thread internal/xds/translator/ratelimit.go Outdated

zhaohuabing reviewed May 13, 2026

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(api): add ClientTrafficPolicy.clientIPDetection.downstreamRemoteAddress for L4-transparent geoip#8956

feat(api): add ClientTrafficPolicy.clientIPDetection.downstreamRemoteAddress for L4-transparent geoip#8956
sboulkour wants to merge 8 commits into
envoyproxy:mainfrom
sboulkour:geoip-allow-no-clientipdetection

sboulkour commented May 10, 2026 •

edited

Loading

Uh oh!

netlify Bot commented May 10, 2026 •

edited

Loading

Uh oh!

sboulkour commented May 11, 2026

Uh oh!

sboulkour commented May 11, 2026

Uh oh!

codecov Bot commented May 11, 2026

Uh oh!

sboulkour commented May 11, 2026 •

edited

Loading

Uh oh!

sboulkour commented May 11, 2026

Uh oh!

Uh oh!

Uh oh!

sboulkour commented May 11, 2026

Uh oh!

sboulkour commented May 11, 2026 •

edited

Loading

Uh oh!

Uh oh!

zhaohuabing May 13, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Conversation

sboulkour commented May 10, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

netlify Bot commented May 10, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

✅ Deploy Preview for cerulean-figolla-1f9435 ready!

Uh oh!

sboulkour commented May 11, 2026

Uh oh!

sboulkour commented May 11, 2026

Uh oh!

codecov Bot commented May 11, 2026

Codecov Report

Uh oh!

sboulkour commented May 11, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

sboulkour commented May 11, 2026

Uh oh!

Uh oh!

Uh oh!

sboulkour commented May 11, 2026

Uh oh!

sboulkour commented May 11, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

zhaohuabing May 13, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

sboulkour commented May 10, 2026 •

edited

Loading

netlify Bot commented May 10, 2026 •

edited

Loading

sboulkour commented May 11, 2026 •

edited

Loading

sboulkour commented May 11, 2026 •

edited

Loading