diff --git a/app/pages/system/networking/IpPoolPage.tsx b/app/pages/system/networking/IpPoolPage.tsx index 877a5c555..bdac9b005 100644 --- a/app/pages/system/networking/IpPoolPage.tsx +++ b/app/pages/system/networking/IpPoolPage.tsx @@ -9,7 +9,7 @@ import { useQuery } from '@tanstack/react-query' import { createColumnHelper } from '@tanstack/react-table' import { useCallback, useMemo, useState } from 'react' -import { useForm } from 'react-hook-form' +import { useForm, useWatch } from 'react-hook-form' import { Outlet, useNavigate, type LoaderFunctionArgs } from 'react-router' import { @@ -27,6 +27,7 @@ import { IpGlobal16Icon, IpGlobal24Icon } from '@oxide/design-system/icons/react import { Badge } from '@oxide/design-system/ui' import { DocsPopover } from '~/components/DocsPopover' +import { CheckboxField } from '~/components/form/fields/CheckboxField' import { ComboboxField } from '~/components/form/fields/ComboboxField' import { HL } from '~/components/HL' import { IpVersionBadge } from '~/components/IpVersionBadge' @@ -35,6 +36,10 @@ import { QueryParamTabs } from '~/components/QueryParamTabs' import { makeCrumb } from '~/hooks/use-crumbs' import { getIpPoolSelector, useIpPoolSelector } from '~/hooks/use-params' import { useQuickActions } from '~/hooks/use-quick-actions' +import { + ReplacedDefaultNote, + useLinkIpPoolSiloFlow, +} from '~/pages/system/useLinkPoolSiloFlow' import { confirmAction } from '~/stores/confirm-action' import { confirmDelete } from '~/stores/confirm-delete' import { addToast } from '~/stores/toast' @@ -462,27 +467,26 @@ function LinkedSilosTable() { type LinkSiloFormValues = { silo: string | undefined + isDefault: boolean } -const defaultValues: LinkSiloFormValues = { silo: undefined } +const defaultValues: LinkSiloFormValues = { silo: undefined, isDefault: false } function LinkSiloModal({ onDismiss }: { onDismiss: () => void }) { const { pool } = useIpPoolSelector() + const { data: poolData } = usePrefetchedQuery(ipPoolView({ pool })) const { control, handleSubmit } = useForm({ defaultValues }) - const linkSilo = useApiMutation(api.systemIpPoolSiloLink, { - onSuccess() { - queryClient.invalidateEndpoint('systemIpPoolSiloList') - onDismiss() - }, - onError(err) { - addToast({ title: 'Could not link silo', content: err.message, variant: 'error' }) - }, + const { linkAndMaybePromote, isPending } = useLinkIpPoolSiloFlow({ + linkErrorTitle: 'Could not link silo', + promoteErrorTitle: 'Silo linked, but pool not set as default', }) - function onSubmit({ silo }: LinkSiloFormValues) { + async function onSubmit({ silo, isDefault }: LinkSiloFormValues) { if (!silo) return // can't happen, silo is required - linkSilo.mutate({ path: { pool }, body: { silo, isDefault: false } }) + const linked = await linkAndMaybePromote({ pool, silo, isDefault }) + if (!linked) return + onDismiss() } const linkedSilos = useQuery( @@ -490,6 +494,23 @@ function LinkSiloModal({ onDismiss }: { onDismiss: () => void }) { ) const allSilos = useQuery(q(api.siloList, { query: { limit: ALL_ISH } })) + // The pool is fixed here, so its version+type fix the default slot. To warn + // that linking as default would replace the selected silo's current default + // of that slot, fetch that silo's pools once a silo is picked. + const selectedSilo = useWatch({ control, name: 'silo' }) + const selectedSiloPools = useQuery( + getListQFn( + api.siloIpPoolList, + // silo non-null asserted because the query is disabled until one is picked + { path: { silo: selectedSilo! }, query: { limit: ALL_ISH } }, + { enabled: !!selectedSilo } + ).optionsFn() + ) + const replacedDefault = selectedSiloPools.data?.items.find( + (p) => + p.isDefault && p.ipVersion === poolData.ipVersion && p.poolType === poolData.poolType + )?.name + // in order to get the list of remaining unlinked silos, we have to get the // list of all silos and remove the already linked ones @@ -532,13 +553,22 @@ function LinkSiloModal({ onDismiss }: { onDismiss: () => void }) { required control={control} /> + + + {`Make default IP${poolData.ipVersion} ${poolData.poolType} pool for silo`} + {replacedDefault && ( + + Replaces {replacedDefault}, which stays linked + + )} + diff --git a/app/pages/system/networking/SubnetPoolPage.tsx b/app/pages/system/networking/SubnetPoolPage.tsx index 20743f9e0..eb0a9c36a 100644 --- a/app/pages/system/networking/SubnetPoolPage.tsx +++ b/app/pages/system/networking/SubnetPoolPage.tsx @@ -9,7 +9,7 @@ import { useQuery } from '@tanstack/react-query' import { createColumnHelper } from '@tanstack/react-table' import { useCallback, useMemo, useState } from 'react' -import { useForm } from 'react-hook-form' +import { useForm, useWatch } from 'react-hook-form' import { Outlet, useNavigate, type LoaderFunctionArgs } from 'react-router' import { @@ -27,6 +27,7 @@ import { Subnet16Icon, Subnet24Icon } from '@oxide/design-system/icons/react' import { Badge } from '@oxide/design-system/ui' import { DocsPopover } from '~/components/DocsPopover' +import { CheckboxField } from '~/components/form/fields/CheckboxField' import { ComboboxField } from '~/components/form/fields/ComboboxField' import { HL } from '~/components/HL' import { IpVersionBadge } from '~/components/IpVersionBadge' @@ -35,6 +36,10 @@ import { QueryParamTabs } from '~/components/QueryParamTabs' import { makeCrumb } from '~/hooks/use-crumbs' import { getSubnetPoolSelector, useSubnetPoolSelector } from '~/hooks/use-params' import { useQuickActions } from '~/hooks/use-quick-actions' +import { + ReplacedDefaultNote, + useLinkSubnetPoolSiloFlow, +} from '~/pages/system/useLinkPoolSiloFlow' import { confirmAction } from '~/stores/confirm-action' import { confirmDelete } from '~/stores/confirm-delete' import { addToast } from '~/stores/toast' @@ -448,27 +453,27 @@ function LinkedSilosTable() { type LinkSiloFormValues = { silo: string | undefined + isDefault: boolean } -const defaultValues: LinkSiloFormValues = { silo: undefined } +const defaultValues: LinkSiloFormValues = { silo: undefined, isDefault: false } function LinkSiloModal({ onDismiss }: { onDismiss: () => void }) { - const { subnetPool } = useSubnetPoolSelector() + const poolSelector = useSubnetPoolSelector() + const { subnetPool } = poolSelector + const { data: poolData } = usePrefetchedQuery(subnetPoolView(poolSelector)) const { control, handleSubmit } = useForm({ defaultValues }) - const linkSilo = useApiMutation(api.systemSubnetPoolSiloLink, { - onSuccess() { - queryClient.invalidateEndpoint('systemSubnetPoolSiloList') - onDismiss() - }, - onError(err) { - addToast({ title: 'Could not link silo', content: err.message, variant: 'error' }) - }, + const { linkAndMaybePromote, isPending } = useLinkSubnetPoolSiloFlow({ + linkErrorTitle: 'Could not link silo', + promoteErrorTitle: 'Silo linked, but pool not set as default', }) - function onSubmit({ silo }: LinkSiloFormValues) { + async function onSubmit({ silo, isDefault }: LinkSiloFormValues) { if (!silo) return - linkSilo.mutate({ path: { pool: subnetPool }, body: { silo, isDefault: false } }) + const linked = await linkAndMaybePromote({ pool: subnetPool, silo, isDefault }) + if (!linked) return + onDismiss() } const linkedSilos = useQuery( @@ -479,6 +484,22 @@ function LinkSiloModal({ onDismiss }: { onDismiss: () => void }) { ) const allSilos = useQuery(q(api.siloList, { query: { limit: ALL_ISH } })) + // The pool is fixed here, so its version fixes the default slot. To warn that + // linking as default would replace the selected silo's current default of that + // version, fetch that silo's subnet pools once a silo is picked. + const selectedSilo = useWatch({ control, name: 'silo' }) + const selectedSiloPools = useQuery( + getListQFn( + api.siloSubnetPoolList, + // silo non-null asserted because the query is disabled until one is picked + { path: { silo: selectedSilo! }, query: { limit: ALL_ISH } }, + { enabled: !!selectedSilo } + ).optionsFn() + ) + const replacedDefault = selectedSiloPools.data?.items.find( + (p) => p.isDefault && p.ipVersion === poolData.ipVersion + )?.name + const linkedSiloIds = useMemo( () => linkedSilos.data ? new Set(linkedSilos.data.items.map((s) => s.siloId)) : undefined, @@ -518,13 +539,22 @@ function LinkSiloModal({ onDismiss }: { onDismiss: () => void }) { required control={control} /> + + + {`Make default IP${poolData.ipVersion} subnet pool for silo`} + {replacedDefault && ( + + Replaces {replacedDefault}, which stays linked + + )} + diff --git a/app/pages/system/silos/SiloIpPoolsTab.tsx b/app/pages/system/silos/SiloIpPoolsTab.tsx index 7750d816c..08b96923b 100644 --- a/app/pages/system/silos/SiloIpPoolsTab.tsx +++ b/app/pages/system/silos/SiloIpPoolsTab.tsx @@ -9,7 +9,7 @@ import { useQuery } from '@tanstack/react-query' import { createColumnHelper } from '@tanstack/react-table' import { useCallback, useMemo, useState } from 'react' -import { useForm } from 'react-hook-form' +import { useForm, useWatch } from 'react-hook-form' import { type LoaderFunctionArgs } from 'react-router' import { @@ -24,11 +24,16 @@ import { import { Networking24Icon } from '@oxide/design-system/icons/react' import { Badge } from '@oxide/design-system/ui' +import { CheckboxField } from '~/components/form/fields/CheckboxField' import { ComboboxField } from '~/components/form/fields/ComboboxField' import { HL } from '~/components/HL' import { IpVersionBadge } from '~/components/IpVersionBadge' import { makeCrumb } from '~/hooks/use-crumbs' import { getSiloSelector, useSiloSelector } from '~/hooks/use-params' +import { + ReplacedDefaultNote, + useLinkIpPoolSiloFlow, +} from '~/pages/system/useLinkPoolSiloFlow' import { confirmAction } from '~/stores/confirm-action' import { addToast } from '~/stores/toast' import { LinkCell } from '~/table/cells/LinkCell' @@ -263,33 +268,43 @@ export const handle = makeCrumb('IP Pools') type LinkPoolFormValues = { pool: string | undefined + isDefault: boolean } -const defaultValues: LinkPoolFormValues = { pool: undefined } +const defaultValues: LinkPoolFormValues = { pool: undefined, isDefault: false } function LinkPoolModal({ onDismiss }: { onDismiss: () => void }) { const { silo } = useSiloSelector() const { control, handleSubmit } = useForm({ defaultValues }) - const linkPool = useApiMutation(api.systemIpPoolSiloLink, { - onSuccess() { - queryClient.invalidateEndpoint('siloIpPoolList') - queryClient.invalidateEndpoint('systemIpPoolSiloList') - onDismiss() - }, - onError(err) { - addToast({ title: 'Could not link pool', content: err.message, variant: 'error' }) - }, + const { linkAndMaybePromote, isPending } = useLinkIpPoolSiloFlow({ + linkErrorTitle: 'Could not link pool', + promoteErrorTitle: 'Pool linked, but not set as default', }) - function onSubmit({ pool }: LinkPoolFormValues) { - if (!pool) return // can't happen, silo is required - linkPool.mutate({ path: { pool }, body: { silo, isDefault: false } }) + async function onSubmit({ pool, isDefault }: LinkPoolFormValues) { + if (!pool) return // can't happen, pool is required + const linked = await linkAndMaybePromote({ pool, silo, isDefault }) + if (!linked) return + onDismiss() } const allLinkedPools = useQuery(allSiloPoolsQuery(silo).optionsFn()) const allPools = useQuery(allPoolsQuery.optionsFn()) + // The selected pool's version+type determine which default slot the checkbox + // fills, and whether linking as default would replace an existing default. + const selectedPoolName = useWatch({ control, name: 'pool' }) + const selectedPool = allPools.data?.items.find((p) => p.name === selectedPoolName) + const replacedDefault = + selectedPool && + allLinkedPools.data?.items.find( + (p) => + p.isDefault && + p.ipVersion === selectedPool.ipVersion && + p.poolType === selectedPool.poolType + )?.name + // in order to get the list of remaining unlinked pools, we have to get the // list of all pools and remove the already linked ones @@ -334,6 +349,17 @@ function LinkPoolModal({ onDismiss }: { onDismiss: () => void }) { required control={control} /> + + + {selectedPool + ? `Make default IP${selectedPool.ipVersion} ${selectedPool.poolType} pool for silo` + : 'Make default pool for silo'} + {replacedDefault && ( + + Replaces {replacedDefault}, which stays linked + + )} + @@ -341,7 +367,7 @@ function LinkPoolModal({ onDismiss }: { onDismiss: () => void }) { onDismiss={onDismiss} onAction={handleSubmit(onSubmit)} actionText="Link" - actionLoading={linkPool.isPending} + actionLoading={isPending} /> ) diff --git a/app/pages/system/silos/SiloSubnetPoolsTab.tsx b/app/pages/system/silos/SiloSubnetPoolsTab.tsx index 349d42fb4..ceac54340 100644 --- a/app/pages/system/silos/SiloSubnetPoolsTab.tsx +++ b/app/pages/system/silos/SiloSubnetPoolsTab.tsx @@ -9,7 +9,7 @@ import { useQuery } from '@tanstack/react-query' import { createColumnHelper } from '@tanstack/react-table' import { useCallback, useMemo, useState } from 'react' -import { useForm } from 'react-hook-form' +import { useForm, useWatch } from 'react-hook-form' import { type LoaderFunctionArgs } from 'react-router' import { @@ -24,11 +24,16 @@ import { import { Networking24Icon } from '@oxide/design-system/icons/react' import { Badge } from '@oxide/design-system/ui' +import { CheckboxField } from '~/components/form/fields/CheckboxField' import { ComboboxField } from '~/components/form/fields/ComboboxField' import { HL } from '~/components/HL' import { IpVersionBadge } from '~/components/IpVersionBadge' import { makeCrumb } from '~/hooks/use-crumbs' import { getSiloSelector, useSiloSelector } from '~/hooks/use-params' +import { + ReplacedDefaultNote, + useLinkSubnetPoolSiloFlow, +} from '~/pages/system/useLinkPoolSiloFlow' import { confirmAction } from '~/stores/confirm-action' import { addToast } from '~/stores/toast' import { LinkCell } from '~/table/cells/LinkCell' @@ -252,33 +257,40 @@ export const handle = makeCrumb('Subnet Pools') type LinkPoolFormValues = { pool: string | undefined + isDefault: boolean } -const defaultValues: LinkPoolFormValues = { pool: undefined } +const defaultValues: LinkPoolFormValues = { pool: undefined, isDefault: false } function LinkPoolModal({ onDismiss }: { onDismiss: () => void }) { const { silo } = useSiloSelector() const { control, handleSubmit } = useForm({ defaultValues }) - const linkPool = useApiMutation(api.systemSubnetPoolSiloLink, { - onSuccess() { - queryClient.invalidateEndpoint('siloSubnetPoolList') - queryClient.invalidateEndpoint('systemSubnetPoolSiloList') - onDismiss() - }, - onError(err) { - addToast({ title: 'Could not link pool', content: err.message, variant: 'error' }) - }, + const { linkAndMaybePromote, isPending } = useLinkSubnetPoolSiloFlow({ + linkErrorTitle: 'Could not link pool', + promoteErrorTitle: 'Pool linked, but not set as default', }) - function onSubmit({ pool }: LinkPoolFormValues) { + async function onSubmit({ pool, isDefault }: LinkPoolFormValues) { if (!pool) return - linkPool.mutate({ path: { pool }, body: { silo, isDefault: false } }) + const linked = await linkAndMaybePromote({ pool, silo, isDefault }) + if (!linked) return + onDismiss() } const allLinkedPools = useQuery(allSiloPoolsQuery(silo).optionsFn()) const allPools = useQuery(allPoolsQuery.optionsFn()) + // The selected pool's version determines which default slot the checkbox + // fills, and whether linking as default would replace an existing default. + const selectedPoolName = useWatch({ control, name: 'pool' }) + const selectedPool = allPools.data?.items.find((p) => p.name === selectedPoolName) + const replacedDefault = + selectedPool && + allLinkedPools.data?.items.find( + (p) => p.isDefault && p.ipVersion === selectedPool.ipVersion + )?.name + const linkedPoolIds = useMemo( () => allLinkedPools.data ? new Set(allLinkedPools.data.items.map((p) => p.id)) : undefined, @@ -320,6 +332,17 @@ function LinkPoolModal({ onDismiss }: { onDismiss: () => void }) { required control={control} /> + + + {selectedPool + ? `Make default IP${selectedPool.ipVersion} subnet pool for silo` + : 'Make default subnet pool for silo'} + {replacedDefault && ( + + Replaces {replacedDefault}, which stays linked + + )} + @@ -327,7 +350,7 @@ function LinkPoolModal({ onDismiss }: { onDismiss: () => void }) { onDismiss={onDismiss} onAction={handleSubmit(onSubmit)} actionText="Link" - actionLoading={linkPool.isPending} + actionLoading={isPending} /> ) diff --git a/app/pages/system/useLinkPoolSiloFlow.ts b/app/pages/system/useLinkPoolSiloFlow.ts new file mode 100644 index 000000000..3e14e5337 --- /dev/null +++ b/app/pages/system/useLinkPoolSiloFlow.ts @@ -0,0 +1,138 @@ +/* + * 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 { + api, + queryClient, + useApiMutation, + type IpPoolLinkSilo, + type SubnetPoolLinkSilo, + type SystemIpPoolSiloUpdatePathParams, + type SystemSubnetPoolSiloUpdatePathParams, +} from '@oxide/api' + +import { addToast } from '~/stores/toast' +import { classed } from '~/util/classed' + +/** + * "Replaces …" note shown under the make-default checkbox in the link modals. + * The negative bottom margin lets it sit in the modal's existing bottom padding + * instead of growing the modal: its line box is mt-1 (4px) + one line of + * text-sans-sm (line-height 1rem = 16px) = 20px, and -mb-4 (16px) pulls the + * footer back up so it shifts by only ~4px when the note appears. + */ +export const ReplacedDefaultNote = classed.span`mt-1 -mb-4 block text-sans-sm text-tertiary` + +type ToastOptions = { + linkErrorTitle: string + promoteErrorTitle: string +} + +type LinkIpPoolSiloOptions = ToastOptions + +type LinkIpPoolSiloValues = SystemIpPoolSiloUpdatePathParams & { + isDefault: IpPoolLinkSilo['isDefault'] +} + +export function useLinkIpPoolSiloFlow({ + linkErrorTitle, + promoteErrorTitle, +}: LinkIpPoolSiloOptions) { + function invalidate() { + queryClient.invalidateEndpoint('siloIpPoolList') + queryClient.invalidateEndpoint('systemIpPoolSiloList') + } + + const link = useApiMutation(api.systemIpPoolSiloLink, { + onSuccess: invalidate, + onError(err) { + addToast({ title: linkErrorTitle, content: err.message, variant: 'error' }) + }, + }) + const promote = useApiMutation(api.systemIpPoolSiloUpdate, { onSuccess: invalidate }) + + async function linkAndMaybePromote({ pool, silo, isDefault }: LinkIpPoolSiloValues) { + try { + // Link non-default first so callers can replace an existing default through + // the update endpoint, which demotes the previous default automatically. + await link.mutateAsync({ path: { pool }, body: { silo, isDefault: false } }) + } catch { + return false // onError already toasted; leave the modal open to retry + } + if (isDefault) { + try { + await promote.mutateAsync({ path: { pool, silo }, body: { isDefault: true } }) + } catch { + addToast({ + title: promoteErrorTitle, + content: 'Use the row menu to make it the default.', + variant: 'error', + }) + } + } + return true + } + + return { + isPending: link.isPending || promote.isPending, + linkAndMaybePromote, + } +} + +type LinkSubnetPoolSiloOptions = ToastOptions + +type LinkSubnetPoolSiloValues = SystemSubnetPoolSiloUpdatePathParams & { + isDefault: SubnetPoolLinkSilo['isDefault'] +} + +export function useLinkSubnetPoolSiloFlow({ + linkErrorTitle, + promoteErrorTitle, +}: LinkSubnetPoolSiloOptions) { + function invalidate() { + queryClient.invalidateEndpoint('siloSubnetPoolList') + queryClient.invalidateEndpoint('systemSubnetPoolSiloList') + } + + const link = useApiMutation(api.systemSubnetPoolSiloLink, { + onSuccess: invalidate, + onError(err) { + addToast({ title: linkErrorTitle, content: err.message, variant: 'error' }) + }, + }) + const promote = useApiMutation(api.systemSubnetPoolSiloUpdate, { + onSuccess: invalidate, + }) + + async function linkAndMaybePromote({ pool, silo, isDefault }: LinkSubnetPoolSiloValues) { + try { + // Link non-default first so callers can replace an existing default through + // the update endpoint, which demotes the previous default automatically. + await link.mutateAsync({ path: { pool }, body: { silo, isDefault: false } }) + } catch { + return false // onError already toasted; leave the modal open to retry + } + if (isDefault) { + try { + await promote.mutateAsync({ path: { pool, silo }, body: { isDefault: true } }) + } catch { + addToast({ + title: promoteErrorTitle, + content: 'Use the row menu to make it the default.', + variant: 'error', + }) + } + } + return true + } + + return { + isPending: link.isPending || promote.isPending, + linkAndMaybePromote, + } +} diff --git a/mock-api/msw/handlers.ts b/mock-api/msw/handlers.ts index 66afc5d91..999b7382f 100644 --- a/mock-api/msw/handlers.ts +++ b/mock-api/msw/handlers.ts @@ -48,6 +48,7 @@ import { utilizationForSilo, } from './db' import { + alreadyExistsErr, currentUser, errIfExists, errIfInvalidDiskSize, @@ -1275,19 +1276,40 @@ export const handlers = makeHandlers({ const pool = lookup.ipPool(path) const silo_id = lookup.silo({ silo: body.silo }).id + // Re-linking the same (pool, silo) pair hits the ip_pool_resource PK + // (ip_pool_id, resource_type, resource_id); linking as default when the silo + // already has a default for this IP version and pool type hits the partial + // unique index one_default_ip_pool_per_resource_type_version. The IP pool + // API does not inspect the constraint name, so both conflicts surface as the + // same 400 ObjectAlreadyExists. + // https://github.com/oxidecomputer/omicron/blob/13937a1/nexus/db-queries/src/db/datastore/ip_pool.rs#L1127-L1141 + // https://github.com/oxidecomputer/omicron/blob/13937a1/schema/crdb/dbinit.sql#L2402-L2412 + const alreadyLinked = db.ipPoolSilos.some( + (ips) => ips.ip_pool_id === pool.id && ips.silo_id === silo_id + ) + const defaultConflict = + body.is_default && + db.ipPoolSilos.some((ips) => { + if (ips.silo_id !== silo_id || !ips.is_default) return false + const other = db.ipPools.find((p) => p.id === ips.ip_pool_id) + return ( + other && + other.ip_version === pool.ip_version && + other.pool_type === pool.pool_type + ) + }) + if (alreadyLinked || defaultConflict) { + throw alreadyExistsErr( + `already exists: ip_pool_resource "ip_pool_id: ${pool.id}, resource_id: ${silo_id}, resource_type: Silo"` + ) + } + const assoc = { ip_pool_id: pool.id, silo_id, is_default: body.is_default, } - - const alreadyThere = db.ipPoolSilos.find( - (ips) => ips.ip_pool_id === pool.id && ips.silo_id === silo_id - ) - - // TODO: this matches current API logic but makes no sense because is_default - // could be different. Need to fix that. Should 400 or 409 on conflict. - if (!alreadyThere) db.ipPoolSilos.push(assoc) + db.ipPoolSilos.push(assoc) return json(assoc, { status: 201 }) }, @@ -2478,17 +2500,45 @@ export const handlers = makeHandlers({ const pool = lookup.subnetPool({ subnetPool: path.pool }) const silo_id = lookup.silo({ silo: body.silo }).id + // Re-linking the same (pool, silo) pair hits the subnet_pool_silo_link PK + // (subnet_pool_id, silo_id) and 400s as ObjectAlreadyExists. + // https://github.com/oxidecomputer/omicron/blob/13937a1/nexus/db-queries/src/db/datastore/external_subnet.rs#L316-L329 + // https://github.com/oxidecomputer/omicron/blob/13937a1/schema/crdb/dbinit.sql#L2807 + const alreadyLinked = db.subnetPoolSilos.some( + (sps) => sps.subnet_pool_id === pool.id && sps.silo_id === silo_id + ) + if (alreadyLinked) { + throw alreadyExistsErr( + `already exists: subnet_pool_silo_link "subnet_pool_id: ${pool.id}, silo_id: ${silo_id}"` + ) + } + + // Linking as default when the silo already has a default for this IP version + // hits the partial unique index single_default_per_silo. Unlike the IP pool + // API, this case is detected by constraint name and returns a distinct 400 + // invalid_request with a message pointing the caller at the link-then-promote + // workflow. + // https://github.com/oxidecomputer/omicron/blob/13937a1/nexus/db-queries/src/db/datastore/external_subnet.rs#L305-L315 + // https://github.com/oxidecomputer/omicron/blob/13937a1/schema/crdb/dbinit.sql#L2810-L2813 + if (body.is_default) { + const defaultConflict = db.subnetPoolSilos.some((sps) => { + if (sps.silo_id !== silo_id || !sps.is_default) return false + const other = db.subnetPools.find((p) => p.id === sps.subnet_pool_id) + return other && other.ip_version === pool.ip_version + }) + if (defaultConflict) { + throw invalidRequest( + 'Silo already has a default subnet pool for this IP version. Link the pool as non-default, then make it the default, which will demote the existing one.' + ) + } + } + const assoc: Json = { subnet_pool_id: pool.id, silo_id, is_default: body.is_default, } - - const alreadyThere = db.subnetPoolSilos.find( - (sps) => sps.subnet_pool_id === pool.id && sps.silo_id === silo_id - ) - - if (!alreadyThere) db.subnetPoolSilos.push(assoc) + db.subnetPoolSilos.push(assoc) return json(assoc, { status: 201 }) }, diff --git a/mock-api/msw/util.ts b/mock-api/msw/util.ts index daa055f44..d51267a9b 100644 --- a/mock-api/msw/util.ts +++ b/mock-api/msw/util.ts @@ -115,6 +115,12 @@ export const NotImplemented = () => { export const invalidRequest = (message: string) => json({ error_code: 'InvalidRequest', message }, { status: 400 }) +// Omicron maps a UniqueViolation through ErrorHandler::Conflict to a 400 +// ObjectAlreadyExists. +// https://github.com/oxidecomputer/omicron/blob/13937a1/nexus/db-errors/src/transaction_error.rs#L266-L270 +export const alreadyExistsErr = (message: string) => + json({ error_code: 'ObjectAlreadyExists', message }, { status: 400 }) + // 500s in Omicron come from Error::InternalError, which turns into dropshot's // `for_internal_error`, which sets error_code "Internal" and a external message // of "Internal Server Error". It also has an `internal_message` that gets @@ -142,13 +148,7 @@ export const errIfExists = >( : 'id' in match && match.id ? match.id : '' - throw json( - { - error_code: 'ObjectAlreadyExists', - message: `already exists: ${resourceLabel} "${name.toString()}"`, - }, - { status: 400 } - ) + throw alreadyExistsErr(`already exists: ${resourceLabel} "${name.toString()}"`) } } diff --git a/test/e2e/ip-pools.e2e.ts b/test/e2e/ip-pools.e2e.ts index ba4427ac0..fa7455bc4 100644 --- a/test/e2e/ip-pools.e2e.ts +++ b/test/e2e/ip-pools.e2e.ts @@ -114,14 +114,50 @@ test('IP pool link silo', async ({ page }) => { await page.getByRole('button', { name: 'Link silo' }).click() await expect(modal).toBeVisible() - // select silo in combobox and click link (thrax is not yet linked to ip-pool-1) + // select silo in combobox (thrax is not yet linked to ip-pool-1) await page.getByPlaceholder('Select a silo').fill('t') await page.getByRole('option', { name: 'thrax' }).click() + + // checkbox label reflects the pool's version and type; check it to link as default + await page + .getByRole('checkbox', { name: 'Make default IPv4 unicast pool for silo' }) + .check() + await modal.getByRole('button', { name: 'Link' }).click() - // modal closes and we see the thing in the table + // modal closes and we see the silo linked as default in the table await expect(modal).toBeHidden() - await expectRowVisible(table, { Silo: 'thrax', 'Silo default': '' }) + await expectRowVisible(table, { Silo: 'thrax', 'Silo default': 'default' }) +}) + +test('IP pool link silo as default replaces existing default', async ({ page }) => { + // ip-pool-3 is v4 unicast and linked only to myriad, so maze-war is selectable + await page.goto('/system/networking/ip-pools/ip-pool-3?tab=silos') + + const modal = page.getByRole('dialog', { name: 'Link silo' }) + await page.getByRole('button', { name: 'Link silo' }).click() + await expect(modal).toBeVisible() + + // maze-war already has a v4 unicast default (ip-pool-1) + await page.getByPlaceholder('Select a silo').fill('maze') + await page.getByRole('option', { name: 'maze-war' }).click() + + // the modal fetches the selected silo's pools to name the pool that making + // ip-pool-3 the default would demote (it stays linked) + await expect(page.getByText('Replaces ip-pool-1, which stays linked')).toBeVisible() + + // checking the box links ip-pool-3 to maze-war and promotes it in one go; seeing + // it as the silo default confirms the promote (the link itself is non-default) + await page + .getByRole('checkbox', { name: 'Make default IPv4 unicast pool for silo' }) + .check() + await modal.getByRole('button', { name: 'Link' }).click() + + await expect(modal).toBeHidden() + await expectRowVisible(page.getByRole('table'), { + Silo: 'maze-war', + 'Silo default': 'default', + }) }) test('IP pool silo make default (no existing default)', async ({ page }) => { diff --git a/test/e2e/silos.e2e.ts b/test/e2e/silos.e2e.ts index 72192517f..175e41c24 100644 --- a/test/e2e/silos.e2e.ts +++ b/test/e2e/silos.e2e.ts @@ -370,14 +370,78 @@ test('Silo IP pools link pool', async ({ page }) => { await page.getByPlaceholder('Select a pool').fill('x') await expect(page.getByText('No items match')).toBeVisible() - // select silo in combobox and click link + // before a pool is selected, the default checkbox label is generic + await expect( + page.getByRole('checkbox', { name: 'Make default pool for silo' }) + ).toBeVisible() + + // select pool in combobox await page.getByPlaceholder('Select a pool').fill('ip-pool') await page.getByRole('option', { name: 'ip-pool-3' }).click() + + // checkbox label now reflects the selected pool's version and type + const defaultCheckbox = page.getByRole('checkbox', { + name: 'Make default IPv4 unicast pool for silo', + }) + + // maze-war already has a v4 unicast default (ip-pool-1), so the label names the + // pool that making ip-pool-3 default would demote (and reassures it stays linked) + await expect(page.getByText('Replaces ip-pool-1, which stays linked')).toBeVisible() + + // checking the box and linking does it in one go: the console links ip-pool-3, + // then promotes it, which demotes ip-pool-1 to non-default + await defaultCheckbox.check() + await modal.getByRole('button', { name: 'Link' }).click() + + // modal closes; ip-pool-3 is now the v4 unicast default and ip-pool-1 is demoted + // but still linked + await expect(modal).toBeHidden() + await expectRowVisible(table, { name: 'ip-pool-3default', Version: 'v4' }) + await expectRowVisible(table, { name: 'ip-pool-1', Version: 'v4' }) +}) + +test('Silo subnet pools link pool', async ({ page }) => { + await page.goto('/system/silos/maze-war/subnet-pools') + + const table = page.getByRole('table') + await expectRowVisible(table, { name: 'default-v4-subnet-pooldefault', Version: 'v4' }) + + const modal = page.getByRole('dialog', { name: 'Link pool' }) + await expect(modal).toBeHidden() + + await page.getByRole('button', { name: 'Link pool' }).click() + await expect(modal).toBeVisible() + + // before a pool is selected, the default checkbox label is generic + await expect( + page.getByRole('checkbox', { name: 'Make default subnet pool for silo' }) + ).toBeVisible() + + // select pool in combobox + await page.getByPlaceholder('Select a pool').fill('myriad') + await page.getByRole('option', { name: 'myriad-v4-subnet-pool' }).click() + + // checkbox label now reflects the selected pool's version + const defaultCheckbox = page.getByRole('checkbox', { + name: 'Make default IPv4 subnet pool for silo', + }) + + // maze-war already has a v4 default subnet pool (default-v4-subnet-pool), so the + // label names the pool that making myriad default would demote + await expect( + page.getByText('Replaces default-v4-subnet-pool, which stays linked') + ).toBeVisible() + + // checking the box and linking does it in one go: link myriad, then promote, + // which demotes default-v4-subnet-pool + await defaultCheckbox.check() await modal.getByRole('button', { name: 'Link' }).click() - // modal closes and we see the thing in the table + // modal closes; myriad is now the v4 default and the old default is demoted but + // still linked await expect(modal).toBeHidden() - await expectRowVisible(table, { name: 'ip-pool-3', Version: 'v4' }) + await expectRowVisible(table, { name: 'myriad-v4-subnet-pooldefault', Version: 'v4' }) + await expectRowVisible(table, { name: 'default-v4-subnet-pool', Version: 'v4' }) }) // just a convenient form to test this with because it's tall diff --git a/test/e2e/subnet-pools.e2e.ts b/test/e2e/subnet-pools.e2e.ts index 92493314b..764613cd5 100644 --- a/test/e2e/subnet-pools.e2e.ts +++ b/test/e2e/subnet-pools.e2e.ts @@ -197,10 +197,48 @@ test('Subnet pool link silo', async ({ page }) => { await page.getByPlaceholder('Select a silo').fill('m') await page.getByRole('option', { name: 'myriad' }).click() + + // leave the default checkbox unchecked to link without making it the default + await expect( + dialog.getByRole('checkbox', { name: 'Make default IPv4 subnet pool for silo' }) + ).toBeVisible() + await dialog.getByRole('button', { name: 'Link' }).click() const table = page.getByRole('table') - await expectRowVisible(table, { Silo: 'myriad' }) + await expectRowVisible(table, { Silo: 'myriad', 'Silo default': '' }) +}) + +test('Subnet pool link silo as default replaces existing default', async ({ page }) => { + // myriad-v4-subnet-pool is v4 and linked only to myriad, so maze-war is selectable + await page.goto('/system/networking/subnet-pools/myriad-v4-subnet-pool?tab=silos') + + await page.getByRole('button', { name: 'Link silo' }).first().click() + const dialog = page.getByRole('dialog', { name: 'Link silo' }) + await expect(dialog).toBeVisible() + + // maze-war already has a v4 default subnet pool (default-v4-subnet-pool) + await page.getByPlaceholder('Select a silo').fill('maze') + await page.getByRole('option', { name: 'maze-war' }).click() + + // the modal fetches the selected silo's pools to name the pool that making + // myriad-v4-subnet-pool the default would demote (it stays linked) + await expect( + page.getByText('Replaces default-v4-subnet-pool, which stays linked') + ).toBeVisible() + + // checking the box links to maze-war and promotes in one go; seeing it as the + // silo default confirms the promote (the link itself is non-default) + await page + .getByRole('checkbox', { name: 'Make default IPv4 subnet pool for silo' }) + .check() + await dialog.getByRole('button', { name: 'Link' }).click() + + await expect(dialog).toBeHidden() + await expectRowVisible(page.getByRole('table'), { + Silo: 'maze-war', + 'Silo default': 'default', + }) }) test('Subnet pool silo make default (no existing default)', async ({ page }) => {