Add button to attach an ephemeral IP (#2290)

* add detach button for ephemeral IP

* refactor

* add empty state for external ips

* capitalize modal title

* refine copy

* Update modal title to reflect that this is a confirmation step

* clinging to detachment is its own form of attachment

* copy tweak

* Update query to rely on ip passed in as query param; need to talk through API update

* Update mock api to allow for attaching ephemeral ip

* Update networking tab with ephemeral ip attach button

* add modal for attach ephemeral ip

* Add check to ensure nic exists in order to attach ephemeral IP

* Update networking tab tests

* error message improvement

* rows → nics

* more precise locator targeting in test

* refactor empty table copy

* Hide 'attach ephemeral IP' button when disabled

* hide 'Attach ephemeral IP' button instead of disabling it

* prefetch projectIpPoolList so the pool data is available to the modal

* update test to match new field label

* prefer not to use locator strings in new tests

* use usePrefetchedApiQuery for prefetched query!

* don't need to pass instance

* move attach modals to app/components

---------

Co-authored-by: David Crespo <david.crespo@oxidecomputer.com>

Author: charliepark

app/components/AttachEphemeralIpModal.tsx

@@ -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
+ */
+
+import { useMemo } from 'react'
+import { useForm } from 'react-hook-form'
+
+import { useApiMutation, useApiQueryClient, usePrefetchedApiQuery } from '~/api'
+import { ListboxField } from '~/components/form/fields/ListboxField'
+import { useInstanceSelector } from '~/hooks'
+import { addToast } from '~/stores/toast'
+import { Badge } from '~/ui/lib/Badge'
+import { Modal } from '~/ui/lib/Modal'
+
+export const AttachEphemeralIpModal = ({ onDismiss }: { onDismiss: () => void }) => {
+  const queryClient = useApiQueryClient()
+  const { project, instance } = useInstanceSelector()
+  const { data: siloPools } = usePrefetchedApiQuery('projectIpPoolList', {
+    query: { limit: 1000 },
+  })
+  const defaultPool = useMemo(
+    () => siloPools?.items.find((pool) => pool.isDefault),
+    [siloPools]
+  )
+  const instanceEphemeralIpAttach = useApiMutation('instanceEphemeralIpAttach', {
+    onSuccess() {
+      queryClient.invalidateQueries('instanceExternalIpList')
+      addToast({ content: 'Your ephemeral IP has been attached' })
+      onDismiss()
+    },
+    onError: (err) => {
+      addToast({ title: 'Error', content: err.message, variant: 'error' })
+    },
+  })
+  const form = useForm({ defaultValues: { pool: defaultPool?.name } })
+  const pool = form.watch('pool')
+
+  return (
+    <Modal isOpen title="Attach ephemeral IP" onDismiss={onDismiss}>
+      <Modal.Body>
+        <Modal.Section>
+          <form>
+            <ListboxField
+              control={form.control}
+              name="pool"
+              label="IP pool"
+              placeholder={
+                siloPools?.items && siloPools.items.length > 0
+                  ? 'Select pool'
+                  : 'No pools available'
+              }
+              items={
+                siloPools?.items.map((pool) => ({
+                  label: (
+                    <div className="flex items-center gap-2">
+                      {pool.name}
+                      {pool.isDefault && <Badge>default</Badge>}
+                    </div>
+                  ),
+                  value: pool.name,
+                })) || []
+              }
+              required
+            />
+          </form>
+        </Modal.Section>
+      </Modal.Body>
+      <Modal.Footer
+        actionText="Attach"
+        disabled={!pool}
+        onAction={() =>
+          instanceEphemeralIpAttach.mutate({
+            path: { instance },
+            query: { project },
+            body: { pool },
+          })
+        }
+        onDismiss={onDismiss}
+      ></Modal.Footer>
+    </Modal>
+  )
+}

…t/floating-ips/AttachFloatingIpModal.tsx app/components/AttachFloatingIpModal.tsxapp/pages/project/floating-ips/AttachFloatingIpModal.tsx renamed to app/components/AttachFloatingIpModal.tsx app/pages/project/floating-ips/AttachFloatingIpModal.tsx renamed to app/components/AttachFloatingIpModal.tsx

@@ -21,11 +21,12 @@ import {
 } from '@oxide/api'
 import { IpGlobal24Icon, Networking24Icon } from '@oxide/design-system/icons/react'
 
+import { AttachEphemeralIpModal } from '~/components/AttachEphemeralIpModal'
+import { AttachFloatingIpModal } from '~/components/AttachFloatingIpModal'
 import { HL } from '~/components/HL'
 import { CreateNetworkInterfaceForm } from '~/forms/network-interface-create'
 import { EditNetworkInterfaceForm } from '~/forms/network-interface-edit'
 import { getInstanceSelector, useInstanceSelector, useProjectSelector } from '~/hooks'
-import { AttachFloatingIpModal } from '~/pages/project/floating-ips/AttachFloatingIpModal'
 import { confirmAction } from '~/stores/confirm-action'
 import { confirmDelete } from '~/stores/confirm-delete'
 import { addToast } from '~/stores/toast'
@@ -94,6 +95,10 @@ NetworkingTab.loader = async ({ params }: LoaderFunctionArgs) => {
       path: { instance },
       query: { project },
     }),
+    // This is used in AttachEphemeralIpModal
+    apiQueryClient.prefetchQuery('projectIpPoolList', {
+      query: { limit: 1000 },
+    }),
   ])
   return null
 }
@@ -131,7 +136,8 @@ export function NetworkingTab() {
 
   const [createModalOpen, setCreateModalOpen] = useState(false)
   const [editing, setEditing] = useState<InstanceNetworkInterface | null>(null)
-  const [attachModalOpen, setAttachModalOpen] = useState(false)
+  const [attachEphemeralModalOpen, setAttachEphemeralModalOpen] = useState(false)
+  const [attachFloatingModalOpen, setAttachFloatingModalOpen] = useState(false)
 
   // Fetch the floating IPs to show in the "Attach floating IP" modal
   const { data: ips } = usePrefetchedApiQuery('floatingIpList', {
@@ -216,13 +222,13 @@ export function NetworkingTab() {
 
   const columns = useColsWithActions(staticCols, makeActions)
 
-  const rows = usePrefetchedApiQuery('instanceNetworkInterfaceList', {
+  const nics = usePrefetchedApiQuery('instanceNetworkInterfaceList', {
     query: { ...instanceSelector, limit: 1000 },
   }).data.items
 
   const tableInstance = useReactTable({
     columns,
-    data: rows || [],
+    data: nics || [],
     getCoreRowModel: getCoreRowModel(),
   })
 
@@ -337,9 +343,14 @@ export function NetworkingTab() {
     getCoreRowModel: getCoreRowModel(),
   })
 
-  const disabledReason =
-    eips.items.length >= 32
-      ? 'IP address limit of 32 reached for this instance'
+  // If there's already an ephemeral IP, or if there are no network interfaces,
+  // they shouldn't be able to attach an ephemeral IP
+  const enableEphemeralAttachButton =
+    eips.items.filter((ip) => ip.kind === 'ephemeral').length === 0 && nics.length > 0
+
+  const floatingDisabledReason =
+    eips.items.filter((ip) => ip.kind === 'floating').length >= 32
+      ? 'Floating IP address limit of 32 reached for this instance'
       : availableIps.length === 0
         ? 'No available floating IPs'
         : null
@@ -348,18 +359,33 @@ export function NetworkingTab() {
     <>
       <TableControls>
         <TableTitle id="attached-ips-label">External IPs</TableTitle>
-        <CreateButton
-          onClick={() => setAttachModalOpen(true)}
-          disabled={!!disabledReason}
-          disabledReason={disabledReason}
-        >
-          Attach floating IP
-        </CreateButton>
-        {attachModalOpen && (
+        <div className="flex gap-3">
+          {/*
+            We normally wouldn't hide this button and would just have a disabled state on it,
+            but it is very rare for this button to be necessary, and it would be disabled
+            most of the time, for most users. To reduce clutter on the screen, we're hiding it.
+           */}
+          {enableEphemeralAttachButton && (
+            <CreateButton onClick={() => setAttachEphemeralModalOpen(true)}>
+              Attach ephemeral IP
+            </CreateButton>
+          )}
+          <CreateButton
+            onClick={() => setAttachFloatingModalOpen(true)}
+            disabled={!!floatingDisabledReason}
+            disabledReason={floatingDisabledReason}
+          >
+            Attach floating IP
+          </CreateButton>
+        </div>
+        {attachEphemeralModalOpen && (
+          <AttachEphemeralIpModal onDismiss={() => setAttachEphemeralModalOpen(false)} />
+        )}
+        {attachFloatingModalOpen && (
           <AttachFloatingIpModal
             floatingIps={availableIps}
             instance={instance}
-            onDismiss={() => setAttachModalOpen(false)}
+            onDismiss={() => setAttachFloatingModalOpen(false)}
           />
         )}
       </TableControls>
@@ -370,7 +396,7 @@ export function NetworkingTab() {
           <EmptyMessage
             icon={<IpGlobal24Icon />}
             title="No external IPs"
-            body="You need to attach an external IP to be able to see it here"
+            body="Attach an external IP to see it here"
           />
         </TableEmptyBox>
       )}
@@ -397,14 +423,14 @@ export function NetworkingTab() {
           />
         )}
       </TableControls>
-      {rows?.length && rows.length > 0 ? (
+      {nics.length > 0 ? (
         <Table aria-labelledby="nics-label" table={tableInstance} />
       ) : (
         <TableEmptyBox>
           <EmptyMessage
             icon={<Networking24Icon />}
             title="No network interfaces"
-            body="You need to create a network interface to be able to see it here"
+            body="Create a network interface to see it here"
           />
         </TableEmptyBox>
       )}

app/pages/project/instances/instance/tabs/NetworkingTab.tsx

@@ -40,7 +40,7 @@ export const lookupById = <T extends { id: string }>(table: T[], id: string) =>
 export const getIpFromPool = (poolName: string | undefined) => {
   const pool = lookup.ipPool({ pool: poolName })
   const ipPoolRange = db.ipPoolRanges.find((range) => range.ip_pool_id === pool.id)
-  if (!ipPoolRange) throw notFoundErr
+  if (!ipPoolRange) throw notFoundErr('IP pool range')
 
   // right now, we're just using the first address in the range, but we'll
   // want to filter the list of available IPs for the first unused address
@@ -168,12 +168,12 @@ export const lookup = {
     return image
   },
   ipPool({ pool: id }: PP.IpPool): Json<Api.IpPool> {
-    if (!id) throw notFoundErr
+    if (!id) throw notFoundErr('Missing IP pool ID or name')
 
     if (isUuid(id)) return lookupById(db.ipPools, id)
 
     const pool = db.ipPools.find((p) => p.name === id)
-    if (!pool) throw notFoundErr
+    if (!pool) throw notFoundErr('IP pool')
 
     return pool
   },

mock-api/msw/db.ts

@@ -561,6 +561,21 @@ export const handlers = makeHandlers({
     disk.state = { state: 'detached' }
     return disk
   },
+  instanceEphemeralIpAttach({ path, query: projectParams, body }) {
+    const instance = lookup.instance({ ...path, ...projectParams })
+    const { pool } = body
+    const firstAvailableAddress = getIpFromPool(pool)
+    const externalIp = {
+      ip: firstAvailableAddress,
+      kind: 'ephemeral' as const,
+    }
+    db.ephemeralIps.push({
+      instance_id: instance.id,
+      external_ip: externalIp,
+    })
+
+    return externalIp
+  },
   instanceEphemeralIpDetach({ path, query }) {
     const instance = lookup.instance({ ...path, ...query })
     // match API logic: find/remove first ephemeral ip attached to instance
@@ -1291,7 +1306,6 @@ export const handlers = makeHandlers({
   certificateDelete: NotImplemented,
   certificateList: NotImplemented,
   certificateView: NotImplemented,
-  instanceEphemeralIpAttach: NotImplemented,
   instanceMigrate: NotImplemented,
   instanceSerialConsoleStream: NotImplemented,
   instanceSshPublicKeyList: NotImplemented,

mock-api/msw/handlers.ts

@@ -7,13 +7,7 @@
  */
 import { expect, test } from '@playwright/test'
 
-import {
-  clickRowAction,
-  expectNotVisible,
-  expectRowVisible,
-  expectVisible,
-  stopInstance,
-} from './utils'
+import { clickRowAction, expectRowVisible, expectVisible, stopInstance } from './utils'
 
 test('Instance networking tab — NIC table', async ({ page }) => {
   await page.goto('/projects/mock-project/instances/db1')
@@ -74,7 +68,7 @@ test('Instance networking tab — NIC table', async ({ page }) => {
   await clickRowAction(page, 'nic-2', 'Edit')
   await page.fill('role=textbox[name="Name"]', 'nic-3')
   await page.click('role=button[name="Update network interface"]')
-  await expectNotVisible(page, ['role=cell[name="nic-2"]'])
+  await expect(page.getByRole('cell', { name: 'nic-2' })).toBeHidden()
   const nic3 = page.getByRole('cell', { name: 'nic-3' })
   await expect(nic3).toBeVisible()
 
@@ -84,7 +78,42 @@ test('Instance networking tab — NIC table', async ({ page }) => {
   await expect(nic3).toBeHidden()
 })
 
-test('Instance networking tab — External IPs', async ({ page }) => {
+test('Instance networking tab — Detach / Attach Ephemeral IPs', async ({ page }) => {
+  await page.goto('/projects/mock-project/instances/db1/network-interfaces')
+
+  const attachEphemeralIpButton = page.getByRole('button', { name: 'Attach ephemeral IP' })
+  const externalIpTable = page.getByRole('table', { name: 'External IPs' })
+  const ephemeralCell = externalIpTable.getByRole('cell', { name: 'ephemeral' })
+
+  // We start out with an ephemeral IP attached
+  await expect(ephemeralCell).toBeVisible()
+
+  // The 'Attach ephemeral IP' button should be hidden when there is still an existing ephemeral IP
+  await expect(attachEphemeralIpButton).toBeHidden()
+
+  // Detach the existing ephemeral IP
+  await clickRowAction(page, 'ephemeral', 'Detach')
+  await page.getByRole('button', { name: 'Confirm' }).click()
+  await expect(ephemeralCell).toBeHidden()
+
+  // The 'Attach ephemeral IP' button should be visible and enabled now that the existing ephemeral IP has been detached
+  await expect(attachEphemeralIpButton).toBeEnabled()
+
+  // Attach a new ephemeral IP
+  await attachEphemeralIpButton.click()
+  const modal = page.getByRole('dialog', { name: 'Attach ephemeral IP' })
+  await expect(modal).toBeVisible()
+  await page.getByRole('button', { name: 'IP pool' }).click()
+  await page.getByRole('option', { name: 'ip-pool-2' }).click()
+  await page.getByRole('button', { name: 'Attach', exact: true }).click()
+  await expect(modal).toBeHidden()
+  await expect(ephemeralCell).toBeVisible()
+
+  // The 'Attach ephemeral IP' button should be hidden after attaching an ephemeral IP
+  await expect(attachEphemeralIpButton).toBeHidden()
+})
+
+test('Instance networking tab — floating IPs', async ({ page }) => {
   await page.goto('/projects/mock-project/instances/db1/network-interfaces')
   const externalIpTable = page.getByRole('table', { name: 'External IPs' })
   const attachFloatingIpButton = page.getByRole('button', { name: 'Attach floating IP' })
@@ -121,12 +150,6 @@ test('Instance networking tab — External IPs', async ({ page }) => {
   // Since we detached it, we don't expect to see the row any longer
   await expect(externalIpTable.getByRole('cell', { name: 'cola-float' })).toBeHidden()
 
-  // And that button shouldbe enabled again
+  // And that button should be enabled again
   await expect(attachFloatingIpButton).toBeEnabled()
-
-  // Detach the ephemeral IP
-  await expect(externalIpTable.getByRole('cell', { name: 'ephemeral' })).toBeVisible()
-  await clickRowAction(page, 'ephemeral', 'Detach')
-  await page.getByRole('button', { name: 'Confirm' }).click()
-  await expect(externalIpTable.getByRole('cell', { name: 'ephemeral' })).toBeHidden()
 })