Validate IPs and IP networks (#2461)

* move ip utils to their own file

* validateIpNet util based one existing validateIp

* let's try a more interesting result type on the validator

* validate IPs and IP nets on the route form

* redo validateIp to look more like validateIpNet, esp. including message

* tweak validation logic to get more helpful messages, test it

* validate transit IPs on network interface edit

* split parse from validate, it's clean!

* fix e2e failure but it raises existential questions

* test leading zeroes in width

* pass noop validate functions to comboboxes that get swapped with textfields

* validate IPs, IP nets and names on firewall rules form

* fix subform revalidation issue

* really fix subform revalidation and test it like crazy

* add test for transit IPs form, incorporate PR #2480

#2480

* validate transit ip dupes in validate function

* tests that catch clear button bug, fix bug

* have to do the reset on success effect for transit IPs too

* test IP validation in route form

* polish up last two todos to do two twos too

Author: david-crespo

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<TFieldValues>
   onChange?: (value: string | null | undefined) => void
+  validate?: Validate<FieldPathValue<TFieldValues, TName>, 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<TFieldValues, TName>) {
-  const { field, fieldState } = useController({ name, control, rules: { required } })
+  const { field, fieldState } = useController({
+    name,
+    control,
+    rules: { required, validate },
+  })
   return (
     <div className="max-w-lg">
       <Combobox

app/forms/firewall-rules-common.tsx

@@ -6,6 +6,7 @@
  * Copyright Oxide Computer Company
  */
 
+import { useEffect } from 'react'
 import {
   useController,
   useForm,
@@ -27,7 +28,7 @@ import { CheckboxField } from '~/components/form/fields/CheckboxField'
 import { ComboboxField } from '~/components/form/fields/ComboboxField'
 import { DescriptionField } from '~/components/form/fields/DescriptionField'
 import { ListboxField } from '~/components/form/fields/ListboxField'
-import { NameField } from '~/components/form/fields/NameField'
+import { NameField, validateName } from '~/components/form/fields/NameField'
 import { NumberField } from '~/components/form/fields/NumberField'
 import { RadioField } from '~/components/form/fields/RadioField'
 import { TextField, TextFieldInner } from '~/components/form/fields/TextField'
@@ -39,6 +40,7 @@ import * as MiniTable from '~/ui/lib/MiniTable'
 import { TextInputHint } from '~/ui/lib/TextInput'
 import { KEYS } from '~/ui/util/keys'
 import { ALL_ISH } from '~/util/consts'
+import { validateIp, validateIpNet } from '~/util/ip'
 import { links } from '~/util/links'
 import { capitalize } from '~/util/str'
 
@@ -62,7 +64,6 @@ type TargetAndHostFilterType =
 type TargetAndHostFormValues = {
   type: TargetAndHostFilterType
   value: string
-  subnetVpc?: string
 }
 
 // these are part of the target and host filter form;
@@ -132,8 +133,13 @@ const DynamicTypeAndValueFields = ({
           items={items}
           allowArbitraryValues
           hideOptionalTag
-          // TODO: validate here, but it's complicated because it's conditional
-          // on which type is selected
+          validate={(value) =>
+            // 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)
+          }
         />
       ) : (
         <TextField
@@ -146,8 +152,11 @@ const DynamicTypeAndValueFields = ({
               onSubmitTextField(e)
             }
           }}
-          // TODO: validate here, but it's complicated because it's conditional
-          // on which type is selected
+          validate={(value) =>
+            (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}
         />
         <MiniTable.ClearAndAddButtons
           addButtonCopy="Add target"
-          disableClear={!!targetValue}
+          disableClear={!targetValue}
           onClear={() => targetForm.reset()}
           onSubmit={submitTarget}
         />
@@ -468,8 +497,8 @@ export const CommonFields = ({ control, nameTaken, error }: CommonFieldsProps) =
         </div>
         <MiniTable.ClearAndAddButtons
           addButtonCopy="Add port filter"
-          disableClear={!!portValue}
-          onClear={portRangeForm.reset}
+          disableClear={!portValue}
+          onClear={() => portRangeForm.reset()}
           onSubmit={submitPortRange}
         />
       </div>
@@ -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}
         />
         <MiniTable.ClearAndAddButtons
           addButtonCopy="Add host filter"
-          disableClear={!!hostValue}
+          disableClear={!hostValue}
           onClear={() => hostForm.reset()}
           onSubmit={submitHost}
         />

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<IpRange> = {}
 
-  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
   }
 

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 (
     <SideModalForm
@@ -92,7 +99,7 @@ export function EditNetworkInterfaceForm({
             Transit IPs
           </FieldLabel>
           <TextInputHint id="transitIp-help-text" className="mb-2">
-            Enter an IPv4 or IPv6 address.{' '}
+            An IP network, like 192.168.0.0/16.{' '}
             <a href={links.transitIpsDocs} target="_blank" rel="noreferrer">
               Learn more about transit IPs.
             </a>
@@ -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"
           />
         </div>
         <MiniTable.ClearAndAddButtons
           addButtonCopy="Add Transit IP"
-          disableClear={!!transitIpsForm.formState.dirtyFields.transitIp}
-          onClear={transitIpsForm.reset}
+          disableClear={!transitIpValue}
+          onClear={() => transitIpsForm.reset()}
           onSubmit={submitTransitIp}
         />
       </div>