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 issue where empty IP on a NIC is submitted as an empty string #1811

Merged
merged 3 commits into from
Dec 4, 2023

Conversation

zephraph
Copy link
Contributor

@zephraph zephraph commented Nov 14, 2023

Fixes #1438

When creating a NIC without an IP in the current version of console, the request fails due to the IP being malformed (an empty string). I updated the IP's default to be undefined, meaning it'll be left out of the submitted object and not received by the API.

#1438 says that the IP address isn't optional, but the API specifies that it is. If it's not provided it should be chosen by Nexus. If that's not happening, we should fix that upstream.


This also updates the generated validators as per oxidecomputer/oxide.ts#223 to help catch this error. With the validator update but without the fix included here, the tests would fail.

Copy link

vercel bot commented Nov 14, 2023

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Updated (UTC)
console ✅ Ready (Inspect) Visit Preview Dec 2, 2023 6:30am

@@ -23,7 +23,7 @@ import { useForm, useProjectSelector } from 'app/hooks'
const defaultValues: InstanceNetworkInterfaceCreate = {
name: '',
description: '',
ip: '',
ip: undefined,
Copy link
Collaborator

@david-crespo david-crespo Nov 14, 2023

Choose a reason for hiding this comment

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

I believe this will only work with the initial value, but not if you type something in and then delete it. You might need to do something ugly like add a prop to TextField that tells it the value should be undefined when the string is empty.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yep, you're right.

@zephraph
Copy link
Contributor Author

zephraph commented Dec 4, 2023

This fix is incomplete because as @david-crespo pointed out the submission error will reoccur if the field is populated and emptied. #1812 will include a full fix, but we're going to merge this for now to unblock the common case.

@david-crespo david-crespo merged commit 9e82f9a into main Dec 4, 2023
8 checks passed
@david-crespo david-crespo deleted the fix-1438 branch December 4, 2023 22:12
david-crespo added a commit to oxidecomputer/omicron that referenced this pull request Dec 7, 2023
oxidecomputer/console@ae8218d...1802c28

* [1802c285](oxidecomputer/console@1802c285) oxidecomputer/console#1839
* [ce09b547](oxidecomputer/console@ce09b547) bump postcss-pseudo-classes for fake vuln
* [e09b803b](oxidecomputer/console@e09b803b) might as well get vitest 1.0 in there too
* [83dd73ee](oxidecomputer/console@83dd73ee) minor bumps for react router, msw, vite, tailwind, recharts
* [6ae6beeb](oxidecomputer/console@6ae6beeb) oxidecomputer/console#1829
* [a0bf47aa](oxidecomputer/console@a0bf47aa) oxidecomputer/console#1836
* [6c9420ad](oxidecomputer/console@6c9420ad) oxidecomputer/console#1835
* [64e97b01](oxidecomputer/console@64e97b01) api-diff also takes a commit
* [22bef0bb](oxidecomputer/console@22bef0bb) oxidecomputer/console#1833
* [2fe50f51](oxidecomputer/console@2fe50f51) oxidecomputer/console#1810
* [faadb6d3](oxidecomputer/console@faadb6d3) oxidecomputer/console#1832
* [9e82f9ab](oxidecomputer/console@9e82f9ab) oxidecomputer/console#1811
* [5e11fd83](oxidecomputer/console@5e11fd83) tweak api-diff
* [dae20577](oxidecomputer/console@dae20577) oxidecomputer/console#1827
* [ed0ef62e](oxidecomputer/console@ed0ef62e) minor tweaks to api-diff script
* [1c790d27](oxidecomputer/console@1c790d27) oxidecomputer/console#1819
* [97be7724](oxidecomputer/console@97be7724) oxidecomputer/console#1826
* [87f4d8b8](oxidecomputer/console@87f4d8b8) oxidecomputer/console#1814
* [65ae1212](oxidecomputer/console@65ae1212) oxidecomputer/console#1820
* [5a6dcea7](oxidecomputer/console@5a6dcea7) oxidecomputer/console#1822
* [4e1bbe13](oxidecomputer/console@4e1bbe13) oxidecomputer/console#1821
* [17408f64](oxidecomputer/console@17408f64) oxidecomputer/console#1813
david-crespo added a commit to oxidecomputer/omicron that referenced this pull request Dec 7, 2023
### User-facing changes

* [1802c285](oxidecomputer/console@1802c285)
oxidecomputer/console#1839
* [6ae6beeb](oxidecomputer/console@6ae6beeb)
oxidecomputer/console#1829
* [a0bf47aa](oxidecomputer/console@a0bf47aa)
oxidecomputer/console#1836
* [9e82f9ab](oxidecomputer/console@9e82f9ab)
oxidecomputer/console#1811
* [5a6dcea7](oxidecomputer/console@5a6dcea7)
oxidecomputer/console#1822

### All changes

oxidecomputer/console@ae8218d...1802c28

* [1802c285](oxidecomputer/console@1802c285)
oxidecomputer/console#1839
* [ce09b547](oxidecomputer/console@ce09b547)
bump postcss-pseudo-classes for fake vuln
* [e09b803b](oxidecomputer/console@e09b803b)
might as well get vitest 1.0 in there too
* [83dd73ee](oxidecomputer/console@83dd73ee)
minor bumps for react router, msw, vite, tailwind, recharts
* [6ae6beeb](oxidecomputer/console@6ae6beeb)
oxidecomputer/console#1829
* [a0bf47aa](oxidecomputer/console@a0bf47aa)
oxidecomputer/console#1836
* [6c9420ad](oxidecomputer/console@6c9420ad)
oxidecomputer/console#1835
* [64e97b01](oxidecomputer/console@64e97b01)
api-diff also takes a commit
* [22bef0bb](oxidecomputer/console@22bef0bb)
oxidecomputer/console#1833
* [2fe50f51](oxidecomputer/console@2fe50f51)
oxidecomputer/console#1810
* [faadb6d3](oxidecomputer/console@faadb6d3)
oxidecomputer/console#1832
* [9e82f9ab](oxidecomputer/console@9e82f9ab)
oxidecomputer/console#1811
* [5e11fd83](oxidecomputer/console@5e11fd83)
tweak api-diff
* [dae20577](oxidecomputer/console@dae20577)
oxidecomputer/console#1827
* [ed0ef62e](oxidecomputer/console@ed0ef62e)
minor tweaks to api-diff script
* [1c790d27](oxidecomputer/console@1c790d27)
oxidecomputer/console#1819
* [97be7724](oxidecomputer/console@97be7724)
oxidecomputer/console#1826
* [87f4d8b8](oxidecomputer/console@87f4d8b8)
oxidecomputer/console#1814
* [65ae1212](oxidecomputer/console@65ae1212)
oxidecomputer/console#1820
* [5a6dcea7](oxidecomputer/console@5a6dcea7)
oxidecomputer/console#1822
* [4e1bbe13](oxidecomputer/console@4e1bbe13)
oxidecomputer/console#1821
* [17408f64](oxidecomputer/console@17408f64)
oxidecomputer/console#1813
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.

Add NIC to instance: validation problems with IP address field
2 participants