From 84c4b66b7bf9cf46e239ecaa044e6d1fe7b1b6f0 Mon Sep 17 00:00:00 2001 From: Jimmy Thomson Date: Tue, 16 Jun 2026 11:21:23 +0100 Subject: [PATCH] feat: deduplicate requests to the same secret --- .gitignore | 3 + package-lock.json | 20 +++- package.json | 1 + src/keyvault/keyVaultSecretProvider.ts | 22 +++- test/keyvault.test.ts | 146 +++++++++++++++++++++++++ 5 files changed, 185 insertions(+), 7 deletions(-) diff --git a/.gitignore b/.gitignore index 70fc8a6..987de10 100644 --- a/.gitignore +++ b/.gitignore @@ -397,6 +397,9 @@ FodyWeavers.xsd # JetBrains Rider *.sln.iml +# JetBrains IDE +.idea/ + # bundled folder dist/ dist-esm/ diff --git a/package-lock.json b/package-lock.json index 8de9975..1b65b99 100644 --- a/package-lock.json +++ b/package-lock.json @@ -32,6 +32,7 @@ "cpy-cli": "^6.0.0", "dotenv": "^17.2.1", "eslint": "^9.34.0", + "globals": "^17.6.0", "mocha": "^11.7.6", "nock": "^14.0.10", "playwright": "^1.60.0", @@ -518,6 +519,19 @@ "concat-map": "0.0.1" } }, + "node_modules/@eslint/eslintrc/node_modules/globals": { + "version": "14.0.0", + "resolved": "https://registry.npmjs.org/globals/-/globals-14.0.0.tgz", + "integrity": "sha512-oahGvuMGQlPw/ivIYBjVSrWAfWLBeku5tpPE2fOPLi+WHffIWbuh2tCjhyQhTBPMf5E9jDEH4FOmTYgYwbKwtQ==", + "dev": true, + "license": "MIT", + "engines": { + "node": ">=18" + }, + "funding": { + "url": "https://github.com/sponsors/sindresorhus" + } + }, "node_modules/@eslint/eslintrc/node_modules/ignore": { "version": "5.3.2", "resolved": "https://registry.npmjs.org/ignore/-/ignore-5.3.2.tgz", @@ -3166,9 +3180,9 @@ } }, "node_modules/globals": { - "version": "14.0.0", - "resolved": "https://registry.npmjs.org/globals/-/globals-14.0.0.tgz", - "integrity": "sha512-oahGvuMGQlPw/ivIYBjVSrWAfWLBeku5tpPE2fOPLi+WHffIWbuh2tCjhyQhTBPMf5E9jDEH4FOmTYgYwbKwtQ==", + "version": "17.6.0", + "resolved": "https://registry.npmjs.org/globals/-/globals-17.6.0.tgz", + "integrity": "sha512-sepffkT8stwnIYbsMBpoCHJuJM5l98FUF2AnE07hfvE0m/qp3R586hw4jF4uadbhvg1ooIdzuu7CsfD2jzCaNA==", "dev": true, "license": "MIT", "engines": { diff --git a/package.json b/package.json index 0803110..a36703c 100644 --- a/package.json +++ b/package.json @@ -56,6 +56,7 @@ "cpy-cli": "^6.0.0", "dotenv": "^17.2.1", "eslint": "^9.34.0", + "globals": "^17.6.0", "mocha": "^11.7.6", "nock": "^14.0.10", "playwright": "^1.60.0", diff --git a/src/keyvault/keyVaultSecretProvider.ts b/src/keyvault/keyVaultSecretProvider.ts index 2d5e113..af2f2ac 100644 --- a/src/keyvault/keyVaultSecretProvider.ts +++ b/src/keyvault/keyVaultSecretProvider.ts @@ -13,6 +13,7 @@ export class AzureKeyVaultSecretProvider { #minSecretRefreshTimer: RefreshTimer; #secretClients: Map; // map key vault hostname to corresponding secret client #cachedSecretValues: Map = new Map(); // map secret identifier to secret value + #inflightRequests: Map> = new Map>(); // map secret identifier to in-flight fetch promise, used to dedupe concurrent requests constructor(keyVaultOptions: KeyVaultOptions | undefined, refreshTimer?: RefreshTimer) { if (keyVaultOptions?.secretRefreshIntervalInMs !== undefined) { @@ -42,10 +43,23 @@ export class AzureKeyVaultSecretProvider { return this.#cachedSecretValues.get(identifierKey); } - // Fallback to fetching the secret value from Key Vault. - const secretValue = await this.#getSecretValueFromKeyVault(secretIdentifier); - this.#cachedSecretValues.set(identifierKey, secretValue); - return secretValue; + // Dedupe concurrent fetches for the same secret: if a request is already in flight, + // await the existing promise instead of issuing a new Key Vault call. + let pending = this.#inflightRequests.get(identifierKey); + if (pending === undefined) { + pending = this.#getSecretValueFromKeyVault(secretIdentifier) + .then((secretValue) => { + this.#cachedSecretValues.set(identifierKey, secretValue); + return secretValue; + }) + .finally(() => { + // Always remove the in-flight entry so failures are not cached + // and subsequent calls can retry. + this.#inflightRequests.delete(identifierKey); + }); + this.#inflightRequests.set(identifierKey, pending); + } + return pending; } clearCache(): void { diff --git a/test/keyvault.test.ts b/test/keyvault.test.ts index feeb294..23bcfc5 100644 --- a/test/keyvault.test.ts +++ b/test/keyvault.test.ts @@ -142,6 +142,152 @@ describe("key vault reference", function () { }); }); +describe("key vault secret request deduplication", function () { + afterEach(() => { + restoreMocks(); + }); + + function mockDuplicateKeyVaultReferences(count: number, secretUri: string) { + const kvs = Array.from({ length: count }, (_, i) => + createMockedKeyVaultReference(`TestKey${i}`, secretUri)); + mockAppConfigurationClientListConfigurationSettings([kvs]); + return kvs; + } + + it("should issue only one Key Vault request when many settings reference the same secret (sequential mode)", async () => { + const secretUri = "https://fake-vault-name.vault.azure.net/secrets/sharedSecret"; + mockDuplicateKeyVaultReferences(5, secretUri); + + const client = new SecretClient("https://fake-vault-name.vault.azure.net", createMockedTokenCredential()); + const getSecretStub = sinon.stub(client, "getSecret").callsFake(async () => { + // small delay to simulate network so concurrent calls would overlap + await sleepInMs(10); + return { value: "SharedSecretValue" } as KeyVaultSecret; + }); + + const settings = await load(createMockedConnectionString(), { + keyVaultOptions: { + secretClients: [client] + } + }); + + expect(getSecretStub.callCount).eq(1); + for (let i = 0; i < 5; i++) { + expect(settings.get(`TestKey${i}`)).eq("SharedSecretValue"); + } + }); + + it("should issue only one Key Vault request when many settings reference the same secret (parallel mode)", async () => { + const secretUri = "https://fake-vault-name.vault.azure.net/secrets/sharedSecret"; + mockDuplicateKeyVaultReferences(5, secretUri); + + const client = new SecretClient("https://fake-vault-name.vault.azure.net", createMockedTokenCredential()); + const getSecretStub = sinon.stub(client, "getSecret").callsFake(async () => { + await sleepInMs(10); + return { value: "SharedSecretValue" } as KeyVaultSecret; + }); + + const settings = await load(createMockedConnectionString(), { + keyVaultOptions: { + secretClients: [client], + parallelSecretResolutionEnabled: true + } + }); + + expect(getSecretStub.callCount).eq(1); + for (let i = 0; i < 5; i++) { + expect(settings.get(`TestKey${i}`)).eq("SharedSecretValue"); + } + }); + + it("should invoke secretResolver only once for duplicate references (parallel mode)", async () => { + const secretUri = "https://fake-vault-name.vault.azure.net/secrets/sharedSecret"; + mockDuplicateKeyVaultReferences(4, secretUri); + + const resolverSpy = sinon.spy(async (kvrUrl: URL) => { + await sleepInMs(10); + return `Resolved::${kvrUrl.toString()}`; + }); + + const settings = await load(createMockedConnectionString(), { + keyVaultOptions: { + secretResolver: resolverSpy, + parallelSecretResolutionEnabled: true + } + }); + + expect(resolverSpy.callCount).eq(1); + for (let i = 0; i < 4; i++) { + expect(settings.get(`TestKey${i}`)).eq(`Resolved::${secretUri}`); + } + }); + + it("should fetch each version independently when references point to different versions of the same secret", async () => { + const baseUri = "https://fake-vault-name.vault.azure.net/secrets/sharedSecret"; + const v1 = `${baseUri}/version1id`; + const v2 = `${baseUri}/version2id`; + const kvs = [ + createMockedKeyVaultReference("TestKeyA", v1), + createMockedKeyVaultReference("TestKeyB", v1), + createMockedKeyVaultReference("TestKeyC", v2) + ]; + mockAppConfigurationClientListConfigurationSettings([kvs]); + + const client = new SecretClient("https://fake-vault-name.vault.azure.net", createMockedTokenCredential()); + const getSecretStub = sinon.stub(client, "getSecret").callsFake(async (_name, options) => { + await sleepInMs(10); + return { value: `value-${options?.version}` } as KeyVaultSecret; + }); + + const settings = await load(createMockedConnectionString(), { + keyVaultOptions: { + secretClients: [client], + parallelSecretResolutionEnabled: true + } + }); + + expect(getSecretStub.callCount).eq(2); + expect(settings.get("TestKeyA")).eq("value-version1id"); + expect(settings.get("TestKeyB")).eq("value-version1id"); + expect(settings.get("TestKeyC")).eq("value-version2id"); + }); + + it("should not cache failures: a follow-up resolution retries the Key Vault request", async () => { + const secretUri = "https://fake-vault-name.vault.azure.net/secrets/sharedSecret"; + mockDuplicateKeyVaultReferences(3, secretUri); + + // Mock the underlying call to fail on the very first batch, succeed on subsequent batches. + // The provider performs startup retries on failure; we assert that: + // - within the first batch, 3 concurrent references => 1 call (dedup) + // - the failure is NOT cached: the next batch fires a new call (which succeeds) + const client = new SecretClient("https://fake-vault-name.vault.azure.net", createMockedTokenCredential()); + const getSecretStub = sinon.stub(client, "getSecret"); + getSecretStub.onCall(0).callsFake(async () => { + await sleepInMs(10); + throw new Error("boom"); + }); + getSecretStub.callsFake(async () => { + await sleepInMs(10); + return { value: "RecoveredValue" } as KeyVaultSecret; + }); + + const settings = await load(createMockedConnectionString(), { + keyVaultOptions: { + secretClients: [client], + parallelSecretResolutionEnabled: true + } + }); + + // Exactly 2 underlying calls: first batch (deduped, failed), second batch (deduped, succeeded). + // If failures were negatively cached, callCount would be 1 and all references would observe the error; + // if dedup were missing, callCount would be 3 (or 6) instead. + expect(getSecretStub.callCount).eq(2); + for (let i = 0; i < 3; i++) { + expect(settings.get(`TestKey${i}`)).eq("RecoveredValue"); + } + }); +}); + describe("key vault secret refresh", function () { beforeEach(() => {