Updating secret repository tests (#10317)

Author: fabiocav

eng/ci/templates/official/jobs/run-non-e2e-tests.yml

@@ -43,6 +43,24 @@ jobs:
       keyVaultName: azure-functions-host-$(LeaseBlob)
       secretsFilter: '*'
 
+  - task: AzureCLI@2
+    displayName: 'Setup Azure environment'
+    inputs:
+      azureSubscription: Azure-Functions-Host-CI-internal
+      addSpnToEnvironment: true
+      scriptType: bash
+      scriptLocation: inlineScript
+      inlineScript: |
+        echo "##vso[task.setvariable variable=ARM_CLIENT_ID]$servicePrincipalId" 
+        echo "##vso[task.setvariable variable=ARM_ID_TOKEN]$idToken"
+        echo "##vso[task.setvariable variable=ARM_TENANT_ID]$tenantId"
+
+  # This step ensures the azure context defined by the previous task is persisted
+  # and available to subsequent steps/tasks.
+  - bash: |
+      az login --service-principal -u $(ARM_CLIENT_ID) --tenant $(ARM_TENANT_ID) --allow-no-subscriptions --federated-token $(ARM_ID_TOKEN)
+    displayName: 'Login to Azure'
+
   - task: DotNetCoreCLI@2
     displayName: Build Integration.csproj
     inputs:

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

@@ -57,11 +57,12 @@ public KeyVaultSecretsRepository(string secretsSentinelFilePath, string vaultUri
             _environment = environment ?? throw new ArgumentNullException(nameof(environment));
         }
 
-        internal KeyVaultSecretsRepository(string secretsSentinelFilePath, string vaultUri, string clientId, string clientSecret, string tenantId, ILogger logger, IEnvironment environment, TokenCredential testEnvironmentTokenCredential) : this(secretsSentinelFilePath, vaultUri, clientId, clientSecret, tenantId, logger, environment)
+        internal KeyVaultSecretsRepository(string secretsSentinelFilePath, string vaultUri, ILogger logger, IEnvironment environment, TokenCredential testEnvironmentTokenCredential)
+            : this(secretsSentinelFilePath, vaultUri, null, null, null, logger, environment)
         {
             _tokenCredential = new Lazy<TokenCredential>(() =>
             {
-                if (!TryCreateTokenCredential(clientId, clientSecret, tenantId, out TokenCredential credential))
+                if (!TryCreateTokenCredential(null, null, null, out TokenCredential credential))
                 {
                     throw new InvalidOperationException("Failed to create token credential for KeyVaultSecretsRepository");
                 }

src/WebJobs.Script/Utility.cs

@@ -141,8 +141,13 @@ await InvokeWithRetriesAsync(() =>
             }, maxRetries, retryInterval);
         }
 
-        internal static async Task InvokeWithRetriesAsync(Func<Task> action, int maxRetries, TimeSpan retryInterval)
+        internal static async Task InvokeWithRetriesWhenAsync(Func<Task> action, int maxRetries, TimeSpan retryInterval, Func<Exception, bool> predicate)
         {
+            if (predicate is null)
+            {
+                throw new ArgumentNullException(nameof(predicate));
+            }
+
             int attempt = 0;
             while (true)
             {
@@ -151,7 +156,7 @@ internal static async Task InvokeWithRetriesAsync(Func<Task> action, int maxRetr
                     await action();
                     return;
                 }
-                catch (Exception ex) when (!ex.IsFatal())
+                catch (Exception ex) when (predicate(ex))
                 {
                     if (++attempt > maxRetries)
                     {
@@ -162,6 +167,9 @@ internal static async Task InvokeWithRetriesAsync(Func<Task> action, int maxRetr
             }
         }
 
+        internal static Task InvokeWithRetriesAsync(Func<Task> action, int maxRetries, TimeSpan retryInterval)
+            => InvokeWithRetriesWhenAsync(action, maxRetries, retryInterval, (e) => !e.IsFatal());
+
         /// <summary>
         /// Delays while the specified condition remains true.
         /// </summary>

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

@@ -93,49 +93,47 @@ public async Task Constructor_CreatesSecretPathIfNotExists(SecretsRepositoryType
         [InlineData(SecretsRepositoryType.KeyVault, ScriptSecretsType.Function)]
         public async Task ReadAsync_ReadsExpectedFile(SecretsRepositoryType repositoryType, ScriptSecretsType secretsType)
         {
-            using (var directory = new TempDirectory())
+            using var directory = new TempDirectory();
+
+            await _fixture.TestInitialize(repositoryType, directory.Path);
+            ScriptSecrets testSecrets = null;
+            if (secretsType == ScriptSecretsType.Host)
             {
-                await _fixture.TestInitialize(repositoryType, directory.Path);
-                ScriptSecrets testSecrets = null;
-                if (secretsType == ScriptSecretsType.Host)
+                testSecrets = new HostSecrets()
                 {
-                    testSecrets = new HostSecrets()
-                    {
-                        MasterKey = new Key("master", "test"),
-                        FunctionKeys = new List<Key>() { new Key(KeyName, "test") },
-                        SystemKeys = new List<Key>() { new Key(KeyName, "test") }
-                    };
-                }
-                else
+                    MasterKey = new("master", "test"),
+                    FunctionKeys = [new(KeyName, "test")],
+                    SystemKeys = [new(KeyName, "test")]
+                };
+            }
+            else
+            {
+                testSecrets = new FunctionSecrets()
                 {
-                    testSecrets = new FunctionSecrets()
-                    {
-                        Keys = new List<Key>() { new Key(KeyName, "test") }
-                    };
-                }
-                string testFunctionName = secretsType == ScriptSecretsType.Host ? "host" : functionName;
+                    Keys = [new(KeyName, "test")]
+                };
+            }
+            string testFunctionName = secretsType == ScriptSecretsType.Host ? "host" : functionName;
 
-                await _fixture.WriteSecret(testFunctionName, testSecrets);
+            await _fixture.WriteSecret(testFunctionName, testSecrets);
 
-                var target = _fixture.GetNewSecretRepository();
+            var target = _fixture.GetNewSecretRepository();
 
-                ScriptSecrets secretsContent = await target.ReadAsync(secretsType, testFunctionName);
-
-                if (secretsType == ScriptSecretsType.Host)
-                {
-                    Assert.Equal((secretsContent as HostSecrets).MasterKey.Name, "master");
-                    Assert.Equal((secretsContent as HostSecrets).MasterKey.Value, "test");
-                    Assert.Equal((secretsContent as HostSecrets).FunctionKeys[0].Name, KeyName);
-                    Assert.Equal((secretsContent as HostSecrets).FunctionKeys[0].Value, "test");
-                    Assert.Equal((secretsContent as HostSecrets).SystemKeys[0].Name, KeyName);
-                    Assert.Equal((secretsContent as HostSecrets).SystemKeys[0].Value, "test");
-                }
-                else
-                {
-                    Assert.Equal((secretsContent as FunctionSecrets).Keys[0].Name, KeyName);
-                    Assert.Equal((secretsContent as FunctionSecrets).Keys[0].Value, "test");
-                }
+            ScriptSecrets secretsContent = await target.ReadAsync(secretsType, testFunctionName);
 
+            if (secretsType == ScriptSecretsType.Host)
+            {
+                Assert.Equal((secretsContent as HostSecrets).MasterKey.Name, "master");
+                Assert.Equal((secretsContent as HostSecrets).MasterKey.Value, "test");
+                Assert.Equal((secretsContent as HostSecrets).FunctionKeys[0].Name, KeyName);
+                Assert.Equal((secretsContent as HostSecrets).FunctionKeys[0].Value, "test");
+                Assert.Equal((secretsContent as HostSecrets).SystemKeys[0].Name, KeyName);
+                Assert.Equal((secretsContent as HostSecrets).SystemKeys[0].Value, "test");
+            }
+            else
+            {
+                Assert.Equal((secretsContent as FunctionSecrets).Keys[0].Name, KeyName);
+                Assert.Equal((secretsContent as FunctionSecrets).Keys[0].Value, "test");
             }
         }
 
@@ -526,6 +524,8 @@ async Task RunTest()
 
         public class Fixture : IDisposable
         {
+            private readonly DefaultAzureCredentialOptions _azureCredentialOptions;
+
             public Fixture()
             {
                 TestSiteName = "Test_test";
@@ -534,17 +534,15 @@ public Fixture()
                 var configuration = TestHelpers.GetTestConfiguration();
                 BlobConnectionString = configuration.GetWebJobsConnectionString(ConnectionStringNames.Storage);
                 KeyVaultUri = configuration.GetWebJobsConnectionString(EnvironmentSettingNames.AzureWebJobsSecretStorageKeyVaultUri);
-                KeyVaultClientId = configuration.GetWebJobsConnectionString(EnvironmentSettingNames.AzureWebJobsSecretStorageKeyVaultClientId);
-                KeyVaultClientSecret = configuration.GetWebJobsConnectionString(EnvironmentSettingNames.AzureWebJobsSecretStorageKeyVaultClientSecret);
-                KeyVaultTenantId = configuration.GetWebJobsConnectionString(EnvironmentSettingNames.AzureWebJobsSecretStorageKeyVaultTenantId);
 
-                if (KeyVaultTenantId is not null && KeyVaultClientId is not null &&
-                    KeyVaultClientSecret is not null && KeyVaultUri is not null)
+                // Exclude managed identity for the test to avoid using the
+                // credential setup in the test environment.
+                _azureCredentialOptions = new DefaultAzureCredentialOptions
                 {
-                    // These will fail later if required; but sometimes when testing locally you don't care about KeyVault
-                    var credential = new ClientSecretCredential(KeyVaultTenantId, KeyVaultClientId, KeyVaultClientSecret);
-                    SecretClient = new SecretClient(new Uri(KeyVaultUri), credential);
-                }
+                    ExcludeManagedIdentityCredential = true,
+                };
+
+                SecretClient = new SecretClient(new Uri(KeyVaultUri), new DefaultAzureCredential(_azureCredentialOptions));
 
                 AzureBlobStorageProvider = TestHelpers.GetAzureBlobStorageProvider(configuration);
             }
@@ -565,12 +563,6 @@ public Fixture()
 
             public string KeyVaultUri { get; private set; }
 
-            public string KeyVaultClientId { get; private set; }
-
-            public string KeyVaultClientSecret { get; private set; }
-
-            public string KeyVaultTenantId { get; private set; }
-
             public SecretsRepositoryType RepositoryType { get; private set; }
 
             public ILoggerProvider LoggerProvider { get; private set; }
@@ -612,22 +604,14 @@ public async Task TestInitialize(SecretsRepositoryType repositoryType, string se
             public ISecretsRepository GetNewSecretRepository()
             {
                 var logger = LoggerProvider.CreateLogger("Test");
-                if (RepositoryType == SecretsRepositoryType.BlobStorage)
-                {
-                    return new BlobStorageSecretsRepository(SecretsDirectory, ConnectionStringNames.Storage, TestSiteName, logger, Environment, AzureBlobStorageProvider);
-                }
-                else if (RepositoryType == SecretsRepositoryType.BlobStorageSas)
-                {
-                    return new BlobStorageSasSecretsRepository(SecretsDirectory, BlobSasConnectionUri.ToString(), TestSiteName, logger, Environment, AzureBlobStorageProvider);
-                }
-                else if (RepositoryType == SecretsRepositoryType.FileSystem)
-                {
-                    return new FileSystemSecretsRepository(SecretsDirectory, logger, Environment);
-                }
-                else
+
+                return RepositoryType switch
                 {
-                    return new KeyVaultSecretsRepository(SecretsDirectory, KeyVaultUri, KeyVaultClientId, KeyVaultClientSecret, KeyVaultTenantId, logger, Environment, new WorkloadIdentityCredential());
-                }
+                    SecretsRepositoryType.BlobStorage => new BlobStorageSecretsRepository(SecretsDirectory, ConnectionStringNames.Storage, TestSiteName, logger, Environment, AzureBlobStorageProvider),
+                    SecretsRepositoryType.BlobStorageSas => new BlobStorageSasSecretsRepository(SecretsDirectory, BlobSasConnectionUri.ToString(), TestSiteName, logger, Environment, AzureBlobStorageProvider),
+                    SecretsRepositoryType.FileSystem => new FileSystemSecretsRepository(SecretsDirectory, logger, Environment),
+                    _ => new KeyVaultSecretsRepository(SecretsDirectory, KeyVaultUri, logger, Environment, new DefaultAzureCredential(_azureCredentialOptions))
+                };
             }
 
             public void Dispose()
@@ -652,10 +636,8 @@ private string RelativeBlobPath(string functionNameOrHost)
 
             private string SecretsFileOrSentinelPath(string functionNameOrHost)
             {
-                string secretFilePath = null;
                 string fileName = string.Format(CultureInfo.InvariantCulture, "{0}.json", functionNameOrHost);
-                secretFilePath = Path.Combine(SecretsDirectory, fileName);
-                return secretFilePath;
+                return Path.Combine(SecretsDirectory, fileName);
             }
 
             public async Task WriteSecret(string functionNameOrHost, ScriptSecrets scriptSecret)
@@ -703,7 +685,8 @@ private async Task WriteSecretsKeyVaultAndUpdateSectinelFile(string functionName
                 Dictionary<string, string> dictionary = KeyVaultSecretsRepository.GetDictionaryFromScriptSecrets(secrets, functionNameOrHost);
                 foreach (string key in dictionary.Keys)
                 {
-                    await SecretClient.SetSecretAsync(key, dictionary[key]);
+                    await Utility.InvokeWithRetriesWhenAsync(() => SecretClient.SetSecretAsync(key, dictionary[key]),
+                        5, TimeSpan.FromSeconds(1), (e) => e is RequestFailedException rfex && rfex.Status == 409);
                 }
             }
 
@@ -818,16 +801,28 @@ private async Task ClearAllBlobSecrets()
                 }
             }
 
+            private async Task DeleteKeyVaultSecret(SecretProperties secret)
+            {
+                var result = await SecretClient.StartDeleteSecretAsync(secret.Name);
+                await result.WaitForCompletionAsync();
+
+                if (result.HasCompleted && secret.RecoveryLevel.Contains("purgeable", StringComparison.OrdinalIgnoreCase))
+                {
+                    await SecretClient.PurgeDeletedSecretAsync(secret.Name);
+                }
+            }
+
             private async Task ClearAllKeyVaultSecrets()
             {
-                var secretsPages = KeyVaultSecretsRepository.GetKeyVaultSecretsPagesAsync(SecretClient).AsPages();
-                await foreach (Page<SecretProperties> page in secretsPages)
+                var pages = SecretClient.GetPropertiesOfSecretsAsync();
+
+                var secretOperations = new List<Task>();
+                await foreach (var item in pages)
                 {
-                    foreach (SecretProperties item in page.Values)
-                    {
-                        await SecretClient.StartDeleteSecretAsync(item.Name);
-                    }
+                    secretOperations.Add(DeleteKeyVaultSecret(item));
                 }
+
+                await Task.WhenAll(secretOperations);
             }
         }
     }