Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: [M3-8563 & M3-8566] – Fix BSE capability for linodes & prevent unnecessary API request on Volume Create page #10920

Conversation

dwiley-akamai
Copy link
Contributor

Description 📝

  • Change BSE capability for Linodes from blockstorage_encryption --> Block Storage Encryption
  • Adjust enabled condition passed to useLinodeQuery() on Volume Create page so it is true only if linode_id !== null

Target release date 🗓️

9/16/24

Preview 📷

Before After
Screenshot 2024-09-11 at 10 55 05 AM Screenshot 2024-09-11 at 11 01 21 AM

How to test 🧪

Prerequisites

Point at the dev environment with the blockstorage-encryption tag on your account

Verification steps

On the Volume Create page, you should no longer see requests to linode/instances/-1 before selecting a linode.

As an Author I have 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

@dwiley-akamai dwiley-akamai added the Block Storage Encryption (BSE) Relating to Block Storage (Volumes) encryption label Sep 11, 2024
@dwiley-akamai dwiley-akamai self-assigned this Sep 11, 2024
@dwiley-akamai dwiley-akamai requested review from a team as code owners September 11, 2024 15:15
@dwiley-akamai dwiley-akamai requested review from cliu-akamai, cpathipa and abailly-akamai and removed request for a team September 11, 2024 15:15
…ge Encryption` instead of `blockstorage_encryption`
Copy link
Member

@bnussman-akamai bnussman-akamai left a comment

Choose a reason for hiding this comment

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

Looks good!

Should we update the capabilities type to be more strict than just string?

capabilities?: string[]; // @TODO BSE: Remove optionality once BSE is fully rolled out

Copy link

github-actions bot commented Sep 11, 2024

Coverage Report:
Base Coverage: 86.43%
Current Coverage: 86.43%

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.

👍🏼 Extra requests are gone.

Unrelated to this PR, while testing, I did notice a console state error that's happening once a Linode is selected and the config loads - but only initially. This probably existed before BSE changes and can be its own ticket, but mentioning here in case you had any recent context.

Screen.Recording.2024-09-11.at.9.26.27.AM.mov

@mjac0bs mjac0bs added the Approved Multiple approvals and ready to merge! label Sep 11, 2024
Copy link
Contributor

@coliu-akamai coliu-akamai left a comment

Choose a reason for hiding this comment

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

Confirmed extra requests to -1 are gone!

@dwiley-akamai
Copy link
Contributor Author

Unrelated to this PR, while testing, I did notice a console state error that's happening once a Linode is selected and the config loads - but only initially. This probably existed before BSE changes and can be its own ticket, but mentioning here in case you had any recent context.

Maybe it has something to do with the replacement of Select with Autocomplete a few months ago?

@dwiley-akamai dwiley-akamai merged commit adacc82 into linode:develop Sep 11, 2024
18 of 19 checks passed
nikhagra-akamai pushed a commit to nikhagra-akamai/manager that referenced this pull request Sep 23, 2024
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! Block Storage Encryption (BSE) Relating to Block Storage (Volumes) encryption
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants