From db8e524437241348d9b298f089b40af068b77617 Mon Sep 17 00:00:00 2001 From: Benjamin Leonard Date: Fri, 17 May 2024 15:45:28 +0100 Subject: [PATCH 01/11] Switch param to route tabs: VPC firewall rules and subnets --- app/api/path-params.ts | 1 + app/forms/firewall-rules-create.tsx | 41 ++++++-- app/forms/firewall-rules-edit.tsx | 48 +++++++--- app/forms/subnet-create.tsx | 12 ++- app/forms/subnet-edit.tsx | 44 +++++++-- app/hooks/use-params.ts | 5 + app/pages/project/vpcs/VpcPage/VpcPage.tsx | 40 +++----- .../vpcs/VpcPage/tabs/VpcFirewallRulesTab.tsx | 74 ++++++++------ .../vpcs/VpcPage/tabs/VpcSubnetsTab.tsx | 96 +++++++++++++------ app/routes.tsx | 46 +++++++-- app/util/path-builder.spec.ts | 2 + app/util/path-builder.ts | 13 +++ 12 files changed, 299 insertions(+), 123 deletions(-) diff --git a/app/api/path-params.ts b/app/api/path-params.ts index dc5c30a9af..e0be7474ab 100644 --- a/app/api/path-params.ts +++ b/app/api/path-params.ts @@ -16,6 +16,7 @@ export type NetworkInterface = Merge export type Snapshot = Merge export type Vpc = Merge export type VpcSubnet = Merge +export type VpcFirewallRule = Merge export type Silo = { silo?: string } export type IdentityProvider = Merge export type SystemUpdate = { version: string } diff --git a/app/forms/firewall-rules-create.tsx b/app/forms/firewall-rules-create.tsx index 88d009bbc1..53a9e48db4 100644 --- a/app/forms/firewall-rules-create.tsx +++ b/app/forms/firewall-rules-create.tsx @@ -5,13 +5,17 @@ * * Copyright Oxide Computer Company */ +import { useMemo } from 'react' import { useController, type Control } from 'react-hook-form' +import { useNavigate, type LoaderFunctionArgs } from 'react-router-dom' import { + apiQueryClient, firewallRuleGetToPut, parsePortRange, useApiMutation, useApiQueryClient, + usePrefetchedApiQuery, type ApiError, type VpcFirewallRule, type VpcFirewallRuleHostFilter, @@ -28,7 +32,8 @@ import { NumberField } from '~/components/form/fields/NumberField' import { RadioField } from '~/components/form/fields/RadioField' import { TextField, TextFieldInner } from '~/components/form/fields/TextField' import { SideModalForm } from '~/components/form/SideModalForm' -import { useForm, useVpcSelector } from '~/hooks' +import { getVpcSelector, useForm, useVpcSelector } from '~/hooks' +import { addToast } from '~/stores/toast' import { Badge } from '~/ui/lib/Badge' import { Button } from '~/ui/lib/Button' import { FormDivider } from '~/ui/lib/Divider' @@ -36,7 +41,9 @@ import { Message } from '~/ui/lib/Message' import * as MiniTable from '~/ui/lib/MiniTable' import { TextInputHint } from '~/ui/lib/TextInput' import { KEYS } from '~/ui/util/keys' +import { sortBy } from '~/util/array' import { links } from '~/util/links' +import { pb } from '~/util/path-builder' export type FirewallRuleValues = { enabled: boolean @@ -536,25 +543,41 @@ export const CommonFields = ({ error, control }: CommonFieldsProps) => { // priority: Yup.number().integer().min(0).max(65535).required('Required'), // }) -type CreateFirewallRuleFormProps = { - onDismiss: () => void - existingRules: VpcFirewallRule[] +CreateFirewallRuleForm.loader = async ({ params }: LoaderFunctionArgs) => { + const vpcSelector = getVpcSelector(params) + + await Promise.all([ + apiQueryClient.prefetchQuery('vpcFirewallRulesView', { + query: vpcSelector, + }), + apiQueryClient.prefetchQuery('vpcView', { + path: { vpc: vpcSelector.vpc }, + query: { project: vpcSelector.project }, + }), + ]) + return null } -export function CreateFirewallRuleForm({ - onDismiss, - existingRules, -}: CreateFirewallRuleFormProps) { +export function CreateFirewallRuleForm() { const vpcSelector = useVpcSelector() const queryClient = useApiQueryClient() + const navigate = useNavigate() + const onDismiss = () => navigate(pb.vpcFirewallRules(vpcSelector)) + const updateRules = useApiMutation('vpcFirewallRulesUpdate', { onSuccess() { queryClient.invalidateQueries('vpcFirewallRulesView') - onDismiss() + addToast({ content: 'Your firewall rule has been created' }) + navigate(pb.vpcFirewallRules(vpcSelector)) }, }) + const { data } = usePrefetchedApiQuery('vpcFirewallRulesView', { + query: vpcSelector, + }) + const existingRules = useMemo(() => sortBy(data.rules, (r) => r.priority), [data]) + const form = useForm({ defaultValues }) return ( diff --git a/app/forms/firewall-rules-edit.tsx b/app/forms/firewall-rules-edit.tsx index e23428bdc2..0657268da9 100644 --- a/app/forms/firewall-rules-edit.tsx +++ b/app/forms/firewall-rules-edit.tsx @@ -5,15 +5,25 @@ * * Copyright Oxide Computer Company */ +import { useNavigate, type LoaderFunctionArgs } from 'react-router-dom' + import { + apiQueryClient, firewallRuleGetToPut, useApiMutation, useApiQueryClient, - type VpcFirewallRule, + usePrefetchedApiQuery, } from '@oxide/api' import { SideModalForm } from '~/components/form/SideModalForm' -import { useForm, useVpcSelector } from '~/hooks' +import { + getVpcSelector, + useForm, + useVpcFirewallRuleSelector, + useVpcSelector, +} from '~/hooks' +import { invariant } from '~/util/invariant' +import { pb } from '~/util/path-builder' import { CommonFields, @@ -21,20 +31,35 @@ import { type FirewallRuleValues, } from './firewall-rules-create' -type EditFirewallRuleFormProps = { - onDismiss: () => void - existingRules: VpcFirewallRule[] - originalRule: VpcFirewallRule +EditFirewallRuleForm.loader = async ({ params }: LoaderFunctionArgs) => { + const vpcSelector = getVpcSelector(params) + + await apiQueryClient.prefetchQuery('vpcFirewallRulesView', { + query: vpcSelector, + }) + + return null } -export function EditFirewallRuleForm({ - onDismiss, - existingRules, - originalRule, -}: EditFirewallRuleFormProps) { +export function EditFirewallRuleForm() { + const vpcFirewallRuleSelector = useVpcFirewallRuleSelector() const vpcSelector = useVpcSelector() const queryClient = useApiQueryClient() + const { data } = usePrefetchedApiQuery('vpcFirewallRulesView', { + query: { project: vpcFirewallRuleSelector.project, vpc: vpcFirewallRuleSelector.vpc }, + }) + + const existingRules = data.rules + const originalRule = existingRules.find( + (rule) => rule.name === vpcFirewallRuleSelector.firewallRule + ) + + invariant(originalRule, 'Firewall rule must exist') + + const navigate = useNavigate() + const onDismiss = () => navigate(pb.vpcFirewallRules(vpcSelector)) + const updateRules = useApiMutation('vpcFirewallRulesUpdate', { onSuccess() { queryClient.invalidateQueries('vpcFirewallRulesView') @@ -75,6 +100,7 @@ export function EditFirewallRuleForm({ const otherRules = existingRules .filter((r) => r.name !== originalRule.name) .map(firewallRuleGetToPut) + updateRules.mutate({ query: vpcSelector, body: { diff --git a/app/forms/subnet-create.tsx b/app/forms/subnet-create.tsx index a240c61544..0ae3de8696 100644 --- a/app/forms/subnet-create.tsx +++ b/app/forms/subnet-create.tsx @@ -5,6 +5,8 @@ * * Copyright Oxide Computer Company */ +import { useNavigate } from 'react-router-dom' + import { useApiMutation, useApiQueryClient, type VpcSubnetCreate } from '@oxide/api' import { DescriptionField } from '~/components/form/fields/DescriptionField' @@ -13,6 +15,7 @@ import { TextField } from '~/components/form/fields/TextField' import { SideModalForm } from '~/components/form/SideModalForm' import { useForm, useVpcSelector } from '~/hooks' import { FormDivider } from '~/ui/lib/Divider' +import { pb } from '~/util/path-builder' const defaultValues: VpcSubnetCreate = { name: '', @@ -20,14 +23,13 @@ const defaultValues: VpcSubnetCreate = { ipv4Block: '', } -type CreateSubnetFormProps = { - onDismiss: () => void -} - -export function CreateSubnetForm({ onDismiss }: CreateSubnetFormProps) { +export function CreateSubnetForm() { const vpcSelector = useVpcSelector() const queryClient = useApiQueryClient() + const navigate = useNavigate() + const onDismiss = () => navigate(pb.vpcSubnets(vpcSelector)) + const createSubnet = useApiMutation('vpcSubnetCreate', { onSuccess() { queryClient.invalidateQueries('vpcSubnetList') diff --git a/app/forms/subnet-edit.tsx b/app/forms/subnet-edit.tsx index ed93841a54..28a2ada297 100644 --- a/app/forms/subnet-edit.tsx +++ b/app/forms/subnet-edit.tsx @@ -5,23 +5,51 @@ * * Copyright Oxide Computer Company */ -import { useApiMutation, useApiQueryClient, type VpcSubnet } from '@oxide/api' +import { useNavigate, type LoaderFunctionArgs } from 'react-router-dom' + +import { + apiQueryClient, + useApiMutation, + useApiQueryClient, + usePrefetchedApiQuery, +} from '@oxide/api' import { DescriptionField } from '~/components/form/fields/DescriptionField' import { NameField } from '~/components/form/fields/NameField' import { SideModalForm } from '~/components/form/SideModalForm' -import { useForm, useVpcSelector } from '~/hooks' +import { + getVpcSubnetSelector, + useForm, + useVpcSelector, + useVpcSubnetSelector, +} from '~/hooks' import { pick } from '~/util/object' +import { pb } from '~/util/path-builder' + +EditSubnetForm.loader = async ({ params }: LoaderFunctionArgs) => { + const vpcSubnetSelector = getVpcSubnetSelector(params) + + await apiQueryClient.prefetchQuery('vpcSubnetView', { + query: { project: vpcSubnetSelector.project, vpc: vpcSubnetSelector.vpc }, + path: { subnet: vpcSubnetSelector.subnet }, + }) -type EditSubnetFormProps = { - onDismiss: () => void - editing: VpcSubnet + return null } -export function EditSubnetForm({ onDismiss, editing }: EditSubnetFormProps) { +export function EditSubnetForm() { const vpcSelector = useVpcSelector() + const vpcSubnetSelector = useVpcSubnetSelector() const queryClient = useApiQueryClient() + const navigate = useNavigate() + const onDismiss = () => navigate(pb.vpcSubnets(vpcSelector)) + + const { data: subnet } = usePrefetchedApiQuery('vpcSubnetView', { + query: vpcSelector, + path: { subnet: vpcSubnetSelector.subnet }, + }) + const updateSubnet = useApiMutation('vpcSubnetUpdate', { onSuccess() { queryClient.invalidateQueries('vpcSubnetList') @@ -29,7 +57,7 @@ export function EditSubnetForm({ onDismiss, editing }: EditSubnetFormProps) { }, }) - const defaultValues = pick(editing, 'name', 'description') /* satisfies VpcSubnetUpdate */ + const defaultValues = pick(subnet, 'name', 'description') /* satisfies VpcSubnetUpdate */ const form = useForm({ defaultValues }) @@ -41,7 +69,7 @@ export function EditSubnetForm({ onDismiss, editing }: EditSubnetFormProps) { onDismiss={onDismiss} onSubmit={(body) => { updateSubnet.mutate({ - path: { subnet: editing.name }, + path: { subnet: subnet.name }, query: vpcSelector, body, }) diff --git a/app/hooks/use-params.ts b/app/hooks/use-params.ts index 8cca1c39de..f4f44fa117 100644 --- a/app/hooks/use-params.ts +++ b/app/hooks/use-params.ts @@ -36,6 +36,8 @@ export const getProjectSelector = requireParams('project') export const getFloatingIpSelector = requireParams('project', 'floatingIp') export const getInstanceSelector = requireParams('project', 'instance') export const getVpcSelector = requireParams('project', 'vpc') +export const getVpcFirewallRuleSelector = requireParams('project', 'vpc', 'firewallRule') +export const getVpcSubnetSelector = requireParams('project', 'vpc', 'subnet') export const getSiloSelector = requireParams('silo') export const getSiloImageSelector = requireParams('image') export const getIdpSelector = requireParams('silo', 'provider') @@ -77,6 +79,9 @@ export const useProjectSnapshotSelector = () => useSelectedParams(getProjectSnapshotSelector) export const useInstanceSelector = () => useSelectedParams(getInstanceSelector) export const useVpcSelector = () => useSelectedParams(getVpcSelector) +export const useVpcSubnetSelector = () => useSelectedParams(getVpcSubnetSelector) +export const useVpcFirewallRuleSelector = () => + useSelectedParams(getVpcFirewallRuleSelector) export const useSiloSelector = () => useSelectedParams(getSiloSelector) export const useSiloImageSelector = () => useSelectedParams(getSiloImageSelector) export const useIdpSelector = () => useSelectedParams(getIdpSelector) diff --git a/app/pages/project/vpcs/VpcPage/VpcPage.tsx b/app/pages/project/vpcs/VpcPage/VpcPage.tsx index 662810f99e..17f539b1df 100644 --- a/app/pages/project/vpcs/VpcPage/VpcPage.tsx +++ b/app/pages/project/vpcs/VpcPage/VpcPage.tsx @@ -10,38 +10,28 @@ import type { LoaderFunctionArgs } from 'react-router-dom' import { apiQueryClient, usePrefetchedApiQuery } from '@oxide/api' import { Networking24Icon } from '@oxide/design-system/icons/react' -import { QueryParamTabs } from '~/components/QueryParamTabs' +import { RouteTabs, Tab } from '~/components/RouteTabs' import { getVpcSelector, useVpcSelector } from '~/hooks' import { EmptyCell } from '~/table/cells/EmptyCell' -import { PAGE_SIZE } from '~/table/QueryTable' import { DateTime } from '~/ui/lib/DateTime' import { PageHeader, PageTitle } from '~/ui/lib/PageHeader' import { PropertiesTable } from '~/ui/lib/PropertiesTable' -import { Tabs } from '~/ui/lib/Tabs' +import { pb } from '~/util/path-builder' import { VpcDocsPopover } from '../VpcsPage' -import { VpcFirewallRulesTab } from './tabs/VpcFirewallRulesTab' -import { VpcSubnetsTab } from './tabs/VpcSubnetsTab' VpcPage.loader = async ({ params }: LoaderFunctionArgs) => { const { project, vpc } = getVpcSelector(params) - await Promise.all([ - apiQueryClient.prefetchQuery('vpcView', { path: { vpc }, query: { project } }), - apiQueryClient.prefetchQuery('vpcFirewallRulesView', { - query: { project, vpc }, - }), - apiQueryClient.prefetchQuery('vpcSubnetList', { - query: { project, vpc, limit: PAGE_SIZE }, - }), - ]) + apiQueryClient.prefetchQuery('vpcView', { path: { vpc }, query: { project } }) + return null } export function VpcPage() { - const { project, vpc: vpcName } = useVpcSelector() + const vpcSelector = useVpcSelector() const { data: vpc } = usePrefetchedApiQuery('vpcView', { - path: { vpc: vpcName }, - query: { project }, + path: { vpc: vpcSelector.vpc }, + query: { project: vpcSelector.project }, }) return ( @@ -67,18 +57,10 @@ export function VpcPage() { - - - Firewall Rules - Subnets - - - - - - - - + + Firewall Rules + Subnets + ) } diff --git a/app/pages/project/vpcs/VpcPage/tabs/VpcFirewallRulesTab.tsx b/app/pages/project/vpcs/VpcPage/tabs/VpcFirewallRulesTab.tsx index c70a6b0e7d..312c186fd8 100644 --- a/app/pages/project/vpcs/VpcPage/tabs/VpcFirewallRulesTab.tsx +++ b/app/pages/project/vpcs/VpcPage/tabs/VpcFirewallRulesTab.tsx @@ -6,9 +6,11 @@ * Copyright Oxide Computer Company */ import { createColumnHelper, getCoreRowModel, useReactTable } from '@tanstack/react-table' -import { useMemo, useState } from 'react' +import { useMemo } from 'react' +import { Outlet, useNavigate, type LoaderFunctionArgs } from 'react-router-dom' import { + apiQueryClient, useApiMutation, useApiQueryClient, usePrefetchedApiQuery, @@ -16,21 +18,20 @@ import { } from '@oxide/api' import { ListPlusCell } from '~/components/ListPlusCell' -import { CreateFirewallRuleForm } from '~/forms/firewall-rules-create' -import { EditFirewallRuleForm } from '~/forms/firewall-rules-edit' -import { useVpcSelector } from '~/hooks' +import { getVpcSelector, useVpcSelector } from '~/hooks' import { confirmDelete } from '~/stores/confirm-delete' import { EnabledCell } from '~/table/cells/EnabledCell' -import { ButtonCell } from '~/table/cells/LinkCell' +import { LinkCell } from '~/table/cells/LinkCell' import { TypeValueCell } from '~/table/cells/TypeValueCell' import { getActionsCol } from '~/table/columns/action-col' import { Columns } from '~/table/columns/common' import { Table } from '~/table/Table' import { Badge } from '~/ui/lib/Badge' -import { CreateButton } from '~/ui/lib/CreateButton' +import { CreateLink } from '~/ui/lib/CreateButton' import { EmptyMessage } from '~/ui/lib/EmptyMessage' import { TableEmptyBox } from '~/ui/lib/Table' import { sortBy } from '~/util/array' +import { pb } from '~/util/path-builder' import { titleCase } from '~/util/str' const colHelper = createColumnHelper() @@ -94,7 +95,22 @@ const staticColumns = [ colHelper.accessor('timeCreated', Columns.timeCreated), ] -export const VpcFirewallRulesTab = () => { +VpcFirewallRulesTab.loader = async ({ params }: LoaderFunctionArgs) => { + const vpcSelector = getVpcSelector(params) + + await Promise.all([ + apiQueryClient.prefetchQuery('vpcFirewallRulesView', { + query: vpcSelector, + }), + apiQueryClient.prefetchQuery('vpcView', { + path: { vpc: vpcSelector.vpc }, + query: { project: vpcSelector.project }, + }), + ]) + return null +} + +export function VpcFirewallRulesTab() { const queryClient = useApiQueryClient() const vpcSelector = useVpcSelector() @@ -103,8 +119,7 @@ export const VpcFirewallRulesTab = () => { }) const rules = useMemo(() => sortBy(data.rules, (r) => r.priority), [data]) - const [createModalOpen, setCreateModalOpen] = useState(false) - const [editing, setEditing] = useState(null) + const navigate = useNavigate() const updateRules = useApiMutation('vpcFirewallRulesUpdate', { onSuccess() { @@ -118,14 +133,29 @@ export const VpcFirewallRulesTab = () => { colHelper.accessor('name', { header: 'Name', cell: (info) => ( - setEditing(info.row.original)}> + {info.getValue()} - + ), }), ...staticColumns, getActionsCol((rule: VpcFirewallRule) => [ - { label: 'Edit', onActivate: () => setEditing(rule) }, + { + label: 'Edit', + onActivate() { + navigate( + pb.vpcFirewallRuleEdit({ + ...vpcSelector, + firewallRule: rule.name, + }) + ) + }, + }, { label: 'Delete', onActivate: confirmDelete({ @@ -141,7 +171,7 @@ export const VpcFirewallRulesTab = () => { }, ]), ] - }, [setEditing, rules, updateRules, vpcSelector]) + }, [navigate, rules, updateRules, vpcSelector]) const table = useReactTable({ columns, data: rules, getCoreRowModel: getCoreRowModel() }) @@ -151,7 +181,7 @@ export const VpcFirewallRulesTab = () => { title="No firewall rules" body="You need to create a rule to be able to see it here" buttonText="New rule" - onClick={() => setCreateModalOpen(true)} + buttonTo={pb.vpcFirewallRulesNew(vpcSelector)} /> ) @@ -159,22 +189,10 @@ export const VpcFirewallRulesTab = () => { return ( <>
- setCreateModalOpen(true)}>New rule - {createModalOpen && ( - setCreateModalOpen(false)} - /> - )} - {editing && ( - setEditing(null)} - /> - )} + New rule
{rules.length > 0 ? : emptyState} + ) } diff --git a/app/pages/project/vpcs/VpcPage/tabs/VpcSubnetsTab.tsx b/app/pages/project/vpcs/VpcPage/tabs/VpcSubnetsTab.tsx index 6c121c3479..e25ed0ed95 100644 --- a/app/pages/project/vpcs/VpcPage/tabs/VpcSubnetsTab.tsx +++ b/app/pages/project/vpcs/VpcPage/tabs/VpcSubnetsTab.tsx @@ -6,38 +6,51 @@ * Copyright Oxide Computer Company */ import { createColumnHelper } from '@tanstack/react-table' -import { useCallback, useState } from 'react' +import { useCallback, useMemo } from 'react' +import { Outlet, useNavigate, type LoaderFunctionArgs } from 'react-router-dom' -import { useApiMutation, useApiQueryClient, type VpcSubnet } from '@oxide/api' +import { + apiQueryClient, + useApiMutation, + useApiQueryClient, + type VpcSubnet, +} from '@oxide/api' -import { CreateSubnetForm } from '~/forms/subnet-create' -import { EditSubnetForm } from '~/forms/subnet-edit' -import { useVpcSelector } from '~/hooks' +import { getVpcSelector, useVpcSelector } from '~/hooks' import { confirmDelete } from '~/stores/confirm-delete' +import { makeLinkCell } from '~/table/cells/LinkCell' import { TwoLineCell } from '~/table/cells/TwoLineCell' -import { useColsWithActions, type MenuAction } from '~/table/columns/action-col' +import { getActionsCol, type MenuAction } from '~/table/columns/action-col' import { Columns } from '~/table/columns/common' -import { useQueryTable } from '~/table/QueryTable' -import { CreateButton } from '~/ui/lib/CreateButton' +import { PAGE_SIZE, useQueryTable } from '~/table/QueryTable' +import { CreateLink } from '~/ui/lib/CreateButton' import { EmptyMessage } from '~/ui/lib/EmptyMessage' +import { pb } from '~/util/path-builder' const colHelper = createColumnHelper() -const staticCols = [ - colHelper.accessor('name', {}), - colHelper.accessor((vpc) => [vpc.ipv4Block, vpc.ipv6Block] as const, { - header: 'IP Block', - cell: (info) => , - }), - colHelper.accessor('timeCreated', Columns.timeCreated), -] - -export const VpcSubnetsTab = () => { + +VpcSubnetsTab.loader = async ({ params }: LoaderFunctionArgs) => { + const { project, vpc } = getVpcSelector(params) + + const vpcSelector = getVpcSelector(params) + + await Promise.all([ + apiQueryClient.prefetchQuery('vpcSubnetList', { + query: { project, vpc, limit: PAGE_SIZE }, + }), + apiQueryClient.prefetchQuery('vpcView', { + path: { vpc: vpcSelector.vpc }, + query: { project: vpcSelector.project }, + }), + ]) + return null +} + +export function VpcSubnetsTab() { const vpcSelector = useVpcSelector() const queryClient = useApiQueryClient() const { Table } = useQueryTable('vpcSubnetList', { query: vpcSelector }) - const [creating, setCreating] = useState(false) - const [editing, setEditing] = useState(null) const deleteSubnet = useApiMutation('vpcSubnetDelete', { onSuccess() { @@ -45,11 +58,20 @@ export const VpcSubnetsTab = () => { }, }) + const navigate = useNavigate() + const makeActions = useCallback( (subnet: VpcSubnet): MenuAction[] => [ { label: 'Edit', - onActivate: () => setEditing(subnet), + onActivate: () => { + navigate( + pb.vpcSubnetsEdit({ + ...vpcSelector, + subnet: subnet.name, + }) + ) + }, }, // TODO: only show if you have permission to do this { @@ -60,28 +82,48 @@ export const VpcSubnetsTab = () => { }), }, ], - [deleteSubnet] + [navigate, deleteSubnet, vpcSelector] ) - const columns = useColsWithActions(staticCols, makeActions) + const columns = useMemo( + () => [ + colHelper.accessor('name', { + cell: makeLinkCell((subnet) => + pb.vpcSubnetsEdit({ + ...vpcSelector, + subnet: subnet, + }) + ), + }), + colHelper.accessor((vpc) => [vpc.ipv4Block, vpc.ipv6Block] as const, { + header: 'IP Block', + cell: (info) => , + }), + colHelper.accessor('timeCreated', Columns.timeCreated), + getActionsCol(makeActions), + ], + [vpcSelector, makeActions] + ) + + // const columns = useColsWithActions(staticCols, makeActions) const emptyState = ( setCreating(true)} + buttonTo={pb.vpcSubnetsNew(vpcSelector)} /> ) return ( <>
- setCreating(true)}>New subnet - {creating && setCreating(false)} />} - {editing && setEditing(null)} />} + New subnet
+
+ ) } diff --git a/app/routes.tsx b/app/routes.tsx index 16b6239e79..9c558733a7 100644 --- a/app/routes.tsx +++ b/app/routes.tsx @@ -10,6 +10,8 @@ import { createRoutesFromElements, Navigate, Route } from 'react-router-dom' import { RouterDataErrorBoundary } from './components/ErrorBoundary' import { NotFound } from './components/ErrorPage' import { CreateDiskSideModalForm } from './forms/disk-create' +import { CreateFirewallRuleForm } from './forms/firewall-rules-create' +import { EditFirewallRuleForm } from './forms/firewall-rules-edit' import { CreateFloatingIpSideModalForm } from './forms/floating-ip-create' import { EditFloatingIpSideModalForm } from './forms/floating-ip-edit' import { CreateIdpSideModalForm } from './forms/idp/create' @@ -29,6 +31,8 @@ import { EditProjectSideModalForm } from './forms/project-edit' import { CreateSiloSideModalForm } from './forms/silo-create' import { CreateSnapshotSideModalForm } from './forms/snapshot-create' import { CreateSSHKeySideModalForm } from './forms/ssh-key-create' +import { CreateSubnetForm } from './forms/subnet-create' +import { EditSubnetForm } from './forms/subnet-edit' import { CreateVpcSideModalForm } from './forms/vpc-create' import { EditVpcSideModalForm } from './forms/vpc-edit' import type { CrumbFunc } from './hooks/use-title' @@ -58,6 +62,8 @@ import { NetworkingTab } from './pages/project/instances/instance/tabs/Networkin import { StorageTab } from './pages/project/instances/instance/tabs/StorageTab' import { InstancesPage } from './pages/project/instances/InstancesPage' import { SnapshotsPage } from './pages/project/snapshots/SnapshotsPage' +import { VpcFirewallRulesTab } from './pages/project/vpcs/VpcPage/tabs/VpcFirewallRulesTab' +import { VpcSubnetsTab } from './pages/project/vpcs/VpcPage/tabs/VpcSubnetsTab' import { VpcPage } from './pages/project/vpcs/VpcPage/VpcPage' import { VpcsPage } from './pages/project/vpcs/VpcsPage' import { ProjectsPage } from './pages/ProjectsPage' @@ -343,12 +349,40 @@ export const routes = createRoutesFromElements( - } - loader={VpcPage.loader} - handle={{ crumb: vpcCrumb }} - /> + + } /> + } loader={VpcPage.loader}> + } loader={VpcFirewallRulesTab.loader}> + + } + loader={CreateFirewallRuleForm.loader} + handle={{ crumb: 'New Firewall Rule' }} + /> + } + loader={EditFirewallRuleForm.loader} + handle={{ crumb: 'Edit Firewall Rule' }} + /> + + } loader={VpcSubnetsTab.loader}> + + } + handle={{ crumb: 'New Subnet' }} + /> + } + loader={EditSubnetForm.loader} + handle={{ crumb: 'Edit Subnet' }} + /> + + + } loader={FloatingIpsPage.loader}> diff --git a/app/util/path-builder.spec.ts b/app/util/path-builder.spec.ts index c08f072473..46bb65295c 100644 --- a/app/util/path-builder.spec.ts +++ b/app/util/path-builder.spec.ts @@ -22,6 +22,8 @@ const params = { image: 'im', snapshot: 'sn', pool: 'pl', + firewallRule: 'fr', + subnet: 'su', } test('path builder', () => { diff --git a/app/util/path-builder.ts b/app/util/path-builder.ts index bb1de149d7..dc2bfb345a 100644 --- a/app/util/path-builder.ts +++ b/app/util/path-builder.ts @@ -21,6 +21,8 @@ type Snapshot = Required type SiloImage = Required type IpPool = Required type FloatingIp = Required +type VpcFirewallRule = Required +type VpcSubnet = Required // this is used as the basis for many routes, but is itself not a route we ever // want to link directly to. so we use this to build the routes but pb.project() @@ -72,6 +74,17 @@ export const pb = { vpcs: (params: Project) => `${projectBase(params)}/vpcs`, vpc: (params: Vpc) => `${pb.vpcs(params)}/${params.vpc}`, vpcEdit: (params: Vpc) => `${pb.vpc(params)}/edit`, + + vpcFirewallRules: (params: Vpc) => `${pb.vpcs(params)}/${params.vpc}/firewall-rules`, + vpcFirewallRulesNew: (params: Vpc) => + `${pb.vpcs(params)}/${params.vpc}/firewall-rules-new`, + vpcFirewallRuleEdit: (params: VpcFirewallRule) => + `${pb.vpcs(params)}/${params.vpc}/firewall-rules/${params.firewallRule}/edit`, + vpcSubnets: (params: Vpc) => `${pb.vpcs(params)}/${params.vpc}/subnets`, + vpcSubnetsNew: (params: Vpc) => `${pb.vpcs(params)}/${params.vpc}/subnets-new`, + vpcSubnetsEdit: (params: VpcSubnet) => + `${pb.vpcs(params)}/${params.vpc}/subnets/${params.subnet}/edit`, + floatingIps: (params: Project) => `${projectBase(params)}/floating-ips`, floatingIpsNew: (params: Project) => `${projectBase(params)}/floating-ips-new`, floatingIp: (params: FloatingIp) => `${pb.floatingIps(params)}/${params.floatingIp}`, From d5ef769b8f0c3b1eeb330fbd84cac04caf3cac26 Mon Sep 17 00:00:00 2001 From: Benjamin Leonard Date: Fri, 17 May 2024 16:05:57 +0100 Subject: [PATCH 02/11] Test fixes --- test/e2e/firewall-rules.e2e.ts | 8 ++++---- test/e2e/networking.e2e.ts | 2 +- test/e2e/vpcs.e2e.ts | 6 ++++-- 3 files changed, 9 insertions(+), 7 deletions(-) diff --git a/test/e2e/firewall-rules.e2e.ts b/test/e2e/firewall-rules.e2e.ts index 7287a598e9..1ee0d1c116 100644 --- a/test/e2e/firewall-rules.e2e.ts +++ b/test/e2e/firewall-rules.e2e.ts @@ -25,7 +25,7 @@ test('can create firewall rule', async ({ page }) => { await expect(modal).toBeHidden() // open modal - await page.getByRole('button', { name: 'New rule' }).click() + await page.getByRole('link', { name: 'New rule' }).click() // modal is now open await expect(modal).toBeVisible() @@ -141,7 +141,7 @@ test('firewall rule form targets table', async ({ page }) => { await page.getByRole('tab', { name: 'Firewall Rules' }).click() // open modal - await page.getByRole('button', { name: 'New rule' }).click() + await page.getByRole('link', { name: 'New rule' }).click() const targets = page.getByRole('table', { name: 'Targets' }) const addButton = page.getByRole('button', { name: 'Add target' }) @@ -191,7 +191,7 @@ test('firewall rule form hosts table', async ({ page }) => { await page.getByRole('tab', { name: 'Firewall Rules' }).click() // open modal - await page.getByRole('button', { name: 'New rule' }).click() + await page.getByRole('link', { name: 'New rule' }).click() const hosts = page.getByRole('table', { name: 'Host filters' }) const addButton = page.getByRole('button', { name: 'Add host filter' }) @@ -254,7 +254,7 @@ test('can update firewall rule', async ({ page }) => { await expect(modal).toBeHidden() // can click name cell to edit - await page.getByRole('button', { name: 'allow-icmp' }).click() + await page.getByRole('link', { name: 'allow-icmp' }).click() // modal is now open await expect(modal).toBeVisible() diff --git a/test/e2e/networking.e2e.ts b/test/e2e/networking.e2e.ts index 9d3760ff4c..7b975dea61 100644 --- a/test/e2e/networking.e2e.ts +++ b/test/e2e/networking.e2e.ts @@ -64,7 +64,7 @@ test('Create and edit subnet', async ({ page }) => { await page.getByRole('tab', { name: 'Subnets' }).click() // Create subnet - await page.click('role=button[name="New subnet"]') + await page.click('role=link[name="New subnet"]') await expectVisible(page, [ 'role=heading[name="Create subnet"]', 'role=button[name="Create subnet"]', diff --git a/test/e2e/vpcs.e2e.ts b/test/e2e/vpcs.e2e.ts index 2fa7d57e39..b8e87fe35f 100644 --- a/test/e2e/vpcs.e2e.ts +++ b/test/e2e/vpcs.e2e.ts @@ -27,8 +27,10 @@ test('can nav to VpcPage from /', async ({ page }) => { await expect(page.getByRole('heading', { name: 'mock-vpc' })).toBeVisible() await expect(page.getByRole('tab', { name: 'Firewall rules' })).toBeVisible() await expect(page.getByRole('cell', { name: 'allow-icmp' })).toBeVisible() - await expect(page).toHaveURL('/projects/mock-project/vpcs/mock-vpc') - await expect(page).toHaveTitle('mock-vpc / VPCs / mock-project / Oxide Console') + await expect(page).toHaveURL('/projects/mock-project/vpcs/mock-vpc/firewall-rules') + await expect(page).toHaveTitle( + 'Firewall Rules / mock-vpc / VPCs / mock-project / Oxide Console' + ) // we can also click the firewall rules cell to get to the VPC detail await page.goBack() From a5ddc631d6d18f46351573530cf7e18fd2865f81 Mon Sep 17 00:00:00 2001 From: Benjamin Leonard Date: Fri, 17 May 2024 16:11:30 +0100 Subject: [PATCH 03/11] Add missing paths to test --- app/util/path-builder.spec.ts | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/app/util/path-builder.spec.ts b/app/util/path-builder.spec.ts index 46bb65295c..4a514a5187 100644 --- a/app/util/path-builder.spec.ts +++ b/app/util/path-builder.spec.ts @@ -89,6 +89,12 @@ test('path builder', () => { "systemUtilization": "/system/utilization", "vpc": "/projects/p/vpcs/v", "vpcEdit": "/projects/p/vpcs/v/edit", + "vpcFirewallRuleEdit": "/projects/p/vpcs/v/firewall-rules/fr/edit", + "vpcFirewallRules": "/projects/p/vpcs/v/firewall-rules", + "vpcFirewallRulesNew": "/projects/p/vpcs/v/firewall-rules-new", + "vpcSubnets": "/projects/p/vpcs/v/subnets", + "vpcSubnetsEdit": "/projects/p/vpcs/v/subnets/su/edit", + "vpcSubnetsNew": "/projects/p/vpcs/v/subnets-new", "vpcs": "/projects/p/vpcs", "vpcsNew": "/projects/p/vpcs-new", } From 5ed3c68714bca7a5927abc95d25f78fbab2b488a Mon Sep 17 00:00:00 2001 From: David Crespo Date: Thu, 23 May 2024 15:06:03 -0500 Subject: [PATCH 04/11] clean up vpc and instance detail in path builder --- app/forms/instance-create.tsx | 2 +- app/pages/project/instances/InstancesPage.tsx | 4 +-- app/routes.tsx | 6 +++- app/table/cells/InstanceLinkCell.tsx | 2 +- app/util/path-builder.spec.ts | 5 ++-- app/util/path-builder.ts | 29 +++++++++++-------- test/e2e/instance-networking.e2e.ts | 4 +-- 7 files changed, 30 insertions(+), 22 deletions(-) diff --git a/app/forms/instance-create.tsx b/app/forms/instance-create.tsx index 5bbffa4cb9..bfb25d21fa 100644 --- a/app/forms/instance-create.tsx +++ b/app/forms/instance-create.tsx @@ -174,7 +174,7 @@ export function CreateInstanceForm() { instance ) addToast({ content: 'Your instance has been created' }) - navigate(pb.instancePage({ project, instance: instance.name })) + navigate(pb.instance({ project, instance: instance.name })) }, }) diff --git a/app/pages/project/instances/InstancesPage.tsx b/app/pages/project/instances/InstancesPage.tsx index aae82e939d..e0e6c55887 100644 --- a/app/pages/project/instances/InstancesPage.tsx +++ b/app/pages/project/instances/InstancesPage.tsx @@ -80,7 +80,7 @@ export function InstancesPage() { }, ...(instances?.items || []).map((i) => ({ value: i.name, - onSelect: () => navigate(pb.instancePage({ project, instance: i.name })), + onSelect: () => navigate(pb.instance({ project, instance: i.name })), navGroup: 'Go to instance', })), ], @@ -97,7 +97,7 @@ export function InstancesPage() { const columns = useMemo( () => [ colHelper.accessor('name', { - cell: makeLinkCell((instance) => pb.instancePage({ project, instance })), + cell: makeLinkCell((instance) => pb.instance({ project, instance })), }), colHelper.accessor((i) => ({ ncpus: i.ncpus, memory: i.memory }), { header: 'CPU, RAM', diff --git a/app/routes.tsx b/app/routes.tsx index 9c558733a7..f3dcdf2ebb 100644 --- a/app/routes.tsx +++ b/app/routes.tsx @@ -350,8 +350,12 @@ export const routes = createRoutesFromElements( - } /> } loader={VpcPage.loader}> + } + loader={VpcFirewallRulesTab.loader} + /> } loader={VpcFirewallRulesTab.loader}> { if (!instance) return return ( - + {instance.name} ) diff --git a/app/util/path-builder.spec.ts b/app/util/path-builder.spec.ts index 4a514a5187..7244193d8a 100644 --- a/app/util/path-builder.spec.ts +++ b/app/util/path-builder.spec.ts @@ -38,10 +38,9 @@ test('path builder', () => { "floatingIpEdit": "/projects/p/floating-ips/f/edit", "floatingIps": "/projects/p/floating-ips", "floatingIpsNew": "/projects/p/floating-ips-new", - "instance": "/projects/p/instances/i", + "instance": "/projects/p/instances/i/storage", "instanceConnect": "/projects/p/instances/i/connect", "instanceMetrics": "/projects/p/instances/i/metrics", - "instancePage": "/projects/p/instances/i/storage", "instanceStorage": "/projects/p/instances/i/storage", "instances": "/projects/p/instances", "instancesNew": "/projects/p/instances-new", @@ -87,7 +86,7 @@ test('path builder', () => { "systemHealth": "/system/health", "systemIssues": "/system/issues", "systemUtilization": "/system/utilization", - "vpc": "/projects/p/vpcs/v", + "vpc": "/projects/p/vpcs/v/firewall-rules", "vpcEdit": "/projects/p/vpcs/v/edit", "vpcFirewallRuleEdit": "/projects/p/vpcs/v/firewall-rules/fr/edit", "vpcFirewallRules": "/projects/p/vpcs/v/firewall-rules", diff --git a/app/util/path-builder.ts b/app/util/path-builder.ts index dc2bfb345a..853e9d7698 100644 --- a/app/util/path-builder.ts +++ b/app/util/path-builder.ts @@ -24,10 +24,13 @@ type FloatingIp = Required type VpcFirewallRule = Required type VpcSubnet = Required -// this is used as the basis for many routes, but is itself not a route we ever -// want to link directly to. so we use this to build the routes but pb.project() -// is different (includes /instances) +// these are used as the basis for many routes but are not themselves routes we +// ever want to link to. so we use this to build the routes but pb.project() is +// different (includes /instances) const projectBase = ({ project }: Project) => `${pb.projects()}/${project}` +const instanceBase = ({ project, instance }: Instance) => + `${pb.instances({ project })}/${instance}` +const vpcBase = ({ project, vpc }: Vpc) => `${pb.vpcs({ project })}/${vpc}` export const pb = { projects: () => `/projects`, @@ -43,7 +46,6 @@ export const pb = { instances: (params: Project) => `${projectBase(params)}/instances`, instancesNew: (params: Project) => `${projectBase(params)}/instances-new`, - instance: (params: Instance) => `${pb.instances(params)}/${params.instance}`, /** * This route exists as a direct link to the default tab of the instance page. Unfortunately @@ -52,15 +54,15 @@ export const pb = { * * @see https://github.com/oxidecomputer/console/pull/1267#discussion_r1016766205 */ - instancePage: (params: Instance) => pb.instanceStorage(params), + instance: (params: Instance) => pb.instanceStorage(params), - instanceMetrics: (params: Instance) => `${pb.instance(params)}/metrics`, - instanceStorage: (params: Instance) => `${pb.instance(params)}/storage`, - instanceConnect: (params: Instance) => `${pb.instance(params)}/connect`, + instanceMetrics: (params: Instance) => `${instanceBase(params)}/metrics`, + instanceStorage: (params: Instance) => `${instanceBase(params)}/storage`, + instanceConnect: (params: Instance) => `${instanceBase(params)}/connect`, - nics: (params: Instance) => `${pb.instance(params)}/network-interfaces`, + nics: (params: Instance) => `${instanceBase(params)}/network-interfaces`, - serialConsole: (params: Instance) => `${pb.instance(params)}/serial-console`, + serialConsole: (params: Instance) => `${instanceBase(params)}/serial-console`, disksNew: (params: Project) => `${projectBase(params)}/disks-new`, disks: (params: Project) => `${projectBase(params)}/disks`, @@ -72,8 +74,11 @@ export const pb = { vpcsNew: (params: Project) => `${projectBase(params)}/vpcs-new`, vpcs: (params: Project) => `${projectBase(params)}/vpcs`, - vpc: (params: Vpc) => `${pb.vpcs(params)}/${params.vpc}`, - vpcEdit: (params: Vpc) => `${pb.vpc(params)}/edit`, + + // same deal as instance detail: go straight to first tab + vpc: (params: Vpc) => pb.vpcFirewallRules(params), + + vpcEdit: (params: Vpc) => `${vpcBase(params)}/edit`, vpcFirewallRules: (params: Vpc) => `${pb.vpcs(params)}/${params.vpc}/firewall-rules`, vpcFirewallRulesNew: (params: Vpc) => diff --git a/test/e2e/instance-networking.e2e.ts b/test/e2e/instance-networking.e2e.ts index 11e9695ef0..a18c692a57 100644 --- a/test/e2e/instance-networking.e2e.ts +++ b/test/e2e/instance-networking.e2e.ts @@ -30,9 +30,9 @@ test('Instance networking tab — NIC table', async ({ page }) => { await expectRowVisible(nicTable, { name: 'my-nicprimary' }) // check VPC link in table points to the right page - await expect(page.locator('role=cell >> role=link[name="mock-vpc"]')).toHaveAttribute( + await expect(nicTable.getByRole('link', { name: 'mock-vpc' })).toHaveAttribute( 'href', - '/projects/mock-project/vpcs/mock-vpc' + '/projects/mock-project/vpcs/mock-vpc/firewall-rules' ) const addNicButton = page.getByRole('button', { name: 'Add network interface' }) From 373fedaf6f551f1485b527f1371cd164e27b2655 Mon Sep 17 00:00:00 2001 From: David Crespo Date: Thu, 23 May 2024 15:30:17 -0500 Subject: [PATCH 05/11] fix warning on firewall rules leaf route without element --- app/routes.tsx | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/app/routes.tsx b/app/routes.tsx index f3dcdf2ebb..33179cf425 100644 --- a/app/routes.tsx +++ b/app/routes.tsx @@ -357,7 +357,11 @@ export const routes = createRoutesFromElements( loader={VpcFirewallRulesTab.loader} /> } loader={VpcFirewallRulesTab.loader}> - + } From 2ddf55dd48d53ce2b4f7ca9c1f3cb7d29d8bc0f2 Mon Sep 17 00:00:00 2001 From: David Crespo Date: Fri, 24 May 2024 15:41:05 -0500 Subject: [PATCH 06/11] clean up path params and path builder stuff a bit --- app/api/path-params.ts | 2 +- app/forms/firewall-rules-edit.tsx | 18 +++++------------- app/hooks/use-params.ts | 5 ++--- .../vpcs/VpcPage/tabs/VpcFirewallRulesTab.tsx | 4 ++-- app/routes.tsx | 2 +- app/util/path-builder.spec.ts | 2 +- app/util/path-builder.ts | 18 ++++++++---------- 7 files changed, 20 insertions(+), 31 deletions(-) diff --git a/app/api/path-params.ts b/app/api/path-params.ts index e0be7474ab..a14c1c38c2 100644 --- a/app/api/path-params.ts +++ b/app/api/path-params.ts @@ -16,7 +16,7 @@ export type NetworkInterface = Merge export type Snapshot = Merge export type Vpc = Merge export type VpcSubnet = Merge -export type VpcFirewallRule = Merge +export type FirewallRule = Merge export type Silo = { silo?: string } export type IdentityProvider = Merge export type SystemUpdate = { version: string } diff --git a/app/forms/firewall-rules-edit.tsx b/app/forms/firewall-rules-edit.tsx index 0657268da9..6bd64ccd95 100644 --- a/app/forms/firewall-rules-edit.tsx +++ b/app/forms/firewall-rules-edit.tsx @@ -16,12 +16,7 @@ import { } from '@oxide/api' import { SideModalForm } from '~/components/form/SideModalForm' -import { - getVpcSelector, - useForm, - useVpcFirewallRuleSelector, - useVpcSelector, -} from '~/hooks' +import { getVpcSelector, useFirewallRuleSelector, useForm, useVpcSelector } from '~/hooks' import { invariant } from '~/util/invariant' import { pb } from '~/util/path-builder' @@ -42,18 +37,15 @@ EditFirewallRuleForm.loader = async ({ params }: LoaderFunctionArgs) => { } export function EditFirewallRuleForm() { - const vpcFirewallRuleSelector = useVpcFirewallRuleSelector() + const { vpc, project, rule } = useFirewallRuleSelector() const vpcSelector = useVpcSelector() const queryClient = useApiQueryClient() const { data } = usePrefetchedApiQuery('vpcFirewallRulesView', { - query: { project: vpcFirewallRuleSelector.project, vpc: vpcFirewallRuleSelector.vpc }, + query: { project, vpc }, }) - const existingRules = data.rules - const originalRule = existingRules.find( - (rule) => rule.name === vpcFirewallRuleSelector.firewallRule - ) + const originalRule = data.rules.find((r) => r.name === rule) invariant(originalRule, 'Firewall rule must exist') @@ -97,7 +89,7 @@ export function EditFirewallRuleForm() { onSubmit={(values) => { // note different filter logic from create: filter out the rule with the // *original* name because we need to overwrite that rule - const otherRules = existingRules + const otherRules = data.rules .filter((r) => r.name !== originalRule.name) .map(firewallRuleGetToPut) diff --git a/app/hooks/use-params.ts b/app/hooks/use-params.ts index f4f44fa117..35932cce8d 100644 --- a/app/hooks/use-params.ts +++ b/app/hooks/use-params.ts @@ -36,7 +36,7 @@ export const getProjectSelector = requireParams('project') export const getFloatingIpSelector = requireParams('project', 'floatingIp') export const getInstanceSelector = requireParams('project', 'instance') export const getVpcSelector = requireParams('project', 'vpc') -export const getVpcFirewallRuleSelector = requireParams('project', 'vpc', 'firewallRule') +export const getFirewallRuleSelector = requireParams('project', 'vpc', 'rule') export const getVpcSubnetSelector = requireParams('project', 'vpc', 'subnet') export const getSiloSelector = requireParams('silo') export const getSiloImageSelector = requireParams('image') @@ -80,8 +80,7 @@ export const useProjectSnapshotSelector = () => export const useInstanceSelector = () => useSelectedParams(getInstanceSelector) export const useVpcSelector = () => useSelectedParams(getVpcSelector) export const useVpcSubnetSelector = () => useSelectedParams(getVpcSubnetSelector) -export const useVpcFirewallRuleSelector = () => - useSelectedParams(getVpcFirewallRuleSelector) +export const useFirewallRuleSelector = () => useSelectedParams(getFirewallRuleSelector) export const useSiloSelector = () => useSelectedParams(getSiloSelector) export const useSiloImageSelector = () => useSelectedParams(getSiloImageSelector) export const useIdpSelector = () => useSelectedParams(getIdpSelector) diff --git a/app/pages/project/vpcs/VpcPage/tabs/VpcFirewallRulesTab.tsx b/app/pages/project/vpcs/VpcPage/tabs/VpcFirewallRulesTab.tsx index 312c186fd8..6c155a3b78 100644 --- a/app/pages/project/vpcs/VpcPage/tabs/VpcFirewallRulesTab.tsx +++ b/app/pages/project/vpcs/VpcPage/tabs/VpcFirewallRulesTab.tsx @@ -136,7 +136,7 @@ export function VpcFirewallRulesTab() { {info.getValue()} @@ -151,7 +151,7 @@ export function VpcFirewallRulesTab() { navigate( pb.vpcFirewallRuleEdit({ ...vpcSelector, - firewallRule: rule.name, + rule: rule.name, }) ) }, diff --git a/app/routes.tsx b/app/routes.tsx index 33179cf425..b958fee844 100644 --- a/app/routes.tsx +++ b/app/routes.tsx @@ -369,7 +369,7 @@ export const routes = createRoutesFromElements( handle={{ crumb: 'New Firewall Rule' }} /> } loader={EditFirewallRuleForm.loader} handle={{ crumb: 'Edit Firewall Rule' }} diff --git a/app/util/path-builder.spec.ts b/app/util/path-builder.spec.ts index 7244193d8a..627aa0187e 100644 --- a/app/util/path-builder.spec.ts +++ b/app/util/path-builder.spec.ts @@ -22,7 +22,7 @@ const params = { image: 'im', snapshot: 'sn', pool: 'pl', - firewallRule: 'fr', + rule: 'fr', subnet: 'su', } diff --git a/app/util/path-builder.ts b/app/util/path-builder.ts index 853e9d7698..875b6ae4ef 100644 --- a/app/util/path-builder.ts +++ b/app/util/path-builder.ts @@ -21,7 +21,7 @@ type Snapshot = Required type SiloImage = Required type IpPool = Required type FloatingIp = Required -type VpcFirewallRule = Required +type FirewallRule = Required type VpcSubnet = Required // these are used as the basis for many routes but are not themselves routes we @@ -80,15 +80,13 @@ export const pb = { vpcEdit: (params: Vpc) => `${vpcBase(params)}/edit`, - vpcFirewallRules: (params: Vpc) => `${pb.vpcs(params)}/${params.vpc}/firewall-rules`, - vpcFirewallRulesNew: (params: Vpc) => - `${pb.vpcs(params)}/${params.vpc}/firewall-rules-new`, - vpcFirewallRuleEdit: (params: VpcFirewallRule) => - `${pb.vpcs(params)}/${params.vpc}/firewall-rules/${params.firewallRule}/edit`, - vpcSubnets: (params: Vpc) => `${pb.vpcs(params)}/${params.vpc}/subnets`, - vpcSubnetsNew: (params: Vpc) => `${pb.vpcs(params)}/${params.vpc}/subnets-new`, - vpcSubnetsEdit: (params: VpcSubnet) => - `${pb.vpcs(params)}/${params.vpc}/subnets/${params.subnet}/edit`, + vpcFirewallRules: (params: Vpc) => `${vpcBase(params)}/firewall-rules`, + vpcFirewallRulesNew: (params: Vpc) => `${vpcBase(params)}/firewall-rules-new`, + vpcFirewallRuleEdit: (params: FirewallRule) => + `${pb.vpcFirewallRules(params)}/${params.rule}/edit`, + vpcSubnets: (params: Vpc) => `${vpcBase(params)}/subnets`, + vpcSubnetsNew: (params: Vpc) => `${vpcBase(params)}/subnets-new`, + vpcSubnetsEdit: (params: VpcSubnet) => `${pb.vpcSubnets(params)}/${params.subnet}/edit`, floatingIps: (params: Project) => `${projectBase(params)}/floating-ips`, floatingIpsNew: (params: Project) => `${projectBase(params)}/floating-ips-new`, From 67769ed95a661ada4ff4d5a7da5b6bf48eb7ada4 Mon Sep 17 00:00:00 2001 From: David Crespo Date: Fri, 7 Jun 2024 17:04:54 -0500 Subject: [PATCH 07/11] tweaks --- app/forms/subnet-edit.tsx | 26 +++++++------------ .../vpcs/VpcPage/tabs/VpcFirewallRulesTab.tsx | 26 ++++--------------- 2 files changed, 14 insertions(+), 38 deletions(-) diff --git a/app/forms/subnet-edit.tsx b/app/forms/subnet-edit.tsx index 48ca501d4d..734f0197b4 100644 --- a/app/forms/subnet-edit.tsx +++ b/app/forms/subnet-edit.tsx @@ -19,36 +19,28 @@ import { import { DescriptionField } from '~/components/form/fields/DescriptionField' import { NameField } from '~/components/form/fields/NameField' import { SideModalForm } from '~/components/form/SideModalForm' -import { - getVpcSubnetSelector, - useForm, - useVpcSelector, - useVpcSubnetSelector, -} from '~/hooks' +import { getVpcSubnetSelector, useForm, useVpcSubnetSelector } from '~/hooks' import { pb } from '~/util/path-builder' EditSubnetForm.loader = async ({ params }: LoaderFunctionArgs) => { - const vpcSubnetSelector = getVpcSubnetSelector(params) - + const { project, vpc, subnet } = getVpcSubnetSelector(params) await apiQueryClient.prefetchQuery('vpcSubnetView', { - query: { project: vpcSubnetSelector.project, vpc: vpcSubnetSelector.vpc }, - path: { subnet: vpcSubnetSelector.subnet }, + query: { project, vpc }, + path: { subnet }, }) - return null } export function EditSubnetForm() { - const vpcSelector = useVpcSelector() - const vpcSubnetSelector = useVpcSubnetSelector() + const { project, vpc, subnet: subnetName } = useVpcSubnetSelector() const queryClient = useApiQueryClient() const navigate = useNavigate() - const onDismiss = () => navigate(pb.vpcSubnets(vpcSelector)) + const onDismiss = () => navigate(pb.vpcSubnets({ project, vpc })) const { data: subnet } = usePrefetchedApiQuery('vpcSubnetView', { - query: vpcSelector, - path: { subnet: vpcSubnetSelector.subnet }, + query: { project, vpc }, + path: { subnet: subnetName }, }) const updateSubnet = useApiMutation('vpcSubnetUpdate', { @@ -71,7 +63,7 @@ export function EditSubnetForm() { onSubmit={(body) => { updateSubnet.mutate({ path: { subnet: subnet.name }, - query: vpcSelector, + query: { project, vpc }, body, }) }} diff --git a/app/pages/project/vpcs/VpcPage/tabs/VpcFirewallRulesTab.tsx b/app/pages/project/vpcs/VpcPage/tabs/VpcFirewallRulesTab.tsx index a0b137bc04..2fafe294c9 100644 --- a/app/pages/project/vpcs/VpcPage/tabs/VpcFirewallRulesTab.tsx +++ b/app/pages/project/vpcs/VpcPage/tabs/VpcFirewallRulesTab.tsx @@ -98,16 +98,10 @@ const staticColumns = [ ] VpcFirewallRulesTab.loader = async ({ params }: LoaderFunctionArgs) => { - const vpcSelector = getVpcSelector(params) - + const { project, vpc } = getVpcSelector(params) await Promise.all([ - apiQueryClient.prefetchQuery('vpcFirewallRulesView', { - query: vpcSelector, - }), - apiQueryClient.prefetchQuery('vpcView', { - path: { vpc: vpcSelector.vpc }, - query: { project: vpcSelector.project }, - }), + apiQueryClient.prefetchQuery('vpcFirewallRulesView', { query: { project, vpc } }), + apiQueryClient.prefetchQuery('vpcView', { path: { vpc }, query: { project } }), ]) return null } @@ -135,12 +129,7 @@ export function VpcFirewallRulesTab() { colHelper.accessor('name', { header: 'Name', cell: (info) => ( - + {info.getValue()} ), @@ -150,12 +139,7 @@ export function VpcFirewallRulesTab() { { label: 'Edit', onActivate() { - navigate( - pb.vpcFirewallRuleEdit({ - ...vpcSelector, - rule: rule.name, - }) - ) + navigate(pb.vpcFirewallRuleEdit({ ...vpcSelector, rule: rule.name })) }, }, { From ead8cfaf02a14111e68e79c3f9a13da1211a405b Mon Sep 17 00:00:00 2001 From: David Crespo Date: Wed, 26 Jun 2024 17:03:51 -0500 Subject: [PATCH 08/11] cut some lines --- .../vpcs/VpcPage/tabs/VpcSubnetsTab.tsx | 25 +++---------------- 1 file changed, 4 insertions(+), 21 deletions(-) diff --git a/app/pages/project/vpcs/VpcPage/tabs/VpcSubnetsTab.tsx b/app/pages/project/vpcs/VpcPage/tabs/VpcSubnetsTab.tsx index e25ed0ed95..96fd56b08c 100644 --- a/app/pages/project/vpcs/VpcPage/tabs/VpcSubnetsTab.tsx +++ b/app/pages/project/vpcs/VpcPage/tabs/VpcSubnetsTab.tsx @@ -31,17 +31,11 @@ const colHelper = createColumnHelper() VpcSubnetsTab.loader = async ({ params }: LoaderFunctionArgs) => { const { project, vpc } = getVpcSelector(params) - - const vpcSelector = getVpcSelector(params) - await Promise.all([ apiQueryClient.prefetchQuery('vpcSubnetList', { query: { project, vpc, limit: PAGE_SIZE }, }), - apiQueryClient.prefetchQuery('vpcView', { - path: { vpc: vpcSelector.vpc }, - query: { project: vpcSelector.project }, - }), + apiQueryClient.prefetchQuery('vpcView', { path: { vpc }, query: { project } }), ]) return null } @@ -64,14 +58,8 @@ export function VpcSubnetsTab() { (subnet: VpcSubnet): MenuAction[] => [ { label: 'Edit', - onActivate: () => { - navigate( - pb.vpcSubnetsEdit({ - ...vpcSelector, - subnet: subnet.name, - }) - ) - }, + onActivate: () => + navigate(pb.vpcSubnetsEdit({ ...vpcSelector, subnet: subnet.name })), }, // TODO: only show if you have permission to do this { @@ -88,12 +76,7 @@ export function VpcSubnetsTab() { const columns = useMemo( () => [ colHelper.accessor('name', { - cell: makeLinkCell((subnet) => - pb.vpcSubnetsEdit({ - ...vpcSelector, - subnet: subnet, - }) - ), + cell: makeLinkCell((subnet) => pb.vpcSubnetsEdit({ ...vpcSelector, subnet })), }), colHelper.accessor((vpc) => [vpc.ipv4Block, vpc.ipv6Block] as const, { header: 'IP Block', From b3bb8873d3bf98795e837846b1e0e80cc01b355a Mon Sep 17 00:00:00 2001 From: David Crespo Date: Wed, 26 Jun 2024 17:07:50 -0500 Subject: [PATCH 09/11] don't need to prefetch vpcView on the tabs, parent route covers it --- .../project/vpcs/VpcPage/tabs/VpcFirewallRulesTab.tsx | 5 +---- app/pages/project/vpcs/VpcPage/tabs/VpcSubnetsTab.tsx | 9 +++------ 2 files changed, 4 insertions(+), 10 deletions(-) diff --git a/app/pages/project/vpcs/VpcPage/tabs/VpcFirewallRulesTab.tsx b/app/pages/project/vpcs/VpcPage/tabs/VpcFirewallRulesTab.tsx index 2fafe294c9..950b9f4470 100644 --- a/app/pages/project/vpcs/VpcPage/tabs/VpcFirewallRulesTab.tsx +++ b/app/pages/project/vpcs/VpcPage/tabs/VpcFirewallRulesTab.tsx @@ -99,10 +99,7 @@ const staticColumns = [ VpcFirewallRulesTab.loader = async ({ params }: LoaderFunctionArgs) => { const { project, vpc } = getVpcSelector(params) - await Promise.all([ - apiQueryClient.prefetchQuery('vpcFirewallRulesView', { query: { project, vpc } }), - apiQueryClient.prefetchQuery('vpcView', { path: { vpc }, query: { project } }), - ]) + await apiQueryClient.prefetchQuery('vpcFirewallRulesView', { query: { project, vpc } }) return null } diff --git a/app/pages/project/vpcs/VpcPage/tabs/VpcSubnetsTab.tsx b/app/pages/project/vpcs/VpcPage/tabs/VpcSubnetsTab.tsx index 96fd56b08c..57ff95a407 100644 --- a/app/pages/project/vpcs/VpcPage/tabs/VpcSubnetsTab.tsx +++ b/app/pages/project/vpcs/VpcPage/tabs/VpcSubnetsTab.tsx @@ -31,12 +31,9 @@ const colHelper = createColumnHelper() VpcSubnetsTab.loader = async ({ params }: LoaderFunctionArgs) => { const { project, vpc } = getVpcSelector(params) - await Promise.all([ - apiQueryClient.prefetchQuery('vpcSubnetList', { - query: { project, vpc, limit: PAGE_SIZE }, - }), - apiQueryClient.prefetchQuery('vpcView', { path: { vpc }, query: { project } }), - ]) + await apiQueryClient.prefetchQuery('vpcSubnetList', { + query: { project, vpc, limit: PAGE_SIZE }, + }) return null } From 7617e2d1bff56d7e7a9c404944a8005de47a4171 Mon Sep 17 00:00:00 2001 From: David Crespo Date: Wed, 26 Jun 2024 17:21:47 -0500 Subject: [PATCH 10/11] 404 on edit non-existent rule --- app/forms/firewall-rules-edit.tsx | 18 ++++++++++++++---- test/e2e/firewall-rules.e2e.ts | 15 +++++++++++++++ 2 files changed, 29 insertions(+), 4 deletions(-) diff --git a/app/forms/firewall-rules-edit.tsx b/app/forms/firewall-rules-edit.tsx index 6bd64ccd95..2c1086d641 100644 --- a/app/forms/firewall-rules-edit.tsx +++ b/app/forms/firewall-rules-edit.tsx @@ -15,8 +15,14 @@ import { usePrefetchedApiQuery, } from '@oxide/api' +import { trigger404 } from '~/components/ErrorBoundary' import { SideModalForm } from '~/components/form/SideModalForm' -import { getVpcSelector, useFirewallRuleSelector, useForm, useVpcSelector } from '~/hooks' +import { + getFirewallRuleSelector, + useFirewallRuleSelector, + useForm, + useVpcSelector, +} from '~/hooks' import { invariant } from '~/util/invariant' import { pb } from '~/util/path-builder' @@ -27,12 +33,15 @@ import { } from './firewall-rules-create' EditFirewallRuleForm.loader = async ({ params }: LoaderFunctionArgs) => { - const vpcSelector = getVpcSelector(params) + const { project, vpc, rule } = getFirewallRuleSelector(params) - await apiQueryClient.prefetchQuery('vpcFirewallRulesView', { - query: vpcSelector, + const data = await apiQueryClient.fetchQuery('vpcFirewallRulesView', { + query: { project, vpc }, }) + const originalRule = data.rules.find((r) => r.name === rule) + if (!originalRule) throw trigger404 + return null } @@ -47,6 +56,7 @@ export function EditFirewallRuleForm() { const originalRule = data.rules.find((r) => r.name === rule) + // we shouldn't hit this because of the trigger404 in the loader invariant(originalRule, 'Firewall rule must exist') const navigate = useNavigate() diff --git a/test/e2e/firewall-rules.e2e.ts b/test/e2e/firewall-rules.e2e.ts index 1ee0d1c116..0467c81ef6 100644 --- a/test/e2e/firewall-rules.e2e.ts +++ b/test/e2e/firewall-rules.e2e.ts @@ -309,3 +309,18 @@ test('can update firewall rule', async ({ page }) => { await expect(page.locator(`text="${name}"`)).toBeVisible() } }) + +const rulePath = '/projects/mock-project/vpcs/mock-vpc/firewall-rules/allow-icmp/edit' + +test('can edit rule directly by URL', async ({ page }) => { + await page.goto(rulePath) + await expect(page.getByRole('dialog', { name: 'Edit rule' })).toBeVisible() + await expect(page.getByRole('textbox', { name: 'Name', exact: true })).toHaveValue( + 'allow-icmp' + ) +}) + +test('404s on edit non-existent rule', async ({ page }) => { + await page.goto(rulePath.replace('icmp', 'boop')) + await expect(page.getByText('Page not found')).toBeVisible() +}) From 1ce02618b44d48cc807fe0f445105b222d5ffcae Mon Sep 17 00:00:00 2001 From: David Crespo Date: Wed, 26 Jun 2024 17:24:21 -0500 Subject: [PATCH 11/11] don't need to prefetch vpcView on firewall rule create form either --- app/forms/firewall-rules-create.tsx | 19 +++---------------- 1 file changed, 3 insertions(+), 16 deletions(-) diff --git a/app/forms/firewall-rules-create.tsx b/app/forms/firewall-rules-create.tsx index 17699c0ee8..2ec6340e5f 100644 --- a/app/forms/firewall-rules-create.tsx +++ b/app/forms/firewall-rules-create.tsx @@ -559,23 +559,10 @@ export const CommonFields = ({ error, control }: CommonFieldsProps) => { ) } -// TODO: validate priority again -// export const validationSchema = Yup.object({ -// priority: Yup.number().integer().min(0).max(65535).required('Required'), -// }) - CreateFirewallRuleForm.loader = async ({ params }: LoaderFunctionArgs) => { - const vpcSelector = getVpcSelector(params) - - await Promise.all([ - apiQueryClient.prefetchQuery('vpcFirewallRulesView', { - query: vpcSelector, - }), - apiQueryClient.prefetchQuery('vpcView', { - path: { vpc: vpcSelector.vpc }, - query: { project: vpcSelector.project }, - }), - ]) + await apiQueryClient.prefetchQuery('vpcFirewallRulesView', { + query: getVpcSelector(params), + }) return null }