fix(kubernetes): apply namespace selector filtering to List operations#8312
fix(kubernetes): apply namespace selector filtering to List operations#8312cnvergence merged 10 commits intoenvoyproxy:mainfrom
Conversation
Signed-off-by: Shahar Harari <shahar.harari@sap.com>
Signed-off-by: Shahar Harari <shahar.harari@sap.com>
Signed-off-by: Shahar Harari <shahar.harari@sap.com>
Signed-off-by: Shahar Harari <shahar.harari@sap.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 #8312 +/- ##
==========================================
+ Coverage 73.73% 73.83% +0.10%
==========================================
Files 241 242 +1
Lines 37077 37046 -31
==========================================
+ Hits 27337 27353 +16
+ Misses 7796 7761 -35
+ Partials 1944 1932 -12 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Signed-off-by: Shahar Harari <shahar.harari@sap.com>
Signed-off-by: Shahar Harari <shahar.harari@sap.com>
|
/retest |
| } | ||
|
|
||
| // Set the filtered items back to the list | ||
| if err := meta.SetList(list, filtered); err != nil { |
There was a problem hiding this comment.
Nit: this can be skipped if the length of the resulting list is the same as the original one.
| // Filter items based on namespace labels | ||
| var filtered []runtime.Object | ||
| for _, item := range items { | ||
| obj, ok := item.(metav1.Object) |
There was a problem hiding this comment.
Nit: Could we cache namespace match results in a map (keyed by namespace) and check that first, to avoid repeatedly calling checkObjectNamespaceLabels for objects in the same namespace?
|
Hi @shahar-h thanks for the fix! This looks good overall. I left a few minor nits for follow-up. |
|
/retest |
|
LGTM, thanks! |
|
Hi @shahar-h — once the conflict is resolved, we should be good to go. |
Signed-off-by: Shahar Harari <shahar.harari@sap.com>
Done |
zhaohuabing
left a comment
There was a problem hiding this comment.
LGTM. Thanks for fixing this!
Signed-off-by: shahar-h <shahar.harari@sap.com>
|
/retest |
|
/retest |
1 similar comment
|
/retest |
|
Can this be merged if there are no further comments? |
envoyproxy#8312) * fix(kubernetes): apply namespace selector filtering to List operations Signed-off-by: Shahar Harari <shahar.harari@sap.com> * disable codecov Signed-off-by: Shahar Harari <shahar.harari@sap.com> * cleanup Signed-off-by: Shahar Harari <shahar.harari@sap.com> * fix(kubernetes): apply namespace selector filtering to List operations Signed-off-by: Shahar Harari <shahar.harari@sap.com> * Update release notes Signed-off-by: Shahar Harari <shahar.harari@sap.com> * Improve coverage Signed-off-by: Shahar Harari <shahar.harari@sap.com> * Fix lint error Signed-off-by: Shahar Harari <shahar.harari@sap.com> * cr fixes Signed-off-by: Shahar Harari <shahar.harari@sap.com> --------- Signed-off-by: Shahar Harari <shahar.harari@sap.com> Signed-off-by: shahar-h <shahar.harari@sap.com>
What this PR does / why we need it:
When using the Kubernetes provider with
NamespaceSelectorwatch mode, namespace filtering afterclient.Listoperations was implemented for some resources (Gateway, xRoute, ReferenceGrant) but was missing for xPolicy resources (SecurityPolicy, BackendTrafficPolicy, ClientTrafficPolicy, etc.). This caused xPolicy resources from all namespaces to be processed during reconciliation, even when those namespaces didn't match the configured selector.This PR centralizes the namespace filtering logic by introducing a
namespaceSelectorClientwrapper that automatically filters List results. This approach:Note: Watch predicates in
watchResourcesstill have their own namespace filtering checks for filtering incoming events.Which issue(s) this PR fixes:
Fixes #8305
Release Notes: Yes