fix: allow cross-namespace push with ClusterSecretStore objects#5998
Conversation
Signed-off-by: Gergely Brautigam <182850+Skarlso@users.noreply.github.com>
WalkthroughPushSecret now supports per-secret namespace overrides using ClusterSecretStore, with internal refactoring to route operations through namespace-specific clients. Signatures updated for mergePushSecretData and createOrUpdate functions to accept metadata and client parameters respectively. Changes
🚥 Pre-merge checks | ✅ 2✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
providers/v1/kubernetes/client.go (1)
82-105:⚠️ Potential issue | 🟠 MajorApply namespace routing to
DeleteSecretsimilar toPushSecret, or extendPushSecretRemoteRefto include namespace context.
DeleteSecretusesc.userSecretClientdirectly, which is scoped to the store's defaultRemoteNamespace. However,PushSecretcan push secrets to an overridden namespace via metadata (e.g.,pushMeta.Spec.RemoteNamespace). When deletion is requested for such a secret,DeleteSecretoperates only in the default namespace and cannot locate the secret that was pushed elsewhere, resulting in silent failure or deletion of the wrong secret.Apply the same namespace-routing pattern used in
PushSecret(viac.secretsClientFor) toDeleteSecret, or modify the interface to pass namespace context throughPushSecretRemoteRef.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@providers/v1/kubernetes/client.go` around lines 82 - 105, DeleteSecret currently always uses c.userSecretClient (scoped to the store default namespace) which misses secrets pushed to an overridden namespace; change DeleteSecret to pick the correct namespaced secret client like PushSecret does (use c.secretsClientFor(...) or otherwise resolve namespace from PushSecretRemoteRef) before calling Get/Delete. Specifically, in DeleteSecret replace direct use of c.userSecretClient with the result of c.secretsClientFor(ctx, remoteRef) (or extend PushSecretRemoteRef to expose the remote namespace and call c.secretsClientFor(ctx, namespace)), then proceed with extSecret retrieval, property checks (remoteRef.GetProperty()), removeProperty(...) and fullDelete(...) using that client so deletion targets the same namespace as PushSecret.
🧹 Nitpick comments (1)
providers/v1/kubernetes/provider.go (1)
149-154:secretsClientForcreates a newSecretsclient on every call for non-default namespaces.Each invocation of
secretsClientForwith a novel namespace allocates a fresh client viac.userCoreV1.Secrets(namespace). In the currentPushSecretflow this is called once per push, so it's fine. However, if this is ever called in a hot loop (e.g., batch pushes to the same target namespace), the repeated allocations could add up. Consider caching if usage patterns change, but for now this is acceptable.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@providers/v1/kubernetes/provider.go` around lines 149 - 154, secretsClientFor currently calls c.userCoreV1.Secrets(namespace) on each call for non-default namespaces which allocates a new client per invocation; change this to cache per-namespace clients on the Client struct (e.g., a map[string]KClient plus a sync.RWMutex) so secretsClientFor first checks the cache and returns the cached KClient, and only if missing creates c.userCoreV1.Secrets(namespace), stores it in the cache, and returns it; preserve existing fallback behavior to c.userSecretClient when namespace is empty or equals c.store.RemoteNamespace and ensure concurrent access to the map is protected.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Outside diff comments:
In `@providers/v1/kubernetes/client.go`:
- Around line 82-105: DeleteSecret currently always uses c.userSecretClient
(scoped to the store default namespace) which misses secrets pushed to an
overridden namespace; change DeleteSecret to pick the correct namespaced secret
client like PushSecret does (use c.secretsClientFor(...) or otherwise resolve
namespace from PushSecretRemoteRef) before calling Get/Delete. Specifically, in
DeleteSecret replace direct use of c.userSecretClient with the result of
c.secretsClientFor(ctx, remoteRef) (or extend PushSecretRemoteRef to expose the
remote namespace and call c.secretsClientFor(ctx, namespace)), then proceed with
extSecret retrieval, property checks (remoteRef.GetProperty()),
removeProperty(...) and fullDelete(...) using that client so deletion targets
the same namespace as PushSecret.
---
Nitpick comments:
In `@providers/v1/kubernetes/provider.go`:
- Around line 149-154: secretsClientFor currently calls
c.userCoreV1.Secrets(namespace) on each call for non-default namespaces which
allocates a new client per invocation; change this to cache per-namespace
clients on the Client struct (e.g., a map[string]KClient plus a sync.RWMutex) so
secretsClientFor first checks the cache and returns the cached KClient, and only
if missing creates c.userCoreV1.Secrets(namespace), stores it in the cache, and
returns it; preserve existing fallback behavior to c.userSecretClient when
namespace is empty or equals c.store.RemoteNamespace and ensure concurrent
access to the map is protected.
ℹ️ Review info
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
providers/v1/kubernetes/client.goproviders/v1/kubernetes/client_test.goproviders/v1/kubernetes/provider.go
|
…2.1.0 (#4491) This PR contains the following updates: | Package | Update | Change | |---|---|---| | [external-secrets/external-secrets](https://github.com/external-secrets/external-secrets) | minor | `v2.0.1` → `v2.1.0` | --- ### Release Notes <details> <summary>external-secrets/external-secrets (external-secrets/external-secrets)</summary> ### [`v2.1.0`](https://github.com/external-secrets/external-secrets/releases/tag/v2.1.0) [Compare Source](external-secrets/external-secrets@v2.0.1...v2.1.0) Image: `ghcr.io/external-secrets/external-secrets:v2.1.0` Image: `ghcr.io/external-secrets/external-secrets:v2.1.0-ubi` Image: `ghcr.io/external-secrets/external-secrets:v2.1.0-ubi-boringssl` <!-- Release notes generated using configuration in .github/release.yml at main --> #### What's Changed ##### General - chore(release): Update helm chart by [@​evrardj-roche](https://github.com/evrardj-roche) in [#​5981](external-secrets/external-secrets#5981) - fix: cosign verify does not use signing config by [@​gusfcarvalho](https://github.com/gusfcarvalho) in [#​5982](external-secrets/external-secrets#5982) - docs: Update release process by [@​evrardj-roche](https://github.com/evrardj-roche) in [#​5980](external-secrets/external-secrets#5980) - fix: allow cross-namespace push with ClusterSecretStore objects by [@​Skarlso](https://github.com/Skarlso) in [#​5998](external-secrets/external-secrets#5998) - feat(charts): add new flag enable leader for cert-manager by [@​nutmos](https://github.com/nutmos) in [#​5863](external-secrets/external-secrets#5863) - feat(kubernetes): fall back to system CA roots when no CA is configured by [@​rajsinghtech](https://github.com/rajsinghtech) in [#​5961](external-secrets/external-secrets#5961) - feat: dedup sbom but keep it monolithic by [@​moolen](https://github.com/moolen) in [#​6004](external-secrets/external-secrets#6004) - fix: add missing metrics and fundamentally fix the caching logic by [@​Skarlso](https://github.com/Skarlso) in [#​5894](external-secrets/external-secrets#5894) - docs: designate Oracle Vault provider as 'stable' by [@​anders-swanson](https://github.com/anders-swanson) in [#​6020](external-secrets/external-secrets#6020) - docs: Oracle Vault provider capabilities by [@​anders-swanson](https://github.com/anders-swanson) in [#​6023](external-secrets/external-secrets#6023) - docs(azurekv): cert-manager pushsecret example and cleanups by [@​illrill](https://github.com/illrill) in [#​5972](external-secrets/external-secrets#5972) - feat(kubernetes): implement SecretExists by [@​Saku2](https://github.com/Saku2) in [#​5973](external-secrets/external-secrets#5973) - fix(charts): Fix wrongly set annotations for cert-controller metrics service by [@​josemaia](https://github.com/josemaia) in [#​6029](external-secrets/external-secrets#6029) - feat(providers): Nebius MysteryBox integration by [@​greenmapc](https://github.com/greenmapc) in [#​5868](external-secrets/external-secrets#5868) ##### Dependencies - chore(deps): bump aquasecurity/trivy-action from 0.34.0 to 0.34.1 by [@​dependabot](https://github.com/dependabot)\[bot] in [#​5986](external-secrets/external-secrets#5986) - chore(deps): bump mkdocs-material from 9.7.1 to 9.7.2 in /hack/api-docs by [@​dependabot](https://github.com/dependabot)\[bot] in [#​5992](external-secrets/external-secrets#5992) - chore(deps): bump ubi9/ubi from `b8923f5` to `cecb1cd` by [@​dependabot](https://github.com/dependabot)\[bot] in [#​5984](external-secrets/external-secrets#5984) - chore(deps): bump helm/kind-action from 1.13.0 to 1.14.0 by [@​dependabot](https://github.com/dependabot)\[bot] in [#​5985](external-secrets/external-secrets#5985) - chore(deps): bump actions/dependency-review-action from 4.8.2 to 4.8.3 by [@​dependabot](https://github.com/dependabot)\[bot] in [#​5990](external-secrets/external-secrets#5990) - chore(deps): bump github/codeql-action from 4.32.3 to 4.32.4 by [@​dependabot](https://github.com/dependabot)\[bot] in [#​5989](external-secrets/external-secrets#5989) - chore(deps): bump goreleaser/goreleaser-action from 6.4.0 to 7.0.0 by [@​dependabot](https://github.com/dependabot)\[bot] in [#​5987](external-secrets/external-secrets#5987) - chore(deps): bump regex from 2026.1.15 to 2026.2.19 in /hack/api-docs by [@​dependabot](https://github.com/dependabot)\[bot] in [#​5991](external-secrets/external-secrets#5991) - chore(deps): bump actions/stale from 10.1.1 to 10.2.0 by [@​dependabot](https://github.com/dependabot)\[bot] in [#​5988](external-secrets/external-secrets#5988) - chore(deps): bump regex from 2026.2.19 to 2026.2.28 in /hack/api-docs by [@​dependabot](https://github.com/dependabot)\[bot] in [#​6012](external-secrets/external-secrets#6012) - chore(deps): bump mkdocs-material from 9.7.2 to 9.7.3 in /hack/api-docs by [@​dependabot](https://github.com/dependabot)\[bot] in [#​6014](external-secrets/external-secrets#6014) - chore(deps): bump step-security/harden-runner from 2.14.2 to 2.15.0 by [@​dependabot](https://github.com/dependabot)\[bot] in [#​6015](external-secrets/external-secrets#6015) - chore(deps): bump anchore/sbom-action from 0.22.2 to 0.23.0 by [@​dependabot](https://github.com/dependabot)\[bot] in [#​6016](external-secrets/external-secrets#6016) - chore(deps): bump certifi from 2026.1.4 to 2026.2.25 in /hack/api-docs by [@​dependabot](https://github.com/dependabot)\[bot] in [#​6013](external-secrets/external-secrets#6013) - chore(deps): bump actions/setup-go from 6.2.0 to 6.3.0 by [@​dependabot](https://github.com/dependabot)\[bot] in [#​6010](external-secrets/external-secrets#6010) - chore(deps): bump hashicorp/setup-terraform from [`ce70bcf`](external-secrets/external-secrets@ce70bcf) to [`5e8dbf3`](external-secrets/external-secrets@5e8dbf3) by [@​dependabot](https://github.com/dependabot)\[bot] in [#​6011](external-secrets/external-secrets#6011) - chore(deps): bump actions/attest-build-provenance from 3.2.0 to 4.1.0 by [@​dependabot](https://github.com/dependabot)\[bot] in [#​6009](external-secrets/external-secrets#6009) - chore(deps): bump distroless/static from `972618c` to `28efbe9` by [@​dependabot](https://github.com/dependabot)\[bot] in [#​6008](external-secrets/external-secrets#6008) #### New Contributors - [@​nutmos](https://github.com/nutmos) made their first contribution in [#​5863](external-secrets/external-secrets#5863) - [@​rajsinghtech](https://github.com/rajsinghtech) made their first contribution in [#​5961](external-secrets/external-secrets#5961) - [@​illrill](https://github.com/illrill) made their first contribution in [#​5972](external-secrets/external-secrets#5972) - [@​Saku2](https://github.com/Saku2) made their first contribution in [#​5973](external-secrets/external-secrets#5973) - [@​greenmapc](https://github.com/greenmapc) made their first contribution in [#​5868](external-secrets/external-secrets#5868) **Full Changelog**: <external-secrets/external-secrets@v2.0.1...v2.1.0> </details> --- ### Configuration 📅 **Schedule**: Branch creation - At any time (no schedule defined), Automerge - At any time (no schedule defined). 🚦 **Automerge**: Disabled by config. Please merge this manually once you are satisfied. ♻ **Rebasing**: Whenever PR is behind base branch, or you tick the rebase/retry checkbox. 🔕 **Ignore**: Close this PR and you won't be reminded about this update again. --- - [ ] <!-- rebase-check -->If you want to rebase/retry this PR, check this box --- This PR has been generated by [Renovate Bot](https://github.com/renovatebot/renovate). <!--renovate-debug:eyJjcmVhdGVkSW5WZXIiOiI0My41MS4wIiwidXBkYXRlZEluVmVyIjoiNDMuNTEuMCIsInRhcmdldEJyYW5jaCI6Im1haW4iLCJsYWJlbHMiOlsiaW1hZ2UiXX0=--> Reviewed-on: https://gitea.alexlebens.dev/alexlebens/infrastructure/pulls/4491 Co-authored-by: Renovate Bot <renovate-bot@alexlebens.net> Co-committed-by: Renovate Bot <renovate-bot@alexlebens.net>
This PR contains the following updates: | Package | Update | Change | |---|---|---| | [external-secrets](https://github.com/external-secrets/external-secrets) | minor | `2.0.1` → `2.1.0` | --- ### Release Notes <details> <summary>external-secrets/external-secrets (external-secrets)</summary> ### [`v2.1.0`](https://github.com/external-secrets/external-secrets/releases/tag/v2.1.0) [Compare Source](external-secrets/external-secrets@v2.0.1...v2.1.0) Image: `ghcr.io/external-secrets/external-secrets:v2.1.0` Image: `ghcr.io/external-secrets/external-secrets:v2.1.0-ubi` Image: `ghcr.io/external-secrets/external-secrets:v2.1.0-ubi-boringssl` <!-- Release notes generated using configuration in .github/release.yml at main --> #### What's Changed ##### General - chore(release): Update helm chart by [@​evrardj-roche](https://github.com/evrardj-roche) in [#​5981](external-secrets/external-secrets#5981) - fix: cosign verify does not use signing config by [@​gusfcarvalho](https://github.com/gusfcarvalho) in [#​5982](external-secrets/external-secrets#5982) - docs: Update release process by [@​evrardj-roche](https://github.com/evrardj-roche) in [#​5980](external-secrets/external-secrets#5980) - fix: allow cross-namespace push with ClusterSecretStore objects by [@​Skarlso](https://github.com/Skarlso) in [#​5998](external-secrets/external-secrets#5998) - feat(charts): add new flag enable leader for cert-manager by [@​nutmos](https://github.com/nutmos) in [#​5863](external-secrets/external-secrets#5863) - feat(kubernetes): fall back to system CA roots when no CA is configured by [@​rajsinghtech](https://github.com/rajsinghtech) in [#​5961](external-secrets/external-secrets#5961) - feat: dedup sbom but keep it monolithic by [@​moolen](https://github.com/moolen) in [#​6004](external-secrets/external-secrets#6004) - fix: add missing metrics and fundamentally fix the caching logic by [@​Skarlso](https://github.com/Skarlso) in [#​5894](external-secrets/external-secrets#5894) - docs: designate Oracle Vault provider as 'stable' by [@​anders-swanson](https://github.com/anders-swanson) in [#​6020](external-secrets/external-secrets#6020) - docs: Oracle Vault provider capabilities by [@​anders-swanson](https://github.com/anders-swanson) in [#​6023](external-secrets/external-secrets#6023) - docs(azurekv): cert-manager pushsecret example and cleanups by [@​illrill](https://github.com/illrill) in [#​5972](external-secrets/external-secrets#5972) - feat(kubernetes): implement SecretExists by [@​Saku2](https://github.com/Saku2) in [#​5973](external-secrets/external-secrets#5973) - fix(charts): Fix wrongly set annotations for cert-controller metrics service by [@​josemaia](https://github.com/josemaia) in [#​6029](external-secrets/external-secrets#6029) - feat(providers): Nebius MysteryBox integration by [@​greenmapc](https://github.com/greenmapc) in [#​5868](external-secrets/external-secrets#5868) ##### Dependencies - chore(deps): bump aquasecurity/trivy-action from 0.34.0 to 0.34.1 by [@​dependabot](https://github.com/dependabot)\[bot] in [#​5986](external-secrets/external-secrets#5986) - chore(deps): bump mkdocs-material from 9.7.1 to 9.7.2 in /hack/api-docs by [@​dependabot](https://github.com/dependabot)\[bot] in [#​5992](external-secrets/external-secrets#5992) - chore(deps): bump ubi9/ubi from `b8923f5` to `cecb1cd` by [@​dependabot](https://github.com/dependabot)\[bot] in [#​5984](external-secrets/external-secrets#5984) - chore(deps): bump helm/kind-action from 1.13.0 to 1.14.0 by [@​dependabot](https://github.com/dependabot)\[bot] in [#​5985](external-secrets/external-secrets#5985) - chore(deps): bump actions/dependency-review-action from 4.8.2 to 4.8.3 by [@​dependabot](https://github.com/dependabot)\[bot] in [#​5990](external-secrets/external-secrets#5990) - chore(deps): bump github/codeql-action from 4.32.3 to 4.32.4 by [@​dependabot](https://github.com/dependabot)\[bot] in [#​5989](external-secrets/external-secrets#5989) - chore(deps): bump goreleaser/goreleaser-action from 6.4.0 to 7.0.0 by [@​dependabot](https://github.com/dependabot)\[bot] in [#​5987](external-secrets/external-secrets#5987) - chore(deps): bump regex from 2026.1.15 to 2026.2.19 in /hack/api-docs by [@​dependabot](https://github.com/dependabot)\[bot] in [#​5991](external-secrets/external-secrets#5991) - chore(deps): bump actions/stale from 10.1.1 to 10.2.0 by [@​dependabot](https://github.com/dependabot)\[bot] in [#​5988](external-secrets/external-secrets#5988) - chore(deps): bump regex from 2026.2.19 to 2026.2.28 in /hack/api-docs by [@​dependabot](https://github.com/dependabot)\[bot] in [#​6012](external-secrets/external-secrets#6012) - chore(deps): bump mkdocs-material from 9.7.2 to 9.7.3 in /hack/api-docs by [@​dependabot](https://github.com/dependabot)\[bot] in [#​6014](external-secrets/external-secrets#6014) - chore(deps): bump step-security/harden-runner from 2.14.2 to 2.15.0 by [@​dependabot](https://github.com/dependabot)\[bot] in [#​6015](external-secrets/external-secrets#6015) - chore(deps): bump anchore/sbom-action from 0.22.2 to 0.23.0 by [@​dependabot](https://github.com/dependabot)\[bot] in [#​6016](external-secrets/external-secrets#6016) - chore(deps): bump certifi from 2026.1.4 to 2026.2.25 in /hack/api-docs by [@​dependabot](https://github.com/dependabot)\[bot] in [#​6013](external-secrets/external-secrets#6013) - chore(deps): bump actions/setup-go from 6.2.0 to 6.3.0 by [@​dependabot](https://github.com/dependabot)\[bot] in [#​6010](external-secrets/external-secrets#6010) - chore(deps): bump hashicorp/setup-terraform from [`ce70bcf`](external-secrets/external-secrets@ce70bcf) to [`5e8dbf3`](external-secrets/external-secrets@5e8dbf3) by [@​dependabot](https://github.com/dependabot)\[bot] in [#​6011](external-secrets/external-secrets#6011) - chore(deps): bump actions/attest-build-provenance from 3.2.0 to 4.1.0 by [@​dependabot](https://github.com/dependabot)\[bot] in [#​6009](external-secrets/external-secrets#6009) - chore(deps): bump distroless/static from `972618c` to `28efbe9` by [@​dependabot](https://github.com/dependabot)\[bot] in [#​6008](external-secrets/external-secrets#6008) #### New Contributors - [@​nutmos](https://github.com/nutmos) made their first contribution in [#​5863](external-secrets/external-secrets#5863) - [@​rajsinghtech](https://github.com/rajsinghtech) made their first contribution in [#​5961](external-secrets/external-secrets#5961) - [@​illrill](https://github.com/illrill) made their first contribution in [#​5972](external-secrets/external-secrets#5972) - [@​Saku2](https://github.com/Saku2) made their first contribution in [#​5973](external-secrets/external-secrets#5973) - [@​greenmapc](https://github.com/greenmapc) made their first contribution in [#​5868](external-secrets/external-secrets#5868) **Full Changelog**: <external-secrets/external-secrets@v2.0.1...v2.1.0> </details> --- ### Configuration 📅 **Schedule**: Branch creation - At any time (no schedule defined), Automerge - At any time (no schedule defined). 🚦 **Automerge**: Disabled by config. Please merge this manually once you are satisfied. ♻ **Rebasing**: Whenever PR is behind base branch, or you tick the rebase/retry checkbox. 🔕 **Ignore**: Close this PR and you won't be reminded about this update again. --- - [ ] <!-- rebase-check -->If you want to rebase/retry this PR, check this box --- This PR has been generated by [Renovate Bot](https://github.com/renovatebot/renovate). <!--renovate-debug:eyJjcmVhdGVkSW5WZXIiOiI0My41OS4yIiwidXBkYXRlZEluVmVyIjoiNDMuNTkuMiIsInRhcmdldEJyYW5jaCI6Im1haW4iLCJsYWJlbHMiOlsiY2hhcnQiXX0=--> Reviewed-on: https://gitea.alexlebens.dev/alexlebens/infrastructure/pulls/4516 Co-authored-by: Renovate Bot <renovate-bot@alexlebens.net> Co-committed-by: Renovate Bot <renovate-bot@alexlebens.net>
…rnal-secrets#5998) Signed-off-by: AlexOQ <30403857+AlexOQ@users.noreply.github.com>



Problem Statement
Cross-namespace access with ClusterSecretStore objects.
Related Issue
Fixes #5832
Proposed Changes
How do you like to solve the issue and why?
Format
Please ensure that your PR follows the following format for the title:
Where
scopeis optionally one of:Checklist
git commit --signoffmake testmake reviewableSummary
Enables cross-namespace secret push operations with ClusterSecretStore by allowing per-secret namespace overrides via metadata.
Key Changes
PushSecret Logic (
client.go):remoteNamespaceoverride in PushSecret metadata, restricted to ClusterSecretStore onlysecretsClientFor()to obtain a per-namespace Kubernetes client for each secret operationFunction Signatures:
mergePushSecretData()now accepts parsed metadata (pushMeta) for label/annotation merging instead of re-parsing from remoteRefcreateOrUpdate()now accepts asecretClientparameter, routing all Get/Create/Update operations through the provided client instead of using a hardcoded userSecretClientProvider Setup (
provider.go):userCoreV1field to Client for dynamic namespace-scoped client creationsecretsClientFor()helper method that returns the appropriate Secrets client for a given namespaceTest Coverage:
TestPushSecretRemoteNamespaceRouting: Validates that pushing with a remoteNamespace override creates the secret in the target namespace onlyTestPushSecretRemoteNamespaceRejectedForSecretStore: Ensures SecretStore kind rejects remoteNamespace overrides with a helpful error message