diff --git a/packages/bus/src/limits.mjs b/packages/bus/src/limits.mjs index 593deb79a..c6ec3ba90 100644 --- a/packages/bus/src/limits.mjs +++ b/packages/bus/src/limits.mjs @@ -85,11 +85,16 @@ export function createLimits({ store, read, log, makeLoader = createLoader, keyF return ids; } // The orphaned ids now, [] when no daily key quota is set, or null when the lineages can't be - // read (the remembered set then stays as it was). + // read. Then the remembered set keeps only the ids the document still holds and gains none: an + // unreadable lookup proves no leaf orphaned, but the document does prove which leaves were removed. function observeOrphans(business, limits) { - if (!dailyKeyIds(limits).length) { orphansLogged.set(business, new Set()); return []; } + const ids = dailyKeyIds(limits); + if (!ids.length) { orphansLogged.set(business, new Set()); return []; } const lineages = lineagesFor(business); - return lineages ? noteOrphans(business, limits, lineages) : null; + if (lineages) return noteOrphans(business, limits, lineages); + const before = orphansLogged.get(business); + if (before) orphansLogged.set(business, new Set([...before].filter(id => ids.includes(id)))); + return null; } // The business's lineages, or null when they can't be read or are malformed. function lineagesFor(business) { diff --git a/packages/bus/tests/p5-key-lineage.test.mjs b/packages/bus/tests/p5-key-lineage.test.mjs index b6087b8c6..9c8ed5706 100644 --- a/packages/bus/tests/p5-key-lineage.test.mjs +++ b/packages/bus/tests/p5-key-lineage.test.mjs @@ -51,6 +51,7 @@ async function fixture(t) { const f = { root, configDir, storeRoot, storeDir, proxy, runs, stopped, logs, receipts, prefix, doc: quotaDoc({}), + lookupFails: false, read: () => f.doc, start() { f.store = new Store(storeDir); @@ -61,7 +62,7 @@ async function fixture(t) { stopRuns: async rs => { for (const r of rs) { stopped.push(r.run); runs.delete('demo:' + r.run); } } }); f.limits = createLimits({ store: f.store, read: business => f.read(business), log: e => logs.push(e), keyForSubject: (business, subject) => modelUsageKey(f.credentials.status(), runs, business, subject), - keyLineages: business => modelKeyLineages(f.credentials.status(), business) }); + keyLineages: business => { if (f.lookupFails) throw new Error('lineage lookup down'); return modelKeyLineages(f.credentials.status(), business); } }); }, stop() { f.credentials.close(); f.store.close(); }, // A bus restart: every in-memory object goes; the record, the store and the file come back from disk. @@ -366,3 +367,42 @@ test('R4 an orphaned leaf removed and added back logs twice; repeated refreshes for (let i = 0; i < 3; i++) await f.limits.refresh('demo'); assert.deepEqual(f.logs, [line, line, line]); }); + +// #783: a failed lineage lookup cannot say which present leaves are orphans, so it adds none. It can +// still see which leaves the refreshed document dropped, and forgets those, so a leaf added back logs. +test('#783 a leaf removed while the lineage lookup fails logs again when it is added back; a leaf present throughout does not', async t => { + const f = await fixture(t); await f.keys.reconcile(); await f.keys.ensure('demo/coder'); + const live = f.record().keyId, X = hash('fired-agent-key'), Z = hash('another-fired-key'); + const lineFor = id => ({ event: 'limit-key-orphaned', business: 'demo', path: ['tokens', 'keys', id] }); + f.doc = quotaDoc({ [X]: quota(100), [live]: quota(100) }); + assert.deepEqual(f.limits.orphanedKeys('demo'), [X]); + assert.deepEqual(f.logs, [lineFor(X)]); + // The lookup fails with X still in the file: nothing is known and nothing is logged. + f.lookupFails = true; + await f.limits.refresh('demo'); + assert.equal(f.limits.orphanedKeys('demo'), null); + assert.deepEqual(f.logs, [lineFor(X)]); + // X is removed while the lookup is down; the live leaf stays, so a daily key quota still exists. + f.doc = quotaDoc({ [live]: quota(100) }); await f.limits.refresh('demo'); + // X is added back before any good observation: still nothing to say. + f.doc = quotaDoc({ [X]: quota(100), [live]: quota(100) }); await f.limits.refresh('demo'); + assert.deepEqual(f.logs, [lineFor(X)]); + // The lookup recovers: the next observation logs X once more. + f.lookupFails = false; + await f.limits.refresh('demo'); + assert.deepEqual(f.limits.orphanedKeys('demo'), [X]); + assert.deepEqual(f.logs, [lineFor(X), lineFor(X)]); + // A second failure with X present throughout adds no line, and a leaf added during it is not + // called an orphan until the lookup can say so; then it logs once. + f.lookupFails = true; + f.doc = quotaDoc({ [X]: quota(100), [Z]: quota(100), [live]: quota(100) }); await f.limits.refresh('demo'); + assert.equal(f.limits.orphanedKeys('demo'), null); + assert.deepEqual(f.logs, [lineFor(X), lineFor(X)]); + f.lookupFails = false; + await f.limits.refresh('demo'); await f.limits.refresh('demo'); + assert.deepEqual(f.limits.orphanedKeys('demo'), [X, Z]); + assert.deepEqual(f.logs, [lineFor(X), lineFor(X), lineFor(Z)]); + // Quota decisions still refuse while the lookup is down. + f.lookupFails = true; + assert.deepEqual(await f.limits.mayLaunch('demo', SUBJECT), { allow: false, check: null, code: 'proxy-key-refused' }); +});