diff --git a/app/components/AttachEphemeralIpModal.tsx b/app/components/AttachEphemeralIpModal.tsx index 1e17d8dda..7088e4e9f 100644 --- a/app/components/AttachEphemeralIpModal.tsx +++ b/app/components/AttachEphemeralIpModal.tsx @@ -17,37 +17,34 @@ import { queryClient, useApiMutation, usePrefetchedQuery, + type IpVersion, } from '~/api' import { IpPoolSelector } from '~/components/form/fields/IpPoolSelector' import { HL } from '~/components/HL' import { useInstanceSelector } from '~/hooks/use-params' import { addToast } from '~/stores/toast' +import { Message } from '~/ui/lib/Message' import { Modal } from '~/ui/lib/Modal' import { ALL_ISH } from '~/util/consts' -import { getCompatibleVersionsFromNics } from '~/util/ip' -export const AttachEphemeralIpModal = ({ onDismiss }: { onDismiss: () => void }) => { +type AttachEphemeralIpModalProps = { + availableVersions: IpVersion[] + onDismiss: () => void +} + +export const AttachEphemeralIpModal = ({ + availableVersions, + onDismiss, +}: AttachEphemeralIpModalProps) => { const { project, instance } = useInstanceSelector() const { data: siloPools } = usePrefetchedQuery( q(api.ipPoolList, { query: { limit: ALL_ISH } }) ) - const { data: nics } = usePrefetchedQuery( - q(api.instanceNetworkInterfaceList, { query: { limit: ALL_ISH, project, instance } }) - ) - // Determine compatible IP versions based on instance's primary network interface - // External IPs route through the primary interface, so only its IP stack matters - // https://github.com/oxidecomputer/omicron/blob/d52aad0/nexus/db-queries/src/db/datastore/external_ip.rs#L544-L661 - const compatibleVersions = useMemo( - () => getCompatibleVersionsFromNics(nics.items), - [nics] - ) - - // Only unicast pools can be used for ephemeral IPs + // Only show unicast pools for the IP versions that still have open slots const compatibleUnicastPools = useMemo( - () => - siloPools.items.filter(isUnicastPool).filter(poolHasIpVersion(compatibleVersions)), - [siloPools, compatibleVersions] + () => siloPools.items.filter(isUnicastPool).filter(poolHasIpVersion(availableVersions)), + [siloPools, availableVersions] ) const defaultPool = useMemo(() => { @@ -73,25 +70,31 @@ export const AttachEphemeralIpModal = ({ onDismiss }: { onDismiss: () => void }) const pool = form.watch('pool') const disabledReason = - compatibleVersions.length === 0 - ? 'Instance has no network interfaces with compatible IP stacks' - : compatibleUnicastPools.length === 0 - ? 'No compatible unicast pools available for this instance' - : !pool - ? 'Select a pool to continue' - : undefined + compatibleUnicastPools.length === 0 + ? 'No compatible unicast pools available for this instance' + : !pool + ? 'Select a pool to continue' + : undefined + + const message = + availableVersions.length === 1 + ? `Only ${availableVersions[0]} pools are shown because this instance already has a ${availableVersions[0] === 'v4' ? 'v6' : 'v4'} ephemeral IP.` + : availableVersions.length === 2 + ? 'Dual-stack network interfaces support one ephemeral IP per version.' + : undefined return ( + {message && }
diff --git a/app/pages/project/instances/NetworkingTab.tsx b/app/pages/project/instances/NetworkingTab.tsx index 994836927..fa4c970ec 100644 --- a/app/pages/project/instances/NetworkingTab.tsx +++ b/app/pages/project/instances/NetworkingTab.tsx @@ -14,6 +14,7 @@ import { match } from 'ts-pattern' import { api, instanceCan, + isUnicastPool, q, qErrorsAllowed, queryClient, @@ -58,7 +59,12 @@ import { TableEmptyBox } from '~/ui/lib/Table' import { TipIcon } from '~/ui/lib/TipIcon' import { Tooltip } from '~/ui/lib/Tooltip' import { ALL_ISH } from '~/util/consts' -import { getCompatibleVersionsFromNics, ipHasVersion, parseIp } from '~/util/ip' +import { + getCompatibleVersionsFromNics, + getEphemeralIpSlots, + ipHasVersion, + parseIp, +} from '~/util/ip' import { pb } from '~/util/path-builder' import { fancifyStates } from './common' @@ -301,6 +307,11 @@ export default function NetworkingTab() { }) ).data.items + const { data: siloPools } = usePrefetchedQuery( + q(api.ipPoolList, { query: { limit: ALL_ISH } }) + ) + const unicastPools = useMemo(() => siloPools.items.filter(isUnicastPool), [siloPools]) + // Determine compatible IP versions from the instance's primary NIC // External IPs route through the primary interface, so only its IP stack matters const compatibleVersions = useMemo(() => getCompatibleVersionsFromNics(nics), [nics]) @@ -466,11 +477,14 @@ export default function NetworkingTab() { } const doDetach = match(externalIp) - .with( - { kind: 'ephemeral' }, - () => () => - ephemeralIpDetach({ path: { instance: instanceName }, query: { project } }) - ) + .with({ kind: 'ephemeral' }, () => () => { + const parsed = parseIp(externalIp.ip) + const ipVersion = parsed.type === 'error' ? undefined : parsed.type + return ephemeralIpDetach({ + path: { instance: instanceName }, + query: { project, ipVersion }, + }) + }) .with( { kind: 'floating' }, ({ name }) => @@ -517,12 +531,17 @@ export default function NetworkingTab() { getCoreRowModel: getCoreRowModel(), }) - const ephemeralDisabledReason = - nics.length === 0 - ? 'Instance has no network interfaces' - : eips.items.some((ip) => ip.kind === 'ephemeral') - ? 'Instance already has an ephemeral IP' - : null + const attachedEphemeralIps = useMemo( + () => eips.items.filter((ip) => ip.kind === 'ephemeral'), + [eips] + ) + const { + availableVersions: ephemeralAvailableVersions, + disabledReason: ephemeralDisabledReason, + } = useMemo( + () => getEphemeralIpSlots(compatibleVersions, attachedEphemeralIps, unicastPools), + [compatibleVersions, attachedEphemeralIps, unicastPools] + ) const floatingDisabledReason = eips.items.filter((ip) => ip.kind === 'floating').length >= 32 @@ -574,7 +593,10 @@ export default function NetworkingTab() { {attachEphemeralModalOpen && ( - setAttachEphemeralModalOpen(false)} /> + setAttachEphemeralModalOpen(false)} + /> )} {attachFloatingModalOpen && ( ({ + id: `id-${name}`, + name, + description: '', + ipVersion, + isDefault: false, + poolType: 'unicast', + timeCreated: new Date(), + timeModified: new Date(), +}) + +const v4Pool = makePool('v4') +const v6Pool = makePool('v6') + +const v4Ephemeral: ExternalIp = { ip: '10.0.0.1', ipPoolId: 'p1', kind: 'ephemeral' } +const v6Ephemeral: ExternalIp = { ip: 'fd00::1', ipPoolId: 'p2', kind: 'ephemeral' } + +describe('getEphemeralIpSlots', () => { + test('no NICs', () => { + expect(getEphemeralIpSlots([], [], [v4Pool, v6Pool])).toEqual({ + availableVersions: [], + disabledReason: 'Instance has no network interfaces', + }) + }) + + test('v4-only, no attached ephemeral', () => { + expect(getEphemeralIpSlots(['v4'], [], [v4Pool])).toEqual({ + availableVersions: ['v4'], + disabledReason: null, + }) + }) + + test('v4-only, v4 attached', () => { + expect(getEphemeralIpSlots(['v4'], [v4Ephemeral], [v4Pool])).toEqual({ + availableVersions: [], + disabledReason: 'Instance already has an ephemeral IP', + }) + }) + + test('v6-only, no attached ephemeral', () => { + expect(getEphemeralIpSlots(['v6'], [], [v6Pool])).toEqual({ + availableVersions: ['v6'], + disabledReason: null, + }) + }) + + test('v6-only, v6 attached', () => { + expect(getEphemeralIpSlots(['v6'], [v6Ephemeral], [v6Pool])).toEqual({ + availableVersions: [], + disabledReason: 'Instance already has an ephemeral IP', + }) + }) + + test('dual-stack, no attached', () => { + expect(getEphemeralIpSlots(['v4', 'v6'], [], [v4Pool, v6Pool])).toEqual({ + availableVersions: ['v4', 'v6'], + disabledReason: null, + }) + }) + + test('dual-stack, v4 attached, v6 pools available', () => { + expect(getEphemeralIpSlots(['v4', 'v6'], [v4Ephemeral], [v4Pool, v6Pool])).toEqual({ + availableVersions: ['v6'], + disabledReason: null, + }) + }) + + test('dual-stack, v6 attached, v4 pools available', () => { + expect(getEphemeralIpSlots(['v4', 'v6'], [v6Ephemeral], [v4Pool, v6Pool])).toEqual({ + availableVersions: ['v4'], + disabledReason: null, + }) + }) + + test('dual-stack, both attached', () => { + expect( + getEphemeralIpSlots(['v4', 'v6'], [v4Ephemeral, v6Ephemeral], [v4Pool, v6Pool]) + ).toEqual({ + availableVersions: [], + disabledReason: 'Instance already has ephemeral IPs for all supported address types', + }) + }) + + test('dual-stack, no attached, only v4 pools available', () => { + expect(getEphemeralIpSlots(['v4', 'v6'], [], [v4Pool])).toEqual({ + availableVersions: ['v4'], + disabledReason: null, + }) + }) + + test('dual-stack, no attached, only v6 pools available', () => { + expect(getEphemeralIpSlots(['v4', 'v6'], [], [v6Pool])).toEqual({ + availableVersions: ['v6'], + disabledReason: null, + }) + }) + + test('dual-stack, v4 attached, no v6 pools', () => { + expect(getEphemeralIpSlots(['v4', 'v6'], [v4Ephemeral], [v4Pool])).toEqual({ + availableVersions: [], + disabledReason: 'No V6 pools available for ephemeral IPs', + }) + }) + + test('dual-stack, v6 attached, no v4 pools', () => { + expect(getEphemeralIpSlots(['v4', 'v6'], [v6Ephemeral], [v6Pool])).toEqual({ + availableVersions: [], + disabledReason: 'No V4 pools available for ephemeral IPs', + }) + }) + + test('dual-stack, no attached, no pools at all', () => { + expect(getEphemeralIpSlots(['v4', 'v6'], [], [])).toEqual({ + availableVersions: [], + disabledReason: 'No V4/V6 pools available for ephemeral IPs', + }) + }) + + test('v4-only, no pools available', () => { + expect(getEphemeralIpSlots(['v4'], [], [v6Pool])).toEqual({ + availableVersions: [], + disabledReason: 'No V4 pools available for ephemeral IPs', + }) + }) + + test('v4-only, v6 attached (ignored)', () => { + expect(getEphemeralIpSlots(['v4'], [v6Ephemeral], [v4Pool])).toEqual({ + availableVersions: ['v4'], + disabledReason: null, + }) + }) + + test('dual-stack, invalid attached IP is ignored', () => { + const invalidIp: ExternalIp = { ip: 'not-an-ip', ipPoolId: 'p3', kind: 'ephemeral' } + expect(getEphemeralIpSlots(['v4', 'v6'], [invalidIp], [v4Pool, v6Pool])).toEqual({ + availableVersions: ['v4', 'v6'], + disabledReason: null, + }) + }) +}) // 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. diff --git a/app/util/ip.ts b/app/util/ip.ts index c4f882a14..074a7aaee 100644 --- a/app/util/ip.ts +++ b/app/util/ip.ts @@ -6,7 +6,7 @@ * Copyright Oxide Computer Company */ -import type { InstanceNetworkInterface, IpVersion, UnicastIpPool } from '~/api' +import type { ExternalIp, InstanceNetworkInterface, IpVersion, UnicastIpPool } from '~/api' // 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 @@ -119,6 +119,58 @@ export const ipHasVersion = return ipVersion.type !== 'error' && versions.includes(ipVersion.type) } +export type EphemeralIpSlots = { + availableVersions: IpVersion[] + disabledReason: string | null +} + +/** + * Determine which ephemeral IP versions can still be attached and whether the + * attach button should be disabled. The API allows one ephemeral IP per IP + * version supported by the primary NIC (e.g., a dual-stack instance can have + * both a v4 and a v6 ephemeral IP). + */ +export function getEphemeralIpSlots( + compatibleVersions: IpVersion[], + attachedEphemeralIps: ExternalIp[], + unicastPools: UnicastIpPool[] +): EphemeralIpSlots { + if (compatibleVersions.length === 0) { + return { availableVersions: [], disabledReason: 'Instance has no network interfaces' } + } + + const attachedVersions = new Set() + for (const eip of attachedEphemeralIps) { + const parsed = parseIp(eip.ip) + if (parsed.type !== 'error') attachedVersions.add(parsed.type) + } + + const openVersions = compatibleVersions.filter( + (version) => !attachedVersions.has(version) + ) + + if (openVersions.length === 0) { + const msg = + compatibleVersions.length === 1 + ? 'Instance already has an ephemeral IP' + : 'Instance already has ephemeral IPs for all supported address types' + return { availableVersions: [], disabledReason: msg } + } + + const poolVersions = new Set(unicastPools.map((pool) => pool.ipVersion)) + const availableVersions = openVersions.filter((version) => poolVersions.has(version)) + + if (availableVersions.length === 0) { + const versionLabel = openVersions.map((version) => version.toUpperCase()).join('/') + return { + availableVersions: [], + disabledReason: `No ${versionLabel} pools available for ephemeral IPs`, + } + } + + return { availableVersions, disabledReason: null } +} + export const getDefaultIps = (pools: UnicastIpPool[]) => { const defaultPools = pools.filter((pool) => pool.isDefault) const v4Default = defaultPools.find((p) => p.ipVersion === 'v4') diff --git a/mock-api/msw/handlers.ts b/mock-api/msw/handlers.ts index 1ad736944..486c53a89 100644 --- a/mock-api/msw/handlers.ts +++ b/mock-api/msw/handlers.ts @@ -885,7 +885,7 @@ export const handlers = makeHandlers({ const ip = getIpFromPool(pool) // Validate that external IP version matches primary NIC's IP stack - // Based on Omicron validation in nexus/db-queries/src/db/datastore/external_ip.rs:544-661 + // https://github.com/oxidecomputer/omicron/blob/558f89e/nexus/db-queries/src/db/datastore/external_ip.rs#L673-L687 const nics = db.networkInterfaces.filter((n) => n.instance_id === instance.id) const primaryNic = nics.find((n) => n.primary) @@ -931,12 +931,42 @@ export const handlers = makeHandlers({ return externalIp }, instanceEphemeralIpDetach({ path, query }) { - const instance = lookup.instance({ ...path, ...query }) - // match API logic: find/remove first ephemeral ip attached to instance - // https://github.com/oxidecomputer/omicron/blob/d52aad0/nexus/db-queries/src/db/datastore/external_ip.rs#L782-L794 - // https://github.com/oxidecomputer/omicron/blob/d52aad0/nexus/src/app/sagas/instance_ip_detach.rs#L79-L82 - const ip = db.ephemeralIps.find((eip) => eip.instance_id === instance.id) - if (!ip) throw notFoundErr(`ephemeral IP for instance ${instance.name}`) + // When an instance has both IPv4 and IPv6 ephemeral IPs attached, Omicron + // requires an explicit `ipVersion` query param to disambiguate which to + // detach. + // https://github.com/oxidecomputer/omicron/blob/558f89e/nexus/types/src/external_api/params.rs#L267-L277 + // https://github.com/oxidecomputer/omicron/blob/558f89e/nexus/src/app/sagas/instance_ip_detach.rs#L75-L99 + // https://github.com/oxidecomputer/omicron/blob/558f89e/nexus/src/external_api/http_entrypoints.rs#L4913-L4939 + const { ipVersion, ...instanceQuery } = query + const instance = lookup.instance({ ...path, ...instanceQuery }) + + const attachedIps = db.ephemeralIps.filter((eip) => eip.instance_id === instance.id) + if (attachedIps.length === 0) { + throw notFoundErr(`ephemeral IP for instance ${instance.name}`) + } + + const versionOf = (ip: string) => (ip.includes(':') ? 'v6' : 'v4') + const attachedVersions = new Set( + attachedIps.map((eip) => versionOf(eip.external_ip.ip)) + ) + + if (attachedVersions.size > 1 && !ipVersion) { + throw json( + { + error_code: 'InvalidRequest', + message: `Instance ${instance.name} has both IPv4 and IPv6 ephemeral IPs; ipVersion is required to detach one`, + }, + { status: 400 } + ) + } + + const ip = + ipVersion === undefined + ? attachedIps[0] + : attachedIps.find((eip) => versionOf(eip.external_ip.ip) === ipVersion) + + if (!ip) throw notFoundErr(`ephemeral IP (${ipVersion}) for instance ${instance.name}`) + db.ephemeralIps = db.ephemeralIps.filter((eip) => eip !== ip) return 204 }, diff --git a/test/e2e/instance-networking.e2e.ts b/test/e2e/instance-networking.e2e.ts index 4cad91ad1..0c1806433 100644 --- a/test/e2e/instance-networking.e2e.ts +++ b/test/e2e/instance-networking.e2e.ts @@ -113,63 +113,97 @@ test('Instance networking tab — Detach / Attach Ephemeral IPs', async ({ page const attachEphemeralIpButton = page.getByRole('button', { name: 'Attach ephemeral IP' }) const externalIpTable = page.getByRole('table', { name: 'External IPs' }) - const ephemeralCell = externalIpTable.getByRole('cell', { name: 'ephemeral' }) + const ephemeralCells = externalIpTable.getByRole('cell', { name: 'ephemeral' }) - // We start out with an ephemeral IP attached - await expect(ephemeralCell).toBeVisible() - - // The 'Attach ephemeral IP' button should be disabled when there is already an ephemeral IP - await expect(attachEphemeralIpButton).toBeDisabled() - - // 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 + // db1 starts with one v4 ephemeral IP and a dual-stack NIC, so the v6 slot + // is still open and the button should be enabled + await expect(ephemeralCells).toHaveCount(1) await expect(attachEphemeralIpButton).toBeEnabled() - // Attach a new ephemeral IP — since db1 has a dual-stack NIC, both v4 and v6 - // defaults are compatible, so no pool is preselected and we must choose one + // Attach a v6 ephemeral IP to fill the remaining slot. Since only the v6 + // slot is open, only v6 pools should appear and the v6 default is preselected. await attachEphemeralIpButton.click() const modal = page.getByRole('dialog', { name: 'Attach ephemeral IP' }) await expect(modal).toBeVisible() - await expect(page.getByLabel('Pool')).toContainText('Select a pool') - await page.getByLabel('Pool').click() - await page.getByRole('option', { name: 'ip-pool-1' }).click() + await expect(page.getByLabel('Pool')).toContainText('ip-pool-2') await page.getByRole('button', { name: 'Attach', exact: true }).click() await expect(modal).toBeHidden() - await expect(ephemeralCell).toBeVisible() + + // Both v4 and v6 ephemeral IPs are now attached await expectRowVisible(externalIpTable, { Kind: 'ephemeral', Version: 'v4', 'IP pool': 'ip-pool-1', }) + await expectRowVisible(externalIpTable, { + Kind: 'ephemeral', + Version: 'v6', + 'IP pool': 'ip-pool-2', + }) - // The 'Attach ephemeral IP' button should be disabled after attaching an ephemeral IP + // Button should be disabled now that both slots are filled await expect(attachEphemeralIpButton).toBeDisabled() - // Detach and test with explicit pool selection - await clickRowAction(page, 'ephemeral', 'Detach') - await page.getByRole('button', { name: 'Confirm' }).click() - await expect(ephemeralCell).toBeHidden() + // Detach the v6 ephemeral IP while both versions are attached (requires + // an explicit ipVersion selector), then reattach it. + await clickRowAction(page, 'fd00::1', 'Detach') + const confirmDetachDialog = page.getByRole('dialog', { + name: 'Confirm detach ephemeral IP', + }) + await expect(confirmDetachDialog).toBeVisible() + await confirmDetachDialog.getByRole('button', { name: 'Confirm' }).click() + await expect(confirmDetachDialog).toBeHidden() + await expect(attachEphemeralIpButton).toBeEnabled() + + await attachEphemeralIpButton.click() + await expect(modal).toBeVisible() + await expect(page.getByLabel('Pool')).toContainText('ip-pool-2') + await page.getByRole('button', { name: 'Attach', exact: true }).click() + await expect(modal).toBeHidden() + await expect(attachEphemeralIpButton).toBeDisabled() + + // Detach the v4 ephemeral IP — button should re-enable for v4 + await clickRowAction(page, '123.4.56.0', 'Detach') + await expect(confirmDetachDialog).toBeVisible() + await confirmDetachDialog.getByRole('button', { name: 'Confirm' }).click() + await expect(confirmDetachDialog).toBeHidden() + await expect(attachEphemeralIpButton).toBeEnabled() + + // Attach a v4 ephemeral IP — only v4 pools should be available + await attachEphemeralIpButton.click() + await expect(modal).toBeVisible() + await expect(page.getByLabel('Pool')).toContainText('ip-pool-1') + await page.getByRole('button', { name: 'Attach', exact: true }).click() + await expect(modal).toBeHidden() + await expect(attachEphemeralIpButton).toBeDisabled() + // Detach both ephemeral IPs + await clickRowAction(page, '123.4.56.0', 'Detach') + await expect(confirmDetachDialog).toBeVisible() + await confirmDetachDialog.getByRole('button', { name: 'Confirm' }).click() + await expect(confirmDetachDialog).toBeHidden() + // Use a more specific locator since we're targeting the v6 row + await clickRowAction(page, 'fd00::1', 'Detach') + await expect(confirmDetachDialog).toBeVisible() + await confirmDetachDialog.getByRole('button', { name: 'Confirm' }).click() + await expect(confirmDetachDialog).toBeHidden() + await expect(ephemeralCells).toHaveCount(0) + + // With no ephemeral IPs and a dual-stack NIC, both v4 and v6 pools should + // be available, meaning no default is preselected + await expect(attachEphemeralIpButton).toBeEnabled() await attachEphemeralIpButton.click() await expect(modal).toBeVisible() await expect(page.getByLabel('Pool')).toContainText('Select a pool') await page.getByLabel('Pool').click() - await page.getByRole('option', { name: 'ip-pool-2' }).click() + await page.getByRole('option', { name: 'ip-pool-1' }).click() await page.getByRole('button', { name: 'Attach', exact: true }).click() await expect(modal).toBeHidden() - await expect(ephemeralCell).toBeVisible() await expectRowVisible(externalIpTable, { Kind: 'ephemeral', - Version: 'v6', - 'IP pool': 'ip-pool-2', + Version: 'v4', + 'IP pool': 'ip-pool-1', }) - - // The 'Attach ephemeral IP' button should be disabled after attaching an ephemeral IP - await expect(attachEphemeralIpButton).toBeDisabled() }) test('Instance networking tab — floating IPs', async ({ page }) => {