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

change: [M3-7485] - Revert Disable Public IP for Linodes Landing #9933

Merged
merged 4 commits into from
Dec 1, 2023

Conversation

hana-akamai
Copy link
Contributor

@hana-akamai hana-akamai commented Nov 27, 2023

Description 📝

We are grabbing configs for every Linode in the Linodes landing page to check if it is a VPC-only Linode which does not scale well (same problem as the VPC Linodes landing column). This PR reverts the Linodes landing portion of #9899 and adds a warning banner

Changes 🔄

  • Revert disabling of the Public IP Address column in the Linodes landing table for VPC-only Linodes
  • Add warning banner to the Linodes landing page

Preview 📷

image

How to test 🧪

Prerequisites

  • Ensure your account has vpc customer tags

Reproduction steps

  • Go to /linodes and filter your Network tab by configs. You should see a configs request for each Linode you have

Verification steps

  • Go to /linodes and filter your Network tab by configs
    • You should no longer see a configs request for each Linode you have
    • You should see a warning banner

As an Author I have considered 🤔

Check all that apply

  • 👀 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

@hana-akamai hana-akamai added the VPC Relating to VPC project label Nov 27, 2023
@hana-akamai hana-akamai self-assigned this Nov 27, 2023
@hana-akamai hana-akamai requested a review from a team as a code owner November 27, 2023 15:45
@hana-akamai hana-akamai requested review from carrillo-erik and coliu-akamai and removed request for a team November 27, 2023 15:45
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 /configs request no longer made ✅
  • confirmed expected behavior (IP no longer disabled on Linodes Landing page, still disabled on Linode Details page if Linode is 'VPC only linode') ✅

would we need a changeset for this (or should we edit the changeset of the original pr)?

@coliu-akamai coliu-akamai added the Approved Multiple approvals and ready to merge! label Nov 27, 2023
@hana-akamai
Copy link
Contributor Author

@coliu-akamai Since the original PR hasn't been released yet, I opted to edit the changeset

@coliu-akamai
Copy link
Contributor

awesome thanks @hana-linode! 🚀

@coliu-akamai
Copy link
Contributor

Also to add, we should prob let Kurt/Andrew know about this change so they're aware. In general seems like anything in the Linodes landing page that depends on the configs just won't be possible for now 😢

@hana-akamai hana-akamai changed the title fix: Revert Disable Public IP for Linodes Landing change: [M3-7485] - Revert Disable Public IP for Linodes Landing Nov 28, 2023
@hana-akamai
Copy link
Contributor Author

Added an additional banner, can you take another look? @coliu-akamai @bnussman-akamai

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 banner appears
✅ reconfirmed existing behavior!

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.

The code looks good, do we really need this banner?

A user who doesn't intend to use VPC signs up for Linode and sees that banner first thing when they load Cloud Manager.

I don't like the experience of showing this banner to every user who has access to VPC...

@hana-akamai
Copy link
Contributor Author

hana-akamai commented Nov 30, 2023

@bnussman-akamai The banner was a decision by Kurt (Product Manager for VPC) to reduce confusion/concerns around security vulnerabilities/misconfiguration since it's no longer viable to visually disable the IP address field

@carrillo-erik
Copy link
Contributor

Banner is present and filtering the network tab renders the expected results.

@hana-akamai hana-akamai merged commit 05dc46d into linode:develop Dec 1, 2023
11 checks passed
@hana-akamai hana-akamai deleted the vpc-linodes-landing-ip branch December 1, 2023 16:37
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! VPC Relating to VPC project
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants