Sync eng/common directory with azure-sdk-tools for PR 8598 (Azure#44935)

* Set storage account test resources to disable blob public access * Skip adding network rules to storage accounts that don't need them during cleanup * Add succeeded check to set pipeline subnet info step * Disable network firewall by default in resource creation/removal --------- Co-authored-by: Ben Broderick Phillips <[email protected]>
tejasm-microsoft · Jul 22, 2024 · 1d59e9f · 1d59e9f
1 parent e5f4d0c
commit 1d59e9f
Show file tree

Hide file tree

Showing 3 changed files with 47 additions and 32 deletions.
diff --git a/eng/common/TestResources/Remove-TestResources.ps1 b/eng/common/TestResources/Remove-TestResources.ps1
@@ -257,7 +257,7 @@ $verifyDeleteScript = {
 # Get any resources that can be purged after the resource group is deleted coerced into a collection even if empty.
 $purgeableResources = Get-PurgeableGroupResources $ResourceGroupName
 
-SetResourceNetworkAccessRules -ResourceGroupName $ResourceGroupName -AllowIpRanges $AllowIpRanges -Override -CI:$CI
+SetResourceNetworkAccessRules -ResourceGroupName $ResourceGroupName -AllowIpRanges $AllowIpRanges -SetFirewall -CI:$CI
 Remove-WormStorageAccounts -GroupPrefix $ResourceGroupName -CI:$CI
 
 Log "Deleting resource group '$ResourceGroupName'"

diff --git a/eng/common/TestResources/build-test-resource-config.yml b/eng/common/TestResources/build-test-resource-config.yml
@@ -16,7 +16,7 @@ parameters:
 steps:
   - task: AzurePowerShell@5
     displayName: Set Pipeline Subnet Info
-    condition: ne(variables['Pool'], '')
+    condition: and(succeeded(), ne(variables['Pool'], ''))
     env: ${{ parameters.EnvVars }}
     inputs:
       azureSubscription: azure-sdk-tests

diff --git a/eng/common/scripts/Helpers/Resource-Helpers.ps1 b/eng/common/scripts/Helpers/Resource-Helpers.ps1
@@ -308,51 +308,66 @@ function Remove-WormStorageAccounts() {
   }
 }
 
-function SetResourceNetworkAccessRules([string]$ResourceGroupName, [array]$AllowIpRanges, [switch]$CI) {
-  SetStorageNetworkAccessRules -ResourceGroupName $ResourceGroupName -AllowIpRanges $AllowIpRanges -CI:$CI
+function SetResourceNetworkAccessRules([string]$ResourceGroupName, [array]$AllowIpRanges, [switch]$CI, [switch]$SetFirewall) {
+  SetStorageNetworkAccessRules -ResourceGroupName $ResourceGroupName -AllowIpRanges $AllowIpRanges -CI:$CI -SetFirewall:$SetFirewall
 }
 
-function SetStorageNetworkAccessRules([string]$ResourceGroupName, [array]$AllowIpRanges, [switch]$CI, [switch]$Override) {
+function SetStorageNetworkAccessRules([string]$ResourceGroupName, [array]$AllowIpRanges, [switch]$CI, [switch]$SetFirewall) {
   $clientIp = $null
   $storageAccounts = Retry { Get-AzResource -ResourceGroupName $ResourceGroupName -ResourceType "Microsoft.Storage/storageAccounts" }
   # Add client IP to storage account when running as local user. Pipeline's have their own vnet with access
   if ($storageAccounts) {
     $appliedRule = $false
     foreach ($account in $storageAccounts) {
+      $properties = Get-AzStorageAccount -ResourceGroupName $ResourceGroupName -AccountName $account.Name
       $rules = Get-AzStorageAccountNetworkRuleSet -ResourceGroupName $ResourceGroupName -AccountName $account.Name
-      if ($rules -and ($Override -or $rules.DefaultAction -eq "Allow")) {
-        Write-Host "Restricting network rules in storage account '$($account.Name)' to deny access by default"
-        Retry { Update-AzStorageAccountNetworkRuleSet -ResourceGroupName $ResourceGroupName -Name $account.Name -DefaultAction Deny }
-        if ($CI -and $env:PoolSubnet) {
-          Write-Host "Enabling access to '$($account.Name)' from pipeline subnet $($env:PoolSubnet)"
-          Retry { Add-AzStorageAccountNetworkRule -ResourceGroupName $ResourceGroupName -Name $account.Name -VirtualNetworkResourceId $env:PoolSubnet }
-          $appliedRule = $true
-        }
-        elseif ($AllowIpRanges) {
-          Write-Host "Enabling access to '$($account.Name)' to $($AllowIpRanges.Length) IP ranges"
-          $ipRanges = $AllowIpRanges | ForEach-Object {
-            @{ Action = 'allow'; IPAddressOrRange = $_ }
-          }
-          Retry { Update-AzStorageAccountNetworkRuleSet -ResourceGroupName $ResourceGroupName -Name $account.Name -IPRule $ipRanges | Out-Null }
-          $appliedRule = $true
+
+      if ($properties.AllowBlobPublicAccess) {
+        Write-Host "Restricting public blob access in storage account '$($account.Name)'"
+        Set-AzStorageAccount -ResourceGroupName $ResourceGroupName -StorageAccountName $account.Name -AllowBlobPublicAccess $false
+      }
+
+      # In override mode, we only want to capture storage accounts that have had incomplete network rules applied,
+      # otherwise it's not worth updating due to timing and throttling issues.
+      # If the network rules are deny only without any vnet/ip allowances, then we can't ever purge the storage account
+      # when immutable blobs need to be removed.
+      if (!$rules -or !$SetFirewall -or $rules.DefaultAction -eq "Allow") {
+        return
+      }
+
+      # Add firewall rules in cases where existing rules added were incomplete to enable blob removal
+      Write-Host "Restricting network rules in storage account '$($account.Name)' to deny access by default"
+      Retry { Update-AzStorageAccountNetworkRuleSet -ResourceGroupName $ResourceGroupName -Name $account.Name -DefaultAction Deny }
+      if ($CI -and $env:PoolSubnet) {
+        Write-Host "Enabling access to '$($account.Name)' from pipeline subnet $($env:PoolSubnet)"
+        Retry { Add-AzStorageAccountNetworkRule -ResourceGroupName $ResourceGroupName -Name $account.Name -VirtualNetworkResourceId $env:PoolSubnet }
+        $appliedRule = $true
+      }
+      elseif ($AllowIpRanges) {
+        Write-Host "Enabling access to '$($account.Name)' to $($AllowIpRanges.Length) IP ranges"
+        $ipRanges = $AllowIpRanges | ForEach-Object {
+          @{ Action = 'allow'; IPAddressOrRange = $_ }
         }
-        elseif (!$CI) {
-          Write-Host "Enabling access to '$($account.Name)' from client IP"
-          $clientIp ??= Retry { Invoke-RestMethod -Uri 'https://icanhazip.com/' }  # cloudflare owned ip site
-          $clientIp = $clientIp.Trim()
-          $ipRanges = Get-AzStorageAccountNetworkRuleSet -ResourceGroupName $ResourceGroupName -Name $account.Name
-          if ($ipRanges) {
-            foreach ($range in $ipRanges.IpRules) {
-              if (DoesSubnetOverlap $range.IPAddressOrRange $clientIp) {
-                return
-              }
+        Retry { Update-AzStorageAccountNetworkRuleSet -ResourceGroupName $ResourceGroupName -Name $account.Name -IPRule $ipRanges | Out-Null }
+        $appliedRule = $true
+      }
+      elseif (!$CI) {
+        Write-Host "Enabling access to '$($account.Name)' from client IP"
+        $clientIp ??= Retry { Invoke-RestMethod -Uri 'https://icanhazip.com/' }  # cloudflare owned ip site
+        $clientIp = $clientIp.Trim()
+        $ipRanges = Get-AzStorageAccountNetworkRuleSet -ResourceGroupName $ResourceGroupName -Name $account.Name
+        if ($ipRanges) {
+          foreach ($range in $ipRanges.IpRules) {
+            if (DoesSubnetOverlap $range.IPAddressOrRange $clientIp) {
+              return
             }
           }
-          Retry { Add-AzStorageAccountNetworkRule -ResourceGroupName $ResourceGroupName -Name $account.Name -IPAddressOrRange $clientIp | Out-Null }
-          $appliedRule = $true
         }
+        Retry { Add-AzStorageAccountNetworkRule -ResourceGroupName $ResourceGroupName -Name $account.Name -IPAddressOrRange $clientIp | Out-Null }
+        $appliedRule = $true
       }
     }
+
     if ($appliedRule) {
       Write-Host "Sleeping for 15 seconds to allow network rules to take effect"
       Start-Sleep 15