Firewall rules create/edit modal (#630)

* stub out firewall rule form based on API (needs design pass)

* msw endpoint for get rules

* hack in vpcId on firewall rule in anticipation of omicron#663

* working but ugly targets subform

* ports, hosts, and targets subforms actually work

* we can PUT rules!

* get rid of vestigial subnet copy, move name/description up

* include all existing rules when creating one

* be super sure we're not getting any extra nonsense in the update object

* fucking epic test

* update API to pull in vpc_id change from omicron main

* automatically update packer-id

* switch back to fireEvent, everything passes, get rid of yarn test:run

* update clear button colors

* lengthen timeout for CI. appalling

* faster testing through science

* explicit dep on @testing-lib/dom was actually messing me up

* wait longer for the modal to close because CI

* memoize tableData more goodly

* update API for rules so we get list of rules instead of map

* automatically update packer-id

* sort firewall rules by name in msw, fix get to put transformer

* prettier format

* make editing work

* test for edit modal

Co-authored-by: github-actions <41898282+github-actions[bot]@users.noreply.github.com>

Author: david-crespo

.github/workflows/lintBuildTest.yml

@@ -28,7 +28,7 @@ jobs:
         run: yarn lint
 
       - name: Test
-        run: yarn test:run
+        run: yarn test run
 
       - name: Build
         run: yarn build

.github/workflows/packer.yaml

@@ -75,7 +75,7 @@ jobs:
           CLOUDFLARE_TOKEN: ${{secrets.CLOUDFLARE_TOKEN}}
           SSL_CERT: ${{secrets.SSL_CERT}}
           SSL_KEY: ${{secrets.SSL_KEY}}
-          API_VERSION: c4e76cb01fa791c4dc9722072800c8969397aa03
+          API_VERSION: f615ee43803902fc8ed37a7e66a39677f885dbdb
 
       # get the image information from gcloud
       - name: Get image information

README.md

@@ -118,7 +118,7 @@ Using the script is strongly recommended, but if you really don't want to, make
 
 | Command         | Description                                                                        |
 | --------------- | ---------------------------------------------------------------------------------- |
-| `yarn test:run` | Vitest tests                                                                       |
+| `yarn test run` | Vitest tests                                                                       |
 | `yarn test`     | Vitest tests in watch mode                                                         |
 | `yarn lint`     | ESLint                                                                             |
 | `yarn tsc`      | Check types                                                                        |

app/pages/__tests__/InstanceCreatePage.spec.tsx

@@ -3,7 +3,7 @@ import {
   override,
   renderAppAt,
   screen,
-  userEvent,
+  typeByRole,
   waitFor,
 } from 'app/test-utils'
 import { org, project, instance } from '@oxide/api-mocks'
@@ -30,8 +30,7 @@ describe('InstanceCreatePage', () => {
 
   it('shows specific message for known server error code', async () => {
     renderAppAt(formUrl)
-    const name = screen.getByRole('textbox', { name: 'Choose a name' })
-    userEvent.type(name, instance.name) // already exists in db
+    typeByRole('textbox', 'Choose a name', instance.name) // already exists in db
 
     fireEvent.click(submitButton())
 
@@ -59,10 +58,9 @@ describe('InstanceCreatePage', () => {
     const instancesPage = `/orgs/${org.name}/projects/${project.name}/instances`
     expect(window.location.pathname).not.toEqual(instancesPage)
 
-    const name = screen.getByRole('textbox', { name: 'Choose a name' })
-    userEvent.type(name, 'new-instance')
-    userEvent.click(screen.getByLabelText(/6 CPUs/))
-    userEvent.click(submitButton())
+    typeByRole('textbox', 'Choose a name', 'new-instance')
+    fireEvent.click(screen.getByLabelText(/6 CPUs/))
+    fireEvent.click(submitButton())
 
     // nav to instances list
     await waitFor(() => expect(window.location.pathname).toEqual(instancesPage))

app/pages/project/instances/actions.tsx

@@ -1,8 +1,4 @@
-import type {
-  ApiListMethods,
-  Instance,
-  ProjectInstancesGetParams,
-} from '@oxide/api'
+import type { Instance, ProjectInstancesGetParams } from '@oxide/api'
 import { useApiMutation, useApiQueryClient } from '@oxide/api'
 import type { MakeActions } from '@oxide/table'
 import { Success16Icon } from '@oxide/ui'
@@ -28,7 +24,7 @@ const instanceCan: Record<string, (i: Instance) => boolean> = {
 
 export const useInstanceActions = (
   params: ProjectInstancesGetParams
-): MakeActions<ApiListMethods, 'projectInstancesGet'> => {
+): MakeActions<Instance> => {
   const addToast = useToast()
   const queryClient = useApiQueryClient()
   const refetch = () =>

app/pages/project/networking/VpcPage/VpcPage.spec.ts

@@ -1,50 +1,209 @@
 import {
-  fireEvent,
+  clickByRole,
   renderAppAt,
   screen,
-  userEvent,
+  typeByRole,
   waitForElementToBeRemoved,
+  userEvent,
+  getByRole,
 } from 'app/test-utils'
+import { defaultFirewallRules } from '@oxide/api-mocks'
 
 describe('VpcPage', () => {
-  describe('subnets tab', () => {
-    it('creating a subnet works', async () => {
+  describe('subnet', () => {
+    it('create works', async () => {
       renderAppAt('/orgs/maze-war/projects/mock-project/vpcs/mock-vpc')
       screen.getByText('Subnets')
 
       // wait for subnet to show up in the table
-      await screen.findByRole('cell', { name: 'mock-subnet' })
-      // second one is not there though
+      await screen.findByText('mock-subnet')
+      // the one we'll be adding is not there
       expect(screen.queryByRole('cell', { name: 'mock-subnet-2' })).toBeNull()
 
       // modal is not already open
-      expect(screen.queryByRole('dialog', { name: 'Create subnet' })).toBeNull()
+      expect(screen.queryByTestId('create-vpc-subnet-modal')).toBeNull()
 
       // click button to open modal
-      fireEvent.click(screen.getByRole('button', { name: 'New subnet' }))
+      await clickByRole('button', 'New subnet')
 
       // modal is open
       screen.getByRole('dialog', { name: 'Create subnet' })
 
-      const ipv4 = screen.getByRole('textbox', { name: 'IPv4 block' })
-      userEvent.type(ipv4, '1.1.1.2/24')
+      typeByRole('textbox', 'IPv4 block', '1.1.1.2/24')
+
+      typeByRole('textbox', 'Name', 'mock-subnet-2')
+
+      // submit the form
+      await clickByRole('button', 'Create subnet')
+
+      // wait for modal to close
+      await waitForElementToBeRemoved(() =>
+        screen.queryByTestId('create-vpc-subnet-modal')
+      )
+
+      // table refetches and now includes second subnet
+      await screen.findByText('mock-subnet')
+      await screen.findByText('mock-subnet-2')
+    }, 10000) // otherwise it flakes in CI
+  })
+
+  describe('firewall rule', () => {
+    it('create works', async () => {
+      renderAppAt('/orgs/maze-war/projects/mock-project/vpcs/mock-vpc')
+      await clickByRole('tab', 'Firewall Rules')
+
+      // default rules show up in the table
+      for (const { name } of defaultFirewallRules) {
+        await screen.findByText(name)
+      }
+      // the one we'll be adding is not there
+      expect(screen.queryByRole('cell', { name: 'my-new-rule' })).toBeNull()
+
+      // modal is not already open
+      expect(
+        screen.queryByRole('dialog', { name: 'Create firewall rule' })
+      ).toBeNull()
+
+      // click button to open modal
+      await clickByRole('button', 'New rule')
+
+      // modal is open
+      screen.getByRole('dialog', { name: 'Create firewall rule' })
+
+      typeByRole('textbox', 'Name', 'my-new-rule')
+
+      await clickByRole('radio', 'Outgoing')
+
+      // input type="number" becomes spinbutton for some reason
+      typeByRole('spinbutton', 'Priority', '5')
+
+      await clickByRole('button', 'Target type')
+      await clickByRole('option', 'VPC')
+      typeByRole('textbox', 'Target name', 'my-target-vpc')
+      await clickByRole('button', 'Add target')
+
+      // target is added to targets table
+      screen.getByRole('cell', { name: 'my-target-vpc' })
+
+      await clickByRole('button', 'Host type')
+      await clickByRole('option', 'Instance')
+      typeByRole('textbox', 'Value', 'host-filter-instance')
+      await clickByRole('button', 'Add host filter')
+
+      // host is added to hosts table
+      screen.getByRole('cell', { name: 'host-filter-instance' })
+
+      // TODO: test invalid port range once I put an error message in there
+      typeByRole('textbox', 'Port filter', '123-456')
+      await clickByRole('button', 'Add port filter')
+
+      // port range is added to port ranges table
+      screen.getByRole('cell', { name: '123-456' })
 
+      await clickByRole('checkbox', 'UDP')
+
+      // submit the form
+      await clickByRole('button', 'Create rule')
+
+      // wait for modal to close
+      await waitForElementToBeRemoved(
+        () => screen.queryByText('Create firewall rule'),
+        // fails in CI without a longer timeout (default 1000). boo
+        { timeout: 2000 }
+      )
+
+      // table refetches and now includes the new rule as well as the originals
+      await screen.findByText('my-new-rule')
+      screen.getByRole('cell', {
+        name: 'instance host-filter-instance UDP 123-456',
+      })
+
+      for (const { name } of defaultFirewallRules) {
+        screen.getByText(name)
+      }
+    }, 15000)
+
+    it('edit works', async () => {
+      renderAppAt('/orgs/maze-war/projects/mock-project/vpcs/mock-vpc')
+      await clickByRole('tab', 'Firewall Rules')
+
+      // default rules show up in the table
+      for (const { name } of defaultFirewallRules) {
+        await screen.findByText(name)
+      }
+      expect(screen.getAllByRole('row').length).toEqual(5) // 4 + header
+
+      // the one we'll be adding is not there
+      expect(screen.queryByRole('cell', { name: 'new-rule-name' })).toBeNull()
+
+      // modal is not already open
+      expect(
+        screen.queryByRole('dialog', { name: 'Edit firewall rule' })
+      ).toBeNull()
+
+      // click more button on allow-icmp row to get menu, then click Edit
+      const allowIcmpRow = screen.getByRole('row', { name: /allow-icmp/ })
+      const more = getByRole(allowIcmpRow, 'button', { name: 'More' })
+      await userEvent.click(more)
+      await clickByRole('menuitem', 'Edit')
+
+      // now the modal is open
+      screen.getByRole('dialog', { name: 'Edit firewall rule' })
+
+      // name is populated
       const name = screen.getByRole('textbox', { name: 'Name' })
-      userEvent.type(name, 'mock-subnet-2')
+      expect(name).toHaveValue('allow-icmp')
+
+      // priority is populated
+      const priority = screen.getByRole('spinbutton', { name: 'Priority' })
+      expect(priority).toHaveValue(65534)
+
+      // protocol is populated
+      expect(screen.getByRole('checkbox', { name: /ICMP/ })).toBeChecked()
+      expect(screen.getByRole('checkbox', { name: /TCP/ })).not.toBeChecked()
+      expect(screen.getByRole('checkbox', { name: /UDP/ })).not.toBeChecked()
+
+      // targets default vpc
+      screen.getByRole('cell', { name: 'vpc' })
+      screen.getByRole('cell', { name: 'default' })
+
+      // update name
+      await userEvent.clear(name)
+      await userEvent.type(name, 'new-rule-name')
+
+      // add host filter
+      await clickByRole('button', 'Host type')
+      await clickByRole('option', 'Instance')
+      typeByRole('textbox', 'Value', 'edit-filter-instance')
+      await clickByRole('button', 'Add host filter')
+
+      // host is added to hosts table
+      screen.getByRole('cell', { name: 'edit-filter-instance' })
 
       // submit the form
-      fireEvent.click(screen.getByRole('button', { name: 'Create subnet' }))
+      await clickByRole('button', 'Update rule')
 
       // wait for modal to close
       await waitForElementToBeRemoved(
-        () => screen.queryByRole('dialog', { name: 'Create subnet' }),
+        () => screen.queryByText('Edit firewall rule'),
         // fails in CI without a longer timeout (default 1000). boo
         { timeout: 2000 }
       )
 
-      // table refetches and now includes second subnet
-      screen.getByRole('cell', { name: 'mock-subnet' })
-      await screen.findByRole('cell', { name: 'mock-subnet-2' })
-    }, 10000) // otherwise it flakes in CI
+      // table refetches and now includes the updated rule name, not the old name
+      await screen.findByText('new-rule-name')
+      expect(screen.queryByRole('cell', { name: 'allow-icmp' })).toBeNull()
+      expect(screen.getAllByRole('row').length).toEqual(5) // 4 + header
+
+      screen.getByRole('cell', {
+        name: 'instance edit-filter-instance ICMP',
+      })
+
+      // other 3 rules are still there
+      const rest = defaultFirewallRules.filter((r) => r.name !== 'allow-icmp')
+      for (const { name } of rest) {
+        screen.getByRole('cell', { name })
+      }
+    }, 20000)
   })
 })