Skip to content

Commit

Permalink
Mock API: error if parent selectors present when resource is fetched …
Browse files Browse the repository at this point in the history 
…by ID (#2396)

* error if parent selectors given when resources are fetched by ID

* more detailed comments about API logic we're imitating
  • Loading branch information
@david-crespo
david-crespo authored Aug 29, 2024
1 parent 286b8d2 commit 681355c
Show file tree
Hide file tree
Showing 2 changed files with 106 additions and 32 deletions.
104 changes: 84 additions & 20 deletions mock-api/msw/db.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -24,17 +24,33 @@ export const notFoundErr = (msg: string) => {
return json({ error_code: 'ObjectNotFound', message } as const, { status: 404 })
}

export const badSelectorErr = (resource: string, parents: string[]) => {
const message = `when ${resource} is specified by ID, ${commaSeries(parents, 'and')} should not be specified`
return json({ error_code: 'InvalidRequest', message }, { status: 400 })
}

export const lookupById = <T extends { id: string }>(table: T[], id: string) => {
const item = table.find((i) => i.id === id)
if (!item) throw notFoundErr(`by id ${id}`)
return item
}

/**
* Given an object representing (potentially) parent selectors for a resource,
* throw an error if any of the keys in that object have truthy values. For
* example, if selecting an instance by ID, we would pass in an object with
* `project` as the key and error out if and only if `parentSelector.project`
* is present.
*/
function ensureNoParentSelectors(
/** Resource name to be used in error message */
resourceLabel: string,
parentSelector: Record<string, string | undefined>
) {
const keysWithValues = Object.entries(parentSelector)
.filter(([_, v]) => v)
.map(([k]) => k)
if (keysWithValues.length > 0) {
const message = `when ${resourceLabel} is specified by ID, ${commaSeries(keysWithValues, 'and')} should not be specified`
throw json({ error_code: 'InvalidRequest', message }, { status: 400 })
}
}

export const getIpFromPool = (poolName: string | undefined) => {
const pool = lookup.ipPool({ pool: poolName })
const ipPoolRange = db.ipPoolRanges.find((range) => range.ip_pool_id === pool.id)
Expand All @@ -61,7 +77,10 @@ export const lookup = {
instance({ instance: id, ...projectSelector }: PP.Instance): Json<Api.Instance> {
if (!id) throw notFoundErr('no instance specified')

if (isUuid(id)) return lookupById(db.instances, id)
if (isUuid(id)) {
ensureNoParentSelectors('instance', projectSelector)
return lookupById(db.instances, id)
}

const project = lookup.project(projectSelector)
const instance = db.instances.find((i) => i.project_id === project.id && i.name === id)
Expand All @@ -75,7 +94,10 @@ export const lookup = {
}: PP.NetworkInterface): Json<Api.InstanceNetworkInterface> {
if (!id) throw notFoundErr('no NIC specified')

if (isUuid(id)) return lookupById(db.networkInterfaces, id)
if (isUuid(id)) {
ensureNoParentSelectors('network interface', instanceSelector)
return lookupById(db.networkInterfaces, id)
}

const instance = lookup.instance(instanceSelector)

Expand All @@ -89,7 +111,10 @@ export const lookup = {
disk({ disk: id, ...projectSelector }: PP.Disk): Json<Api.Disk> {
if (!id) throw notFoundErr('no disk specified')

if (isUuid(id)) return lookupById(db.disks, id)
if (isUuid(id)) {
ensureNoParentSelectors('disk', projectSelector)
return lookupById(db.disks, id)
}

const project = lookup.project(projectSelector)

Expand All @@ -102,7 +127,7 @@ export const lookup = {
if (!id) throw notFoundErr('no floating IP specified')

if (isUuid(id)) {
if (projectSelector.project) throw badSelectorErr('floating IP', ['project'])
ensureNoParentSelectors('floating IP', projectSelector)
return lookupById(db.floatingIps, id)
}

Expand All @@ -117,7 +142,10 @@ export const lookup = {
snapshot({ snapshot: id, ...projectSelector }: PP.Snapshot): Json<Api.Snapshot> {
if (!id) throw notFoundErr('no snapshot specified')

if (isUuid(id)) return lookupById(db.snapshots, id)
if (isUuid(id)) {
ensureNoParentSelectors('snapshot', projectSelector)
return lookupById(db.snapshots, id)
}

const project = lookup.project(projectSelector)
const snapshot = db.snapshots.find((i) => i.project_id === project.id && i.name === id)
Expand All @@ -128,7 +156,10 @@ export const lookup = {
vpc({ vpc: id, ...projectSelector }: PP.Vpc): Json<Api.Vpc> {
if (!id) throw notFoundErr('no VPC specified')

if (isUuid(id)) return lookupById(db.vpcs, id)
if (isUuid(id)) {
ensureNoParentSelectors('vpc', projectSelector)
return lookupById(db.vpcs, id)
}

const project = lookup.project(projectSelector)
const vpc = db.vpcs.find((v) => v.project_id === project.id && v.name === id)
Expand All @@ -139,7 +170,10 @@ export const lookup = {
vpcRouter({ router: id, ...vpcSelector }: PP.VpcRouter): Json<Api.VpcRouter> {
if (!id) throw notFoundErr('no router specified')

if (isUuid(id)) return lookupById(db.vpcRouters, id)
if (isUuid(id)) {
ensureNoParentSelectors('router', vpcSelector)
return lookupById(db.vpcRouters, id)
}

const vpc = lookup.vpc(vpcSelector)
const router = db.vpcRouters.find((r) => r.vpc_id === vpc.id && r.name === id)
Expand All @@ -153,7 +187,10 @@ export const lookup = {
}: PP.VpcRouterRoute): Json<Api.RouterRoute> {
if (!id) throw notFoundErr('no route specified')

if (isUuid(id)) return lookupById(db.vpcRouterRoutes, id)
if (isUuid(id)) {
ensureNoParentSelectors('route', routerSelector)
return lookupById(db.vpcRouterRoutes, id)
}

const router = lookup.vpcRouter(routerSelector)
const route = db.vpcRouterRoutes.find(
Expand All @@ -166,7 +203,10 @@ export const lookup = {
vpcSubnet({ subnet: id, ...vpcSelector }: PP.VpcSubnet): Json<Api.VpcSubnet> {
if (!id) throw notFoundErr('no subnet specified')

if (isUuid(id)) return lookupById(db.vpcSubnets, id)
if (isUuid(id)) {
ensureNoParentSelectors('subnet', vpcSelector)
return lookupById(db.vpcSubnets, id)
}

const vpc = lookup.vpc(vpcSelector)
const subnet = db.vpcSubnets.find((s) => s.vpc_id === vpc.id && s.name === id)
Expand All @@ -177,16 +217,33 @@ export const lookup = {
image({ image: id, project: projectId }: PP.Image): Json<Api.Image> {
if (!id) throw notFoundErr('no image specified')

if (isUuid(id)) return lookupById(db.images, id)
// 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)
}

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}'`)
Expand Down Expand Up @@ -262,8 +319,15 @@ export const lookup = {
samlIdp({ provider: id, silo }: PP.IdentityProvider): Json<Api.SamlIdentityProvider> {
if (!id) throw notFoundErr('no IdP specified')

const dbSilo = lookup.silo({ silo })
if (isUuid(id)) {
ensureNoParentSelectors('identity provider', { silo })
return lookupById(
db.identityProviders.filter((o) => o.type === 'saml').map((o) => o.provider),
id
)
}

const dbSilo = lookup.silo({ silo })
const dbIdp = db.identityProviders.find(
({ type, siloId, provider }) =>
type === 'saml' && siloId === dbSilo.id && provider.name === id
Expand Down
34 changes: 22 additions & 12 deletions mock-api/msw/handlers.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -288,15 +288,22 @@ export const handlers = makeHandlers({

return 204
},
floatingIpAttach({ path, query, body }) {
const floatingIp = lookup.floatingIp({ ...path, ...query })
if (floatingIp.instance_id) {
floatingIpAttach({ path: { floatingIp }, query: { project }, body }) {
const dbFloatingIp = lookup.floatingIp({ floatingIp, project })
if (dbFloatingIp.instance_id) {
throw 'floating IP cannot be attached to one instance while still attached to another'
}
const instance = lookup.instance({ ...path, ...query, instance: body.parent })
floatingIp.instance_id = instance.id
// 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 floatingIp
return dbFloatingIp
},
floatingIpDetach({ path, query }) {
const floatingIp = lookup.floatingIp({ ...path, ...query })
Expand Down Expand Up @@ -359,13 +366,16 @@ export const handlers = makeHandlers({

return json(image, { status: 202 })
},
imageDemote({ path, query }) {
const image = lookup.image({ ...path, ...query })
const project = lookup.project({ ...path, ...query })
imageDemote({ path: { image }, query: { project } }) {
// 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 })

image.project_id = project.id
dbImage.project_id = dbProject.id

return json(image, { status: 202 })
return json(dbImage, { status: 202 })
},
instanceList({ query }) {
const project = lookup.project(query)
Expand Down

0 comments on commit 681355c

Please sign in to comment.