From ccfbe120bae87bc9496e7f94111099cdbb2302de Mon Sep 17 00:00:00 2001 From: "Lingling Ye (from Dev Box)" Date: Mon, 22 Jun 2026 16:53:56 +0800 Subject: [PATCH 1/4] Added map to track pending key vault requests keyed by secret source id --- src/keyvault/keyVaultSecretProvider.ts | 20 +++- test/keyvault.test.ts | 157 +++++++++++++++++++++++++ 2 files changed, 173 insertions(+), 4 deletions(-) diff --git a/src/keyvault/keyVaultSecretProvider.ts b/src/keyvault/keyVaultSecretProvider.ts index fa3d825..03486e1 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 Key Vault request constructor(keyVaultOptions?: KeyVaultOptions, refreshTimer?: RefreshTimer) { if (keyVaultOptions?.secretRefreshIntervalInMs !== undefined) { @@ -42,10 +43,21 @@ 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; + // Deduplicate concurrent requests for the same secret: if a request is already in-flight, await it. + let pendingRequest = this.#inflightRequests.get(identifierKey); + if (pendingRequest === undefined) { + pendingRequest = this.#getSecretValueFromKeyVault(secretIdentifier) + .then((secretValue) => { + this.#cachedSecretValues.set(identifierKey, secretValue); + return secretValue; + }) + .finally(() => { + // Failures are not cached so subsequent calls can retry. + this.#inflightRequests.delete(identifierKey); + }); + this.#inflightRequests.set(identifierKey, pendingRequest); + } + return pendingRequest; } clearCache(): void { diff --git a/test/keyvault.test.ts b/test/keyvault.test.ts index feeb294..daa530c 100644 --- a/test/keyvault.test.ts +++ b/test/keyvault.test.ts @@ -142,6 +142,163 @@ describe("key vault reference", function () { }); }); +describe("key vault reference deduplication", function () { + afterEach(() => { + restoreMocks(); + }); + + // 5 settings all referencing the same secret URI (same sourceId). + const sameSecretUri = "https://fake-vault-name.vault.azure.net/secrets/fakeSecretName"; + function mockDuplicateReferences() { + const kvs = ["TestKey1", "TestKey2", "TestKey3", "TestKey4", "TestKey5"] + .map((key) => createMockedKeyVaultReference(key, sameSecretUri)); + mockAppConfigurationClientListConfigurationSettings([kvs]); + } + + it("should resolve duplicate references with a single Key Vault request in parallel mode", async () => { + mockDuplicateReferences(); + const client = new SecretClient("https://fake-vault-name.vault.azure.net", createMockedTokenCredential()); + const stub = sinon.stub(client, "getSecret").callsFake(async () => { + // Introduce a delay so that all references start before the first one resolves. + await sleepInMs(100); + return { value: "SecretValue" } as KeyVaultSecret; + }); + + const settings = await load(createMockedConnectionString(), { + keyVaultOptions: { + secretClients: [client], + parallelSecretResolutionEnabled: true + } + }); + + expect(stub.callCount).eq(1); + for (const key of ["TestKey1", "TestKey2", "TestKey3", "TestKey4", "TestKey5"]) { + expect(settings.get(key)).eq("SecretValue"); + } + }); + + it("should resolve duplicate references with a single Key Vault request in sequential mode", async () => { + mockDuplicateReferences(); + const client = new SecretClient("https://fake-vault-name.vault.azure.net", createMockedTokenCredential()); + const stub = sinon.stub(client, "getSecret").callsFake(async () => { + return { value: "SecretValue" } as KeyVaultSecret; + }); + + const settings = await load(createMockedConnectionString(), { + keyVaultOptions: { + secretClients: [client] + } + }); + + expect(stub.callCount).eq(1); + for (const key of ["TestKey1", "TestKey2", "TestKey3", "TestKey4", "TestKey5"]) { + expect(settings.get(key)).eq("SecretValue"); + } + }); + + it("should invoke secret resolver only once for duplicate references", async () => { + mockDuplicateReferences(); + const resolver = sinon.stub().callsFake(async () => { + await sleepInMs(100); + return "ResolvedSecretValue"; + }); + + const settings = await load(createMockedConnectionString(), { + keyVaultOptions: { + secretResolver: resolver, + parallelSecretResolutionEnabled: true + } + }); + + expect(resolver.callCount).eq(1); + for (const key of ["TestKey1", "TestKey2", "TestKey3", "TestKey4", "TestKey5"]) { + expect(settings.get(key)).eq("ResolvedSecretValue"); + } + }); + + it("should fetch different versions of the same secret independently", async () => { + const versionedUri = "https://fake-vault-name.vault.azure.net/secrets/fakeSecretName/741a0fc52610449baffd6e1c55b9d459"; + const kvs = [ + createMockedKeyVaultReference("TestKey", sameSecretUri), + createMockedKeyVaultReference("TestKeyVersioned", versionedUri) + ]; + mockAppConfigurationClientListConfigurationSettings([kvs]); + const client = new SecretClient("https://fake-vault-name.vault.azure.net", createMockedTokenCredential()); + const stub = sinon.stub(client, "getSecret").callsFake(async (_name, options) => { + await sleepInMs(100); + return { value: options?.version ? "VersionedValue" : "LatestValue" } as KeyVaultSecret; + }); + + const settings = await load(createMockedConnectionString(), { + keyVaultOptions: { + secretClients: [client], + parallelSecretResolutionEnabled: true + } + }); + + expect(stub.callCount).eq(2); + expect(settings.get("TestKey")).eq("LatestValue"); + expect(settings.get("TestKeyVersioned")).eq("VersionedValue"); + }); + + it("should not cache failures and retry on a subsequent attempt", async () => { + mockDuplicateReferences(); + const client = new SecretClient("https://fake-vault-name.vault.azure.net", createMockedTokenCredential()); + const stub = sinon.stub(client, "getSecret"); + // The first (deduplicated) request rejects; the retry attempt succeeds. + // If the failure were cached, the retry would never succeed. + stub.onCall(0).callsFake(async () => { + await sleepInMs(100); + throw new Error("Key Vault unavailable"); + }); + stub.callsFake(async () => { + return { value: "SecretValue" } as KeyVaultSecret; + }); + + const settings = await load(createMockedConnectionString(), { + keyVaultOptions: { + secretClients: [client], + parallelSecretResolutionEnabled: true + } + }); + + // First round: 5 concurrent references deduped to a single failing request. + // Second round (after load retry): a single succeeding request. + expect(stub.callCount).eq(2); + for (const key of ["TestKey1", "TestKey2", "TestKey3", "TestKey4", "TestKey5"]) { + expect(settings.get(key)).eq("SecretValue"); + } + }); + + it("should re-fetch once per unique secret on each refresh round", async () => { + mockDuplicateReferences(); + const client = new SecretClient("https://fake-vault-name.vault.azure.net", createMockedTokenCredential()); + let callCount = 0; + sinon.stub(client, "getSecret").callsFake(async () => { + callCount++; + await sleepInMs(100); + return { value: `SecretValue-${callCount}` } as KeyVaultSecret; + }); + + const settings = await load(createMockedConnectionString(), { + keyVaultOptions: { + secretClients: [client], + secretRefreshIntervalInMs: 60_000, + parallelSecretResolutionEnabled: true + } + }); + // Initial load resolves duplicates with a single request. + expect(callCount).eq(1); + expect(settings.get("TestKey1")).eq("SecretValue-1"); + + // After the secret refresh interval elapses, the refresh round re-fetches once. + await sleepInMs(60_000 + 100); + await settings.refresh(); + expect(callCount).eq(2); + expect(settings.get("TestKey1")).eq("SecretValue-2"); + }); +}); + describe("key vault secret refresh", function () { beforeEach(() => { From 0e635bb478be8b8a363ff172bdc7d569fdd3bffd Mon Sep 17 00:00:00 2001 From: "Lingling Ye (from Dev Box)" Date: Fri, 10 Jul 2026 17:45:55 +0800 Subject: [PATCH 2/4] dedup key vault ref in preload stage --- src/appConfigurationImpl.ts | 40 ++++++++++++++++--------- src/keyvault/keyVaultKeyValueAdapter.ts | 22 ++++++++++++++ src/keyvault/keyVaultSecretProvider.ts | 31 ++++++------------- 3 files changed, 57 insertions(+), 36 deletions(-) diff --git a/src/appConfigurationImpl.ts b/src/appConfigurationImpl.ts index e247b31..3310d93 100644 --- a/src/appConfigurationImpl.ts +++ b/src/appConfigurationImpl.ts @@ -111,6 +111,7 @@ export class AzureAppConfigurationImpl implements AzureAppConfiguration { #secretReferences: ConfigurationSetting[] = []; // cached key vault references #secretRefreshTimer: RefreshTimer | undefined = undefined; #resolveSecretsInParallel: boolean = false; + #keyVaultAdapter: AzureKeyVaultKeyValueAdapter; /** * Selectors of key-values obtained from @see AzureAppConfigurationOptions.selectors @@ -204,7 +205,8 @@ export class AzureAppConfigurationImpl implements AzureAppConfiguration { } this.#resolveSecretsInParallel = options.keyVaultOptions.parallelSecretResolutionEnabled ?? false; } - this.#adapters.push(new AzureKeyVaultKeyValueAdapter(options?.keyVaultOptions, this.#secretRefreshTimer)); + this.#keyVaultAdapter = new AzureKeyVaultKeyValueAdapter(options?.keyVaultOptions, this.#secretRefreshTimer); + this.#adapters.push(this.#keyVaultAdapter); this.#adapters.push(new JsonKeyValueAdapter()); } @@ -715,6 +717,8 @@ export class AzureAppConfigurationImpl implements AzureAppConfiguration { return Promise.resolve(false); } + // Invalidate cached secret values so the refresh round re-fetches them from Key Vault. + this.#keyVaultAdapter.clearCache(); await this.#resolveSecretReferences(this.#secretReferences, (key, value) => { this.#configMap.set(key, value); }); @@ -895,22 +899,30 @@ export class AzureAppConfigurationImpl implements AzureAppConfiguration { async #resolveSecretReferences(secretReferences: ConfigurationSetting[], resultHandler: (key: string, value: unknown) => void): Promise { if (this.#resolveSecretsInParallel) { - const secretResolutionPromises: Promise[] = []; + // Preload phase: resolve each unique secret exactly once, in parallel, to warm the cache. + // References that resolve to the same secret identifier are deduplicated so that only a + // single Key Vault request is issued per unique secret. + const seenSecretIds = new Set(); + const uniqueSecretReferences: ConfigurationSetting[] = []; for (const setting of secretReferences) { - const secretResolutionPromise = this.#processKeyValue(setting) - .then(([key, value]) => { - resultHandler(key, value); - }); - secretResolutionPromises.push(secretResolutionPromise); + const secretId = this.#keyVaultAdapter.getSecretReferenceId(setting); + if (secretId === undefined) { + // The reference cannot be parsed; resolve it individually so the appropriate error + // is surfaced when it is processed below. + uniqueSecretReferences.push(setting); + } else if (!seenSecretIds.has(secretId)) { + seenSecretIds.add(secretId); + uniqueSecretReferences.push(setting); + } } + await Promise.all(uniqueSecretReferences.map(setting => this.#processKeyValue(setting))); + } - // Wait for all secret resolution promises to be resolved - await Promise.all(secretResolutionPromises); - } else { - for (const setting of secretReferences) { - const [key, value] = await this.#processKeyValue(setting); - resultHandler(key, value); - } + // Resolve every reference. In parallel mode the values are served from the warmed cache + // without any additional I/O. + for (const setting of secretReferences) { + const [key, value] = await this.#processKeyValue(setting); + resultHandler(key, value); } } diff --git a/src/keyvault/keyVaultKeyValueAdapter.ts b/src/keyvault/keyVaultKeyValueAdapter.ts index 2e8eb4d..2ac482d 100644 --- a/src/keyvault/keyVaultKeyValueAdapter.ts +++ b/src/keyvault/keyVaultKeyValueAdapter.ts @@ -49,6 +49,28 @@ export class AzureKeyVaultKeyValueAdapter implements IKeyValueAdapter { } } + /** + * Returns the normalized Key Vault secret identifier (sourceId) for a secret reference setting, + * or undefined if the reference cannot be parsed. Used to deduplicate references that resolve to + * the same secret before resolving them. + */ + getSecretReferenceId(setting: ConfigurationSetting): string | undefined { + try { + return parseKeyVaultSecretIdentifier( + parseSecretReference(setting).value.secretId + ).sourceId; + } catch { + return undefined; + } + } + + /** + * Clears the cached secret values, throttled by the minimum secret refresh interval. + */ + clearCache(): void { + this.#keyVaultSecretProvider.clearCache(); + } + async onChangeDetected(): Promise { this.#keyVaultSecretProvider.clearCache(); return; diff --git a/src/keyvault/keyVaultSecretProvider.ts b/src/keyvault/keyVaultSecretProvider.ts index 03486e1..dc66fb1 100644 --- a/src/keyvault/keyVaultSecretProvider.ts +++ b/src/keyvault/keyVaultSecretProvider.ts @@ -9,11 +9,9 @@ import { KeyVaultReferenceErrorMessages } from "../common/errorMessages.js"; export class AzureKeyVaultSecretProvider { #keyVaultOptions: KeyVaultOptions | undefined; - #secretRefreshTimer: RefreshTimer | undefined; #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 Key Vault request constructor(keyVaultOptions?: KeyVaultOptions, refreshTimer?: RefreshTimer) { if (keyVaultOptions?.secretRefreshIntervalInMs !== undefined) { @@ -25,7 +23,6 @@ export class AzureKeyVaultSecretProvider { } } this.#keyVaultOptions = keyVaultOptions; - this.#secretRefreshTimer = refreshTimer; this.#minSecretRefreshTimer = new RefreshTimer(MIN_SECRET_REFRESH_INTERVAL_IN_MS); this.#secretClients = new Map(); for (const client of this.#keyVaultOptions?.secretClients ?? []) { @@ -37,27 +34,17 @@ export class AzureKeyVaultSecretProvider { async getSecretValue(secretIdentifier: KeyVaultSecretIdentifier): Promise { const identifierKey = secretIdentifier.sourceId; - // If the refresh interval is not expired, return the cached value if available. - if (this.#cachedSecretValues.has(identifierKey) && - (!this.#secretRefreshTimer || !this.#secretRefreshTimer.canRefresh())) { - return this.#cachedSecretValues.get(identifierKey); + // Return the cached value if available. The cache is invalidated externally (on secret refresh + // or when a key-value change is detected) so a stale value is never served. + if (this.#cachedSecretValues.has(identifierKey)) { + return this.#cachedSecretValues.get(identifierKey); } - // Deduplicate concurrent requests for the same secret: if a request is already in-flight, await it. - let pendingRequest = this.#inflightRequests.get(identifierKey); - if (pendingRequest === undefined) { - pendingRequest = this.#getSecretValueFromKeyVault(secretIdentifier) - .then((secretValue) => { - this.#cachedSecretValues.set(identifierKey, secretValue); - return secretValue; - }) - .finally(() => { - // Failures are not cached so subsequent calls can retry. - this.#inflightRequests.delete(identifierKey); - }); - this.#inflightRequests.set(identifierKey, pendingRequest); - } - return pendingRequest; + // Fetch the secret value from Key Vault and cache it. Failures are not cached, so a subsequent + // call will retry. + const secretValue = await this.#getSecretValueFromKeyVault(secretIdentifier); + this.#cachedSecretValues.set(identifierKey, secretValue); + return secretValue; } clearCache(): void { From 8e0d5524c5bf2b1e311d93137889b3d5a5516131 Mon Sep 17 00:00:00 2001 From: "Lingling Ye (from Dev Box)" Date: Mon, 13 Jul 2026 19:14:26 +0800 Subject: [PATCH 3/4] update --- src/appConfigurationImpl.ts | 46 ++++++++--------- src/keyValueAdapter.ts | 6 +++ src/keyvault/keyVaultKeyValueAdapter.ts | 38 +++++++------- src/keyvault/keyVaultSecretProvider.ts | 46 +++++++++++++++-- test/keyvault.test.ts | 68 ++++++++++++++++++++++--- 5 files changed, 151 insertions(+), 53 deletions(-) diff --git a/src/appConfigurationImpl.ts b/src/appConfigurationImpl.ts index 3310d93..067c901 100644 --- a/src/appConfigurationImpl.ts +++ b/src/appConfigurationImpl.ts @@ -111,7 +111,6 @@ export class AzureAppConfigurationImpl implements AzureAppConfiguration { #secretReferences: ConfigurationSetting[] = []; // cached key vault references #secretRefreshTimer: RefreshTimer | undefined = undefined; #resolveSecretsInParallel: boolean = false; - #keyVaultAdapter: AzureKeyVaultKeyValueAdapter; /** * Selectors of key-values obtained from @see AzureAppConfigurationOptions.selectors @@ -205,8 +204,7 @@ export class AzureAppConfigurationImpl implements AzureAppConfiguration { } this.#resolveSecretsInParallel = options.keyVaultOptions.parallelSecretResolutionEnabled ?? false; } - this.#keyVaultAdapter = new AzureKeyVaultKeyValueAdapter(options?.keyVaultOptions, this.#secretRefreshTimer); - this.#adapters.push(this.#keyVaultAdapter); + this.#adapters.push(new AzureKeyVaultKeyValueAdapter(options?.keyVaultOptions, this.#secretRefreshTimer)); this.#adapters.push(new JsonKeyValueAdapter()); } @@ -571,6 +569,9 @@ export class AzureAppConfigurationImpl implements AzureAppConfiguration { } if (this.#secretReferences.length > 0) { + for (const adapter of this.#adapters) { + await adapter.preload?.(this.#secretReferences); // dedup and warm the secret cache + } await this.#resolveSecretReferences(this.#secretReferences, (key, value) => { keyValues.push([key, value]); }); @@ -717,8 +718,9 @@ export class AzureAppConfigurationImpl implements AzureAppConfiguration { return Promise.resolve(false); } - // Invalidate cached secret values so the refresh round re-fetches them from Key Vault. - this.#keyVaultAdapter.clearCache(); + for (const adapter of this.#adapters) { + await adapter.preload?.(this.#secretReferences); // dedup and warm the secret cache + } await this.#resolveSecretReferences(this.#secretReferences, (key, value) => { this.#configMap.set(key, value); }); @@ -899,30 +901,22 @@ export class AzureAppConfigurationImpl implements AzureAppConfiguration { async #resolveSecretReferences(secretReferences: ConfigurationSetting[], resultHandler: (key: string, value: unknown) => void): Promise { if (this.#resolveSecretsInParallel) { - // Preload phase: resolve each unique secret exactly once, in parallel, to warm the cache. - // References that resolve to the same secret identifier are deduplicated so that only a - // single Key Vault request is issued per unique secret. - const seenSecretIds = new Set(); - const uniqueSecretReferences: ConfigurationSetting[] = []; + const secretResolutionPromises: Promise[] = []; for (const setting of secretReferences) { - const secretId = this.#keyVaultAdapter.getSecretReferenceId(setting); - if (secretId === undefined) { - // The reference cannot be parsed; resolve it individually so the appropriate error - // is surfaced when it is processed below. - uniqueSecretReferences.push(setting); - } else if (!seenSecretIds.has(secretId)) { - seenSecretIds.add(secretId); - uniqueSecretReferences.push(setting); - } + const secretResolutionPromise = this.#processKeyValue(setting) + .then(([key, value]) => { + resultHandler(key, value); + }); + secretResolutionPromises.push(secretResolutionPromise); } - await Promise.all(uniqueSecretReferences.map(setting => this.#processKeyValue(setting))); - } - // Resolve every reference. In parallel mode the values are served from the warmed cache - // without any additional I/O. - for (const setting of secretReferences) { - const [key, value] = await this.#processKeyValue(setting); - resultHandler(key, value); + // Wait for all secret resolution promises to be resolved + await Promise.all(secretResolutionPromises); + } else { + for (const setting of secretReferences) { + const [key, value] = await this.#processKeyValue(setting); + resultHandler(key, value); + } } } diff --git a/src/keyValueAdapter.ts b/src/keyValueAdapter.ts index 222461d..865684f 100644 --- a/src/keyValueAdapter.ts +++ b/src/keyValueAdapter.ts @@ -14,6 +14,12 @@ export interface IKeyValueAdapter { */ processKeyValue(setting: ConfigurationSetting): Promise<[string, unknown]>; + /** + * Optionally batch-resolves settings ahead of processKeyValue, e.g. to deduplicate and warm up + * Key Vault secret requests so that processKeyValue only reads from cache. + */ + preload?(settings: ConfigurationSetting[]): Promise; + /** * This method is called when a change is detected in the configuration setting. */ diff --git a/src/keyvault/keyVaultKeyValueAdapter.ts b/src/keyvault/keyVaultKeyValueAdapter.ts index 2ac482d..b866e15 100644 --- a/src/keyvault/keyVaultKeyValueAdapter.ts +++ b/src/keyvault/keyVaultKeyValueAdapter.ts @@ -50,25 +50,29 @@ export class AzureKeyVaultKeyValueAdapter implements IKeyValueAdapter { } /** - * Returns the normalized Key Vault secret identifier (sourceId) for a secret reference setting, - * or undefined if the reference cannot be parsed. Used to deduplicate references that resolve to - * the same secret before resolving them. + * Deduplicates secret references by their normalized secret identifier (sourceId) and preloads each + * unique secret exactly once, warming the cache so that processKeyValue only reads from it. + * Best-effort: unparseable references are skipped and re-surfaced by processKeyValue with full context. */ - getSecretReferenceId(setting: ConfigurationSetting): string | undefined { - try { - return parseKeyVaultSecretIdentifier( - parseSecretReference(setting).value.secretId - ).sourceId; - } catch { - return undefined; + async preload(settings: ConfigurationSetting[]): Promise { + if (!this.#keyVaultOptions) { + return; // nothing to do; processKeyValue will throw the proper ArgumentError } - } - - /** - * Clears the cached secret values, throttled by the minimum secret refresh interval. - */ - clearCache(): void { - this.#keyVaultSecretProvider.clearCache(); + const uniqueSecretIdentifiers = new Map(); + for (const setting of settings) { + if (!this.canProcess(setting)) { + continue; + } + try { + const secretIdentifier = parseKeyVaultSecretIdentifier( + parseSecretReference(setting).value.secretId + ); + uniqueSecretIdentifiers.set(secretIdentifier.sourceId, secretIdentifier); // dedup by sourceId + } catch { + // Skip invalid references; processKeyValue re-parses and raises KeyVaultReferenceError with context. + } + } + await this.#keyVaultSecretProvider.preloadSecrets([...uniqueSecretIdentifiers.values()]); } async onChangeDetected(): Promise { diff --git a/src/keyvault/keyVaultSecretProvider.ts b/src/keyvault/keyVaultSecretProvider.ts index dc66fb1..aee75e3 100644 --- a/src/keyvault/keyVaultSecretProvider.ts +++ b/src/keyvault/keyVaultSecretProvider.ts @@ -9,6 +9,7 @@ import { KeyVaultReferenceErrorMessages } from "../common/errorMessages.js"; export class AzureKeyVaultSecretProvider { #keyVaultOptions: KeyVaultOptions | undefined; + #secretRefreshTimer: RefreshTimer | undefined; #minSecretRefreshTimer: RefreshTimer; #secretClients: Map; // map key vault hostname to corresponding secret client #cachedSecretValues: Map = new Map(); // map secret identifier to secret value @@ -23,6 +24,7 @@ export class AzureKeyVaultSecretProvider { } } this.#keyVaultOptions = keyVaultOptions; + this.#secretRefreshTimer = refreshTimer; this.#minSecretRefreshTimer = new RefreshTimer(MIN_SECRET_REFRESH_INTERVAL_IN_MS); this.#secretClients = new Map(); for (const client of this.#keyVaultOptions?.secretClients ?? []) { @@ -31,17 +33,53 @@ export class AzureKeyVaultSecretProvider { } } + /** + * Fetches the given unique secrets ahead of resolution to warm the cache. Honors the secret refresh + * timer and the parallel resolution option. This is best-effort: per-secret failures are swallowed so + * that the error is surfaced with full context later by getSecretValue during resolution. + */ + async preloadSecrets(secretIdentifiers: KeyVaultSecretIdentifier[]): Promise { + const loadSecret = async (secretIdentifier: KeyVaultSecretIdentifier) => { + try { + await this.#loadSecretValue(secretIdentifier); + } catch { + // Leave uncached; getSecretValue re-fetches and surfaces the error during resolution. + } + }; + + if (this.#keyVaultOptions?.parallelSecretResolutionEnabled) { + await Promise.all(secretIdentifiers.map(loadSecret)); + } else { + for (const secretIdentifier of secretIdentifiers) { + await loadSecret(secretIdentifier); + } + } + } + + /** + * Fetches a secret value into the cache if it is not cached yet, or if the secret refresh interval has + * expired. This is the only place the refresh timer gates a fetch. + */ + async #loadSecretValue(secretIdentifier: KeyVaultSecretIdentifier): Promise { + const identifierKey = secretIdentifier.sourceId; + const shouldRefresh = this.#secretRefreshTimer?.canRefresh() ?? false; + if (this.#cachedSecretValues.has(identifierKey) && !shouldRefresh) { + return; // already cached and still fresh + } + this.#cachedSecretValues.set(identifierKey, await this.#getSecretValueFromKeyVault(secretIdentifier)); + } + async getSecretValue(secretIdentifier: KeyVaultSecretIdentifier): Promise { const identifierKey = secretIdentifier.sourceId; - // Return the cached value if available. The cache is invalidated externally (on secret refresh - // or when a key-value change is detected) so a stale value is never served. + // Return the cached value if available. Freshness is handled by preloadSecrets, which warms the + // cache before resolution. if (this.#cachedSecretValues.has(identifierKey)) { return this.#cachedSecretValues.get(identifierKey); } - // Fetch the secret value from Key Vault and cache it. Failures are not cached, so a subsequent - // call will retry. + // Fallback for secrets that preload skipped or failed to fetch. Failures are not cached, so a + // subsequent call will retry. const secretValue = await this.#getSecretValueFromKeyVault(secretIdentifier); this.#cachedSecretValues.set(identifierKey, secretValue); return secretValue; diff --git a/test/keyvault.test.ts b/test/keyvault.test.ts index daa530c..80e8733 100644 --- a/test/keyvault.test.ts +++ b/test/keyvault.test.ts @@ -241,12 +241,12 @@ describe("key vault reference deduplication", function () { expect(settings.get("TestKeyVersioned")).eq("VersionedValue"); }); - it("should not cache failures and retry on a subsequent attempt", async () => { + it("should recover and not cache the failure when preload fails to fetch a secret", async () => { mockDuplicateReferences(); const client = new SecretClient("https://fake-vault-name.vault.azure.net", createMockedTokenCredential()); const stub = sinon.stub(client, "getSecret"); - // The first (deduplicated) request rejects; the retry attempt succeeds. - // If the failure were cached, the retry would never succeed. + // The preload fetch (best-effort) rejects and must not be cached; the on-demand resolution then + // recovers by re-fetching successfully. stub.onCall(0).callsFake(async () => { await sleepInMs(100); throw new Error("Key Vault unavailable"); @@ -262,14 +262,70 @@ describe("key vault reference deduplication", function () { } }); - // First round: 5 concurrent references deduped to a single failing request. - // Second round (after load retry): a single succeeding request. - expect(stub.callCount).eq(2); + // The failed preload is not cached, so all references still resolve successfully. for (const key of ["TestKey1", "TestKey2", "TestKey3", "TestKey4", "TestKey5"]) { expect(settings.get(key)).eq("SecretValue"); } }); + it("should re-fetch once per unique secret on a key-value change reload", async () => { + const sentinelEtag = "sentinel-etag"; + const kvs = [ + ...["TestKey1", "TestKey2", "TestKey3", "TestKey4", "TestKey5"].map((key) => createMockedKeyVaultReference(key, sameSecretUri)), + createMockedKeyValue({ key: "sentinel", value: "v1", etag: sentinelEtag }) + ]; + mockAppConfigurationClientListConfigurationSettings([kvs]); + mockAppConfigurationClientGetConfigurationSetting(kvs); + + const client = new SecretClient("https://fake-vault-name.vault.azure.net", createMockedTokenCredential()); + let callCount = 0; + sinon.stub(client, "getSecret").callsFake(async () => { + callCount++; + await sleepInMs(100); + return { value: `SecretValue-${callCount}` } as KeyVaultSecret; + }); + + const settings = await load(createMockedConnectionString(), { + refreshOptions: { + enabled: true, + refreshIntervalInMs: 1000, + watchedSettings: [{ key: "sentinel" }] + }, + keyVaultOptions: { + secretClients: [client], + parallelSecretResolutionEnabled: true + } + }); + // Initial load resolves the duplicates with a single request. + expect(callCount).eq(1); + + // Wait past the min secret refresh interval so the key-value change reload re-fetches secrets. + await sleepInMs(60_000 + 100); + + // Trigger a key-value change reload by changing the sentinel. + const updatedKvs = [ + ...["TestKey1", "TestKey2", "TestKey3", "TestKey4", "TestKey5"].map((key) => createMockedKeyVaultReference(key, sameSecretUri)), + createMockedKeyValue({ key: "sentinel", value: "v2", etag: "sentinel-etag-2" }) + ]; + restoreMocks(); + mockAppConfigurationClientListConfigurationSettings([updatedKvs]); + mockAppConfigurationClientGetConfigurationSetting(updatedKvs); + let reloadCallCount = 0; + sinon.stub(client, "getSecret").callsFake(async () => { + reloadCallCount++; + await sleepInMs(100); + return { value: "SecretValue-reloaded" } as KeyVaultSecret; + }); + + await settings.refresh(); + + // The key-value change reload clears the cache and re-fetches, but only once for the unique secret. + expect(reloadCallCount).eq(1); + for (const key of ["TestKey1", "TestKey2", "TestKey3", "TestKey4", "TestKey5"]) { + expect(settings.get(key)).eq("SecretValue-reloaded"); + } + }); + it("should re-fetch once per unique secret on each refresh round", async () => { mockDuplicateReferences(); const client = new SecretClient("https://fake-vault-name.vault.azure.net", createMockedTokenCredential()); From 95c62336c1148b189f7f911557937e405a160136 Mon Sep 17 00:00:00 2001 From: "Lingling Ye (from Dev Box)" Date: Tue, 14 Jul 2026 16:11:08 +0800 Subject: [PATCH 4/4] update --- src/keyValueAdapter.ts | 3 +-- src/keyvault/keyVaultKeyValueAdapter.ts | 5 ----- src/keyvault/keyVaultSecretProvider.ts | 16 ++-------------- 3 files changed, 3 insertions(+), 21 deletions(-) diff --git a/src/keyValueAdapter.ts b/src/keyValueAdapter.ts index 865684f..83b273c 100644 --- a/src/keyValueAdapter.ts +++ b/src/keyValueAdapter.ts @@ -15,8 +15,7 @@ export interface IKeyValueAdapter { processKeyValue(setting: ConfigurationSetting): Promise<[string, unknown]>; /** - * Optionally batch-resolves settings ahead of processKeyValue, e.g. to deduplicate and warm up - * Key Vault secret requests so that processKeyValue only reads from cache. + * This method deduplicates and warms up Key Vault secret requests so that processKeyValue only reads from cache. */ preload?(settings: ConfigurationSetting[]): Promise; diff --git a/src/keyvault/keyVaultKeyValueAdapter.ts b/src/keyvault/keyVaultKeyValueAdapter.ts index b866e15..78a693d 100644 --- a/src/keyvault/keyVaultKeyValueAdapter.ts +++ b/src/keyvault/keyVaultKeyValueAdapter.ts @@ -49,11 +49,6 @@ export class AzureKeyVaultKeyValueAdapter implements IKeyValueAdapter { } } - /** - * Deduplicates secret references by their normalized secret identifier (sourceId) and preloads each - * unique secret exactly once, warming the cache so that processKeyValue only reads from it. - * Best-effort: unparseable references are skipped and re-surfaced by processKeyValue with full context. - */ async preload(settings: ConfigurationSetting[]): Promise { if (!this.#keyVaultOptions) { return; // nothing to do; processKeyValue will throw the proper ArgumentError diff --git a/src/keyvault/keyVaultSecretProvider.ts b/src/keyvault/keyVaultSecretProvider.ts index aee75e3..83910be 100644 --- a/src/keyvault/keyVaultSecretProvider.ts +++ b/src/keyvault/keyVaultSecretProvider.ts @@ -33,11 +33,7 @@ export class AzureKeyVaultSecretProvider { } } - /** - * Fetches the given unique secrets ahead of resolution to warm the cache. Honors the secret refresh - * timer and the parallel resolution option. This is best-effort: per-secret failures are swallowed so - * that the error is surfaced with full context later by getSecretValue during resolution. - */ + // Fetches the given unique secrets to warm the cache. async preloadSecrets(secretIdentifiers: KeyVaultSecretIdentifier[]): Promise { const loadSecret = async (secretIdentifier: KeyVaultSecretIdentifier) => { try { @@ -56,10 +52,6 @@ export class AzureKeyVaultSecretProvider { } } - /** - * Fetches a secret value into the cache if it is not cached yet, or if the secret refresh interval has - * expired. This is the only place the refresh timer gates a fetch. - */ async #loadSecretValue(secretIdentifier: KeyVaultSecretIdentifier): Promise { const identifierKey = secretIdentifier.sourceId; const shouldRefresh = this.#secretRefreshTimer?.canRefresh() ?? false; @@ -71,15 +63,11 @@ export class AzureKeyVaultSecretProvider { async getSecretValue(secretIdentifier: KeyVaultSecretIdentifier): Promise { const identifierKey = secretIdentifier.sourceId; - - // Return the cached value if available. Freshness is handled by preloadSecrets, which warms the - // cache before resolution. if (this.#cachedSecretValues.has(identifierKey)) { return this.#cachedSecretValues.get(identifierKey); } - // Fallback for secrets that preload skipped or failed to fetch. Failures are not cached, so a - // subsequent call will retry. + // Fallback for secrets that preload skipped or failed to fetch. const secretValue = await this.#getSecretValueFromKeyVault(secretIdentifier); this.#cachedSecretValues.set(identifierKey, secretValue); return secretValue;