Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,13 @@ public class AzureAppConfigurationKeyVaultOptions
internal TimeSpan? DefaultSecretRefreshInterval = null;
internal bool IsKeyVaultRefreshConfigured = false;

/// <summary>
/// Specifies whether Key Vault references should be resolved in parallel.
/// Default value is false. Enabling this can reduce the time required to resolve Key Vault references
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
/// Default value is false. Enabling this can reduce the time required to resolve Key Vault references
/// Enabling this can reduce the time required to resolve Key Vault references

/// when many references are loaded from Azure App Configuration.
/// </summary>
public bool ParallelSecretResolutionEnabled { get; set; }

/// <summary>
/// Sets the credentials used to authenticate to key vaults that have no registered <see cref="SecretClient"/>.
/// </summary>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -137,12 +137,17 @@ internal IEnumerable<IKeyValueAdapter> Adapters
/// <summary>
/// Flag to indicate whether Key Vault options have been configured.
/// </summary>
internal bool IsKeyVaultConfigured { get; private set; } = false;
internal bool IsKeyVaultConfigured { get; private set; }

/// <summary>
/// Flag to indicate whether Key Vault secret values will be refreshed automatically.
/// </summary>
internal bool IsKeyVaultRefreshConfigured { get; private set; } = false;
internal bool IsKeyVaultRefreshConfigured { get; private set; }

/// <summary>
/// Flag to indicate whether Key Vault references should be resolved in parallel.
/// </summary>
internal bool IsParallelSecretResolutionEnabled { get; private set; }

/// <summary>
/// Indicates all feature flag features used by the application.
Expand Down Expand Up @@ -520,6 +525,7 @@ public AzureAppConfigurationOptions ConfigureKeyVault(Action<AzureAppConfigurati
_adapters.Add(new AzureKeyVaultKeyValueAdapter(new AzureKeyVaultSecretProvider(keyVaultOptions)));

IsKeyVaultRefreshConfigured = keyVaultOptions.IsKeyVaultRefreshConfigured;
IsParallelSecretResolutionEnabled = keyVaultOptions.ParallelSecretResolutionEnabled;
IsKeyVaultConfigured = true;
return this;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -625,16 +625,25 @@ private async Task<Dictionary<string, string>> PrepareData(Dictionary<string, Co
_requestTracingOptions.ResetAiConfigurationTracing();
}

// When parallel secret resolution is enabled, let each adapter pre-warm its caches
// (Key Vault references are dispatched concurrently here) so the sequential loop below
// can process settings in the original order without losing precedence on key collisions.
if (_options.IsParallelSecretResolutionEnabled)
{
await Task.WhenAll(
_options.Adapters.Select(adapter =>
adapter.PreloadAsync(data.Values, _logger, cancellationToken)))
.ConfigureAwait(false);
}

foreach (KeyValuePair<string, ConfigurationSetting> kvp in data)
{
IEnumerable<KeyValuePair<string, string>> keyValuePairs = null;

if (_requestTracingEnabled && _requestTracingOptions != null)
{
_requestTracingOptions.UpdateAiConfigurationTracing(kvp.Value.ContentType);
}

keyValuePairs = await ProcessAdapters(kvp.Value, cancellationToken).ConfigureAwait(false);
IEnumerable<KeyValuePair<string, string>> keyValuePairs = await ProcessAdapters(kvp.Value, cancellationToken).ConfigureAwait(false);

foreach (KeyValuePair<string, string> kv in keyValuePairs)
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,6 @@
using System;
using System.Collections.Generic;
using System.Linq;
using System.Net.Mime;
using System.Text.Json;
using System.Threading;
using System.Threading.Tasks;
Expand Down Expand Up @@ -75,15 +74,12 @@ KeyVaultReferenceException CreateKeyVaultReferenceException(string message, Conf

public bool CanProcess(ConfigurationSetting setting)
{
if (setting == null ||
string.IsNullOrWhiteSpace(setting.Value) ||
string.IsNullOrWhiteSpace(setting.ContentType))
if (setting == null || string.IsNullOrWhiteSpace(setting.Value))
{
return false;
}

return setting.ContentType.TryParseContentType(out ContentType contentType)
&& contentType.IsKeyVaultReference();
return setting.IsKeyVaultReference();
}

public void OnChangeDetected(ConfigurationSetting setting = null)
Expand Down Expand Up @@ -116,6 +112,79 @@ public bool NeedsRefresh()
return _secretProvider.ShouldRefreshKeyVaultSecrets();
}

public async Task PreloadAsync(IEnumerable<ConfigurationSetting> settings, Logger logger, CancellationToken cancellationToken)
{
if (settings == null)
{
return;
}

HashSet<Uri> seen = null;
List<(KeyVaultSecretIdentifier Identifier, string Key, string Label)> toFetch = null;

foreach (ConfigurationSetting setting in settings)
{
if (!CanProcess(setting))
{
continue;
}

string secretRefUri = ParseSecretReferenceUri(setting);

if (string.IsNullOrEmpty(secretRefUri) ||
!Uri.TryCreate(secretRefUri, UriKind.Absolute, out Uri secretUri) ||
!KeyVaultSecretIdentifier.TryCreate(secretUri, out KeyVaultSecretIdentifier secretIdentifier))
{
// Invalid references are surfaced from ProcessKeyValue with full exception context.
continue;
}

seen = seen ?? new HashSet<Uri>();

if (!seen.Add(secretIdentifier.SourceId))
{
continue;
}

toFetch = toFetch ?? new List<(KeyVaultSecretIdentifier, string, string)>();
toFetch.Add((secretIdentifier, setting.Key, setting.Label));
}

if (toFetch == null)
{
return;
}

var tasks = new Task[toFetch.Count];

for (int i = 0; i < toFetch.Count; i++)
{
(KeyVaultSecretIdentifier identifier, string key, string label) = toFetch[i];
tasks[i] = PreloadSecretAsync(identifier, key, label, logger, cancellationToken);
}

await Task.WhenAll(tasks).ConfigureAwait(false);
}

private async Task PreloadSecretAsync(KeyVaultSecretIdentifier identifier, string key, string label, Logger logger, CancellationToken cancellationToken)
{
try
{
await _secretProvider.GetSecretValue(identifier, key, label, logger, cancellationToken).ConfigureAwait(false);
}
catch (OperationCanceledException)
{
throw;
}
catch
{
// Per-secret failures are deferred so ProcessKeyValue can throw a properly populated
// KeyVaultReferenceException. Evict the negative cache entry written by GetSecretValue
// so the retry actually re-fetches instead of returning a cached null within the backoff.
_secretProvider.RemoveSecretFromCache(identifier.SourceId);
}
}

private string ParseSecretReferenceUri(ConfigurationSetting setting)
{
string secretRefUri = null;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,8 +4,8 @@
using Azure.Security.KeyVault.Secrets;
using Microsoft.Extensions.Configuration.AzureAppConfiguration.Extensions;
using System;
using System.Collections.Concurrent;
using System.Collections.Generic;
using System.Linq;
using System.Threading;
using System.Threading.Tasks;

Expand All @@ -14,16 +14,14 @@ namespace Microsoft.Extensions.Configuration.AzureAppConfiguration.AzureKeyVault
internal class AzureKeyVaultSecretProvider
{
private readonly AzureAppConfigurationKeyVaultOptions _keyVaultOptions;
private readonly IDictionary<string, SecretClient> _secretClients;
private readonly Dictionary<Uri, CachedKeyVaultSecret> _cachedKeyVaultSecrets;
private Uri _nextRefreshSourceId;
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The reason we remove the _nextRefreshSourceId and _nextRefreshTime is to simplify the implementation. Otherwise, we have to add lock here, because they will be updated in the GetSecretValue code path (the SetSecretInCache call)

The benefit of maintaining the _netRefresh pair is that the ShouldRefreshKeyVaultSecrets call will be an O(1) operation. The implementation in this PR makes it an O(n) operation. But I think there won't be too many cached secrets and iterate the whole concurrent dictionary is not that expensive. So I think it is fine.

cc @avanigupta @jimmyca15

private DateTimeOffset? _nextRefreshTime;
private readonly ConcurrentDictionary<string, SecretClient> _secretClients;
private readonly ConcurrentDictionary<Uri, CachedKeyVaultSecret> _cachedKeyVaultSecrets;

public AzureKeyVaultSecretProvider(AzureAppConfigurationKeyVaultOptions keyVaultOptions = null)
{
_keyVaultOptions = keyVaultOptions ?? new AzureAppConfigurationKeyVaultOptions();
_cachedKeyVaultSecrets = new Dictionary<Uri, CachedKeyVaultSecret>();
_secretClients = new Dictionary<string, SecretClient>(StringComparer.OrdinalIgnoreCase);
_cachedKeyVaultSecrets = new ConcurrentDictionary<Uri, CachedKeyVaultSecret>();
_secretClients = new ConcurrentDictionary<string, SecretClient>(StringComparer.OrdinalIgnoreCase);

if (_keyVaultOptions.SecretClients != null)
{
Expand Down Expand Up @@ -52,6 +50,7 @@ public async Task<string> GetSecretValue(KeyVaultSecretIdentifier secretIdentifi
throw new UnauthorizedAccessException("No key vault credential or secret resolver callback configured, and no matching secret client could be found.");
}

CachedKeyVaultSecret updatedCachedSecret = null;
bool success = false;

try
Expand All @@ -68,55 +67,44 @@ public async Task<string> GetSecretValue(KeyVaultSecretIdentifier secretIdentifi
secretValue = await _keyVaultOptions.SecretResolver(secretIdentifier.SourceId).ConfigureAwait(false);
}

cachedSecret = new CachedKeyVaultSecret(secretValue, secretIdentifier.SourceId);
updatedCachedSecret = new CachedKeyVaultSecret(secretValue, secretIdentifier.SourceId);
success = true;
}
finally
{
SetSecretInCache(secretIdentifier.SourceId, key, cachedSecret, success);
SetSecretInCache(secretIdentifier.SourceId, key, updatedCachedSecret, success);
}

return secretValue;
}

public bool ShouldRefreshKeyVaultSecrets()
{
return _nextRefreshTime.HasValue && _nextRefreshTime.Value < DateTimeOffset.UtcNow;
}

public void ClearCache()
{
var sourceIdsToRemove = new List<Uri>();

var utcNow = DateTimeOffset.UtcNow;

foreach (KeyValuePair<Uri, CachedKeyVaultSecret> secret in _cachedKeyVaultSecrets)
{
if (secret.Value.LastRefreshTime + RefreshConstants.MinimumSecretRefreshInterval < utcNow)
if (secret.Value.RefreshAt.HasValue && secret.Value.RefreshAt.Value < DateTimeOffset.UtcNow)
{
sourceIdsToRemove.Add(secret.Key);
return true;
}
}

foreach (Uri sourceId in sourceIdsToRemove)
{
_cachedKeyVaultSecrets.Remove(sourceId);
}
return false;
}
Comment thread
linglingye001 marked this conversation as resolved.

if (_cachedKeyVaultSecrets.Any())
public void ClearCache()
{
foreach (KeyValuePair<Uri, CachedKeyVaultSecret> secret in _cachedKeyVaultSecrets)
{
UpdateNextRefreshableSecretFromCache();
if (secret.Value.LastRefreshTime + RefreshConstants.MinimumSecretRefreshInterval < DateTimeOffset.UtcNow)
{
_cachedKeyVaultSecrets.TryRemove(secret.Key, out _);
}
}
}

public void RemoveSecretFromCache(Uri sourceId)
{
_cachedKeyVaultSecrets.Remove(sourceId);

if (sourceId == _nextRefreshSourceId)
{
UpdateNextRefreshableSecretFromCache();
}
_cachedKeyVaultSecrets.TryRemove(sourceId, out _);
}

private SecretClient GetSecretClient(Uri secretUri)
Expand All @@ -133,14 +121,12 @@ private SecretClient GetSecretClient(Uri secretUri)
return null;
}

client = new SecretClient(
new Uri(secretUri.GetLeftPart(UriPartial.Authority)),
_keyVaultOptions.Credential,
_keyVaultOptions.ClientOptions);

_secretClients.Add(keyVaultId, client);

return client;
return _secretClients.GetOrAdd(
keyVaultId,
_ => new SecretClient(
new Uri(secretUri.GetLeftPart(UriPartial.Authority)),
_keyVaultOptions.Credential,
_keyVaultOptions.ClientOptions));
}

private void SetSecretInCache(Uri sourceId, string key, CachedKeyVaultSecret cachedSecret, bool success = true)
Expand All @@ -152,37 +138,6 @@ private void SetSecretInCache(Uri sourceId, string key, CachedKeyVaultSecret cac

UpdateCacheExpirationTimeForSecret(key, cachedSecret, success);
_cachedKeyVaultSecrets[sourceId] = cachedSecret;

if (sourceId == _nextRefreshSourceId)
{
UpdateNextRefreshableSecretFromCache();
}
else if ((cachedSecret.RefreshAt.HasValue && _nextRefreshTime.HasValue && cachedSecret.RefreshAt.Value < _nextRefreshTime.Value)
|| (cachedSecret.RefreshAt.HasValue && !_nextRefreshTime.HasValue))
{
_nextRefreshSourceId = sourceId;
_nextRefreshTime = cachedSecret.RefreshAt.Value;
}
}

private void UpdateNextRefreshableSecretFromCache()
{
_nextRefreshSourceId = null;
_nextRefreshTime = DateTimeOffset.MaxValue;

foreach (KeyValuePair<Uri, CachedKeyVaultSecret> secret in _cachedKeyVaultSecrets)
{
if (secret.Value.RefreshAt.HasValue && secret.Value.RefreshAt.Value < _nextRefreshTime)
{
_nextRefreshTime = secret.Value.RefreshAt;
_nextRefreshSourceId = secret.Key;
}
}

if (_nextRefreshTime == DateTimeOffset.MaxValue)
{
_nextRefreshTime = null;
}
}

private void UpdateCacheExpirationTimeForSecret(string key, CachedKeyVaultSecret cachedSecret, bool success)
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
// Copyright (c) Microsoft Corporation.
// Licensed under the MIT license.
//
using Azure.Data.AppConfiguration;
using Microsoft.Extensions.Configuration.AzureAppConfiguration.AzureKeyVault;
using System.Net.Mime;

namespace Microsoft.Extensions.Configuration.AzureAppConfiguration.Extensions
{
internal static class ConfigurationSettingExtensions
{
public static bool IsKeyVaultReference(this ConfigurationSetting setting)
{
return setting != null
&& setting.ContentType.TryParseContentType(out ContentType contentType)
&& contentType.IsKeyVaultReference();
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -87,6 +87,11 @@ public void OnConfigUpdated()
return;
}

public Task PreloadAsync(IEnumerable<ConfigurationSetting> settings, Logger logger, CancellationToken cancellationToken)
{
return Task.CompletedTask;
}

private List<KeyValuePair<string, string>> ProcessDotnetSchemaFeatureFlag(FeatureFlag featureFlag, ConfigurationSetting setting, Uri endpoint)
{
var keyValues = new List<KeyValuePair<string, string>>();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,10 @@ internal interface IKeyValueAdapter
{
Task<IEnumerable<KeyValuePair<string, string>>> ProcessKeyValue(ConfigurationSetting setting, Uri endpoint, Logger logger, CancellationToken cancellationToken);

// Pre-warm any per-setting state (e.g. Key Vault secret cache) before ProcessKeyValue is invoked
// on each setting. Adapters with no pre-fetchable state can return a completed task.
Task PreloadAsync(IEnumerable<ConfigurationSetting> settings, Logger logger, CancellationToken cancellationToken);

bool CanProcess(ConfigurationSetting setting);

void OnChangeDetected(ConfigurationSetting setting = null);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -80,5 +80,10 @@ public bool NeedsRefresh()
{
return false;
}

public Task PreloadAsync(IEnumerable<ConfigurationSetting> settings, Logger logger, CancellationToken cancellationToken)
{
return Task.CompletedTask;
}
}
}
Loading
Loading