Skip to content

fix: add deployments/finalizers RBAC marker for AgentCardSync controller#240

Open
Bobbins228 wants to merge 2 commits intokagenti:mainfrom
Bobbins228:fix/deployments-finalizers-rbac-RHAIENG-3819
Open

fix: add deployments/finalizers RBAC marker for AgentCardSync controller#240
Bobbins228 wants to merge 2 commits intokagenti:mainfrom
Bobbins228:fix/deployments-finalizers-rbac-RHAIENG-3819

Conversation

@Bobbins228
Copy link
Copy Markdown
Contributor

Problem

The AgentCardSync controller sets blockOwnerDeletion: true on ownerReference entries for AgentCard objects owned by Deployment resources. Kubernetes requires that the acting service account hold update permission on the apps/deployments/finalizers subresource to do this — without it, the API server returns a hard denial on every create/update of the AgentCard.

The +kubebuilder:rbac markers in agentcardsync_controller.go covered apps/deployments (get, list, watch) but not apps/deployments/finalizers. As a result, controller-gen omitted the rule from config/rbac/role.yaml, leaving the kustomize-based install path non-functional on clusters with strict RBAC enforcement. The AgentCardSync reconciler would loop indefinitely, never producing AgentCard objects.

Solution

Added the missing +kubebuilder:rbac marker to agentcardsync_controller.go and regenerated config/rbac/role.yaml via make manifests.

Files Changed

  • internal/controller/agentcardsync_controller.go — added +kubebuilder:rbac:groups=apps,resources=deployments/finalizers,verbs=update
  • config/rbac/role.yaml — regenerated; new deployments/finalizers: update rule added

Made with Cursor

@Bobbins228 Bobbins228 requested a review from a team as a code owner March 23, 2026 10:20
    Kubernetes requires update permission on apps/deployments/finalizers
    when blockOwnerDeletion=true is set on ownerReferences. The +kubebuilder:rbac
    marker was missing from agentcardsync_controller.go, causing controller-gen
    to omit the rule from config/rbac/role.yaml. This caused a hard RBAC denial
    on every reconcile loop on HyperShift/ROSA clusters.

    Fixes RHAIENG-3819

    Assisted-By: Claude
    Made-with: Cursor

Signed-off-by: Bobbins228 <mcampbel@redhat.com>
@Bobbins228 Bobbins228 force-pushed the fix/deployments-finalizers-rbac-RHAIENG-3819 branch from ca73b26 to 9f2c89b Compare March 23, 2026 10:22
Copy link
Copy Markdown
Contributor

@r3v5 r3v5 left a comment

Choose a reason for hiding this comment

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

Great work, Mark! Should we do the same for statefulsets/finalizers ?

Copy link
Copy Markdown
Contributor

@r3v5 r3v5 left a comment

Choose a reason for hiding this comment

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

LGTM!

…ller

    The AgentCardSync controller reconciles both Deployments and StatefulSets,
    calling controllerutil.SetControllerReference (blockOwnerDeletion: true) for
    both. Kubernetes requires statefulsets/finalizers:update to set
    blockOwnerDeletion on a StatefulSet owner — enforced as a hard denial on
    HyperShift/ROSA. Adds the missing +kubebuilder:rbac marker and regenerates
    config/rbac/role.yaml. The Helm chart already carries this permission.

    Made-with: Cursor

Signed-off-by: Bobbins228 <mcampbel@redhat.com>
@Bobbins228
Copy link
Copy Markdown
Contributor Author

Good catch @r3v5 updated now and verified thanks for the review

Copy link
Copy Markdown
Collaborator

@cwiklik cwiklik left a comment

Choose a reason for hiding this comment

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

Targeted RBAC fix for the kustomize install path. The AgentCardSync controller uses blockOwnerDeletion: true via SetControllerReference on both Deployments and StatefulSets, which requires update on the /finalizers subresource. The Helm chart already had these permissions; this PR aligns the kubebuilder markers and regenerated config/rbac/role.yaml to match.

Verified: Helm chart at charts/kagenti-operator/templates/rbac/role.yaml already carries both deployments/finalizers and statefulsets/finalizers — this brings kustomize into parity.

Areas reviewed: Go (RBAC markers), YAML (role.yaml), Security (RBAC scope)
Commits: 2 commits, all signed-off ✓
CI status: All 14 checks passing ✓

Clean, well-scoped fix. Good catch from @r3v5 on the StatefulSet gap.

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