diff --git a/.vscode/cspell.json b/.vscode/cspell.json index 655cdd95bc..f560f21a78 100644 --- a/.vscode/cspell.json +++ b/.vscode/cspell.json @@ -633,6 +633,45 @@ "navm", "rego", "virtualmachine", - "virtualnetwork" + "virtualnetwork", + "afsfileshare", + "azurebackup", + "iaasvmcontainerv", + "mydisk", + "myvm", + "protecteditem", + "slowvm", + "adls", + "alwayson", + "backupconfig", + "dbname", + "disasterrecovery", + "esan", + "fileshare", + "hana", + "lifecycles", + "pgflex", + "protectable", + "protectableitem", + "recoverypoint", + "saphana", + "saphanadatabase", + "saphanadbi", + "saphanadbinstance", + "saphanasystem", + "sqldatabase", + "sqlinstance", + "systemassigned", + "systemassigneduserassigned", + "userassigned", + "vaultconfig", + "vmname", + "azurebackuprg", + "diskname", + "georedundant", + "locallyredundant", + "undelete", + "undeletes", + "zoneredundant" ] } diff --git a/servers/Azure.Mcp.Server/CHANGELOG.md b/servers/Azure.Mcp.Server/CHANGELOG.md index 38033c533b..0868879a9f 100644 --- a/servers/Azure.Mcp.Server/CHANGELOG.md +++ b/servers/Azure.Mcp.Server/CHANGELOG.md @@ -10,6 +10,10 @@ The Azure MCP Server updates automatically by default whenever a new release com ### Bugs Fixed +- Fixed `azurebackup vault create --vault-type dpp` to enable a System-Assigned Managed Identity by default. Without this, every subsequent `protecteditem protect` call against the new DPP vault would fail server-side with `VaultMSIUnauthorized`. Existing vaults are unaffected; use `azurebackup vault update --identity-type SystemAssigned` if needed. +- Fixed `azurebackup protecteditem protect` to surface the real outcome of the protect operation instead of returning a synthetic "Accepted" with a base64 job id that callers could not resolve. RSV now polls the underlying `ConfigureBackup` job to a terminal state and returns its real status (`Completed`, `CompletedWithWarnings`, `Failed`, `Cancelled`, or `InProgress` if polling exceeds the budget) along with any error details. DPP now waits for the protect operation to complete and reads back the backup instance, returning the actual `protectionStatus` (e.g. `ProtectionConfigured`); DPP responses no longer carry a misleading `jobId` because DPP protection is not surfaced as a job — use `azurebackup protecteditem get` or `list` to verify. +- Fixed `azurebackup protecteditem protect` for IaaS VM (RSV) so it submits the protect request directly instead of doing a 180-second container-discovery pre-poll. Freshly created VMs no longer time out; container registration is performed by the Recovery Services backend as part of accepting the protect request, matching the behavior of `az backup protection enable-for-vm`. + ### Other Changes ## 3.0.0-beta.5 (2026-04-23) diff --git a/servers/Azure.Mcp.Server/docs/azmcp-commands.md b/servers/Azure.Mcp.Server/docs/azmcp-commands.md index b354404933..f11aa3d1a0 100644 --- a/servers/Azure.Mcp.Server/docs/azmcp-commands.md +++ b/servers/Azure.Mcp.Server/docs/azmcp-commands.md @@ -757,7 +757,7 @@ azmcp appservice webapp diagnostic diagnose --subscription "my-subscription" \ #### Vault ```bash -# Creates a new backup vault. Specify --vault-type as 'rsv' for a Recovery Services vault or 'dpp' for a Backup vault (Data Protection). Returns the created vault details. +# Creates a new backup vault. Specify --vault-type as 'rsv' for a Recovery Services vault or 'dpp' for a Backup vault (Data Protection). For DPP vaults a System-Assigned Managed Identity is enabled by default so the vault can authenticate to protected datasources (storage accounts, disks, PG Flex, etc.) - change later with 'azurebackup vault update --identity-type ...' if needed. Returns the created vault details. # ✅ Destructive | ❌ Idempotent | ❌ OpenWorld | ❌ ReadOnly | ❌ Secret | ❌ LocalRequired azmcp azurebackup vault create --subscription \ --resource-group \ @@ -823,7 +823,7 @@ azmcp azurebackup protecteditem get --subscription \ [--protected-item ] \ [--container ] -# Enables backup protection for a resource by creating a protected item or backup instance. +# Enables backup protection for a resource by creating a protected item or backup instance. For RSV the tool waits for the underlying ConfigureBackup job to reach a terminal state and returns the final job status; for DPP the tool waits for the protect operation to complete and reads back the backup instance, returning ProtectionStatus (DPP protection is not a job - use 'azurebackup protecteditem get' or 'list' to verify). # ✅ Destructive | ❌ Idempotent | ❌ OpenWorld | ❌ ReadOnly | ❌ Secret | ❌ LocalRequired azmcp azurebackup protecteditem protect --subscription \ --resource-group \ diff --git a/tools/Azure.Mcp.Tools.AzureBackup/src/Commands/Vault/VaultCreateCommand.cs b/tools/Azure.Mcp.Tools.AzureBackup/src/Commands/Vault/VaultCreateCommand.cs index 96798d1373..3fbe029fd4 100644 --- a/tools/Azure.Mcp.Tools.AzureBackup/src/Commands/Vault/VaultCreateCommand.cs +++ b/tools/Azure.Mcp.Tools.AzureBackup/src/Commands/Vault/VaultCreateCommand.cs @@ -25,7 +25,11 @@ public sealed class VaultCreateCommand(ILogger logger, IAzur public override string Description => """ Creates a new backup vault. Specify --vault-type as 'rsv' for a Recovery Services vault - or 'dpp' for a Backup vault (Data Protection). Returns the created vault details. + or 'dpp' for a Backup vault (Data Protection). For DPP vaults a System-Assigned + Managed Identity is enabled by default so the vault can authenticate to protected + datasources (storage accounts, disks, PG Flex, etc.) - change later with + 'azurebackup vault update --identity-type ...' if needed. Returns the created + vault details. """; public override string Title => CommandTitle; public override ToolMetadata Metadata => new() diff --git a/tools/Azure.Mcp.Tools.AzureBackup/src/Models/ProtectResult.cs b/tools/Azure.Mcp.Tools.AzureBackup/src/Models/ProtectResult.cs index 20b2533e43..544fdd7cf9 100644 --- a/tools/Azure.Mcp.Tools.AzureBackup/src/Models/ProtectResult.cs +++ b/tools/Azure.Mcp.Tools.AzureBackup/src/Models/ProtectResult.cs @@ -3,8 +3,41 @@ namespace Azure.Mcp.Tools.AzureBackup.Models; +/// +/// Result of an azurebackup protecteditem protect call. +/// +/// +/// Final outcome of the protect operation as observed by MCP after polling. +/// For RSV: terminal status of the ConfigureBackup job (e.g. Completed, +/// CompletedWithWarnings, Failed, Cancelled) or InProgress +/// if the job is still running when the polling budget is exhausted. +/// For DPP: Succeeded when the backup instance reaches ProtectionConfigured +/// (or ConfiguringProtection if still finalizing) or Failed on error. +/// +/// +/// RSV protected item name or DPP backup instance name. Use this with +/// azurebackup protecteditem get. +/// +/// +/// RSV ConfigureBackup job id (use with azurebackup job get). Always +/// null for DPP — DPP protection is not surfaced as a job; verify with +/// azurebackup protecteditem get or list. +/// +/// Human-readable summary of the outcome. +/// +/// DPP only — actual protectionStatus.status read back from the backup +/// instance after the operation (e.g. ProtectionConfigured, +/// ConfiguringProtection, ProtectionError). +/// +/// +/// Error detail when is Failed. For RSV this comes +/// from the failed ConfigureBackup job; for DPP it comes from the +/// async operationStatus error envelope. +/// public sealed record ProtectResult( string Status, string? ProtectedItemName, string? JobId, - string? Message); + string? Message, + string? ProtectionStatus = null, + string? ErrorMessage = null); diff --git a/tools/Azure.Mcp.Tools.AzureBackup/src/Services/DppBackupOperations.cs b/tools/Azure.Mcp.Tools.AzureBackup/src/Services/DppBackupOperations.cs index 34fe3a4658..e6264315fb 100644 --- a/tools/Azure.Mcp.Tools.AzureBackup/src/Services/DppBackupOperations.cs +++ b/tools/Azure.Mcp.Tools.AzureBackup/src/Services/DppBackupOperations.cs @@ -64,7 +64,16 @@ public async Task CreateVaultAsync( } }; - var vaultData = new DataProtectionBackupVaultData(new AzureLocation(location), new DataProtectionBackupVaultProperties(storageSettings)); + var vaultData = new DataProtectionBackupVaultData(new AzureLocation(location), new DataProtectionBackupVaultProperties(storageSettings)) + { + // DPP (Backup Vault) requires a Managed Identity to authenticate to protected + // datasources (storage accounts, disks, PG Flex, etc.). Without it every + // 'protecteditem protect' call would fail server-side with VaultMSIUnauthorized. + // Default to SystemAssigned so the vault is usable out of the box; callers can + // change this later via 'vault update --identity-type ...'. + Identity = new Azure.ResourceManager.Models.ManagedServiceIdentity( + Azure.ResourceManager.Models.ManagedServiceIdentityType.SystemAssigned) + }; var result = await collection.CreateOrUpdateAsync(WaitUntil.Completed, vaultName, vaultData, cancellationToken); @@ -197,15 +206,55 @@ public async Task ProtectItemAsync( Properties = instanceProperties }; - var result = await collection.CreateOrUpdateAsync(WaitUntil.Started, instanceName, instanceData, cancellationToken); + // DPP protection is asynchronous on the server side and is NOT surfaced as a + // backup job (only on-demand backup, restore, etc. are jobs). MCP must therefore + // wait for the underlying operationStatus to reach a terminal state and then read + // back the BackupInstance to confirm the protection actually configured. Using + // WaitUntil.Completed lets the SDK poll the Azure-AsyncOperation header for us + // and surface the real server-side error (e.g. VaultMSIUnauthorized) as a + // RequestFailedException, instead of silently returning "Accepted". + ArmOperation operation; + try + { + operation = await collection.CreateOrUpdateAsync( + WaitUntil.Completed, instanceName, instanceData, cancellationToken); + } + catch (RequestFailedException ex) + { + return new ProtectResult( + "Failed", + instanceName, + JobId: null, + $"Protection failed for backup instance '{instanceName}': {ex.Message}", + ProtectionStatus: null, + ErrorMessage: ex.Message); + } - var jobId = ExtractJobIdFromOperation(result.GetRawResponse()); + // Re-read the backup instance to capture the authoritative protection status. + // The LRO can complete while the BI is still in ConfiguringProtection; both + // outcomes are surfaced to the caller via ProtectionStatus. If the re-read + // fails with a transient error, report success (protection did complete) and + // let the caller verify with 'protecteditem get'. + string? protectionStatus = null; + try + { + var instanceResource = armClient.GetDataProtectionBackupInstanceResource(operation.Value.Id); + var bi = await instanceResource.GetAsync(cancellationToken); + protectionStatus = bi.Value.Data.Properties?.ProtectionStatus?.Status?.ToString(); + } + catch (RequestFailedException) + { + // Transient re-read failure; protection itself succeeded. + } return new ProtectResult( - "Accepted", + "Succeeded", instanceName, - jobId, - jobId != null ? $"Protection initiated. Use 'azurebackup job get --job {jobId}' to monitor progress." : "Protection initiated."); + JobId: null, + $"Protection configured for backup instance '{instanceName}' (status: {protectionStatus ?? "Unknown"}). " + + $"Use 'azurebackup protecteditem get --protected-item {instanceName}' to view details.", + ProtectionStatus: protectionStatus, + ErrorMessage: null); } public async Task GetProtectedItemAsync( diff --git a/tools/Azure.Mcp.Tools.AzureBackup/src/Services/RsvBackupOperations.cs b/tools/Azure.Mcp.Tools.AzureBackup/src/Services/RsvBackupOperations.cs index 3b9b064999..a41558c8d9 100644 --- a/tools/Azure.Mcp.Tools.AzureBackup/src/Services/RsvBackupOperations.cs +++ b/tools/Azure.Mcp.Tools.AzureBackup/src/Services/RsvBackupOperations.cs @@ -150,8 +150,9 @@ public async Task ProtectItemAsync( var jobId = await FindLatestJobIdAsync(armClient, subscription, resourceGroup, vaultName, "ConfigureBackup", cancellationToken); jobId ??= ExtractOperationIdFromResponse(result.GetRawResponse()); - return new ProtectResult("Accepted", protectedItemName, jobId, - jobId != null ? $"Workload protection initiated. Use 'azurebackup job get --job {jobId}' to monitor progress." : "Workload protection initiated."); + return await BuildRsvProtectResultAsync( + armClient, subscription, resourceGroup, vaultName, protectedItemName, jobId, + "Workload protection", cancellationToken); } if (profile.ProtectedItemType == RsvProtectedItemType.AzureFileShare) @@ -196,45 +197,16 @@ public async Task ProtectItemAsync( var fsJobId = await FindLatestJobIdAsync(armClient, subscription, resourceGroup, vaultName, "ConfigureBackup", cancellationToken); fsJobId ??= ExtractOperationIdFromResponse(fsResult.GetRawResponse()); - return new ProtectResult("Accepted", fsProtectedItemName, fsJobId, - fsJobId != null ? $"File share protection initiated. Use 'azurebackup job get --job {fsJobId}' to monitor progress." : "File share protection initiated."); + return await BuildRsvProtectResultAsync( + armClient, subscription, resourceGroup, vaultName, fsProtectedItemName, fsJobId, + "File share protection", cancellationToken); } - var rgId = ResourceGroupResource.CreateResourceIdentifier(subscription, resourceGroup); - var rgResource = armClient.GetResourceGroupResource(rgId); - await rgResource.RefreshProtectionContainerAsync(vaultName, FabricName, filter: null, cancellationToken); - + // For IaaS VM protection MCP follows the same approach as `az backup protection enable-for-vm`: + // submit the protected-item PUT directly. The Recovery Services backend registers the + // VM container as part of accepting the protect request, so a separate refresh + + // discovery poll is unnecessary and was causing 180s timeouts on freshly created VMs. var container = containerName ?? RsvNamingHelper.DeriveContainerName(datasourceId); - - // Poll for container visibility after refresh (up to 180s with 5s intervals). - // The RSV RefreshProtectionContainerAsync API does not return a pollable LRO, - // so we must manually poll for the container to become visible. - // Container discovery can take 2-3 minutes for some workloads. - const int maxRetries = 36; - const int delayMs = 5000; - for (int i = 0; i < maxRetries; i++) - { - await Task.Delay(delayMs, cancellationToken); - try - { - var checkContainerId = BackupProtectionContainerResource.CreateResourceIdentifier( - subscription, resourceGroup, vaultName, FabricName, container); - var checkContainer = armClient.GetBackupProtectionContainerResource(checkContainerId); - await checkContainer.GetAsync(cancellationToken: cancellationToken); - break; // Container is visible - } - catch (RequestFailedException ex) when (ex.Status == 404) - { - if (i == maxRetries - 1) - { - throw new InvalidOperationException( - $"Container '{container}' was not discovered after {maxRetries * delayMs / 1000}s. " + - "Container discovery can take several minutes for some workloads. " + - "Retry later or verify the VM resource ID is correct.", ex); - } - } - } - var vmProtectedItemName = RsvNamingHelper.DeriveProtectedItemName(datasourceId); var vmProtectedItemId = BackupProtectedItemResource.CreateResourceIdentifier( @@ -255,11 +227,9 @@ public async Task ProtectItemAsync( var vmJobId = await FindLatestJobIdAsync(armClient, subscription, resourceGroup, vaultName, "ConfigureBackup", cancellationToken); vmJobId ??= ExtractOperationIdFromResponse(vmResult.GetRawResponse()); // Fallback to operation ID - return new ProtectResult( - "Accepted", - vmProtectedItemName, - vmJobId, - vmJobId != null ? $"Protection initiated. Use 'azurebackup job get --job {vmJobId}' to monitor progress." : "Protection initiated."); + return await BuildRsvProtectResultAsync( + armClient, subscription, resourceGroup, vaultName, vmProtectedItemName, vmJobId, + "VM protection", cancellationToken); } public async Task GetProtectedItemAsync( @@ -1235,6 +1205,120 @@ private static string ExtractContainerName(string resourceId) return resourceId[start..end]; } + /// + /// Polls the RSV ConfigureBackup job to a terminal state and builds a + /// reflecting the actual job outcome. RSV protection is + /// asynchronous; the protect PUT only accepts the request, so MCP must follow up by + /// reading the job until it reports success or failure. If polling exceeds the timeout + /// the result is returned with status InProgress and the job id, so the caller + /// can continue monitoring with azurebackup job get. + /// + private static async Task BuildRsvProtectResultAsync( + ArmClient armClient, string subscription, string resourceGroup, string vaultName, + string protectedItemName, string? jobId, string operationDescription, + CancellationToken cancellationToken) + { + if (string.IsNullOrEmpty(jobId)) + { + return new ProtectResult( + "Accepted", + protectedItemName, + null, + $"{operationDescription} initiated. Use 'azurebackup protecteditem get' to verify."); + } + + var finalJob = await WaitForJobAsync( + armClient, subscription, resourceGroup, vaultName, jobId, cancellationToken); + + if (finalJob == null) + { + return new ProtectResult( + "InProgress", + protectedItemName, + jobId, + $"{operationDescription} is still running after the polling budget elapsed. " + + $"Use 'azurebackup job get --job {jobId}' to continue monitoring."); + } + + var status = finalJob.Status ?? "Unknown"; + var errorMessage = ExtractJobErrorMessage(finalJob); + var isFailure = status.Contains("Fail", StringComparison.OrdinalIgnoreCase) || + status.Equals("Cancelled", StringComparison.OrdinalIgnoreCase); + + var message = isFailure + ? $"{operationDescription} failed: {errorMessage ?? status}. See 'azurebackup job get --job {jobId}' for details." + : $"{operationDescription} status: {status}. Use 'azurebackup protecteditem get' to verify the protected item."; + + return new ProtectResult( + status, + protectedItemName, + jobId, + message, + ProtectionStatus: null, + ErrorMessage: isFailure ? errorMessage ?? status : null); + } + + /// + /// Polls a Recovery Services backup job until it reaches a terminal state. Returns the + /// final on completion, or null if the job did not + /// reach a terminal state within the polling budget. ConfigureBackup jobs typically + /// finish in 2-10 minutes, so a 12-minute budget with 10-second intervals balances + /// responsiveness and tolerance for slow operations. + /// + private static async Task WaitForJobAsync( + ArmClient armClient, string subscription, string resourceGroup, string vaultName, + string jobId, CancellationToken cancellationToken) + { + const int maxAttempts = 72; // 72 * 10s = 12 minutes + var pollDelay = TimeSpan.FromSeconds(10); + + var jobResourceId = BackupJobResource.CreateResourceIdentifier( + subscription, resourceGroup, vaultName, jobId); + var jobResource = armClient.GetBackupJobResource(jobResourceId); + + for (var attempt = 0; attempt < maxAttempts; attempt++) + { + try + { + var jobResponse = await jobResource.GetAsync(cancellationToken); + if (jobResponse.Value.Data.Properties is BackupGenericJob job && + !string.IsNullOrEmpty(job.Status) && + !job.Status.Equals("InProgress", StringComparison.OrdinalIgnoreCase)) + { + return job; + } + } + catch (RequestFailedException ex) when (ex.Status == 404) + { + // Job entry not yet visible; keep polling. + } + + await Task.Delay(pollDelay, cancellationToken); + } + + return null; + } + + private static string? ExtractJobErrorMessage(BackupGenericJob job) + { + switch (job) + { + case IaasVmBackupJob vm when vm.ErrorDetails.Count > 0: + return FirstNonEmpty(vm.ErrorDetails[0].ErrorString, vm.ErrorDetails[0].ErrorTitle); + case IaasVmBackupJobV2 vm2 when vm2.ErrorDetails.Count > 0: + return FirstNonEmpty(vm2.ErrorDetails[0].ErrorString, vm2.ErrorDetails[0].ErrorTitle); + case StorageBackupJob storage when storage.ErrorDetails.Count > 0: + return storage.ErrorDetails[0].ErrorString; + case WorkloadBackupJob wl when wl.ErrorDetails.Count > 0: + return FirstNonEmpty(wl.ErrorDetails[0].ErrorString, wl.ErrorDetails[0].ErrorTitle); + default: + return null; + } + } + + private static string? FirstNonEmpty(string? primary, string? fallback) => + string.IsNullOrEmpty(primary) ? fallback : primary; + public async Task> ListProtectableItemsAsync( string vaultName, string resourceGroup, string subscription, string? workloadType, string? containerName, string? tenant, diff --git a/tools/Azure.Mcp.Tools.AzureBackup/tests/Azure.Mcp.Tools.AzureBackup.LiveTests/AzureBackupCommandTests.cs b/tools/Azure.Mcp.Tools.AzureBackup/tests/Azure.Mcp.Tools.AzureBackup.LiveTests/AzureBackupCommandTests.cs index cf9441f488..0ab42e8fd2 100644 --- a/tools/Azure.Mcp.Tools.AzureBackup/tests/Azure.Mcp.Tools.AzureBackup.LiveTests/AzureBackupCommandTests.cs +++ b/tools/Azure.Mcp.Tools.AzureBackup/tests/Azure.Mcp.Tools.AzureBackup.LiveTests/AzureBackupCommandTests.cs @@ -48,6 +48,20 @@ public class AzureBackupCommandTests(ITestOutputHelper output, TestProxyFixture { Regex = "(?i)azurebackuprg_mcp-test", Value = "Sanitized", + }), + // ARM x-ms-arm-resource-system-data header may include the recording user's UPN + // when fresh resources are created (createdBy / lastModifiedBy fields). + new GeneralRegexSanitizer(new GeneralRegexSanitizerBody() + { + Regex = @"[A-Za-z0-9._%+-]+@microsoft\.com", + Value = "sanitized@example.com", + }), + // x-ms-operation-identifier and certificate URLs in response headers leak the + // tenant id of the recording subscription. Replace with the well-known zero GUID. + new GeneralRegexSanitizer(new GeneralRegexSanitizerBody() + { + Regex = "72f988bf-86f1-41af-91ab-2d7cd011db47", + Value = "00000000-0000-0000-0000-000000000000", }) ]; @@ -162,6 +176,25 @@ public async Task VaultCreate_CreatesDppVault_Successfully() var vault = result.AssertProperty("vault"); Assert.Equal("Succeeded", vault.AssertProperty("provisioningState").GetString()); + + // DPP vault create must enable a System-Assigned Managed Identity by default so + // the vault can authenticate to protected datasources without a separate + // 'vault update --identity-type SystemAssigned' step. + var getResult = await CallToolAsync( + "azurebackup_vault_get", + new() + { + { "subscription", Settings.SubscriptionId }, + { "resource-group", Settings.ResourceGroupName }, + { "vault", vaultName }, + { "vault-type", "dpp" } + }); + // azurebackup_vault_get returns a 'vaults' array; pick the first matching entry. + var vaults = getResult.AssertProperty("vaults"); + Assert.Equal(JsonValueKind.Array, vaults.ValueKind); + var fetchedVault = vaults.EnumerateArray().First(); + var identityType = fetchedVault.AssertProperty("identityType").GetString(); + Assert.Equal("SystemAssigned", identityType, ignoreCase: true); } [Fact] @@ -686,6 +719,81 @@ public async Task ProtectedItemGet_DppVault_ListsProtectedItems_Successfully() Assert.Equal(JsonValueKind.Array, items.ValueKind); } + /// + /// End-to-end Disk protection through DPP vault. + /// Validates the Bug #2 (DPP) fix: protecteditem protect waits for the operation + /// to complete (), reads the backup-instance back, + /// and surfaces a real protectionStatus rather than a fake "Accepted". + /// Also implicitly validates the Bug #1 fix because protection succeeds only when the + /// DPP vault MSI created by vault create has the right RBAC on the disk + RG. + /// + [Fact] + public async Task ProtectedItemProtect_DppVault_DiskProtection_Succeeds_E2E() + { + var vaultName = $"{Settings.ResourceBaseName}-dpp"; + var policyName = $"{Settings.ResourceBaseName}-disk-policy"; + var diskName = $"{Settings.ResourceBaseName}-disk"; + var diskId = $"/subscriptions/{Settings.SubscriptionId}/resourceGroups/{Settings.ResourceGroupName}/providers/Microsoft.Compute/disks/{diskName}"; + + // 1. Create disk-workload backup policy via MCP + var policyResult = await CallToolAsync( + "azurebackup_policy_create", + new() + { + { "subscription", Settings.SubscriptionId }, + { "resource-group", Settings.ResourceGroupName }, + { "vault", vaultName }, + { "vault-type", "dpp" }, + { "policy", policyName }, + { "workload-type", "AzureDisk" } + }); + + var policyOp = policyResult.AssertProperty("result"); + Assert.Equal("Succeeded", policyOp.AssertProperty("status").GetString()); + + // 2. Protect the disk via MCP — exercises the new DPP code path + var protectResult = await CallToolAsync( + "azurebackup_protecteditem_protect", + new() + { + { "subscription", Settings.SubscriptionId }, + { "resource-group", Settings.ResourceGroupName }, + { "vault", vaultName }, + { "vault-type", "dpp" }, + { "datasource-id", diskId }, + { "policy", policyName }, + { "datasource-type", "AzureDisk" } + }); + + var protectOp = protectResult.AssertProperty("result"); + + // The new code returns a real terminal status. Acceptable values: + // "Succeeded" — backend accepted the configuration + // "Failed" — backend rejected; the test infrastructure should make Succeeded the norm, + // but if the backend transiently fails we still want to assert the new + // contract (real errorMessage is present, JobId is null for DPP). + var status = protectOp.AssertProperty("status").GetString(); + Assert.True(status is "Succeeded" or "Failed", $"Unexpected DPP protect status: {status}"); + + // Bug #2 DPP contract: a backup-instance name is always returned, JobId is never set. + protectOp.AssertProperty("protectedItemName"); + Assert.False(protectOp.TryGetProperty("jobId", out var jobId) && jobId.ValueKind != JsonValueKind.Null, + "DPP protect must not return a jobId (DPP is not a job)."); + + if (status == "Succeeded") + { + // Surface the protection status (e.g., "ConfiguringProtection" / "ProtectionConfigured") + protectOp.AssertProperty("protectionStatus"); + } + else + { + // Failed responses must include a non-empty errorMessage from the backend + var errorMessage = protectOp.AssertProperty("errorMessage").GetString(); + Assert.False(string.IsNullOrWhiteSpace(errorMessage), "Failed DPP protect must include errorMessage."); + Output.WriteLine($"DPP disk protect returned Failed: {errorMessage}"); + } + } + #endregion #region Protectable Item Tests diff --git a/tools/Azure.Mcp.Tools.AzureBackup/tests/Azure.Mcp.Tools.AzureBackup.LiveTests/assets.json b/tools/Azure.Mcp.Tools.AzureBackup/tests/Azure.Mcp.Tools.AzureBackup.LiveTests/assets.json index 52ea34338f..981bbc7780 100644 --- a/tools/Azure.Mcp.Tools.AzureBackup/tests/Azure.Mcp.Tools.AzureBackup.LiveTests/assets.json +++ b/tools/Azure.Mcp.Tools.AzureBackup/tests/Azure.Mcp.Tools.AzureBackup.LiveTests/assets.json @@ -2,5 +2,5 @@ "AssetsRepo": "Azure/azure-sdk-assets", "AssetsRepoPrefixPath": "", "TagPrefix": "Azure.Mcp.Tools.AzureBackup.LiveTests", - "Tag": "Azure.Mcp.Tools.AzureBackup.LiveTests_c70ee092ee" + "Tag": "Azure.Mcp.Tools.AzureBackup.LiveTests_7a5bddd1ef" } diff --git a/tools/Azure.Mcp.Tools.AzureBackup/tests/Azure.Mcp.Tools.AzureBackup.UnitTests/ProtectedItem/ProtectedItemProtectCommandTests.cs b/tools/Azure.Mcp.Tools.AzureBackup/tests/Azure.Mcp.Tools.AzureBackup.UnitTests/ProtectedItem/ProtectedItemProtectCommandTests.cs index 82d3f3fe4b..d1d57fce6a 100644 --- a/tools/Azure.Mcp.Tools.AzureBackup/tests/Azure.Mcp.Tools.AzureBackup.UnitTests/ProtectedItem/ProtectedItemProtectCommandTests.cs +++ b/tools/Azure.Mcp.Tools.AzureBackup/tests/Azure.Mcp.Tools.AzureBackup.UnitTests/ProtectedItem/ProtectedItemProtectCommandTests.cs @@ -135,4 +135,152 @@ public void BindOptions_BindsOptionsCorrectly() Assert.Contains(options, o => o.Name == "--container"); Assert.Contains(options, o => o.Name == "--datasource-type"); } + + [Fact] + public async Task ExecuteAsync_DppResult_SurfacesProtectionStatusAndOmitsJobId() + { + // Arrange — DPP protection is not a job; result should expose ProtectionStatus + // (read back from the backup instance) and leave JobId null. + Service.ProtectItemAsync( + Arg.Is("v"), Arg.Is("rg"), Arg.Is("sub"), Arg.Is("/subscriptions/.../disks/d1"), Arg.Is("policy-disk"), + Arg.Any(), Arg.Any(), Arg.Any(), Arg.Any(), Arg.Any(), Arg.Any()) + .Returns(new ProtectResult( + Status: "Succeeded", + ProtectedItemName: "rg-mydisk-abcd1234", + JobId: null, + Message: "Protection configured for backup instance 'rg-mydisk-abcd1234' (status: ProtectionConfigured).", + ProtectionStatus: "ProtectionConfigured", + ErrorMessage: null)); + + // Act + var response = await ExecuteCommandAsync( + "--subscription", "sub", + "--vault", "v", + "--resource-group", "rg", + "--datasource-id", "/subscriptions/.../disks/d1", + "--policy", "policy-disk"); + + // Assert + var result = ValidateAndDeserializeResponse(response, AzureBackupJsonContext.Default.ProtectedItemProtectCommandResult); + Assert.Equal("Succeeded", result.Result.Status); + Assert.Null(result.Result.JobId); + Assert.Equal("ProtectionConfigured", result.Result.ProtectionStatus); + } + + [Fact] + public async Task ExecuteAsync_DppResult_SurfacesFailureWithErrorMessage() + { + // Arrange — when DPP backend rejects (e.g. VaultMSIUnauthorized) the result must + // carry Status="Failed" + ErrorMessage rather than a misleading "Accepted". + Service.ProtectItemAsync( + Arg.Is("v"), Arg.Is("rg"), Arg.Is("sub"), Arg.Is("/subscriptions/.../sa1"), Arg.Is("policy-blob"), + Arg.Any(), Arg.Any(), Arg.Any(), Arg.Any(), Arg.Any(), Arg.Any()) + .Returns(new ProtectResult( + Status: "Failed", + ProtectedItemName: "rg-blob-xyz", + JobId: null, + Message: "Protection failed for backup instance 'rg-blob-xyz': VaultMSIUnauthorized", + ProtectionStatus: null, + ErrorMessage: "VaultMSIUnauthorized: Vault MSI is not authorized.")); + + // Act + var response = await ExecuteCommandAsync( + "--subscription", "sub", + "--vault", "v", + "--resource-group", "rg", + "--datasource-id", "/subscriptions/.../sa1", + "--policy", "policy-blob"); + + // Assert + var result = ValidateAndDeserializeResponse(response, AzureBackupJsonContext.Default.ProtectedItemProtectCommandResult); + Assert.Equal("Failed", result.Result.Status); + Assert.Null(result.Result.JobId); + Assert.Contains("VaultMSIUnauthorized", result.Result.ErrorMessage); + } + + [Fact] + public async Task ExecuteAsync_RsvResult_SurfacesTerminalJobStatus() + { + // Arrange — RSV protection should report the polled ConfigureBackup job's + // terminal status (Completed, CompletedWithWarnings, Failed) along with the job id. + Service.ProtectItemAsync( + Arg.Is("v"), Arg.Is("rg"), Arg.Is("sub"), Arg.Is("/subscriptions/.../vms/myvm"), Arg.Is("policy-vm"), + Arg.Any(), Arg.Any(), Arg.Any(), Arg.Any(), Arg.Any(), Arg.Any()) + .Returns(new ProtectResult( + Status: "Completed", + ProtectedItemName: "vm;iaasvmcontainerv2;rg;myvm", + JobId: "11111111-1111-1111-1111-111111111111", + Message: "VM protection completed. Use 'azurebackup protecteditem get' to verify.")); + + // Act + var response = await ExecuteCommandAsync( + "--subscription", "sub", + "--vault", "v", + "--resource-group", "rg", + "--datasource-id", "/subscriptions/.../vms/myvm", + "--policy", "policy-vm"); + + // Assert + var result = ValidateAndDeserializeResponse(response, AzureBackupJsonContext.Default.ProtectedItemProtectCommandResult); + Assert.Equal("Completed", result.Result.Status); + Assert.Equal("11111111-1111-1111-1111-111111111111", result.Result.JobId); + } + + [Fact] + public async Task ExecuteAsync_RsvResult_SurfacesFailedJobWithErrorMessage() + { + // Arrange — when ConfigureBackup ends in Failed, MCP must surface Status=Failed + // and ErrorMessage from the job rather than the previous "Accepted". + Service.ProtectItemAsync( + Arg.Is("v"), Arg.Is("rg"), Arg.Is("sub"), Arg.Is("/subscriptions/.../sa/fileServices/default/shares/share"), Arg.Is("policy-afs"), + Arg.Any(), Arg.Any(), Arg.Any(), Arg.Any(), Arg.Any(), Arg.Any()) + .Returns(new ProtectResult( + Status: "Failed", + ProtectedItemName: "afsfileshare;sa;share", + JobId: "22222222-2222-2222-2222-222222222222", + Message: "File share protection failed: Item not found. See 'azurebackup job get --job 22222222-...' for details.", + ProtectionStatus: null, + ErrorMessage: "Item not found")); + + // Act + var response = await ExecuteCommandAsync( + "--subscription", "sub", + "--vault", "v", + "--resource-group", "rg", + "--datasource-id", "/subscriptions/.../sa/fileServices/default/shares/share", + "--policy", "policy-afs"); + + // Assert + var result = ValidateAndDeserializeResponse(response, AzureBackupJsonContext.Default.ProtectedItemProtectCommandResult); + Assert.Equal("Failed", result.Result.Status); + Assert.Equal("Item not found", result.Result.ErrorMessage); + } + + [Fact] + public async Task ExecuteAsync_RsvResult_SurfacesInProgressWhenPollingBudgetExpires() + { + // Arrange — long-running ConfigureBackup must not cause the tool to fail; it + // should return InProgress with the job id so the caller can keep monitoring. + Service.ProtectItemAsync( + Arg.Is("v"), Arg.Is("rg"), Arg.Is("sub"), Arg.Is("/subscriptions/.../vms/slowvm"), Arg.Is("policy-vm"), + Arg.Any(), Arg.Any(), Arg.Any(), Arg.Any(), Arg.Any(), Arg.Any()) + .Returns(new ProtectResult( + Status: "InProgress", + ProtectedItemName: "vm;iaasvmcontainerv2;rg;slowvm", + JobId: "33333333-3333-3333-3333-333333333333", + Message: "VM protection is still running after the polling budget elapsed. Use 'azurebackup job get --job 33333333-...' to continue monitoring.")); + + // Act + var response = await ExecuteCommandAsync( + "--subscription", "sub", + "--vault", "v", + "--resource-group", "rg", + "--datasource-id", "/subscriptions/.../vms/slowvm", + "--policy", "policy-vm"); + + // Assert + var result = ValidateAndDeserializeResponse(response, AzureBackupJsonContext.Default.ProtectedItemProtectCommandResult); + Assert.Equal("InProgress", result.Result.Status); + Assert.Equal("33333333-3333-3333-3333-333333333333", result.Result.JobId); + } } diff --git a/tools/Azure.Mcp.Tools.AzureBackup/tests/test-resources.bicep b/tools/Azure.Mcp.Tools.AzureBackup/tests/test-resources.bicep index da80c38cb9..8e6cefaea4 100644 --- a/tools/Azure.Mcp.Tools.AzureBackup/tests/test-resources.bicep +++ b/tools/Azure.Mcp.Tools.AzureBackup/tests/test-resources.bicep @@ -133,6 +133,7 @@ resource dppDiskBackupReaderRoleAssignment 'Microsoft.Authorization/roleAssignme // Output the resource IDs for the post-deployment script output diskId string = testDisk.id +output diskName string = testDisk.name // ─── RSV Undelete Test Resources (Storage Account + File Share) ───