Skip to content

Commit 42bd2cc

Browse files
andriypolanskiandriy-polanskicursoragent
authored
fix(control-plane): product-scope TenantRegistry keys (#8024) (#8029)
Key registry storage and HTTP conflict/delete lookups by ${product}:${name} so same-named ORB and AMS tenants stay independent. Co-authored-by: Andriy Polanski <andriy.polanski@gmail.com> Co-authored-by: Cursor <cursoragent@cursor.com>
1 parent 0cd7dc4 commit 42bd2cc

4 files changed

Lines changed: 146 additions & 47 deletions

File tree

control-plane/src/http-app.ts

Lines changed: 14 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
// The real HTTP transport for control-plane's tenant-provisioning API (#7654), matching
2-
// packages/loopover-miner/lib/tenant-client.ts's already-merged contract exactly: `POST /v1/tenants`
3-
// (`{name, product}`), `GET /v1/tenants` (`{tenants: [...]}`), `DELETE /v1/tenants/:name`, all Bearer-authed.
2+
// packages/loopover-miner/lib/tenant-client.ts's already-merged contract for create/list shapes:
3+
// `POST /v1/tenants` (`{name, product}`), `GET /v1/tenants` (`{tenants: [...]}`), and
4+
// `DELETE /v1/tenants/:name?product=` (#8024: product is required so registry lookups stay product-scoped).
45
// Factored out as a plain Hono app (not the real Worker entry point, see worker.ts) so it's testable via
56
// Hono's own `app.request()` against injected fakes under plain `node:test` -- mirrors
67
// packages/discovery-index/src/app.ts's identical split for the identical reason.
@@ -62,10 +63,10 @@ export function createTenantHttpApp(deps: TenantHttpAppDeps): Hono {
6263
if (typeof product !== "string" || !product.trim()) return c.json({ error: "invalid_request", message: "product is required" }, 400);
6364

6465
// Not idempotent by design (tenant-client.ts's own doc comment: "a create is not idempotent, so it must
65-
// not be silently re-sent") -- a currently-active tenant of the same name is a real conflict, not a no-op.
66-
// A previously torn-down tenant may be recreated (its createdAt is NOT preserved -- this is a fresh
67-
// provision, not a resurrection of the old one).
68-
const existing = await deps.registry.get(name);
66+
// not be silently re-sent") -- a currently-active tenant of the same name *and product* is a real conflict,
67+
// not a no-op (#8024: ORB "acme" must not block AMS "acme"). A previously torn-down tenant may be recreated
68+
// (its createdAt is NOT preserved -- this is a fresh provision, not a resurrection of the old one).
69+
const existing = await deps.registry.get(name, product);
6970
if (existing && existing.state !== "torn down") return c.json({ error: "tenant_already_exists" }, 409);
7071

7172
const result = await provisionTenant({ name }, product, deps.driver, deps.pagerDuty ?? {});
@@ -81,7 +82,13 @@ export function createTenantHttpApp(deps: TenantHttpAppDeps): Hono {
8182

8283
app.delete("/v1/tenants/:name", async (c) => {
8384
const name = c.req.param("name");
84-
const existing = await deps.registry.get(name);
85+
// Product is required so the registry can resolve the same `${product}:${name}` key used at create (#8024).
86+
const product = c.req.query("product");
87+
if (typeof product !== "string" || !product.trim()) {
88+
return c.json({ error: "invalid_request", message: "product query parameter is required" }, 400);
89+
}
90+
91+
const existing = await deps.registry.get(name, product);
8592
if (!existing) return c.json({ error: "tenant_not_found" }, 404);
8693

8794
const result = await deprovisionTenant(existing.tenant, existing.product, deps.driver, deps.pagerDuty ?? {});

control-plane/src/tenant-registry.ts

Lines changed: 30 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -16,26 +16,41 @@ export type TenantRegistryRecord = {
1616

1717
export interface TenantRegistry {
1818
/** Insert or update a tenant's record. Preserves the original `createdAt` on an update (looked up by the
19-
* caller, not this method -- see `http-app.ts`'s own upsert helper). */
19+
* caller, not this method -- see `http-app.ts`'s own upsert helper). Keyed by `(product, name)` so ORB and
20+
* AMS tenants that share a name stay independent (#8024). */
2021
upsert(record: TenantRegistryRecord): Promise<void>;
21-
get(name: string): Promise<TenantRegistryRecord | undefined>;
22+
/** Lookup by the same `${product}:${name}` composite as container-driver.ts's `instanceNameFor` (#8024). */
23+
get(name: string, product: Product): Promise<TenantRegistryRecord | undefined>;
2224
/** Every tenant this service has ever created, including torn-down ones (mirrors a cloud console showing
23-
* terminated instances rather than making them vanish) -- ordered by `tenant.name` for a stable listing. */
25+
* terminated instances rather than making them vanish) -- ordered by `tenant.name` then `product` for a
26+
* stable listing across products. */
2427
list(): Promise<TenantRegistryRecord[]>;
2528
}
2629

30+
/** Same composite key as container-driver.ts's `instanceNameFor` (#8024) — ORB and AMS tenants that share a
31+
* name must not collide in the admin inventory. */
32+
function instanceKeyFor(name: string, product: Product): string {
33+
return `${product}:${name}`;
34+
}
35+
36+
function sortRecords(records: TenantRegistryRecord[]): TenantRegistryRecord[] {
37+
return records.sort(
38+
(a, b) => a.tenant.name.localeCompare(b.tenant.name) || a.product.localeCompare(b.product),
39+
);
40+
}
41+
2742
/** In-memory fake for tests -- mirrors `createFakeTenantProvisioningDriver`'s own minimal-fake convention. */
2843
export function createFakeTenantRegistry(): TenantRegistry {
2944
const records = new Map<string, TenantRegistryRecord>();
3045
return {
3146
async upsert(record) {
32-
records.set(record.tenant.name, record);
47+
records.set(instanceKeyFor(record.tenant.name, record.product), record);
3348
},
34-
async get(name) {
35-
return records.get(name);
49+
async get(name, product) {
50+
return records.get(instanceKeyFor(name, product));
3651
},
3752
async list() {
38-
return [...records.values()].sort((a, b) => a.tenant.name.localeCompare(b.tenant.name));
53+
return sortRecords([...records.values()]);
3954
},
4055
};
4156
}
@@ -51,19 +66,20 @@ export type KvNamespaceLike = {
5166

5267
const KEY_PREFIX = "tenant:";
5368

54-
function keyFor(name: string): string {
55-
return `${KEY_PREFIX}${name}`;
69+
function keyFor(name: string, product: Product): string {
70+
return `${KEY_PREFIX}${instanceKeyFor(name, product)}`;
5671
}
5772

5873
/** Real registry backed by Workers KV. `list()` pages through every `tenant:`-prefixed key (KV's own `list()`
59-
* caps each call at 1000 keys) rather than assuming a single page covers the whole registry. */
74+
* caps each call at 1000 keys) rather than assuming a single page covers the whole registry. Keys are
75+
* `tenant:${product}:${name}` (#8024). */
6076
export function createKvTenantRegistry(kv: KvNamespaceLike): TenantRegistry {
6177
return {
6278
async upsert(record) {
63-
await kv.put(keyFor(record.tenant.name), JSON.stringify(record));
79+
await kv.put(keyFor(record.tenant.name, record.product), JSON.stringify(record));
6480
},
65-
async get(name) {
66-
const raw = await kv.get(keyFor(name));
81+
async get(name, product) {
82+
const raw = await kv.get(keyFor(name, product));
6783
return raw ? (JSON.parse(raw) as TenantRegistryRecord) : undefined;
6884
},
6985
async list() {
@@ -78,7 +94,7 @@ export function createKvTenantRegistry(kv: KvNamespaceLike): TenantRegistry {
7894
if (page.list_complete || !page.cursor) break;
7995
cursor = page.cursor;
8096
}
81-
return records.sort((a, b) => a.tenant.name.localeCompare(b.tenant.name));
97+
return sortRecords(records);
8298
},
8399
};
84100
}

control-plane/test/http-app.test.ts

Lines changed: 43 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -78,7 +78,7 @@ test("POST /v1/tenants creates a tenant, returns only the safe {tenant,product,s
7878
assert.deepEqual(payload, { tenant: { name: "acme" }, product: "orb", state: "active" });
7979
assert.equal("database" in payload, false);
8080
// The registry was actually updated, not just the HTTP response shaped correctly.
81-
assert.equal((await registry.get("acme"))?.state, "active");
81+
assert.equal((await registry.get("acme", "orb"))?.state, "active");
8282
});
8383

8484
test("POST /v1/tenants never echoes a tenant's database connection details on the wire", async () => {
@@ -152,7 +152,32 @@ test("POST /v1/tenants allows recreating a torn-down tenant", async () => {
152152
);
153153

154154
assert.equal(res.status, 201);
155-
assert.equal((await registry.get("acme"))?.state, "active");
155+
assert.equal((await registry.get("acme", "orb"))?.state, "active");
156+
});
157+
158+
test("POST /v1/tenants allows the same name under a different product (#8024)", async () => {
159+
const registry = createFakeTenantRegistry();
160+
const driver = createFakeTenantProvisioningDriver();
161+
const app = createTenantHttpApp(baseDeps({ registry, driver }));
162+
163+
const orb = await app.request(
164+
"/v1/tenants",
165+
authed({ method: "POST", headers: { "content-type": "application/json" }, body: JSON.stringify({ name: "acme", product: "orb" }) }),
166+
);
167+
const ams = await app.request(
168+
"/v1/tenants",
169+
authed({ method: "POST", headers: { "content-type": "application/json" }, body: JSON.stringify({ name: "acme", product: "ams" }) }),
170+
);
171+
172+
assert.equal(orb.status, 201);
173+
assert.equal(ams.status, 201);
174+
assert.equal((await registry.get("acme", "orb"))?.state, "active");
175+
assert.equal((await registry.get("acme", "ams"))?.state, "active");
176+
177+
const deleted = await app.request("/v1/tenants/acme?product=orb", authed({ method: "DELETE" }));
178+
assert.equal(deleted.status, 200);
179+
assert.equal((await registry.get("acme", "orb"))?.state, "torn down");
180+
assert.equal((await registry.get("acme", "ams"))?.state, "active");
156181
});
157182

158183
test("GET /v1/tenants lists every registered tenant, sorted, with timestamps", async () => {
@@ -187,18 +212,29 @@ test("DELETE /v1/tenants/:name tears down a known tenant and reports it torn dow
187212
await driver.createContainer({ tenant: { name: "acme" }, product: "orb" });
188213
const app = createTenantHttpApp(baseDeps({ registry, driver }));
189214

190-
const res = await app.request("/v1/tenants/acme", authed({ method: "DELETE" }));
215+
const res = await app.request("/v1/tenants/acme?product=orb", authed({ method: "DELETE" }));
191216

192217
assert.equal(res.status, 200);
193218
assert.deepEqual(await res.json(), { tenant: { name: "acme" }, product: "orb", state: "torn down" });
194-
assert.equal((await registry.get("acme"))?.state, "torn down");
219+
assert.equal((await registry.get("acme", "orb"))?.state, "torn down");
195220
assert.equal(await driver.containerExists({ tenant: { name: "acme" }, product: "orb" }), false);
196221
});
197222

223+
test("DELETE /v1/tenants/:name rejects a missing product query parameter (400)", async () => {
224+
const registry = createFakeTenantRegistry();
225+
await registry.upsert({ tenant: { name: "acme" }, product: "orb", state: "active", createdAt: "t0", updatedAt: "t0" });
226+
const app = createTenantHttpApp(baseDeps({ registry }));
227+
228+
const res = await app.request("/v1/tenants/acme", authed({ method: "DELETE" }));
229+
230+
assert.equal(res.status, 400);
231+
assert.equal((await res.json() as { error: string }).error, "invalid_request");
232+
});
233+
198234
test("DELETE /v1/tenants/:name on an unknown tenant is a 404, not a silent no-op", async () => {
199235
const app = createTenantHttpApp(baseDeps());
200236

201-
const res = await app.request("/v1/tenants/ghost", authed({ method: "DELETE" }));
237+
const res = await app.request("/v1/tenants/ghost?product=orb", authed({ method: "DELETE" }));
202238

203239
assert.equal(res.status, 404);
204240
assert.deepEqual(await res.json(), { error: "tenant_not_found" });
@@ -209,7 +245,7 @@ test("DELETE /v1/tenants/:name URL-decodes the name path segment", async () => {
209245
await registry.upsert({ tenant: { name: "acme corp" }, product: "orb", state: "active", createdAt: "t0", updatedAt: "t0" });
210246
const app = createTenantHttpApp(baseDeps({ registry }));
211247

212-
const res = await app.request(`/v1/tenants/${encodeURIComponent("acme corp")}`, authed({ method: "DELETE" }));
248+
const res = await app.request(`/v1/tenants/${encodeURIComponent("acme corp")}?product=orb`, authed({ method: "DELETE" }));
213249

214250
assert.equal(res.status, 200);
215251
});
@@ -223,7 +259,7 @@ test("create and delete both work when pagerDuty options are omitted entirely (d
223259
);
224260
assert.equal(created.status, 201);
225261

226-
const deleted = await app.request("/v1/tenants/acme", authed({ method: "DELETE" }));
262+
const deleted = await app.request("/v1/tenants/acme?product=orb", authed({ method: "DELETE" }));
227263
assert.equal(deleted.status, 200);
228264
});
229265

control-plane/test/tenant-registry.test.ts

Lines changed: 59 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -10,34 +10,56 @@ import {
1010
type TenantRegistryRecord,
1111
} from "../dist/index.js";
1212

13-
function recordFor(name: string, state: TenantRegistryRecord["state"] = "active"): TenantRegistryRecord {
14-
return { tenant: { name }, product: "orb", state, createdAt: "2026-01-01T00:00:00.000Z", updatedAt: "2026-01-01T00:00:00.000Z" };
13+
function recordFor(
14+
name: string,
15+
product: TenantRegistryRecord["product"] = "orb",
16+
state: TenantRegistryRecord["state"] = "active",
17+
): TenantRegistryRecord {
18+
return { tenant: { name }, product, state, createdAt: "2026-01-01T00:00:00.000Z", updatedAt: "2026-01-01T00:00:00.000Z" };
1519
}
1620

17-
test("createFakeTenantRegistry: upsert/get/list round-trip, sorted by tenant name", async () => {
21+
test("createFakeTenantRegistry: upsert/get/list round-trip, sorted by tenant name then product", async () => {
1822
const registry = createFakeTenantRegistry();
1923

2024
await registry.upsert(recordFor("zebra"));
2125
await registry.upsert(recordFor("acme"));
2226

23-
assert.deepEqual(await registry.get("acme"), recordFor("acme"));
24-
assert.equal(await registry.get("ghost"), undefined);
27+
assert.deepEqual(await registry.get("acme", "orb"), recordFor("acme"));
28+
assert.equal(await registry.get("ghost", "orb"), undefined);
2529
assert.deepEqual(
2630
(await registry.list()).map((record) => record.tenant.name),
2731
["acme", "zebra"],
2832
);
2933
});
3034

31-
test("createFakeTenantRegistry: upsert overwrites an existing record for the same tenant", async () => {
35+
test("createFakeTenantRegistry: upsert overwrites an existing record for the same product+tenant", async () => {
3236
const registry = createFakeTenantRegistry();
33-
await registry.upsert(recordFor("acme", "active"));
37+
await registry.upsert(recordFor("acme", "orb", "active"));
3438

35-
await registry.upsert(recordFor("acme", "torn down"));
39+
await registry.upsert(recordFor("acme", "orb", "torn down"));
3640

37-
assert.equal((await registry.get("acme"))?.state, "torn down");
41+
assert.equal((await registry.get("acme", "orb"))?.state, "torn down");
3842
assert.equal((await registry.list()).length, 1);
3943
});
4044

45+
// Mirrors container-driver.test.ts's product-scoped instance key — same name across products must not share
46+
// one registry row on a single shared registry (production's HTTP composition shape; #8024).
47+
test("createFakeTenantRegistry: state is product-scoped (${product}:${name}), not just the tenant name", async () => {
48+
const registry = createFakeTenantRegistry();
49+
50+
await registry.upsert(recordFor("acme", "orb", "active"));
51+
await registry.upsert(recordFor("acme", "ams", "active"));
52+
53+
assert.equal((await registry.get("acme", "orb"))?.product, "orb");
54+
assert.equal((await registry.get("acme", "ams"))?.product, "ams");
55+
assert.equal((await registry.list()).length, 2);
56+
57+
await registry.upsert(recordFor("acme", "orb", "torn down"));
58+
59+
assert.equal((await registry.get("acme", "orb"))?.state, "torn down");
60+
assert.equal((await registry.get("acme", "ams"))?.state, "active");
61+
});
62+
4163
function fakeKv(initial: Record<string, string> = {}): KvNamespaceLike & { store: Map<string, string> } {
4264
const store = new Map(Object.entries(initial));
4365
return {
@@ -60,33 +82,33 @@ function fakeKv(initial: Record<string, string> = {}): KvNamespaceLike & { store
6082
};
6183
}
6284

63-
test("createKvTenantRegistry: upsert writes a JSON-encoded value under the tenant: prefix", async () => {
85+
test("createKvTenantRegistry: upsert writes a JSON-encoded value under tenant:${product}:${name}", async () => {
6486
const kv = fakeKv();
6587
const registry = createKvTenantRegistry(kv);
6688

6789
await registry.upsert(recordFor("acme"));
6890

69-
assert.equal(kv.store.get("tenant:acme"), JSON.stringify(recordFor("acme")));
91+
assert.equal(kv.store.get("tenant:orb:acme"), JSON.stringify(recordFor("acme")));
7092
});
7193

7294
test("createKvTenantRegistry: get returns undefined for a key that was never written", async () => {
7395
const registry = createKvTenantRegistry(fakeKv());
7496

75-
assert.equal(await registry.get("ghost"), undefined);
97+
assert.equal(await registry.get("ghost", "orb"), undefined);
7698
});
7799

78100
test("createKvTenantRegistry: get parses a previously written record back", async () => {
79-
const kv = fakeKv({ "tenant:acme": JSON.stringify(recordFor("acme")) });
101+
const kv = fakeKv({ "tenant:orb:acme": JSON.stringify(recordFor("acme")) });
80102
const registry = createKvTenantRegistry(kv);
81103

82-
assert.deepEqual(await registry.get("acme"), recordFor("acme"));
104+
assert.deepEqual(await registry.get("acme", "orb"), recordFor("acme"));
83105
});
84106

85107
test("createKvTenantRegistry: list pages through multiple KV list() pages and returns every record, sorted", async () => {
86108
const kv = fakeKv({
87-
"tenant:charlie": JSON.stringify(recordFor("charlie")),
88-
"tenant:alpha": JSON.stringify(recordFor("alpha")),
89-
"tenant:bravo": JSON.stringify(recordFor("bravo")),
109+
"tenant:orb:charlie": JSON.stringify(recordFor("charlie")),
110+
"tenant:orb:alpha": JSON.stringify(recordFor("alpha")),
111+
"tenant:orb:bravo": JSON.stringify(recordFor("bravo")),
90112
});
91113
const registry = createKvTenantRegistry(kv);
92114

@@ -98,12 +120,30 @@ test("createKvTenantRegistry: list pages through multiple KV list() pages and re
98120
);
99121
});
100122

123+
test("createKvTenantRegistry: list returns both products when the same tenant name is registered twice", async () => {
124+
const kv = fakeKv({
125+
"tenant:orb:acme": JSON.stringify(recordFor("acme", "orb")),
126+
"tenant:ams:acme": JSON.stringify(recordFor("acme", "ams")),
127+
});
128+
const registry = createKvTenantRegistry(kv);
129+
130+
const records = await registry.list();
131+
132+
assert.deepEqual(
133+
records.map((record) => [record.tenant.name, record.product]),
134+
[
135+
["acme", "ams"],
136+
["acme", "orb"],
137+
],
138+
);
139+
});
140+
101141
test("createKvTenantRegistry: list tolerates a key disappearing between the list() page and the get() read", async () => {
102-
const kv = fakeKv({ "tenant:acme": JSON.stringify(recordFor("acme")) });
142+
const kv = fakeKv({ "tenant:orb:acme": JSON.stringify(recordFor("acme")) });
103143
const originalGet = kv.get.bind(kv);
104144
kv.get = async (key: string) => {
105145
// Simulate a concurrent delete: the key was listed, but its value is gone by the time we read it.
106-
if (key === "tenant:acme") return null;
146+
if (key === "tenant:orb:acme") return null;
107147
return originalGet(key);
108148
};
109149
const registry = createKvTenantRegistry(kv);

0 commit comments

Comments
 (0)