Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 13 additions & 0 deletions app/api/util.ts
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ import type {
SiloIpPool,
SiloUtilization,
Sled,
Vpc,
VpcFirewallRule,
VpcFirewallRuleUpdate,
} from './__generated__/Api'
Expand All @@ -46,6 +47,18 @@ export const MIN_DISK_SIZE_GiB = 1
*/
export const MAX_DISK_SIZE_GiB = 1023

/**
* The `default_*` network interface attachment types resolve a VPC and VPC
* subnet both named literally 'default', so they fail with a 404 if that VPC
* doesn't exist, even when the project has other VPCs.
*
* https://github.com/oxidecomputer/omicron/blob/7a15082/nexus/src/app/sagas/instance_create.rs#L739-L773
*/
export const DEFAULT_VPC_NAME = 'default'

export const hasDefaultVpc = (vpcs: Vpc[]) =>
vpcs.some((vpc) => vpc.name === DEFAULT_VPC_NAME)

type PortRange = [number, number]

/** Parse '1234' into [1234, 1234] and '80-100' into [80, 100] */
Expand Down
45 changes: 32 additions & 13 deletions app/components/form/fields/NetworkInterfaceField.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -8,15 +8,22 @@
import { useState } from 'react'
import { useController, type Control } from 'react-hook-form'

import type { InstanceNetworkInterfaceCreate } from '@oxide/api'
import {
DEFAULT_VPC_NAME,
hasDefaultVpc,
type InstanceNetworkInterfaceCreate,
type Vpc,
} from '@oxide/api'

import { HL } from '~/components/HL'
import type { InstanceCreateInput } from '~/forms/instance-create'
import { CreateNetworkInterfaceForm } from '~/forms/network-interface-create'
import { Button } from '~/ui/lib/Button'
import { FieldLabel } from '~/ui/lib/FieldLabel'
import { Listbox } from '~/ui/lib/Listbox'
import { MiniTable } from '~/ui/lib/MiniTable'
import { Radio } from '~/ui/lib/Radio'
import { TipIcon } from '~/ui/lib/TipIcon'

const networkInterfaceTableColumns = [
{ header: 'Name', cell: (item: InstanceNetworkInterfaceCreate) => item.name },
Expand All @@ -31,13 +38,14 @@ const networkInterfaceTableColumns = [
export function NetworkInterfaceField({
control,
disabled,
hasVpcs,
vpcs,
}: {
control: Control<InstanceCreateInput>
disabled: boolean
hasVpcs: boolean
vpcs: Vpc[]
}) {
const [showForm, setShowForm] = useState(false)
const defaultVpcExists = hasDefaultVpc(vpcs)

/**
* Used to preserve previous user choices in case they accidentally
Expand Down Expand Up @@ -79,15 +87,26 @@ export function NetworkInterfaceField({
aria-labelledby="network-interface-type-label"
>
<div className="space-y-2">
<Radio
name="networkInterfaceType"
value="default"
disabled={!hasVpcs || disabled}

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This was the wrongest bit.

checked={currentMode === 'default'}
onChange={(e) => handleModeChange(e.target.value)}
>
Default
</Radio>
<span className="inline-flex items-center gap-1.5">
<Radio
name="networkInterfaceType"
value="default"
disabled={!defaultVpcExists || disabled}
checked={currentMode === 'default'}
onChange={(e) => handleModeChange(e.target.value)}
>
Default
</Radio>
{
// the no VPCs case is covered by a separate yellow banner message
// saying you can't have any network interfaces
vpcs.length > 0 && !defaultVpcExists && (
<TipIcon>
Default networking requires a VPC named <HL>{DEFAULT_VPC_NAME}</HL>
</TipIcon>
)
}
</span>
{currentMode === 'default' && (
<div className="mb-2 ml-6">
<Listbox
Expand All @@ -108,7 +127,7 @@ export function NetworkInterfaceField({
<Radio
name="networkInterfaceType"
value="create"
disabled={!hasVpcs || disabled}
disabled={vpcs.length === 0 || disabled}
checked={currentMode === 'create'}
onChange={(e) => handleModeChange(e.target.value)}
>
Expand Down
42 changes: 20 additions & 22 deletions app/forms/instance-create.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ import {
api,
diskCan,
genName,
hasDefaultVpc,
INSTANCE_MAX_CPU,
INSTANCE_MAX_RAM_GiB,
isUnicastPool,
Expand All @@ -34,6 +35,7 @@ import {
type IpVersion,
type NameOrId,
type UnicastIpPool,
type Vpc,
} from '@oxide/api'
import {
Images16Icon,
Expand Down Expand Up @@ -410,19 +412,16 @@ export default function CreateInstanceForm() {
[siloPools]
)

// Check if VPCs exist to determine default network interface type
const { data: vpcs } = usePrefetchedQuery(
q(api.vpcList, { query: { project, limit: ALL_ISH } })
)
const hasVpcs = vpcs.items.length > 0

// Determine default network interface type:
// - If VPCs exist: default to dual-stack (API default, works with both IPv4 and IPv6 subnets)
// - If no VPCs exist: default to 'none' (user must create VPC first or use custom NICs)
// - If a default VPC exists: default to dual-stack (API default, works with both IPv4 and IPv6 subnets)
// - Otherwise: default to 'none' (user must create a VPC first or use custom NICs)
// Note: Decoupled from external IP pool configuration, as NIC IP stack and external IPs are separate concerns
const defaultNetworkInterfaceType: InstanceNetworkInterfaceAttachment['type'] = hasVpcs
? 'default_dual_stack'
: 'none'
const defaultNetworkInterfaceType: InstanceNetworkInterfaceAttachment['type'] =
hasDefaultVpc(vpcs.items) ? 'default_dual_stack' : 'none'

const defaultSource =
siloImages.length > 0 ? 'siloImage' : projectImages.length > 0 ? 'projectImage' : 'disk'
Expand Down Expand Up @@ -841,7 +840,7 @@ export default function CreateInstanceForm() {
control={control}
isSubmitting={isSubmitting}
unicastPools={unicastPools}
hasVpcs={hasVpcs}
vpcs={vpcs.items}
/>
<FormDivider />
<Form.Heading id="advanced">Advanced</Form.Heading>
Expand Down Expand Up @@ -878,12 +877,12 @@ const NetworkingSection = ({
control,
isSubmitting,
unicastPools,
hasVpcs,
vpcs,
}: {
control: Control<InstanceCreateInput>
isSubmitting: boolean
unicastPools: UnicastIpPool[]
hasVpcs: boolean
vpcs: Vpc[]
}) => {
const networkInterfaces = useWatch({ control, name: 'networkInterfaces' })
const [floatingIpModalOpen, setFloatingIpModalOpen] = useState(false)
Expand Down Expand Up @@ -953,21 +952,20 @@ const NetworkingSection = ({
</>
)

const vpcMessage =
vpcs.length === 0 ? (
<>
A VPC is required to add network interfaces.{' '}
<Link to={pb.vpcsNew({ project })}>Create a VPC</Link> to enable networking.
</>
) : null

return (
<>
{!hasVpcs && (
<Message
className="mb-4"
variant="notice"
content={
<>
A VPC is required to add network interfaces.{' '}
<Link to={pb.vpcsNew({ project })}>Create a VPC</Link> to enable networking.
</>
}
/>
{vpcMessage && (
<Message className="mb-4 max-w-lg" variant="notice" content={vpcMessage} />
)}
<NetworkInterfaceField control={control} disabled={isSubmitting} hasVpcs={hasVpcs} />
<NetworkInterfaceField control={control} disabled={isSubmitting} vpcs={vpcs} />

<div className="flex flex-1 flex-col gap-4">
<h2 className="text-sans-md flex items-center">
Expand Down
8 changes: 6 additions & 2 deletions app/ui/lib/Radio.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ const fieldStyles = `
`

export const Radio = ({ children, className, ...inputProps }: RadioProps) => (
<label className="text-sans-md inline-flex items-start">
<label className="group text-sans-md inline-flex items-start">
{/* Center the 1rem (h-4) radio button with the first line of text.
1lh is the line height, so (1lh - 1rem) / 2 is the top offset
that vertically centers the indicator within that line. */}
Expand All @@ -37,7 +37,11 @@ export const Radio = ({ children, className, ...inputProps }: RadioProps) => (
<div className="bg-accent-inverse light:bg-(--theme-accent-600) pointer-events-none absolute top-1 left-1 hidden h-2 w-2 rounded-full peer-checked:block" />
</span>

{children && <span className="text-default ml-2.5">{children}</span>}
{children && (
<span className="text-default group-has-disabled:text-disabled ml-2.5">
{children}
</span>
)}
</label>
)

Expand Down
8 changes: 8 additions & 0 deletions mock-api/msw/handlers.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ import { match } from 'ts-pattern'
import { validate as isUuid, v4 as uuid } from 'uuid'

import {
DEFAULT_VPC_NAME,
diskCan,
fleetRoles,
FLEET_ID,
Expand Down Expand Up @@ -609,6 +610,13 @@ export const handlers = makeHandlers({
lookup.vpc({ ...query, vpc: vpc_name })
lookup.vpcSubnet({ ...query, vpc: vpc_name, subnet: subnet_name })
})
} else if (body.network_interfaces?.type.startsWith('default_')) {
// The default attachment types resolve a VPC and subnet both named
// literally 'default', so they 404 when that VPC doesn't exist, even if
// the project has other VPCs.
// https://github.com/oxidecomputer/omicron/blob/7a15082/nexus/src/app/sagas/instance_create.rs#L739-L773
lookup.vpc({ ...query, vpc: DEFAULT_VPC_NAME })
lookup.vpcSubnet({ ...query, vpc: DEFAULT_VPC_NAME, subnet: DEFAULT_VPC_NAME })
}

// validate floating IP attachments before we actually do anything
Expand Down
12 changes: 12 additions & 0 deletions mock-api/vpc.ts
Original file line number Diff line number Diff line change
Expand Up @@ -211,6 +211,17 @@ export const vpcSubnet2: Json<VpcSubnet> = {
custom_router_id: customRouter.id,
}

export const subnetOtherProject: Json<VpcSubnet> = {
id: 'd4f387db-e012-4424-9226-d8a10070e0f3',
name: 'other-subnet',
description: 'a subnet in other-project',
time_created,
time_modified,
vpc_id: vpc2.id,
ipv4_block: '10.1.2.0/24',
ipv6_block: 'fd9b:870a:4245:1::/64',
}

// Subnets for test silos
export const subnetKosman: Json<VpcSubnet> = {
id: 'a1b2c3d4-e5f6-4890-9234-567890abcdef',
Expand Down Expand Up @@ -248,6 +259,7 @@ export const subnetAdorno: Json<VpcSubnet> = {
export const vpcSubnets: Json<VpcSubnet[]> = [
vpcSubnet,
vpcSubnet2,
subnetOtherProject,
subnetKosman,
subnetAnscombe,
subnetAdorno,
Expand Down
56 changes: 56 additions & 0 deletions test/e2e/instance-create.e2e.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1171,6 +1171,62 @@ test('network interface options disabled when no VPCs exist', async ({ page }) =
await expect(noneRadio).toBeChecked()
})

// The default_* attachment types resolve a VPC named 'default', so they 404 if
// that VPC has been deleted. other-project has a VPC, just not one named
// 'default', so only custom interfaces work there.
test('custom network interface works without a default VPC', async ({ page }) => {
await page.goto('/projects/other-project/instances-new')
const instanceName = 'custom-nic-without-default-vpc'

const defaultRadio = page.getByRole('radio', { name: 'Default', exact: true })
const customRadio = page.getByRole('radio', { name: 'Custom', exact: true })
const noneRadio = page.getByRole('radio', { name: 'None', exact: true })

// default is out, but the project has a VPC, so custom interfaces still work
await expect(defaultRadio).toBeDisabled()
await expect(defaultRadio).not.toBeChecked()
await expect(customRadio).toBeEnabled()

const defaultRow = defaultRadio.locator('..').locator('..').locator('..')
const defaultTip = defaultRow.getByRole('button', { name: 'Tip' })
const tooltip = page.getByRole('tooltip')

await defaultTip.hover()
await expect(tooltip).toHaveText('Default networking requires a VPC named default')

await page.mouse.move(0, 0)
await expect(tooltip).toBeHidden()
await defaultTip.focus()
await expect(tooltip).toHaveText('Default networking requires a VPC named default')

await expect(noneRadio).toBeEnabled()
await expect(noneRadio).toBeChecked()

await page.getByRole('textbox', { name: 'Name', exact: true }).fill(instanceName)
await selectASiloImage(page, 'ubuntu-22-04')

await customRadio.click()
await page.getByRole('button', { name: 'Add network interface' }).click()

const modal = page.getByRole('dialog', { name: 'Add network interface' })
await modal.getByRole('textbox', { name: 'Name' }).fill('custom-primary')
await expect(modal.getByLabel('VPC', { exact: true })).toContainText('mock-vpc-2')
await modal.getByRole('button', { name: 'VPC subnet' }).click()
await page.getByRole('option', { name: 'other-subnet', exact: true }).click()
await modal.getByRole('button', { name: 'Add network interface' }).click()

await page.getByRole('button', { name: 'Create instance' }).click()
await closeToast(page)
await expect(page).toHaveURL(`/projects/other-project/instances/${instanceName}/storage`)

await page.getByRole('tab', { name: 'Networking' }).click()
await expectRowVisible(page.getByRole('table', { name: 'Network interfaces' }), {
name: 'custom-primaryprimary',
vpc: 'mock-vpc-2',
subnet: 'other-subnet',
})
})

test('floating IPs are filtered by NIC IP version', async ({ page }) => {
await page.goto('/projects/mock-project/instances-new')

Expand Down
Loading