Skip to content

Commit 681355c

Browse files
david-crespodavid-crespo
authored
Mock API: error if parent selectors present when resource is fetched by ID (#2396)
* error if parent selectors given when resources are fetched by ID * more detailed comments about API logic we're imitating
1 parent 286b8d2 commit 681355c

File tree

2 files changed

+106
-32
lines changed

2 files changed

+106
-32
lines changed

‎mock-api/msw/db.ts‎

Lines changed: 84 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -24,17 +24,33 @@ export const notFoundErr = (msg: string) => {
2424
return json({ error_code: 'ObjectNotFound', message } as const, { status: 404 })
2525
}
2626

27-
export const badSelectorErr = (resource: string, parents: string[]) => {
28-
const message = `when ${resource} is specified by ID, ${commaSeries(parents, 'and')} should not be specified`
29-
return json({ error_code: 'InvalidRequest', message }, { status: 400 })
30-
}
31-
3227
export const lookupById = <T extends { id: string }>(table: T[], id: string) => {
3328
const item = table.find((i) => i.id === id)
3429
if (!item) throw notFoundErr(`by id ${id}`)
3530
return item
3631
}
3732

33+
/**
34+
* Given an object representing (potentially) parent selectors for a resource,
35+
* throw an error if any of the keys in that object have truthy values. For
36+
* example, if selecting an instance by ID, we would pass in an object with
37+
* `project` as the key and error out if and only if `parentSelector.project`
38+
* is present.
39+
*/
40+
function ensureNoParentSelectors(
41+
/** Resource name to be used in error message */
42+
resourceLabel: string,
43+
parentSelector: Record<string, string | undefined>
44+
) {
45+
const keysWithValues = Object.entries(parentSelector)
46+
.filter(([_, v]) => v)
47+
.map(([k]) => k)
48+
if (keysWithValues.length > 0) {
49+
const message = `when ${resourceLabel} is specified by ID, ${commaSeries(keysWithValues, 'and')} should not be specified`
50+
throw json({ error_code: 'InvalidRequest', message }, { status: 400 })
51+
}
52+
}
53+
3854
export const getIpFromPool = (poolName: string | undefined) => {
3955
const pool = lookup.ipPool({ pool: poolName })
4056
const ipPoolRange = db.ipPoolRanges.find((range) => range.ip_pool_id === pool.id)
@@ -61,7 +77,10 @@ export const lookup = {
6177
instance({ instance: id, ...projectSelector }: PP.Instance): Json<Api.Instance> {
6278
if (!id) throw notFoundErr('no instance specified')
6379

64-
if (isUuid(id)) return lookupById(db.instances, id)
80+
if (isUuid(id)) {
81+
ensureNoParentSelectors('instance', projectSelector)
82+
return lookupById(db.instances, id)
83+
}
6584

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

78-
if (isUuid(id)) return lookupById(db.networkInterfaces, id)
97+
if (isUuid(id)) {
98+
ensureNoParentSelectors('network interface', instanceSelector)
99+
return lookupById(db.networkInterfaces, id)
100+
}
79101

80102
const instance = lookup.instance(instanceSelector)
81103

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

92-
if (isUuid(id)) return lookupById(db.disks, id)
114+
if (isUuid(id)) {
115+
ensureNoParentSelectors('disk', projectSelector)
116+
return lookupById(db.disks, id)
117+
}
93118

94119
const project = lookup.project(projectSelector)
95120

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

104129
if (isUuid(id)) {
105-
if (projectSelector.project) throw badSelectorErr('floating IP', ['project'])
130+
ensureNoParentSelectors('floating IP', projectSelector)
106131
return lookupById(db.floatingIps, id)
107132
}
108133

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

120-
if (isUuid(id)) return lookupById(db.snapshots, id)
145+
if (isUuid(id)) {
146+
ensureNoParentSelectors('snapshot', projectSelector)
147+
return lookupById(db.snapshots, id)
148+
}
121149

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

131-
if (isUuid(id)) return lookupById(db.vpcs, id)
159+
if (isUuid(id)) {
160+
ensureNoParentSelectors('vpc', projectSelector)
161+
return lookupById(db.vpcs, id)
162+
}
132163

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

142-
if (isUuid(id)) return lookupById(db.vpcRouters, id)
173+
if (isUuid(id)) {
174+
ensureNoParentSelectors('router', vpcSelector)
175+
return lookupById(db.vpcRouters, id)
176+
}
143177

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

156-
if (isUuid(id)) return lookupById(db.vpcRouterRoutes, id)
190+
if (isUuid(id)) {
191+
ensureNoParentSelectors('route', routerSelector)
192+
return lookupById(db.vpcRouterRoutes, id)
193+
}
157194

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

169-
if (isUuid(id)) return lookupById(db.vpcSubnets, id)
206+
if (isUuid(id)) {
207+
ensureNoParentSelectors('subnet', vpcSelector)
208+
return lookupById(db.vpcSubnets, id)
209+
}
170210

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

180-
if (isUuid(id)) return lookupById(db.images, id)
220+
// We match the API logic:
221+
//
222+
// match image_selector {
223+
// (image ID, no project) =>
224+
// look up image in DB by ID
225+
// if project ID is present, it's a project image, otherwise silo image
226+
// (image Name, project specified) => it's a project image
227+
// (image Name, no project) => it's a silo image
228+
// (image ID, project specified) => error
229+
// }
230+
//
231+
// https://github.com/oxidecomputer/omicron/blob/219121a/nexus/src/app/image.rs#L32-L76
232+
233+
if (isUuid(id)) {
234+
// this matches the error case above
235+
ensureNoParentSelectors('image', { project: projectId })
236+
return lookupById(db.images, id)
237+
}
181238

182239
let image: Json<Api.Image> | undefined
183-
if (projectId === undefined) {
184-
// silo image
185-
image = db.images.find((d) => d.project_id === undefined && d.name === id)
186-
} else {
240+
if (projectId) {
187241
// project image
188242
const project = lookup.project({ project: projectId })
189243
image = db.images.find((d) => d.project_id === project.id && d.name === id)
244+
} else {
245+
// silo image
246+
image = db.images.find((d) => d.project_id === undefined && d.name === id)
190247
}
191248

192249
if (!image) throw notFoundErr(`image '${id}'`)
@@ -262,8 +319,15 @@ export const lookup = {
262319
samlIdp({ provider: id, silo }: PP.IdentityProvider): Json<Api.SamlIdentityProvider> {
263320
if (!id) throw notFoundErr('no IdP specified')
264321

265-
const dbSilo = lookup.silo({ silo })
322+
if (isUuid(id)) {
323+
ensureNoParentSelectors('identity provider', { silo })
324+
return lookupById(
325+
db.identityProviders.filter((o) => o.type === 'saml').map((o) => o.provider),
326+
id
327+
)
328+
}
266329

330+
const dbSilo = lookup.silo({ silo })
267331
const dbIdp = db.identityProviders.find(
268332
({ type, siloId, provider }) =>
269333
type === 'saml' && siloId === dbSilo.id && provider.name === id

‎mock-api/msw/handlers.ts‎

Lines changed: 22 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@
77
*/
88
import { delay } from 'msw'
99
import * as R from 'remeda'
10-
import { v4 as uuid } from 'uuid'
10+
import { validate as isUuid, v4 as uuid } from 'uuid'
1111

1212
import {
1313
diskCan,
@@ -288,15 +288,22 @@ export const handlers = makeHandlers({
288288

289289
return 204
290290
},
291-
floatingIpAttach({ path, query, body }) {
292-
const floatingIp = lookup.floatingIp({ ...path, ...query })
293-
if (floatingIp.instance_id) {
291+
floatingIpAttach({ path: { floatingIp }, query: { project }, body }) {
292+
const dbFloatingIp = lookup.floatingIp({ floatingIp, project })
293+
if (dbFloatingIp.instance_id) {
294294
throw 'floating IP cannot be attached to one instance while still attached to another'
295295
}
296-
const instance = lookup.instance({ ...path, ...query, instance: body.parent })
297-
floatingIp.instance_id = instance.id
296+
// Following the API logic here, which says that when the instance is passed
297+
// by name, we pull the project ID off the floating IP.
298+
//
299+
// https://github.com/oxidecomputer/omicron/blob/e434307/nexus/src/app/external_ip.rs#L171-L201
300+
const dbInstance = lookup.instance({
301+
instance: body.parent,
302+
project: isUuid(body.parent) ? undefined : project,
303+
})
304+
dbFloatingIp.instance_id = dbInstance.id
298305

299-
return floatingIp
306+
return dbFloatingIp
300307
},
301308
floatingIpDetach({ path, query }) {
302309
const floatingIp = lookup.floatingIp({ ...path, ...query })
@@ -359,13 +366,16 @@ export const handlers = makeHandlers({
359366

360367
return json(image, { status: 202 })
361368
},
362-
imageDemote({ path, query }) {
363-
const image = lookup.image({ ...path, ...query })
364-
const project = lookup.project({ ...path, ...query })
369+
imageDemote({ path: { image }, query: { project } }) {
370+
// unusual case because the project is never used to resolve the image. you
371+
// can only demote silo images, and whether we have an image name or ID, if
372+
// there is no project specified, the lookup assumes it's a silo image
373+
const dbImage = lookup.image({ image })
374+
const dbProject = lookup.project({ project })
365375

366-
image.project_id = project.id
376+
dbImage.project_id = dbProject.id
367377

368-
return json(image, { status: 202 })
378+
return json(dbImage, { status: 202 })
369379
},
370380
instanceList({ query }) {
371381
const project = lookup.project(query)

0 commit comments

Comments
 (0)