diff --git a/control-plane/src/tenant-registry.ts b/control-plane/src/tenant-registry.ts index d53a08f11..0eb69d814 100644 --- a/control-plane/src/tenant-registry.ts +++ b/control-plane/src/tenant-registry.ts @@ -78,14 +78,30 @@ function sortRecords(records: TenantRegistryRecord[]): TenantRegistryRecord[] { } /** In-memory fake for tests -- mirrors `createFakeTenantProvisioningDriver`'s own minimal-fake convention. - * `getByOrbInstallationId` is a plain linear scan -- fine for a fake with, at most, a handful of test - * records; the real KV-backed registry below needs an actual secondary index instead, since KV has no query - * capability at all. */ + * `getByOrbInstallationId` resolves through the same write-time installation -> primary-key secondary + * index the real KV-backed registry below keeps (last writer wins, with #9143's compare-and-delete rule + * on a cleared/changed claim), held as an in-memory Map -- NOT a scan of `records`, whose insertion order + * would answer duplicated-installation scenarios first-inserted-wins, the opposite of KV. */ export function createFakeTenantRegistry(): TenantRegistry { const records = new Map(); + const installationIndex = new Map(); return { async upsert(record) { - records.set(instanceKeyFor(record.tenant.name, record.product), record); + const primaryKey = instanceKeyFor(record.tenant.name, record.product); + // Mirror createKvTenantRegistry's write-time index maintenance: when this update changes (or clears) + // the tenant's installation claim, drop the stale pointer -- but only if it still points at THIS + // tenant's own primary key (#9143's compare-and-delete: the ID may since have been legitimately + // taken over by another tenant, whose live pointer must survive this record's late upsert). + const previous = records.get(primaryKey); + if (previous?.orbInstallationId !== undefined && previous.orbInstallationId !== record.orbInstallationId) { + if (installationIndex.get(previous.orbInstallationId) === primaryKey) { + installationIndex.delete(previous.orbInstallationId); + } + } + records.set(primaryKey, record); + if (record.orbInstallationId !== undefined) { + installationIndex.set(record.orbInstallationId, primaryKey); + } }, async get(name, product) { return records.get(instanceKeyFor(name, product)); @@ -94,7 +110,8 @@ export function createFakeTenantRegistry(): TenantRegistry { return sortRecords([...records.values()]); }, async getByOrbInstallationId(installationId) { - return [...records.values()].find((record) => record.orbInstallationId === installationId); + const primaryKey = installationIndex.get(installationId); + return primaryKey === undefined ? undefined : records.get(primaryKey); }, }; } diff --git a/control-plane/test/tenant-registry.test.ts b/control-plane/test/tenant-registry.test.ts index c8b8cb8bf..1f40135a9 100644 --- a/control-plane/test/tenant-registry.test.ts +++ b/control-plane/test/tenant-registry.test.ts @@ -255,3 +255,45 @@ test("createKvTenantRegistry: an unrelated tenant's installation-index entry SUR assert.equal(kv.store.get("installation:555"), "tenant:orb:newcomer"); assert.equal((await registry.getByOrbInstallationId(555))?.tenant.name, "newcomer"); }); + +// #9613: the fake used to answer getByOrbInstallationId with a linear scan of `records` in Map insertion +// order (first-inserted-wins) while the KV registry resolves through its last-writer-wins installation +// index -- so for #9143's duplicated-installation scenario the two implementations returned OPPOSITE +// tenants, and http-app.test.ts (which injects the fake) asserted inverted routing/409 semantics. +test("createFakeTenantRegistry: an unrelated tenant's installation-index entry SURVIVES a stale record's later re-creation/teardown upsert (#9143)", async () => { + const registry = createFakeTenantRegistry(); + + // Tenant "old" claimed installation 555 at creation, then failed -- its record still carries the ID. + await registry.upsert({ ...recordFor("old", "orb", "failed"), orbInstallationId: 555 }); + // A brand-new tenant legitimately re-claims installation 555: last writer wins, exactly like KV. + await registry.upsert({ ...recordFor("newcomer", "orb", "active"), orbInstallationId: 555 }); + assert.equal((await registry.getByOrbInstallationId(555))?.tenant.name, "newcomer"); + + // "old" is later torn down with no orbInstallationId: the compare-and-delete rule sees the index no + // longer points at "old" and leaves newcomer's live pointer alone. + await registry.upsert(recordFor("old", "orb", "torn down")); + assert.equal((await registry.getByOrbInstallationId(555))?.tenant.name, "newcomer"); +}); + +test("createFakeTenantRegistry: re-linking a tenant to a different installation ID clears the stale index entry", async () => { + const registry = createFakeTenantRegistry(); + await registry.upsert({ ...recordFor("acme"), orbInstallationId: 555 }); + + await registry.upsert({ ...recordFor("acme"), orbInstallationId: 777 }); + + assert.equal(await registry.getByOrbInstallationId(555), undefined); + assert.equal((await registry.getByOrbInstallationId(777))?.tenant.name, "acme"); + + // Re-upserting with the SAME installation ID (state-only change) leaves the pointer in place. + await registry.upsert({ ...recordFor("acme", "orb", "torn down"), orbInstallationId: 777 }); + assert.equal((await registry.getByOrbInstallationId(777))?.state, "torn down"); +}); + +test("createFakeTenantRegistry: unlinking a tenant's installation ID (upsert without it) clears the stale index entry", async () => { + const registry = createFakeTenantRegistry(); + await registry.upsert({ ...recordFor("acme"), orbInstallationId: 555 }); + + await registry.upsert(recordFor("acme")); + + assert.equal(await registry.getByOrbInstallationId(555), undefined); +});