IP range add is IPv4 only (#2083)

update IP range add form and validation to be IPv4 only

Author: david-crespo

app/forms/ip-pool-range-add.tsx

@@ -5,6 +5,7 @@
  *
  * Copyright Oxide Computer Company
  */
+import { type FieldErrors } from 'react-hook-form'
 import { useNavigate } from 'react-router-dom'
 
 import { useApiMutation, useApiQueryClient, type IpRange } from '@oxide/api'
@@ -24,10 +25,7 @@ const defaultValues: IpRange = {
 
 const invalidAddressError = { type: 'pattern', message: 'Not a valid IP address' }
 
-const diffVersionError = {
-  type: 'custom',
-  message: 'First and last must be the same version',
-}
+const ipv6Error = { type: 'pattern', message: 'IPv6 ranges are not yet supported' }
 
 /**
  * Pretty straightforward -- make sure IPs are valid and both first and last
@@ -40,21 +38,27 @@ function resolver(values: IpRange) {
   const first = validateIp(values.first)
   const last = validateIp(values.last)
 
-  let errors = undefined
+  const errors: FieldErrors<IpRange> = {}
+
+  if (!first.valid) {
+    errors.first = invalidAddressError
+  } else if (first.isv6) {
+    errors.first = ipv6Error
+  }
 
-  if (!first.valid || !last.valid) {
-    errors = {
-      first: first.valid ? undefined : invalidAddressError,
-      last: last.valid ? undefined : invalidAddressError,
-    }
-  } else if ((first.isv4 && last.isv6) || (first.isv6 && last.isv4)) {
-    errors = { first: diffVersionError, last: diffVersionError }
+  if (!last.valid) {
+    errors.last = invalidAddressError
+  } else if (last.isv6) {
+    errors.last = ipv6Error
   }
 
-  // TODO: if we were really cool we could check first <= last but that sounds
-  // like a pain
+  // TODO: once we support IPv6 we need to check for version mismatch here
+
+  // TODO: if we were really cool we could check first <= last but it would add
+  // 6k gzipped to the bundle with ip-num
 
-  return errors ? { values: {}, errors } : { values, errors: {} }
+  // no errors
+  return Object.keys(errors).length > 0 ? { values: {}, errors } : { values, errors: {} }
 }
 
 export function IpPoolAddRangeSideModalForm() {
@@ -89,7 +93,7 @@ export function IpPoolAddRangeSideModalForm() {
     >
       <Message
         variant="info"
-        content="IP ranges are inclusive. Addresses can be either IPv4 or IPv6, but first and last must be the same version, and first must be less than or equal to last."
+        content="Only IPv4 ranges are currently supported. Ranges are inclusive, and first must be less than or equal to last."
       />
       <TextField
         name="first"

mock-api/ip-pool.ts

@@ -77,12 +77,13 @@ export const ipPoolRanges: Json<IpPoolRange[]> = [
     },
     time_created: new Date().toISOString(),
   },
+
   {
     id: '7e6e94b9-748e-4219-83a3-cec76253ec70',
     ip_pool_id: ipPool2.id,
     range: {
-      first: '10.0.0.33',
-      last: '10.0.0.38',
+      first: 'fd00::1',
+      last: 'fd00::20',
     },
     time_created: new Date().toISOString(),
   },

test/e2e/ip-pools.e2e.ts

@@ -23,7 +23,10 @@ test('IP pool list', async ({ page }) => {
   await expect(table.getByRole('row')).toHaveCount(5) // header + 4 rows
 
   await expectRowVisible(table, { name: 'ip-pool-1', Utilization: '6 / 24' })
-  await expectRowVisible(table, { name: 'ip-pool-2', Utilization: '0 / 6' })
+  await expectRowVisible(table, {
+    name: 'ip-pool-2',
+    Utilization: 'v4' + '0 / 0' + 'v6' + '0 / 32',
+  })
   await expectRowVisible(table, { name: 'ip-pool-3', Utilization: '0 / 0' })
   await expectRowVisible(table, {
     name: 'ip-pool-4',
@@ -133,12 +136,13 @@ test('IP pool create', async ({ page }) => {
 })
 
 test('IP range validation and add', async ({ page }) => {
-  await page.goto('/system/networking/ip-pools/ip-pool-1')
+  await page.goto('/system/networking/ip-pools/ip-pool-2')
 
   // check the utilization bar
-  await expect(page.getByText('IPv4(IPs)25%')).toBeVisible()
-  await expect(page.getByText('Allocated6')).toBeVisible()
-  await expect(page.getByText('Capacity24')).toBeVisible()
+  await expect(page.getByText('IPv4(IPs)')).toBeHidden()
+  await expect(page.getByText('IPv6(IPs)0%')).toBeVisible()
+  await expect(page.getByText('Allocated0')).toBeVisible()
+  await expect(page.getByText('Capacity32')).toBeVisible()
 
   await page.getByRole('link', { name: 'Add range' }).click()
 
@@ -148,10 +152,9 @@ test('IP range validation and add', async ({ page }) => {
   const submit = dialog.getByRole('button', { name: 'Add IP range' })
   const invalidMsg = dialog.getByText('Not a valid IP address')
   // exact to differentiate from same text in help message at the top of the form
-  const sameVersionMsg = dialog.getByText('First and last must be the same version', {
-    exact: true,
-  })
+  const ipv6Msg = dialog.getByText('IPv6 ranges are not yet supported')
 
+  const v4Addr = '192.1.2.3'
   const v6Addr = '2001:db8::1234:5678'
 
   await expect(dialog).toBeVisible()
@@ -162,50 +165,41 @@ test('IP range validation and add', async ({ page }) => {
 
   await expect(invalidMsg).toHaveCount(2)
 
-  // fix last
-  await last.fill('123.4.56.7')
-
-  // first is still bad
+  // change last to v6, not allowed
+  await last.fill(v6Addr)
   await expect(invalidMsg).toHaveCount(1)
+  await expect(ipv6Msg).toHaveCount(1)
 
-  // change first to a valid ipv6
+  // change first to v6, still not allowed
   await first.fill(v6Addr)
-
-  // now we get the error about the same version on first because it has had
-  // an error, so it is now validating onChange, but it doesn't show up on last
-  // until we try to submit
-  await expect(sameVersionMsg).toHaveCount(1)
+  await expect(ipv6Msg).toHaveCount(2)
   await expect(invalidMsg).toBeHidden()
 
-  await submit.click()
-  await expect(sameVersionMsg).toHaveCount(2)
-
-  // now make last also a v6 and we're good
-  await last.fill(v6Addr)
-
-  // actually first's error doesn't disappear until we blur it or submit
-  await expect(sameVersionMsg).toHaveCount(1)
-  await expect(invalidMsg).toBeHidden()
+  // now make first v4, then last
+  await first.fill(v4Addr)
+  await expect(ipv6Msg).toHaveCount(1)
+  await last.fill(v4Addr)
+  await expect(ipv6Msg).toBeHidden()
 
   await submit.click()
   await expect(dialog).toBeHidden()
 
   const table = page.getByRole('table')
-  await expectRowVisible(table, { First: v6Addr, Last: v6Addr })
+  await expectRowVisible(table, { First: v4Addr, Last: v4Addr })
 
   // now the utilization bars are split in two
-  await expect(page.getByText('IPv4(IPs)25%')).toBeVisible()
-  await expect(page.getByText('Allocated6')).toBeVisible()
-  await expect(page.getByText('Capacity24')).toBeVisible()
-  await expect(page.getByText('IPv6(IPs)0%')).toBeVisible()
-  await expect(page.getByText('Allocated0')).toBeVisible()
+  await expect(page.getByText('IPv4(IPs)0%')).toBeVisible()
+  await expect(page.getByText('Allocated0')).toHaveCount(2)
   await expect(page.getByText('Capacity1')).toBeVisible()
 
+  await expect(page.getByText('IPv6(IPs)0%')).toBeVisible()
+  await expect(page.getByText('Capacity32')).toBeVisible()
+
   // go back to the pool and verify the utilization column changed
   await page.getByRole('link', { name: 'Networking' }).click()
   await expectRowVisible(table, {
-    name: 'ip-pool-1',
-    Utilization: 'v4' + '6 / 24' + 'v6' + '0 / 1',
+    name: 'ip-pool-2',
+    Utilization: 'v4' + '0 / 1' + 'v6' + '0 / 32',
   })
 })
 
@@ -264,10 +258,14 @@ test('deleting floating IP decrements utilization', async ({ page }) => {
 })
 
 test('no ranges means no utilization bar', async ({ page }) => {
-  await page.goto('/system/networking/ip-pools/ip-pool-2')
+  await page.goto('/system/networking/ip-pools/ip-pool-1')
   await expect(page.getByText('IPv4(IPs)')).toBeVisible()
   await expect(page.getByText('IPv6(IPs)')).toBeHidden()
 
+  await page.goto('/system/networking/ip-pools/ip-pool-2')
+  await expect(page.getByText('IPv4(IPs)')).toBeHidden()
+  await expect(page.getByText('IPv6(IPs)')).toBeVisible()
+
   await page.goto('/system/networking/ip-pools/ip-pool-3')
   await expect(page.getByText('IPv4(IPs)')).toBeHidden()
   await expect(page.getByText('IPv6(IPs)')).toBeHidden()