Skip to content

Commit 8e0d552

Browse files
committed
update
1 parent 0e635bb commit 8e0d552

5 files changed

Lines changed: 151 additions & 53 deletions

File tree

src/appConfigurationImpl.ts

Lines changed: 20 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -111,7 +111,6 @@ export class AzureAppConfigurationImpl implements AzureAppConfiguration {
111111
#secretReferences: ConfigurationSetting[] = []; // cached key vault references
112112
#secretRefreshTimer: RefreshTimer | undefined = undefined;
113113
#resolveSecretsInParallel: boolean = false;
114-
#keyVaultAdapter: AzureKeyVaultKeyValueAdapter;
115114

116115
/**
117116
* Selectors of key-values obtained from @see AzureAppConfigurationOptions.selectors
@@ -205,8 +204,7 @@ export class AzureAppConfigurationImpl implements AzureAppConfiguration {
205204
}
206205
this.#resolveSecretsInParallel = options.keyVaultOptions.parallelSecretResolutionEnabled ?? false;
207206
}
208-
this.#keyVaultAdapter = new AzureKeyVaultKeyValueAdapter(options?.keyVaultOptions, this.#secretRefreshTimer);
209-
this.#adapters.push(this.#keyVaultAdapter);
207+
this.#adapters.push(new AzureKeyVaultKeyValueAdapter(options?.keyVaultOptions, this.#secretRefreshTimer));
210208
this.#adapters.push(new JsonKeyValueAdapter());
211209
}
212210

@@ -571,6 +569,9 @@ export class AzureAppConfigurationImpl implements AzureAppConfiguration {
571569
}
572570

573571
if (this.#secretReferences.length > 0) {
572+
for (const adapter of this.#adapters) {
573+
await adapter.preload?.(this.#secretReferences); // dedup and warm the secret cache
574+
}
574575
await this.#resolveSecretReferences(this.#secretReferences, (key, value) => {
575576
keyValues.push([key, value]);
576577
});
@@ -717,8 +718,9 @@ export class AzureAppConfigurationImpl implements AzureAppConfiguration {
717718
return Promise.resolve(false);
718719
}
719720

720-
// Invalidate cached secret values so the refresh round re-fetches them from Key Vault.
721-
this.#keyVaultAdapter.clearCache();
721+
for (const adapter of this.#adapters) {
722+
await adapter.preload?.(this.#secretReferences); // dedup and warm the secret cache
723+
}
722724
await this.#resolveSecretReferences(this.#secretReferences, (key, value) => {
723725
this.#configMap.set(key, value);
724726
});
@@ -899,30 +901,22 @@ export class AzureAppConfigurationImpl implements AzureAppConfiguration {
899901

900902
async #resolveSecretReferences(secretReferences: ConfigurationSetting[], resultHandler: (key: string, value: unknown) => void): Promise<void> {
901903
if (this.#resolveSecretsInParallel) {
902-
// Preload phase: resolve each unique secret exactly once, in parallel, to warm the cache.
903-
// References that resolve to the same secret identifier are deduplicated so that only a
904-
// single Key Vault request is issued per unique secret.
905-
const seenSecretIds = new Set<string>();
906-
const uniqueSecretReferences: ConfigurationSetting[] = [];
904+
const secretResolutionPromises: Promise<void>[] = [];
907905
for (const setting of secretReferences) {
908-
const secretId = this.#keyVaultAdapter.getSecretReferenceId(setting);
909-
if (secretId === undefined) {
910-
// The reference cannot be parsed; resolve it individually so the appropriate error
911-
// is surfaced when it is processed below.
912-
uniqueSecretReferences.push(setting);
913-
} else if (!seenSecretIds.has(secretId)) {
914-
seenSecretIds.add(secretId);
915-
uniqueSecretReferences.push(setting);
916-
}
906+
const secretResolutionPromise = this.#processKeyValue(setting)
907+
.then(([key, value]) => {
908+
resultHandler(key, value);
909+
});
910+
secretResolutionPromises.push(secretResolutionPromise);
917911
}
918-
await Promise.all(uniqueSecretReferences.map(setting => this.#processKeyValue(setting)));
919-
}
920912

921-
// Resolve every reference. In parallel mode the values are served from the warmed cache
922-
// without any additional I/O.
923-
for (const setting of secretReferences) {
924-
const [key, value] = await this.#processKeyValue(setting);
925-
resultHandler(key, value);
913+
// Wait for all secret resolution promises to be resolved
914+
await Promise.all(secretResolutionPromises);
915+
} else {
916+
for (const setting of secretReferences) {
917+
const [key, value] = await this.#processKeyValue(setting);
918+
resultHandler(key, value);
919+
}
926920
}
927921
}
928922

src/keyValueAdapter.ts

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,12 @@ export interface IKeyValueAdapter {
1414
*/
1515
processKeyValue(setting: ConfigurationSetting): Promise<[string, unknown]>;
1616

17+
/**
18+
* Optionally batch-resolves settings ahead of processKeyValue, e.g. to deduplicate and warm up
19+
* Key Vault secret requests so that processKeyValue only reads from cache.
20+
*/
21+
preload?(settings: ConfigurationSetting[]): Promise<void>;
22+
1723
/**
1824
* This method is called when a change is detected in the configuration setting.
1925
*/

src/keyvault/keyVaultKeyValueAdapter.ts

Lines changed: 21 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -50,25 +50,29 @@ export class AzureKeyVaultKeyValueAdapter implements IKeyValueAdapter {
5050
}
5151

5252
/**
53-
* Returns the normalized Key Vault secret identifier (sourceId) for a secret reference setting,
54-
* or undefined if the reference cannot be parsed. Used to deduplicate references that resolve to
55-
* the same secret before resolving them.
53+
* Deduplicates secret references by their normalized secret identifier (sourceId) and preloads each
54+
* unique secret exactly once, warming the cache so that processKeyValue only reads from it.
55+
* Best-effort: unparseable references are skipped and re-surfaced by processKeyValue with full context.
5656
*/
57-
getSecretReferenceId(setting: ConfigurationSetting): string | undefined {
58-
try {
59-
return parseKeyVaultSecretIdentifier(
60-
parseSecretReference(setting).value.secretId
61-
).sourceId;
62-
} catch {
63-
return undefined;
57+
async preload(settings: ConfigurationSetting[]): Promise<void> {
58+
if (!this.#keyVaultOptions) {
59+
return; // nothing to do; processKeyValue will throw the proper ArgumentError
6460
}
65-
}
66-
67-
/**
68-
* Clears the cached secret values, throttled by the minimum secret refresh interval.
69-
*/
70-
clearCache(): void {
71-
this.#keyVaultSecretProvider.clearCache();
61+
const uniqueSecretIdentifiers = new Map<string, KeyVaultSecretIdentifier>();
62+
for (const setting of settings) {
63+
if (!this.canProcess(setting)) {
64+
continue;
65+
}
66+
try {
67+
const secretIdentifier = parseKeyVaultSecretIdentifier(
68+
parseSecretReference(setting).value.secretId
69+
);
70+
uniqueSecretIdentifiers.set(secretIdentifier.sourceId, secretIdentifier); // dedup by sourceId
71+
} catch {
72+
// Skip invalid references; processKeyValue re-parses and raises KeyVaultReferenceError with context.
73+
}
74+
}
75+
await this.#keyVaultSecretProvider.preloadSecrets([...uniqueSecretIdentifiers.values()]);
7276
}
7377

7478
async onChangeDetected(): Promise<void> {

src/keyvault/keyVaultSecretProvider.ts

Lines changed: 42 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@ import { KeyVaultReferenceErrorMessages } from "../common/errorMessages.js";
99

1010
export class AzureKeyVaultSecretProvider {
1111
#keyVaultOptions: KeyVaultOptions | undefined;
12+
#secretRefreshTimer: RefreshTimer | undefined;
1213
#minSecretRefreshTimer: RefreshTimer;
1314
#secretClients: Map<string, SecretClient>; // map key vault hostname to corresponding secret client
1415
#cachedSecretValues: Map<string, any> = new Map<string, any>(); // map secret identifier to secret value
@@ -23,6 +24,7 @@ export class AzureKeyVaultSecretProvider {
2324
}
2425
}
2526
this.#keyVaultOptions = keyVaultOptions;
27+
this.#secretRefreshTimer = refreshTimer;
2628
this.#minSecretRefreshTimer = new RefreshTimer(MIN_SECRET_REFRESH_INTERVAL_IN_MS);
2729
this.#secretClients = new Map();
2830
for (const client of this.#keyVaultOptions?.secretClients ?? []) {
@@ -31,17 +33,53 @@ export class AzureKeyVaultSecretProvider {
3133
}
3234
}
3335

36+
/**
37+
* Fetches the given unique secrets ahead of resolution to warm the cache. Honors the secret refresh
38+
* timer and the parallel resolution option. This is best-effort: per-secret failures are swallowed so
39+
* that the error is surfaced with full context later by getSecretValue during resolution.
40+
*/
41+
async preloadSecrets(secretIdentifiers: KeyVaultSecretIdentifier[]): Promise<void> {
42+
const loadSecret = async (secretIdentifier: KeyVaultSecretIdentifier) => {
43+
try {
44+
await this.#loadSecretValue(secretIdentifier);
45+
} catch {
46+
// Leave uncached; getSecretValue re-fetches and surfaces the error during resolution.
47+
}
48+
};
49+
50+
if (this.#keyVaultOptions?.parallelSecretResolutionEnabled) {
51+
await Promise.all(secretIdentifiers.map(loadSecret));
52+
} else {
53+
for (const secretIdentifier of secretIdentifiers) {
54+
await loadSecret(secretIdentifier);
55+
}
56+
}
57+
}
58+
59+
/**
60+
* Fetches a secret value into the cache if it is not cached yet, or if the secret refresh interval has
61+
* expired. This is the only place the refresh timer gates a fetch.
62+
*/
63+
async #loadSecretValue(secretIdentifier: KeyVaultSecretIdentifier): Promise<void> {
64+
const identifierKey = secretIdentifier.sourceId;
65+
const shouldRefresh = this.#secretRefreshTimer?.canRefresh() ?? false;
66+
if (this.#cachedSecretValues.has(identifierKey) && !shouldRefresh) {
67+
return; // already cached and still fresh
68+
}
69+
this.#cachedSecretValues.set(identifierKey, await this.#getSecretValueFromKeyVault(secretIdentifier));
70+
}
71+
3472
async getSecretValue(secretIdentifier: KeyVaultSecretIdentifier): Promise<unknown> {
3573
const identifierKey = secretIdentifier.sourceId;
3674

37-
// Return the cached value if available. The cache is invalidated externally (on secret refresh
38-
// or when a key-value change is detected) so a stale value is never served.
75+
// Return the cached value if available. Freshness is handled by preloadSecrets, which warms the
76+
// cache before resolution.
3977
if (this.#cachedSecretValues.has(identifierKey)) {
4078
return this.#cachedSecretValues.get(identifierKey);
4179
}
4280

43-
// Fetch the secret value from Key Vault and cache it. Failures are not cached, so a subsequent
44-
// call will retry.
81+
// Fallback for secrets that preload skipped or failed to fetch. Failures are not cached, so a
82+
// subsequent call will retry.
4583
const secretValue = await this.#getSecretValueFromKeyVault(secretIdentifier);
4684
this.#cachedSecretValues.set(identifierKey, secretValue);
4785
return secretValue;

test/keyvault.test.ts

Lines changed: 62 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -241,12 +241,12 @@ describe("key vault reference deduplication", function () {
241241
expect(settings.get("TestKeyVersioned")).eq("VersionedValue");
242242
});
243243

244-
it("should not cache failures and retry on a subsequent attempt", async () => {
244+
it("should recover and not cache the failure when preload fails to fetch a secret", async () => {
245245
mockDuplicateReferences();
246246
const client = new SecretClient("https://fake-vault-name.vault.azure.net", createMockedTokenCredential());
247247
const stub = sinon.stub(client, "getSecret");
248-
// The first (deduplicated) request rejects; the retry attempt succeeds.
249-
// If the failure were cached, the retry would never succeed.
248+
// The preload fetch (best-effort) rejects and must not be cached; the on-demand resolution then
249+
// recovers by re-fetching successfully.
250250
stub.onCall(0).callsFake(async () => {
251251
await sleepInMs(100);
252252
throw new Error("Key Vault unavailable");
@@ -262,14 +262,70 @@ describe("key vault reference deduplication", function () {
262262
}
263263
});
264264

265-
// First round: 5 concurrent references deduped to a single failing request.
266-
// Second round (after load retry): a single succeeding request.
267-
expect(stub.callCount).eq(2);
265+
// The failed preload is not cached, so all references still resolve successfully.
268266
for (const key of ["TestKey1", "TestKey2", "TestKey3", "TestKey4", "TestKey5"]) {
269267
expect(settings.get(key)).eq("SecretValue");
270268
}
271269
});
272270

271+
it("should re-fetch once per unique secret on a key-value change reload", async () => {
272+
const sentinelEtag = "sentinel-etag";
273+
const kvs = [
274+
...["TestKey1", "TestKey2", "TestKey3", "TestKey4", "TestKey5"].map((key) => createMockedKeyVaultReference(key, sameSecretUri)),
275+
createMockedKeyValue({ key: "sentinel", value: "v1", etag: sentinelEtag })
276+
];
277+
mockAppConfigurationClientListConfigurationSettings([kvs]);
278+
mockAppConfigurationClientGetConfigurationSetting(kvs);
279+
280+
const client = new SecretClient("https://fake-vault-name.vault.azure.net", createMockedTokenCredential());
281+
let callCount = 0;
282+
sinon.stub(client, "getSecret").callsFake(async () => {
283+
callCount++;
284+
await sleepInMs(100);
285+
return { value: `SecretValue-${callCount}` } as KeyVaultSecret;
286+
});
287+
288+
const settings = await load(createMockedConnectionString(), {
289+
refreshOptions: {
290+
enabled: true,
291+
refreshIntervalInMs: 1000,
292+
watchedSettings: [{ key: "sentinel" }]
293+
},
294+
keyVaultOptions: {
295+
secretClients: [client],
296+
parallelSecretResolutionEnabled: true
297+
}
298+
});
299+
// Initial load resolves the duplicates with a single request.
300+
expect(callCount).eq(1);
301+
302+
// Wait past the min secret refresh interval so the key-value change reload re-fetches secrets.
303+
await sleepInMs(60_000 + 100);
304+
305+
// Trigger a key-value change reload by changing the sentinel.
306+
const updatedKvs = [
307+
...["TestKey1", "TestKey2", "TestKey3", "TestKey4", "TestKey5"].map((key) => createMockedKeyVaultReference(key, sameSecretUri)),
308+
createMockedKeyValue({ key: "sentinel", value: "v2", etag: "sentinel-etag-2" })
309+
];
310+
restoreMocks();
311+
mockAppConfigurationClientListConfigurationSettings([updatedKvs]);
312+
mockAppConfigurationClientGetConfigurationSetting(updatedKvs);
313+
let reloadCallCount = 0;
314+
sinon.stub(client, "getSecret").callsFake(async () => {
315+
reloadCallCount++;
316+
await sleepInMs(100);
317+
return { value: "SecretValue-reloaded" } as KeyVaultSecret;
318+
});
319+
320+
await settings.refresh();
321+
322+
// The key-value change reload clears the cache and re-fetches, but only once for the unique secret.
323+
expect(reloadCallCount).eq(1);
324+
for (const key of ["TestKey1", "TestKey2", "TestKey3", "TestKey4", "TestKey5"]) {
325+
expect(settings.get(key)).eq("SecretValue-reloaded");
326+
}
327+
});
328+
273329
it("should re-fetch once per unique secret on each refresh round", async () => {
274330
mockDuplicateReferences();
275331
const client = new SecretClient("https://fake-vault-name.vault.azure.net", createMockedTokenCredential());

0 commit comments

Comments
 (0)