Merge pull request #604 from Azure/rossgrambo/merge-main-to-preview

Merge main to preview with resolved conflicts
Azure · Oct 23, 2024 · 9216437 · 9216437
2 parents 51d4ad7 + 0619626
commit 9216437
Show file tree

Hide file tree

Showing 15 changed files with 118 additions and 121 deletions.
diff --git a/...soft.Azure.AppConfiguration.AspNetCore/Microsoft.Azure.AppConfiguration.AspNetCore.csproj b/...soft.Azure.AppConfiguration.AspNetCore/Microsoft.Azure.AppConfiguration.AspNetCore.csproj
@@ -21,7 +21,7 @@
   <!-- Nuget Package Version Settings -->
 
   <PropertyGroup>
-    <OfficialVersion>8.0.0-preview.3</OfficialVersion>
+    <OfficialVersion>8.0.0</OfficialVersion>
   </PropertyGroup>
 
   <PropertyGroup Condition="'$(CDP_PATCH_NUMBER)'!='' AND '$(CDP_BUILD_TYPE)'=='Official'">

diff --git a/...ppConfiguration.Functions.Worker/Microsoft.Azure.AppConfiguration.Functions.Worker.csproj b/...ppConfiguration.Functions.Worker/Microsoft.Azure.AppConfiguration.Functions.Worker.csproj
@@ -24,7 +24,7 @@
   <!-- Nuget Package Version Settings -->
 
   <PropertyGroup>
-    <OfficialVersion>8.0.0-preview.3</OfficialVersion>
+    <OfficialVersion>8.0.0</OfficialVersion>
   </PropertyGroup>
 
   <PropertyGroup Condition="'$(CDP_PATCH_NUMBER)'!='' AND '$(CDP_BUILD_TYPE)'=='Official'">

diff --git a/...ft.Extensions.Configuration.AzureAppConfiguration/AzureAppConfigurationKeyVaultOptions.cs b/...ft.Extensions.Configuration.AzureAppConfiguration/AzureAppConfigurationKeyVaultOptions.cs
@@ -14,7 +14,17 @@ namespace Microsoft.Extensions.Configuration.AzureAppConfiguration
     /// </summary>
     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
+        {
+            Retry = {
+                MaxRetries = KeyVaultMaxRetries
+            }
+        };
         internal List<SecretClient> SecretClients = new List<SecretClient>();
         internal Func<Uri, ValueTask<string>> SecretResolver;
         internal Dictionary<string, TimeSpan> SecretRefreshIntervals = new Dictionary<string, TimeSpan>();
@@ -31,6 +41,17 @@ public AzureAppConfigurationKeyVaultOptions SetCredential(TokenCredential creden
             return this;
         }
 
+        /// <summary>
+        /// Configures the client options used when connecting to key vaults that have no registered <see cref="SecretClient"/>. 
+        /// The client options will not affect <see cref="SecretClient"/> instances registered via <see cref="Register(SecretClient)"/>.
+        /// </summary>
+        /// <param name="configure">A callback used to configure secret client options.</param>
+        public AzureAppConfigurationKeyVaultOptions ConfigureClientOptions(Action<SecretClientOptions> configure)
+        {
+            configure?.Invoke(ClientOptions);
+            return this;
+        }
+
         /// <summary>
         /// Registers the specified <see cref="SecretClient"/> instance to use to resolve key vault references for secrets from associated key vault.
         /// </summary>

diff --git a/...Microsoft.Extensions.Configuration.AzureAppConfiguration/AzureAppConfigurationProvider.cs b/...Microsoft.Extensions.Configuration.AzureAppConfiguration/AzureAppConfigurationProvider.cs
@@ -390,7 +390,15 @@ await CallWithRequestTracing(
                             // Invalidate the cached Key Vault secret (if any) for this ConfigurationSetting
                             foreach (IKeyValueAdapter adapter in _options.Adapters)
                             {
-                                adapter.OnChangeDetected(change.Current);
+                                // If the current setting is null, try to pass the previous setting instead
+                                if (change.Current != null)
+                                {
+                                    adapter.OnChangeDetected(change.Current);
+                                }
+                                else if (change.Previous != null)
+                                {
+                                    adapter.OnChangeDetected(change.Previous);
+                                }
                             }
                         }
                     }
@@ -671,17 +679,6 @@ private async Task<bool> TryInitializeAsync(IEnumerable<ConfigurationClient> cli
 
                 throw;
             }
-            catch (KeyVaultReferenceException exception)
-            {
-                if (IsFailOverable(exception))
-                {
-                    startupExceptions.Add(exception);
-
-                    return false;
-                }
-
-                throw;
-            }
             catch (AggregateException exception)
             {
                 if (exception.InnerExceptions?.Any(e => e is OperationCanceledException) ?? false)
@@ -1065,15 +1062,6 @@ private async Task<T> ExecuteWithFailOverPolicyAsync<T>(
                         throw;
                     }
                 }
-                catch (KeyVaultReferenceException kvre)
-                {
-                    if (!IsFailOverable(kvre) || !clientEnumerator.MoveNext())
-                    {
-                        backoffAllClients = true;
-
-                        throw;
-                    }
-                }
                 catch (AggregateException ae)
                 {
                     if (!IsFailOverable(ae) || !clientEnumerator.MoveNext())
@@ -1145,7 +1133,9 @@ private bool IsFailOverable(RequestFailedException rfe)
         {
             if (rfe.Status == HttpStatusCodes.TooManyRequests ||
                 rfe.Status == (int)HttpStatusCode.RequestTimeout ||
-                rfe.Status >= (int)HttpStatusCode.InternalServerError)
+                rfe.Status >= (int)HttpStatusCode.InternalServerError ||
+                rfe.Status == (int)HttpStatusCode.Forbidden ||
+                rfe.Status == (int)HttpStatusCode.Unauthorized)
             {
                 return true;
             }
@@ -1167,20 +1157,6 @@ innerException is SocketException ||
                    innerException is IOException;
         }
 
-        private bool IsFailOverable(KeyVaultReferenceException kvre)
-        {
-            if (kvre.InnerException is RequestFailedException rfe && IsFailOverable(rfe))
-            {
-                return true;
-            }
-            else if (kvre.InnerException is AggregateException ae && IsFailOverable(ae))
-            {
-                return true;
-            }
-
-            return false;
-        }
-
         private async Task<Dictionary<string, ConfigurationSetting>> MapConfigurationSettings(Dictionary<string, ConfigurationSetting> data)
         {
             Dictionary<string, ConfigurationSetting> mappedData = new Dictionary<string, ConfigurationSetting>(StringComparer.OrdinalIgnoreCase);

diff --git a/...onfiguration.AzureAppConfiguration/AzureKeyVaultReference/AzureKeyVaultKeyValueAdapter.cs b/...onfiguration.AzureAppConfiguration/AzureKeyVaultReference/AzureKeyVaultKeyValueAdapter.cs
@@ -84,7 +84,15 @@ public void OnChangeDetected(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/...Configuration.AzureAppConfiguration/AzureKeyVaultReference/AzureKeyVaultSecretProvider.cs b/...Configuration.AzureAppConfiguration/AzureKeyVaultReference/AzureKeyVaultSecretProvider.cs
@@ -15,14 +15,14 @@ internal class AzureKeyVaultSecretProvider
     {
         private readonly AzureAppConfigurationKeyVaultOptions _keyVaultOptions;
         private readonly IDictionary<string, SecretClient> _secretClients;
-        private readonly Dictionary<string, CachedKeyVaultSecret> _cachedKeyVaultSecrets;
-        private string _nextRefreshKey;
+        private readonly Dictionary<Uri, CachedKeyVaultSecret> _cachedKeyVaultSecrets;
+        private Uri _nextRefreshSourceId;
         private DateTimeOffset? _nextRefreshTime;
 
         public AzureKeyVaultSecretProvider(AzureAppConfigurationKeyVaultOptions keyVaultOptions = null)
         {
             _keyVaultOptions = keyVaultOptions ?? new AzureAppConfigurationKeyVaultOptions();
-            _cachedKeyVaultSecrets = new Dictionary<string, CachedKeyVaultSecret>(StringComparer.OrdinalIgnoreCase);
+            _cachedKeyVaultSecrets = new Dictionary<Uri, CachedKeyVaultSecret>();
             _secretClients = new Dictionary<string, SecretClient>(StringComparer.OrdinalIgnoreCase);
 
             if (_keyVaultOptions.SecretClients != null)
@@ -39,7 +39,7 @@ public async Task<string> 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<string> 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<Uri>();
+
+            var utcNow = DateTimeOffset.UtcNow;
+
+            foreach (KeyValuePair<Uri, CachedKeyVaultSecret> 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();
             }
@@ -115,44 +133,49 @@ private SecretClient GetSecretClient(Uri secretUri)
                 return null;
             }
 
-            client = new SecretClient(new Uri(secretUri.GetLeftPart(UriPartial.Authority)), _keyVaultOptions.Credential);
+            client = new SecretClient(
+                new Uri(secretUri.GetLeftPart(UriPartial.Authority)),
+                _keyVaultOptions.Credential,
+                _keyVaultOptions.ClientOptions);
+
             _secretClients.Add(keyVaultId, client);
+
             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)
             {
                 cachedSecret = new CachedKeyVaultSecret();
             }
 
             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<string, CachedKeyVaultSecret> secret in _cachedKeyVaultSecrets)
+            foreach (KeyValuePair<Uri, CachedKeyVaultSecret> 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/...nsions.Configuration.AzureAppConfiguration/AzureKeyVaultReference/CachedKeyVaultSecret.cs b/...nsions.Configuration.AzureAppConfiguration/AzureKeyVaultReference/CachedKeyVaultSecret.cs
@@ -22,11 +22,23 @@ internal class CachedKeyVaultSecret
         /// </summary>
         public int RefreshAttempts { get; set; }
 
-        public CachedKeyVaultSecret(string secretValue = null, DateTimeOffset? refreshAt = null, int refreshAttempts = 0)
+        /// <summary>
+        /// The last time this secret was reloaded from Key Vault.
+        /// </summary>
+        public DateTimeOffset LastRefreshTime { get; set; }
+
+        /// <summary>
+        /// The source <see cref="Uri"/> for this secret.
+        /// </summary>
+        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
@@ -16,7 +16,7 @@ internal class RefreshConstants
         public static readonly TimeSpan MinimumFeatureFlagRefreshInterval = 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/...xtensions.Configuration.AzureAppConfiguration/Extensions/ConfigurationClientExtensions.cs b/...xtensions.Configuration.AzureAppConfiguration/Extensions/ConfigurationClientExtensions.cs
@@ -36,6 +36,7 @@ public static async Task<KeyValueChange> 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<KeyValueChange> 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<KeyValueChange> 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
@@ -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/...ion.AzureAppConfiguration/Microsoft.Extensions.Configuration.AzureAppConfiguration.csproj b/...ion.AzureAppConfiguration/Microsoft.Extensions.Configuration.AzureAppConfiguration.csproj
@@ -35,7 +35,7 @@
   <!-- Nuget Package Version Settings -->
 
   <PropertyGroup>
-    <OfficialVersion>8.0.0-preview.3</OfficialVersion>
+    <OfficialVersion>8.0.0</OfficialVersion>
   </PropertyGroup>
 
   <PropertyGroup Condition="'$(CDP_PATCH_NUMBER)'!='' AND '$(CDP_BUILD_TYPE)'=='Official'">