Move SecretDictionary away from sync-over-async

src/NuGet.Services.Configuration/SecretDictionary.cs

@@ -1,11 +1,10 @@
-// Copyright (c) .NET Foundation. All rights reserved. 
+// Copyright (c) .NET Foundation. All rights reserved. 
 // Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. 
 
 using System;
 using System.Collections;
 using System.Collections.Generic;
 using System.Linq;
-using System.Threading.Tasks;
 using NuGet.Services.KeyVault;
 
 namespace NuGet.Services.Configuration
@@ -122,14 +121,9 @@ private string InjectOrSkip(string key, string value)
         {
             if (!_notInjectedKeys.Contains(key))
             {
-                return Inject(value).Result;
+                return _secretInjector.Inject(value);
             }
             return value;
         }
-
-        private Task<string> Inject(string value)
-        {
-            return _secretInjector.InjectAsync(value);
-        }
     }
 }

src/NuGet.Services.KeyVault/CachingSecretReader.cs

@@ -1,4 +1,4 @@
-// Copyright (c) .NET Foundation. All rights reserved. 
+// Copyright (c) .NET Foundation. All rights reserved. 
 // Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information.
 
 using System;
@@ -29,6 +29,16 @@ public CachingSecretReader(ISecretReader secretReader,
             _refreshIntervalBeforeExpiry = TimeSpan.FromSeconds(refreshIntervalBeforeExpirySec);
         }
 
+        public string GetSecret(string secretName)
+        {
+            return GetSecret(secretName, logger: null);
+        }
+
+        public string GetSecret(string secretName, ILogger logger)
+        {
+            return GetSecretObject(secretName, logger).Value;
+        }
+
         public async Task<string> GetSecretAsync(string secretName)
         {
             return await GetSecretAsync(secretName, logger: null);
@@ -39,32 +49,51 @@ public async Task<string> GetSecretAsync(string secretName, ILogger logger)
             return (await GetSecretObjectAsync(secretName, logger)).Value;
         }
 
-        public async Task<ISecret> GetSecretObjectAsync(string secretName)
+        public ISecret GetSecretObject(string secretName)
         {
-            return await GetSecretObjectAsync(secretName, logger: null);
+            return GetSecretObject(secretName, logger: null);
         }
 
-        public async Task<ISecret> GetSecretObjectAsync(string secretName, ILogger logger)
+        public ISecret GetSecretObject(string secretName, ILogger logger)
         {
-            if (string.IsNullOrEmpty(secretName))
+            if (TryGetCachedSecretObject(secretName, logger, out var cachedSecret))
             {
-                throw new ArgumentException("Null or empty secret name", nameof(secretName));
+                return cachedSecret;
             }
 
-            // If the cache contains the secret and it is not expired, return the cached value.
+            var start = DateTimeOffset.UtcNow;
+
+            var updatedValue = new CachedSecret(_internalReader.GetSecretObject(secretName));
+
+            return UpdateCachedSecret(secretName, logger, start, updatedValue);
+        }
+
+        public async Task<ISecret> GetSecretObjectAsync(string secretName)
+        {
+            return await GetSecretObjectAsync(secretName, logger: null);
+        }
+
+        public async Task<ISecret> GetSecretObjectAsync(string secretName, ILogger logger)
+        {
             if (TryGetCachedSecretObject(secretName, logger, out var cachedSecret))
             {
                 return cachedSecret;
             }
 
             var start = DateTimeOffset.UtcNow;
-            // The cache does not contain a fresh copy of the secret. Fetch and cache the secret.
+
             var updatedValue = new CachedSecret(await _internalReader.GetSecretObjectAsync(secretName));
+
+            return UpdateCachedSecret(secretName, logger, start, updatedValue);
+        }
+
+        private ISecret UpdateCachedSecret(string secretName, ILogger logger, DateTimeOffset start, CachedSecret updatedValue)
+        {
             var updatedSecret = _cache.AddOrUpdate(secretName, updatedValue, (key, old) => updatedValue).Secret;
 
             logger?.LogInformation("Refreshed secret {SecretName}, Expiring at: {ExpirationTime}. Took {ElapsedMilliseconds}ms.",
                 updatedSecret.Name,
-                updatedSecret.Expiration == null ? "null" : ((DateTimeOffset) updatedSecret.Expiration).UtcDateTime.ToString(),
+                updatedSecret.Expiration == null ? "null" : ((DateTimeOffset)updatedSecret.Expiration).UtcDateTime.ToString(),
                 (DateTimeOffset.UtcNow - start).TotalMilliseconds.ToString("F2"));
 
             return updatedSecret;
@@ -87,6 +116,11 @@ public bool TryGetCachedSecret(string secretName, ILogger logger, out string sec
 
         public bool TryGetCachedSecretObject(string secretName, ILogger logger, out ISecret secretObject)
         {
+            if (string.IsNullOrEmpty(secretName))
+            {
+                throw new ArgumentException("Null or empty secret name", nameof(secretName));
+            }
+
             secretObject = null;
             if (_cache.TryGetValue(secretName, out CachedSecret result)
                 && !IsSecretOutdated(result))
@@ -122,4 +156,4 @@ public CachedSecret(ISecret secret)
 
         }
     }
-}
+}

src/NuGet.Services.KeyVault/EmptySecretReader.cs

@@ -1,4 +1,4 @@
-// Copyright (c) .NET Foundation. All rights reserved.
+// Copyright (c) .NET Foundation. All rights reserved.
 // Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information.
 
 using System.Threading.Tasks;
@@ -8,18 +8,38 @@ namespace NuGet.Services.KeyVault
 {
     public class EmptySecretReader : ICachingSecretReader
     {
+        public string GetSecret(string secretName)
+        {
+            return GetSecret(secretName, logger: null);
+        }
+
+        public string GetSecret(string secretName, ILogger logger)
+        {
+            return secretName;
+        }
+
         public Task<string> GetSecretAsync(string secretName) => GetSecretAsync(secretName, logger: null);
 
         public Task<string> GetSecretAsync(string secretName, ILogger logger)
         {
             return Task.FromResult(secretName);
         }
 
+        public ISecret GetSecretObject(string secretName)
+        {
+            return GetSecretObject(secretName, logger: null);
+        }
+
+        public ISecret GetSecretObject(string secretName, ILogger logger)
+        {
+            return new KeyVaultSecret(secretName, secretName, null);
+        }
+
         public Task<ISecret> GetSecretObjectAsync(string secretName) => GetSecretObjectAsync(secretName, logger: null);
 
         public Task<ISecret> GetSecretObjectAsync(string secretName, ILogger logger)
         {
-            return Task.FromResult((ISecret)new KeyVaultSecret(secretName, secretName, null));
+            return Task.FromResult(GetSecretObject(secretName, logger));
         }
 
         public bool TryGetCachedSecret(string secretName, out string secretValue) => TryGetCachedSecret(secretName, logger: null, out secretValue);
@@ -38,4 +58,4 @@ public bool TryGetCachedSecretObject(string secretName, ILogger logger, out ISec
             return true;
         }
     }
-}
+}

src/NuGet.Services.KeyVault/IRefreshableSecretReaderFactory.cs

@@ -1,4 +1,4 @@
-// Copyright (c) .NET Foundation. All rights reserved.
+// Copyright (c) .NET Foundation. All rights reserved.
 // Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information.
 
 using System.Threading;
@@ -11,6 +11,13 @@ namespace NuGet.Services.KeyVault
     /// </summary>
     public interface IRefreshableSecretReaderFactory : ISecretReaderFactory
     {
+        /// <summary>
+        /// Refresh the values of the secrets that have already been read and cached. Since the cache is shared between
+        /// all <see cref="ISecretReader"/> instances creates, this refresh applies to all secret readers created by
+        /// this factory.
+        /// </summary>
+        void Refresh();
+
         /// <summary>
         /// Refresh the values of the secrets that have already been read and cached. Since the cache is shared between
         /// all <see cref="ISecretReader"/> instances creates, this refresh applies to all secret readers created by

src/NuGet.Services.KeyVault/ISecretInjector.cs

@@ -1,4 +1,4 @@
-// Copyright (c) .NET Foundation. All rights reserved.
+// Copyright (c) .NET Foundation. All rights reserved.
 // Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information.
 
 using System.Threading.Tasks;
@@ -8,7 +8,9 @@ namespace NuGet.Services.KeyVault
 {
     public interface ISecretInjector
     {
+        string Inject(string input);
+        string Inject(string input, ILogger logger);
         Task<string> InjectAsync(string input);
         Task<string> InjectAsync(string input, ILogger logger);
     }
-}
+}

src/NuGet.Services.KeyVault/ISecretReader.cs

@@ -1,4 +1,4 @@
-// Copyright (c) .NET Foundation. All rights reserved.
+// Copyright (c) .NET Foundation. All rights reserved.
 // Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information.
 
 using System.Threading.Tasks;
@@ -8,9 +8,13 @@ namespace NuGet.Services.KeyVault
 {
     public interface ISecretReader
     {
+        string GetSecret(string secretName);
+        string GetSecret(string secretName, ILogger logger);
         Task<string> GetSecretAsync(string secretName);
         Task<string> GetSecretAsync(string secretName, ILogger logger);
+        ISecret GetSecretObject(string secretName);
+        ISecret GetSecretObject(string secretName, ILogger logger);
         Task<ISecret> GetSecretObjectAsync(string secretName);
         Task<ISecret> GetSecretObjectAsync(string secretName, ILogger logger);
     }
-}
+}

src/NuGet.Services.KeyVault/KeyVaultReader.cs

@@ -1,4 +1,4 @@
-// Copyright (c) .NET Foundation. All rights reserved.
+// Copyright (c) .NET Foundation. All rights reserved.
 // Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information.
 
 using System;
@@ -33,6 +33,17 @@ public KeyVaultReader(KeyVaultConfiguration configuration)
             _keyVaultClient = new Lazy<SecretClient>(InitializeClient);
         }
 
+        public string GetSecret(string secretName)
+        {
+            return GetSecret(secretName, logger: null);
+        }
+
+        public string GetSecret(string secretName, ILogger logger)
+        {
+            AzureSecurityKeyVaultSecret secret = _keyVaultClient.Value.GetSecret(secretName);
+            return secret.Value;
+        }
+
         public async Task<string> GetSecretAsync(string secretName)
         {
             return await GetSecretAsync(secretName, logger: null);
@@ -44,6 +55,17 @@ public async Task<string> GetSecretAsync(string secretName, ILogger logger)
             return secret.Value;
         }
 
+        public ISecret GetSecretObject(string secretName)
+        {
+            return GetSecretObject(secretName, logger: null);
+        }
+
+        public ISecret GetSecretObject(string secretName, ILogger logger)
+        {
+            AzureSecurityKeyVaultSecret secret = _keyVaultClient.Value.GetSecret(secretName);
+            return MapSecret(secretName, secret);
+        }
+
         public async Task<ISecret> GetSecretObjectAsync(string secretName)
         {
             return await GetSecretObjectAsync(secretName, logger: null);
@@ -52,6 +74,11 @@ public async Task<ISecret> GetSecretObjectAsync(string secretName)
         public async Task<ISecret> GetSecretObjectAsync(string secretName, ILogger logger)
         {
             AzureSecurityKeyVaultSecret secret = await _keyVaultClient.Value.GetSecretAsync(secretName);
+            return MapSecret(secretName, secret);
+        }
+
+        private static ISecret MapSecret(string secretName, AzureSecurityKeyVaultSecret secret)
+        {
             return new KeyVaultSecret(secretName, secret.Value, secret.Properties.ExpiresOn);
         }
 

src/NuGet.Services.KeyVault/RefreshableSecretReader.cs

@@ -1,4 +1,4 @@
-// Copyright (c) .NET Foundation. All rights reserved. 
+// Copyright (c) .NET Foundation. All rights reserved. 
 // Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. 
 
 using System;
@@ -32,6 +32,14 @@ public RefreshableSecretReader(
             _settings = settings ?? throw new ArgumentNullException(nameof(settings));
         }
 
+        public void Refresh()
+        {
+            foreach (var secretName in _cache.Keys)
+            {
+                UncachedGetSecretObject(secretName);
+            }
+        }
+
         public async Task RefreshAsync(CancellationToken token)
         {
             foreach (var secretName in _cache.Keys)
@@ -45,19 +53,39 @@ public async Task RefreshAsync(CancellationToken token)
             }
         }
 
+        public string GetSecret(string secretName)
+        {
+            return GetSecret(secretName, logger: null);
+        }
+
+        public string GetSecret(string secretName, ILogger logger)
+        {
+            return GetSecretObject(secretName, logger).Value;
+        }
+
         public Task<string> GetSecretAsync(string secretName)
         {
             return GetSecretAsync(secretName, logger: null);
         }
 
-        public Task<string> GetSecretAsync(string secretName, ILogger logger)
+        public async Task<string> GetSecretAsync(string secretName, ILogger logger)
+        {
+            return (await GetSecretObjectAsync(secretName, logger)).Value;
+        }
+
+        public ISecret GetSecretObject(string secretName)
+        {
+            return GetSecretObject(secretName, logger: null);
+        }
+
+        public ISecret GetSecretObject(string secretName, ILogger logger)
         {
             if (TryGetCachedSecretObject(secretName, out var secret))
             {
-                return Task.FromResult(secret.Value);
+                return secret;
             }
 
-            return UncachedGetSecretAsync(secretName);
+            return UncachedGetSecretObject(secretName);
         }
 
         public Task<ISecret> GetSecretObjectAsync(string secretName)
@@ -108,10 +136,11 @@ public bool TryGetCachedSecretObject(string secretName, ILogger logger, out ISec
         public bool TryGetCachedSecretObject(string secretName, out ISecret secretObject)
             => TryGetCachedSecretObject(secretName, logger: null, secretObject: out secretObject);
 
-        private async Task<string> UncachedGetSecretAsync(string secretName)
+        private ISecret UncachedGetSecretObject(string secretName)
         {
-            var secretObject = await UncachedGetSecretObjectAsync(secretName);
-            return secretObject.Value;
+            var secretObject = _secretReader.GetSecretObject(secretName);
+            _cache.AddOrUpdate(secretName, secretObject, (_, __) => secretObject);
+            return secretObject;
         }
 
         private async Task<ISecret> UncachedGetSecretObjectAsync(string secretName)
@@ -121,4 +150,4 @@ private async Task<ISecret> UncachedGetSecretObjectAsync(string secretName)
             return secretObject;
         }
     }
-}
+}