Skip to content

feat: [UIE-8934] - IAM RBAC: add a permission check for linode storage tab#12484

Merged
jaalah-akamai merged 4 commits intolinode:developfrom
aaleksee-akamai:UIE-8934-linodes-tab-storage
Jul 8, 2025
Merged

feat: [UIE-8934] - IAM RBAC: add a permission check for linode storage tab#12484
jaalah-akamai merged 4 commits intolinode:developfrom
aaleksee-akamai:UIE-8934-linodes-tab-storage

Conversation

@aaleksee-akamai
Copy link
Contributor

@aaleksee-akamai aaleksee-akamai commented Jul 8, 2025

Description 📝

Implement the new RBAC permission hook in Linodes storage tab

Changes 🔄

  1. Permissions Refactor
    Replaced usage of useRestrictedGlobalGrantCheck and related grant checks with a new usePermissions hook throughout Linodes storage tab
    All permission checks for adding, renamimg, resizing, cloning, and deleting linode disks now use the new usePermissions API, improving consistency and maintainability.

  2. Test Coverage
    Added new tests for permission-based UI states:
    Tests for enabling/disabling the "Add A Disk" button based on user permissions.
    Added tests for the LinodeDiskActionMenu to verify that action items are enabled/disabled according to permissions.

  3. Files Affected:

LinodeDisks.tsx + LinodeDisks.test.tsx
LinodeDiskActionMenu.tsx + LinodeDiskActionMenu.test.tsx

Target release date 🗓️

July 15th

Preview 📷

Before After
image image

How to test 🧪

Prerequisites

(How to setup test environment)

  • devcloud IAM account or local devenv setup or mock data
  • Mock data + turn Legacy MSW on + use Custom Profile preset and make it a Restricted account:
  http.get('*/iam/users/:username/permissions/:entity_type/:entity_id', () => {
    return HttpResponse.json([
      'update_linode_disk',
      'resize_linode_disk',
      'delete_linode_disk',
      'clone_linode',
    ]);
  }),

Verification steps

(How to verify changes)

  • Confirm the buttons are disabled according the grant model with IAM is off, and according to the RBAC model when IAM is on.
Author Checklists

As an Author, to speed up the review process, I considered 🤔

👀 Doing a self review
❔ Our contribution guidelines
🤏 Splitting feature into small PRs
➕ Adding a changeset
🧪 Providing/improving test coverage
🔐 Removing all sensitive information from the code and PR description
🚩 Using a feature flag to protect the release
👣 Providing comprehensive reproduction steps
📑 Providing or updating our documentation
🕛 Scheduling a pair reviewing session
📱 Providing mobile support
♿ Providing accessibility support


  • I have read and considered all applicable items listed above.

As an Author, before moving this PR from Draft to Open, I confirmed ✅

  • All unit tests are passing
  • TypeScript compilation succeeded without errors
  • Code passes all linting rules

@aaleksee-akamai aaleksee-akamai requested a review from a team as a code owner July 8, 2025 12:44
@aaleksee-akamai aaleksee-akamai requested review from hasyed-akamai and mjac0bs and removed request for a team July 8, 2025 12:45
@aaleksee-akamai aaleksee-akamai self-assigned this Jul 8, 2025
@aaleksee-akamai aaleksee-akamai added Do Not Merge Linodes Dealing with the Linodes section of the app IAM (Identity & Access Management) labels Jul 8, 2025
@cpathipa cpathipa added the Related Cypress Test Failure Test failure(s) are the result of changes in this PR and must be addressed before merging. label Jul 8, 2025
Copy link
Contributor

@cpathipa cpathipa left a comment

Choose a reason for hiding this comment

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

@aaleksee-akamai Shows new UI when the flag is off

image

@aaleksee-akamai
Copy link
Contributor Author

@aaleksee-akamai Shows new UI when the flag is off

image

you mean moving the buttons into the action menu? if so, it's correct

@aaleksee-akamai aaleksee-akamai requested a review from a team as a code owner July 8, 2025 16:17
@aaleksee-akamai aaleksee-akamai requested review from cliu-akamai and removed request for a team July 8, 2025 16:17
@aaleksee-akamai
Copy link
Contributor Author

@jdamore-linode hi! I've been trying to fix these tests, and it seems I still have a problem with it. Could you help me out or point the direction? Thanks!

@cpathipa
Copy link
Contributor

cpathipa commented Jul 8, 2025

@aaleksee-akamai Shows new UI when the flag is off
image

you mean moving the buttons into the action menu? if so, it's correct

When the flag is off it should be old UI right ?
image

@jdamore-linode
Copy link
Contributor

jdamore-linode commented Jul 8, 2025

@cpathipa this seems consistent with the other IAM PRs: Firewalls and Configurations, for example, both moved the table row buttons into the action menu and that change was not behind a feature flag in either of those cases 👍.

@aaleksee-akamai taking a look!

@jdamore-linode
Copy link
Contributor

Hey @aaleksee-akamai, just a heads up that I pushed a couple small changes that will hopefully get the tests passing 👍 you were on the right track! I just moved some of the action menu item interactions outside of the Cypress .within(() => { ... }) blocks since those menus technically exist elsewhere in the DOM.

@linode-gh-bot
Copy link
Collaborator

Cloud Manager UI test results

🔺 1 failing test on test run #4 ↗︎

❌ Failing✅ Passing↪️ Skipped🕐 Duration
1 Failing673 Passing5 Skipped117m 27s

Details

Failing Tests
SpecTest
access-key.e2e.spec.tsCloud Manager Cypress Tests→object storage access key end-to-end tests » object storage access key end-to-end tests

Troubleshooting

Use this command to re-run the failing tests:

pnpm cy:run -s "cypress/e2e/core/objectStorage/access-key.e2e.spec.ts"

Copy link
Contributor

@mjac0bs mjac0bs left a comment

Choose a reason for hiding this comment

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

Confirmed that grants are used when the IAM feature flag is disabled and permissions are used when IAM is enabled.

Confirmed having the entity permissions enables the buttons with the feature flag on, otherwise buttons are disabled. (Except Create Disk Image, noted in comment.)

@mjac0bs mjac0bs removed the Related Cypress Test Failure Test failure(s) are the result of changes in this PR and must be addressed before merging. label Jul 8, 2025
@mjac0bs mjac0bs added the Add'tl Approval Needed Waiting on another approval! label Jul 8, 2025
@github-project-automation github-project-automation bot moved this from Review to Approved in Cloud Manager Jul 8, 2025
@jaalah-akamai
Copy link
Contributor

access-key.e2e.spec.ts passes the majority of the time locally - seems to be flaky with regarding:

- cluster is not valid (field 'cluster')
- region is required (field 'region')

@jaalah-akamai jaalah-akamai merged commit d5d7185 into linode:develop Jul 8, 2025
34 of 35 checks passed
@github-project-automation github-project-automation bot moved this from Approved to Merged in Cloud Manager Jul 8, 2025
@mjac0bs mjac0bs added Approved Multiple approvals and ready to merge! and removed Add'tl Approval Needed Waiting on another approval! labels Jul 8, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Approved Multiple approvals and ready to merge! IAM (Identity & Access Management) Linodes Dealing with the Linodes section of the app 🚨 Urgent

Projects

Archived in project

Development

Successfully merging this pull request may close these issues.

7 participants