Skip to content

fix: restoring tracking labels for cleaning up of orphaned roles and rolebindings#2017

Merged
svghadi merged 12 commits intoargoproj-labs:masterfrom
akhilnittala:usr/akhil/GITOPS-8537
Feb 3, 2026
Merged

fix: restoring tracking labels for cleaning up of orphaned roles and rolebindings#2017
svghadi merged 12 commits intoargoproj-labs:masterfrom
akhilnittala:usr/akhil/GITOPS-8537

Conversation

@akhilnittala
Copy link
Copy Markdown
Contributor

@akhilnittala akhilnittala commented Jan 9, 2026

What type of PR is this?

/kind chore

What does this PR do / why we need it:
Identifies Roles and RoleBindings associated with namespace-scoped Argo CD instances and restores any missing labels to ensure correct resource tracking and cleanup.
Have you updated the necessary documentation?

  • Documentation update is required by this PR.
  • Documentation has been updated.

Which issue(s) this PR fixes:
https://issues.redhat.com/browse/GITOPS-8537
Fixes #?
https://issues.redhat.com/browse/GITOPS-8537
How to test changes / Special notes to the reviewer:
First reproduce the issue based on the steps mentioned in the jira

Install gitops operator with changes and verify the resources which are not cleanedup as per above step.

Summary by CodeRabbit

  • New Features

    • Automatically restores missing tracking labels on orphaned namespaces, applying only required labels, skipping the controller's own namespace, logging updates, and aggregating errors encountered.
  • Tests

    • Added tests covering multiple namespace and RBAC scenarios to verify label restoration, immutability of the controller namespace, and error aggregation.

…ings for namespacescoped argocd instance

Signed-off-by: akhil nittala <nakhil@redhat.com>
@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented Jan 9, 2026

📝 Walkthrough

Walkthrough

Adds logic to reconcile orphaned Roles: finds Roles labeled for ArgoCD that reference Application/ApplicationSet source namespaces, verifies they are orphaned and application-scoped, and restores missing ArgoCD tracking labels on the referenced Namespaces before continuing reconciliation. Includes unit test coverage.

Changes

Cohort / File(s) Summary
Orphan namespace label restoration
controllers/argocd/argocd_controller.go
Inserted call to restoreTrackingLabelsForOrphanedNamespaces in internalReconcile. Added unexported helpers to list Roles with ArgoCD labels, validate orphaned Roles (isOrphanedRole, hasApplicationScopedRules), compute required tracking labels, and apply missing labels to Namespaces (skipping ArgoCD's own namespace). Aggregates errors and logs updates.
Test coverage for label restoration
controllers/argocd/argocd_controller_test.go
Imported maps and added Test_restoreTrackingLabelsForOrphanedNamespaces to exercise multiple namespace/Role scenarios using a fake client; asserts labels are restored where appropriate.

Sequence Diagram(s)

sequenceDiagram
    participant Reconciler as Reconciler
    participant K8sAPI as Kubernetes API
    participant RoleValidator as Role Validator
    participant NSMutator as Namespace Mutator

    Reconciler->>K8sAPI: List Roles with ArgoCD labels
    K8sAPI-->>Reconciler: Roles list

    loop per Role
        Reconciler->>RoleValidator: isOrphanedRole(role)
        RoleValidator->>RoleValidator: check name vs App/AppSet sources
        RoleValidator->>RoleValidator: hasApplicationScopedRules()
        RoleValidator-->>Reconciler: valid / invalid

        alt valid orphan role
            Reconciler->>K8sAPI: Get referenced Namespace (skip ArgoCD namespace)
            K8sAPI-->>Reconciler: Namespace

            Reconciler->>RoleValidator: requiredTrackingLabelsForRole(role)
            RoleValidator-->>Reconciler: labels to ensure

            Reconciler->>NSMutator: addMissingLabels(namespace, labels)
            NSMutator->>K8sAPI: Patch/Update Namespace
            K8sAPI-->>NSMutator: Patch result
            NSMutator-->>Reconciler: updated / no-op
        end
    end

    Reconciler-->>Reconciler: Aggregate errors and continue
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Poem

🐰 I hopped through Roles and Namespace lanes,
Restored the labels, mended the chains.
Orphaned tags now found their home,
RBAC checked, no more to roam.
A tiny hop — reconciliation done.

🚥 Pre-merge checks | ✅ 2 | ❌ 1
❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 50.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately describes the main change: restoring tracking labels for orphaned roles and rolebindings to enable cleanup.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

…ings for namespacescoped argocd instance

Signed-off-by: akhil nittala <nakhil@redhat.com>
…ings for namespacescoped argocd instance

Signed-off-by: akhil nittala <nakhil@redhat.com>
@akhilnittala akhilnittala marked this pull request as ready for review January 19, 2026 06:37
…ings for namespacescoped argocd instance

Signed-off-by: akhil nittala <nakhil@redhat.com>
…ings for namespacescoped argocd instance

Signed-off-by: akhil nittala <nakhil@redhat.com>
Comment thread controllers/argocd/argocd_controller.go Outdated
Comment thread controllers/argocd/argocd_controller.go Outdated
…ings for namespacescoped argocd instance

Signed-off-by: akhil nittala <nakhil@redhat.com>
Comment thread controllers/argocd/argocd_controller.go Outdated
…ings for namespacescoped argocd instance

Signed-off-by: akhil nittala <nakhil@redhat.com>
Copy link
Copy Markdown
Collaborator

@anandf anandf left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🤖 Fix all issues with AI agents
In `@controllers/argocd/argocd_controller.go`:
- Around line 448-467: The hasApplicationScopedRules function misses wildcard
resources like "*" and therefore returns false for roles that actually grant
access; update the logic in hasApplicationScopedRules to treat a resource entry
of "*" (and optionally the group-wide wildcard) as a match: when iterating
rule.Resources inside hasApplicationScopedRules, first check if res == "*" (or
rule.Resources contains "*") and return true if so, otherwise continue the
existing switch that checks for "applications", "applications/status",
"applicationsets", "applicationsets/status"; ensure the check occurs only after
confirming the rule.APIGroups contains "argoproj.io" (const argoCDAPIGroup) so
wildcard resources in that API group are correctly recognized.

Comment thread controllers/argocd/argocd_controller.go
…ings for namespacescoped argocd instance

Signed-off-by: akhil nittala <nakhil@redhat.com>
…ings for namespacescoped argocd instance

Signed-off-by: akhil nittala <nakhil@redhat.com>
…ings for namespacescoped argocd instance

Signed-off-by: akhil nittala <nakhil@redhat.com>
…ings for namespacescoped argocd instance

Signed-off-by: akhil nittala <nakhil@redhat.com>
@svghadi svghadi changed the title chore: restoring missing labels for cleaning up of roles and rolebindings for namespacescoped argocd instance fix: restoring missing labels for cleaning up of orphaned roles and rolebindings Jan 27, 2026
@svghadi svghadi changed the title fix: restoring missing labels for cleaning up of orphaned roles and rolebindings fix: restoring tracking labels for cleaning up of orphaned roles and rolebindings Jan 27, 2026
Comment thread controllers/argocd/argocd_controller.go Outdated
Comment thread controllers/argocd/argocd_controller.go Outdated
…ings for namespacescoped argocd instance

Signed-off-by: akhil nittala <nakhil@redhat.com>
Copy link
Copy Markdown
Collaborator

@svghadi svghadi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. Thanks

@svghadi svghadi merged commit c057992 into argoproj-labs:master Feb 3, 2026
8 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants