fix: scope RBAC namespaces per reconciler

[vrutkovs]

Use each reconciler's watched namespaces for VMAgent and VMSingle RBAC creation and orphan cleanup. Prevent shared global config from creating or deleting RBAC in unrelated namespaces.

[vrutkovs]

Lets not merge it yet though - I think there are few more bugs around this

[AndrewChubatiuk]

looks like a part of API package

[cubic-dev-ai]

5 issues found across 9 files

Prompt for AI agents (unresolved issues)

Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.


<file name="internal/controller/operator/factory/vmagent/vmagent.go">

<violation number="1" location="internal/controller/operator/factory/vmagent/vmagent.go:134">
P1: When a VMAgent reconciler’s `BaseConf.WatchNamespaces` differs from the process-global configuration, this path creates RBAC in one namespace set but orphan cleanup uses another. During RBAC cleanup, it can delete objects managed for another reconciler or leave this reconciler’s old Roles and RoleBindings behind; pass the reconciler namespace set through orphan cleanup and apply the same `rbacNamespaces` selection.</violation>

<violation number="2" location="internal/controller/operator/factory/vmagent/vmagent.go:157">
P2: VMAgent RBAC creation now uses the reconciler's `baseConf.WatchNamespaces` (via `rbacNamespaces`), but the counterpart orphan cleanup in `deleteOrphaned` still reads `config.MustGetBaseConfig().WatchNamespaces`. When a reconciler's config differs from the shared global config, creation and cleanup operate on different namespace sets. Pass `baseConf.WatchNamespaces` into `deleteOrphaned` for VMAgent the same way this PR did for VMSingle.</violation>
</file>

<file name="internal/controller/operator/vmagent_controller.go">

<violation number="1" location="internal/controller/operator/vmagent_controller.go:121">
P2: The RBAC cleanup during prev-state reconcile (deleteOrphaned, called from CreateOrUpdateWithConfig that this line now invokes) still uses config.MustGetBaseConfig().WatchNamespaces instead of the reconciler's scoped config. The PR's goal of scoping orphan cleanup per reconciler is therefore not met: with per-reconciler watch namespaces it will still delete Roles/RoleBindings in unrelated namespaces and can miss the reconciler's own namespaces. Thread baseConf into deleteOrphaned and use it for both the VPA flag and the watch namespaces, mirroring the creation path.</violation>
</file>

<file name="internal/controller/operator/vmagent_controller_test.go">

<violation number="1" location="internal/controller/operator/vmagent_controller_test.go:191">
P2: TestVMAgent_Reconcile_SkipsUnselectedNamespaces does not actually exercise the regression the PR fixes. It puts the unrelated 'cleanup' namespace in the per-reconciler WatchNamespaces, but the production path this fix changes (createK8sAPIAccess using config.MustGetBaseConfig().WatchNamespaces) is driven by the shared global config, which the test leaves untouched. Because rbacNamespaces() already restricts to cr.Namespace when the VMAgent has no namespace selectors, this test passes both before and after the fix, so it cannot catch a regression. To guard the fix, set the global config's WatchNamespaces to include the unrelated namespace and keep the reconciler config scoped to the agent namespace, then assert no Role/RoleBinding is created there.</violation>
</file>

<file name="docs/CHANGELOG.md">

<violation number="1" location="docs/CHANGELOG.md:33">
P2: Custom agent: **Changelog Review Agent**

This new tip entry omits the mandatory issue or pull-request reference. Add a relevant `See [#...](...)` reference before the period.</violation>
</file>

<sub>Tip: instead of fixing issues one by one fix them all with cubic

Re-trigger cubic</sub>

[cubic-dev-ai]

P1: When a VMAgent reconciler’s BaseConf.WatchNamespaces differs from the process-global configuration, this path creates RBAC in one namespace set but orphan cleanup uses another. During RBAC cleanup, it can delete objects managed for another reconciler or leave this reconciler’s old Roles and RoleBindings behind; pass the reconciler namespace set through orphan cleanup and apply the same rbacNamespaces selection.

Prompt for AI agents

Check if this issue is valid — if so, understand the root cause and fix it. At internal/controller/operator/factory/vmagent/vmagent.go, line 134:

<comment>When a VMAgent reconciler’s `BaseConf.WatchNamespaces` differs from the process-global configuration, this path creates RBAC in one namespace set but orphan cleanup uses another. During RBAC cleanup, it can delete objects managed for another reconciler or leave this reconciler’s old Roles and RoleBindings behind; pass the reconciler namespace set through orphan cleanup and apply the same `rbacNamespaces` selection.</comment>

<file context>
@@ -128,6 +128,10 @@ func createOrUpdateService(ctx context.Context, rclient client.Client, cr, prevC
+	return CreateOrUpdateWithConfig(ctx, cr, rclient, config.MustGetBaseConfig())
+}
+
+func CreateOrUpdateWithConfig(ctx context.Context, cr *vmv1beta1.VMAgent, rclient client.Client, baseConf *config.BaseOperatorConf) error {
 	if cr.Paused() {
 		return nil
</file context>

[cubic-dev-ai]

P2: The RBAC cleanup during prev-state reconcile (deleteOrphaned, called from CreateOrUpdateWithConfig that this line now invokes) still uses config.MustGetBaseConfig().WatchNamespaces instead of the reconciler's scoped config. The PR's goal of scoping orphan cleanup per reconciler is therefore not met: with per-reconciler watch namespaces it will still delete Roles/RoleBindings in unrelated namespaces and can miss the reconciler's own namespaces. Thread baseConf into deleteOrphaned and use it for both the VPA flag and the watch namespaces, mirroring the creation path.

Prompt for AI agents

Check if this issue is valid — if so, understand the root cause and fix it. At internal/controller/operator/vmagent_controller.go, line 121:

<comment>The RBAC cleanup during prev-state reconcile (deleteOrphaned, called from CreateOrUpdateWithConfig that this line now invokes) still uses config.MustGetBaseConfig().WatchNamespaces instead of the reconciler's scoped config. The PR's goal of scoping orphan cleanup per reconciler is therefore not met: with per-reconciler watch namespaces it will still delete Roles/RoleBindings in unrelated namespaces and can miss the reconciler's own namespaces. Thread baseConf into deleteOrphaned and use it for both the VPA flag and the watch namespaces, mirroring the creation path.</comment>

<file context>
@@ -118,7 +118,7 @@ func (r *VMAgentReconciler) Reconcile(ctx context.Context, req ctrl.Request) (re
 
 	result, err = reconcileAndTrackStatus(ctx, r.Client, instance.DeepCopy(), r.name, func() (ctrl.Result, error) {
-		if err := vmagent.CreateOrUpdate(ctx, &instance, r); err != nil {
+		if err := vmagent.CreateOrUpdateWithConfig(ctx, &instance, r, r.BaseConf); err != nil {
 			return result, err
 		}
</file context>

[cubic-dev-ai]

P2: TestVMAgent_Reconcile_SkipsUnselectedNamespaces does not actually exercise the regression the PR fixes. It puts the unrelated 'cleanup' namespace in the per-reconciler WatchNamespaces, but the production path this fix changes (createK8sAPIAccess using config.MustGetBaseConfig().WatchNamespaces) is driven by the shared global config, which the test leaves untouched. Because rbacNamespaces() already restricts to cr.Namespace when the VMAgent has no namespace selectors, this test passes both before and after the fix, so it cannot catch a regression. To guard the fix, set the global config's WatchNamespaces to include the unrelated namespace and keep the reconciler config scoped to the agent namespace, then assert no Role/RoleBinding is created there.

Prompt for AI agents

Check if this issue is valid — if so, understand the root cause and fix it. At internal/controller/operator/vmagent_controller_test.go, line 191:

<comment>TestVMAgent_Reconcile_SkipsUnselectedNamespaces does not actually exercise the regression the PR fixes. It puts the unrelated 'cleanup' namespace in the per-reconciler WatchNamespaces, but the production path this fix changes (createK8sAPIAccess using config.MustGetBaseConfig().WatchNamespaces) is driven by the shared global config, which the test leaves untouched. Because rbacNamespaces() already restricts to cr.Namespace when the VMAgent has no namespace selectors, this test passes both before and after the fix, so it cannot catch a regression. To guard the fix, set the global config's WatchNamespaces to include the unrelated namespace and keep the reconciler config scoped to the agent namespace, then assert no Role/RoleBinding is created there.</comment>

<file context>
@@ -172,3 +176,62 @@ func TestVMAgent_Reconcile_AgentSync_Unmanaged(t *testing.T) {
+	}
+	fclient := k8stools.GetTestClientWithObjects([]runtime.Object{vmagent})
+	reconciler := &VMAgentReconciler{}
+	reconciler.Init("vmagent", fclient, logr.Discard(), scheme.Scheme, &config.BaseOperatorConf{WatchNamespaces: []string{agentNamespace, cleanupNamespace}})
+
+	_, err := reconciler.Reconcile(context.Background(), reconcile.Request{NamespacedName: types.NamespacedName{Name: vmagent.Name, Namespace: vmagent.Namespace}})
</file context>

[cubic-dev-ai]

P2: VMAgent RBAC creation now uses the reconciler's baseConf.WatchNamespaces (via rbacNamespaces), but the counterpart orphan cleanup in deleteOrphaned still reads config.MustGetBaseConfig().WatchNamespaces. When a reconciler's config differs from the shared global config, creation and cleanup operate on different namespace sets. Pass baseConf.WatchNamespaces into deleteOrphaned for VMAgent the same way this PR did for VMSingle.

Prompt for AI agents

Check if this issue is valid — if so, understand the root cause and fix it. At internal/controller/operator/factory/vmagent/vmagent.go, line 157:

<comment>VMAgent RBAC creation now uses the reconciler's `baseConf.WatchNamespaces` (via `rbacNamespaces`), but the counterpart orphan cleanup in `deleteOrphaned` still reads `config.MustGetBaseConfig().WatchNamespaces`. When a reconciler's config differs from the shared global config, creation and cleanup operate on different namespace sets. Pass `baseConf.WatchNamespaces` into `deleteOrphaned` for VMAgent the same way this PR did for VMSingle.</comment>

<file context>
@@ -150,7 +154,7 @@ func CreateOrUpdate(ctx context.Context, cr *vmv1beta1.VMAgent, rclient client.C
 		}
 		if !ptr.Deref(cr.Spec.IngestOnlyMode, false) || cr.HasRemoteWriteSecrets() {
-			if err := createK8sAPIAccess(ctx, rclient, cr, prevCR, cfg.WatchNamespaces); err != nil {
+			if err := createK8sAPIAccess(ctx, rclient, cr, prevCR, rbacNamespaces(cr, baseConf.WatchNamespaces)); err != nil {
 				return fmt.Errorf("cannot create vmagent role and binding for it, err: %w", err)
 			}
</file context>

[cubic-dev-ai]

P2: Custom agent: Changelog Review Agent

This new tip entry omits the mandatory issue or pull-request reference. Add a relevant See [#...](...) reference before the period.

Prompt for AI agents

Check if this issue is valid — if so, understand the root cause and fix it. At docs/CHANGELOG.md, line 33:

<comment>This new tip entry omits the mandatory issue or pull-request reference. Add a relevant `See [#...](...)` reference before the period.</comment>

<file context>
@@ -30,6 +30,7 @@ aliases:
 * BUGFIX: [vmoperator](https://docs.victoriametrics.com/operator/): fix reconciliation error on Kubernetes 1.29 caused by setting an empty `preStop` lifecycle handler. The native `Sleep` preStop action now requires Kubernetes >= 1.30, since the `PodLifecycleSleepAction` feature gate is not enabled by default on 1.29.
 * BUGFIX: [vmagent](https://docs.victoriametrics.com/operator/resources/vmagent/): removing `spec.hpa` no longer leaves the previously created `HorizontalPodAutoscaler` behind. `HorizontalPodAutoscaler` and `VerticalPodAutoscaler` objects created for `VMAgent` and `VMAnomaly` now use the operator's consistent naming convention for child objects (e.g. `vmagent-<name>`) instead of the bare CR name; a leftover object under the old name is cleaned up automatically on the next reconcile. See [#2518](https://github.com/VictoriaMetrics/operator/issues/2518).
 * BUGFIX: [vmuser](https://docs.victoriametrics.com/operator/resources/vmuser/): fix multiple `VMUser` resources configured with `spec.jwt` in the same namespace being treated as duplicates and dropped from the `vmauth` config, since they weren't keyed by their own name. See [#2532](https://github.com/VictoriaMetrics/operator/issues/2532).
+* BUGFIX: [vmagent](https://docs.victoriametrics.com/operator/resources/vmagent/), [vmsingle](https://docs.victoriametrics.com/operator/resources/vmsingle/): create and remove RBAC resources in namespaces configured for each reconciler.
 
 ## [v0.74.1](https://github.com/VictoriaMetrics/operator/releases/tag/v0.74.1)
</file context>

[vrutkovs]

Tested this in e2e tests - seems to help with resources cleanup

[AndrewChubatiuk]

this function also makes sense for vmsingle