Skip to content

Commit 414a8c8

Browse files
zhiyuanliang-mszhiyuanliang-ms
committed
disallow sentinel key refresh for AFD
1 parent d3c5f96 commit 414a8c8

File tree

7 files changed

+63
-135
lines changed

7 files changed

+63
-135
lines changed

‎src/appConfigurationImpl.ts‎

Lines changed: 32 additions & 68 deletions
Original file line numberDiff line numberDiff line change
@@ -105,16 +105,12 @@ export class AzureAppConfigurationImpl implements AzureAppConfiguration {
105105
#watchAll: boolean = false;
106106
#kvRefreshInterval: number = DEFAULT_REFRESH_INTERVAL_IN_MS;
107107
#kvRefreshTimer: RefreshTimer;
108-
#lastKvChangeDetectedTime: Date = new Date(0);
109-
#isKvStale: boolean = false;
110108

111109
// Feature flags
112110
#featureFlagEnabled: boolean = false;
113111
#featureFlagRefreshEnabled: boolean = false;
114112
#ffRefreshInterval: number = DEFAULT_REFRESH_INTERVAL_IN_MS;
115113
#ffRefreshTimer: RefreshTimer;
116-
#lastFfChangeDetectedTime: Date = new Date(0);
117-
#isFfStale: boolean = false;
118114

119115
// Key Vault references
120116
#secretRefreshEnabled: boolean = false;
@@ -173,7 +169,7 @@ export class AzureAppConfigurationImpl implements AzureAppConfiguration {
173169
if (setting.label?.includes("*") || setting.label?.includes(",")) {
174170
throw new ArgumentError(ErrorMessages.INVALID_WATCHED_SETTINGS_LABEL);
175171
}
176-
this.#sentinels.set(setting, { etag: undefined, timestamp: new Date(0) });
172+
this.#sentinels.set(setting, { etag: undefined, lastServerResponseTime: new Date(0) });
177173
}
178174
}
179175

@@ -522,19 +518,6 @@ export class AzureAppConfigurationImpl implements AzureAppConfiguration {
522518
};
523519
const { items, pageWatchers } = await this.#listConfigurationSettings(listOptions);
524520

525-
for (const pageWatcher of pageWatchers) {
526-
if (loadFeatureFlag) {
527-
this.#isFfStale = pageWatcher.timestamp < this.#lastFfChangeDetectedTime;
528-
if (this.#isFfStale) {
529-
return [];
530-
}
531-
} else {
532-
this.#isKvStale = pageWatcher.timestamp < this.#lastKvChangeDetectedTime;
533-
if (this.#isKvStale) {
534-
return [];
535-
}
536-
}
537-
}
538521
selector.pageWatchers = pageWatchers;
539522
settings = items;
540523
} else { // snapshot selector
@@ -611,7 +594,7 @@ export class AzureAppConfigurationImpl implements AzureAppConfiguration {
611594
const response = await this.#getConfigurationSetting(configurationSettingId, { onlyIfChanged: false });
612595
this.#sentinels.set(watchedSetting, {
613596
etag: isRestError(response) ? undefined : response.etag,
614-
timestamp: this.#getResponseTimestamp(response)
597+
lastServerResponseTime: this.#getResponseTimestamp(response)
615598
});
616599
}
617600
}
@@ -666,38 +649,26 @@ export class AzureAppConfigurationImpl implements AzureAppConfiguration {
666649
let needRefresh = false;
667650
let changedSentinel;
668651
let changedSentinelWatcher;
669-
if (this.#isKvStale) {
670-
needRefresh = true;
671-
// skip checking changes
652+
if (this.#watchAll) {
653+
needRefresh = await this.#checkConfigurationSettingsChange(this.#kvSelectors);
672654
} else {
673-
if (this.#watchAll) {
674-
needRefresh = await this.#checkConfigurationSettingsChange(this.#kvSelectors);
675-
} else {
676-
const getOptions: GetConfigurationSettingOptions = {
677-
// send conditional request only when CDN is not used
678-
onlyIfChanged: !this.#isCdnUsed
679-
};
680-
for (const watchedSetting of this.#sentinels.keys()) {
681-
const configurationSettingId: ConfigurationSettingId = { key: watchedSetting.key, label: watchedSetting.label };
682-
const response: GetConfigurationSettingResponse | RestError =
683-
await this.#getConfigurationSetting(configurationSettingId, getOptions);
684-
685-
const timestamp = this.#getResponseTimestamp(response);
686-
const watcher = this.#sentinels.get(watchedSetting);
687-
const isUpToDate = watcher?.timestamp ? timestamp > watcher.timestamp : true;
688-
const isDeleted = isRestError(response) && watcher?.etag !== undefined; // previously existed, now deleted
689-
const isChanged =
690-
!isRestError(response) &&
691-
response.statusCode === 200 &&
692-
watcher?.etag !== response.etag; // etag changed
693-
694-
if (isUpToDate && (isDeleted || isChanged)) {
695-
this.#lastKvChangeDetectedTime = timestamp;
696-
changedSentinel = watchedSetting;
697-
changedSentinelWatcher = watcher;
698-
needRefresh = true;
699-
break;
700-
}
655+
for (const watchedSetting of this.#sentinels.keys()) {
656+
const configurationSettingId: ConfigurationSettingId = { key: watchedSetting.key, label: watchedSetting.label };
657+
const response: GetConfigurationSettingResponse | RestError =
658+
await this.#getConfigurationSetting(configurationSettingId, { onlyIfChanged: true });
659+
660+
const watcher: SettingWatcher = this.#sentinels.get(watchedSetting)!; // watcher should always exist for sentinels
661+
const isDeleted = isRestError(response) && watcher.etag !== undefined; // previously existed, now deleted
662+
const isChanged =
663+
!isRestError(response) &&
664+
response.statusCode === 200 &&
665+
watcher.etag !== response.etag; // etag changed
666+
667+
if (isDeleted || isChanged) {
668+
changedSentinel = watchedSetting;
669+
changedSentinelWatcher = { etag: isChanged ? response.etag : undefined };
670+
needRefresh = true;
671+
break;
701672
}
702673
}
703674
}
@@ -707,11 +678,15 @@ export class AzureAppConfigurationImpl implements AzureAppConfiguration {
707678
await adapter.onChangeDetected();
708679
}
709680
await this.#loadSelectedKeyValues();
710-
this.#sentinels.set(changedSentinel, changedSentinelWatcher); // update the changed sentinel's watcher
681+
682+
if (changedSentinel && changedSentinelWatcher) {
683+
// update the changed sentinel's watcher after loading new values, this can ensure a failed refresh will retry on next refresh
684+
this.#sentinels.set(changedSentinel, changedSentinelWatcher);
685+
}
711686
}
712687

713688
this.#kvRefreshTimer.reset();
714-
return Promise.resolve(needRefresh && !this.#isKvStale);
689+
return Promise.resolve(needRefresh);
715690
}
716691

717692
/**
@@ -725,20 +700,14 @@ export class AzureAppConfigurationImpl implements AzureAppConfiguration {
725700
}
726701

727702
let needRefresh = false;
728-
if (this.#isFfStale) {
729-
needRefresh = true;
730-
// skip checking changes
731-
} else {
732-
const refreshFeatureFlag = true;
733-
needRefresh = await this.#checkConfigurationSettingsChange(this.#ffSelectors, refreshFeatureFlag);
734-
}
703+
needRefresh = await this.#checkConfigurationSettingsChange(this.#ffSelectors);
735704

736705
if (needRefresh) {
737706
await this.#loadFeatureFlags();
738707
}
739708

740709
this.#ffRefreshTimer.reset();
741-
return Promise.resolve(needRefresh && !this.#isFfStale);
710+
return Promise.resolve(needRefresh);
742711
}
743712

744713
async #refreshSecrets(): Promise<boolean> {
@@ -765,7 +734,7 @@ export class AzureAppConfigurationImpl implements AzureAppConfiguration {
765734
* @param selectors - The @see PagedSettingSelector of the kev-value collection.
766735
* @returns true if key-value collection has changed, false otherwise.
767736
*/
768-
async #checkConfigurationSettingsChange(selectors: PagedSettingsWatcher[], refreshFeatureFlag: boolean = false): Promise<boolean> {
737+
async #checkConfigurationSettingsChange(selectors: PagedSettingsWatcher[]): Promise<boolean> {
769738
const funcToExecute = async (client) => {
770739
for (const selector of selectors) {
771740
if (selector.snapshotName) { // skip snapshot selector
@@ -794,14 +763,9 @@ export class AzureAppConfigurationImpl implements AzureAppConfiguration {
794763
const timestamp = this.#getResponseTimestamp(page);
795764
// when conditional request is sent, the response will be 304 if not changed
796765
if (i >= pageWatchers.length || // new page
797-
(timestamp > pageWatchers[i].timestamp && // up to date
766+
(timestamp > pageWatchers[i].lastServerResponseTime && // up to date
798767
page._response.status === 200 && // page changed
799768
page.etag !== pageWatchers[i].etag)) {
800-
if (refreshFeatureFlag) {
801-
this.#lastFfChangeDetectedTime = timestamp;
802-
} else {
803-
this.#lastKvChangeDetectedTime = timestamp;
804-
}
805769
return true;
806770
}
807771
i++;
@@ -852,7 +816,7 @@ export class AzureAppConfigurationImpl implements AzureAppConfiguration {
852816

853817
const items: ConfigurationSetting[] = [];
854818
for await (const page of pageIterator) {
855-
pageWatchers.push({ etag: page.etag, timestamp: this.#getResponseTimestamp(page) });
819+
pageWatchers.push({ etag: page.etag, lastServerResponseTime: this.#getResponseTimestamp(page) });
856820
items.push(...page.items);
857821
}
858822
return { items, pageWatchers };

‎src/common/errorMessages.ts‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,8 @@ export const enum ErrorMessages {
2121
INVALID_TAG_FILTER = "Tag filter must follow the format 'tagName=tagValue'",
2222
CONNECTION_STRING_OR_ENDPOINT_MISSED = "A connection string or an endpoint with credential must be specified to create a client.",
2323
REPLICA_DISCOVERY_NOT_SUPPORTED = "Replica discovery is not supported when loading from Azure Front Door.",
24-
LOAD_BALANCING_NOT_SUPPORTED = "Load balancing is not supported when loading from Azure Front Door."
24+
LOAD_BALANCING_NOT_SUPPORTED = "Load balancing is not supported when loading from Azure Front Door.",
25+
WATCHED_SETTINGS_NOT_SUPPORTED = "Watched settings are not supported when loading from Azure Front Door."
2526
}
2627

2728
export const enum KeyVaultReferenceErrorMessages {

‎src/load.ts‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -83,6 +83,10 @@ export async function loadFromAzureFrontDoor(
8383
if (appConfigOptions.loadBalancingEnabled) {
8484
throw new ArgumentError(ErrorMessages.LOAD_BALANCING_NOT_SUPPORTED);
8585
}
86+
if (appConfigOptions.refreshOptions?.watchedSettings && appConfigOptions.refreshOptions.watchedSettings.length > 0) {
87+
throw new ArgumentError(ErrorMessages.WATCHED_SETTINGS_NOT_SUPPORTED);
88+
}
89+
8690
appConfigOptions.replicaDiscoveryEnabled = false; // Disable replica discovery when loading from Azure Front Door
8791

8892
appConfigOptions.clientOptions = {

‎src/types.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -95,7 +95,7 @@ export type WatchedSetting = {
9595

9696
export type SettingWatcher = {
9797
etag?: string;
98-
timestamp: Date;
98+
lastServerResponseTime: Date;
9999
}
100100

101101
export type PagedSettingsWatcher = SettingSelector & {

test/cdn.test.ts renamed to test/afd.test.ts

Lines changed: 22 additions & 63 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@ const expect = chai.expect;
99

1010
import { AppConfigurationClient } from "@azure/app-configuration";
1111
import { load, loadFromAzureFrontDoor } from "../src/index.js";
12+
import { ErrorMessages } from "../src/common/errorMessages.js";
1213
import { createMockedKeyValue, createMockedFeatureFlag, HttpRequestHeadersPolicy, getCachedIterator, sinon, restoreMocks, createMockedConnectionString, createMockedAzureFrontDoorEndpoint, sleepInMs } from "./utils/testHelper.js";
1314
import { TIMESTAMP_HEADER } from "../src/cdn/constants.js";
1415
import { isBrowser } from "../src/requestTracing/utils.js";
@@ -26,6 +27,27 @@ describe("loadFromAzureFrontDoor", function() {
2627
restoreMocks();
2728
});
2829

30+
it("should throw if watched settings are provided", async () => {
31+
await expect(loadFromAzureFrontDoor(createMockedAzureFrontDoorEndpoint(), {
32+
refreshOptions: {
33+
enabled: true,
34+
watchedSettings: [{ key: "sentinel" }]
35+
}
36+
})).to.be.rejectedWith(ErrorMessages.WATCHED_SETTINGS_NOT_SUPPORTED);
37+
});
38+
39+
it("should throw if replica discovery is enabled", async () => {
40+
await expect(loadFromAzureFrontDoor(createMockedAzureFrontDoorEndpoint(), {
41+
replicaDiscoveryEnabled: true
42+
})).to.be.rejectedWith(ErrorMessages.REPLICA_DISCOVERY_NOT_SUPPORTED);
43+
});
44+
45+
it("should throw if load balancing is enabled", async () => {
46+
await expect(loadFromAzureFrontDoor(createMockedAzureFrontDoorEndpoint(), {
47+
loadBalancingEnabled: true
48+
})).to.be.rejectedWith(ErrorMessages.LOAD_BALANCING_NOT_SUPPORTED);
49+
});
50+
2951
it("should not include authorization and sync-token header when loading from Azure Front Door", async () => {
3052
const headerPolicy = new HttpRequestHeadersPolicy();
3153
const position: "perCall" | "perRetry" = "perCall";
@@ -207,68 +229,5 @@ describe("loadFromAzureFrontDoor", function() {
207229
expect(featureFlags[0].id).to.equal("Beta");
208230
expect(featureFlags[0].enabled).to.equal(false);
209231
});
210-
211-
it("should keep refreshing key value until cache expires", async () => {
212-
let refreshSuccessfulCount = 0;
213-
const sentinel = createMockedKeyValue({ key: "sentinel", value: "initial value" });
214-
const sentinel_updated = createMockedKeyValue({ key: "sentinel", value: "updated value" });
215-
const kv1 = createMockedKeyValue({ key: "app.key1", value: "value1" });
216-
const kv2 = createMockedKeyValue({ key: "app.key2", value: "value2" });
217-
const kv2_updated = createMockedKeyValue({ key: "app.key2", value: "value2-updated" });
218-
219-
const getStub = sinon.stub(AppConfigurationClient.prototype, "getConfigurationSetting");
220-
const listStub = sinon.stub(AppConfigurationClient.prototype, "listConfigurationSettings");
221-
222-
getStub.onCall(0).returns(Promise.resolve({ statusCode: 200, _response: { headers: createTimestampHeaders("2025-09-07T00:00:00Z") }, ...sentinel } as any));
223-
getStub.onCall(1).returns(Promise.resolve({ statusCode: 200, _response: { headers: createTimestampHeaders("2025-09-07T00:00:01Z") }, ...sentinel_updated } as any));
224-
getStub.onCall(2).returns(Promise.resolve({ statusCode: 200, _response: { headers: createTimestampHeaders("2025-09-07T00:00:01Z") }, ...sentinel_updated } as any));
225-
getStub.onCall(4).returns(Promise.resolve({ statusCode: 200, _response: { headers: createTimestampHeaders("2025-09-07T00:00:00Z") }, ...sentinel } as any)); // server old value from another cache server
226-
227-
listStub.onCall(0).returns(getCachedIterator([
228-
{ items: [kv1, kv2], response: { status: 200, headers: createTimestampHeaders("2025-09-07T00:00:00Z") } }
229-
]));
230-
listStub.onCall(1).returns(getCachedIterator([
231-
{ items: [kv1, kv2], response: { status: 200, headers: createTimestampHeaders("2025-09-07T00:00:00Z") } } // cache has not expired
232-
]));
233-
listStub.onCall(2).returns(getCachedIterator([
234-
{ items: [kv1, kv2_updated], response: { status: 200, headers: createTimestampHeaders("2025-09-07T00:00:02Z") } } // cache has expired
235-
]));
236-
237-
const appConfig = await loadFromAzureFrontDoor(createMockedAzureFrontDoorEndpoint(), {
238-
selectors: [{ keyFilter: "app.*" }],
239-
refreshOptions: {
240-
enabled: true,
241-
refreshIntervalInMs: 1000,
242-
watchedSettings: [
243-
{ key: "sentinel" }
244-
]
245-
}
246-
});
247-
248-
appConfig.onRefresh(() => {
249-
refreshSuccessfulCount++;
250-
});
251-
252-
expect(appConfig.get("app.key2")).to.equal("value2");
253-
254-
await sleepInMs(1500);
255-
await appConfig.refresh();
256-
257-
// cdn cache hasn't expired, even if the sentinel key changed, key2 should still return the old value
258-
expect(appConfig.get("app.key2")).to.equal("value2");
259-
expect(refreshSuccessfulCount).to.equal(0);
260-
261-
await sleepInMs(1500);
262-
await appConfig.refresh();
263-
expect(refreshSuccessfulCount).to.equal(1);
264-
265-
// cdn cache has expired, key2 should return the updated value even if sentinel remains the same
266-
expect(appConfig.get("app.key2")).to.equal("value2-updated");
267-
268-
await sleepInMs(1500);
269-
// even if the sentinel is different from previous value, but the timestamp is older, it should not trigger refresh
270-
await appConfig.refresh();
271-
expect(refreshSuccessfulCount).to.equal(1);
272-
});
273232
});
274233
/* eslint-ensable @typescript-eslint/no-unused-expressions */

‎test/refresh.test.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -275,7 +275,7 @@ describe("dynamic refresh", function () {
275275
updateSetting("sentinel", "updatedValue");
276276
failNextListKv = true; // force next listConfigurationSettings request to fail
277277
await sleepInMs(2 * 1000 + 1);
278-
await settings.refresh(); // even if the provider detects the sentinel key change, this refresh will fail, so we won't get the updated value of sentinel
278+
await settings.refresh(); // even if the provider detects the sentinel key change, this refresh will fail, so we won't get the updated value
279279
expect(listKvRequestCount).eq(1);
280280
expect(getKvRequestCount).eq(2);
281281
expect(settings.get("app.settings.bgColor")).to.be.undefined;

‎vitest.browser.config.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,7 @@ export default defineConfig({
1616
"out/esm/test/featureFlag.test.js",
1717
"out/esm/test/json.test.js",
1818
"out/esm/test/startup.test.js",
19-
"out/esm/test/cdn.test.js"
19+
"out/esm/test/afd.test.js"
2020
],
2121
testTimeout: 200_000,
2222
hookTimeout: 200_000,

0 commit comments

Comments
 (0)