Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
27 changes: 22 additions & 5 deletions control-plane/src/tenant-registry.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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<string, TenantRegistryRecord>();
const installationIndex = new Map<number, string>();
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));
Expand All @@ -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);
},
};
}
Expand Down
42 changes: 42 additions & 0 deletions control-plane/test/tenant-registry.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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);
});