From 1fe880b19c1281bd3dc7ffa7f4c3e81d878b53b6 Mon Sep 17 00:00:00 2001 From: AMER JUSUPOVIC Date: Wed, 28 Aug 2024 10:52:15 -0700 Subject: [PATCH 01/10] increase default maxretries for key vault client to max value --- .../AzureAppConfigurationKeyVaultOptions.cs | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/src/Microsoft.Extensions.Configuration.AzureAppConfiguration/AzureAppConfigurationKeyVaultOptions.cs b/src/Microsoft.Extensions.Configuration.AzureAppConfiguration/AzureAppConfigurationKeyVaultOptions.cs index 7d32f3a7..1c4e38a8 100644 --- a/src/Microsoft.Extensions.Configuration.AzureAppConfiguration/AzureAppConfigurationKeyVaultOptions.cs +++ b/src/Microsoft.Extensions.Configuration.AzureAppConfiguration/AzureAppConfigurationKeyVaultOptions.cs @@ -15,7 +15,12 @@ namespace Microsoft.Extensions.Configuration.AzureAppConfiguration public class AzureAppConfigurationKeyVaultOptions { internal TokenCredential Credential; - internal SecretClientOptions ClientOptions = new SecretClientOptions(); + internal SecretClientOptions ClientOptions = new SecretClientOptions + { + Retry = { + MaxRetries = int.MaxValue + } + }; internal List SecretClients = new List(); internal Func> SecretResolver; internal Dictionary SecretRefreshIntervals = new Dictionary(); From 99ff10509a1d91ae7fed45fbb1fd3f1b17d5f6b7 Mon Sep 17 00:00:00 2001 From: AMER JUSUPOVIC Date: Thu, 19 Sep 2024 13:54:30 -0700 Subject: [PATCH 02/10] update minimum secret refresh interval to 1 minute, enforce for refresh all operations as well --- .../AzureKeyVaultSecretProvider.cs | 19 ++++++++++++++----- .../CachedKeyVaultSecret.cs | 14 +++++++++++++- .../Constants/RefreshConstants.cs | 2 +- .../KeyVaultReferenceTests.cs | 6 +++--- .../LoggingTests.cs | 2 +- tests/Tests.AzureAppConfiguration/MapTests.cs | 4 ++-- 6 files changed, 34 insertions(+), 13 deletions(-) diff --git a/src/Microsoft.Extensions.Configuration.AzureAppConfiguration/AzureKeyVaultReference/AzureKeyVaultSecretProvider.cs b/src/Microsoft.Extensions.Configuration.AzureAppConfiguration/AzureKeyVaultReference/AzureKeyVaultSecretProvider.cs index 86e61429..6902db47 100644 --- a/src/Microsoft.Extensions.Configuration.AzureAppConfiguration/AzureKeyVaultReference/AzureKeyVaultSecretProvider.cs +++ b/src/Microsoft.Extensions.Configuration.AzureAppConfiguration/AzureKeyVaultReference/AzureKeyVaultSecretProvider.cs @@ -5,7 +5,6 @@ using Microsoft.Extensions.Configuration.AzureAppConfiguration.Extensions; using System; using System.Collections.Generic; -using System.Linq; using System.Threading; using System.Threading.Tasks; @@ -40,6 +39,7 @@ public async Task GetSecretValue(KeyVaultSecretIdentifier secretIdentifi string secretValue = null; if (_cachedKeyVaultSecrets.TryGetValue(key, out CachedKeyVaultSecret cachedSecret) && + (cachedSecret.SourceId == secretIdentifier.SourceId) && (!cachedSecret.RefreshAt.HasValue || DateTimeOffset.UtcNow < cachedSecret.RefreshAt.Value)) { return cachedSecret.SecretValue; @@ -68,7 +68,7 @@ public async Task GetSecretValue(KeyVaultSecretIdentifier secretIdentifi secretValue = await _keyVaultOptions.SecretResolver(secretIdentifier.SourceId).ConfigureAwait(false); } - cachedSecret = new CachedKeyVaultSecret(secretValue); + cachedSecret = new CachedKeyVaultSecret(secretValue, secretIdentifier.SourceId); success = true; } finally @@ -86,9 +86,18 @@ public bool ShouldRefreshKeyVaultSecrets() public void ClearCache() { - _cachedKeyVaultSecrets.Clear(); - _nextRefreshKey = null; - _nextRefreshTime = null; + foreach (KeyValuePair secret in _cachedKeyVaultSecrets) + { + if (secret.Value.LastRefreshTime + RefreshConstants.MinimumSecretRefreshInterval < DateTimeOffset.UtcNow) + { + _cachedKeyVaultSecrets.Remove(secret.Key); + } + + if (secret.Key == _nextRefreshKey) + { + UpdateNextRefreshableSecretFromCache(); + } + } } public void RemoveSecretFromCache(string key) diff --git a/src/Microsoft.Extensions.Configuration.AzureAppConfiguration/AzureKeyVaultReference/CachedKeyVaultSecret.cs b/src/Microsoft.Extensions.Configuration.AzureAppConfiguration/AzureKeyVaultReference/CachedKeyVaultSecret.cs index 1b813d36..98eb0ffb 100644 --- a/src/Microsoft.Extensions.Configuration.AzureAppConfiguration/AzureKeyVaultReference/CachedKeyVaultSecret.cs +++ b/src/Microsoft.Extensions.Configuration.AzureAppConfiguration/AzureKeyVaultReference/CachedKeyVaultSecret.cs @@ -22,11 +22,23 @@ internal class CachedKeyVaultSecret /// public int RefreshAttempts { get; set; } - public CachedKeyVaultSecret(string secretValue = null, DateTimeOffset? refreshAt = null, int refreshAttempts = 0) + /// + /// The last time this secret was reloaded from Key Vault. + /// + public DateTimeOffset? LastRefreshTime { get; set; } + + /// + /// The source for this secret. + /// + public Uri SourceId { get; } + + public CachedKeyVaultSecret(string secretValue = null, Uri sourceId = null, DateTimeOffset? refreshAt = null, int refreshAttempts = 0) { SecretValue = secretValue; RefreshAt = refreshAt; + LastRefreshTime = DateTimeOffset.UtcNow; RefreshAttempts = refreshAttempts; + SourceId = sourceId; } } } diff --git a/src/Microsoft.Extensions.Configuration.AzureAppConfiguration/Constants/RefreshConstants.cs b/src/Microsoft.Extensions.Configuration.AzureAppConfiguration/Constants/RefreshConstants.cs index 5805191a..954e3775 100644 --- a/src/Microsoft.Extensions.Configuration.AzureAppConfiguration/Constants/RefreshConstants.cs +++ b/src/Microsoft.Extensions.Configuration.AzureAppConfiguration/Constants/RefreshConstants.cs @@ -16,7 +16,7 @@ internal class RefreshConstants public static readonly TimeSpan MinimumFeatureFlagsCacheExpirationInterval = TimeSpan.FromSeconds(1); // Key Vault secrets - public static readonly TimeSpan MinimumSecretRefreshInterval = TimeSpan.FromSeconds(1); + public static readonly TimeSpan MinimumSecretRefreshInterval = TimeSpan.FromMinutes(1); // Backoff during refresh failures public static readonly TimeSpan DefaultMinBackoff = TimeSpan.FromSeconds(30); diff --git a/tests/Tests.AzureAppConfiguration/KeyVaultReferenceTests.cs b/tests/Tests.AzureAppConfiguration/KeyVaultReferenceTests.cs index d8c832bf..75d07bcd 100644 --- a/tests/Tests.AzureAppConfiguration/KeyVaultReferenceTests.cs +++ b/tests/Tests.AzureAppConfiguration/KeyVaultReferenceTests.cs @@ -830,7 +830,7 @@ Response GetIfChanged(ConfigurationSetting setting, bool o public async Task SecretIsReloadedFromKeyVaultWhenCacheExpires() { IConfigurationRefresher refresher = null; - TimeSpan cacheExpirationTime = TimeSpan.FromSeconds(1); + TimeSpan cacheExpirationTime = TimeSpan.FromSeconds(60); var mockResponse = new Mock(); var mockClient = new Mock(MockBehavior.Strict); @@ -873,7 +873,7 @@ public async Task SecretIsReloadedFromKeyVaultWhenCacheExpires() public async Task SecretsWithDefaultRefreshInterval() { IConfigurationRefresher refresher = null; - TimeSpan shortCacheExpirationTime = TimeSpan.FromSeconds(1); + TimeSpan shortCacheExpirationTime = TimeSpan.FromSeconds(60); var mockResponse = new Mock(); var mockClient = new Mock(MockBehavior.Strict); @@ -918,7 +918,7 @@ public async Task SecretsWithDefaultRefreshInterval() public async Task SecretsWithDifferentRefreshIntervals() { IConfigurationRefresher refresher = null; - TimeSpan shortCacheExpirationTime = TimeSpan.FromSeconds(1); + TimeSpan shortCacheExpirationTime = TimeSpan.FromSeconds(60); TimeSpan longCacheExpirationTime = TimeSpan.FromDays(1); var mockResponse = new Mock(); diff --git a/tests/Tests.AzureAppConfiguration/LoggingTests.cs b/tests/Tests.AzureAppConfiguration/LoggingTests.cs index 04ddf3d9..e4392e89 100644 --- a/tests/Tests.AzureAppConfiguration/LoggingTests.cs +++ b/tests/Tests.AzureAppConfiguration/LoggingTests.cs @@ -582,7 +582,7 @@ public async Task ValidateCorrectKeyVaultSecretLoggedDuringRefresh() refreshOptions.Register("TestKey1", "label", true) .SetCacheExpiration(CacheExpirationTime); }); - options.ConfigureKeyVault(kv => kv.Register(mockSecretClient.Object).SetSecretRefreshInterval(CacheExpirationTime)); + options.ConfigureKeyVault(kv => kv.Register(mockSecretClient.Object).SetSecretRefreshInterval(TimeSpan.FromSeconds(60))); refresher = options.GetRefresher(); }) .Build(); diff --git a/tests/Tests.AzureAppConfiguration/MapTests.cs b/tests/Tests.AzureAppConfiguration/MapTests.cs index 88fdaa6e..56e9fc7a 100644 --- a/tests/Tests.AzureAppConfiguration/MapTests.cs +++ b/tests/Tests.AzureAppConfiguration/MapTests.cs @@ -410,7 +410,7 @@ public void MapResolveKeyVaultReferenceThrowsExceptionInAdapter() .AddAzureAppConfiguration(options => { options.ClientManager = mockClientManager; - options.ConfigureKeyVault(kv => kv.Register(mockSecretClient.Object).SetSecretRefreshInterval(TimeSpan.FromSeconds(1))); + options.ConfigureKeyVault(kv => kv.Register(mockSecretClient.Object).SetSecretRefreshInterval(TimeSpan.FromSeconds(60))); options.Map((setting) => { if (setting.ContentType == KeyVaultConstants.ContentType + "; charset=utf-8") @@ -446,7 +446,7 @@ public void MapAsyncResolveKeyVaultReference() .AddAzureAppConfiguration(options => { options.ClientManager = mockClientManager; - options.ConfigureKeyVault(kv => kv.Register(mockSecretClient.Object).SetSecretRefreshInterval(TimeSpan.FromSeconds(1))); + options.ConfigureKeyVault(kv => kv.Register(mockSecretClient.Object).SetSecretRefreshInterval(TimeSpan.FromSeconds(60))); options.Map(async (setting) => { if (setting.ContentType == KeyVaultConstants.ContentType + "; charset=utf-8") From 15276c910efd6c6d175b4ed92f8579e221d097bf Mon Sep 17 00:00:00 2001 From: AMER JUSUPOVIC Date: Thu, 19 Sep 2024 15:44:50 -0700 Subject: [PATCH 03/10] PR comments --- .../AzureKeyVaultSecretProvider.cs | 9 +++++---- .../AzureKeyVaultReference/CachedKeyVaultSecret.cs | 2 +- .../KeyVaultReferenceTests.cs | 2 +- 3 files changed, 7 insertions(+), 6 deletions(-) diff --git a/src/Microsoft.Extensions.Configuration.AzureAppConfiguration/AzureKeyVaultReference/AzureKeyVaultSecretProvider.cs b/src/Microsoft.Extensions.Configuration.AzureAppConfiguration/AzureKeyVaultReference/AzureKeyVaultSecretProvider.cs index 6902db47..1119a22d 100644 --- a/src/Microsoft.Extensions.Configuration.AzureAppConfiguration/AzureKeyVaultReference/AzureKeyVaultSecretProvider.cs +++ b/src/Microsoft.Extensions.Configuration.AzureAppConfiguration/AzureKeyVaultReference/AzureKeyVaultSecretProvider.cs @@ -5,6 +5,7 @@ using Microsoft.Extensions.Configuration.AzureAppConfiguration.Extensions; using System; using System.Collections.Generic; +using System.Net.Sockets; using System.Threading; using System.Threading.Tasks; @@ -92,11 +93,11 @@ public void ClearCache() { _cachedKeyVaultSecrets.Remove(secret.Key); } + } - if (secret.Key == _nextRefreshKey) - { - UpdateNextRefreshableSecretFromCache(); - } + if (_nextRefreshKey != null && !_cachedKeyVaultSecrets.ContainsKey(_nextRefreshKey)) + { + UpdateNextRefreshableSecretFromCache(); } } diff --git a/src/Microsoft.Extensions.Configuration.AzureAppConfiguration/AzureKeyVaultReference/CachedKeyVaultSecret.cs b/src/Microsoft.Extensions.Configuration.AzureAppConfiguration/AzureKeyVaultReference/CachedKeyVaultSecret.cs index 98eb0ffb..ab09311e 100644 --- a/src/Microsoft.Extensions.Configuration.AzureAppConfiguration/AzureKeyVaultReference/CachedKeyVaultSecret.cs +++ b/src/Microsoft.Extensions.Configuration.AzureAppConfiguration/AzureKeyVaultReference/CachedKeyVaultSecret.cs @@ -25,7 +25,7 @@ internal class CachedKeyVaultSecret /// /// The last time this secret was reloaded from Key Vault. /// - public DateTimeOffset? LastRefreshTime { get; set; } + public DateTimeOffset LastRefreshTime { get; set; } /// /// The source for this secret. diff --git a/tests/Tests.AzureAppConfiguration/KeyVaultReferenceTests.cs b/tests/Tests.AzureAppConfiguration/KeyVaultReferenceTests.cs index 75d07bcd..9517b709 100644 --- a/tests/Tests.AzureAppConfiguration/KeyVaultReferenceTests.cs +++ b/tests/Tests.AzureAppConfiguration/KeyVaultReferenceTests.cs @@ -758,7 +758,7 @@ Response GetIfChanged(ConfigurationSetting setting, bool o public async Task CachedSecretIsInvalidatedWhenRefreshAllIsTrue() { IConfigurationRefresher refresher = null; - TimeSpan cacheExpirationTime = TimeSpan.FromSeconds(1); + TimeSpan cacheExpirationTime = TimeSpan.FromSeconds(60); var mockResponse = new Mock(); var mockClient = new Mock(MockBehavior.Strict); From 0daefb1d0a13eaaa5c9ce5f791d0f60e2d78b123 Mon Sep 17 00:00:00 2001 From: AMER JUSUPOVIC Date: Fri, 20 Sep 2024 12:45:33 -0700 Subject: [PATCH 04/10] PR comments --- .../AzureKeyVaultSecretProvider.cs | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/src/Microsoft.Extensions.Configuration.AzureAppConfiguration/AzureKeyVaultReference/AzureKeyVaultSecretProvider.cs b/src/Microsoft.Extensions.Configuration.AzureAppConfiguration/AzureKeyVaultReference/AzureKeyVaultSecretProvider.cs index 1119a22d..afa8af0f 100644 --- a/src/Microsoft.Extensions.Configuration.AzureAppConfiguration/AzureKeyVaultReference/AzureKeyVaultSecretProvider.cs +++ b/src/Microsoft.Extensions.Configuration.AzureAppConfiguration/AzureKeyVaultReference/AzureKeyVaultSecretProvider.cs @@ -5,6 +5,7 @@ using Microsoft.Extensions.Configuration.AzureAppConfiguration.Extensions; using System; using System.Collections.Generic; +using System.Linq; using System.Net.Sockets; using System.Threading; using System.Threading.Tasks; @@ -87,15 +88,22 @@ public bool ShouldRefreshKeyVaultSecrets() public void ClearCache() { + var keysToRemove = new List(); + foreach (KeyValuePair secret in _cachedKeyVaultSecrets) { if (secret.Value.LastRefreshTime + RefreshConstants.MinimumSecretRefreshInterval < DateTimeOffset.UtcNow) { - _cachedKeyVaultSecrets.Remove(secret.Key); + keysToRemove.Add(secret.Key); } } - if (_nextRefreshKey != null && !_cachedKeyVaultSecrets.ContainsKey(_nextRefreshKey)) + foreach (string key in keysToRemove) + { + _cachedKeyVaultSecrets.Remove(key); + } + + if (_cachedKeyVaultSecrets.Any()) { UpdateNextRefreshableSecretFromCache(); } From 960069f576fcdb40a66a8e7e6481e1fccadb3193 Mon Sep 17 00:00:00 2001 From: AMER JUSUPOVIC Date: Fri, 20 Sep 2024 12:49:44 -0700 Subject: [PATCH 05/10] update max retries default for key vault --- .../AzureAppConfigurationKeyVaultOptions.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Microsoft.Extensions.Configuration.AzureAppConfiguration/AzureAppConfigurationKeyVaultOptions.cs b/src/Microsoft.Extensions.Configuration.AzureAppConfiguration/AzureAppConfigurationKeyVaultOptions.cs index 1c4e38a8..520e3af0 100644 --- a/src/Microsoft.Extensions.Configuration.AzureAppConfiguration/AzureAppConfigurationKeyVaultOptions.cs +++ b/src/Microsoft.Extensions.Configuration.AzureAppConfiguration/AzureAppConfigurationKeyVaultOptions.cs @@ -18,7 +18,7 @@ public class AzureAppConfigurationKeyVaultOptions internal SecretClientOptions ClientOptions = new SecretClientOptions { Retry = { - MaxRetries = int.MaxValue + MaxRetries = 6 } }; internal List SecretClients = new List(); From bd6930c31f68ce4e673d25d240a81be283c7860c Mon Sep 17 00:00:00 2001 From: AMER JUSUPOVIC Date: Wed, 25 Sep 2024 12:43:00 -0700 Subject: [PATCH 06/10] update to use sourceid as key in cached secret dictionary --- .../AzureKeyVaultKeyValueAdapter.cs | 10 ++++- .../AzureKeyVaultSecretProvider.cs | 42 +++++++++---------- .../ConfigurationClientExtensions.cs | 5 +++ .../KeyValueChange.cs | 2 + 4 files changed, 36 insertions(+), 23 deletions(-) diff --git a/src/Microsoft.Extensions.Configuration.AzureAppConfiguration/AzureKeyVaultReference/AzureKeyVaultKeyValueAdapter.cs b/src/Microsoft.Extensions.Configuration.AzureAppConfiguration/AzureKeyVaultReference/AzureKeyVaultKeyValueAdapter.cs index 9580a1cc..f0880809 100644 --- a/src/Microsoft.Extensions.Configuration.AzureAppConfiguration/AzureKeyVaultReference/AzureKeyVaultKeyValueAdapter.cs +++ b/src/Microsoft.Extensions.Configuration.AzureAppConfiguration/AzureKeyVaultReference/AzureKeyVaultKeyValueAdapter.cs @@ -84,7 +84,15 @@ public void InvalidateCache(ConfigurationSetting setting = null) } else { - _secretProvider.RemoveSecretFromCache(setting.Key); + if (CanProcess(setting)) + { + string secretRefUri = ParseSecretReferenceUri(setting); + + if (!string.IsNullOrEmpty(secretRefUri) && Uri.TryCreate(secretRefUri, UriKind.Absolute, out Uri secretUri) && KeyVaultSecretIdentifier.TryCreate(secretUri, out KeyVaultSecretIdentifier secretIdentifier)) + { + _secretProvider.RemoveSecretFromCache(secretIdentifier.SourceId); + } + } } } diff --git a/src/Microsoft.Extensions.Configuration.AzureAppConfiguration/AzureKeyVaultReference/AzureKeyVaultSecretProvider.cs b/src/Microsoft.Extensions.Configuration.AzureAppConfiguration/AzureKeyVaultReference/AzureKeyVaultSecretProvider.cs index afa8af0f..ce091deb 100644 --- a/src/Microsoft.Extensions.Configuration.AzureAppConfiguration/AzureKeyVaultReference/AzureKeyVaultSecretProvider.cs +++ b/src/Microsoft.Extensions.Configuration.AzureAppConfiguration/AzureKeyVaultReference/AzureKeyVaultSecretProvider.cs @@ -6,7 +6,6 @@ using System; using System.Collections.Generic; using System.Linq; -using System.Net.Sockets; using System.Threading; using System.Threading.Tasks; @@ -16,14 +15,14 @@ internal class AzureKeyVaultSecretProvider { private readonly AzureAppConfigurationKeyVaultOptions _keyVaultOptions; private readonly IDictionary _secretClients; - private readonly Dictionary _cachedKeyVaultSecrets; - private string _nextRefreshKey; + private readonly Dictionary _cachedKeyVaultSecrets; + private Uri _nextRefreshSourceId; private DateTimeOffset? _nextRefreshTime; public AzureKeyVaultSecretProvider(AzureAppConfigurationKeyVaultOptions keyVaultOptions = null) { _keyVaultOptions = keyVaultOptions ?? new AzureAppConfigurationKeyVaultOptions(); - _cachedKeyVaultSecrets = new Dictionary(StringComparer.OrdinalIgnoreCase); + _cachedKeyVaultSecrets = new Dictionary(); _secretClients = new Dictionary(StringComparer.OrdinalIgnoreCase); if (_keyVaultOptions.SecretClients != null) @@ -40,8 +39,7 @@ public async Task GetSecretValue(KeyVaultSecretIdentifier secretIdentifi { string secretValue = null; - if (_cachedKeyVaultSecrets.TryGetValue(key, out CachedKeyVaultSecret cachedSecret) && - (cachedSecret.SourceId == secretIdentifier.SourceId) && + if (_cachedKeyVaultSecrets.TryGetValue(secretIdentifier.SourceId, out CachedKeyVaultSecret cachedSecret) && (!cachedSecret.RefreshAt.HasValue || DateTimeOffset.UtcNow < cachedSecret.RefreshAt.Value)) { return cachedSecret.SecretValue; @@ -75,7 +73,7 @@ public async Task GetSecretValue(KeyVaultSecretIdentifier secretIdentifi } finally { - SetSecretInCache(key, cachedSecret, success); + SetSecretInCache(secretIdentifier.SourceId, key, cachedSecret, success); } return secretValue; @@ -88,19 +86,19 @@ public bool ShouldRefreshKeyVaultSecrets() public void ClearCache() { - var keysToRemove = new List(); + var sourceIdsToRemove = new List(); - foreach (KeyValuePair secret in _cachedKeyVaultSecrets) + foreach (KeyValuePair secret in _cachedKeyVaultSecrets) { if (secret.Value.LastRefreshTime + RefreshConstants.MinimumSecretRefreshInterval < DateTimeOffset.UtcNow) { - keysToRemove.Add(secret.Key); + sourceIdsToRemove.Add(secret.Key); } } - foreach (string key in keysToRemove) + foreach (Uri sourceId in sourceIdsToRemove) { - _cachedKeyVaultSecrets.Remove(key); + _cachedKeyVaultSecrets.Remove(sourceId); } if (_cachedKeyVaultSecrets.Any()) @@ -109,11 +107,11 @@ public void ClearCache() } } - public void RemoveSecretFromCache(string key) + public void RemoveSecretFromCache(Uri sourceId) { - _cachedKeyVaultSecrets.Remove(key); + _cachedKeyVaultSecrets.Remove(sourceId); - if (key == _nextRefreshKey) + if (sourceId == _nextRefreshSourceId) { UpdateNextRefreshableSecretFromCache(); } @@ -143,7 +141,7 @@ private SecretClient GetSecretClient(Uri secretUri) return client; } - private void SetSecretInCache(string key, CachedKeyVaultSecret cachedSecret, bool success = true) + private void SetSecretInCache(Uri sourceId, string key, CachedKeyVaultSecret cachedSecret, bool success = true) { if (cachedSecret == null) { @@ -151,31 +149,31 @@ private void SetSecretInCache(string key, CachedKeyVaultSecret cachedSecret, boo } UpdateCacheExpirationTimeForSecret(key, cachedSecret, success); - _cachedKeyVaultSecrets[key] = cachedSecret; + _cachedKeyVaultSecrets[sourceId] = cachedSecret; - if (key == _nextRefreshKey) + if (sourceId == _nextRefreshSourceId) { UpdateNextRefreshableSecretFromCache(); } else if ((cachedSecret.RefreshAt.HasValue && _nextRefreshTime.HasValue && cachedSecret.RefreshAt.Value < _nextRefreshTime.Value) || (cachedSecret.RefreshAt.HasValue && !_nextRefreshTime.HasValue)) { - _nextRefreshKey = key; + _nextRefreshSourceId = sourceId; _nextRefreshTime = cachedSecret.RefreshAt.Value; } } private void UpdateNextRefreshableSecretFromCache() { - _nextRefreshKey = null; + _nextRefreshSourceId = null; _nextRefreshTime = DateTimeOffset.MaxValue; - foreach (KeyValuePair secret in _cachedKeyVaultSecrets) + foreach (KeyValuePair secret in _cachedKeyVaultSecrets) { if (secret.Value.RefreshAt.HasValue && secret.Value.RefreshAt.Value < _nextRefreshTime) { _nextRefreshTime = secret.Value.RefreshAt; - _nextRefreshKey = secret.Key; + _nextRefreshSourceId = secret.Key; } } diff --git a/src/Microsoft.Extensions.Configuration.AzureAppConfiguration/Extensions/ConfigurationClientExtensions.cs b/src/Microsoft.Extensions.Configuration.AzureAppConfiguration/Extensions/ConfigurationClientExtensions.cs index 18c03178..d479ad6b 100644 --- a/src/Microsoft.Extensions.Configuration.AzureAppConfiguration/Extensions/ConfigurationClientExtensions.cs +++ b/src/Microsoft.Extensions.Configuration.AzureAppConfiguration/Extensions/ConfigurationClientExtensions.cs @@ -36,6 +36,7 @@ public static async Task GetKeyValueChange(this ConfigurationCli return new KeyValueChange { ChangeType = KeyValueChangeType.Modified, + Previous = setting, Current = response.Value, Key = setting.Key, Label = setting.Label @@ -47,6 +48,7 @@ public static async Task GetKeyValueChange(this ConfigurationCli return new KeyValueChange { ChangeType = KeyValueChangeType.Deleted, + Previous = setting, Current = null, Key = setting.Key, Label = setting.Label @@ -56,6 +58,7 @@ public static async Task GetKeyValueChange(this ConfigurationCli return new KeyValueChange { ChangeType = KeyValueChangeType.None, + Previous = setting, Current = setting, Key = setting.Key, Label = setting.Label @@ -158,6 +161,7 @@ await TracingUtils.CallWithRequestTracing(options.RequestTracingEnabled, Request ChangeType = KeyValueChangeType.Modified, Key = setting.Key, Label = options.Label.NormalizeNull(), + Previous = null, Current = setting }); string key = setting.Key.Substring(FeatureManagementConstants.FeatureFlagMarker.Length); @@ -176,6 +180,7 @@ await TracingUtils.CallWithRequestTracing(options.RequestTracingEnabled, Request ChangeType = KeyValueChangeType.Deleted, Key = kvp.Key, Label = options.Label.NormalizeNull(), + Previous = null, Current = null }); string key = kvp.Key.Substring(FeatureManagementConstants.FeatureFlagMarker.Length); diff --git a/src/Microsoft.Extensions.Configuration.AzureAppConfiguration/KeyValueChange.cs b/src/Microsoft.Extensions.Configuration.AzureAppConfiguration/KeyValueChange.cs index 7d41e107..2286016d 100644 --- a/src/Microsoft.Extensions.Configuration.AzureAppConfiguration/KeyValueChange.cs +++ b/src/Microsoft.Extensions.Configuration.AzureAppConfiguration/KeyValueChange.cs @@ -21,5 +21,7 @@ internal struct KeyValueChange public string Label { get; set; } public ConfigurationSetting Current { get; set; } + + public ConfigurationSetting Previous { get; set; } } } From f869ccd1f157a95ff2aad13886ec444f935b05e2 Mon Sep 17 00:00:00 2001 From: AMER JUSUPOVIC Date: Tue, 1 Oct 2024 09:38:43 -0700 Subject: [PATCH 07/10] move retries number to const int --- .../AzureAppConfigurationKeyVaultOptions.cs | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/Microsoft.Extensions.Configuration.AzureAppConfiguration/AzureAppConfigurationKeyVaultOptions.cs b/src/Microsoft.Extensions.Configuration.AzureAppConfiguration/AzureAppConfigurationKeyVaultOptions.cs index 520e3af0..24bc61a7 100644 --- a/src/Microsoft.Extensions.Configuration.AzureAppConfiguration/AzureAppConfigurationKeyVaultOptions.cs +++ b/src/Microsoft.Extensions.Configuration.AzureAppConfiguration/AzureAppConfigurationKeyVaultOptions.cs @@ -14,11 +14,13 @@ namespace Microsoft.Extensions.Configuration.AzureAppConfiguration /// public class AzureAppConfigurationKeyVaultOptions { + private const int KeyVaultMaxRetries = 6; + internal TokenCredential Credential; internal SecretClientOptions ClientOptions = new SecretClientOptions { Retry = { - MaxRetries = 6 + MaxRetries = KeyVaultMaxRetries } }; internal List SecretClients = new List(); From 4734767014207ee29d46015f0c4c8fcd7d84cecf Mon Sep 17 00:00:00 2001 From: AMER JUSUPOVIC Date: Tue, 1 Oct 2024 10:29:03 -0700 Subject: [PATCH 08/10] call invalidate cache with previous setting if current one is null --- .../AzureAppConfigurationProvider.cs | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/src/Microsoft.Extensions.Configuration.AzureAppConfiguration/AzureAppConfigurationProvider.cs b/src/Microsoft.Extensions.Configuration.AzureAppConfiguration/AzureAppConfigurationProvider.cs index 892a355a..621755f9 100644 --- a/src/Microsoft.Extensions.Configuration.AzureAppConfiguration/AzureAppConfigurationProvider.cs +++ b/src/Microsoft.Extensions.Configuration.AzureAppConfiguration/AzureAppConfigurationProvider.cs @@ -389,7 +389,15 @@ await CallWithRequestTracing( // Invalidate the cached Key Vault secret (if any) for this ConfigurationSetting foreach (IKeyValueAdapter adapter in _options.Adapters) { - adapter.InvalidateCache(change.Current); + // If the current setting is null, try to invalidate the previous setting instead + if (change.Current != null) + { + adapter.InvalidateCache(change.Current); + } + else if (change.Previous != null) + { + adapter.InvalidateCache(change.Previous); + } } } } From d68c3dadf82660bc01c294b3adf1825b95610645 Mon Sep 17 00:00:00 2001 From: AMER JUSUPOVIC Date: Wed, 2 Oct 2024 12:04:30 -0700 Subject: [PATCH 09/10] add comment explaining number of key vault retries --- .../AzureAppConfigurationKeyVaultOptions.cs | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/Microsoft.Extensions.Configuration.AzureAppConfiguration/AzureAppConfigurationKeyVaultOptions.cs b/src/Microsoft.Extensions.Configuration.AzureAppConfiguration/AzureAppConfigurationKeyVaultOptions.cs index 24bc61a7..cca16df8 100644 --- a/src/Microsoft.Extensions.Configuration.AzureAppConfiguration/AzureAppConfigurationKeyVaultOptions.cs +++ b/src/Microsoft.Extensions.Configuration.AzureAppConfiguration/AzureAppConfigurationKeyVaultOptions.cs @@ -14,6 +14,8 @@ namespace Microsoft.Extensions.Configuration.AzureAppConfiguration /// public class AzureAppConfigurationKeyVaultOptions { + // 6 retries is the highest number that will make the total retry time comfortably fall under the default startup timeout of 100 seconds. + // This allows the provider to throw a KeyVaultReferenceException with all relevant information and halt startup instead of timing out. private const int KeyVaultMaxRetries = 6; internal TokenCredential Credential; From 54a0fdb8088ee89c057bb98fa2d5d93f44897050 Mon Sep 17 00:00:00 2001 From: AMER JUSUPOVIC Date: Wed, 2 Oct 2024 12:07:18 -0700 Subject: [PATCH 10/10] pull out utcnow as a variable in for loop in clearcache --- .../AzureKeyVaultReference/AzureKeyVaultSecretProvider.cs | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/Microsoft.Extensions.Configuration.AzureAppConfiguration/AzureKeyVaultReference/AzureKeyVaultSecretProvider.cs b/src/Microsoft.Extensions.Configuration.AzureAppConfiguration/AzureKeyVaultReference/AzureKeyVaultSecretProvider.cs index ce091deb..57505ff9 100644 --- a/src/Microsoft.Extensions.Configuration.AzureAppConfiguration/AzureKeyVaultReference/AzureKeyVaultSecretProvider.cs +++ b/src/Microsoft.Extensions.Configuration.AzureAppConfiguration/AzureKeyVaultReference/AzureKeyVaultSecretProvider.cs @@ -88,9 +88,11 @@ public void ClearCache() { var sourceIdsToRemove = new List(); + var utcNow = DateTimeOffset.UtcNow; + foreach (KeyValuePair secret in _cachedKeyVaultSecrets) { - if (secret.Value.LastRefreshTime + RefreshConstants.MinimumSecretRefreshInterval < DateTimeOffset.UtcNow) + if (secret.Value.LastRefreshTime + RefreshConstants.MinimumSecretRefreshInterval < utcNow) { sourceIdsToRemove.Add(secret.Key); }