From f0fdcdb982b0006259525070de4526d5a947d328 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       | 24 ++++++++++++++++++------
 mock-api/msw/handlers.ts | 18 ++++++++++++------
 2 files changed, 30 insertions(+), 12 deletions(-)

diff --git a/mock-api/msw/db.ts b/mock-api/msw/db.ts
index 423df85e9..0dcd648c3 100644
--- a/mock-api/msw/db.ts
+++ b/mock-api/msw/db.ts
@@ -219,21 +219,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..8089c8351 100644
--- a/mock-api/msw/handlers.ts
+++ b/mock-api/msw/handlers.ts
@@ -7,7 +7,7 @@
  */
 import { delay } from 'msw'
 import * as R from 'remeda'
-import { v4 as uuid } from 'uuid'
+import { validate as isUuid, v4 as uuid } from 'uuid'
 
 import {
   diskCan,
@@ -293,9 +293,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 +367,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 })