tooling: make mock API default firewall rules more accurate (#2349)

* remove allow-rdp from default firewall rules * populate VPC with default rules on creation in mock API, update e2e test * do the clone test even better * missed one
oxidecomputer · Jul 30, 2024 · 07b6c15 · 07b6c15
1 parent d32fddc
commit 07b6c15
Show file tree

Hide file tree

Showing 7 changed files with 158 additions and 115 deletions.
diff --git a/mock-api/msw/handlers.ts b/mock-api/msw/handlers.ts
@@ -27,6 +27,8 @@ import { GiB } from '~/util/units'
 import { genCumulativeI64Data } from '../metrics'
 import { serial } from '../serial'
 import { defaultSilo, toIdp } from '../silo'
+import { getTimestamps } from '../util'
+import { defaultFirewallRules } from '../vpc'
 import {
   db,
   getIpFromPool,
@@ -41,7 +43,6 @@ import {
   errIfInvalidDiskSize,
   forbiddenErr,
   getStartAndEndTime,
-  getTimestamps,
   handleMetrics,
   ipInAnyRange,
   ipRangeLen,
@@ -997,6 +998,9 @@ export const handlers = makeHandlers({
     }
     db.vpcSubnets.push(newSubnet)
 
+    // populate default firewall rules
+    db.vpcFirewallRules.push(...defaultFirewallRules(newVpc.id))
+
     return json(newVpc, { status: 201 })
   },
   vpcView: ({ path, query }) => lookup.vpc({ ...path, ...query }),

diff --git a/mock-api/msw/util.ts b/mock-api/msw/util.ts
@@ -86,11 +86,6 @@ export function getStartAndEndTime(params: { startTime?: Date; endTime?: Date })
   return { startTime, endTime }
 }
 
-export function getTimestamps() {
-  const now = new Date().toISOString()
-  return { time_created: now, time_modified: now }
-}
-
 export const forbiddenErr = () =>
   json({ error_code: 'Forbidden', request_id: 'fake-id' }, { status: 403 })
 

diff --git a/mock-api/util.ts b/mock-api/util.ts
@@ -0,0 +1,15 @@
+/*
+ * This Source Code Form is subject to the terms of the Mozilla Public
+ * License, v. 2.0. If a copy of the MPL was not distributed with this
+ * file, you can obtain one at https://mozilla.org/MPL/2.0/.
+ *
+ * Copyright Oxide Computer Company
+ */
+
+// this is only in its own file so it can be used in both the mock resources and
+// the mock handlers without circular import issues
+
+export function getTimestamps() {
+  const now = new Date().toISOString()
+  return { time_created: now, time_modified: now }
+}
diff --git a/mock-api/vpc.ts b/mock-api/vpc.ts
@@ -6,10 +6,13 @@
  * Copyright Oxide Computer Company
  */
 
+import { v4 as uuid } from 'uuid'
+
 import type { Vpc, VpcFirewallRule, VpcSubnet } from '@oxide/api'
 
 import type { Json } from './json-type'
 import { project, project2 } from './project'
+import { getTimestamps } from './util'
 
 const time_created = new Date(2021, 0, 1).toISOString()
 const time_modified = new Date(2021, 0, 2).toISOString()
@@ -63,78 +66,67 @@ export const vpcSubnet2: Json<VpcSubnet> = {
   ipv4_block: '10.1.1.2/24',
 }
 
-export const firewallRules: Json<VpcFirewallRule[]> = [
-  {
-    id: 'b74aeea8-1201-4efd-b6ec-011f10a0b176',
-    name: 'allow-internal-inbound',
-    status: 'enabled',
-    direction: 'inbound',
-    targets: [{ type: 'vpc', value: 'default' }],
-    action: 'allow',
-    description:
-      'allow inbound traffic to all instances within the VPC if originated within the VPC',
-    filters: {
-      hosts: [{ type: 'vpc', value: 'default' }],
+export function defaultFirewallRules(vpcId: string): Json<VpcFirewallRule[]> {
+  return [
+    {
+      id: uuid(),
+      vpc_id: vpcId,
+      name: 'allow-internal-inbound',
+      status: 'enabled',
+      direction: 'inbound',
+      targets: [{ type: 'vpc', value: 'default' }],
+      action: 'allow',
+      description:
+        'allow inbound traffic to all instances within the VPC if originated within the VPC',
+      filters: {
+        hosts: [{ type: 'vpc', value: 'default' }],
+      },
+      priority: 65534,
+      ...getTimestamps(),
     },
-    priority: 65534,
-    time_created,
-    time_modified,
-    vpc_id: vpc.id,
-  },
-  {
-    id: '9802cd8e-1e59-4fdf-9b40-99c189f7a19b',
-    name: 'allow-ssh',
-    status: 'enabled',
-    direction: 'inbound',
-    targets: [{ type: 'vpc', value: 'default' }],
-    description: 'allow inbound TCP connections on port 22 from anywhere',
-    filters: {
-      ports: ['22'],
-      protocols: ['TCP'],
+    {
+      id: uuid(),
+      vpc_id: vpcId,
+      name: 'allow-ssh',
+      status: 'enabled',
+      direction: 'inbound',
+      targets: [{ type: 'vpc', value: 'default' }],
+      description: 'allow inbound TCP connections on port 22 from anywhere',
+      filters: {
+        ports: ['22'],
+        protocols: ['TCP'],
+      },
+      action: 'allow',
+      priority: 65534,
+      ...getTimestamps(),
     },
-    action: 'allow',
-    priority: 65534,
-    time_created,
-    time_modified,
-    vpc_id: vpc.id,
-  },
-  {
-    id: 'cde07d86-b8c0-49ed-8754-55f1bdee20fe',
-    name: 'allow-icmp',
-    status: 'enabled',
-    direction: 'inbound',
-    targets: [{ type: 'vpc', value: 'default' }],
-    description: 'allow inbound ICMP traffic from anywhere',
-    filters: {
-      protocols: ['ICMP'],
-    },
-    action: 'allow',
-    priority: 65534,
-    time_created,
-    time_modified,
-    vpc_id: vpc.id,
-  },
-  {
-    id: '5ed562d9-2566-496d-b7b3-7976b04a0b80',
-    name: 'allow-rdp',
-    status: 'enabled',
-    direction: 'inbound',
-    targets: [{ type: 'vpc', value: 'default' }],
-    description: 'allow inbound TCP connections on port 3389 from anywhere',
-    filters: {
-      ports: ['3389'],
-      protocols: ['TCP'],
+    {
+      id: uuid(),
+      vpc_id: vpcId,
+      name: 'allow-icmp',
+      status: 'enabled',
+      direction: 'inbound',
+      targets: [{ type: 'vpc', value: 'default' }],
+      description: 'allow inbound ICMP traffic from anywhere',
+      filters: {
+        protocols: ['ICMP'],
+      },
+      action: 'allow',
+      priority: 65534,
+      ...getTimestamps(),
     },
-    action: 'allow',
-    priority: 65534,
-    time_created,
-    time_modified,
-    vpc_id: vpc.id,
-  },
+  ]
+}
+
+// usually we try to hard-code resource IDs, but in this case
+// we don't rely on them anywhere and it's easier to wrap up if they're dynamic
+
+export const firewallRules: Json<VpcFirewallRule[]> = [
+  ...defaultFirewallRules(vpc.id),
   // second mock VPC in other project, meant to test display with lots  of
   // targets and filters
   {
-    id: '097c849e-68c8-43f7-9ceb-b1855c51f178',
+    id: uuid(),
     name: 'lots-of-filters',
     status: 'enabled',
     direction: 'inbound',
@@ -156,7 +148,7 @@ export const firewallRules: Json<VpcFirewallRule[]> = [
     vpc_id: vpc2.id,
   },
   {
-    id: '097c849e-68c8-43f7-9ceb-b1855c51f178',
+    id: uuid(),
     name: 'lots-of-targets',
     status: 'enabled',
     direction: 'inbound',

diff --git a/test/e2e/firewall-rules.e2e.ts b/test/e2e/firewall-rules.e2e.ts
@@ -8,7 +8,7 @@
 
 import { clickRowAction, expect, expectRowVisible, test } from './utils'
 
-const defaultRules = ['allow-internal-inbound', 'allow-ssh', 'allow-icmp', 'allow-rdp']
+const defaultRules = ['allow-internal-inbound', 'allow-ssh', 'allow-icmp']
 
 test('can create firewall rule', async ({ page }) => {
   await page.goto('/projects/mock-project/vpcs/mock-vpc')
@@ -19,7 +19,7 @@ test('can create firewall rule', async ({ page }) => {
     await expect(page.locator(`text="${name}"`)).toBeVisible()
   }
   const rows = page.locator('tbody >> tr')
-  await expect(rows).toHaveCount(4)
+  await expect(rows).toHaveCount(3)
 
   const modal = page.getByRole('dialog', { name: 'Add firewall rule' })
   await expect(modal).toBeHidden()
@@ -100,7 +100,7 @@ test('can create firewall rule', async ({ page }) => {
   const tooltip = page.getByRole('tooltip', { name: 'Other filters UDP Port 123-' })
   await expect(tooltip).toBeVisible()
 
-  await expect(rows).toHaveCount(5)
+  await expect(rows).toHaveCount(4)
   for (const name of defaultRules) {
     await expect(page.locator(`text="${name}"`)).toBeVisible()
   }
@@ -241,7 +241,7 @@ test('can update firewall rule', async ({ page }) => {
   await page.getByRole('tab', { name: 'Firewall Rules' }).click()
 
   const rows = page.locator('tbody >> tr')
-  await expect(rows).toHaveCount(4)
+  await expect(rows).toHaveCount(3)
 
   // allow-icmp is the one we're doing to change
   const oldNameCell = page.locator('td >> text="allow-icmp"')
@@ -298,7 +298,7 @@ test('can update firewall rule', async ({ page }) => {
   await expect(newNameCell).toBeVisible()
   await expect(oldNameCell).toBeHidden()
 
-  await expect(rows).toHaveCount(4)
+  await expect(rows).toHaveCount(3)
 
   // new target shows up in target cell
   await expect(page.locator('text=subnetedit-filter-subnetICMP')).toBeVisible()
@@ -317,28 +317,44 @@ test('create from existing rule', async ({ page }) => {
   const modal = page.getByRole('dialog', { name: 'Add firewall rule' })
   await expect(modal).toBeHidden()
 
-  await clickRowAction(page, 'allow-rdp', 'Clone')
+  await clickRowAction(page, 'allow-icmp', 'Clone')
+
+  await expect(page).toHaveURL(url + '-new/allow-icmp')
+  await expect(modal).toBeVisible()
+  await expect(modal.getByRole('textbox', { name: 'Name', exact: true })).toHaveValue(
+    'allow-icmp-copy'
+  )
+
+  await expect(modal.getByRole('checkbox', { name: 'TCP' })).not.toBeChecked()
+  await expect(modal.getByRole('checkbox', { name: 'UDP' })).not.toBeChecked()
+  await expect(modal.getByRole('checkbox', { name: 'ICMP' })).toBeChecked()
+
+  // no port filters
+  const portFilters = modal.getByRole('table', { name: 'Port filters' })
+  await expect(portFilters).toBeHidden()
+
+  const targets = modal.getByRole('table', { name: 'Targets' })
+  await expect(targets.getByRole('row', { name: 'Name: default, Type: vpc' })).toBeVisible()
+
+  // close the modal
+  await page.keyboard.press('Escape')
+  await expect(modal).toBeHidden()
+
+  // do it again with a different rule
+  await clickRowAction(page, 'allow-ssh', 'Clone')
 
-  await expect(page).toHaveURL(url + '-new/allow-rdp')
   await expect(modal).toBeVisible()
   await expect(modal.getByRole('textbox', { name: 'Name', exact: true })).toHaveValue(
-    'allow-rdp-copy'
+    'allow-ssh-copy'
   )
 
+  await expect(portFilters.getByRole('cell', { name: '22', exact: true })).toBeVisible()
+
   await expect(modal.getByRole('checkbox', { name: 'TCP' })).toBeChecked()
   await expect(modal.getByRole('checkbox', { name: 'UDP' })).not.toBeChecked()
   await expect(modal.getByRole('checkbox', { name: 'ICMP' })).not.toBeChecked()
 
-  await expect(
-    modal
-      .getByRole('table', { name: 'Port filters' })
-      .getByRole('cell', { name: '3389', exact: true })
-  ).toBeVisible()
-  await expect(
-    modal
-      .getByRole('table', { name: 'Targets' })
-      .getByRole('row', { name: 'Name: default, Type: vpc' })
-  ).toBeVisible()
+  await expect(targets.getByRole('row', { name: 'Name: default, Type: vpc' })).toBeVisible()
 })
 
 const rulePath = '/projects/mock-project/vpcs/mock-vpc/firewall-rules/allow-icmp/edit'