+
+
+ ))}
+
+
+ )}
@@ -279,128 +289,145 @@ export const CommonFields = ({ error, control }: CommonFieldsProps) => {
required
control={hostForm.control}
/>
- {/* For everything but IP this is a name, but for IP it's an IP.
+
+
+ {/* For everything but IP this is a name, but for IP it's an IP.
So we should probably have the label on this field change when the
host type changes. Also need to confirm that it's just an IP and
not a block. */}
-
+
+
+
+
+
-
-
-
- Type
- Value
-
-
-
-
- {hosts.value.map((h) => (
-
- {/* TODO: should be the pretty type label, not the type key */}
- {h.type}
- {h.value}
-
- {
- hosts.onChange(
- hosts.value.filter((h1) => h1.value !== h.value && h1.type !== h.type)
- )
- }}
- />
-
-
- ))}
-
-
+ {!!hosts.value.length && (
+
+
+ Type
+ Value
+ {/* For remove button */}
+
+
+
+ {hosts.value.map((h, index) => (
+
+
+ {h.type}
+
+ {h.value}
+
+
+
+
+ ))}
+
+
+ )}
-
-
-
-
+
+
+
+
+
+
-
-
-
- Range
-
-
-
-
- {ports.value.map((p) => (
-
- {/* TODO: should be the pretty type label, not the type key */}
- {p}
-
- {
- ports.onChange(ports.value.filter((p1) => p1 !== p))
- }}
- />
-
-
- ))}
-
-
+
+ {!!ports.value.length && (
+
+
+ Range
+ {/* For remove button */}
+
+
+
+ {ports.value.map((p) => (
+
+ {p}
+
+
+
+
+ ))}
+
+
+ )}
diff --git a/app/pages/project/networking/VpcPage/VpcPage.tsx b/app/pages/project/networking/VpcPage/VpcPage.tsx
index 6ede9eab20..1b0fefb6a7 100644
--- a/app/pages/project/networking/VpcPage/VpcPage.tsx
+++ b/app/pages/project/networking/VpcPage/VpcPage.tsx
@@ -19,7 +19,15 @@ import { VpcSubnetsTab } from './tabs/VpcSubnetsTab'
VpcPage.loader = async ({ params }: LoaderFunctionArgs) => {
const { project, vpc } = getVpcSelector(params)
- await apiQueryClient.prefetchQuery('vpcView', { path: { vpc }, query: { project } })
+ await Promise.all([
+ apiQueryClient.prefetchQuery('vpcView', { path: { vpc }, query: { project } }),
+ apiQueryClient.prefetchQuery('vpcFirewallRulesView', {
+ query: { project, vpc },
+ }),
+ apiQueryClient.prefetchQuery('vpcSubnetList', {
+ query: { project, vpc, limit: 25 },
+ }),
+ ])
return null
}
diff --git a/app/pages/project/networking/VpcPage/tabs/VpcFirewallRulesTab.tsx b/app/pages/project/networking/VpcPage/tabs/VpcFirewallRulesTab.tsx
index 6682bbc99d..c2113c3f8e 100644
--- a/app/pages/project/networking/VpcPage/tabs/VpcFirewallRulesTab.tsx
+++ b/app/pages/project/networking/VpcPage/tabs/VpcFirewallRulesTab.tsx
@@ -9,8 +9,8 @@ import { useMemo, useState } from 'react'
import {
useApiMutation,
- useApiQuery,
useApiQueryClient,
+ usePrefetchedApiQuery,
type VpcFirewallRule,
} from '@oxide/api'
import {
@@ -24,6 +24,7 @@ import {
useReactTable,
} from '@oxide/table'
import { Button, EmptyMessage, TableEmptyBox } from '@oxide/ui'
+import { sortBy, titleCase } from '@oxide/util'
import { CreateFirewallRuleForm } from 'app/forms/firewall-rules-create'
import { EditFirewallRuleForm } from 'app/forms/firewall-rules-edit'
@@ -35,7 +36,18 @@ const colHelper = createColumnHelper
()
/** columns that don't depend on anything in `render` */
const staticColumns = [
colHelper.accessor('name', { header: 'Name' }),
- colHelper.accessor('action', { header: 'Action' }),
+ colHelper.accessor('priority', {
+ header: 'Priority',
+ cell: (info) => {info.getValue()}
,
+ }),
+ colHelper.accessor('action', {
+ header: 'Action',
+ cell: (info) => {titleCase(info.getValue())}
,
+ }),
+ colHelper.accessor('direction', {
+ header: 'Direction',
+ cell: (info) => {titleCase(info.getValue())}
,
+ }),
colHelper.accessor('targets', {
header: 'Targets',
cell: (info) => ,
@@ -59,8 +71,10 @@ export const VpcFirewallRulesTab = () => {
const queryClient = useApiQueryClient()
const vpcSelector = useVpcSelector()
- const { data, isLoading } = useApiQuery('vpcFirewallRulesView', { query: vpcSelector })
- const rules = useMemo(() => data?.rules || [], [data])
+ const { data } = usePrefetchedApiQuery('vpcFirewallRulesView', {
+ query: vpcSelector,
+ })
+ const rules = useMemo(() => sortBy(data.rules, (r) => r.priority), [data])
const [createModalOpen, setCreateModalOpen] = useState(false)
const [editing, setEditing] = useState(null)
@@ -127,7 +141,7 @@ export const VpcFirewallRulesTab = () => {
/>
)}
- {rules.length > 0 || isLoading ?
: emptyState}
+ {rules.length > 0 ?
: emptyState}
)
}
diff --git a/app/test/e2e/firewall-rules.e2e.ts b/app/test/e2e/firewall-rules.e2e.ts
index 60b8f84f12..144088bae9 100644
--- a/app/test/e2e/firewall-rules.e2e.ts
+++ b/app/test/e2e/firewall-rules.e2e.ts
@@ -5,15 +5,14 @@
*
* Copyright Oxide Computer Company
*/
-import { test } from '@playwright/test'
-import { expect, expectNotVisible, expectVisible } from './utils'
+import { expect, expectRowVisible, test } from './utils'
const defaultRules = ['allow-internal-inbound', 'allow-ssh', 'allow-icmp', 'allow-rdp']
test('can create firewall rule', async ({ page }) => {
await page.goto('/projects/mock-project/vpcs/mock-vpc')
- await page.locator('text="Firewall Rules"').click()
+ await page.getByRole('tab', { name: 'Firewall Rules' }).click()
// default rules are all there
for (const name of defaultRules) {
@@ -22,28 +21,28 @@ test('can create firewall rule', async ({ page }) => {
const rows = page.locator('tbody >> tr')
await expect(rows).toHaveCount(4)
- const modal = 'role=dialog[name="Add firewall rule"]'
- await expectNotVisible(page, [modal])
+ const modal = page.getByRole('dialog', { name: 'Add firewall rule' })
+ await expect(modal).toBeHidden()
// open modal
- await page.locator('text="New rule"').click()
+ await page.getByRole('button', { name: 'New rule' }).click()
// modal is now open
- await expectVisible(page, [modal])
+ await expect(modal).toBeVisible()
await page.fill('input[name=name]', 'my-new-rule')
- await page.locator('text=Outgoing').click()
+ await page.locator('text=Outbound').click()
- await page.fill('role=spinbutton[name="Priority"]', '5')
+ await page.fill('role=textbox[name="Priority"]', '5')
- // add target VPC "my-target-vpc"
- await page.locator('role=button[name*="Target type"]').click()
- await page.locator('role=option[name="IP"]').click()
- await page.fill('role=textbox[name="IP address"]', '192.168.0.1')
- await page.locator('text="Add target"').click()
+ // add targets with overlapping names and types to test delete
+ const targets = page.getByRole('table', { name: 'Targets' })
- // target is added to targets table
- await expect(page.locator('td:has-text("192.168.0.1")')).toBeVisible()
+ await page.getByRole('button', { name: 'Target type' }).click()
+ await page.getByRole('option', { name: 'IP', exact: true }).click()
+ await page.getByRole('textbox', { name: 'IP address' }).fill('192.168.0.1')
+ await page.getByRole('button', { name: 'Add target' }).click()
+ await expectRowVisible(targets, { Type: 'ip', Value: '192.168.0.1' })
// add host filter instance "host-filter-instance"
await page.locator('role=button[name*="Host type"]').click()
@@ -52,14 +51,16 @@ test('can create firewall rule', async ({ page }) => {
await page.locator('text="Add host filter"').click()
// host is added to hosts table
- await expect(page.locator('td:has-text("host-filter-instance")')).toBeVisible()
+ const hosts = page.getByRole('table', { name: 'Host filters' })
+ await expectRowVisible(hosts, { Type: 'instance', Value: 'host-filter-instance' })
// TODO: test invalid port range once I put an error message in there
await page.fill('role=textbox[name="Port filter"]', '123-456')
- await page.locator('text="Add port filter"').click()
+ await page.getByRole('button', { name: 'Add port filter' }).click()
// port range is added to port ranges table
- await expect(page.locator('td:has-text("123-456")')).toBeVisible()
+ const ports = page.getByRole('table', { name: 'Ports' })
+ await expectRowVisible(ports, { Range: '123-456' })
// check the UDP box
await page.locator('text=UDP').click()
@@ -68,14 +69,15 @@ test('can create firewall rule', async ({ page }) => {
await page.locator('text="Add rule"').click()
// modal closes again
- await expectNotVisible(page, [modal])
+ await expect(modal).toBeHidden()
// table refetches and now includes the new rule as well as the originals
- await expect(page.locator('td >> text="my-new-rule"')).toBeVisible()
- // target shows up in target cell
- await expect(page.locator('text=ip192.168.0.1')).toBeVisible()
- // other stuff filled out shows up in the filters column
- await expect(page.locator('text=instancehost-filter-instanceUDP123-456')).toBeVisible()
+ await expectRowVisible(page.getByRole('table'), {
+ Name: 'my-new-rule',
+ Priority: '5',
+ Targets: 'ip192.168.0.1',
+ Filters: 'instancehost-filter-instanceUDP123-456',
+ })
await expect(rows).toHaveCount(5)
for (const name of defaultRules) {
@@ -83,9 +85,109 @@ test('can create firewall rule', async ({ page }) => {
}
})
+test('firewall rule form targets table', async ({ page }) => {
+ await page.goto('/projects/mock-project/vpcs/mock-vpc')
+ await page.getByRole('tab', { name: 'Firewall Rules' }).click()
+
+ // open modal
+ await page.getByRole('button', { name: 'New rule' }).click()
+
+ const targets = page.getByRole('table', { name: 'Targets' })
+ const addButton = page.getByRole('button', { name: 'Add target' })
+
+ // add targets with overlapping names and types to test delete
+
+ // there are two of these because the hosts table also defaults to VPC
+ await page.getByRole('textbox', { name: 'VPC name' }).nth(0).fill('abc')
+ await addButton.click()
+ await expectRowVisible(targets, { Type: 'vpc', Value: 'abc' })
+
+ await page.getByRole('textbox', { name: 'VPC name' }).nth(0).fill('def')
+ await addButton.click()
+ await expectRowVisible(targets, { Type: 'vpc', Value: 'def' })
+
+ await page.getByRole('button', { name: 'Target type' }).click()
+ await page.getByRole('option', { name: 'VPC Subnet' }).click()
+ await page.getByRole('textbox', { name: 'Subnet name' }).fill('abc')
+ await addButton.click()
+ await expectRowVisible(targets, { Type: 'subnet', Value: 'abc' })
+
+ await page.getByRole('button', { name: 'Target type' }).click()
+ await page.getByRole('option', { name: 'IP', exact: true }).click()
+ await page.getByRole('textbox', { name: 'IP address' }).fill('192.168.0.1')
+ await addButton.click()
+ await expectRowVisible(targets, { Type: 'ip', Value: '192.168.0.1' })
+
+ // test table row delete, only keep the IP one
+ const firstDeleteButton = targets
+ .getByRole('row')
+ .nth(1)
+ .getByRole('button', { name: 'remove' })
+ await expect(targets.getByRole('row')).toHaveCount(5)
+ await firstDeleteButton.click()
+ await expect(targets.getByRole('row')).toHaveCount(4)
+ await firstDeleteButton.click()
+ await expect(targets.getByRole('row')).toHaveCount(3)
+ await firstDeleteButton.click()
+ await expect(targets.getByRole('row')).toHaveCount(2)
+
+ // we still have the IP target
+ await expectRowVisible(targets, { Type: 'ip', Value: '192.168.0.1' })
+})
+
+test('firewall rule form hosts table', async ({ page }) => {
+ await page.goto('/projects/mock-project/vpcs/mock-vpc')
+ await page.getByRole('tab', { name: 'Firewall Rules' }).click()
+
+ // open modal
+ await page.getByRole('button', { name: 'New rule' }).click()
+
+ const hosts = page.getByRole('table', { name: 'Host filters' })
+ const addButton = page.getByRole('button', { name: 'Add host filter' })
+
+ // add hosts with overlapping names and types to test delete
+
+ // there are two of these because the targets table also defaults to VPC
+ await page.getByRole('textbox', { name: 'VPC name' }).nth(1).fill('abc')
+ await addButton.click()
+ await expectRowVisible(hosts, { Type: 'vpc', Value: 'abc' })
+
+ await page.getByRole('textbox', { name: 'VPC name' }).nth(1).fill('def')
+ await addButton.click()
+ await expectRowVisible(hosts, { Type: 'vpc', Value: 'def' })
+
+ await page.getByRole('button', { name: 'Host type' }).click()
+ await page.getByRole('option', { name: 'VPC Subnet' }).click()
+ await page.getByRole('textbox', { name: 'Subnet name' }).fill('abc')
+ await addButton.click()
+ await expectRowVisible(hosts, { Type: 'subnet', Value: 'abc' })
+
+ await page.getByRole('button', { name: 'Host type' }).click()
+ await page.getByRole('option', { name: 'IP', exact: true }).click()
+ await page.getByRole('textbox', { name: 'IP address' }).fill('192.168.0.1')
+ await addButton.click()
+ await expectRowVisible(hosts, { Type: 'ip', Value: '192.168.0.1' })
+
+ // test table row delete, only keep the IP one
+ const firstDeleteButton = hosts
+ .getByRole('row')
+ .nth(1)
+ .getByRole('button', { name: 'remove' })
+ await expect(hosts.getByRole('row')).toHaveCount(5)
+ await firstDeleteButton.click()
+ await expect(hosts.getByRole('row')).toHaveCount(4)
+ await firstDeleteButton.click()
+ await expect(hosts.getByRole('row')).toHaveCount(3)
+ await firstDeleteButton.click()
+ await expect(hosts.getByRole('row')).toHaveCount(2)
+
+ // we still have the IP target
+ await expectRowVisible(hosts, { Type: 'ip', Value: '192.168.0.1' })
+})
+
test('can update firewall rule', async ({ page }) => {
await page.goto('/projects/mock-project/vpcs/mock-vpc')
- await page.locator('text="Firewall Rules"').click()
+ await page.getByRole('tab', { name: 'Firewall Rules' }).click()
const rows = page.locator('tbody >> tr')
await expect(rows).toHaveCount(4)
@@ -97,7 +199,7 @@ test('can update firewall rule', async ({ page }) => {
const newNameCell = page.locator('td >> text="new-rule-name"')
await expect(newNameCell).toBeHidden()
- const modal = page.locator('text="Edit rule"')
+ const modal = page.getByRole('dialog', { name: 'Edit rule' })
await expect(modal).toBeHidden()
// click more button on allow-icmp row to get menu, then click Edit
@@ -118,7 +220,7 @@ test('can update firewall rule', async ({ page }) => {
await expect(page.locator('input[name=name]')).toHaveValue('allow-icmp')
// priority is populated
- await expect(page.locator('input[name=priority]')).toHaveValue('65534')
+ await expect(page.locator('role=textbox[name=Priority]')).toHaveValue('65534')
// protocol is populated
await expect(page.locator('label >> text=ICMP')).toBeChecked()
@@ -139,7 +241,7 @@ test('can update firewall rule', async ({ page }) => {
await page.locator('text="Add host filter"').click()
// new host is added to hosts table
- await expect(page.locator('td:has-text("edit-filter-subnet")')).toBeVisible()
+ await expect(page.locator('role=cell >> text="edit-filter-subnet"')).toBeVisible()
// submit the form
await page.locator('text="Update rule"').click()
diff --git a/libs/util/str.spec.ts b/libs/util/str.spec.ts
index 4a4dd3d079..3a4a8a28e2 100644
--- a/libs/util/str.spec.ts
+++ b/libs/util/str.spec.ts
@@ -7,7 +7,7 @@
*/
import { describe, expect, it } from 'vitest'
-import { camelCase, capitalize, commaSeries, kebabCase } from './str'
+import { camelCase, capitalize, commaSeries, kebabCase, titleCase } from './str'
describe('capitalize', () => {
it('capitalizes the first letter', () => {
@@ -46,3 +46,33 @@ it('commaSeries', () => {
expect(commaSeries(['a', 'b'], 'or')).toBe('a or b')
expect(commaSeries(['a', 'b', 'c'], 'or')).toBe('a, b, or c')
})
+
+describe('titleCase', () => {
+ it('converts single words to title case', () => {
+ expect(titleCase('hello')).toBe('Hello')
+ })
+
+ it('converts multiple words to title case', () => {
+ expect(titleCase('hello world')).toBe('Hello World')
+ })
+
+ it('handles mixed case input correctly', () => {
+ expect(titleCase('hElLo WoRlD')).toBe('Hello World')
+ })
+
+ it('works correctly with strings containing punctuation', () => {
+ expect(titleCase('hello, world!')).toBe('Hello, World!')
+ })
+
+ it('works correctly with empty strings', () => {
+ expect(titleCase('')).toBe('')
+ })
+
+ it('handles strings with only one character', () => {
+ expect(titleCase('a')).toBe('A')
+ })
+
+ it('doesn’t modify non-letter characters', () => {
+ expect(titleCase('123 abc')).toBe('123 Abc')
+ })
+})
diff --git a/libs/util/str.ts b/libs/util/str.ts
index 32739a404d..b6ca1d73d3 100644
--- a/libs/util/str.ts
+++ b/libs/util/str.ts
@@ -42,3 +42,10 @@ export const commaSeries = (items: string[], conj: string) => {
}
return [...items.slice(0, -1), `${conj} ${items.at(-1)}`].join(', ')
}
+
+export const titleCase = (text: string): string => {
+ return text.replace(
+ /\w\S*/g,
+ (text) => text.charAt(0).toUpperCase() + text.substring(1).toLowerCase()
+ )
+}