Restrict VTEP write RBAC to status/finalizers subresources

[kyrtapz]

No description provided.

[coderabbitai]

Walkthrough

RBAC permissions for OVN virtual tunnel endpoints (vteps) were reorganized in the control plane ClusterRole. Verbs update and patch were removed from vteps/status and the EVPN-gated vteps resource block, then added to a new vteps/finalizers resource entry.

Changes

Cohort / File(s)	Summary
RBAC ClusterRole Configuration bindata/network/ovn-kubernetes/common/004-rbac-control-plane.yaml	Reorganized permissions for OVN vteps resource: removed update and patch verbs from vteps/status, removed these verbs from EVPN-gated vteps block, and added new vteps/finalizers resource with patch and update verbs.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests

---

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

[tssurya]

/lgtm

[openshift-ci]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: kyrtapz, tssurya

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Details

Needs approval from an approver in each of these files:

• OWNERS [kyrtapz,tssurya]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

[coderabbitai]

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)

bindata/network/ovn-kubernetes/common/004-rbac-control-plane.yaml (1)

268-276: ⚠️ Potential issue | 🟠 Major

Add finalizers subresource declaration to the vteps CRD.

The RBAC rule grants patch/update on vteps/finalizers, but the vteps CRD (line 5391 in 001-crd.yaml) only declares subresources.status: {} without declaring the finalizers subresource. This mismatch means the finalizers RBAC grant will not be effective. Add finalizers: {} to the subresources section:

subresources:
  status: {}
  finalizers: {}

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@bindata/network/ovn-kubernetes/common/004-rbac-control-plane.yaml` around
lines 268 - 276, The RBAC grants patch/update on the vteps/finalizers
subresource but the vteps CRD only declares subresources.status, so add the
missing finalizers subresource to the vteps CRD: locate the vteps
CustomResourceDefinition (the CRD that currently contains subresources.status:
{}) and update its subresources block to include finalizers: {} alongside status
so the vteps/finalizers RBAC rule becomes effective.

🤖 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 `@bindata/network/ovn-kubernetes/common/004-rbac-control-plane.yaml`:
- Around line 268-276: The RBAC grants patch/update on the vteps/finalizers
subresource but the vteps CRD only declares subresources.status, so add the
missing finalizers subresource to the vteps CRD: locate the vteps
CustomResourceDefinition (the CRD that currently contains subresources.status:
{}) and update its subresources block to include finalizers: {} alongside status
so the vteps/finalizers RBAC rule becomes effective.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 26934d51-d619-4d0e-a7aa-8a5e6a862e2d

📥 Commits

Reviewing files that changed from the base of the PR and between f3e41c5 and 834a38f.

📒 Files selected for processing (1)

• bindata/network/ovn-kubernetes/common/004-rbac-control-plane.yaml

[kyrtapz]

/hold
#2956 (review)

[openshift-ci]

@kyrtapz: The following tests failed, say /retest to rerun all failed tests or /retest-required to rerun all mandatory failed tests:

Test name	Commit	Details	Required	Rerun command
ci/prow/e2e-aws-ovn-upgrade	834a38f	link	true	/test e2e-aws-ovn-upgrade
ci/prow/e2e-metal-ipi-ovn-dualstack-bgp	834a38f	link	true	/test e2e-metal-ipi-ovn-dualstack-bgp
ci/prow/security	834a38f	link	false	/test security
ci/prow/e2e-metal-ipi-ovn-ipv6	834a38f	link	true	/test e2e-metal-ipi-ovn-ipv6
ci/prow/e2e-aws-ovn-rhcos10-techpreview	834a38f	link	false	/test e2e-aws-ovn-rhcos10-techpreview
ci/prow/e2e-ovn-ipsec-step-registry	834a38f	link	true	/test e2e-ovn-ipsec-step-registry
ci/prow/e2e-metal-ipi-ovn-dualstack-bgp-local-gw	834a38f	link	true	/test e2e-metal-ipi-ovn-dualstack-bgp-local-gw

Full PR test history. Your PR dashboard.

Details

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. I understand the commands that are listed here.

[tssurya]

@coderabbitai: are you sure about your comment? see kubernetes/enhancements#3286

[coderabbitai]

Tip

For best results, initiate chat on the files or code changes.

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Get the review comment I made on the PR
gh api repos/openshift/cluster-network-operator/pulls/2956/reviews --jq '.[] | select(.user.login == "coderabbitai") | {id: .id, body: .body, state: .state}'

Length of output: 0

---

🏁 Script executed:

#!/bin/bash
# Also get review comments (inline) from coderabbitai
gh api repos/openshift/cluster-network-operator/pulls/2956/comments --jq '.[] | select(.user.login == "coderabbitai") | {id: .id, body: .body, path: .path, line: .line}'

Length of output: 0

---

🏁 Script executed:

#!/bin/bash
# Fetch info about the referenced KEP PR
gh pr view 3286 --repo kubernetes/enhancements --json title,body,url,state | jq .

Length of output: 875

---

🏁 Script executed:

#!/bin/bash
# Get all reviews on the PR (not filtered) to find my comment
gh api repos/openshift/cluster-network-operator/pulls/2956/reviews --jq '.[] | {id: .id, user: .user.login, body: .body, state: .state}'

Length of output: 3784

---

🏁 Script executed:

#!/bin/bash
# Get the KEP-3285 issue details
gh issue view 3285 --repo kubernetes/enhancements --json title,body,url,state | jq .

Length of output: 1486

---

🏁 Script executed:

#!/bin/bash
# Look at the actual diff/files in the PR
gh pr view 2956 --repo openshift/cluster-network-operator --json files | jq '.files[].path'

Length of output: 67

---

@tssurya, you are correct, and I apologize for the incorrect review comment. My comment was wrong.

KEP-3285 ("Subresource finalizers", stable since v1.28) introduced <resource>/finalizers as a built-in RBAC subresource available for all resources — including CRDs — without requiring an explicit finalizers: {} declaration in the CRD subresources block. Unlike status (which must be opted into via the CRD spec), finalizers is part of metadata and is always present, so the API machinery handles it implicitly.

The RBAC grant on vteps/finalizers is therefore valid and effective as-is. No CRD changes are needed. The PR's approach is correct.

---

📝 Learnings were identified and not saved (knowledge base disabled). Enable

[kyrtapz]

vteps/finalizers is not a valid resource.