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-7085] Edit VLAN config validation bug #9641

Closed
wants to merge 13 commits into from

Conversation

abailly-akamai
Copy link
Contributor

@abailly-akamai abailly-akamai commented Sep 7, 2023

closed in favor of #9644

Description 📝

Latest API release was responsible for exposing an issue with our validation schema when editing a linode config.

NOTE: I will write an e2e test to cover this issue in a subsequent PR

Our config interfaces properly types interface.label and interface.ipam_address as string | null, however our yup schema validation (which apparently never encountered a null value before) was throwing an error.

Screenshot 2023-09-06 at 08 58 36

This is due to the fact that the API now seems to return a default public record (brand new Linode created with VLAN) in the interface array which has null ipam_address & label.

⚠️ I am wondering if this is the desired behavior (because of this it is impossible to add a new public interface when editing - you will get an "Only one public interface per config is allowed." error). CC @jaalah @jcallahan-akamai @bnussman

ex payload for config interfaces with a newly created linode with VLAN

"interfaces": [
        {
          "id": 659956,
          "purpose": "public",
          "ipam_address": null,
          "label": null
        },
        {
          "id": 659957,
          "purpose": "vlan",
          "ipam_address": "",
          "label": "testvlan"
        }
      ]

Wether or not this is intended, our validation schema should still be able to handle null values as defined by our types so this PR should be relevant nonetheless.

Major Changes 🔄

  • Allow null interface record label & ipam_address to go through YUP validation

How to test 🧪

  1. Pull code locally
  2. Create a new Linode with an attached VLAN
  3. Navigate to /linodes/49433992/configurations and edit config
    Screenshot 2023-09-07 at 09 53 40
  4. Try saving the form and confirm no errors in the console and PUT config API request succeeds

@abailly-akamai abailly-akamai self-assigned this Sep 7, 2023
@abailly-akamai abailly-akamai marked this pull request as ready for review September 7, 2023 13:56
bnussman-akamai
bnussman-akamai previously approved these changes Sep 7, 2023
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.

Linode Creation and Config Creation / Editing looks good from my testing. I think we should have some VPC people take a look at this fix!

@bnussman-akamai
Copy link
Member

Also, we may want to point at staging if we want to hotfix this.

@abailly-akamai abailly-akamai changed the base branch from develop to staging September 7, 2023 16:34
@abailly-akamai abailly-akamai dismissed bnussman-akamai’s stale review September 7, 2023 16:34

The base branch was changed.

abailly-akamai and others added 13 commits September 7, 2023 12:35
* feat: [M3-6966] Initial commit: selection panels

* feat: [M3-6966] Linode: Checkout summary

* feat: [M3-6966] Fix display conditional

* Added changeset: Add DC specific Linode Create pricing support

* feat: [M3-6967] add comment

* feat: [M3-6967] reset flag state on component update

* feat: [M3-6967] consolidate tests and utils

* feat: [M3-6967] cleanup

* feat: [M3-6967] moar cleanup and comments

* feat: [M3-6967] quick display fix

* feat: [M3-6967] fix for DB create flow

* feat: [M3-6967] address feedback

* feat: [M3-6967] add tests

* feat: [M3-6967] cleanup tests

* feat: [M3-6967] cleanup tests

* feat: [M3-6967] address feedback (notice)

* feat: [M3-6967] small fix to flag prop and cleanup
* add spacing back

* add changeset

* Update packages/manager/.changeset/pr-9619-fixed-1693518126299.md

Co-authored-by: Alban Bailly <130582365+abailly-akamai@users.noreply.github.com>

---------

Co-authored-by: Alban Bailly <130582365+abailly-akamai@users.noreply.github.com>
…#9625)

* remove VPC column from linode landing table and update tests

* Added changeset: Removed VPC column from Linodes landing page table
…9606)

* Update kube utils for dynamic pricing

* Add DC-specific dynamic pricing for Add and Resize Node Pool drawers

* Add DC-specific dynamic pricing to cluster specs and checkout bar

* Update test

* Add DC-specific dynamic pricing in kube node pools tables

* Display dynamic prices in the selection card, mobile view

* Render mobile view empty state plan table helper text in Notice

* WIP: suspicious tests

* Update unit tests

* Cleanup

* Added changeset: Add DC-specific pricing to Kubernetes node pools

* feat: [M3-7048] feedback

---------

Co-authored-by: Alban Bailly <abailly@akamai.com>
…ies (#9585)

* add vpc grant and permissions

* yarn changeset

* Update packages/api-v4/.changeset/pr-9585-changed-1692736077405.md

Co-authored-by: Dajahi Wiley <114682940+dwiley-akamai@users.noreply.github.com>

* Update packages/manager/.changeset/pr-9585-upcoming-features-1692736112038.md

Co-authored-by: Dajahi Wiley <114682940+dwiley-akamai@users.noreply.github.com>

* fix unresolved merge conflict issues

* update vpc global grant text

* add missing files

* Update packages/manager/src/features/VPC/VPCLanding/VPCEditDrawer.tsx

Co-authored-by: Dajahi Wiley <114682940+dwiley-akamai@users.noreply.github.com>

* add comment to explain VPC permissions

---------

Co-authored-by: Dajahi Wiley <114682940+dwiley-akamai@users.noreply.github.com>
…ed (#9558)

* Pass `false` for `control_plane.high_availability` when creating LKE cluster when no HA price is defined

* Added changeset: Fix stuck LKE node pools when HA Control Plane is unavailable

---------

Co-authored-by: mjac0bs <mjacobs@akamai.com>
* add basic region columns to pdf, csv, and table

* handle region transfer

* Added changeset: DC Specific Pricing Invoice Support

* fix error state

* add min width

* improve invoice label condition check

* fix loading state

* make comment more clear

---------

Co-authored-by: Banks Nussman <banks@nussman.us>
## Description 📝
Add VPC Subnets Table to the VPC details page. Action menu is stubbed for now

## How to test 🧪
```
yarn test VPCSubnetsTable
```
```
yarn test SubnetLinodeRow
```
Turn on the MSW, go to a VPC's details page and check the Subnets table
…fers (#9582)

* initial migration commit

* finished EntityTransfers/ and EntityTransfersCreate/

* Partly done EntityTransfersLanding directory

* finished css migration, have not tested yet

* Added changeset: MUI v5 Migration - SRC > Features > EntityTransfers

* removed test code

* testing formating issues

* fixed inconsistent styling issues

* fixed pr suggestions

---------

Co-authored-by: TylerWJ <tylerwjones99@gmail.com>
@abailly-akamai
Copy link
Contributor Author

@bnussman-akamai closed in favor of #9644 to point at staging

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants