From abb1d64402e1561366aa7ceda3ce9e6485707c2a Mon Sep 17 00:00:00 2001 From: Azure SDK Bot <53356347+azure-sdk@users.noreply.github.com> Date: Thu, 31 Oct 2024 13:18:13 -0700 Subject: [PATCH] Reduce unnecessary delete calls to ARM for storage accounts (#6161) Co-authored-by: Ben Broderick Phillips --- .../scripts/Helpers/Resource-Helpers.ps1 | 190 +++++++++--------- 1 file changed, 93 insertions(+), 97 deletions(-) diff --git a/eng/common/scripts/Helpers/Resource-Helpers.ps1 b/eng/common/scripts/Helpers/Resource-Helpers.ps1 index a5806ea1ea..ce8e40d15f 100644 --- a/eng/common/scripts/Helpers/Resource-Helpers.ps1 +++ b/eng/common/scripts/Helpers/Resource-Helpers.ps1 @@ -232,104 +232,10 @@ function Remove-WormStorageAccounts() { foreach ($group in $groups) { Write-Host "=========================================" $accounts = Get-AzStorageAccount -ResourceGroupName $group.ResourceGroupName - if ($accounts) { - foreach ($account in $accounts) { - if ($WhatIfPreference) { - Write-Host "What if: Removing $($account.StorageAccountName) in $($account.ResourceGroupName)" - } - else { - Write-Host "Removing $($account.StorageAccountName) in $($account.ResourceGroupName)" - } - - $hasContainers = ($account.Kind -ne "FileStorage") - - # If it doesn't have containers then we can skip the explicit clean-up of this storage account - if (!$hasContainers) { continue } - - $ctx = New-AzStorageContext -StorageAccountName $account.StorageAccountName - $containers = $ctx | Get-AzStorageContainer - $blobs = $containers | Get-AzStorageBlob - - $immutableBlobs = $containers ` - | Where-Object { $_.BlobContainerProperties.HasImmutableStorageWithVersioning } ` - | Get-AzStorageBlob - try { - foreach ($blob in $immutableBlobs) { - # We can't edit blobs with customer encryption without using that key - # so just try to delete them fully instead. It is unlikely they - # will also have a legal hold enabled. - if (($blob | Get-Member 'ListBlobProperties') ` - -and $blob.ListBlobProperties.Properties.CustomerProvidedKeySha256) { - Write-Host "Removing customer encrypted blob: $($blob.Name), account: $($account.StorageAccountName), group: $($group.ResourceGroupName)" - $blob | Remove-AzStorageBlob -Force - continue - } - - if (!($blob | Get-Member 'BlobProperties')) { - continue - } - - if ($blob.BlobProperties.LeaseState -eq 'Leased') { - Write-Host "Breaking blob lease: $($blob.Name), account: $($account.StorageAccountName), group: $($group.ResourceGroupName)" - $blob.ICloudBlob.BreakLease() - } + if (!$accounts) { break } - if ($blob.BlobProperties.HasLegalHold) { - Write-Host "Removing legal hold - blob: $($blob.Name), account: $($account.StorageAccountName), group: $($group.ResourceGroupName)" - $blob | Set-AzStorageBlobLegalHold -DisableLegalHold | Out-Null - } - } - } catch { - Write-Warning "Ensure user has 'Storage Blob Data Owner' RBAC permission on subscription or resource group" - Write-Error $_ - throw - } - # Sometimes we get a 404 blob not found but can still delete containers, - # and sometimes we must delete the blob if there's a legal hold. - # Try to remove the blob, but keep running regardless. - $succeeded = $false - for ($attempt = 0; $attempt -lt 2; $attempt++) { - if ($succeeded) { - break - } - - try { - foreach ($blob in $blobs) { - if ($blob.BlobProperties.ImmutabilityPolicy.PolicyMode) { - Write-Host "Removing immutability policy - blob: $($blob.Name), account: $($ctx.StorageAccountName), group: $($group.ResourceGroupName)" - $null = $blob | Remove-AzStorageBlobImmutabilityPolicy - } - } - } - catch {} - - try { - foreach ($blob in $blobs) { - $blob | Remove-AzStorageBlob -Force - } - $succeeded = $true - } - catch { - Write-Warning "Failed to remove blobs - account: $($ctx.StorageAccountName), group: $($group.ResourceGroupName)" - Write-Warning $_ - } - } - - try { - # Use AzRm cmdlet as deletion will only work through ARM with the immutability policies defined on the blobs - $containers | ForEach-Object { Remove-AzRmStorageContainer -Name $_.Name -StorageAccountName $ctx.StorageAccountName -ResourceGroupName $group.ResourceGroupName -Force } - } catch { - Write-Warning "Container removal failed. Ignoring the error and trying to delete the storage account." - Write-Warning $_ - } - Remove-AzStorageAccount -StorageAccountName $account.StorageAccountName -ResourceGroupName $account.ResourceGroupName -Force - } - } - if ($WhatIfPreference) { - Write-Host "What if: Removing resource group $($group.ResourceGroupName)" - } - else { - Remove-AzResourceGroup -ResourceGroupName $group.ResourceGroupName -Force -AsJob + foreach ($account in $accounts) { + RemoveStorageAccount -Account $account } } } @@ -401,6 +307,96 @@ function SetStorageNetworkAccessRules([string]$ResourceGroupName, [array]$AllowI } } +function RemoveStorageAccount($Account) { + Write-Host ($WhatIfPreference ? 'What if: ' : '') + "Readying $($Account.StorageAccountName) in $($Account.ResourceGroupName) for deletion" + # If it doesn't have containers then we can skip the explicit clean-up of this storage account + if ($Account.Kind -eq "FileStorage") { return } + + $containers = New-AzStorageContext -StorageAccountName $Account.StorageAccountName | Get-AzStorageContainer + $blobs = $containers | Get-AzStorageBlob + $deleteNow = @() + + try { + foreach ($blob in $blobs) { + $shouldDelete = EnableBlobDeletion -Blob $blob -StorageAccountName $Account.StorageAccountName -ResourceGroupName $Account.ResourceGroupName + if ($shouldDelete) { + $deleteNow += $blob + } + } + } catch { + Write-Warning "Ensure user has 'Storage Blob Data Owner' RBAC permission on subscription or resource group" + Write-Error $_ + throw + } + + # Blobs with legal holds or immutability policies must be deleted individually + # before the container/account can be deleted + foreach ($blobToDelete in $deleteNow) { + try { + $blobToDelete | Remove-AzStorageBlob -Force + } catch { + Write-Host "Blob removal failed: $($Blob.Name), account: $($Account.storageAccountName), group: $($Account.ResourceGroupName)" + Write-Warning "Ignoring the error and trying to delete the storage account" + Write-Warning $_ + } + } + + foreach ($container in $containers) { + if ($container.BlobContainerProperties.HasImmutableStorageWithVersioning) { + try { + # Use AzRm cmdlet as deletion will only work through ARM with the immutability policies defined on the blobs + Remove-AzRmStorageContainer -Name $container.Name -StorageAccountName $Account.StorageAccountName -ResourceGroupName $Account.ResourceGroupName -Force + #$container | Remove-AzStorageContainer + } catch { + Write-Host "Container removal failed: $($container.Name), account: $($Account.storageAccountName), group: $($Account.ResourceGroupName)" + Write-Warning "Ignoring the error and trying to delete the storage account" + Write-Warning $_ + } + } + } + + if ($containers) { + Remove-AzStorageAccount -StorageAccountName $Account.StorageAccountName -ResourceGroupName $Account.ResourceGroupName -Force + } +} + +function EnableBlobDeletion($Blob, $StorageAccountName, $ResourceGroupName) { + # Some properties like immutability policies require the blob to be + # deleted before the container can be deleted + $forceBlobDeletion = $false + + # We can't edit blobs with customer encryption without using that key + # so just try to delete them fully instead. It is unlikely they + # will also have a legal hold enabled. + if (($Blob | Get-Member 'ListBlobProperties') ` + -and $Blob.ListBlobProperties.Properties.CustomerProvidedKeySha256) { + return $true + } + + if (!($Blob | Get-Member 'BlobProperties')) { + return $false + } + + if ($Blob.BlobProperties.ImmutabilityPolicy.PolicyMode) { + Write-Host "Removing immutability policy - blob: $($Blob.Name), account: $StorageAccountName, group: $ResourceGroupName" + $null = $Blob | Remove-AzStorageBlobImmutabilityPolicy + $forceBlobDeletion = $true + } + + if ($Blob.BlobProperties.HasLegalHold) { + Write-Host "Removing legal hold - blob: $($Blob.Name), account: $StorageAccountName, group: $ResourceGroupName" + $Blob | Set-AzStorageBlobLegalHold -DisableLegalHold | Out-Null + $forceBlobDeletion = $true + } + + if ($Blob.BlobProperties.LeaseState -eq 'Leased') { + Write-Host "Breaking blob lease: $($Blob.Name), account: $StorageAccountName, group: $ResourceGroupName" + $Blob.ICloudBlob.BreakLease() + } + + return $forceBlobDeletion +} + function DoesSubnetOverlap([string]$ipOrCidr, [string]$overlapIp) { [System.Net.IPAddress]$overlapIpAddress = $overlapIp $parsed = $ipOrCidr -split '/'