[Az.Migrate] Add validation for HA, storagepath, storageaccount, and bug fix#29414
[Az.Migrate] Add validation for HA, storagepath, storageaccount, and bug fix#29414minhsuanlee wants to merge 7 commits intoAzure:mainfrom
Conversation
| Thanks for your contribution! The pull request validation has started. Please revisit this comment for updated status. |
There was a problem hiding this comment.
Pull request overview
Adds additional runtime validation and error handling for AzLocal migration/replication flows in Az.Migrate to prevent misconfiguration (HA state, storage path, and cache storage account settings) and improves caller identity resolution.
Changes:
- Tightens replication pre-checks around Hyper-V HA status (including an explicit “Unknown” path).
- Adds storage path existence/state validation during
New-AzMigrateLocalServerReplication. - Updates
Initialize-AzMigrateLocalReplicationInfrastructureto resolve caller identity across user/SP/managed identity and adds cache storage account validations.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
| src/Migrate/Migrate.Autorest/custom/New-AzMigrateLocalServerReplication.ps1 | Makes ARM ID validation unconditional and adds storage path existence/provisioning checks before starting replication. |
| src/Migrate/Migrate.Autorest/custom/Initialize-AzMigrateLocalReplicationInfrastructure.ps1 | Updates caller identity resolution logic and adds cache storage account validation (SKU and access settings). |
| src/Migrate/Migrate.Autorest/custom/Helper/AzLocalCommonSettings.ps1 | Extends HA constants/messages to include an “Unknown” state and improved wording. |
| src/Migrate/Migrate.Autorest/custom/Helper/AzLocalCommonHelper.ps1 | Enhances replication prerequisites to treat clustered Hyper-V VMs with unknown HA status as invalid. |
|
/azp run |
|
Azure Pipelines successfully started running 3 pipeline(s). |
|
/azp run |
|
Azure Pipelines successfully started running 3 pipeline(s). |
|
/azp run |
|
Azure Pipelines successfully started running 3 pipeline(s). |
|
|
1 similar comment
|
|
|
Validating changed scenarios:
2. [passed] Storage path check in `New-AzMigrateLocalServerReplication`
|
|
/azp run |
|
Azure Pipelines successfully started running 3 pipeline(s). |
|
/azp run |
|
Azure Pipelines successfully started running 3 pipeline(s). |
|
/azp run |
|
Azure Pipelines successfully started running 3 pipeline(s). |
| * Fixed bugs in `Initialize-AzMigrateLocalReplicationInfrastructure` | ||
| - Added early Azure login validation with a clear error message when user is not logged in | ||
| - Removed unnecessary caller identity resolution | ||
| - Added cache storage account validations to reject unsupported SKU tiers and disabled public network access |
There was a problem hiding this comment.
The changelog entry mentions "Removed unnecessary caller identity resolution", but the PR description calls out a bug fix for caller identity resolution specifically for Service Principal and Managed Identity. Consider updating this entry to describe the user-visible impact (e.g., initialization now works reliably when logged in with a service principal or managed identity) so release notes match the stated fix.
| # Validate Cache Storage Account SKU tier is Standard (not Premium) | ||
| if ($cacheStorageAccount.Sku.Tier -ne "Standard") { | ||
| throw "Cache Storage Account '$($cacheStorageAccount.StorageAccountName)' uses an unsupported SKU tier '$($cacheStorageAccount.Sku.Tier)'. Only 'Standard' tier storage accounts are supported. Please provide a Standard tier storage account." | ||
| } | ||
|
|
||
| # Validate public network access should not be disabled even for private endpoint | ||
| if (![string]::IsNullOrEmpty($cacheStorageAccount.PublicNetworkAccess) -and | ||
| $cacheStorageAccount.PublicNetworkAccess -eq "Disabled") { | ||
| throw "Cache Storage Account '$($cacheStorageAccount.StorageAccountName)' does not allow public network access. Please enable 'Public network access' on the storage account and re-run this command." | ||
| } |
There was a problem hiding this comment.
New cache storage account validations (SKU tier and PublicNetworkAccess) are added here, but the existing Pester tests for this cmdlet only cover the happy path. Please add test cases that exercise these failure modes (e.g., Premium tier and PublicNetworkAccess Disabled) so the new validation behavior doesn’t regress.
| # Validate storage path exists and is in a usable state | ||
| $storagePath = Get-AzResource ` | ||
| -ResourceId $TargetStoragePathId ` | ||
| -ErrorVariable notPresent ` | ||
| -ErrorAction SilentlyContinue | ||
| if ($null -eq $storagePath) { | ||
| throw "Storage path with Id '$TargetStoragePathId' not found. Please provide a valid storage path ARM ID." | ||
| } | ||
|
|
||
| # Creation must have succeeded for the storage path to be usable | ||
| $creationStatus = $storagePath.Properties.status.provisioningStatus.status | ||
| if ([string]::IsNullOrEmpty($creationStatus)) { | ||
| throw "Storage path '$($storagePath.Name)' creation status is unavailable. Please verify the storage path resource is fully provisioned." | ||
| } | ||
| if ($creationStatus -ne "Succeeded") { | ||
| throw "Storage path '$($storagePath.Name)' has a creation provisioning status of '$creationStatus'. Only storage paths with a successful creation can be used. Please select a different storage path or wait for provisioning to complete." | ||
| } | ||
|
|
||
| # The latest operation (ProvisioningState) must also be Succeeded | ||
| $provisioningState = $storagePath.Properties.provisioningState | ||
| if ([string]::IsNullOrEmpty($provisioningState)) { | ||
| throw "Storage path '$($storagePath.Name)' provisioning state is unavailable. Please verify the storage path resource is fully provisioned." | ||
| } | ||
| if ($provisioningState -ne "Succeeded") { | ||
| throw "Storage path '$($storagePath.Name)' has a provisioning state of '$provisioningState'. Only storage paths with a 'Succeeded' provisioning state can be used. Please resolve the issue or select a different storage path." |
There was a problem hiding this comment.
Storage path health validation is newly introduced here, but the Pester tests for New-AzMigrateLocalServerReplication are currently skipped and don’t cover these new error paths. Please add/enable tests that validate behavior for (1) non-existent storage path id and (2) non-succeeded provisioning states, ideally using playback recordings/mocking.
|
/azp run |
|
Azure Pipelines successfully started running 3 pipeline(s). |
Description
Add validation for HA, storagepath, storageaccount, and bug fix for caller identity resolution for Service Principal and Managed Identity
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.