Display external IPs on primary network interface (#1070)

* stub out basics

* display external IPs on primary network interface

* assert rows with an object instead of an array

* point to omicron main now that the ResultsPage change is merged

Author: david-crespo

.github/workflows/lintBuildTest.yml

@@ -46,4 +46,4 @@ jobs:
       - name: Install Playwright
         run: npx playwright install --with-deps
       - name: Run Playwright tests
-        run: yarn playwright test
+        run: yarn playwright test --workers=3

OMICRON_VERSION

@@ -1 +1 @@
-5e835f19680655a89ff1dca2a6e18f1f269ac021
+6d8d6a4580db44a885f7a213aa39ada76d8af2d6

app/pages/__tests__/instance/networking.e2e.ts

@@ -9,14 +9,9 @@ test('Instance networking tab', async ({ page }) => {
 
   // Instance networking tab
   await page.click('role=tab[name="Networking"]')
-  await expectRowVisible(page, 'my-nic', [
-    'my-nic',
-    'a network interface',
-    '172.30.0.10',
-    'mock-vpc',
-    'mock-subnet',
-    'primary',
-  ])
+
+  const table = page.locator('table')
+  await expectRowVisible(table, { name: 'my-nic', primary: 'primary' })
 
   // check VPC link in table points to the right page
   await expect(page.locator('role=cell >> role=link[name="mock-vpc"]')).toHaveAttribute(
@@ -54,15 +49,8 @@ test('Instance networking tab', async ({ page }) => {
     .locator('role=button[name="Row actions"]')
     .click()
   await page.click('role=menuitem[name="Make primary"]')
-  await expectRowVisible(page, 'my-nic', [
-    'my-nic',
-    'a network interface',
-    '172.30.0.10',
-    'mock-vpc',
-    'mock-subnet',
-    '',
-  ])
-  await expectRowVisible(page, 'nic-2', ['nic-2', null, null, null, null, 'primary'])
+  await expectRowVisible(table, { name: 'my-nic', primary: '' })
+  await expectRowVisible(table, { name: 'nic-2', primary: 'primary' })
 
   // Make an edit to the network interface
   await page

app/pages/__tests__/org-access.e2e.ts

@@ -5,13 +5,15 @@ import { expectNotVisible, expectRowVisible, expectVisible } from 'app/util/e2e'
 test('Click through org access page', async ({ page }) => {
   await page.goto('/orgs/maze-war')
 
-  // page is there, we see AL but not FDR
+  const table = page.locator('role=table')
+
+  // page is there, we see user 1 but not 2
   await page.click('role=link[name*="Access & IAM"]')
   await expectVisible(page, ['role=heading[name*="Access & IAM"]'])
-  await expectRowVisible(page, 'user-1', ['user-1', 'Hannah Arendt', 'admin'])
+  await expectRowVisible(table, { ID: 'user-1', Name: 'Hannah Arendt', Role: 'admin' })
   await expectNotVisible(page, ['role=cell[name="user-2"]'])
 
-  // Add FDR as collab
+  // Add user 2 as collab
   await page.click('role=button[name="Add user to organization"]')
   await expectVisible(page, ['role=heading[name*="Add user to organization"]'])
 
@@ -32,10 +34,10 @@ test('Click through org access page', async ({ page }) => {
   await page.click('role=option[name="Collaborator"]')
   await page.click('role=button[name="Add user"]')
 
-  // FDR shows up in the table
-  await expectRowVisible(page, 'user-2', ['user-2', 'Hans Jonas', 'collaborator'])
+  // User 2 shows up in the table
+  await expectRowVisible(table, { ID: 'user-2', Name: 'Hans Jonas', Role: 'collaborator' })
 
-  // now change FDR's role from collab to viewer
+  // now change user 2's role from collab to viewer
   await page
     .locator('role=row', { hasText: 'user-2' })
     .locator('role=button[name="Row actions"]')
@@ -49,9 +51,9 @@ test('Click through org access page', async ({ page }) => {
   await page.click('role=option[name="Viewer"]')
   await page.click('role=button[name="Update role"]')
 
-  await expectRowVisible(page, 'user-2', ['user-2', 'Hans Jonas', 'viewer'])
+  await expectRowVisible(table, { ID: 'user-2', Role: 'viewer' })
 
-  // now delete FDR
+  // now delete user 2
   await page
     .locator('role=row', { hasText: 'user-2' })
     .locator('role=button[name="Row actions"]')

app/pages/__tests__/project-access.e2e.ts

@@ -4,14 +4,15 @@ import { expectNotVisible, expectRowVisible, expectVisible } from 'app/util/e2e'
 
 test('Click through project access page', async ({ page }) => {
   await page.goto('/orgs/maze-war/projects/mock-project')
-
-  // page is there, we see AL but not FDR
   await page.click('role=link[name*="Access & IAM"]')
+
+  // page is there, we see user 1 but not 2
   await expectVisible(page, ['role=heading[name*="Access & IAM"]'])
-  await expectRowVisible(page, 'user-1', ['user-1', 'Hannah Arendt', 'admin'])
+  const table = page.locator('table')
+  await expectRowVisible(table, { ID: 'user-1', Name: 'Hannah Arendt', Role: 'admin' })
   await expectNotVisible(page, ['role=cell[name="user-2"]'])
 
-  // Add FDR as collab
+  // Add user 2 as collab
   await page.click('role=button[name="Add user to project"]')
   await expectVisible(page, ['role=heading[name*="Add user to project"]'])
 
@@ -32,10 +33,10 @@ test('Click through project access page', async ({ page }) => {
   await page.click('role=option[name="Collaborator"]')
   await page.click('role=button[name="Add user"]')
 
-  // FDR shows up in the table
-  await expectRowVisible(page, 'user-2', ['user-2', 'Hans Jonas', 'collaborator'])
+  // User 2 shows up in the table
+  await expectRowVisible(table, { ID: 'user-2', Name: 'Hans Jonas', Role: 'collaborator' })
 
-  // now change FDR's role from collab to viewer
+  // now change user 2 role from collab to viewer
   await page
     .locator('role=row', { hasText: 'user-2' })
     .locator('role=button[name="Row actions"]')
@@ -49,9 +50,9 @@ test('Click through project access page', async ({ page }) => {
   await page.click('role=option[name="Viewer"]')
   await page.click('role=button[name="Update role"]')
 
-  await expectRowVisible(page, 'user-2', ['user-2', 'Hans Jonas', 'viewer'])
+  await expectRowVisible(table, { ID: 'user-2', Role: 'viewer' })
 
-  // now delete FDR
+  // now delete user 2
   await page
     .locator('role=row', { hasText: 'user-2' })
     .locator('role=button[name="Row actions"]')

app/pages/__tests__/ssh-keys.e2e.ts

@@ -18,7 +18,8 @@ test('SSH keys', async ({ page }) => {
 
   // it's there in the table
   await expectNotVisible(page, ['text="No SSH keys"'])
-  await expectRowVisible(page, 'my-key', ['my-key', 'definitely a key'])
+  const table = page.locator('role=table')
+  await expectRowVisible(table, { Name: 'my-key', Description: 'definitely a key' })
 
   // now delete it
   await page.click('role=button[name="Row actions"]')

app/pages/project/instances/instance/tabs/NetworkingTab.tsx

@@ -39,6 +39,13 @@ const SubnetNameFromId = ({ value }: { value: string }) => (
   </span>
 )
 
+function ExternalIpsFromInstanceName({ value: primary }: { value: boolean }) {
+  const instanceParams = useParams('orgName', 'projectName', 'instanceName')
+  const { data } = useApiQuery('instanceExternalIpList', instanceParams)
+  const ips = data?.items.map((eip) => eip.ip).join(', ')
+  return <span className="text-default">{primary ? ips : <>&mdash;</>}</span>
+}
+
 export function NetworkingTab() {
   const instanceParams = useParams('orgName', 'projectName', 'instanceName')
   const queryClient = useApiQueryClient()
@@ -119,6 +126,12 @@ export function NetworkingTab() {
         <Column accessor="description" />
         {/* TODO: mark v4 or v6 explicitly? */}
         <Column accessor="ip" />
+        <Column
+          header="External IP"
+          // we use primary to decide whether to show the IP in that row
+          accessor="primary"
+          cell={ExternalIpsFromInstanceName}
+        />
         <Column header="vpc" accessor="vpcId" cell={VpcNameFromId} />
         <Column header="subnet" accessor="subnetId" cell={SubnetNameFromId} />
         <Column

app/util/e2e.ts

@@ -1,13 +1,24 @@
 import type { Locator, Page } from '@playwright/test'
 import { expect } from '@playwright/test'
 
-export async function forEach(loc: Locator, fn: (loc0: Locator) => void) {
+export async function forEach(loc: Locator, fn: (loc0: Locator, i: number) => void) {
   const count = await loc.count()
   for (let i = 0; i < count; i++) {
-    await fn(loc.nth(i))
+    await fn(loc.nth(i), i)
   }
 }
 
+export async function map<T>(
+  loc: Locator,
+  fn: (loc0: Locator, i: number) => Promise<T>
+): Promise<T[]> {
+  const result: T[] = []
+  await forEach(loc, async (loc0, i) => {
+    result.push(await fn(loc0, i))
+  })
+  return result
+}
+
 export async function expectVisible(page: Page, selectors: string[]) {
   for (const selector of selectors) {
     await expect(page.locator(selector)).toBeVisible()
@@ -21,26 +32,36 @@ export async function expectNotVisible(page: Page, selectors: string[]) {
 }
 
 /**
- * Assert about the values of a row, identified by `rowSelectorText`. It doesn't
- * need to be the entire row; the test will pass as long as the identified row
- * exists and the first N cells match the N values in `cellTexts`. Pass `''` for
- * a checkbox cell.
- *
- * @param rowSelectorText Text that should uniquely identify the row, like an ID
- * @param cellTexts Text to match in each cell of that row
+ * Assert that a row matching `expectedRow` is present in `table`. The match
+ * uses `objectContaining`, so `expectedRow` does not need to contain every
+ * cell. Works by converting `table` to a list of objects where the keys are
+ * header cell text and the values are row cell text.
  */
 export async function expectRowVisible(
-  page: Page,
-  rowSelectorText: string,
-  cellTexts: Array<string | null>
+  table: Locator,
+  expectedRow: Record<string, string>
 ) {
-  const row = page.locator(`tr:has-text("${rowSelectorText}")`)
-  await expect(row).toBeVisible()
-  for (let i = 0; i < cellTexts.length; i++) {
-    const text = cellTexts[i]
-    if (text === null) {
-      continue
-    }
-    await expect(row.locator(`role=cell >> nth=${i}`)).toHaveText(text)
-  }
+  // wait for header and rows to avoid flake town
+  const headerLoc = table.locator('thead >> role=cell')
+  await headerLoc.locator('nth=0').waitFor() // nth=0 bc error if there's more than 1
+
+  const rowLoc = table.locator('tbody >> role=row')
+  await rowLoc.locator('nth=0').waitFor()
+
+  const headerKeys = await map(
+    table.locator('thead >> role=cell'),
+    async (cell) => await cell.textContent()
+  )
+
+  const rows = await map(table.locator('tbody >> role=row'), async (row) => {
+    const rowPairs = await map(row.locator('role=cell'), async (cell, i) => [
+      headerKeys[i],
+      // accessible name would be better but it's not in yet
+      // https://github.com/microsoft/playwright/issues/13517
+      await cell.textContent(),
+    ])
+    return Object.fromEntries(rowPairs.filter(([k]) => k && k.length > 0))
+  })
+
+  await expect(rows).toEqual(expect.arrayContaining([expect.objectContaining(expectedRow)]))
 }

libs/api-mocks/msw/handlers.ts

@@ -462,6 +462,22 @@ export const handlers = [
     }
   ),
 
+  rest.get<never, InstanceParams, Json<Api.ExternalIpResultsPage> | GetErr>(
+    '/api/organizations/:orgName/projects/:projectName/instances/:instanceName/external-ips',
+    (req, res) => {
+      const [, err] = lookupInstance(req.params)
+      if (err) return res(err)
+      // TODO: proper mock table
+      const items = [
+        {
+          ip: '123.4.56.7',
+          kind: 'ephemeral',
+        } as const,
+      ]
+      return res(json({ items }))
+    }
+  ),
+
   rest.get<never, InstanceParams, Json<Api.NetworkInterfaceResultsPage> | GetErr>(
     '/api/organizations/:orgName/projects/:projectName/instances/:instanceName/network-interfaces',
     (req, res) => {