Skip to content

Commit d7e92f6

Browse files
Clone firewall rule (#2250)
* Switch param to route tabs: VPC firewall rules and subnets * Test fixes * Add missing paths to test * Create firewall from template * Update path-builder.spec.ts * Add missing license * Move `incrementName` into `str` and add test * Update import * refactor rule to form values logic * basic test * clean up vpc and instance detail in path builder * fix warning on firewall rules leaf route without element * clean up path params and path builder stuff a bit * merge vpc-routes into firewall-template * whoops * let's go with clone * make the min-width for more actions menus smaller * remove special min width on top bar dropdown menu that wasn't working anyway * fix e2e test and assert more stuff about the form * be less smart about copied name, just add -copy * rename path builder function * we're doing comments. we love comments --------- Co-authored-by: David Crespo <[email protected]>
1 parent d7712f1 commit d7e92f6

File tree

9 files changed

+68
-13
lines changed

9 files changed

+68
-13
lines changed

app/components/TopBar.tsx

Lines changed: 1 addition & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -62,11 +62,7 @@ export function TopBar({ children }: { children: React.ReactNode }) {
6262
<DirectionDownIcon className="!w-2.5" />
6363
</Button>
6464
</DropdownMenu.Trigger>
65-
<DropdownMenu.Content
66-
align="end"
67-
sideOffset={8}
68-
className="min-w-[12.8125rem]"
69-
>
65+
<DropdownMenu.Content align="end" sideOffset={8}>
7066
<DropdownMenu.Item onSelect={() => navigate(pb.profile())}>
7167
Settings
7268
</DropdownMenu.Item>

app/forms/firewall-rules-create.tsx

Lines changed: 24 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@
77
*/
88
import { useMemo } from 'react'
99
import { useController, type Control } from 'react-hook-form'
10-
import { useNavigate, type LoaderFunctionArgs } from 'react-router-dom'
10+
import { useNavigate, useParams, type LoaderFunctionArgs } from 'react-router-dom'
1111
import * as R from 'remeda'
1212

1313
import {
@@ -74,7 +74,17 @@ export const valuesToRuleUpdate = (values: FirewallRuleValues): VpcFirewallRuleU
7474
targets: values.targets,
7575
})
7676

77-
const defaultValues: FirewallRuleValues = {
77+
/** convert in the opposite direction for when we're creating from existing rule */
78+
const ruleToValues = (rule: VpcFirewallRule): FirewallRuleValues => ({
79+
...rule,
80+
enabled: rule.status === 'enabled',
81+
protocols: rule.filters.protocols || [],
82+
ports: rule.filters.ports || [],
83+
hosts: rule.filters.hosts || [],
84+
})
85+
86+
/** Empty form for when we're not creating from an existing rule */
87+
const defaultValuesEmpty: FirewallRuleValues = {
7888
enabled: true,
7989
name: '',
8090
description: '',
@@ -586,6 +596,18 @@ export function CreateFirewallRuleForm() {
586596
})
587597
const existingRules = useMemo(() => R.sortBy(data.rules, (r) => r.priority), [data])
588598

599+
// The :rule path param is optional. If it is present, we are creating a
600+
// rule from an existing one, so we find that rule and copy it into the form
601+
// values. Note that if we fail to find the rule by name (which should be
602+
// very unlikely) we just pretend we never saw a name in the path and start
603+
// from scratch.
604+
const { rule: ruleName } = useParams()
605+
const originalRule = existingRules.find((rule) => rule.name === ruleName)
606+
607+
const defaultValues: FirewallRuleValues = originalRule
608+
? ruleToValues({ ...originalRule, name: originalRule.name + '-copy' })
609+
: defaultValuesEmpty
610+
589611
const form = useForm({ defaultValues })
590612

591613
return (

app/pages/project/vpcs/VpcPage/tabs/VpcFirewallRulesTab.tsx

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -139,6 +139,12 @@ export function VpcFirewallRulesTab() {
139139
navigate(pb.vpcFirewallRuleEdit({ ...vpcSelector, rule: rule.name }))
140140
},
141141
},
142+
{
143+
label: 'Clone',
144+
onActivate() {
145+
navigate(pb.vpcFirewallRuleClone({ ...vpcSelector, rule: rule.name }))
146+
},
147+
},
142148
{
143149
label: 'Delete',
144150
onActivate: confirmDelete({

app/pages/project/vpcs/VpcPage/tabs/VpcSubnetsTab.tsx

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -85,8 +85,6 @@ export function VpcSubnetsTab() {
8585
[vpcSelector, makeActions]
8686
)
8787

88-
// const columns = useColsWithActions(staticCols, makeActions)
89-
9088
const emptyState = (
9189
<EmptyMessage
9290
title="No VPC subnets"

app/routes.tsx

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -364,7 +364,7 @@ export const routes = createRoutesFromElements(
364364
element={null}
365365
/>
366366
<Route
367-
path="firewall-rules-new"
367+
path="firewall-rules-new/:rule?"
368368
element={<CreateFirewallRuleForm />}
369369
loader={CreateFirewallRuleForm.loader}
370370
handle={{ crumb: 'New Firewall Rule' }}

app/ui/styles/components/menu-button.css

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@
77
*/
88

99
.DropdownMenuContent {
10-
@apply z-30 min-w-[14rem] rounded border p-0 bg-raise border-secondary;
10+
@apply z-30 min-w-36 rounded border p-0 bg-raise border-secondary;
1111

1212
& .DropdownMenuItem {
1313
@apply block cursor-pointer select-none border-b py-2 pl-3 pr-6 text-left text-sans-md text-secondary border-secondary last-of-type:border-b-0;

app/util/path-builder.spec.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -88,6 +88,7 @@ test('path builder', () => {
8888
"systemUtilization": "/system/utilization",
8989
"vpc": "/projects/p/vpcs/v/firewall-rules",
9090
"vpcEdit": "/projects/p/vpcs/v/edit",
91+
"vpcFirewallRuleClone": "/projects/p/vpcs/v/firewall-rules-new/fr",
9192
"vpcFirewallRuleEdit": "/projects/p/vpcs/v/firewall-rules/fr/edit",
9293
"vpcFirewallRules": "/projects/p/vpcs/v/firewall-rules",
9394
"vpcFirewallRulesNew": "/projects/p/vpcs/v/firewall-rules-new",

app/util/path-builder.ts

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -75,11 +75,12 @@ export const pb = {
7575

7676
// same deal as instance detail: go straight to first tab
7777
vpc: (params: Vpc) => pb.vpcFirewallRules(params),
78-
7978
vpcEdit: (params: Vpc) => `${vpcBase(params)}/edit`,
8079

8180
vpcFirewallRules: (params: Vpc) => `${vpcBase(params)}/firewall-rules`,
8281
vpcFirewallRulesNew: (params: Vpc) => `${vpcBase(params)}/firewall-rules-new`,
82+
vpcFirewallRuleClone: (params: FirewallRule) =>
83+
`${pb.vpcFirewallRulesNew(params)}/${params.rule}`,
8384
vpcFirewallRuleEdit: (params: FirewallRule) =>
8485
`${pb.vpcFirewallRules(params)}/${params.rule}/edit`,
8586
vpcSubnets: (params: Vpc) => `${vpcBase(params)}/subnets`,

test/e2e/firewall-rules.e2e.ts

Lines changed: 32 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@
66
* Copyright Oxide Computer Company
77
*/
88

9-
import { expect, expectRowVisible, test } from './utils'
9+
import { clickRowAction, expect, expectRowVisible, test } from './utils'
1010

1111
const defaultRules = ['allow-internal-inbound', 'allow-ssh', 'allow-icmp', 'allow-rdp']
1212

@@ -310,6 +310,37 @@ test('can update firewall rule', async ({ page }) => {
310310
}
311311
})
312312

313+
test('create from existing rule', async ({ page }) => {
314+
const url = '/projects/mock-project/vpcs/mock-vpc/firewall-rules'
315+
await page.goto(url)
316+
317+
const modal = page.getByRole('dialog', { name: 'Add firewall rule' })
318+
await expect(modal).toBeHidden()
319+
320+
await clickRowAction(page, 'allow-rdp', 'Clone')
321+
322+
await expect(page).toHaveURL(url + '-new/allow-rdp')
323+
await expect(modal).toBeVisible()
324+
await expect(modal.getByRole('textbox', { name: 'Name', exact: true })).toHaveValue(
325+
'allow-rdp-copy'
326+
)
327+
328+
await expect(modal.getByRole('checkbox', { name: 'TCP' })).toBeChecked()
329+
await expect(modal.getByRole('checkbox', { name: 'UDP' })).not.toBeChecked()
330+
await expect(modal.getByRole('checkbox', { name: 'ICMP' })).not.toBeChecked()
331+
332+
await expect(
333+
modal
334+
.getByRole('table', { name: 'Port filters' })
335+
.getByRole('cell', { name: '3389', exact: true })
336+
).toBeVisible()
337+
await expect(
338+
modal
339+
.getByRole('table', { name: 'Targets' })
340+
.getByRole('row', { name: 'Name: default, Type: vpc' })
341+
).toBeVisible()
342+
})
343+
313344
const rulePath = '/projects/mock-project/vpcs/mock-vpc/firewall-rules/allow-icmp/edit'
314345

315346
test('can edit rule directly by URL', async ({ page }) => {

0 commit comments

Comments
 (0)