Skip to content

Commit c325191

Browse files
joelverhagenjoelverhagen
authored
Move SecretDictionary away from sync-over-async (#10147)
1 parent 146473e commit c325191

17 files changed

Lines changed: 391 additions & 58 deletions

‎src/NuGet.Services.Configuration/SecretDictionary.cs‎

Lines changed: 2 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,10 @@
1-
// Copyright (c) .NET Foundation. All rights reserved.
1+
// Copyright (c) .NET Foundation. All rights reserved.
22
// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information.
33

44
using System;
55
using System.Collections;
66
using System.Collections.Generic;
77
using System.Linq;
8-
using System.Threading.Tasks;
98
using NuGet.Services.KeyVault;
109

1110
namespace NuGet.Services.Configuration
@@ -122,14 +121,9 @@ private string InjectOrSkip(string key, string value)
122121
{
123122
if (!_notInjectedKeys.Contains(key))
124123
{
125-
return Inject(value).Result;
124+
return _secretInjector.Inject(value);
126125
}
127126
return value;
128127
}
129-
130-
private Task<string> Inject(string value)
131-
{
132-
return _secretInjector.InjectAsync(value);
133-
}
134128
}
135129
}

‎src/NuGet.Services.KeyVault/CachingSecretReader.cs‎

Lines changed: 44 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
// Copyright (c) .NET Foundation. All rights reserved.
1+
// Copyright (c) .NET Foundation. All rights reserved.
22
// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information.
33

44
using System;
@@ -29,6 +29,16 @@ public CachingSecretReader(ISecretReader secretReader,
2929
_refreshIntervalBeforeExpiry = TimeSpan.FromSeconds(refreshIntervalBeforeExpirySec);
3030
}
3131

32+
public string GetSecret(string secretName)
33+
{
34+
return GetSecret(secretName, logger: null);
35+
}
36+
37+
public string GetSecret(string secretName, ILogger logger)
38+
{
39+
return GetSecretObject(secretName, logger).Value;
40+
}
41+
3242
public async Task<string> GetSecretAsync(string secretName)
3343
{
3444
return await GetSecretAsync(secretName, logger: null);
@@ -39,32 +49,51 @@ public async Task<string> GetSecretAsync(string secretName, ILogger logger)
3949
return (await GetSecretObjectAsync(secretName, logger)).Value;
4050
}
4151

42-
public async Task<ISecret> GetSecretObjectAsync(string secretName)
52+
public ISecret GetSecretObject(string secretName)
4353
{
44-
return await GetSecretObjectAsync(secretName, logger: null);
54+
return GetSecretObject(secretName, logger: null);
4555
}
4656

47-
public async Task<ISecret> GetSecretObjectAsync(string secretName, ILogger logger)
57+
public ISecret GetSecretObject(string secretName, ILogger logger)
4858
{
49-
if (string.IsNullOrEmpty(secretName))
59+
if (TryGetCachedSecretObject(secretName, logger, out var cachedSecret))
5060
{
51-
throw new ArgumentException("Null or empty secret name", nameof(secretName));
61+
return cachedSecret;
5262
}
5363

54-
// If the cache contains the secret and it is not expired, return the cached value.
64+
var start = DateTimeOffset.UtcNow;
65+
66+
var updatedValue = new CachedSecret(_internalReader.GetSecretObject(secretName));
67+
68+
return UpdateCachedSecret(secretName, logger, start, updatedValue);
69+
}
70+
71+
public async Task<ISecret> GetSecretObjectAsync(string secretName)
72+
{
73+
return await GetSecretObjectAsync(secretName, logger: null);
74+
}
75+
76+
public async Task<ISecret> GetSecretObjectAsync(string secretName, ILogger logger)
77+
{
5578
if (TryGetCachedSecretObject(secretName, logger, out var cachedSecret))
5679
{
5780
return cachedSecret;
5881
}
5982

6083
var start = DateTimeOffset.UtcNow;
61-
// The cache does not contain a fresh copy of the secret. Fetch and cache the secret.
84+
6285
var updatedValue = new CachedSecret(await _internalReader.GetSecretObjectAsync(secretName));
86+
87+
return UpdateCachedSecret(secretName, logger, start, updatedValue);
88+
}
89+
90+
private ISecret UpdateCachedSecret(string secretName, ILogger logger, DateTimeOffset start, CachedSecret updatedValue)
91+
{
6392
var updatedSecret = _cache.AddOrUpdate(secretName, updatedValue, (key, old) => updatedValue).Secret;
6493

6594
logger?.LogInformation("Refreshed secret {SecretName}, Expiring at: {ExpirationTime}. Took {ElapsedMilliseconds}ms.",
6695
updatedSecret.Name,
67-
updatedSecret.Expiration == null ? "null" : ((DateTimeOffset) updatedSecret.Expiration).UtcDateTime.ToString(),
96+
updatedSecret.Expiration == null ? "null" : ((DateTimeOffset)updatedSecret.Expiration).UtcDateTime.ToString(),
6897
(DateTimeOffset.UtcNow - start).TotalMilliseconds.ToString("F2"));
6998

7099
return updatedSecret;
@@ -87,6 +116,11 @@ public bool TryGetCachedSecret(string secretName, ILogger logger, out string sec
87116

88117
public bool TryGetCachedSecretObject(string secretName, ILogger logger, out ISecret secretObject)
89118
{
119+
if (string.IsNullOrEmpty(secretName))
120+
{
121+
throw new ArgumentException("Null or empty secret name", nameof(secretName));
122+
}
123+
90124
secretObject = null;
91125
if (_cache.TryGetValue(secretName, out CachedSecret result)
92126
&& !IsSecretOutdated(result))
@@ -122,4 +156,4 @@ public CachedSecret(ISecret secret)
122156

123157
}
124158
}
125-
}
159+
}

‎src/NuGet.Services.KeyVault/EmptySecretReader.cs‎

Lines changed: 23 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
// Copyright (c) .NET Foundation. All rights reserved.
1+
// Copyright (c) .NET Foundation. All rights reserved.
22
// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information.
33

44
using System.Threading.Tasks;
@@ -8,18 +8,38 @@ namespace NuGet.Services.KeyVault
88
{
99
public class EmptySecretReader : ICachingSecretReader
1010
{
11+
public string GetSecret(string secretName)
12+
{
13+
return GetSecret(secretName, logger: null);
14+
}
15+
16+
public string GetSecret(string secretName, ILogger logger)
17+
{
18+
return secretName;
19+
}
20+
1121
public Task<string> GetSecretAsync(string secretName) => GetSecretAsync(secretName, logger: null);
1222

1323
public Task<string> GetSecretAsync(string secretName, ILogger logger)
1424
{
1525
return Task.FromResult(secretName);
1626
}
1727

28+
public ISecret GetSecretObject(string secretName)
29+
{
30+
return GetSecretObject(secretName, logger: null);
31+
}
32+
33+
public ISecret GetSecretObject(string secretName, ILogger logger)
34+
{
35+
return new KeyVaultSecret(secretName, secretName, null);
36+
}
37+
1838
public Task<ISecret> GetSecretObjectAsync(string secretName) => GetSecretObjectAsync(secretName, logger: null);
1939

2040
public Task<ISecret> GetSecretObjectAsync(string secretName, ILogger logger)
2141
{
22-
return Task.FromResult((ISecret)new KeyVaultSecret(secretName, secretName, null));
42+
return Task.FromResult(GetSecretObject(secretName, logger));
2343
}
2444

2545
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
3858
return true;
3959
}
4060
}
41-
}
61+
}

‎src/NuGet.Services.KeyVault/IRefreshableSecretReaderFactory.cs‎

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
// Copyright (c) .NET Foundation. All rights reserved.
1+
// Copyright (c) .NET Foundation. All rights reserved.
22
// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information.
33

44
using System.Threading;
@@ -11,6 +11,13 @@ namespace NuGet.Services.KeyVault
1111
/// </summary>
1212
public interface IRefreshableSecretReaderFactory : ISecretReaderFactory
1313
{
14+
/// <summary>
15+
/// Refresh the values of the secrets that have already been read and cached. Since the cache is shared between
16+
/// all <see cref="ISecretReader"/> instances creates, this refresh applies to all secret readers created by
17+
/// this factory.
18+
/// </summary>
19+
void Refresh();
20+
1421
/// <summary>
1522
/// Refresh the values of the secrets that have already been read and cached. Since the cache is shared between
1623
/// all <see cref="ISecretReader"/> instances creates, this refresh applies to all secret readers created by

‎src/NuGet.Services.KeyVault/ISecretInjector.cs‎

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
// Copyright (c) .NET Foundation. All rights reserved.
1+
// Copyright (c) .NET Foundation. All rights reserved.
22
// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information.
33

44
using System.Threading.Tasks;
@@ -8,7 +8,9 @@ namespace NuGet.Services.KeyVault
88
{
99
public interface ISecretInjector
1010
{
11+
string Inject(string input);
12+
string Inject(string input, ILogger logger);
1113
Task<string> InjectAsync(string input);
1214
Task<string> InjectAsync(string input, ILogger logger);
1315
}
14-
}
16+
}

‎src/NuGet.Services.KeyVault/ISecretReader.cs‎

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
// Copyright (c) .NET Foundation. All rights reserved.
1+
// Copyright (c) .NET Foundation. All rights reserved.
22
// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information.
33

44
using System.Threading.Tasks;
@@ -8,9 +8,13 @@ namespace NuGet.Services.KeyVault
88
{
99
public interface ISecretReader
1010
{
11+
string GetSecret(string secretName);
12+
string GetSecret(string secretName, ILogger logger);
1113
Task<string> GetSecretAsync(string secretName);
1214
Task<string> GetSecretAsync(string secretName, ILogger logger);
15+
ISecret GetSecretObject(string secretName);
16+
ISecret GetSecretObject(string secretName, ILogger logger);
1317
Task<ISecret> GetSecretObjectAsync(string secretName);
1418
Task<ISecret> GetSecretObjectAsync(string secretName, ILogger logger);
1519
}
16-
}
20+
}

‎src/NuGet.Services.KeyVault/KeyVaultReader.cs‎

Lines changed: 28 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
// Copyright (c) .NET Foundation. All rights reserved.
1+
// Copyright (c) .NET Foundation. All rights reserved.
22
// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information.
33

44
using System;
@@ -33,6 +33,17 @@ public KeyVaultReader(KeyVaultConfiguration configuration)
3333
_keyVaultClient = new Lazy<SecretClient>(InitializeClient);
3434
}
3535

36+
public string GetSecret(string secretName)
37+
{
38+
return GetSecret(secretName, logger: null);
39+
}
40+
41+
public string GetSecret(string secretName, ILogger logger)
42+
{
43+
AzureSecurityKeyVaultSecret secret = _keyVaultClient.Value.GetSecret(secretName);
44+
return secret.Value;
45+
}
46+
3647
public async Task<string> GetSecretAsync(string secretName)
3748
{
3849
return await GetSecretAsync(secretName, logger: null);
@@ -44,6 +55,17 @@ public async Task<string> GetSecretAsync(string secretName, ILogger logger)
4455
return secret.Value;
4556
}
4657

58+
public ISecret GetSecretObject(string secretName)
59+
{
60+
return GetSecretObject(secretName, logger: null);
61+
}
62+
63+
public ISecret GetSecretObject(string secretName, ILogger logger)
64+
{
65+
AzureSecurityKeyVaultSecret secret = _keyVaultClient.Value.GetSecret(secretName);
66+
return MapSecret(secretName, secret);
67+
}
68+
4769
public async Task<ISecret> GetSecretObjectAsync(string secretName)
4870
{
4971
return await GetSecretObjectAsync(secretName, logger: null);
@@ -52,6 +74,11 @@ public async Task<ISecret> GetSecretObjectAsync(string secretName)
5274
public async Task<ISecret> GetSecretObjectAsync(string secretName, ILogger logger)
5375
{
5476
AzureSecurityKeyVaultSecret secret = await _keyVaultClient.Value.GetSecretAsync(secretName);
77+
return MapSecret(secretName, secret);
78+
}
79+
80+
private static ISecret MapSecret(string secretName, AzureSecurityKeyVaultSecret secret)
81+
{
5582
return new KeyVaultSecret(secretName, secret.Value, secret.Properties.ExpiresOn);
5683
}
5784

‎src/NuGet.Services.KeyVault/RefreshableSecretReader.cs‎

Lines changed: 37 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
// Copyright (c) .NET Foundation. All rights reserved.
1+
// Copyright (c) .NET Foundation. All rights reserved.
22
// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information.
33

44
using System;
@@ -32,6 +32,14 @@ public RefreshableSecretReader(
3232
_settings = settings ?? throw new ArgumentNullException(nameof(settings));
3333
}
3434

35+
public void Refresh()
36+
{
37+
foreach (var secretName in _cache.Keys)
38+
{
39+
UncachedGetSecretObject(secretName);
40+
}
41+
}
42+
3543
public async Task RefreshAsync(CancellationToken token)
3644
{
3745
foreach (var secretName in _cache.Keys)
@@ -45,19 +53,39 @@ public async Task RefreshAsync(CancellationToken token)
4553
}
4654
}
4755

56+
public string GetSecret(string secretName)
57+
{
58+
return GetSecret(secretName, logger: null);
59+
}
60+
61+
public string GetSecret(string secretName, ILogger logger)
62+
{
63+
return GetSecretObject(secretName, logger).Value;
64+
}
65+
4866
public Task<string> GetSecretAsync(string secretName)
4967
{
5068
return GetSecretAsync(secretName, logger: null);
5169
}
5270

53-
public Task<string> GetSecretAsync(string secretName, ILogger logger)
71+
public async Task<string> GetSecretAsync(string secretName, ILogger logger)
72+
{
73+
return (await GetSecretObjectAsync(secretName, logger)).Value;
74+
}
75+
76+
public ISecret GetSecretObject(string secretName)
77+
{
78+
return GetSecretObject(secretName, logger: null);
79+
}
80+
81+
public ISecret GetSecretObject(string secretName, ILogger logger)
5482
{
5583
if (TryGetCachedSecretObject(secretName, out var secret))
5684
{
57-
return Task.FromResult(secret.Value);
85+
return secret;
5886
}
5987

60-
return UncachedGetSecretAsync(secretName);
88+
return UncachedGetSecretObject(secretName);
6189
}
6290

6391
public Task<ISecret> GetSecretObjectAsync(string secretName)
@@ -108,10 +136,11 @@ public bool TryGetCachedSecretObject(string secretName, ILogger logger, out ISec
108136
public bool TryGetCachedSecretObject(string secretName, out ISecret secretObject)
109137
=> TryGetCachedSecretObject(secretName, logger: null, secretObject: out secretObject);
110138

111-
private async Task<string> UncachedGetSecretAsync(string secretName)
139+
private ISecret UncachedGetSecretObject(string secretName)
112140
{
113-
var secretObject = await UncachedGetSecretObjectAsync(secretName);
114-
return secretObject.Value;
141+
var secretObject = _secretReader.GetSecretObject(secretName);
142+
_cache.AddOrUpdate(secretName, secretObject, (_, __) => secretObject);
143+
return secretObject;
115144
}
116145

117146
private async Task<ISecret> UncachedGetSecretObjectAsync(string secretName)
@@ -121,4 +150,4 @@ private async Task<ISecret> UncachedGetSecretObjectAsync(string secretName)
121150
return secretObject;
122151
}
123152
}
124-
}
153+
}

0 commit comments

Comments
 (0)