fix: alway include the controller namespace in the watched namespaces - #9100
Conversation
✅ Deploy Preview for cerulean-figolla-1f9435 ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 065e3c15af
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #9100 +/- ##
==========================================
+ Coverage 74.76% 74.81% +0.04%
==========================================
Files 252 252
Lines 40687 40756 +69
==========================================
+ Hits 30421 30490 +69
Misses 8186 8186
Partials 2080 2080 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
bb64f89 to
ea80207
Compare
ef94fcc to
2931bfc
Compare
|
@codex review |
| - key: kubernetes.io/metadata.name | ||
| operator: In | ||
| values: | ||
| - btp-cross-ns-denied |
There was a problem hiding this comment.
Remove the explicitly configured envoy-gateway-system - it should always be included by default to watch Infra resources.
Signed-off-by: Huabing Zhao <zhaohuabing@gmail.com>
Signed-off-by: Huabing Zhao <zhaohuabing@gmail.com> (cherry picked from commit 84be503d855e968b7b6999d476a6897e98d4e330)
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 3fa526706a
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
There was a problem hiding this comment.
Pull request overview
This PR fixes a regression in Kubernetes namespace-scoped watch modes by ensuring the Envoy Gateway controller namespace is always included, so the controller can consistently reconcile its own infrastructure resources (e.g., Envoy Services/Deployments/Secrets) even when users restrict watched namespaces via an explicit list or a namespace label selector.
Changes:
- Always add the controller namespace to the controller-runtime cache’s
DefaultNamespaceswhen using explicit namespace list watch mode. - Extend namespace-selector-based list filtering and watch predicates to bypass user selectors for the controller namespace.
- Update docs, API reference text, test configuration, and release notes to reflect the new behavior.
Reviewed changes
Copilot reviewed 15 out of 15 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| test/config/envoy-gateaway-config/watch-namespaces.yaml | Switches test config to NamespaceSelector-based watching without explicitly listing the controller namespace. |
| site/content/en/v1.8/tasks/operations/gateway-namespace-mode.md | Documents that the controller namespace is always included in watched namespaces. |
| site/content/en/v1.8/tasks/operations/deployment-mode.md | Clarifies controller namespace inclusion for reconciling managed infra resources. |
| site/content/en/v1.8/api/extension_types.md | Updates API reference text to state controller namespace is always included. |
| site/content/en/latest/tasks/operations/gateway-namespace-mode.md | Same documentation update for “latest”. |
| site/content/en/latest/tasks/operations/deployment-mode.md | Same documentation update for “latest”. |
| site/content/en/latest/api/extension_types.md | Same API reference update for “latest”. |
| release-notes/current.yaml | Adds a bug-fix release note describing the controller-namespace inclusion behavior. |
| internal/provider/kubernetes/predicates.go | Ensures controller-namespace events bypass namespace-selector filtering. |
| internal/provider/kubernetes/predicates_test.go | Adds test coverage validating controller-namespace inclusion for List-based infra lookups. |
| internal/provider/kubernetes/namespace_selector_client.go | Adds an “included namespaces” bypass set (used for controller namespace) for namespace-selector filtered List operations. |
| internal/provider/kubernetes/namespace_selector_client_test.go | Extends tests to cover included-namespace bypass behavior. |
| internal/provider/kubernetes/kubernetes.go | Ensures cache includes controller namespace in explicit namespace list mode. |
| internal/provider/kubernetes/controller.go | Wraps the reconciler client so the controller namespace is always included under NamespaceSelector watch mode. |
| api/v1alpha1/envoygateway_types.go | Updates API comments to document controller namespace inclusion in both watch modes. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Signed-off-by: Huabing Zhao <zhaohuabing@gmail.com>
| // When configured with an explicit namespace watch list, scope the default | ||
| // cache to those namespaces and add type-specific controller namespace | ||
| // exceptions below. | ||
| if svrCfg.EnvoyGateway.WatchesNamespaces() { |
There was a problem hiding this comment.
This and the above if svrCfg.EnvoyGateway.GatewayNamespaceMode() { code block are starting to get a bit messy. For easier review, I keep that block untouched in this PR, and we can clean it up in a follow-up PR.
Signed-off-by: Huabing (Robin) Zhao <zhaohuabing@gmail.com>
|
@codex review |
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> Signed-off-by: Huabing (Robin) Zhao <zhaohuabing@gmail.com>
Signed-off-by: Huabing Zhao <zhaohuabing@gmail.com>
|
|
||
| // If there is only one item, we don't need to delete it, | ||
| // because it normally means custom resource name is not enabled. | ||
| if items.Len() <= 1 { |
There was a problem hiding this comment.
This is not safe since now we use the cached kube client.
| gatewayObjMeta := metav1.ObjectMeta{ | ||
| Name: "e2e-test-infra", | ||
| Namespace: "envoy-gateway-system", | ||
| Namespace: "gateway-conformance-infra", |
There was a problem hiding this comment.
Use gateway-conformance-infra for the Gateway, as the envoy-gateway-system namespace is not configured in the watch-namespaces profile anymore.
For some reason, these two tests fail with the watch-namespaces profile. cc @jukie, do you have any ideas? |
* fix: restore last transition time in merge status conditions (#8962) * fix: restore last transition time in merge status conditions Signed-off-by: Rudrakh Panigrahi <rudrakh97@gmail.com> * add release note Signed-off-by: Rudrakh Panigrahi <rudrakh97@gmail.com> --------- Signed-off-by: Rudrakh Panigrahi <rudrakh97@gmail.com> (cherry picked from commit 9430ab9) Signed-off-by: jukie <10012479+jukie@users.noreply.github.com> * fix: applyBackendTLSSetting panic (#8998) * fix: applyBackendTLSSetting panic Signed-off-by: zirain <zirain2009@gmail.com> (cherry picked from commit b754b68) Signed-off-by: jukie <10012479+jukie@users.noreply.github.com> * fix(xds): hot-reload xDS server cert in GatewayNamespaceMode (#8959) * fix(xds): hot-reload xDS server cert in GatewayNamespaceMode Signed-off-by: nguyenptk <nguyenptk@gmail.com> * extract loadServerTLSConfig helper and add tests Signed-off-by: nguyenptk <nguyenptk@gmail.com> --------- Signed-off-by: nguyenptk <nguyenptk@gmail.com> Signed-off-by: Nguyên (Harry) <nguyenptk@gmail.com> (cherry picked from commit 35c5a12) Signed-off-by: jukie <10012479+jukie@users.noreply.github.com> * fix(helm): correct typo in HPA maxReplicas required message (#9032) Signed-off-by: benjaminch <benjamin.chastanier@gmail.com> (cherry picked from commit 8da6dc7) Signed-off-by: jukie <10012479+jukie@users.noreply.github.com> * fix: allow IANA cipher names (#9073) allow IANA cipher names Signed-off-by: Huabing Zhao <zhaohuabing@gmail.com> (cherry picked from commit ea41d6f) Signed-off-by: jukie <10012479+jukie@users.noreply.github.com> * fix(gatewayapi): prefer sectioned BackendTLSPolicy over wildcard for same target (#9068) * add passing testcase showing wildcard overriding sectioned backendTLSPolicy Signed-off-by: AlecDiraimondo <50001153+AlecDiraimondo@users.noreply.github.com> * fix(gatewayapi): prefer sectioned BackendTLSPolicy over wildcard for same target When two BackendTLSPolicies target the same backend — one with sectionName set, one without — the wildcard wins whenever it sorts earlier, since policies are iterated in creationTimestamp order with no specificity preference. The sectioned policy is silently dropped: never selected by the translator, no status written. Make getBackendTLSPolicy prefer policies with an explicit sectionName match before falling back to wildcard matches. Matches the specificity ordering SecurityPolicy already implements. Refs #9035 Signed-off-by: AlecDiraimondo <50001153+AlecDiraimondo@users.noreply.github.com> * update release notes Signed-off-by: AlecDiraimondo <50001153+AlecDiraimondo@users.noreply.github.com> * fixes from review: dedup helper, return early if backendTarget has no section specified Signed-off-by: AlecDiraimondo <50001153+AlecDiraimondo@users.noreply.github.com> Co-authored-by: Rudrakh Panigrahi <rudrakh97@gmail.com> --------- Signed-off-by: AlecDiraimondo <50001153+AlecDiraimondo@users.noreply.github.com> Signed-off-by: Alec Diraimondo <drayalec@gmail.com> Co-authored-by: Rudrakh Panigrahi <rudrakh97@gmail.com> (cherry picked from commit 07ec95c) Signed-off-by: jukie <10012479+jukie@users.noreply.github.com> * fix: encode TLS secrets with canonical PEM formatting (#8744) * exclude non-ready https Signed-off-by: Karol Szwaj <karol.szwaj@gmail.com> * prevent BoringSSL errors Signed-off-by: Karol Szwaj <karol.szwaj@gmail.com> * Add trailing newline to testdata PEMs Signed-off-by: Karol Szwaj <karol.szwaj@gmail.com> * add release notes Signed-off-by: Karol Szwaj <karol.szwaj@gmail.com> * use shallow copy instead of DeepCopy Signed-off-by: Karol Szwaj <karol.szwaj@gmail.com> * use maps.Clone for the data and split the PRs Signed-off-by: Karol Szwaj <karol.szwaj@gmail.com> --------- Signed-off-by: Karol Szwaj <karol.szwaj@gmail.com> (cherry picked from commit 560a262) Signed-off-by: jukie <10012479+jukie@users.noreply.github.com> * fix: MaxStreamDuration should worked for non-route cluster (#9028) * fix: MaxStreamDuration should worked on commonHttpProtocolOptions Signed-off-by: zirain <zirain2009@gmail.com> * should only worked for non-route cluster Signed-off-by: zirain <zirain2009@gmail.com> * fix Signed-off-by: zirain <zirain2009@gmail.com> --------- Signed-off-by: zirain <zirain2009@gmail.com> (cherry picked from commit 0edf51a) Signed-off-by: jukie <10012479+jukie@users.noreply.github.com> * fix(egctl): skip missing CRDs in `x status` bulk modes (#9099) `egctl x status all` (and `xroute`/`xpolicy`) aborted with an error when a Gateway API CRD was not installed in the cluster (e.g. TCPRoute). Missing CRDs are now skipped, and reported on stderr when `-v` is set. Explicit single-resource invocations still error loudly so users get a clear signal when a kind they asked for is unavailable. Signed-off-by: Jeremiah Snapp <jeremiahs@tomonetworks.com> (cherry picked from commit 3ab110e) Signed-off-by: jukie <10012479+jukie@users.noreply.github.com> * fix(gatewayapi): reject unsupported BackendRef URLRewrite path rewrites (#9131) gatewayapi: reject BackendRef URLRewrite path modifiers Signed-off-by: Aditya7880900936 <adityasanskarsrivastav788@gmail.com> (cherry picked from commit acaf4cd) Signed-off-by: jukie <10012479+jukie@users.noreply.github.com> * fix: force HTTP1 for Service/ServiceImport with websocket appProtocol (#9138) force HTTP1 for Service/ServiceImport with websocket appProtocol Signed-off-by: Huabing Zhao <zhaohuabing@gmail.com> (cherry picked from commit b1aa5d9) Signed-off-by: jukie <10012479+jukie@users.noreply.github.com> * fix: move validation admission policy outside of crds directory (#9024) * move validation admission policy outside of crds directory Signed-off-by: Huabing Zhao <zhaohuabing@gmail.com> * fix upgrade Signed-off-by: Huabing Zhao <zhaohuabing@gmail.com> * add release note Signed-off-by: Huabing Zhao <zhaohuabing@gmail.com> * fix Signed-off-by: Huabing Zhao <zhaohuabing@gmail.com> * update docs Signed-off-by: Huabing Zhao <zhaohuabing@gmail.com> * address comment Signed-off-by: Huabing Zhao <zhaohuabing@gmail.com> * address comment Signed-off-by: Huabing Zhao <zhaohuabing@gmail.com> * address comments Signed-off-by: Huabing Zhao <zhaohuabing@gmail.com> * update relese note Signed-off-by: Huabing Zhao <zhaohuabing@gmail.com> * address comments Signed-off-by: Huabing Zhao <zhaohuabing@gmail.com> * fix gen Signed-off-by: Huabing Zhao <zhaohuabing@gmail.com> * use a more general name for this option Signed-off-by: Huabing Zhao <zhaohuabing@gmail.com> * update docs Signed-off-by: Huabing Zhao <zhaohuabing@gmail.com> * rename Signed-off-by: Huabing Zhao <zhaohuabing@gmail.com> * update test Signed-off-by: Huabing Zhao <zhaohuabing@gmail.com> * update release note Signed-off-by: Huabing Zhao <zhaohuabing@gmail.com> * fix gen Signed-off-by: Huabing Zhao <zhaohuabing@gmail.com> * rename supportingResources to safeUpgradePolicy Signed-off-by: Huabing Zhao <zhaohuabing@gmail.com> * update Signed-off-by: Huabing Zhao <zhaohuabing@gmail.com> --------- Signed-off-by: Huabing Zhao <zhaohuabing@gmail.com> Signed-off-by: Huabing (Robin) Zhao <zhaohuabing@gmail.com> Co-authored-by: zirain <zirain2009@gmail.com> (cherry picked from commit 35ad8f2) Signed-off-by: jukie <10012479+jukie@users.noreply.github.com> * fix: alway include the controller namespace in the watched namespaces (#9100) * always include the controller namespace in the k8s provider Signed-off-by: Huabing Zhao <zhaohuabing@gmail.com> * update docs Signed-off-by: Huabing Zhao <zhaohuabing@gmail.com> (cherry picked from commit 84be503d855e968b7b6999d476a6897e98d4e330) * address comment Signed-off-by: Huabing Zhao <zhaohuabing@gmail.com> * address comments Signed-off-by: Huabing Zhao <zhaohuabing@gmail.com> * address comments Signed-off-by: Huabing Zhao <zhaohuabing@gmail.com> * Potential fix for pull request finding Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> Signed-off-by: Huabing (Robin) Zhao <zhaohuabing@gmail.com> * minor changes Signed-off-by: Huabing Zhao <zhaohuabing@gmail.com> * fix test Signed-off-by: Huabing Zhao <zhaohuabing@gmail.com> * fix lint Signed-off-by: Huabing Zhao <zhaohuabing@gmail.com> * fix test Signed-off-by: Huabing Zhao <zhaohuabing@gmail.com> * fix test Signed-off-by: Huabing Zhao <zhaohuabing@gmail.com> * fix test Signed-off-by: Huabing Zhao <zhaohuabing@gmail.com> --------- Signed-off-by: Huabing Zhao <zhaohuabing@gmail.com> Signed-off-by: Huabing (Robin) Zhao <zhaohuabing@gmail.com> Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> (cherry picked from commit 5266322) Signed-off-by: jukie <10012479+jukie@users.noreply.github.com> * test(helm): regenerate safe-upgrade-policy-disabled golden for release/v1.8 The safe-upgrade-policy-disabled test case added in #9024 shipped a golden rendered against main's chart templates, which include features not present on release/v1.8 (automountServiceAccountToken, expanded HPA RBAC verbs, serviceaccount securityContext, ArgoCD/Flux hook comment). Regenerate the golden against the release branch templates and restore the release-pinned ratelimit image tag (ff287602). Signed-off-by: jukie <10012479+jukie@users.noreply.github.com> * regen Signed-off-by: jukie <10012479+jukie@users.noreply.github.com> * chore: bump golang to 1.26.4 (#9147) Signed-off-by: zirain <zirain2009@gmail.com> Signed-off-by: jukie <10012479+jukie@users.noreply.github.com> --------- Signed-off-by: Rudrakh Panigrahi <rudrakh97@gmail.com> Signed-off-by: jukie <10012479+jukie@users.noreply.github.com> Signed-off-by: zirain <zirain2009@gmail.com> Signed-off-by: nguyenptk <nguyenptk@gmail.com> Signed-off-by: Nguyên (Harry) <nguyenptk@gmail.com> Signed-off-by: benjaminch <benjamin.chastanier@gmail.com> Signed-off-by: Huabing Zhao <zhaohuabing@gmail.com> Signed-off-by: AlecDiraimondo <50001153+AlecDiraimondo@users.noreply.github.com> Signed-off-by: Alec Diraimondo <drayalec@gmail.com> Signed-off-by: Karol Szwaj <karol.szwaj@gmail.com> Signed-off-by: Jeremiah Snapp <jeremiahs@tomonetworks.com> Signed-off-by: Aditya7880900936 <adityasanskarsrivastav788@gmail.com> Signed-off-by: Huabing (Robin) Zhao <zhaohuabing@gmail.com> Co-authored-by: Rudrakh Panigrahi <rudrakh97@gmail.com> Co-authored-by: zirain <zirain2009@gmail.com> Co-authored-by: Nguyên (Harry) <nguyenptk@gmail.com> Co-authored-by: BenjaminCh <benjamin.chastanier@gmail.com> Co-authored-by: Huabing (Robin) Zhao <zhaohuabing@gmail.com> Co-authored-by: Alec Diraimondo <drayalec@gmail.com> Co-authored-by: Karol Szwaj <karol.szwaj@gmail.com> Co-authored-by: Jeremiah Snapp <jeremiah.snapp@gmail.com> Co-authored-by: Aditya Sanskar Srivastav <161202916+Aditya7880900936@users.noreply.github.com> Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
…envoyproxy#9100) * always include the controller namespace in the k8s provider Signed-off-by: Huabing Zhao <zhaohuabing@gmail.com> * update docs Signed-off-by: Huabing Zhao <zhaohuabing@gmail.com> (cherry picked from commit 84be503d855e968b7b6999d476a6897e98d4e330) * address comment Signed-off-by: Huabing Zhao <zhaohuabing@gmail.com> * address comments Signed-off-by: Huabing Zhao <zhaohuabing@gmail.com> * address comments Signed-off-by: Huabing Zhao <zhaohuabing@gmail.com> * Potential fix for pull request finding Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> Signed-off-by: Huabing (Robin) Zhao <zhaohuabing@gmail.com> * minor changes Signed-off-by: Huabing Zhao <zhaohuabing@gmail.com> * fix test Signed-off-by: Huabing Zhao <zhaohuabing@gmail.com> * fix lint Signed-off-by: Huabing Zhao <zhaohuabing@gmail.com> * fix test Signed-off-by: Huabing Zhao <zhaohuabing@gmail.com> * fix test Signed-off-by: Huabing Zhao <zhaohuabing@gmail.com> * fix test Signed-off-by: Huabing Zhao <zhaohuabing@gmail.com> --------- Signed-off-by: Huabing Zhao <zhaohuabing@gmail.com> Signed-off-by: Huabing (Robin) Zhao <zhaohuabing@gmail.com> Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
This PR always includes the controller namespace in the watched namespaces and watch infra resources in the controller namespace. This is required to
The current behavior of kubernetes provider in the watched namespace mode is verified in #9104. It fails when the controller namespace is not explicitly configured.
fixes: #9094