diff --git a/src/Microsoft.Extensions.Configuration.AzureAppConfiguration/AzureAppConfigurationKeyVaultOptions.cs b/src/Microsoft.Extensions.Configuration.AzureAppConfiguration/AzureAppConfigurationKeyVaultOptions.cs index 7d32f3a7..cca16df8 100644 --- a/src/Microsoft.Extensions.Configuration.AzureAppConfiguration/AzureAppConfigurationKeyVaultOptions.cs +++ b/src/Microsoft.Extensions.Configuration.AzureAppConfiguration/AzureAppConfigurationKeyVaultOptions.cs @@ -14,8 +14,17 @@ 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; - internal SecretClientOptions ClientOptions = new SecretClientOptions(); + internal SecretClientOptions ClientOptions = new SecretClientOptions + { + Retry = { + MaxRetries = KeyVaultMaxRetries + } + }; internal List SecretClients = new List(); internal Func> SecretResolver; internal Dictionary SecretRefreshIntervals = new Dictionary(); 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); + } } } } 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 86e61429..57505ff9 100644 --- a/src/Microsoft.Extensions.Configuration.AzureAppConfiguration/AzureKeyVaultReference/AzureKeyVaultSecretProvider.cs +++ b/src/Microsoft.Extensions.Configuration.AzureAppConfiguration/AzureKeyVaultReference/AzureKeyVaultSecretProvider.cs @@ -15,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) @@ -39,7 +39,7 @@ public async Task GetSecretValue(KeyVaultSecretIdentifier secretIdentifi { string secretValue = null; - if (_cachedKeyVaultSecrets.TryGetValue(key, out CachedKeyVaultSecret cachedSecret) && + if (_cachedKeyVaultSecrets.TryGetValue(secretIdentifier.SourceId, out CachedKeyVaultSecret cachedSecret) && (!cachedSecret.RefreshAt.HasValue || DateTimeOffset.UtcNow < cachedSecret.RefreshAt.Value)) { return cachedSecret.SecretValue; @@ -68,12 +68,12 @@ 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 { - SetSecretInCache(key, cachedSecret, success); + SetSecretInCache(secretIdentifier.SourceId, key, cachedSecret, success); } return secretValue; @@ -86,16 +86,34 @@ public bool ShouldRefreshKeyVaultSecrets() public void ClearCache() { - _cachedKeyVaultSecrets.Clear(); - _nextRefreshKey = null; - _nextRefreshTime = null; + var sourceIdsToRemove = new List(); + + var utcNow = DateTimeOffset.UtcNow; + + foreach (KeyValuePair secret in _cachedKeyVaultSecrets) + { + if (secret.Value.LastRefreshTime + RefreshConstants.MinimumSecretRefreshInterval < utcNow) + { + sourceIdsToRemove.Add(secret.Key); + } + } + + foreach (Uri sourceId in sourceIdsToRemove) + { + _cachedKeyVaultSecrets.Remove(sourceId); + } + + if (_cachedKeyVaultSecrets.Any()) + { + UpdateNextRefreshableSecretFromCache(); + } } - public void RemoveSecretFromCache(string key) + public void RemoveSecretFromCache(Uri sourceId) { - _cachedKeyVaultSecrets.Remove(key); + _cachedKeyVaultSecrets.Remove(sourceId); - if (key == _nextRefreshKey) + if (sourceId == _nextRefreshSourceId) { UpdateNextRefreshableSecretFromCache(); } @@ -125,7 +143,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) { @@ -133,31 +151,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/AzureKeyVaultReference/CachedKeyVaultSecret.cs b/src/Microsoft.Extensions.Configuration.AzureAppConfiguration/AzureKeyVaultReference/CachedKeyVaultSecret.cs index 1b813d36..ab09311e 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/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; } } } diff --git a/tests/Tests.AzureAppConfiguration/KeyVaultReferenceTests.cs b/tests/Tests.AzureAppConfiguration/KeyVaultReferenceTests.cs index d8c832bf..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); @@ -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")