Adds missing commits for cherry-pick secret repository tests fix (#10333

) * Updating Host CI lease access (#10314) * Updating Host CI lease access (#10294) * Fixing Azure Powershell argument configuration (#10307) * WorkloadIdentityCredential in KVSecretsRepository (#10310) * Updating secret repository tests (#10317) (#10328) --------- Co-authored-by: Fabio Cavalcante <[email protected]> Co-authored-by: Matthew Henderson <[email protected]>
Azure · Jul 25, 2024 · d991ffd · d991ffd
1 parent 832efff
commit d991ffd
Show file tree

Hide file tree

Showing 8 changed files with 182 additions and 140 deletions.
diff --git a/azure-pipelines.yml b/azure-pipelines.yml
@@ -340,17 +340,12 @@ jobs:
       targetType: 'inline'
       script: 'Install-Module -Name Az.Storage -RequiredVersion 1.11.0 -Scope CurrentUser -Force -AllowClobber'
 
-  - task: AzureKeyVault@1
-    inputs:
-      # Note: This is actually a Service Connection in DevOps, not an Azure subscription name
-      azureSubscription: 'Azure-Functions-Host-CI'
-      keyVaultName: 'azure-functions-host-ci'
-      secretsFilter: '*'
-  - task: PowerShell@2
+  - task: AzurePowerShell@5
     displayName: 'Checkout secrets'
     inputs:
-      filePath: '$(Build.Repository.LocalPath)\build\checkout-secrets.ps1'
-      arguments: '-connectionString ''$(Storage-azurefunctionshostci0)'''
+      azureSubscription: Azure-Functions-Host-CI
+      ScriptPath: '$(Build.Repository.LocalPath)\build\checkout-secrets.ps1'
+      azurePowerShellVersion: 'LatestVersion'
   - task: AzureKeyVault@1
     inputs:
       # Note: This is actually a Service Connection in DevOps, not an Azure subscription name
@@ -375,12 +370,14 @@ jobs:
       AzureWebJobsSecretStorageKeyVaultTenantId: $(AzureTenantId)
       AzureWebJobsSecretStorageKeyVaultClientId: $(AzureClientId)
       AzureWebJobsSecretStorageKeyVaultClientSecret: $(AzureClientSecret)
-  - task: PowerShell@2
+  - task: AzurePowerShell@5
     condition: always()
     displayName: 'Checkin secrets'
     inputs:
-      filePath: '$(Build.Repository.LocalPath)\build\checkin-secrets.ps1'
-      arguments: '-connectionString ''$(Storage-azurefunctionshostci0)'' -leaseBlob $(LeaseBlob) -leaseToken $(LeaseToken)'
+      azureSubscription: Azure-Functions-Host-CI
+      ScriptPath: '$(Build.Repository.LocalPath)\build\checkin-secrets.ps1'
+      ScriptArguments: '-leaseBlob $(LeaseBlob) -leaseToken $(LeaseToken)'
+      azurePowerShellVersion: 'LatestVersion'
 
 - job: RunIntegrationTests
   strategy: 
@@ -418,11 +415,12 @@ jobs:
       azureSubscription: 'Azure-Functions-Host-CI'
       keyVaultName: 'azure-functions-host-ci'
       secretsFilter: '*'
-  - task: PowerShell@2
+  - task: AzurePowerShell@5
     displayName: 'Checkout secrets'
     inputs:
-      filePath: '$(Build.Repository.LocalPath)\build\checkout-secrets.ps1'
-      arguments: '-connectionString ''$(Storage-azurefunctionshostci0)'''
+      azureSubscription: Azure-Functions-Host-CI
+      ScriptPath: '$(Build.Repository.LocalPath)\build\checkout-secrets.ps1'
+      azurePowerShellVersion: 'LatestVersion'
   - task: AzureKeyVault@1
     inputs:
       # Note: This is actually a Service Connection in DevOps, not an Azure subscription name
@@ -567,12 +565,14 @@ jobs:
       arguments: '--filter "Group=ReleaseTests" --no-build -f $(targetFramework)'
       projects: |
         **\WebJobs.Script.Tests.Integration.csproj
-  - task: PowerShell@2
+  - task: AzurePowerShell@5
     condition: always()
     displayName: 'Checkin secrets'
     inputs:
-      filePath: '$(Build.Repository.LocalPath)\build\checkin-secrets.ps1'
-      arguments: '-connectionString ''$(Storage-azurefunctionshostci0)'' -leaseBlob $(LeaseBlob) -leaseToken $(LeaseToken)'
+      azureSubscription: Azure-Functions-Host-CI
+      ScriptPath: '$(Build.Repository.LocalPath)\build\checkin-secrets.ps1'
+      ScriptArguments: '-leaseBlob $(LeaseBlob) -leaseToken $(LeaseToken)'
+      azurePowerShellVersion: 'LatestVersion'
 
 - job: PublishArtifacts
   dependsOn:

diff --git a/build/checkin-secrets.ps1 b/build/checkin-secrets.ps1
@@ -1,5 +1,4 @@
 param (
-  [string]$connectionString = "",
   [string]$leaseBlob = "",
   [string]$leaseToken = ""
 )
@@ -16,7 +15,7 @@ if ($leaseToken -eq "") {
 
 Write-Host "Breaking lease for $leaseBlob."
 
-$storageContext = New-AzStorageContext -ConnectionString $connectionString
+$storageContext = New-AzStorageContext -StorageAccountName "azurefunctionshostci0" -UseConnectedAccount
 $blob = Get-AzStorageBlob -Context $storageContext -Container "ci-locks" -Blob $leaseBlob
 
 $accessCondition = New-Object -TypeName Microsoft.Azure.Storage.AccessCondition

diff --git a/build/checkout-secrets.ps1 b/build/checkout-secrets.ps1
@@ -1,7 +1,3 @@
-param (
-  [string]$connectionString = ""
-)
-
 function AcquireLease($blob) {
   try {
     return $blob.ICloudBlob.AcquireLease($null, $null, $null, $null, $null)
@@ -14,15 +10,10 @@ function AcquireLease($blob) {
 # use this for tracking metadata in lease blobs
 $buildName = "3.0." + $env:buildNumber + "_" + $env:SYSTEM_JOBDISPLAYNAME
 
-$azVersion = "1.11.0"
 Import-Module Az.Storage
-$azModule = Get-Module -Name Az.Storage
-if ($azModule.Version -ne $azVersion) {
-  throw "Az.Storage module version $azVersion was not found. Current version: $($azModule.Version)"
-}
 
 # get a blob lease to prevent test overlap
-$storageContext = New-AzStorageContext -ConnectionString $connectionString
+$storageContext = New-AzStorageContext -StorageAccountName "azurefunctionshostci0" -UseConnectedAccount
 
 While($true) {
   $blobs = Get-AzStorageBlob -Context $storageContext -Container "ci-locks"

diff --git a/eng/ci/templates/official/jobs/run-integration-tests.yml b/eng/ci/templates/official/jobs/run-integration-tests.yml
@@ -41,18 +41,12 @@ jobs:
       command: ci
       workingDir: sample/CustomHandlerRetry
 
-  - task: AzureKeyVault@1
-    inputs:
-      # Note: This is actually a Service Connection in DevOps, not an Azure subscription name
-      azureSubscription: Azure-Functions-Host-CI-internal
-      keyVaultName: azure-functions-host-ci
-      secretsFilter: '*'
-
-  - task: PowerShell@2
+  - task: AzurePowerShell@5
     displayName: Checkout secrets
     inputs:
-      filePath: build/checkout-secrets.ps1
-      arguments: '-connectionString ''$(Storage-azurefunctionshostci0)'''
+      azureSubscription: Azure-Functions-Host-CI-internal
+      azurePowerShellVersion: 'LatestVersion'
+      ScriptPath: build/checkout-secrets.ps1
 
   - task: AzureKeyVault@1
     inputs:
@@ -200,9 +194,11 @@ jobs:
       arguments: '--filter "Group=ReleaseTests" --no-build'
       projects: $(IntegrationProject)
 
-  - task: PowerShell@2
+  - task: AzurePowerShell@5
     condition: always()
     displayName: Checkin secrets
     inputs:
-      filePath: build/checkin-secrets.ps1
-      arguments: '-connectionString ''$(Storage-azurefunctionshostci0)'' -leaseBlob $(LeaseBlob) -leaseToken $(LeaseToken)'
+      azureSubscription: Azure-Functions-Host-CI-internal
+      azurePowerShellVersion: 'LatestVersion'
+      ScriptPath: build/checkin-secrets.ps1
+      ScriptArguments: '-leaseBlob $(LeaseBlob) -leaseToken $(LeaseToken)'
diff --git a/eng/ci/templates/official/jobs/run-non-e2e-tests.yml b/eng/ci/templates/official/jobs/run-non-e2e-tests.yml
@@ -29,18 +29,12 @@ jobs:
       targetType: inline
       script: 'Install-Module -Name Az.Storage -RequiredVersion 1.11.0 -Scope CurrentUser -Force -AllowClobber'
 
-  - task: AzureKeyVault@1
-    inputs:
-      # Note: This is actually a Service Connection in DevOps, not an Azure subscription name
-      azureSubscription: Azure-Functions-Host-CI-internal
-      keyVaultName: azure-functions-host-ci
-      secretsFilter: '*'
-
-  - task: PowerShell@2
+  - task: AzurePowerShell@5
     displayName: Checkout secrets
     inputs:
-      filePath: build/checkout-secrets.ps1
-      arguments: '-connectionString ''$(Storage-azurefunctionshostci0)'''
+      azureSubscription: Azure-Functions-Host-CI-internal
+      azurePowerShellVersion: 'LatestVersion'
+      ScriptPath: build/checkout-secrets.ps1
 
   - task: AzureKeyVault@1
     inputs:
@@ -49,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:
@@ -73,9 +85,11 @@ jobs:
       AzureWebJobsSecretStorageKeyVaultClientId: $(AzureClientId)
       AzureWebJobsSecretStorageKeyVaultClientSecret: $(AzureClientSecret)
 
-  - task: PowerShell@2
+  - task: AzurePowerShell@5
     condition: always()
     displayName: Checkin secrets
     inputs:
-      filePath: build/checkin-secrets.ps1
-      arguments: '-connectionString ''$(Storage-azurefunctionshostci0)'' -leaseBlob $(LeaseBlob) -leaseToken $(LeaseToken)'
+      azureSubscription: Azure-Functions-Host-CI-internal
+      azurePowerShellVersion: 'LatestVersion'
+      ScriptPath: build/checkin-secrets.ps1
+      ScriptArguments: '-leaseBlob $(LeaseBlob) -leaseToken $(LeaseToken)'
diff --git a/src/WebJobs.Script.WebHost/Security/KeyManagement/KeyVaultSecretsRepository.cs b/src/WebJobs.Script.WebHost/Security/KeyManagement/KeyVaultSecretsRepository.cs
@@ -26,6 +26,7 @@ public class KeyVaultSecretsRepository : BaseSecretsRepository
         private const string FunctionKeyPrefix = "host--functionKey--";
         private const string SystemKeyPrefix = "host--systemKey--";
 
+        private readonly Lazy<TokenCredential> _tokenCredential;
         private readonly Lazy<SecretClient> _secretClient;
         private readonly IEnvironment _environment;
 
@@ -38,19 +39,38 @@ public KeyVaultSecretsRepository(string secretsSentinelFilePath, string vaultUri
 
             Uri keyVaultUri = string.IsNullOrEmpty(vaultUri) ? throw new ArgumentException(nameof(vaultUri)) : new Uri(vaultUri);
 
-            _secretClient = new Lazy<SecretClient>(() =>
+            _tokenCredential = new Lazy<TokenCredential>(() =>
             {
-                // If clientSecret and tenantId are provided, use ClientSecret credential; otherwise use managed identity
-                TokenCredential credential = !string.IsNullOrEmpty(clientSecret) && !string.IsNullOrEmpty(tenantId)
-                    ? new ClientSecretCredential(tenantId, clientId, clientSecret)
-                    : new ChainedTokenCredential(new ManagedIdentityCredential(clientId), new ManagedIdentityCredential());
+                if (!TryCreateTokenCredential(clientId, clientSecret, tenantId, out TokenCredential credential))
+                {
+                    throw new InvalidOperationException("Failed to create token credential for KeyVaultSecretsRepository");
+                }
 
-                return new SecretClient(keyVaultUri, credential);
+                return credential;
+            });
+
+            _secretClient = new Lazy<SecretClient>(() =>
+            {
+                return new SecretClient(keyVaultUri, _tokenCredential.Value);
             });
 
             _environment = environment ?? throw new ArgumentNullException(nameof(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(null, null, null, out TokenCredential credential))
+                {
+                    throw new InvalidOperationException("Failed to create token credential for KeyVaultSecretsRepository");
+                }
+
+                return new ChainedTokenCredential(testEnvironmentTokenCredential, credential);
+            });
+        }
+
         // For testing
         internal KeyVaultSecretsRepository(SecretClient secretClient, string secretsSentinelFilePath, ILogger logger, IEnvironment environment) : base(secretsSentinelFilePath, logger, environment)
         {
@@ -72,6 +92,25 @@ public override async Task<ScriptSecrets> ReadAsync(ScriptSecretsType type, stri
             return type == ScriptSecretsType.Host ? await ReadHostSecrets() : await ReadFunctionSecrets(functionName);
         }
 
+        private static bool TryCreateTokenCredential(string clientId, string clientSecret, string tenantId, out TokenCredential credential)
+        {
+            if (!string.IsNullOrEmpty(clientId) && !string.IsNullOrEmpty(clientSecret) && !string.IsNullOrEmpty(tenantId))
+            {
+                credential = new ClientSecretCredential(tenantId, clientId, clientSecret);
+                return true;
+            }
+            else if (!string.IsNullOrEmpty(clientId))
+            {
+                credential = new ManagedIdentityCredential(clientId);
+                return true;
+            }
+            else
+            {
+                credential = new ManagedIdentityCredential();
+                return true;
+            }
+        }
+
         public override async Task WriteAsync(ScriptSecretsType type, string functionName, ScriptSecrets secrets)
         {
             // Get secrets as dictionary

diff --git a/src/WebJobs.Script/Utility.cs b/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>