Enforcement of min secret refresh interval (#264)

* enforcement of min secret refresh interval

* add test

* update

Author: linglingye001

src/keyvault/keyVaultSecretProvider.ts

@@ -1,7 +1,7 @@
 // Copyright (c) Microsoft Corporation.
 // Licensed under the MIT license.
 
-import { KeyVaultOptions } from "./keyVaultOptions.js";
+import { KeyVaultOptions, MIN_SECRET_REFRESH_INTERVAL_IN_MS } from "./keyVaultOptions.js";
 import { RefreshTimer } from "../refresh/refreshTimer.js";
 import { ArgumentError } from "../common/errors.js";
 import { SecretClient, KeyVaultSecretIdentifier } from "@azure/keyvault-secrets";
@@ -10,6 +10,7 @@ import { KeyVaultReferenceErrorMessages } from "../common/errorMessages.js";
 export class AzureKeyVaultSecretProvider {
     #keyVaultOptions: KeyVaultOptions | undefined;
     #secretRefreshTimer: RefreshTimer | undefined;
+    #minSecretRefreshTimer: RefreshTimer;
     #secretClients: Map<string, SecretClient>; // map key vault hostname to corresponding secret client
     #cachedSecretValues: Map<string, any> = new Map<string, any>(); // map secret identifier to secret value
 
@@ -24,6 +25,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 ?? []) {
             const clientUrl = new URL(client.vaultUrl);
@@ -47,7 +49,10 @@ export class AzureKeyVaultSecretProvider {
     }
 
     clearCache(): void {
-        this.#cachedSecretValues.clear();
+        if (this.#minSecretRefreshTimer.canRefresh()) {
+            this.#cachedSecretValues.clear();
+            this.#minSecretRefreshTimer.reset();
+        }
     }
 
     async #getSecretValueFromKeyVault(secretIdentifier: KeyVaultSecretIdentifier): Promise<unknown> {

test/keyvault.test.ts

@@ -7,7 +7,7 @@ import chaiAsPromised from "chai-as-promised";
 chai.use(chaiAsPromised);
 const expect = chai.expect;
 import { load } from "../src/index.js";
-import { sinon, createMockedConnectionString, createMockedTokenCredential, mockAppConfigurationClientListConfigurationSettings, mockSecretClientGetSecret, restoreMocks, createMockedKeyVaultReference, sleepInMs } from "./utils/testHelper.js";
+import { sinon, createMockedConnectionString, createMockedTokenCredential, mockAppConfigurationClientListConfigurationSettings, mockAppConfigurationClientGetConfigurationSetting, mockSecretClientGetSecret, restoreMocks, createMockedKeyVaultReference, createMockedKeyValue, sleepInMs } from "./utils/testHelper.js";
 import { KeyVaultSecret, SecretClient } from "@azure/keyvault-secrets";
 import { ErrorMessages, KeyVaultReferenceErrorMessages } from "../src/common/errorMessages.js";
 
@@ -199,4 +199,151 @@ describe("key vault secret refresh", function () {
         expect(settings.get("TestKey")).eq("SecretValue - Updated");
     });
 });
+
+describe("min secret refresh interval during key-value refresh", function () {
+    let getSecretCallCount = 0;
+    let sentinelEtag = "initial-etag";
+
+    afterEach(() => {
+        restoreMocks();
+        getSecretCallCount = 0;
+    });
+
+    /**
+     * This test verifies the enforcement of the minimum secret refresh interval during key-value refresh.
+     * When key-value refresh is triggered (by a watched setting change), the provider calls clearCache()
+     * on the KeyVaultSecretProvider. However, clearCache() only clears the cache if the minimum secret
+     * refresh interval (60 seconds) has passed. This prevents overwhelming Key Vaults with too many requests.
+     */
+    it("should not re-fetch secrets when key-value refresh happens within min secret refresh interval", async () => {
+        // Setup: key vault reference + sentinel key for watching
+        const kvWithSentinel = [
+            createMockedKeyVaultReference("TestKey", "https://fake-vault-name.vault.azure.net/secrets/fakeSecretName"),
+            createMockedKeyValue({ key: "sentinel", value: "initialValue", etag: sentinelEtag })
+        ];
+        mockAppConfigurationClientListConfigurationSettings([kvWithSentinel]);
+        mockAppConfigurationClientGetConfigurationSetting(kvWithSentinel);
+
+        // Mock SecretClient with call counting
+        const client = new SecretClient("https://fake-vault-name.vault.azure.net", createMockedTokenCredential());
+        sinon.stub(client, "getSecret").callsFake(async () => {
+            getSecretCallCount++;
+            return { value: "SecretValue" } as KeyVaultSecret;
+        });
+
+        // Load with key-value refresh enabled (watching sentinel)
+        const settings = await load(createMockedConnectionString(), {
+            refreshOptions: {
+                enabled: true,
+                refreshIntervalInMs: 1000, // 1 second refresh interval for key-values
+                watchedSettings: [{ key: "sentinel" }]
+            },
+            keyVaultOptions: {
+                secretClients: [client]
+            }
+        });
+
+        expect(settings.get("TestKey")).eq("SecretValue");
+        expect(getSecretCallCount).eq(1); // Initial load fetched the secret
+
+        // Simulate sentinel change to trigger key-value refresh
+        sentinelEtag = "changed-etag-1";
+        const updatedKvs = [
+            createMockedKeyVaultReference("TestKey", "https://fake-vault-name.vault.azure.net/secrets/fakeSecretName"),
+            createMockedKeyValue({ key: "sentinel", value: "changedValue1", etag: sentinelEtag })
+        ];
+        restoreMocks();
+        mockAppConfigurationClientListConfigurationSettings([updatedKvs]);
+        mockAppConfigurationClientGetConfigurationSetting(updatedKvs);
+        sinon.stub(client, "getSecret").callsFake(async () => {
+            getSecretCallCount++;
+            return { value: "SecretValue" } as KeyVaultSecret;
+        });
+
+        // Wait for refresh interval and trigger refresh
+        await sleepInMs(1000 + 100);
+        await settings.refresh();
+
+        // Key-value refresh happened, but secret should NOT be re-fetched
+        // because min secret refresh interval (60s) hasn't passed
+        expect(getSecretCallCount).eq(1); // Still 1, no additional getSecret call
+
+        // Trigger another key-value refresh
+        sentinelEtag = "changed-etag-2";
+        const updatedKvs2 = [
+            createMockedKeyVaultReference("TestKey", "https://fake-vault-name.vault.azure.net/secrets/fakeSecretName"),
+            createMockedKeyValue({ key: "sentinel", value: "changedValue2", etag: sentinelEtag })
+        ];
+        restoreMocks();
+        mockAppConfigurationClientListConfigurationSettings([updatedKvs2]);
+        mockAppConfigurationClientGetConfigurationSetting(updatedKvs2);
+        sinon.stub(client, "getSecret").callsFake(async () => {
+            getSecretCallCount++;
+            return { value: "SecretValue" } as KeyVaultSecret;
+        });
+
+        await sleepInMs(1000 + 100);
+        await settings.refresh();
+
+        // Still no additional getSecret call due to min interval enforcement
+        expect(getSecretCallCount).eq(1);
+    });
+
+    it("should re-fetch secrets after min secret refresh interval passes during key-value refresh", async () => {
+        // Setup: key vault reference + sentinel key for watching
+        let currentSentinelValue = "initialValue";
+        sentinelEtag = "initial-etag";
+
+        const getKvs = () => [
+            createMockedKeyVaultReference("TestKey", "https://fake-vault-name.vault.azure.net/secrets/fakeSecretName"),
+            createMockedKeyValue({ key: "sentinel", value: currentSentinelValue, etag: sentinelEtag })
+        ];
+
+        mockAppConfigurationClientListConfigurationSettings([getKvs()]);
+        mockAppConfigurationClientGetConfigurationSetting(getKvs());
+
+        // Mock SecretClient with call counting
+        const client = new SecretClient("https://fake-vault-name.vault.azure.net", createMockedTokenCredential());
+        sinon.stub(client, "getSecret").callsFake(async () => {
+            getSecretCallCount++;
+            return { value: `SecretValue-${getSecretCallCount}` } as KeyVaultSecret;
+        });
+
+        // Load with key-value refresh enabled
+        const settings = await load(createMockedConnectionString(), {
+            refreshOptions: {
+                enabled: true,
+                refreshIntervalInMs: 1000,
+                watchedSettings: [{ key: "sentinel" }]
+            },
+            keyVaultOptions: {
+                secretClients: [client]
+            }
+        });
+
+        expect(settings.get("TestKey")).eq("SecretValue-1");
+        expect(getSecretCallCount).eq(1);
+
+        // Wait for min secret refresh interval (60 seconds) to pass
+        await sleepInMs(60_000 + 100);
+
+        // Now change sentinel to trigger key-value refresh
+        currentSentinelValue = "changedValue";
+        sentinelEtag = "changed-etag";
+        restoreMocks();
+        mockAppConfigurationClientListConfigurationSettings([getKvs()]);
+        mockAppConfigurationClientGetConfigurationSetting(getKvs());
+        sinon.stub(client, "getSecret").callsFake(async () => {
+            getSecretCallCount++;
+            return { value: `SecretValue-${getSecretCallCount}` } as KeyVaultSecret;
+        });
+
+        await sleepInMs(1000 + 100); // Wait for kv refresh interval
+        await settings.refresh();
+
+        // Now getSecret SHOULD be called again because min interval has passed
+        expect(getSecretCallCount).eq(2);
+        expect(settings.get("TestKey")).eq("SecretValue-2");
+    });
+});
 /* eslint-enable @typescript-eslint/no-unused-expressions */