Update RecoveryServices fetch ILR mount scripts with the new API - #30104
Open
Himanshu Agarwal (hiaga) wants to merge 5 commits into
Open
Himanshu Agarwal (hiaga) wants to merge 5 commits into
Himanshu Agarwal (hiaga) wants to merge 5 commits into
Conversation
…tion (2026-08-01) Regenerate Backup SDK against api-version 2026-08-01 (fetch ILR mount scripts / clientScripts via the new listMountScripts action instead of the broad ILR operationsStatus response) and wire Get-AzRecoveryServicesBackupRPMountScript through it. Addresses MSRC-114273 (iSCSI CHAP redaction). SDK generated from the pre-merge spec fork; repointed to the merged Azure spec in a follow-up commit. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 6d420238-f921-4e0f-ac6d-9acd4ed164b8
… #44639, 2026-08-01) Switch AutoRest input-file from the @hiaga pre-merge fork to Azure/azure-rest-api-specs at merge commit 4e6e13d398d4c2616237322a62394fc4f1dfeb81 (merged 2026-08-29). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 6d420238-f921-4e0f-ac6d-9acd4ed164b8
Contributor
There was a problem hiding this comment.
Copilot wasn't able to review any files in this pull request.
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Himanshu Agarwal (hiaga)
changed the title
… 2026-08-01 Refresh the ILR listMountScripts branch (MSRC-114273) on top of main's 2026-07-01 migration (Defender Source Scan + immutability AsPerPolicy fix). The 2026-08-01 generated SDK is a superset of 2026-07-01, so generated code is taken from this branch; hand-written adapter conflicts (BMSAPIs) take main's refactored helpers/LRO shape, which compile against the 2026-08-01 client. ChangeLog and Backup SDK README reconciled. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 6d420238-f921-4e0f-ac6d-9acd4ed164b8
Copilot started reviewing on behalf of
Himanshu Agarwal (hiaga)
September 13, 2026 16:23
View session
Contributor
There was a problem hiding this comment.
🟡 Changes recommended
Unresolved critical and moderate findings remain in null-result handling, terminal-status handling, and playback coverage.
Get a fresh assessment by requesting another Copilot review.
Review details
Suppressed comments (2)
src/RecoveryServices/RecoveryServices.Backup.Providers/Providers/IaasVmPsBackupProvider.cs:885
GetOperationStatusonly waits forInProgressand returns failed or canceled terminal statuses unchanged (TrackingHelpers.cs:48-63). Because that result is discarded here, a failed provision still calls the success-only script action, which can return a 4xx (or leave the existing null-result failure); inspect the terminal status and handle unsuccessful provisioning before fetching scripts.
InstantItemRecoveryTarget recoveryTarget =
ServiceClientAdapter.GetInstantItemRecoveryOperationResult(
src/RecoveryServices/RecoveryServices.Backup.Providers/Providers/IaasVmPsBackupProvider.cs:900
- The new action does not guarantee two entries:
ClientScriptForConnectexposesOSType, and the merged 2026-08-01 contract includes a one-item Windows response. A one-script Windows response therefore entersGenerateILRResponseForLinuxVMs, which expects base64 content containingTargetPassword, instead of handling the Windows URL/content form. Dispatch from the script's OS/type and honor theUrlversusScriptContentcontract rather than using the list length.
if (recoveryTarget.ClientScripts.Count == 2)
{
// clientScriptForConnection.OsType == "Windows"
result = this.GenerateILRResponseForWindowsVMs(
recoveryTarget.ClientScripts[1], out content);
- Files reviewed: 10/17 changed files
- Comments generated: 5
- Review effort level: Lite
Comment on lines
+893
to
+894
| if (recoveryTarget != null && recoveryTarget.ClientScripts != null && | ||
| recoveryTarget.ClientScripts.Count != 0) |
| this.PrivateEndpoint = new PrivateEndpointOperations(this); | ||
| this.BaseUri = new System.Uri("https://management.azure.com"); | ||
| this.ApiVersion = "2026-07-01"; | ||
| this.ApiVersion = "2026-08-01"; |
Comment on lines
+884
to
+885
| InstantItemRecoveryTarget recoveryTarget = | ||
| ServiceClientAdapter.GetInstantItemRecoveryOperationResult( |
| ``` yaml | ||
| # Lock to the merged Swagger PR commit so SDK generation remains reproducible. | ||
| commit: 53fc184a55bd2214e3bec2f1d9098501072e7d1e | ||
| # Spec source: merged Azure/azure-rest-api-specs PR #44639 (RecoveryServicesBackup 2026-08-01, listMountScripts / MSRC-114273), merged 2026-08-29. |
| --> | ||
|
|
||
| ## Upcoming Release | ||
| * Moved Instant Item Recovery (ILR) mount script retrieval to the dedicated `listMountScripts` action (api-version `2026-08-01`); `Get-AzRecoveryServicesBackupRPMountScript` no longer reads iSCSI CHAP connection details from the broad ILR operation-status response (MSRC-114273). |
…action Update Test-AzureVMRPMountScript to reflect the 2026-08-01 listInstantItemRecoveryOperationResult path: assert OsType/Password/Filename/ FilePath are returned via the dedicated action, and clarify the skip is live-only pending a re-record against an ILR-capable vault. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 6d420238-f921-4e0f-ac6d-9acd4ed164b8
Copilot started reviewing on behalf of
Himanshu Agarwal (hiaga)
September 14, 2026 03:11
View session
Contributor
There was a problem hiding this comment.
🟡 Changes recommended
Unresolved provider correctness, playback coverage, API metadata, and resource-cleanup issues remain.
Get a fresh assessment by requesting another Copilot review.
Review details
Suppressed comments (5)
src/RecoveryServices/RecoveryServices.Backup.Management.Sdk/README.md:27
- This source note uses
listMountScripts, but the merged 2026-08-01 spec and the generated SDK uselistInstantItemRecoveryOperationResultfor both the operation name and route. Please keep the generation metadata aligned with the actual spec operation.
# Spec source: merged Azure/azure-rest-api-specs PR #44639 (RecoveryServicesBackup 2026-08-01, listMountScripts / MSRC-114273), merged 2026-08-29.
src/RecoveryServices/RecoveryServices.Backup.Providers/Providers/IaasVmPsBackupProvider.cs:875
- The terminal operation status is discarded here, so a
FailedorCanceledprovision still proceeds to the mount-script action. That action is only valid for a successfully provisioned active ILR session, so failures will be masked by a secondary result-call error instead of surfacing the original operation error. Capture the returned status and handle unsuccessful terminal states before fetching scripts, as the other backup cmdlet flows do.
TrackingHelpers.GetOperationStatus(
ilRResponse,
opId => ServiceClientAdapter.GetProtectedItemOperationStatus(
src/RecoveryServices/RecoveryServices.Backup.ServiceClientAdapter/BMSAPIs/RecoveryPointsAPIs.cs:283
- This low-level generated call returns an
AzureOperationResponsethat is disposable, but the adapter readsBodyand drops the response without disposing it. The generated wrapper usesusingfor the same call (ItemLevelRecoveryConnectionsOperationsExtensions.cs:77-82); without that cleanup, repeated mount-script requests can retain HTTP response/connection resources until GC. Wrap this response inusing(or call the generated body-returning wrapper).
var response = BmsAdapter.Client.ItemLevelRecoveryConnections.ListInstantItemRecoveryOperationResultWithHttpMessagesAsync(
resourceGroupName ?? BmsAdapter.GetResourceGroupName(),
vaultName ?? BmsAdapter.GetResourceName(),
AzureFabricName,
containerName,
protectedItemName,
recoveryPointId,
provisionInstantItemRecoveryOperationId,
cancellationToken: BmsAdapter.CmdletCancellationToken).Result;
return response.Body;
src/RecoveryServices/RecoveryServices/ChangeLog.md:22
- The merged 2026-08-01 spec names the action
listInstantItemRecoveryOperationResult, which is also the generated SDK method and wire path;listMountScriptsis not the action name. Please update this release note so users can correlate it with the actual API.
* Moved Instant Item Recovery (ILR) mount script retrieval to the dedicated `listMountScripts` action (api-version `2026-08-01`); `Get-AzRecoveryServicesBackupRPMountScript` no longer reads iSCSI CHAP connection details from the broad ILR operation-status response (MSRC-114273).
src/RecoveryServices/RecoveryServices/ChangeLog.md:22
- The new release-note entry introduces the less-obvious acronyms
iSCSIandCHAPwithout expanding them. Expand them on first use so users can understand the security-sensitive connection details being moved.
* Moved Instant Item Recovery (ILR) mount script retrieval to the dedicated `listMountScripts` action (api-version `2026-08-01`); `Get-AzRecoveryServicesBackupRPMountScript` no longer reads iSCSI CHAP connection details from the broad ILR operation-status response (MSRC-114273).
- Files reviewed: 12/19 changed files
- Comments generated: 2
- Review effort level: Lite
Comment on lines
+896
to
+900
| if (recoveryTarget.ClientScripts.Count == 2) | ||
| { | ||
| if (recoveryTarget.ClientScripts.Count == 2) | ||
| { | ||
| // clientScriptForConnection.OsType == "Windows" | ||
| result = this.GenerateILRResponseForWindowsVMs( | ||
| recoveryTarget.ClientScripts[1], out content); | ||
| } | ||
| else | ||
| { | ||
| // clientScriptForConnection.OsType == "Linux" | ||
| result = this.GenerateILRResponseForLinuxVMs( | ||
| recoveryTarget.ClientScripts[0], | ||
| protectedItemName, rp.RecoveryPointTime.ToString(), out content); | ||
| } | ||
| // clientScriptForConnection.OsType == "Windows" | ||
| result = this.GenerateILRResponseForWindowsVMs( | ||
| recoveryTarget.ClientScripts[1], out content); |
Member
|
/azp run |
Contributor
|
Azure Pipelines: Successfully started running 3 pipeline(s). |
…cordings for api-version 2026-08-01 - Truly re-recorded Test-AzureVMRPMountScript live at api-version 2026-08-01 against a pre-provisioned ILR-capable setup, capturing the dedicated listInstantItemRecoveryOperationResult mount-script path (MSRC-114273). Un-skipped the [Fact] so it runs in playback in CI. Uses a deterministic recovery-point query window and does not delete the shared pre-provisioned vault/VM. - Updated the remaining Backup scenario recordings to api-version 2026-08-01 (backup-scoped URLs only, with recomputed EncodedRequestUri match keys). - Removed a trailing newline in generated CloudErrorBody.cs to clear the Analyze (9090) build failure. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 6d420238-f921-4e0f-ac6d-9acd4ed164b8
Copilot started reviewing on behalf of
Himanshu Agarwal (hiaga)
September 16, 2026 05:40
View session
Contributor
There was a problem hiding this comment.
🟡 Changes recommended
One or more issues must be addressed before approval.
Get a fresh assessment by requesting another Copilot review.
Review details
Suppressed comments (5)
src/RecoveryServices/RecoveryServices.Backup.Management.Sdk/README.md:27
- The generation provenance names the action
listMountScripts, but the merged spec and generated route uselistInstantItemRecoveryOperationResult. Keeping the obsolete name here makes the documented input misleading and can confuse future regeneration or investigation of the generated method.
# Spec source: merged Azure/azure-rest-api-specs PR #44639 (RecoveryServicesBackup 2026-08-01, listMountScripts / MSRC-114273), merged 2026-08-29.
src/RecoveryServices/RecoveryServices.Backup.Providers/Providers/IaasVmPsBackupProvider.cs:895
ClientScriptsis optional in the generatedInstantItemRecoveryTarget, but this branch leavesresultnull when the action returns no scripts and line 917 then dereferences it. Raise the existingILRNoClientScriptsReturnederror before usingresultinstead of exposing aNullReferenceException.
if (recoveryTarget != null && recoveryTarget.ClientScripts != null &&
recoveryTarget.ClientScripts.Count != 0)
{
src/RecoveryServices/RecoveryServices.Backup.Test/ScenarioTests/IaasVm/ItemTests.cs:118
- The newly enabled check-in fixture only uses the Windows VM, so it exercises the
ClientScripts.Count == 2branch but never validates the distinct Linux parsing path inGenerateILRResponseForLinuxVMs. Add a Linux recorded fixture or parameterize this scenario so the new action is covered for both supported OS variants.
[Fact]
[Trait(Category.AcceptanceType, Category.CheckIn)]
[Trait(TestConstants.Workload, TestConstants.AzureVM)]
public void TestAzureVMRPMountScript()
src/RecoveryServices/RecoveryServices.Backup.Test/ScenarioTests/IaasVm/ItemTests.ps1:1253
- The recorded action response contains two Windows scripts, so the provider selects the second and performs a direct
HttpWebRequestto itsdownload.microsoft.comURL. That download is outside the Azure session record and is not mocked here, making the newly enabled playback test depend on external network availability and prone to fail across CI operating systems; use an inline Linux fixture or mock/record this download before enabling the check-in test.
$mountScriptDetails = Get-AzRecoveryServicesBackupRPMountScript `
-VaultId $vault.ID `
-RecoveryPoint $rp
src/RecoveryServices/RecoveryServices/ChangeLog.md:22
- The user-facing changelog repeats the obsolete
listMountScriptsname, while the actual stable operation islistInstantItemRecoveryOperationResult. Update the entry so users and maintainers can identify the action that the cmdlet now calls.
- Files reviewed: 44/61 changed files
- Comments generated: 5
- Review effort level: Lite
| public System.Collections.Generic.IList<ErrorAdditionalInfo> AdditionalInfo {get; private set; } | ||
| } | ||
| } No newline at end of file | ||
| } |
Comment on lines
+873
to
877
| TrackingHelpers.GetOperationStatus( | ||
| ilRResponse, | ||
| operationId => ServiceClientAdapter.GetProtectedItemOperationStatus( | ||
| operationId, | ||
| opId => ServiceClientAdapter.GetProtectedItemOperationStatus( | ||
| opId, | ||
| vaultName: vaultName, |
Comment on lines
+115
to
118
| [Fact] | ||
| [Trait(Category.AcceptanceType, Category.CheckIn)] | ||
| [Trait(TestConstants.Workload, TestConstants.AzureVM)] | ||
| public void TestAzureVMRPMountScript() |
Comment on lines
+1262
to
+1264
| # Disable (revoke) the mount session created above. No resource-group cleanup: the | ||
| # vault and VM are a shared pre-provisioned setup and must not be deleted. | ||
| Disable-AzRecoveryServicesBackupRPMountScript -VaultId $vault.ID -RecoveryPoint $rp |
Member
|
/azp run |
Contributor
|
Azure Pipelines will not run the associated pipelines, because the pull request was updated after the run command was issued. Review the pull request again and issue a new run command. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
🤖 PR Validation — ❌ Action needed
️✔️Az.Accounts
❌Az.Automation
️✔️Az.Compute
️✔️Az.Network
❌Az.RecoveryServices
️✔️Az.RecoveryServices.Backup
Description
Moves Instant Item Recovery (ILR) mount script retrieval to the dedicated, RBAC-gated
listMountScriptsaction introduced in the stable API version2026-08-01ofMicrosoft.RecoveryServices/RecoveryServicesBackup.Previously the ILR mount scripts (which contain iSCSI CHAP connection details) were returned inline on the broad ILR-provision
operationsStatusresponse.Get-AzRecoveryServicesBackupRPMountScriptnow fetches them via the new action instead.Changes
2026-08-01from the merged spec: RecoveryServicesBackup: add 2026-08-01 stable version with fetchInstantItemRecoveryOperationResult action (MSRC-114273) azure-rest-api-specs#44639 (merged 2026-08-29, commit4e6e13d398d4c2616237322a62394fc4f1dfeb81).Get-AzRecoveryServicesBackupRPMountScript(and the Backup ServiceClientAdapter APIs) through the newlistMountScriptsaction.ChangeLog.mdupdated under Upcoming Release.Validation
eastus2euap) for both Linux and Windows IaaS VM ILR: provision -> operation-status polls ->listMountScriptscomplete under a single operation, and the mount script is returned by the new action (no CHAP details on the operation-status response). Mount + revoke succeed for both guest OS variants.Checklist
README.mdinput-file repointed from the pre-merge fork to Azure/azure-rest-api-specs).ChangeLog.mdupdated.clientScriptsremains optional).