Conversation
✅ Deploy Preview for cerulean-figolla-1f9435 canceled.
|
dd737d9 to
16882c2
Compare
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #8002 +/- ##
==========================================
- Coverage 73.58% 73.57% -0.01%
==========================================
Files 242 242
Lines 37003 37003
==========================================
- Hits 27228 27226 -2
- Misses 7856 7857 +1
- Partials 1919 1920 +1 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
3a6461f to
b56a4b1
Compare
b56a4b1 to
e930544
Compare
|
Hi @zhaohuabing
Client IP SourceIn my opinion, this setting is redundant to [ClientIPDetectionSettings Envoy allows the different XFF configuration for HCM and GeoIP filter, I don't know the exact reason, but there might be use cases that they could be different? Huabing: One possible use case is that different routes may need different XFF settings (e.g., different source headers or different hop counts). That isn’t feasible with ClientTrafficPolicy today since it applies at the Gateway level, not per-route. Another concern with this approach is that it would generate XFF configuration for both the HCM and the GeoIP filter. That could have unintended side effects in cases where HCM-level XFF configuration isn’t desired. That said, I think consolidating this in one place in the control plane is a better UX and helps avoid misconfiguration. An optional XFF configuration in the SecurityPolicy should also be supported to allow route-level configuration. ref:
GeoIP Database sourceThe database source is an infrastructure responsibility, and I don't see that this information should be provided for each Huabing: Access rulesI think configuring access decision with geo location information as part of the This would ensure that the geo information population and the headers to match are entirely within the responsibility of the controller manager. Huabing: What do you think? Huabing:
That feels harder for users to understand and configure correctly. |
|
Hi @zhaohuabing, do you know if this a PR that could be shipped with v1.8.0 ? My team and I are currently benchmarking possible replacement of BTW, thank you very much for your work (you personnaly and Envoy/Gateway teams in general) on this project ! |
|
@funkluk replied to your comment inline(text in italics) to make it easier to follow. |
|
@sboulkour No guarantees, but aiming for v1.8.0 — and it looks very likely if we can agree on the API. |
|
@zhaohuabing seems like we are on the same page, and I fully agree that the drawback is that the config is a bit more cluttered. But on the other hand, it allows IMHO a more flexible usage. Two last remarks:
|
e930544 to
18aaa21
Compare
Signed-off-by: Huabing (Robin) Zhao <zhaohuabing@gmail.com>
ecd3c12 to
8ebe1ff
Compare
| // EnvoyProxyGeoIP defines shared GeoIP provider settings for EnvoyProxy. | ||
| type EnvoyProxyGeoIP struct { | ||
| // Provider defines the GeoIP provider configuration used by GeoIP filter instances. | ||
| Provider GeoIPProvider `json:"provider"` |
There was a problem hiding this comment.
Add a EnvoyProxyGeoIP layer for future-proofing.
api/v1alpha1/authorization_types.go
Outdated
| // | ||
| // +optional | ||
| // +notImplementedHide | ||
| GeoLocation *GeoLocationPrincipal `json:"geoLocation,omitempty"` |
There was a problem hiding this comment.
The. GeoLocation access control can be applied on both the HTTP and TCP layer, depending on the targeted route type.
Signed-off-by: Huabing (Robin) Zhao <zhaohuabing@gmail.com>
8ebe1ff to
4ecbfe9
Compare
207ac9c to
d1b8712
Compare
Signed-off-by: Huabing (Robin) Zhao <zhaohuabing@gmail.com>
d1b8712 to
5b4db72
Compare
API for #4412.
API:
EnvoyProxy.spec.geoIP.provider.ClientTrafficPolicy.spec.clientIPDetectionfor client IP extraction (for example, trusted XFF hops). If route-level settings are required in the future, an override can be added to the SecurityPolicy later.SecurityPolicy.spec.authorization.rules[].principal.geoLocation.Example:
EnvoyProxy.spec.provider.kubernetes.pod.{volumes,container.volumeMounts}.geoipupdateas a sidecar writing into the shared mount.ClientTrafficPolicy.SecurityPolicy.This keeps vendor-specific download logic out of Envoy Gateway while giving users a native way to enforce GeoIP-based policies.
Alternate approach:
Push GeoIP decisions into SecurityPolicy.authorization by matching on the headers the GeoIP filter emits. That technically works (you’d add header conditions under uthorization.rules[].principal.headers), but it’s brittle: users must remember the exact header names, every policy has to duplicate the boilerplate, and any future header renames become breaking. Embedding allow/deny rules inside the geoip block keeps the UX intuitive—configure lookup + provider + enforcement in one place—and lets the controller manage header wiring internally.