Skip to content

Commit 0928d24

Browse files
Add snat to external ips (#2868)
* Add SNAT to external IPs table * add test; add more mock SNATs to mock DB * Update IP Pools tests now that more IPs are allocated, but is the count off because of shared IPs * update mock data; dedupe utilization * copy * proper port range value * don't show SNAT in external IPs list at top of page * comments and cleanups * tweak tooltip copy --------- Co-authored-by: David Crespo <[email protected]>
1 parent 2959967 commit 0928d24

File tree

9 files changed

+172
-60
lines changed

9 files changed

+172
-60
lines changed

app/components/ExternalIps.tsx

Lines changed: 8 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -18,9 +18,9 @@ import { intersperse } from '~/util/array'
1818
import { pb } from '~/util/path-builder'
1919
import type * as PP from '~/util/path-params'
2020

21-
/** Move ephemeral IP (if present) to the end of the list of external IPs */
22-
export const orderIps = (ips: ExternalIp[]) =>
23-
R.sortBy(ips, (a) => (a.kind === 'ephemeral' ? 1 : -1))
21+
/** Order IPs: floating first, then ephemeral, then SNAT */
22+
const IP_ORDER = { floating: 0, ephemeral: 1, snat: 2 } as const
23+
export const orderIps = (ips: ExternalIp[]) => R.sortBy(ips, (a) => IP_ORDER[a.kind])
2424

2525
export function ExternalIps({ project, instance }: PP.Instance) {
2626
const { data, isPending } = useApiQuery('instanceExternalIpList', {
@@ -29,7 +29,11 @@ export function ExternalIps({ project, instance }: PP.Instance) {
2929
})
3030
if (isPending) return <SkeletonCell />
3131

32-
const ips = data?.items
32+
// Exclude SNAT IPs from the properties table because they are rarely going
33+
// to be what the user wants as the "external IP" of the instance -- they
34+
// want one that can receive inbound traffic. This will have to change with
35+
// https://github.com/oxidecomputer/omicron/issues/4317
36+
const ips = data?.items.filter((ip) => ip.kind !== 'snat')
3337
if (!ips || ips.length === 0) return <EmptyCell />
3438
const orderedIps = orderIps(ips)
3539
const ipsToShow = orderedIps.slice(0, 2)

app/pages/project/instances/InstancePage.tsx

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -263,7 +263,7 @@ export default function InstancePage() {
263263
<PropertiesTable.DateRow date={instance.timeCreated} label="Created" />
264264
<PropertiesTable.IdRow id={instance.id} />
265265
<PropertiesTable.Row label="external IPs">
266-
{<ExternalIps {...instanceSelector} />}
266+
<ExternalIps {...instanceSelector} />
267267
</PropertiesTable.Row>
268268
</PropertiesTable>
269269
<RouteTabs fullWidth>

app/pages/project/instances/NetworkingTab.tsx

Lines changed: 67 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@
88
import { createColumnHelper, getCoreRowModel, useReactTable } from '@tanstack/react-table'
99
import { useCallback, useMemo, useState } from 'react'
1010
import { type LoaderFunctionArgs } from 'react-router'
11-
import { match, P } from 'ts-pattern'
11+
import { match } from 'ts-pattern'
1212

1313
import {
1414
apiQueryClient,
@@ -88,8 +88,11 @@ const SubnetNameFromId = ({ value }: { value: string }) => {
8888
return <span className="text-default">{subnet.name}</span>
8989
}
9090

91-
const EphemeralIPEmptyCell = () => (
92-
<Tooltip content="Ephemeral IPs don’t have names or descriptions" placement="top">
91+
const NonFloatingEmptyCell = ({ kind }: { kind: 'snat' | 'ephemeral' }) => (
92+
<Tooltip
93+
content={`${kind === 'snat' ? 'SNAT' : 'Ephemeral'} IPs don’t have names or descriptions`}
94+
placement="top"
95+
>
9396
<div>
9497
<EmptyCell />
9598
</div>
@@ -168,14 +171,30 @@ const updateNicStates = fancifyStates(instanceCan.updateNic.states)
168171
const ipColHelper = createColumnHelper<ExternalIp>()
169172
const staticIpCols = [
170173
ipColHelper.accessor('ip', {
171-
cell: (info) => <CopyableIp ip={info.getValue()} />,
174+
cell: (info) => (
175+
<div className="flex items-center gap-2">
176+
<CopyableIp ip={info.getValue()} />
177+
{info.row.original.kind === 'snat' && (
178+
<Tooltip content="Outbound traffic uses this IP and port range" placement="top">
179+
{/* div needed for Tooltip */}
180+
<div>
181+
<Badge color="neutral">
182+
{info.row.original.firstPort}{info.row.original.lastPort}
183+
</Badge>
184+
</div>
185+
</Tooltip>
186+
)}
187+
</div>
188+
),
172189
}),
173190
ipColHelper.accessor('kind', {
174191
header: () => (
175192
<>
176193
Kind
177194
<TipIcon className="ml-2">
178-
Floating IPs can be detached from this instance and attached to another
195+
Floating IPs can be detached from this instance and attached to another. SNAT IPs
196+
cannot receive traffic; they are used for outbound traffic when there are no
197+
ephemeral or floating IPs.
179198
</TipIcon>
180199
</>
181200
),
@@ -187,15 +206,19 @@ const staticIpCols = [
187206
}),
188207
ipColHelper.accessor('name', {
189208
cell: (info) =>
190-
info.row.original.kind === 'ephemeral' ? <EphemeralIPEmptyCell /> : info.getValue(),
209+
info.row.original.kind === 'floating' ? (
210+
info.getValue()
211+
) : (
212+
<NonFloatingEmptyCell kind={info.row.original.kind} />
213+
),
191214
}),
192215
ipColHelper.accessor((row) => ('description' in row ? row.description : undefined), {
193216
header: 'description',
194217
cell: (info) =>
195-
info.row.original.kind === 'ephemeral' ? (
196-
<EphemeralIPEmptyCell />
197-
) : (
218+
info.row.original.kind === 'floating' ? (
198219
<DescriptionCell text={info.getValue()} />
220+
) : (
221+
<NonFloatingEmptyCell kind={info.row.original.kind} />
199222
),
200223
}),
201224
]
@@ -364,18 +387,38 @@ export default function NetworkingTab() {
364387
},
365388
}
366389

367-
const doAction =
368-
externalIp.kind === 'floating'
369-
? () =>
370-
floatingIpDetach({
371-
path: { floatingIp: externalIp.name },
372-
query: { project },
373-
})
374-
: () =>
375-
ephemeralIpDetach({
376-
path: { instance: instanceName },
377-
query: { project },
378-
})
390+
if (externalIp.kind === 'snat') {
391+
return [
392+
copyAction,
393+
{
394+
label: 'Detach',
395+
disabled: "SNAT IPs can't be detached",
396+
onActivate: () => {},
397+
},
398+
]
399+
}
400+
401+
const doDetach = match(externalIp)
402+
.with(
403+
{ kind: 'ephemeral' },
404+
() => () =>
405+
ephemeralIpDetach({ path: { instance: instanceName }, query: { project } })
406+
)
407+
.with(
408+
{ kind: 'floating' },
409+
({ name }) =>
410+
() =>
411+
floatingIpDetach({ path: { floatingIp: name }, query: { project } })
412+
)
413+
.exhaustive()
414+
415+
const label = match(externalIp)
416+
.with({ kind: 'ephemeral' }, () => 'this ephemeral IP')
417+
.with(
418+
{ kind: 'floating' },
419+
({ name }) => <>floating IP <HL>{name}</HL></> // prettier-ignore
420+
)
421+
.exhaustive()
379422

380423
return [
381424
copyAction,
@@ -384,21 +427,12 @@ export default function NetworkingTab() {
384427
onActivate: () =>
385428
confirmAction({
386429
actionType: 'danger',
387-
doAction,
430+
doAction: doDetach,
388431
modalTitle: `Confirm detach ${externalIp.kind} IP`,
389432
modalContent: (
390433
<p>
391-
Are you sure you want to detach{' '}
392-
{match(externalIp)
393-
.with({ kind: P.union('ephemeral', 'snat') }, () => 'this ephemeral IP')
394-
.with({ kind: 'floating' }, ({ name }) => (
395-
<>
396-
floating IP <HL>{name}</HL>
397-
</>
398-
))
399-
.exhaustive()}{' '}
400-
from <HL>{instanceName}</HL>? The instance will no longer be reachable at{' '}
401-
<HL>{externalIp.ip}</HL>.
434+
Are you sure you want to detach {label} from <HL>{instanceName}</HL>? The
435+
instance will no longer be reachable at <HL>{externalIp.ip}</HL>.
402436
</p>
403437
),
404438
errorTitle: `Error detaching ${externalIp.kind} IP`,

mock-api/external-ip.ts

Lines changed: 54 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@
77
*/
88
import type { ExternalIp } from '@oxide/api'
99

10-
import { instances } from './instance'
10+
import { failedInstance, instance, instanceDb2, startingInstance } from './instance'
1111
import { ipPool1 } from './ip-pool'
1212
import type { Json } from './json-type'
1313

@@ -24,44 +24,88 @@ type DbExternalIp = {
2424
external_ip: Json<ExternalIp>
2525
}
2626

27-
// Note that ExternalIp is a union of types representing ephemeral and floating
28-
// IPs, but we only put the ephemeral ones here. We have a separate table for
29-
// floating IPs analogous to the floating_ip view in Nexus.
27+
// Note that ExternalIp is a union of types representing ephemeral, floating, and
28+
// SNAT IPs, but we only put the ephemeral and SNAT ones here. We have a separate
29+
// table for floating IPs, analogous to the floating_ip view in Nexus.
3030

3131
// Note that these addresses should come from ranges in ip-pool-1
32-
3332
export const ephemeralIps: DbExternalIp[] = [
3433
{
35-
instance_id: instances[0].id,
34+
instance_id: instance.id,
3635
external_ip: {
3736
ip: '123.4.56.0',
3837
ip_pool_id: ipPool1.id,
3938
kind: 'ephemeral',
4039
},
4140
},
42-
// middle one has no IPs
41+
// failedInstance has no ephemeral IPs
4342
{
44-
instance_id: instances[2].id,
43+
instance_id: startingInstance.id,
4544
external_ip: {
4645
ip: '123.4.56.1',
4746
ip_pool_id: ipPool1.id,
4847
kind: 'ephemeral',
4948
},
5049
},
5150
{
52-
instance_id: instances[2].id,
51+
instance_id: startingInstance.id,
5352
external_ip: {
5453
ip: '123.4.56.2',
5554
ip_pool_id: ipPool1.id,
5655
kind: 'ephemeral',
5756
},
5857
},
5958
{
60-
instance_id: instances[2].id,
59+
instance_id: startingInstance.id,
6160
external_ip: {
6261
ip: '123.4.56.3',
6362
ip_pool_id: ipPool1.id,
6463
kind: 'ephemeral',
6564
},
6665
},
6766
]
67+
68+
// Note that SNAT IPs are subdivided into four ranges of ports,
69+
// with each instance getting a unique range.
70+
export const snatIps: DbExternalIp[] = [
71+
{
72+
instance_id: instance.id,
73+
external_ip: {
74+
ip: '123.4.56.10',
75+
ip_pool_id: ipPool1.id,
76+
kind: 'snat',
77+
first_port: 0,
78+
last_port: 16383,
79+
},
80+
},
81+
{
82+
instance_id: startingInstance.id,
83+
external_ip: {
84+
ip: '123.4.56.10',
85+
ip_pool_id: ipPool1.id,
86+
kind: 'snat',
87+
first_port: 16384,
88+
last_port: 32767,
89+
},
90+
},
91+
{
92+
instance_id: instanceDb2.id,
93+
external_ip: {
94+
ip: '123.4.56.10',
95+
ip_pool_id: ipPool1.id,
96+
kind: 'snat',
97+
first_port: 32768,
98+
last_port: 49151,
99+
},
100+
},
101+
{
102+
instance_id: failedInstance.id,
103+
external_ip: {
104+
ip: '123.4.56.10',
105+
ip_pool_id: ipPool1.id,
106+
kind: 'snat',
107+
first_port: 49152,
108+
last_port: 65535,
109+
},
110+
},
111+
]

mock-api/instance.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -34,7 +34,7 @@ export const instance: Json<Instance> = {
3434
boot_disk_id: '7f2309a5-13e3-47e0-8a4c-2a3b3bc992fd', // disk-1: needs to be written out here to reduce circular dependencies
3535
}
3636

37-
const failedInstance: Json<Instance> = {
37+
export const failedInstance: Json<Instance> = {
3838
...base,
3939
id: 'b5946edc-5bed-4597-88ab-9a8beb9d32a4',
4040
name: 'you-fail',

mock-api/msw/db.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -512,6 +512,7 @@ const initDb = {
512512
sleds: [...mock.sleds],
513513
switches: [...mock.switches],
514514
snapshots: [...mock.snapshots],
515+
snatIps: [...mock.snatIps],
515516
sshKeys: [...mock.sshKeys],
516517
users: [...mock.users],
517518
vpcFirewallRules: [...mock.firewallRules],

mock-api/msw/handlers.ts

Lines changed: 9 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -752,13 +752,17 @@ export const handlers = makeHandlers({
752752
.filter((eip) => eip.instance_id === instance.id)
753753
.map((eip) => eip.external_ip)
754754

755+
const snatIps = db.snatIps
756+
.filter((sip) => sip.instance_id === instance.id)
757+
.map((sip) => sip.external_ip)
758+
755759
// floating IPs are missing their `kind` field in the DB so we add it
756760
const floatingIps = db.floatingIps
757761
.filter((f) => f.instance_id === instance.id)
758762
.map((f) => ({ kind: 'floating' as const, ...f }))
759763

760764
// endpoint is not paginated. or rather, it's fake paginated
761-
return { items: [...ephemeralIps, ...floatingIps] }
765+
return { items: [...ephemeralIps, ...snatIps, ...floatingIps] }
762766
},
763767
instanceNetworkInterfaceList({ query }) {
764768
const instance = lookup.instance(query)
@@ -868,11 +872,12 @@ export const handlers = makeHandlers({
868872
(r) => parseIp(r.first).type === 'v4'
869873
)
870874

871-
// in the real backend there are also SNAT IPs, but we don't currently
872-
// represent those because they are not exposed through the API (except
873-
// through the counts)
875+
// Include SNAT IPs in IP utilization calculation, deduplicating by IP address
876+
// since multiple instances can share the same SNAT IP with different port ranges
877+
const uniqueSnatIps = [...new Set(db.snatIps.map((sip) => sip.external_ip.ip))]
874878
const allIps = [
875879
...db.ephemeralIps.map((eip) => eip.external_ip.ip),
880+
...uniqueSnatIps,
876881
...db.floatingIps.map((fip) => fip.ip),
877882
]
878883

test/e2e/instance-networking.e2e.ts

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -143,9 +143,14 @@ test('Instance networking tab — floating IPs', async ({ page }) => {
143143
// See list of external IPs
144144
await expectRowVisible(externalIpTable, { ip: '123.4.56.0', Kind: 'ephemeral' })
145145
await expectRowVisible(externalIpTable, { ip: '123.4.56.5', Kind: 'floating' })
146+
await expectRowVisible(externalIpTable, { ip: '123.4.56.100–16383', Kind: 'snat' })
146147

147148
await expect(page.getByText('external IPs123.4.56.5/123.4.56.0')).toBeVisible()
148149

150+
// The list of IPs at the top of the page should not show the SNAT IP
151+
await expect(page.getByText('external IPs123.4.56.5/123.4.56.0')).toBeVisible()
152+
await expect(page.getByText('external IPs123.4.56.5/123.4.56.0/')).toBeHidden()
153+
149154
// Attach a new external IP
150155
await attachFloatingIpButton.click()
151156
await expectVisible(page, ['role=heading[name="Attach floating IP"]'])
@@ -183,6 +188,25 @@ test('Instance networking tab — floating IPs', async ({ page }) => {
183188
await expect(attachFloatingIpButton).toBeEnabled()
184189
})
185190

191+
test('Instance networking tab — SNAT IPs', async ({ page }) => {
192+
await page.goto('/projects/mock-project/instances/db1/networking')
193+
const externalIpTable = page.getByRole('table', { name: 'External IPs' })
194+
const snatRow = externalIpTable.locator('tr').filter({ hasText: 'snat' })
195+
await expect(snatRow).toBeVisible()
196+
197+
// expect the SNAT IP to have a port range badge
198+
await expect(snatRow).toContainText('0–16383')
199+
200+
const actionsButton = snatRow.getByRole('button', { name: 'Row actions' })
201+
await actionsButton.click()
202+
203+
// Should have "Copy IP address" action, just for consistency with other IP rows
204+
await expect(page.getByRole('menuitem', { name: 'Copy IP address' })).toBeVisible()
205+
206+
// Should have a disabled "Detach" action
207+
await expect(page.getByRole('menuitem', { name: 'Detach' })).toBeDisabled()
208+
})
209+
186210
test('Edit network interface - Transit IPs', async ({ page }) => {
187211
await page.goto('/projects/mock-project/instances/db1/networking')
188212

0 commit comments

Comments
 (0)