Skip to content

Commit 497bd19

Browse files
gzubergzuber
committed
KeyVaultSecretsRepository will now page through all secrets in Key Vault.
1 parent 64ff906 commit 497bd19

File tree

4 files changed

+185
-15
lines changed

4 files changed

+185
-15
lines changed

‎release_notes.md‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,7 @@
2828
- Issue [#6574](https://github.com/Azure/azure-functions-host/issues/6574)
2929
- Metadata / Input binding data of type DateTime will not be serialzed as string
3030
If you are using these properties, please ensure your app is able to detect and handle the new schema.
31+
- Fixes [#6031](https://github.com/Azure/azure-functions-host/issues/6031), an issue where having a large number of secrets in a Key Vault Secrets Repository would cause Function and Host secrets to regenerate constantly. These secrets should no longer regenerate due to this error.
3132

3233
**Release sprint:** Sprint 84
3334
[ [bugs](https://github.com/Azure/azure-functions-host/issues?q=is%3Aissue+milestone%3A%22Functions+Sprint+84%22+label%3Abug+is%3Aclosed) | [features](https://github.com/Azure/azure-functions-host/issues?q=is%3Aissue+milestone%3A%22Functions+Sprint+84%22+label%3Afeature+is%3Aclosed) ]

‎src/WebJobs.Script.WebHost/Security/KeyManagement/KeyVaultSecretsRepository.cs‎

Lines changed: 55 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -68,17 +68,23 @@ public override async Task WriteAsync(ScriptSecretsType type, string functionNam
6868
Dictionary<string, string> dictionary = GetDictionaryFromScriptSecrets(secrets, functionName);
6969

7070
// Delete existing keys
71-
string prefix = (type == ScriptSecretsType.Host) ? HostPrefix : FunctionPrefix + Normalize(functionName);
71+
List<IEnumerable<SecretItem>> secretsPages = await GetKeyVaultSecretsPagesAsync(_keyVaultClient.Value, GetVaultBaseUrl());
7272
List<Task> deleteTasks = new List<Task>();
73-
foreach (SecretItem item in await _keyVaultClient.Value.GetSecretsAsync(GetVaultBaseUrl()))
73+
string prefix = (type == ScriptSecretsType.Host) ? HostPrefix : FunctionPrefix + Normalize(functionName);
74+
75+
foreach (SecretItem item in FindSecrets(secretsPages, x => x.Identifier.Name.StartsWith(prefix)))
7476
{
75-
// Delete only keys which are no longer exist in passed secrets
76-
if (item.Identifier.Name.StartsWith(prefix) && !dictionary.Keys.Contains(item.Identifier.Name))
77+
// Delete only keys which no longer exist in passed-in secrets
78+
if (!dictionary.Keys.Contains(item.Identifier.Name))
7779
{
7880
deleteTasks.Add(_keyVaultClient.Value.DeleteSecretAsync(GetVaultBaseUrl(), item.Identifier.Name));
7981
}
8082
}
81-
await Task.WhenAll(deleteTasks);
83+
84+
if (deleteTasks.Any())
85+
{
86+
await Task.WhenAll(deleteTasks);
87+
}
8288

8389
// Set new secrets
8490
List<Task> setTasks = new List<Task>();
@@ -112,28 +118,28 @@ public override Task<string[]> GetSecretSnapshots(ScriptSecretsType type, string
112118

113119
private async Task<ScriptSecrets> ReadHostSecrets()
114120
{
115-
IPage<SecretItem> secretItems = await _keyVaultClient.Value.GetSecretsAsync(GetVaultBaseUrl());
121+
List<IEnumerable<SecretItem>> secretsPages = await GetKeyVaultSecretsPagesAsync(_keyVaultClient.Value, GetVaultBaseUrl());
116122
List<Task<SecretBundle>> tasks = new List<Task<SecretBundle>>();
117123

118124
// Add master key task
119-
SecretItem masterItem = secretItems.FirstOrDefault(x => x.Identifier.Name.StartsWith(MasterKey));
120-
if (masterItem != null)
125+
List<SecretItem> masterItems = FindSecrets(secretsPages, x => x.Identifier.Name.StartsWith(MasterKey));
126+
if (masterItems.Count > 0)
121127
{
122-
tasks.Add(_keyVaultClient.Value.GetSecretAsync(GetVaultBaseUrl(), masterItem.Identifier.Name));
128+
tasks.Add(_keyVaultClient.Value.GetSecretAsync(GetVaultBaseUrl(), masterItems[0].Identifier.Name));
123129
}
124130
else
125131
{
126132
return null;
127133
}
128134

129135
// Add functionKey tasks
130-
foreach (SecretItem item in secretItems.Where(x => x.Identifier.Name.StartsWith(FunctionKeyPrefix)))
136+
foreach (SecretItem item in FindSecrets(secretsPages, x => x.Identifier.Name.StartsWith(FunctionKeyPrefix)))
131137
{
132138
tasks.Add(_keyVaultClient.Value.GetSecretAsync(GetVaultBaseUrl(), item.Identifier.Name));
133139
}
134140

135141
// Add systemKey tasks
136-
foreach (SecretItem item in secretItems.Where(x => x.Identifier.Name.StartsWith(SystemKeyPrefix)))
142+
foreach (SecretItem item in FindSecrets(secretsPages, x => x.Identifier.Name.StartsWith(SystemKeyPrefix)))
137143
{
138144
tasks.Add(_keyVaultClient.Value.GetSecretAsync(GetVaultBaseUrl(), item.Identifier.Name));
139145
}
@@ -168,13 +174,15 @@ private async Task<ScriptSecrets> ReadHostSecrets()
168174

169175
private async Task<ScriptSecrets> ReadFunctionSecrets(string functionName)
170176
{
171-
IPage<SecretItem> secretItems = await _keyVaultClient.Value.GetSecretsAsync(GetVaultBaseUrl());
177+
List<IEnumerable<SecretItem>> secretsPages = await GetKeyVaultSecretsPagesAsync(_keyVaultClient.Value, GetVaultBaseUrl());
172178
List<Task<SecretBundle>> tasks = new List<Task<SecretBundle>>();
173179
string prefix = $"{FunctionPrefix}{Normalize(functionName)}--";
174-
foreach (SecretItem item in secretItems.Where(x => x.Identifier.Name.StartsWith(prefix, StringComparison.OrdinalIgnoreCase)))
180+
181+
foreach (SecretItem item in FindSecrets(secretsPages, x => x.Identifier.Name.StartsWith(prefix, StringComparison.OrdinalIgnoreCase)))
175182
{
176183
tasks.Add(_keyVaultClient.Value.GetSecretAsync(GetVaultBaseUrl(), item.Identifier.Name));
177184
}
185+
178186
if (!tasks.Any())
179187
{
180188
return null;
@@ -195,6 +203,40 @@ private async Task<ScriptSecrets> ReadFunctionSecrets(string functionName)
195203
return functionSecrets;
196204
}
197205

206+
public static async Task<List<IEnumerable<SecretItem>>> GetKeyVaultSecretsPagesAsync(KeyVaultClient keyVaultClient, string keyVaultBaseUrl)
207+
{
208+
IPage<SecretItem> secretItems = await keyVaultClient.GetSecretsAsync(keyVaultBaseUrl);
209+
List<IEnumerable<SecretItem>> secretsPages = new List<IEnumerable<SecretItem>>() { secretItems };
210+
211+
while (!string.IsNullOrEmpty(secretItems.NextPageLink))
212+
{
213+
secretItems = await keyVaultClient.GetSecretsNextAsync(secretItems.NextPageLink);
214+
secretsPages.Add(secretItems);
215+
}
216+
217+
return secretsPages;
218+
}
219+
220+
public static List<SecretItem> FindSecrets(List<IEnumerable<SecretItem>> secretsPages, Func<SecretItem, bool> comparison = null)
221+
{
222+
// if no comparison is provided, every item is a match
223+
if (comparison == null)
224+
{
225+
comparison = x => true;
226+
}
227+
228+
var secretItems = new List<SecretItem>();
229+
foreach (IEnumerable<SecretItem> secretsPage in secretsPages)
230+
{
231+
foreach (SecretItem secretItem in secretsPage.Where(x => comparison(x)))
232+
{
233+
secretItems.Add(secretItem);
234+
}
235+
}
236+
237+
return secretItems;
238+
}
239+
198240
private string GetVaultBaseUrl()
199241
{
200242
return $"https://{_vaultName}.vault.azure.net";

‎test/WebJobs.Script.Tests.Integration/Host/SecretsRepositoryTests.cs‎

Lines changed: 79 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@
1616
using Microsoft.Extensions.Configuration;
1717
using Microsoft.Extensions.Logging;
1818
using Microsoft.Extensions.Logging.Abstractions;
19+
using Microsoft.Rest.Azure;
1920
using Microsoft.WebJobs.Script.Tests;
2021
using WebJobs.Script.Tests;
2122
using Xunit;
@@ -134,6 +135,78 @@ public async Task ReadAsync_ReadsExpectedFile(SecretsRepositoryType repositoryTy
134135
}
135136
}
136137

138+
[Theory] // Only for Key Vault to test paging over large number of secrets
139+
[InlineData(SecretsRepositoryType.KeyVault, ScriptSecretsType.Host)]
140+
[InlineData(SecretsRepositoryType.KeyVault, ScriptSecretsType.Function)]
141+
public async Task ReadAsync_ReadsExpectedKeyVaultPages(SecretsRepositoryType repositoryType, ScriptSecretsType secretsType)
142+
{
143+
using (var directory = new TempDirectory())
144+
{
145+
await _fixture.TestInitialize(repositoryType, directory.Path);
146+
ScriptSecrets testSecrets = null;
147+
int keyCount = 35;
148+
149+
List<Key> functionKeys = new List<Key>();
150+
for (int i = 0; i < keyCount; ++i)
151+
{
152+
functionKeys.Add(new Key(KeyName + Guid.NewGuid().ToString(), "test" + i.ToString()));
153+
}
154+
155+
if (secretsType == ScriptSecretsType.Host)
156+
{
157+
testSecrets = new HostSecrets()
158+
{
159+
MasterKey = new Key("master", "test"),
160+
FunctionKeys = functionKeys,
161+
SystemKeys = new List<Key>() { new Key(KeyName, "test") }
162+
};
163+
}
164+
else
165+
{
166+
testSecrets = new FunctionSecrets()
167+
{
168+
Keys = functionKeys
169+
};
170+
}
171+
string testFunctionName = secretsType == ScriptSecretsType.Host ? "host" : functionName;
172+
173+
await _fixture.WriteSecret(testFunctionName, testSecrets);
174+
175+
var target = _fixture.GetNewSecretRepository();
176+
177+
ScriptSecrets secretsContent = await target.ReadAsync(secretsType, testFunctionName);
178+
179+
if (secretsType == ScriptSecretsType.Host)
180+
{
181+
Assert.Equal((secretsContent as HostSecrets).MasterKey.Name, "master");
182+
Assert.Equal((secretsContent as HostSecrets).MasterKey.Value, "test");
183+
184+
Assert.Equal((secretsContent as HostSecrets).FunctionKeys.Count, functionKeys.Count);
185+
foreach (Key originalKey in functionKeys)
186+
{
187+
var matchingKeys = (secretsContent as HostSecrets).FunctionKeys.Where(x => string.Equals(x.Name, originalKey.Name));
188+
Assert.Equal(matchingKeys.Count(), 1);
189+
Assert.Equal(matchingKeys.First().Value, originalKey.Value);
190+
}
191+
192+
Assert.Equal((secretsContent as HostSecrets).SystemKeys[0].Name, KeyName);
193+
Assert.Equal((secretsContent as HostSecrets).SystemKeys[0].Value, "test");
194+
}
195+
else
196+
{
197+
Assert.Equal((secretsContent as FunctionSecrets).Keys.Count, functionKeys.Count);
198+
foreach (Key originalKey in functionKeys)
199+
{
200+
var matchingKeys = (secretsContent as FunctionSecrets).Keys.Where(x => string.Equals(x.Name, originalKey.Name));
201+
Assert.Equal(matchingKeys.Count(), 1);
202+
Assert.Equal(matchingKeys.First().Value, originalKey.Value);
203+
}
204+
}
205+
206+
}
207+
}
208+
209+
137210
[Theory]
138211
[InlineData(SecretsRepositoryType.BlobStorage, ScriptSecretsType.Host)]
139212
[InlineData(SecretsRepositoryType.BlobStorage, ScriptSecretsType.Function)]
@@ -565,9 +638,13 @@ private async Task ClearAllBlobSecrets()
565638

566639
private async Task ClearAllKeyVaultSecrets()
567640
{
568-
foreach (SecretItem item in await KeyVaultClient.GetSecretsAsync(GetKeyVaultBaseUrl()))
641+
var secretsPages = await KeyVaultSecretsRepository.GetKeyVaultSecretsPagesAsync(KeyVaultClient, GetKeyVaultBaseUrl());
642+
foreach (IPage<SecretItem> secretsPage in secretsPages)
569643
{
570-
await KeyVaultClient.DeleteSecretAsync(GetKeyVaultBaseUrl(), item.Identifier.Name);
644+
foreach (SecretItem item in secretsPage)
645+
{
646+
await KeyVaultClient.DeleteSecretAsync(GetKeyVaultBaseUrl(), item.Identifier.Name);
647+
}
571648
}
572649
}
573650
}

‎test/WebJobs.Script.Tests/Security/KeyVaultSecretsRepositoryTests.cs‎

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

4+
using System;
45
using System.Collections.Generic;
6+
using System.Linq;
7+
using Autofac.Core.Lifetime;
8+
using Microsoft.Azure.KeyVault.Models;
59
using Microsoft.Azure.WebJobs.Script.WebHost;
10+
using Microsoft.Rest.Azure;
611
using Xunit;
712

813
namespace Microsoft.Azure.WebJobs.Script.Tests
@@ -53,5 +58,50 @@ public void FunctionKeys(string functionName, string secretName)
5358

5459
Assert.True(dictionary[$"function--{KeyVaultSecretsRepository.Normalize(functionName)}--{KeyVaultSecretsRepository.Normalize(secretName)}"] == "test");
5560
}
61+
62+
[Theory]
63+
[MemberData(nameof(FindSecretsDataProvider.TestCases), MemberType = typeof(FindSecretsDataProvider))]
64+
public void FindSecrets(Func<SecretItem, bool> comparison, List<string> expectedMatches)
65+
{
66+
List<IEnumerable<SecretItem>> secretsPages = new List<IEnumerable<SecretItem>>()
67+
{
68+
new List<SecretItem>()
69+
{
70+
new SecretItem("https://testkeyvault.vault.azure.net/secrets/Atlanta"),
71+
new SecretItem("https://testkeyvault.vault.azure.net/secrets/Seattle"),
72+
new SecretItem("https://testkeyvault.vault.azure.net/secrets/NewYork"),
73+
new SecretItem("https://testkeyvault.vault.azure.net/secrets/Chicago")
74+
},
75+
new List<SecretItem>()
76+
{
77+
new SecretItem("https://testkeyvault.vault.azure.net/secrets/Portland"),
78+
new SecretItem("https://testkeyvault.vault.azure.net/secrets/Austin"),
79+
new SecretItem("https://testkeyvault.vault.azure.net/secrets/SanDiego"),
80+
new SecretItem("https://testkeyvault.vault.azure.net/secrets/LosAngeles")
81+
}
82+
};
83+
84+
var matches = KeyVaultSecretsRepository.FindSecrets(secretsPages, comparison);
85+
86+
Assert.Equal(expectedMatches.Count, matches.Count);
87+
foreach (string name in expectedMatches)
88+
{
89+
var matchingNames = matches.Where(x => x.Identifier.Name == name);
90+
Assert.Equal(matchingNames.Count(), 1);
91+
Assert.Equal(matchingNames.First().Identifier.Name, name);
92+
}
93+
}
94+
95+
public class FindSecretsDataProvider
96+
{
97+
public static IEnumerable<object[]> TestCases
98+
{
99+
get
100+
{
101+
yield return new object[] { (Func<SecretItem, bool>)(x => x.Identifier.Name.StartsWith("S")), new List<string>() { "Seattle", "SanDiego" } };
102+
yield return new object[] { (Func<SecretItem, bool>)(x => x.Identifier.Name.EndsWith("o")), new List<string>() { "Chicago", "SanDiego" } };
103+
}
104+
}
105+
}
56106
}
57107
}

0 commit comments

Comments
 (0)