Skip to content

CNV-59877: Fix permissions request for NADs#352

Merged
openshift-merge-bot[bot] merged 1 commit intoopenshift:mainfrom
upalatucci:fix/CNV-59877
Feb 25, 2026
Merged

CNV-59877: Fix permissions request for NADs#352
openshift-merge-bot[bot] merged 1 commit intoopenshift:mainfrom
upalatucci:fix/CNV-59877

Conversation

@upalatucci
Copy link
Contributor

@upalatucci upalatucci commented Feb 25, 2026

Summary

  • Adds namespace to the useAccessReview hook in NetworkAttachmentDefinitionList so it checks namespace-scoped RBAC permissions instead of cluster-level access
  • Passes the access review result to the NADListEmpty component so the "Create NetworkAttachmentDefinition" button is hidden when the user lacks create permission for NADs in the current namespace

Fixes: https://issues.redhat.com/browse/CNV-59877

Test plan

  • Login as a non-privileged user
  • Navigate to the NetworkAttachmentDefinitions page in a namespace where the user does not have NAD create permission
  • Verify the "Create NetworkAttachmentDefinition" button is not shown (both in the empty list state and the empty filtered state)
  • Switch to a namespace where the user does have NAD create permission
  • Verify the "Create NetworkAttachmentDefinition" button is shown

Made with Cursor

Summary by CodeRabbit

  • Bug Fixes
    • Network Attachment Definition creation button now respects user permissions and only appears for users with appropriate access rights.

Add namespace to the useAccessReview hook so it checks namespace-scoped
RBAC instead of cluster-level access. Pass the permission result to
NADListEmpty so the create button is hidden when the user lacks
permission to create NetworkAttachmentDefinitions.

Co-authored-by: Cursor <cursoragent@cursor.com>
@openshift-ci-robot
Copy link

openshift-ci-robot commented Feb 25, 2026

@upalatucci: This pull request references CNV-59877 which is a valid jira issue.

Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the bug to target the "4.22.0" version, but no target version was set.

Details

In response to this:

Summary

  • Adds namespace to the useAccessReview hook in NetworkAttachmentDefinitionList so it checks namespace-scoped RBAC permissions instead of cluster-level access
  • Passes the access review result to the NADListEmpty component so the "Create NetworkAttachmentDefinition" button is hidden when the user lacks create permission for NADs in the current namespace

Fixes: https://issues.redhat.com/browse/CNV-59877

Test plan

  • Login as a non-privileged user
  • Navigate to the NetworkAttachmentDefinitions page in a namespace where the user does not have NAD create permission
  • Verify the "Create NetworkAttachmentDefinition" button is not shown (both in the empty list state and the empty filtered state)
  • Switch to a namespace where the user does have NAD create permission
  • Verify the "Create NetworkAttachmentDefinition" button is shown

Made with Cursor

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 openshift-eng/jira-lifecycle-plugin repository.

@openshift-ci-robot openshift-ci-robot added the jira/valid-reference Indicates that this PR references a valid Jira ticket of any type. label Feb 25, 2026
@openshift-ci openshift-ci bot requested review from gouyang and metalice February 25, 2026 09:46
@openshift-ci openshift-ci bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Feb 25, 2026
@coderabbitai
Copy link

coderabbitai bot commented Feb 25, 2026

Walkthrough

Updated NetworkAttachmentDefinition list component to support namespace-aware permission checking. The parent component now passes a canCreateNAD prop to NADListEmpty, which conditionally renders the Create button based on this permission flag instead of always displaying it.

Changes

Cohort / File(s) Summary
Permission-based NAD creation rendering
src/views/nads/list/NetworkAttachmentDefinitionList.tsx, src/views/nads/list/components/NADListEmpty/NADListEmpty.tsx
Added namespace parameter to canCreateNAD access control check and passed it as a prop to NADListEmpty. Updated NADListEmpty to accept optional canCreateNAD prop (default true) and conditionally render the Create button only when permissions allow.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Stable And Deterministic Test Names ✅ Passed PR modifies React components without adding or modifying test files; no test names or identifiers are present.
Test Structure And Quality ✅ Passed No test files were modified or added in this PR, so test structure and quality requirements are not applicable.
Title check ✅ Passed The title accurately reflects the main change: fixing permissions handling for NAD (NetworkAttachmentDefinition) creation by adding namespace scope to access review checks and conditionally hiding the create button.

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

✨ Finishing Touches
  • 📝 Generate docstrings (stacked PR)
  • 📝 Generate docstrings (commit on current branch)
🧪 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.

@upalatucci upalatucci changed the title CNV-59877: Hide NAD create button for users without create permission CNV-59877: Fix permissions request for NADs Feb 25, 2026
@openshift-ci-robot
Copy link

openshift-ci-robot commented Feb 25, 2026

@upalatucci: This pull request references CNV-59877 which is a valid jira issue.

Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the bug to target the "4.22.0" version, but no target version was set.

Details

In response to this:

Summary

  • Adds namespace to the useAccessReview hook in NetworkAttachmentDefinitionList so it checks namespace-scoped RBAC permissions instead of cluster-level access
  • Passes the access review result to the NADListEmpty component so the "Create NetworkAttachmentDefinition" button is hidden when the user lacks create permission for NADs in the current namespace

Fixes: https://issues.redhat.com/browse/CNV-59877

Test plan

  • Login as a non-privileged user
  • Navigate to the NetworkAttachmentDefinitions page in a namespace where the user does not have NAD create permission
  • Verify the "Create NetworkAttachmentDefinition" button is not shown (both in the empty list state and the empty filtered state)
  • Switch to a namespace where the user does have NAD create permission
  • Verify the "Create NetworkAttachmentDefinition" button is shown

Made with Cursor

Summary by CodeRabbit

  • Bug Fixes
  • Network Attachment Definition creation button now respects user permissions and only appears for users with appropriate access rights.

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 openshift-eng/jira-lifecycle-plugin repository.

Copy link

@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.

🧹 Nitpick comments (1)
src/views/nads/list/components/NADListEmpty/NADListEmpty.tsx (1)

23-27: Consider making canCreateNAD required instead of defaulting to true.

Using a permissive default can silently reintroduce the button if a future call site forgets to pass permission state. Requiring the prop makes this safer at compile time.

Suggested tightening of prop contract
 type NADListEmptyProps = {
-  canCreateNAD?: boolean;
+  canCreateNAD: boolean;
   namespace: string;
 };
 
-const NADListEmpty: FC<NADListEmptyProps> = ({ canCreateNAD = true, namespace }) => {
+const NADListEmpty: FC<NADListEmptyProps> = ({ canCreateNAD, namespace }) => {
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/views/nads/list/components/NADListEmpty/NADListEmpty.tsx` around lines 23
- 27, The prop canCreateNAD on NADListEmpty (declared in NADListEmptyProps and
used in the NADListEmpty functional component) should be made required instead
of optional with a default true: remove the "?" from canCreateNAD in the
NADListEmptyProps type and remove the default assignment ({ canCreateNAD = true,
namespace }) in the NADListEmpty component signature; update any call sites that
render NADListEmpty to explicitly pass a boolean for canCreateNAD so the
compiler enforces permission handling.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Nitpick comments:
In `@src/views/nads/list/components/NADListEmpty/NADListEmpty.tsx`:
- Around line 23-27: The prop canCreateNAD on NADListEmpty (declared in
NADListEmptyProps and used in the NADListEmpty functional component) should be
made required instead of optional with a default true: remove the "?" from
canCreateNAD in the NADListEmptyProps type and remove the default assignment ({
canCreateNAD = true, namespace }) in the NADListEmpty component signature;
update any call sites that render NADListEmpty to explicitly pass a boolean for
canCreateNAD so the compiler enforces permission handling.

ℹ️ Review info

Configuration used: Repository: openshift/coderabbit/.coderabbit.yaml

Review profile: CHILL

Plan: Pro

Cache: Disabled due to data retention organization setting

Knowledge base: Disabled due to data retention organization setting

📥 Commits

Reviewing files that changed from the base of the PR and between 0ca0a0b and a9ba62b.

📒 Files selected for processing (2)
  • src/views/nads/list/NetworkAttachmentDefinitionList.tsx
  • src/views/nads/list/components/NADListEmpty/NADListEmpty.tsx

@batyana
Copy link
Contributor

batyana commented Feb 25, 2026

/lgtm

@openshift-ci openshift-ci bot added the lgtm Indicates that a PR is ready to be merged. label Feb 25, 2026
@openshift-ci
Copy link

openshift-ci bot commented Feb 25, 2026

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: adamviktora, upalatucci

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 [adamviktora,upalatucci]

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

@upalatucci
Copy link
Contributor Author

/retest

@openshift-ci
Copy link

openshift-ci bot commented Feb 25, 2026

@upalatucci: all tests passed!

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.

@openshift-merge-bot openshift-merge-bot bot merged commit 86f180a into openshift:main Feb 25, 2026
6 checks passed
@upalatucci
Copy link
Contributor Author

/cherry-pick release-4.21

@openshift-cherrypick-robot

@upalatucci: new pull request created: #353

Details

In response to this:

/cherry-pick release-4.21

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

approved Indicates a PR has been approved by an approver from all required OWNERS files. jira/valid-reference Indicates that this PR references a valid Jira ticket of any type. lgtm Indicates that a PR is ready to be merged.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants