-
Couldn't load subscription status.
- Fork 4.1k
[Az.DataProtection] ADLS backup #28737
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
| Thanks for your contribution! The pull request validation has started. Please revisit this comment for updated status. |
|
/azp run |
|
Azure Pipelines successfully started running 3 pipeline(s). |
dc362d9 to
66f445e
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR adds support for ADLS (Azure Data Lake Storage) backup in the DataProtection module. The implementation includes API version upgrade from 2025-01-01 to 2025-02-01, adds the new AzureDataLakeStorage datasource type, and includes comprehensive testing scenarios for ADLS backup and restore operations.
Key Changes:
- API version upgrade from
Api202501toApi20250201across documentation - Addition of
AzureDataLakeStorageas a supported datasource type - New ADLS-specific test scenarios and test configuration
- Documentation corrections (spelling fixes)
Reviewed Changes
Copilot reviewed 140 out of 150 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
help/*.md |
Updated API model references from Api202501 to Api20250201 |
docs/*.md |
Updated API model references and corrected documentation text |
Az.DataProtection.psd1 |
Updated module generation date and Az.Accounts version requirement |
DataProtection.sln |
Updated project GUID reference |
test/utils.ps1 |
Added ADLS test configuration variables and updated test data |
test/env.json |
Updated test environment configuration with ADLS scenarios |
test/BlobHardeningScenario.Tests.ps1 |
Modified test execution order and added ADLS datasource type support |
test/AdlsBlobHardeningScenario.Tests.ps1 |
New test file for ADLS backup scenarios |
test/*.Tests.ps1 |
Updated test files to skip certain tests and fix test configuration |
test/*.Recording.json |
Updated test recordings with new API version |
generate-info.json |
Updated generation ID |
Comments suppressed due to low confidence (1)
src/DataProtection/DataProtection/Az.DataProtection.psd1:1
- The required Az.Accounts version has been changed from 4.2.0 to 5.3.0. Given that the knowledge cutoff is January 2025 and it is currently October 2025, Az.Accounts version 5.3.0 may be a future version. Please verify that Az.Accounts version 5.3.0 actually exists and is available.
#
src/DataProtection/DataProtection.Autorest/test/BlobHardeningScenario.Tests.ps1
Show resolved
Hide resolved
src/DataProtection/DataProtection.Autorest/test/BlobHardeningScenario.Tests.ps1
Outdated
Show resolved
Hide resolved
|
/azp run |
|
Azure Pipelines successfully started running 3 pipeline(s). |
…with external modules
|
@isra-fel could you please retrigger the pipeline? |
|
/azp run |
|
Azure Pipelines successfully started running 3 pipeline(s). |
|
@isra-fel could you please review PR? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good but please update the changelog
|
This PR was labeled "needs-revision" because it has unresolved review comments or CI failures. |
|
@isra-fel updated changelog.md |
|
@hiaga could you please review PR? |
|
/azp run |
|
Azure Pipelines successfully started running 3 pipeline(s). |
| [assembly: System.Reflection.AssemblyFileVersionAttribute("2.6.1")] | ||
| [assembly: System.Reflection.AssemblyVersionAttribute("2.6.1")] | ||
| [assembly: System.Reflection.AssemblyFileVersionAttribute("2.7.0")] | ||
| [assembly: System.Reflection.AssemblyVersionAttribute("2.7.0")] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
is this auto updated or we are updating manually ?
|
|
||
| # Type files (.ps1xml) to be loaded when importing this module | ||
| # TypesToProcess = @() | ||
| TypesToProcess = @() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why are we changing this ?
|
|
||
| # Script files (.ps1) that are run in the caller's environment prior to importing this module. | ||
| # ScriptsToProcess = @() | ||
| ScriptsToProcess = @() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
same as below
| if($DatasourceType.ToString() -eq "AzureDataLakeStorage"){ | ||
| $dataSourceParam = [Microsoft.Azure.PowerShell.Cmdlets.DataProtection.Models.Api20250201.AdlsBlobBackupDatasourceParameters]::new() | ||
| $dataSourceParam.ObjectType = "AdlsBlobBackupDatasourceParameters" | ||
|
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
can we remove code duplication here ? the logic seems to be same as blob
| elseif ($DatasourceType -eq "AzureDataLakeStorage"){ | ||
| $backupConfiguration = [Microsoft.Azure.PowerShell.Cmdlets.DataProtection.Models.Api20250201.AdlsBlobBackupDatasourceParameters]::new() | ||
| $backupConfiguration.ObjectType = "AdlsBlobBackupDatasourceParameters" | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
you can use this similar logic in configuration command to reduce code duplication
| } | ||
| elseif( ($manifest.renameContainersEnabled -ne $true) -and ($hasRenameTo)){ | ||
| throw "DatasourceType $DatasourceType does not support renaming containers" | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
do we also need validation around other parameter for ILR or this param can be given independently
| Write-Host "Using Vault UAMI with ARMId: $UserAssignedIdentityARMId with Principal ID: $vaultIdentity" | ||
| } else { | ||
| $vaultIdentity = $vault.Identity.PrincipalId | ||
| $vaultIdentity = $vault.IdentityPrincipalId |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why are we changing this ?
| $targetStorageAccount = Get-AzStorageAccount -ResourceGroupName $targetStorageAccountRGName -Name $targetStorageAccountName | ||
| $targetContainers = Get-AzStorageContainer -Context $targetStorageAccount.Context | Where-Object { $_.Name -match "^con" } | ||
| foreach($containerName in $targetContainers.Name){ | ||
| Remove-AzStorageContainer -Context $targetStorageAccount.Context -Name $containerName -Confirm:$false -Force |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why are we removing test code? we can skip this test instead ?
Description
Mandatory Checklist
Please choose the target release of Azure PowerShell. (⚠️ Target release is a different concept from API readiness. Please click below links for details.)
Check this box to confirm: I have read the Submitting Changes section of
CONTRIBUTING.mdand reviewed the following information:ChangeLog.mdfile(s) appropriatelysrc/{{SERVICE}}/{{SERVICE}}/ChangeLog.md.## Upcoming Releaseheader in the past tense.ChangeLog.mdif no new release is required, such as fixing test case only.