From c174d8fce0510b9c65aec207771c3e4af4ea7d47 Mon Sep 17 00:00:00 2001
From: David Crespo <david.crespo@oxidecomputer.com>
Date: Mon, 26 Aug 2024 16:54:28 -0500
Subject: [PATCH] more detailed comments about API logic we're imitating

---
 mock-api/msw/db.ts       | 27 +++++++++++++++++++--------
 mock-api/msw/handlers.ts | 17 ++++++++++++-----
 mock-api/msw/util.ts     |  7 +++++++
 3 files changed, 38 insertions(+), 13 deletions(-)

diff --git a/mock-api/msw/db.ts b/mock-api/msw/db.ts
index 423df85e9..e9616295d 100644
--- a/mock-api/msw/db.ts
+++ b/mock-api/msw/db.ts
@@ -8,7 +8,6 @@
 // note that isUuid checks for any kind of UUID. strictly speaking, we should
 // only be checking for v4
 import * as R from 'remeda'
-import { validate as isUuid } from 'uuid'
 
 import type { ApiTypes as Api, PathParams as PP } from '@oxide/api'
 import * as mock from '@oxide/api-mocks'
@@ -17,7 +16,7 @@ import { json } from '~/api/__generated__/msw-handlers'
 import { commaSeries } from '~/util/str'
 
 import type { Json } from '../json-type'
-import { internalError } from './util'
+import { internalError, isUuid } from './util'
 
 const notFoundBody = { error_code: 'ObjectNotFound' } as const
 export type NotFound = typeof notFoundBody
@@ -219,21 +218,33 @@ export const lookup = {
   image({ image: id, project: projectId }: PP.Image): Json<Api.Image> {
     if (!id) throw notFoundErr('no image specified')
 
+    // We match the API logic:
+    //
+    //   match image_selector {
+    //     (image ID, no project) =>
+    //       look up image in DB by ID
+    //       if project ID is present, it's a project image, otherwise silo image
+    //     (image Name, project specified) => it's a project image
+    //     (image Name, no project) => it's a silo image
+    //     (image ID, project specified) => error
+    //   }
+    //
+    // https://github.com/oxidecomputer/omicron/blob/219121a/nexus/src/app/image.rs#L32-L76
+
     if (isUuid(id)) {
+      // this matches the error case above
       ensureNoParentSelectors('image', { project: projectId })
       return lookupById(db.images, id)
     }
 
-    // TODO: is this logic right? seems kinda weird. you should be able to look
-    // up either kind by ID. we should probably have two lookup functions
     let image: Json<Api.Image> | undefined
-    if (projectId === undefined) {
-      // silo image
-      image = db.images.find((d) => d.project_id === undefined && d.name === id)
-    } else {
+    if (projectId) {
       // project image
       const project = lookup.project({ project: projectId })
       image = db.images.find((d) => d.project_id === project.id && d.name === id)
+    } else {
+      // silo image
+      image = db.images.find((d) => d.project_id === undefined && d.name === id)
     }
 
     if (!image) throw notFoundErr(`image '${id}'`)
diff --git a/mock-api/msw/handlers.ts b/mock-api/msw/handlers.ts
index b58f96982..8da669534 100644
--- a/mock-api/msw/handlers.ts
+++ b/mock-api/msw/handlers.ts
@@ -47,6 +47,7 @@ import {
   handleMetrics,
   ipInAnyRange,
   ipRangeLen,
+  isUuid,
   NotImplemented,
   paginated,
   requireFleetCollab,
@@ -293,9 +294,14 @@ export const handlers = makeHandlers({
     if (dbFloatingIp.instance_id) {
       throw 'floating IP cannot be attached to one instance while still attached to another'
     }
-    // TODO: make sure the logic around when project is passed matches
-    // the API
-    const dbInstance = lookup.instance({ instance: body.parent })
+    // Following the API logic here, which says that when the instance is passed
+    // by name, we pull the project ID off the floating IP.
+    //
+    // https://github.com/oxidecomputer/omicron/blob/e434307/nexus/src/app/external_ip.rs#L171-L201
+    const dbInstance = lookup.instance({
+      instance: body.parent,
+      project: isUuid(body.parent) ? undefined : project,
+    })
     dbFloatingIp.instance_id = dbInstance.id
 
     return dbFloatingIp
@@ -362,8 +368,9 @@ export const handlers = makeHandlers({
     return json(image, { status: 202 })
   },
   imageDemote({ path: { image }, query: { project } }) {
-    // TODO: change this to lookup.siloImage when I add that. or at least
-    // check API logic to make sure we match it
+    // unusual case because the project is never used to resolve the image. you
+    // can only demote silo images, and whether we have an image name or ID, if
+    // there is no project specified, the lookup assumes it's a silo image
     const dbImage = lookup.image({ image })
     const dbProject = lookup.project({ project })
 
diff --git a/mock-api/msw/util.ts b/mock-api/msw/util.ts
index bcc864712..658cdf0ae 100644
--- a/mock-api/msw/util.ts
+++ b/mock-api/msw/util.ts
@@ -9,6 +9,7 @@ import { differenceInSeconds, subHours } from 'date-fns'
 // Works without the .js for dev server and prod build in MSW mode, but
 // playwright wants the .js. No idea why, let's just add the .js.
 import { IPv4, IPv6 } from 'ip-num/IPNumber.js'
+import { validate as uuidValidate, version as uuidVersion } from 'uuid'
 
 import {
   FLEET_ID,
@@ -413,3 +414,9 @@ export function updateDesc(
     resource.description = update.description
   }
 }
+
+/**
+ * Check if something is a v4 UUID. Implementation follows uuid readme:
+ * https://github.com/uuidjs/uuid/blob/9e69599/README.md#L407-L422
+ */
+export const isUuid = (s: string) => uuidValidate(s) && uuidVersion(s) === 4