diff --git a/app/components/form/fields/ComboboxField.tsx b/app/components/form/fields/ComboboxField.tsx index 3b9ee63f51..ce79af7e4d 100644 --- a/app/components/form/fields/ComboboxField.tsx +++ b/app/components/form/fields/ComboboxField.tsx @@ -10,7 +10,9 @@ import { useController, type Control, type FieldPath, + type FieldPathValue, type FieldValues, + type Validate, } from 'react-hook-form' import { Combobox, type ComboboxBaseProps } from '~/ui/lib/Combobox' @@ -25,6 +27,7 @@ export type ComboboxFieldProps< name: TName control: Control onChange?: (value: string | null | undefined) => void + validate?: Validate, TFieldValues> } & ComboboxBaseProps export function ComboboxField< @@ -51,9 +54,14 @@ export function ComboboxField< : allowArbitraryValues ? 'Select an option or enter a custom value' : 'Select an option', + validate, ...props }: ComboboxFieldProps) { - const { field, fieldState } = useController({ name, control, rules: { required } }) + const { field, fieldState } = useController({ + name, + control, + rules: { required, validate }, + }) return (
+ // required: false arg is desirable because otherwise if you put in + // a bad name and submit, causing it to revalidate onChange, then + // clear the field you're left with a BS "Target name is required" + // error message + validateName(value, `${capitalize(sectionType)} name`, false) + } /> ) : ( + (valueType === 'ip' && validateIp(value)) || + (valueType === 'ip_net' && validateIpNet(value)) || + undefined + } /> )} @@ -233,14 +242,14 @@ type CommonFieldsProps = { error: ApiError | null } +const targetAndHostDefaultValues: TargetAndHostFormValues = { + type: 'vpc', + value: '', +} + export const CommonFields = ({ control, nameTaken, error }: CommonFieldsProps) => { const { project, vpc } = useVpcSelector() - const targetAndHostDefaultValues: TargetAndHostFormValues = { - type: 'vpc', - value: '', - // only becomes relevant when the type is 'VPC subnet'; ignored otherwise - subnetVpc: vpc, - } + // prefetchedApiQueries below are prefetched in firewall-rules-create and -edit const { data: { items: instances }, @@ -255,8 +264,7 @@ export const CommonFields = ({ control, nameTaken, error }: CommonFieldsProps) = // Targets const targetForm = useForm({ defaultValues: targetAndHostDefaultValues }) const targets = useController({ name: 'targets', control }).field - const targetType = targetForm.watch('type') - const targetValue = targetForm.watch('value') + const [targetType, targetValue] = targetForm.watch(['type', 'value']) // get the list of items that are not already in the list of targets const targetItems = { vpc: availableItems(targets.value, 'vpc', vpcs), @@ -272,8 +280,20 @@ export const CommonFields = ({ control, nameTaken, error }: CommonFieldsProps) = if (!type || !value) return if (targets.value.some((t) => t.value === value && t.type === type)) return targets.onChange([...targets.value, { type, value }]) - targetForm.reset() + targetForm.reset(targetAndHostDefaultValues) }) + // HACK: we need to reset the target form completely after a successful submit, + // including especially `isSubmitted`, because that governs whether we're in + // the validate regime (which doesn't validate until submit) or the reValidate + // regime (which validate on every keypress). The reset inside `handleSubmit` + // doesn't do that for us because `handleSubmit` set `isSubmitted: true` after + // running the callback. So we have to watch for a successful submit and call + // the reset again here. + // https://github.com/react-hook-form/react-hook-form/blob/9a497a70a/src/logic/createFormControl.ts#L1194-L1203 + const { isSubmitSuccessful: targetSubmitSuccessful } = targetForm.formState + useEffect(() => { + if (targetSubmitSuccessful) targetForm.reset(targetAndHostDefaultValues) + }, [targetSubmitSuccessful, targetForm]) // Ports const portRangeForm = useForm({ defaultValues: { portRange: '' } }) @@ -290,8 +310,7 @@ export const CommonFields = ({ control, nameTaken, error }: CommonFieldsProps) = // Host Filters const hostForm = useForm({ defaultValues: targetAndHostDefaultValues }) const hosts = useController({ name: 'hosts', control }).field - const hostType = hostForm.watch('type') - const hostValue = hostForm.watch('value') + const [hostType, hostValue] = hostForm.watch(['type', 'value']) // get the list of items that are not already in the list of host filters const hostFilterItems = { vpc: availableItems(hosts.value, 'vpc', vpcs), @@ -306,8 +325,13 @@ export const CommonFields = ({ control, nameTaken, error }: CommonFieldsProps) = if (!type || !value) return if (hosts.value.some((t) => t.value === value && t.type === type)) return hosts.onChange([...hosts.value, { type, value }]) - hostForm.reset() + hostForm.reset(targetAndHostDefaultValues) }) + // HACK: see comment above about doing the same for target form + const { isSubmitSuccessful: hostSubmitSuccessful } = hostForm.formState + useEffect(() => { + if (hostSubmitSuccessful) hostForm.reset(targetAndHostDefaultValues) + }, [hostSubmitSuccessful, hostForm]) return ( <> @@ -411,13 +435,18 @@ export const CommonFields = ({ control, nameTaken, error }: CommonFieldsProps) = control={targetForm.control} valueType={targetType} items={targetItems[targetType]} - onTypeChange={() => targetForm.setValue('value', '')} + // HACK: reset the whole subform, keeping type (because we just set + // it). most importantly, this resets isSubmitted so the form can go + // back to validating on submit instead of change + onTypeChange={() => + targetForm.reset({ type: targetForm.getValues('type'), value: '' }) + } onInputChange={(value) => targetForm.setValue('value', value)} onSubmitTextField={submitTarget} /> targetForm.reset()} onSubmit={submitTarget} /> @@ -468,8 +497,8 @@ export const CommonFields = ({ control, nameTaken, error }: CommonFieldsProps) =
portRangeForm.reset()} onSubmit={submitPortRange} /> @@ -518,13 +547,18 @@ export const CommonFields = ({ control, nameTaken, error }: CommonFieldsProps) = control={hostForm.control} valueType={hostType} items={hostFilterItems[hostType]} - onTypeChange={() => hostForm.setValue('value', '')} + // HACK: reset the whole subform, keeping type (because we just set + // it). most importantly, this resets isSubmitted so the form can go + // back to validating on submit instead of change + onTypeChange={() => + hostForm.reset({ type: hostForm.getValues('type'), value: '' }) + } onInputChange={(value) => hostForm.setValue('value', value)} onSubmitTextField={submitHost} /> hostForm.reset()} onSubmit={submitHost} /> diff --git a/app/forms/ip-pool-range-add.tsx b/app/forms/ip-pool-range-add.tsx index 96fa8a1036..b5cb0488e0 100644 --- a/app/forms/ip-pool-range-add.tsx +++ b/app/forms/ip-pool-range-add.tsx @@ -15,16 +15,14 @@ import { SideModalForm } from '~/components/form/SideModalForm' import { useIpPoolSelector } from '~/hooks/use-params' import { addToast } from '~/stores/toast' import { Message } from '~/ui/lib/Message' +import { parseIp } from '~/util/ip' import { pb } from '~/util/path-builder' -import { validateIp } from '~/util/str' const defaultValues: IpRange = { first: '', last: '', } -const invalidAddressError = { type: 'pattern', message: 'Not a valid IP address' } - const ipv6Error = { type: 'pattern', message: 'IPv6 ranges are not yet supported' } /** @@ -35,20 +33,20 @@ const ipv6Error = { type: 'pattern', message: 'IPv6 ranges are not yet supported * regex twice, though. */ function resolver(values: IpRange) { - const first = validateIp(values.first) - const last = validateIp(values.last) + const first = parseIp(values.first) + const last = parseIp(values.last) const errors: FieldErrors = {} - if (!first.valid) { - errors.first = invalidAddressError - } else if (first.isv6) { + if (first.type === 'error') { + errors.first = { type: 'pattern', message: first.message } + } else if (first.type === 'v6') { errors.first = ipv6Error } - if (!last.valid) { - errors.last = invalidAddressError - } else if (last.isv6) { + if (last.type === 'error') { + errors.last = { type: 'pattern', message: last.message } + } else if (last.type === 'v6') { errors.last = ipv6Error } diff --git a/app/forms/network-interface-edit.tsx b/app/forms/network-interface-edit.tsx index bb9cdf558e..c57bde7899 100644 --- a/app/forms/network-interface-edit.tsx +++ b/app/forms/network-interface-edit.tsx @@ -5,6 +5,7 @@ * * Copyright Oxide Computer Company */ +import { useEffect } from 'react' import { useForm } from 'react-hook-form' import * as R from 'remeda' @@ -25,6 +26,7 @@ import { FieldLabel } from '~/ui/lib/FieldLabel' import * as MiniTable from '~/ui/lib/MiniTable' import { TextInputHint } from '~/ui/lib/TextInput' import { KEYS } from '~/ui/util/keys' +import { validateIpNet } from '~/util/ip' import { links } from '~/util/links' type EditNetworkInterfaceFormProps = { @@ -56,13 +58,18 @@ export function EditNetworkInterfaceForm({ const transitIps = form.watch('transitIps') || [] const transitIpsForm = useForm({ defaultValues: { transitIp: '' } }) + const transitIpValue = transitIpsForm.watch('transitIp') + const { isSubmitSuccessful: transitIpSubmitSuccessful } = transitIpsForm.formState - const submitTransitIp = () => { - const transitIp = transitIpsForm.getValues('transitIp') - if (!transitIp) return + const submitTransitIp = transitIpsForm.handleSubmit(({ transitIp }) => { + // validate has already checked to make sure it's not in the list form.setValue('transitIps', [...transitIps, transitIp]) transitIpsForm.reset() - } + }) + + useEffect(() => { + if (transitIpSubmitSuccessful) transitIpsForm.reset() + }, [transitIpSubmitSuccessful, transitIpsForm]) return ( - Enter an IPv4 or IPv6 address.{' '} + An IP network, like 192.168.0.0/16.{' '} Learn more about transit IPs. @@ -107,12 +114,19 @@ export function EditNetworkInterfaceForm({ submitTransitIp() } }} + validate={(value) => { + const error = validateIpNet(value) + if (error) return error + + if (transitIps.includes(value)) return 'Transit IP already in list' + }} + placeholder="Enter an IP network" /> transitIpsForm.reset()} onSubmit={submitTransitIp} /> diff --git a/app/forms/vpc-router-route-common.tsx b/app/forms/vpc-router-route-common.tsx index 6207d8b305..7215113bea 100644 --- a/app/forms/vpc-router-route-common.tsx +++ b/app/forms/vpc-router-route-common.tsx @@ -24,6 +24,7 @@ import { NameField } from '~/components/form/fields/NameField' import { TextField } from '~/components/form/fields/TextField' import { useVpcRouterSelector } from '~/hooks/use-params' import { Message } from '~/ui/lib/Message' +import { validateIp, validateIpNet } from '~/util/ip' export type RouteFormValues = RouterRouteCreate | Required @@ -122,6 +123,9 @@ export const RouteFormFields = ({ form, disabled }: RouteFormFieldsProps) => { required: true, disabled, description: destinationValueDescription[destinationType], + // need a default to prevent the text field validation function from + // sticking around when we switch to the combobox + validate: () => undefined, } const targetValueProps = { name: 'target.value' as const, @@ -132,6 +136,9 @@ export const RouteFormFields = ({ form, disabled }: RouteFormFieldsProps) => { // 'internet_gateway' targetTypes can only have the value 'outbound', so we disable the field disabled: disabled || targetType === 'internet_gateway', description: targetValueDescription[targetType], + // need a default to prevent the text field validation function from + // sticking around when we switch to the combobox + validate: () => undefined, } return ( <> @@ -149,13 +156,22 @@ export const RouteFormFields = ({ form, disabled }: RouteFormFieldsProps) => { required onChange={() => { form.setValue('destination.value', '') + form.clearErrors('destination.value') }} disabled={disabled} /> {destinationType === 'subnet' ? ( ) : ( - + + (destination.type === 'ip_net' && validateIpNet(value)) || + (destination.type === 'ip' && validateIp(value)) || + // false is invalid but true and undefined are valid so we need this + undefined + } + /> )} { required onChange={(value) => { form.setValue('target.value', value === 'internet_gateway' ? 'outbound' : '') + form.clearErrors('target.value') }} disabled={disabled} /> {targetType === 'drop' ? null : targetType === 'instance' ? ( ) : ( - + + (target.type === 'ip' && validateIp(value)) || undefined + } + /> )} ) diff --git a/app/util/ip.spec.ts b/app/util/ip.spec.ts new file mode 100644 index 0000000000..1b41d8ea0a --- /dev/null +++ b/app/util/ip.spec.ts @@ -0,0 +1,118 @@ +/* + * This Source Code Form is subject to the terms of the Mozilla Public + * License, v. 2.0. If a copy of the MPL was not distributed with this + * file, you can obtain one at https://mozilla.org/MPL/2.0/. + * + * Copyright Oxide Computer Company + */ + +import { expect, test } from 'vitest' + +import { parseIp, parseIpNet } from './ip' + +// Small Rust project where we validate that the built-in Ipv4Addr and Ipv6Addr +// and oxnet's Ipv4Net and Ipv6Net have the same validation behavior as our code. +// https://github.com/oxidecomputer/test-ip-validation + +const v4 = ['123.4.56.7', '1.2.3.4'] + +test.each(v4)('parseIp catches valid IPV4 / invalid IPV6: %s', (s) => { + expect(parseIp(s)).toStrictEqual({ type: 'v4', address: s }) +}) + +const v6 = [ + '2001:db8:3333:4444:5555:6666:7777:8888', + '2001:db8:3333:4444:CCCC:DDDD:EEEE:FFFF', + '::', + '2001:db8::', + '::1234:5678', + '2001:db8::1234:5678', + '2001:0db8:85a3:0000:0000:8a2e:0370:7334', + '::ffff:192.0.2.128', + '1:2:3:4:5:6:7:8', + '::ffff:10.0.0.1', + '::ffff:1.2.3.4', + '::ffff:0.0.0.0', + '1:2:3:4:5:6:77:88', + '::ffff:255.255.255.255', + 'fe08::7:8', + 'ffff:ffff:ffff:ffff:ffff:ffff:ffff:ffff', +] + +test.each(v6)('parseIp catches invalid IPV4 / valid IPV6: %s', (s) => { + expect(parseIp(s).type).toEqual('v6') +}) + +const invalid = [ + '', + '1', + 'abc', + 'a.b.c.d', + // some implementations (I think incorrectly) allow leading zeros but nexus does not + '01.102.103.104', + '127.0.0', + '127.0.0.1.', + '127.0.0.1 ', + ' 127.0.0.1', + '10002.3.4', + '1.2.3.4.5', + '256.0.0.0', + '260.0.0.0', + '256.1.1.1', + '192.168.0.0.0', + '2001:0db8:85a3:0000:0000:8a2e:0370:7334 ', + ' 2001:db8::', + '1:2:3:4:5:6:7:8:9', + '1:2:3:4:5:6::7:8', + ':1:2:3:4:5:6:7:8', + '1:2:3:4:5:6:7:8:', + '::1:2:3:4:5:6:7:8', + '1:2:3:4:5:6:7:8::', + '1:2:3:4:5:6:7:88888', + '2001:db8:3:4:5::192.0.2.33', // std::new::Ipv6Net allows this one + 'fe08::7:8%', + 'fe08::7:8i', + 'fe08::7:8interface', +] + +test.each(invalid)('parseIp catches invalid IP: %s', (s) => { + expect(parseIp(s)).toStrictEqual({ type: 'error', message: 'Not a valid IP address' }) +}) + +test.each([...v4.map((ip) => ip + '/10'), '1.1.1.1/04', '1.1.1.1/000004'])('%s', (s) => { + expect(parseIpNet(s).type).toBe('v4') +}) + +test.each([...v6.map((ip) => ip + '/10'), '2001:db8::/128', '2001:db8::/00004'])( + '%s', + (s) => { + expect(parseIpNet(s).type).toBe('v6') + } +) + +test.each(invalid.map((ip) => ip + '/10'))('parseIpNet catches invalid value: %s', (s) => { + expect(parseIpNet(s).type).toBe('error') +}) + +const nonsense = 'Must contain an IP address and a width, separated by a /' +const badWidth = 'Width must be an integer' +const ipv4Width = 'Max width for IPv4 is 32' +const ipv6Width = 'Max width for IPv6 is 128' + +test.each([ + ['', nonsense], + ['abc', nonsense], + ['/32', nonsense], + ['1.1.1.1', nonsense], + ['abc/2', nonsense], + ['1.1.1.1/', nonsense], + ['192.168.0.0.0/24', nonsense], + ['fd::/', nonsense], + ['1.1.1.1/a', badWidth], + ['192.168.0.0/-1', badWidth], + ['fd::/a', badWidth], + ['1.1.1.1/33', ipv4Width], + ['fd::/129', ipv6Width], +])('parseIpNet message: %s', (input, message) => { + expect(parseIpNet(input)).toEqual({ type: 'error', message }) +}) diff --git a/app/util/ip.ts b/app/util/ip.ts new file mode 100644 index 0000000000..9d060aab5b --- /dev/null +++ b/app/util/ip.ts @@ -0,0 +1,86 @@ +/* + * This Source Code Form is subject to the terms of the Mozilla Public + * License, v. 2.0. If a copy of the MPL was not distributed with this + * file, you can obtain one at https://mozilla.org/MPL/2.0/. + * + * Copyright Oxide Computer Company + */ + +// Borrowed from Valibot. I tried some from Zod and an O'Reilly regex cookbook +// but they didn't match results with std::net on simple test cases +// https://github.com/fabian-hiller/valibot/blob/2554aea5/library/src/regex.ts#L43-L54 + +const IPV4_REGEX = + /^(?:(?:[1-9]|1\d|2[0-4])?\d|25[0-5])(?:\.(?:(?:[1-9]|1\d|2[0-4])?\d|25[0-5])){3}$/u + +const IPV6_REGEX = + /^(?:(?:[\da-f]{1,4}:){7}[\da-f]{1,4}|(?:[\da-f]{1,4}:){1,7}:|(?:[\da-f]{1,4}:){1,6}:[\da-f]{1,4}|(?:[\da-f]{1,4}:){1,5}(?::[\da-f]{1,4}){1,2}|(?:[\da-f]{1,4}:){1,4}(?::[\da-f]{1,4}){1,3}|(?:[\da-f]{1,4}:){1,3}(?::[\da-f]{1,4}){1,4}|(?:[\da-f]{1,4}:){1,2}(?::[\da-f]{1,4}){1,5}|[\da-f]{1,4}:(?::[\da-f]{1,4}){1,6}|:(?:(?::[\da-f]{1,4}){1,7}|:)|fe80:(?::[\da-f]{0,4}){0,4}%[\da-z]+|::(?:f{4}(?::0{1,4})?:)?(?:(?:25[0-5]|(?:2[0-4]|1?\d)?\d)\.){3}(?:25[0-5]|(?:2[0-4]|1?\d)?\d)|(?:[\da-f]{1,4}:){1,4}:(?:(?:25[0-5]|(?:2[0-4]|1?\d)?\d)\.){3}(?:25[0-5]|(?:2[0-4]|1?\d)?\d))$/iu + +type ParsedIp = { type: 'v4' | 'v6'; address: string } | { type: 'error'; message: string } + +export function parseIp(ip: string): ParsedIp { + if (IPV4_REGEX.test(ip)) return { type: 'v4', address: ip } + if (IPV6_REGEX.test(ip)) return { type: 'v6', address: ip } + return { type: 'error', message: 'Not a valid IP address' } +} + +/** + * Convenience helper that can be plugged into RHF simply as + * `validate={validateIp}` + */ +export function validateIp(ip: string): string | undefined { + const result = parseIp(ip) + if (result.type === 'error') return result.message +} + +// Following oxnet: +// https://github.com/oxidecomputer/oxnet/blob/7dacd265f1bcd0f8b47bd4805250c4f0812da206/src/ipnet.rs#L373-L385 +// https://github.com/oxidecomputer/oxnet/blob/7dacd265f1bcd0f8b47bd4805250c4f0812da206/src/ipnet.rs#L217-L223 + +type ParsedIpNet = + | { type: 'v4' | 'v6'; address: string; width: number } + | { type: 'error'; message: string } + +const nonsenseError = { + type: 'error' as const, + message: 'Must contain an IP address and a width, separated by a /', +} + +export function parseIpNet(ipNet: string): ParsedIpNet { + const splits = ipNet.split('/') + if (splits.length !== 2) return nonsenseError + + const [addrStr, widthStr] = splits + + const { type: ipType } = parseIp(addrStr) + + if (ipType === 'error') return nonsenseError + if (widthStr.trim().length === 0) return nonsenseError + + if (!/^\d+$/.test(widthStr)) { + return { type: 'error', message: 'Width must be an integer' } + } + const width = parseInt(widthStr, 10) + + if (ipType === 'v4' && width > 32) { + return { type: 'error', message: 'Max width for IPv4 is 32' } + } + if (ipType === 'v6' && width > 128) { + return { type: 'error', message: 'Max width for IPv6 is 128' } + } + + return { + type: ipType, + address: addrStr, + width: width, + } +} + +/** + * Convenience helper that can be plugged into RHF simply as + * `validate={validateIpNet}` + */ +export function validateIpNet(ipNet: string): string | undefined { + const result = parseIpNet(ipNet) + if (result.type === 'error') return result.message +} diff --git a/app/util/str.spec.ts b/app/util/str.spec.ts index 37115bd02e..3a4a8a28e2 100644 --- a/app/util/str.spec.ts +++ b/app/util/str.spec.ts @@ -5,9 +5,9 @@ * * Copyright Oxide Computer Company */ -import { describe, expect, it, test } from 'vitest' +import { describe, expect, it } from 'vitest' -import { camelCase, capitalize, commaSeries, kebabCase, titleCase, validateIp } from './str' +import { camelCase, capitalize, commaSeries, kebabCase, titleCase } from './str' describe('capitalize', () => { it('capitalizes the first letter', () => { @@ -76,67 +76,3 @@ describe('titleCase', () => { expect(titleCase('123 abc')).toBe('123 Abc') }) }) - -// Rust playground comparing results with std::net::{Ipv4Addr, Ipv6Addr} -// https://play.rust-lang.org/?version=stable&mode=debug&edition=2021&gist=493b3345b9f6c0b1c8ee91834e99ef7b - -test.each(['123.4.56.7', '1.2.3.4'])( - 'validateIp catches valid IPV4 / invalid IPV6: %s', - (s) => { - expect(validateIp(s)).toStrictEqual({ isv4: true, isv6: false, valid: true }) - } -) - -test.each([ - '2001:db8:3333:4444:5555:6666:7777:8888', - '2001:db8:3333:4444:CCCC:DDDD:EEEE:FFFF', - '::', - '2001:db8::', - '::1234:5678', - '2001:db8::1234:5678', - '2001:0db8:85a3:0000:0000:8a2e:0370:7334', - '::ffff:192.0.2.128', - '1:2:3:4:5:6:7:8', - '::ffff:10.0.0.1', - '::ffff:1.2.3.4', - '::ffff:0.0.0.0', - '1:2:3:4:5:6:77:88', - '::ffff:255.255.255.255', - 'fe08::7:8', - 'ffff:ffff:ffff:ffff:ffff:ffff:ffff:ffff', -])('validateIp catches invalid IPV4 / valid IPV6: %s', (s) => { - expect(validateIp(s)).toStrictEqual({ isv4: false, isv6: true, valid: true }) -}) - -test.each([ - '', - '1', - 'abc', - 'a.b.c.d', - // some implementations (I think incorrectly) allow leading zeros but nexus does not - '01.102.103.104', - '127.0.0', - '127.0.0.1.', - '127.0.0.1 ', - ' 127.0.0.1', - '10002.3.4', - '1.2.3.4.5', - '256.0.0.0', - '260.0.0.0', - '256.1.1.1', - '2001:0db8:85a3:0000:0000:8a2e:0370:7334 ', - ' 2001:db8::', - '1:2:3:4:5:6:7:8:9', - '1:2:3:4:5:6::7:8', - ':1:2:3:4:5:6:7:8', - '1:2:3:4:5:6:7:8:', - '::1:2:3:4:5:6:7:8', - '1:2:3:4:5:6:7:8::', - '1:2:3:4:5:6:7:88888', - '2001:db8:3:4:5::192.0.2.33', // std::new::Ipv6Net allows this one - 'fe08::7:8%', - 'fe08::7:8i', - 'fe08::7:8interface', -])('validateIp catches invalid IP: %s', (s) => { - expect(validateIp(s)).toStrictEqual({ isv4: false, isv6: false, valid: false }) -}) diff --git a/app/util/str.ts b/app/util/str.ts index 069f8508ee..934530917c 100644 --- a/app/util/str.ts +++ b/app/util/str.ts @@ -50,22 +50,6 @@ export const titleCase = (text: string): string => { ) } -// Borrowed from Valibot. I tried some from Zod and an O'Reilly regex cookbook -// but they didn't match results with std::new on simple test cases -// https://github.com/fabian-hiller/valibot/blob/2554aea5/library/src/regex.ts#L43-L54 - -const IPV4_REGEX = - /^(?:(?:[1-9]|1\d|2[0-4])?\d|25[0-5])(?:\.(?:(?:[1-9]|1\d|2[0-4])?\d|25[0-5])){3}$/u - -const IPV6_REGEX = - /^(?:(?:[\da-f]{1,4}:){7}[\da-f]{1,4}|(?:[\da-f]{1,4}:){1,7}:|(?:[\da-f]{1,4}:){1,6}:[\da-f]{1,4}|(?:[\da-f]{1,4}:){1,5}(?::[\da-f]{1,4}){1,2}|(?:[\da-f]{1,4}:){1,4}(?::[\da-f]{1,4}){1,3}|(?:[\da-f]{1,4}:){1,3}(?::[\da-f]{1,4}){1,4}|(?:[\da-f]{1,4}:){1,2}(?::[\da-f]{1,4}){1,5}|[\da-f]{1,4}:(?::[\da-f]{1,4}){1,6}|:(?:(?::[\da-f]{1,4}){1,7}|:)|fe80:(?::[\da-f]{0,4}){0,4}%[\da-z]+|::(?:f{4}(?::0{1,4})?:)?(?:(?:25[0-5]|(?:2[0-4]|1?\d)?\d)\.){3}(?:25[0-5]|(?:2[0-4]|1?\d)?\d)|(?:[\da-f]{1,4}:){1,4}:(?:(?:25[0-5]|(?:2[0-4]|1?\d)?\d)\.){3}(?:25[0-5]|(?:2[0-4]|1?\d)?\d))$/iu - -export const validateIp = (ip: string) => { - const isv4 = IPV4_REGEX.test(ip) - const isv6 = !isv4 && IPV6_REGEX.test(ip) - return { isv4, isv6, valid: isv4 || isv6 } -} - /** * Does a base64 string represent underlying data that's all zeros, i.e., does * it look like `AAAAAAAAAAAAAAAA==`? diff --git a/mock-api/msw/handlers.ts b/mock-api/msw/handlers.ts index d1d346615e..7527181c2e 100644 --- a/mock-api/msw/handlers.ts +++ b/mock-api/msw/handlers.ts @@ -22,7 +22,8 @@ import { import { json, makeHandlers, type Json } from '~/api/__generated__/msw-handlers' import { instanceCan } from '~/api/util' -import { commaSeries, validateIp } from '~/util/str' +import { parseIp } from '~/util/ip' +import { commaSeries } from '~/util/str' import { GiB } from '~/util/units' import { genCumulativeI64Data } from '../metrics' @@ -728,7 +729,10 @@ export const handlers = makeHandlers({ const ranges = db.ipPoolRanges .filter((r) => r.ip_pool_id === pool.id) .map((r) => r.range) - const [ipv4Ranges, ipv6Ranges] = R.partition(ranges, (r) => validateIp(r.first).isv4) + const [ipv4Ranges, ipv6Ranges] = R.partition( + ranges, + (r) => parseIp(r.first).type === 'v4' + ) // in the real backend there are also SNAT IPs, but we don't currently // represent those because they are not exposed through the API (except diff --git a/mock-api/msw/util.ts b/mock-api/msw/util.ts index bcc8647127..8b309eb66d 100644 --- a/mock-api/msw/util.ts +++ b/mock-api/msw/util.ts @@ -25,7 +25,7 @@ import { } from '@oxide/api' import { json, type Json } from '~/api/__generated__/msw-handlers' -import { validateIp } from '~/util/str' +import { parseIp } from '~/util/ip' import { GiB, TiB } from '~/util/units' import type { DbRoleAssignmentResourceType } from '..' @@ -384,14 +384,14 @@ export function requireRole( } const ipToBigInt = (ip: string): bigint => - validateIp(ip).isv4 ? new IPv4(ip).value : new IPv6(ip).value + parseIp(ip).type === 'v4' ? new IPv4(ip).value : new IPv6(ip).value export const ipRangeLen = ({ first, last }: IpRange) => ipToBigInt(last) - ipToBigInt(first) + 1n function ipInRange(ip: string, { first, last }: IpRange): boolean { - const ipIsV4 = validateIp(ip).isv4 - const rangeIsV4 = validateIp(first).isv4 + const ipIsV4 = parseIp(ip).type === 'v4' + const rangeIsV4 = parseIp(first).type === 'v4' // if they're not the same version then definitely false if (ipIsV4 !== rangeIsV4) return false diff --git a/test/e2e/firewall-rules.e2e.ts b/test/e2e/firewall-rules.e2e.ts index 7046fedbd8..44f3ba7731 100644 --- a/test/e2e/firewall-rules.e2e.ts +++ b/test/e2e/firewall-rules.e2e.ts @@ -67,6 +67,10 @@ test('can create firewall rule', async ({ page }) => { await addPortButton.click() await expect(invalidPort).toBeVisible() + // test clear button + await page.getByRole('button', { name: 'Clear' }).nth(1).click() + await expect(portRangeField).toHaveValue('') + await portRangeField.fill('123-456') await addPortButton.click() await expect(invalidPort).toBeHidden() @@ -156,7 +160,8 @@ test('firewall rule form targets table', async ({ page }) => { await page.getByRole('link', { name: 'New rule' }).click() const targets = page.getByRole('table', { name: 'Targets' }) - const targetVpcNameField = page.getByRole('combobox', { name: 'VPC name' }).nth(0) + const targetVpcNameField = page.getByRole('combobox', { name: 'VPC name' }).first() + const addButton = page.getByRole('button', { name: 'Add target' }) // add targets with overlapping names and types to test delete @@ -204,6 +209,133 @@ test('firewall rule form targets table', async ({ page }) => { await expectRowVisible(targets, { Type: 'ip', Value: '192.168.0.1' }) }) +test('firewall rule form target validation', async ({ page }) => { + await page.goto('/projects/mock-project/vpcs/mock-vpc/firewall-rules-new') + + const addButton = page.getByRole('button', { name: 'Add target' }) + const targets = page.getByRole('table', { name: 'Targets' }) + + const formModal = page.getByRole('dialog', { name: 'Add firewall rule' }) + const nameError = formModal.getByText('Must end with a letter or number') + + // Enter invalid VPC name + const vpcNameField = page.getByRole('combobox', { name: 'VPC name' }).first() + await vpcNameField.fill('ab-') + await addButton.click() + await expect(nameError).toBeVisible() + + // Change to IP, error should disappear and value field should be empty + await selectOption(page, 'Target type', 'IP') + await expect(nameError).toBeHidden() + const ipField = page.getByRole('textbox', { name: 'IP address' }) + await expect(ipField).toHaveValue('') + + // Type '1', error should not appear immediately (back to validating onSubmit) + await ipField.fill('1') + const ipError = formModal.getByText('Not a valid IP address') + await expect(ipError).toBeHidden() + await addButton.click() + await expect(ipError).toBeVisible() + + // test clear button + await page.getByRole('button', { name: 'Clear' }).first().click() + await expect(ipField).toHaveValue('') + + // Change back to VPC, enter valid value + await selectOption(page, 'Target type', 'VPC') + await expect(ipError).toBeHidden() + await expect(nameError).toBeHidden() + await vpcNameField.fill('abc') + await addButton.click() + await expectRowVisible(targets, { Type: 'vpc', Value: 'abc' }) + + // Switch to IP again + await selectOption(page, 'Target type', 'IP') + await ipField.fill('1') + + // No error means we're back to validating on submit + await expect(ipError).toBeHidden() + + // Hit submit to get error + await addButton.click() + await expect(ipError).toBeVisible() + + // Fill out valid IP and submit + await ipField.fill('1.1.1.1') + await addButton.click() + await expect(ipError).toBeHidden() + await expectRowVisible(targets, { Type: 'ip', Value: '1.1.1.1' }) +}) + +// This test may appear redundant because host and target share their logic, but +// testing them separately is still valuable because we want to make sure we're +// hooking up the shared code correctly and we don't break them if we refactor +// that code. + +test('firewall rule form host validation', async ({ page }) => { + await page.goto('/projects/mock-project/vpcs/mock-vpc/firewall-rules-new') + + const addButton = page.getByRole('button', { name: 'Add host filter' }) + const hosts = page.getByRole('table', { name: 'Host filters' }) + + const formModal = page.getByRole('dialog', { name: 'Add firewall rule' }) + const nameError = formModal.getByText('Must end with a letter or number') + + // Enter invalid VPC name + const vpcNameField = page.getByRole('combobox', { name: 'VPC name' }).nth(1) + await vpcNameField.fill('ab-') + await addButton.click() + await expect(nameError).toBeVisible() + + // Change to IP, error should disappear and value field should be empty + await selectOption(page, 'Host type', 'IP') + await expect(nameError).toBeHidden() + const ipField = page.getByRole('textbox', { name: 'IP address' }) + await expect(ipField).toHaveValue('') + + // Type '1', error should not appear immediately (back to validating onSubmit) + await ipField.fill('1') + const ipError = formModal.getByText('Not a valid IP address') + await expect(ipError).toBeHidden() + await addButton.click() + await expect(ipError).toBeVisible() + + // test clear button + await page.getByRole('button', { name: 'Clear' }).nth(2).click() + await expect(ipField).toHaveValue('') + + // Change back to VPC, enter valid value + await selectOption(page, 'Host type', 'VPC') + await expect(ipError).toBeHidden() + await expect(nameError).toBeHidden() + await vpcNameField.fill('abc') + await addButton.click() + await expectRowVisible(hosts, { Type: 'vpc', Value: 'abc' }) + + // use subnet to be slightly different from the target validation test + + // Switch to IP subnet + await selectOption(page, 'Host type', 'IP subnet') + const ipSubnetField = page.getByRole('textbox', { name: 'IP network' }) + await ipSubnetField.fill('1') + + // No error means we're back to validating on submit + const ipNetError = formModal.getByText( + 'Must contain an IP address and a width, separated by a /' + ) + await expect(ipNetError).toBeHidden() + + // Hit submit to get error + await addButton.click() + await expect(ipNetError).toBeVisible() + + // Fill out valid IP and submit + await ipSubnetField.fill('1.1.1.1/1') + await addButton.click() + await expect(ipNetError).toBeHidden() + await expectRowVisible(hosts, { Type: 'ip_net', Value: '1.1.1.1/1' }) +}) + test('firewall rule form hosts table', async ({ page }) => { await page.goto('/projects/mock-project/vpcs/mock-vpc') await page.getByRole('tab', { name: 'Firewall Rules' }).click() diff --git a/test/e2e/instance-networking.e2e.ts b/test/e2e/instance-networking.e2e.ts index 2ae4fb4ef3..26b868a751 100644 --- a/test/e2e/instance-networking.e2e.ts +++ b/test/e2e/instance-networking.e2e.ts @@ -153,3 +153,71 @@ test('Instance networking tab — floating IPs', async ({ page }) => { // And that button should be enabled again await expect(attachFloatingIpButton).toBeEnabled() }) + +test('Edit network interface - Transit IPs', async ({ page }) => { + await page.goto('/projects/mock-project/instances/db1/networking') + + // Stop the instance to enable editing + await stopInstance(page) + + await clickRowAction(page, 'my-nic', 'Edit') + + const modal = page.getByRole('dialog', { name: 'Edit network interface' }) + const transitIpField = modal.getByRole('textbox', { name: 'Transit IPs' }) + const addTransitIpButton = modal.getByRole('button', { name: 'Add Transit IP' }) + const clearButton = modal.getByRole('button', { name: 'Clear' }) + const errorMessage = modal.getByText( + 'Must contain an IP address and a width, separated by a /' + ) + const transitIpsTable = modal.getByRole('table', { name: 'Transit IPs' }) + + // Type invalid value + await transitIpField.fill('invalid-ip') + await expect(errorMessage).toBeHidden() + + // Hit add transit IP button + await addTransitIpButton.click() + await expect(errorMessage).toBeVisible() + + // Clear the value and error + await clearButton.click() + await expect(errorMessage).toBeHidden() + await expect(transitIpField).toHaveValue('') + + // Type bad value again + await transitIpField.fill('invalid-ip') + await expect(errorMessage).toBeHidden() + + // Hit add again to get the error + await addTransitIpButton.click() + await expect(errorMessage).toBeVisible() + + // Change to valid IP network + await transitIpField.fill('192.168.0.0/16') + await expect(errorMessage).toBeHidden() + + // Submit and assert it's in the table + await addTransitIpButton.click() + await expect(transitIpField).toHaveValue('') + await expectRowVisible(transitIpsTable, { 'Transit IPs': '192.168.0.0/16' }) + + const dupeError = modal.getByText('Transit IP already in list') + + // try to add the same one again to see the dupe message + await transitIpField.fill('192.168.0.0/16') + await expect(dupeError).toBeHidden() + await addTransitIpButton.click() + await expect(dupeError).toBeVisible() + + // Submit the form + await modal.getByRole('button', { name: 'Update network interface' }).click() + + // Assert the transit IP is in the NICs table + const nicTable = page.getByRole('table', { name: 'Network interfaces' }) + await expectRowVisible(nicTable, { 'Transit IPs': '172.30.0.0/22+1' }) + + await page.getByText('+1').hover() + await expect( + page.getByRole('tooltip', { name: 'Other transit IPs 192.168.0.0/16' }) + ).toBeVisible() +}) diff --git a/test/e2e/vpcs.e2e.ts b/test/e2e/vpcs.e2e.ts index 331337132e..2e6ea3b466 100644 --- a/test/e2e/vpcs.e2e.ts +++ b/test/e2e/vpcs.e2e.ts @@ -218,51 +218,103 @@ test('can’t create or delete Routes on system routers', async ({ page }) => { await expect(page.getByRole('menuitem', { name: 'Delete' })).toBeDisabled() }) -test('can create, update, and delete Route', async ({ page }) => { - // go to the custom-router-2 page +test('create router route', async ({ page }) => { await page.goto('/projects/mock-project/vpcs/mock-vpc/routers/mock-custom-router') - const table = page.getByRole('table') - const routeRows = table.locator('tbody >> tr') - await expectRowVisible(table, { Name: 'drop-local' }) + + // Selectors + const form = page.getByRole('dialog', { name: 'Create route' }) + const nameInput = page.getByRole('textbox', { name: 'Name' }) + const destinationValueInput = page.getByRole('textbox', { name: 'Destination value' }) + const targetValueInput = page.getByRole('textbox', { name: 'Target value' }) + const submitButton = page.getByRole('button', { name: 'Create route' }) + + const invalidIpError = form.getByText('Not a valid IP address') + const invalidIpNetError = form.getByText( + 'Must contain an IP address and a width, separated by a /' + ) // create a new route - await page.click('text=New route') - await page.getByRole('textbox', { name: 'name' }).fill('new-route') - await page.getByRole('textbox', { name: 'Destination value' }).fill('0.0.0.0') - - // we'll set the target in a second, but first verify that selecting internet gateway disables the value - await page.getByRole('button', { name: 'Target type' }).click() - await page.getByRole('option', { name: 'Internet gateway' }).click() - await expect(page.getByRole('textbox', { name: 'Target value' })).toBeDisabled() - await expect(page.getByRole('textbox', { name: 'Target value' })).toHaveValue('outbound') - await page.getByRole('button', { name: 'Target type' }).click() - await page.getByRole('option', { name: 'IP' }).click() - await page.getByRole('textbox', { name: 'Target value' }).fill('1.1.1.1') - await page.getByRole('button', { name: 'Create route' }).click() - await expect(routeRows).toHaveCount(2) - await expectRowVisible(table, { + await page.getByRole('link', { name: 'New route' }).click() + await nameInput.fill('new-route') + + // Test IP validation for destination + await selectOption(page, 'Destination type', 'IP') + await destinationValueInput.fill('invalid-ip') + await expect(invalidIpError).toBeHidden() + await submitButton.click() + // other field was left empty, so is also invalid + await expect(invalidIpError).toHaveCount(2) + + // change target to valid, error disappear + await targetValueInput.fill('192.168.0.2') + await expect(invalidIpError).toHaveCount(1) + + // Test IP net validation for destination + await selectOption(page, 'Destination type', 'IP network') + // error on dest value clears on type change + await expect(invalidIpError).toBeHidden() + await destinationValueInput.fill('invalid-ip-net') + await expect(invalidIpError).toBeHidden() + await submitButton.click() + await expect(invalidIpNetError).toBeVisible() + + // revalidates on change + await destinationValueInput.fill('192.168.0.0/24') + await expect(invalidIpNetError).toBeHidden() + + // Set target + await selectOption(page, 'Target type', 'IP') + await targetValueInput.fill('10.0.0.1') + + await submitButton.click() + await expect(form).toBeHidden() + await expectRowVisible(page.getByRole('table'), { Name: 'new-route', - Destination: 'IP0.0.0.0', - Target: 'IP1.1.1.1', + Destination: 'IP network192.168.0.0/24', + Target: 'IP10.0.0.1', }) +}) + +test('edit and delete router route', async ({ page }) => { + await page.goto('/projects/mock-project/vpcs/mock-vpc/routers/mock-custom-router') - // update the route by clicking the edit button - await clickRowAction(page, 'new-route', 'Edit') - // change the destination type to VPC subnet: `mock-subnet` + const form = page.getByRole('dialog', { name: 'Edit route' }) + await expect(form).toBeHidden() + + await clickRowAction(page, 'drop-local', 'Edit') + await expect(form).toBeVisible() + + const targetValueInput = page.getByRole('textbox', { name: 'Target value' }) + const submitButton = page.getByRole('button', { name: 'Update route' }) + + await form.getByRole('textbox', { name: 'Name' }).fill('new-name') + + // Test IP validation for target + await selectOption(page, 'Target type', 'IP') + await targetValueInput.fill('invalid-ip') + await submitButton.click() + await expect(form.getByText('Not a valid IP address')).toBeVisible() + + await targetValueInput.fill('10.0.0.2') + await expect(form.getByText('Not a valid IP address')).toBeHidden() + + // Change destination to subnet await selectOption(page, 'Destination type', 'Subnet') await selectOption(page, 'Destination value', 'mock-subnet') - await page.getByRole('textbox', { name: 'Target value' }).fill('0.0.0.1') - await page.getByRole('button', { name: 'Update route' }).click() - await expect(routeRows).toHaveCount(2) + + await submitButton.click() + await expect(form).toBeHidden() + + const table = page.getByRole('table') await expectRowVisible(table, { - Name: 'new-route', + Name: 'new-name', Destination: 'VPC subnetmock-subnet', - Target: 'IP0.0.0.1', + Target: 'IP10.0.0.2', }) // delete the route - await clickRowAction(page, 'new-route', 'Delete') + await clickRowAction(page, 'new-name', 'Delete') await page.getByRole('button', { name: 'Confirm' }).click() - await expect(routeRows).toHaveCount(1) - await expect(page.getByRole('row', { name: 'new-route' })).toBeHidden() + await expect(table).toBeHidden() + await expect(page.getByText('No routes')).toBeVisible() })