diff --git a/tools/Azure.Mcp.Tools.AzureBackup/skills/azurebackup-telemetry-report/SKILL.md b/tools/Azure.Mcp.Tools.AzureBackup/skills/azurebackup-telemetry-report/SKILL.md index 9ba6361988..dcbfadb0a4 100644 --- a/tools/Azure.Mcp.Tools.AzureBackup/skills/azurebackup-telemetry-report/SKILL.md +++ b/tools/Azure.Mcp.Tools.AzureBackup/skills/azurebackup-telemetry-report/SKILL.md @@ -1,7 +1,7 @@ --- name: azurebackup-telemetry-report description: 'Generate weekly telemetry reports and customer adoption reports for Azure Backup MCP tools. Runs KQL queries against the Kusto telemetry cluster, analyzes error patterns with 3-way classification (Customer/Azure Service/MCP Tool Bug), identifies customers via P360/C360 cross-cluster joins, compares week-over-week metrics, correlates with merged PRs and releases, and produces an Outlook-compatible HTML report. USE WHEN: weekly telemetry report, Azure Backup MCP telemetry, error analysis, telemetry bugs, weekly report, MCP tool success rate, backup telemetry, error classification, customer usage report, who is using Azure Backup MCP, backup adoption, customer report.' -argument-hint: 'Generate the Azure Backup MCP weekly telemetry report' +argument-hint: 'Generate an Azure Backup MCP telemetry or customer adoption report' --- # Azure Backup MCP — Weekly Telemetry & Customer Report Generator @@ -157,7 +157,7 @@ For the Outlook version, apply these rules: > **Customer Report Queries:** For customer identification, tool adoption, client distribution, > and version analysis, refer to [`kql-customer-queries.md`](https://github.com/microsoft/mcp/blob/main/tools/Azure.Mcp.Tools.AzureBackup/skills/azurebackup-telemetry-report/references/kql-customer-queries.md) -> (queries 14–24). These queries use cross-cluster joins to `mabprod1` (P360) and `icmdataro` (C360) +> (queries 14–24). These queries use cross-cluster joins to `mabprod1` (P360) and `icmdataro.centralus` (C360) > for customer name resolution and external/internal classification. > > **Key rule:** Always use `P360_CustomerName` (not `CustomerName`) from the P360 table. diff --git a/tools/Azure.Mcp.Tools.AzureBackup/skills/azurebackup-telemetry-report/references/kql-customer-queries.md b/tools/Azure.Mcp.Tools.AzureBackup/skills/azurebackup-telemetry-report/references/kql-customer-queries.md index 1b5cc17ba8..8324e750e7 100644 --- a/tools/Azure.Mcp.Tools.AzureBackup/skills/azurebackup-telemetry-report/references/kql-customer-queries.md +++ b/tools/Azure.Mcp.Tools.AzureBackup/skills/azurebackup-telemetry-report/references/kql-customer-queries.md @@ -8,6 +8,11 @@ All queries use the `getAzureMcpEvents_ToolCalls` function from the Azure MCP te Replace `{DAYS}` with the desired time range (e.g., `15` for 15 days). +> **Note on field casing:** Lowercase bag access (e.g., `customDimensions.toolname`) works because +> `getAzureMcpEvents_ToolCalls` normalizes keys. If queries return empty results, try PascalCase +> variants (`customDimensions.ClientName`, `customDimensions.Version`). See the full note in +> [`kql-queries.md`](https://github.com/microsoft/mcp/blob/main/tools/Azure.Mcp.Tools.AzureBackup/skills/azurebackup-telemetry-report/references/kql-queries.md). + > **Critical:** Always use `P360_CustomerName` instead of `CustomerName` in P360. The `CustomerName` > field contains generic tenant names (e.g., "Denis" for all Prometeia SpA subs). `P360_CustomerName` > has the actual company name. diff --git a/tools/Azure.Mcp.Tools.AzureBackup/src/Commands/Backup/BackupStatusCommand.cs b/tools/Azure.Mcp.Tools.AzureBackup/src/Commands/Backup/BackupStatusCommand.cs index 914fceac2b..0905bdd3a6 100644 --- a/tools/Azure.Mcp.Tools.AzureBackup/src/Commands/Backup/BackupStatusCommand.cs +++ b/tools/Azure.Mcp.Tools.AzureBackup/src/Commands/Backup/BackupStatusCommand.cs @@ -92,7 +92,10 @@ public override async Task ExecuteAsync(CommandContext context, RequestFailedException reqEx when reqEx.Status == (int)HttpStatusCode.NotFound => "Resource not found. Verify the datasource ARM resource ID.", RequestFailedException reqEx when reqEx.Status == (int)HttpStatusCode.Forbidden => - $"Authorization failed checking backup status. Details: {reqEx.Message}", + "Authorization failed checking backup status. " + + "This tool requires the 'Microsoft.RecoveryServices/locations/backupStatus/action' permission " + + "(included in 'Backup Reader' role at subscription scope). " + + "Alternatively, use 'vault_get' + 'protecteditem_get' to check protection status.", RequestFailedException reqEx => reqEx.Message, _ => base.GetErrorMessage(ex) }; diff --git a/tools/Azure.Mcp.Tools.AzureBackup/src/Commands/Governance/GovernanceFindUnprotectedCommand.cs b/tools/Azure.Mcp.Tools.AzureBackup/src/Commands/Governance/GovernanceFindUnprotectedCommand.cs index e0531913b4..335dba5f18 100644 --- a/tools/Azure.Mcp.Tools.AzureBackup/src/Commands/Governance/GovernanceFindUnprotectedCommand.cs +++ b/tools/Azure.Mcp.Tools.AzureBackup/src/Commands/Governance/GovernanceFindUnprotectedCommand.cs @@ -1,6 +1,7 @@ // Copyright (c) Microsoft Corporation. // Licensed under the MIT License. +using System.Net; using Azure.Mcp.Core.Commands.Subscription; using Azure.Mcp.Tools.AzureBackup.Models; using Azure.Mcp.Tools.AzureBackup.Options; @@ -86,5 +87,14 @@ public override async Task ExecuteAsync(CommandContext context, return context.Response; } + protected override string GetErrorMessage(Exception ex) => ex switch + { + ArgumentException argEx => argEx.Message, + RequestFailedException reqEx when reqEx.Status == (int)HttpStatusCode.Forbidden => + "Authorization failed scanning for unprotected resources. Ensure you have 'Reader' role at subscription scope.", + RequestFailedException reqEx => reqEx.Message, + _ => base.GetErrorMessage(ex) + }; + internal record GovernanceFindUnprotectedCommandResult(List Resources); } diff --git a/tools/Azure.Mcp.Tools.AzureBackup/src/Commands/ProtectedItem/ProtectedItemProtectCommand.cs b/tools/Azure.Mcp.Tools.AzureBackup/src/Commands/ProtectedItem/ProtectedItemProtectCommand.cs index b36d5a2ec9..7447116bff 100644 --- a/tools/Azure.Mcp.Tools.AzureBackup/src/Commands/ProtectedItem/ProtectedItemProtectCommand.cs +++ b/tools/Azure.Mcp.Tools.AzureBackup/src/Commands/ProtectedItem/ProtectedItemProtectCommand.cs @@ -1,6 +1,7 @@ // Copyright (c) Microsoft Corporation. // Licensed under the MIT License. +using System.CommandLine.Parsing; using System.Net; using Azure.Mcp.Tools.AzureBackup.Models; using Azure.Mcp.Tools.AzureBackup.Options; @@ -51,6 +52,31 @@ protected override void RegisterOptions(Command command) command.Options.Add(AzureBackupOptionDefinitions.AksExcludedNamespaces); command.Options.Add(AzureBackupOptionDefinitions.AksLabelSelectors); command.Options.Add(AzureBackupOptionDefinitions.AksIncludeClusterScopeResources); + command.Validators.Add(commandResult => + { + OptionResult? optionResult = commandResult.GetResult(AzureBackupOptionDefinitions.DatasourceType); + if (optionResult is null || optionResult.Implicit) + { + return; + } + + var value = optionResult.Tokens.LastOrDefault()?.Value ?? string.Empty; + var normalizedValue = value.Trim(); + + if (string.IsNullOrEmpty(normalizedValue) || + RsvDatasourceRegistry.Resolve(normalizedValue) is null && + DppDatasourceRegistry.TryAutoDetect(normalizedValue) is null && + !DppDatasourceRegistry.AllProfiles.Any(p => + p.FriendlyName.Equals(normalizedValue, StringComparison.OrdinalIgnoreCase) || + p.ArmResourceType.Equals(normalizedValue, StringComparison.OrdinalIgnoreCase) || + p.Aliases.Any(a => a.Equals(normalizedValue, StringComparison.OrdinalIgnoreCase)))) + { + commandResult.AddError( + $"Unknown datasource type '{value}'. " + + $"RSV types: {string.Join(", ", RsvDatasourceRegistry.KnownTypeNames)}. " + + $"DPP types: {string.Join(", ", DppDatasourceRegistry.KnownTypeNames)}."); + } + }); } protected override ProtectedItemProtectOptions BindOptions(ParseResult parseResult) diff --git a/tools/Azure.Mcp.Tools.AzureBackup/src/Commands/Vault/VaultGetCommand.cs b/tools/Azure.Mcp.Tools.AzureBackup/src/Commands/Vault/VaultGetCommand.cs index af504f99db..70e71fd8a5 100644 --- a/tools/Azure.Mcp.Tools.AzureBackup/src/Commands/Vault/VaultGetCommand.cs +++ b/tools/Azure.Mcp.Tools.AzureBackup/src/Commands/Vault/VaultGetCommand.cs @@ -136,6 +136,8 @@ public override async Task ExecuteAsync(CommandContext context, KeyNotFoundException => "Vault not found. Verify the vault name, resource group, and that you have access.", RequestFailedException reqEx when reqEx.Status == (int)HttpStatusCode.NotFound => "Vault not found. Verify the vault name and resource group.", + RequestFailedException reqEx when reqEx.Status == (int)HttpStatusCode.Forbidden => + "Authorization failed accessing vault information. Ensure you have 'Reader' or 'Backup Reader' role at subscription scope.", RequestFailedException reqEx => reqEx.Message, _ => base.GetErrorMessage(ex) }; diff --git a/tools/Azure.Mcp.Tools.AzureBackup/src/Services/DppBackupOperations.cs b/tools/Azure.Mcp.Tools.AzureBackup/src/Services/DppBackupOperations.cs index 5ebdaa940b..735369d0d7 100644 --- a/tools/Azure.Mcp.Tools.AzureBackup/src/Services/DppBackupOperations.cs +++ b/tools/Azure.Mcp.Tools.AzureBackup/src/Services/DppBackupOperations.cs @@ -435,8 +435,9 @@ public async Task GetPolicyAsync( // policies and matching by name to work around this SDK limitation. var policies = await ListPoliciesAsync(vaultName, resourceGroup, subscription, tenant, retryPolicy, cancellationToken); return policies.FirstOrDefault(p => p.Name == policyName) - ?? throw new InvalidOperationException( - $"Policy '{policyName}' not found or cannot be parsed by the Azure SDK due to an unsupported retention/duration field."); + ?? throw new KeyNotFoundException( + $"Policy '{policyName}' not found in vault '{vaultName}'. " + + $"If the policy exists, it may contain a retention/duration format not yet supported by the Azure SDK."); } } diff --git a/tools/Azure.Mcp.Tools.AzureBackup/src/Services/Policy/PolicyCreateValidator.cs b/tools/Azure.Mcp.Tools.AzureBackup/src/Services/Policy/PolicyCreateValidator.cs index 83d7bd9408..c003e47e7d 100644 --- a/tools/Azure.Mcp.Tools.AzureBackup/src/Services/Policy/PolicyCreateValidator.cs +++ b/tools/Azure.Mcp.Tools.AzureBackup/src/Services/Policy/PolicyCreateValidator.cs @@ -29,6 +29,18 @@ public static PolicyValidationResult Validate(PolicyCreateOptions options) var workload = (options.WorkloadType ?? string.Empty).Trim(); var family = ClassifyWorkload(workload); + // Reject unknown workload types at the command boundary with an actionable message + // rather than letting invalid values reach the service layer. + if (family == WorkloadFamily.Unknown && !string.IsNullOrWhiteSpace(workload)) + { + issues.Add(new PolicyValidationIssue( + $"--{AzureBackupOptionDefinitions.WorkloadTypeName}", + $"Unknown workload type '{workload}'. Supported types: " + + "VM, SQL, SAPHANA, SAPASE, AzureFileShare, AzureDisk, AzureBlob, AKS, " + + "ElasticSAN, PostgreSQLFlexible, ADLS, CosmosDB.")); + return PolicyValidationResult.Fail(issues); + } + // Rule D: CosmosDB pass-through - no special validator action; fall through to common rules. // (AKS gate removed in Stage 2 - AKS now flows through normal DPP discrete validation.) diff --git a/tools/Azure.Mcp.Tools.AzureBackup/tests/Azure.Mcp.Tools.AzureBackup.Tests/ProtectedItem/ProtectedItemProtectCommandTests.cs b/tools/Azure.Mcp.Tools.AzureBackup/tests/Azure.Mcp.Tools.AzureBackup.Tests/ProtectedItem/ProtectedItemProtectCommandTests.cs index 50aeccff2b..5dd37e8063 100644 --- a/tools/Azure.Mcp.Tools.AzureBackup/tests/Azure.Mcp.Tools.AzureBackup.Tests/ProtectedItem/ProtectedItemProtectCommandTests.cs +++ b/tools/Azure.Mcp.Tools.AzureBackup/tests/Azure.Mcp.Tools.AzureBackup.Tests/ProtectedItem/ProtectedItemProtectCommandTests.cs @@ -283,4 +283,60 @@ public async Task ExecuteAsync_RsvResult_SurfacesInProgressWhenPollingBudgetExpi Assert.Equal("InProgress", result.Result.Status); Assert.Equal("33333333-3333-3333-3333-333333333333", result.Result.JobId); } + + [Theory] + [InlineData("garbage")] + [InlineData("'); DROP TABLE--")] + [InlineData(" ")] + [InlineData("\t")] + public async Task ExecuteAsync_RejectsUnknownDatasourceType_AsValidationError(string datasourceType) + { + // Act + var response = await ExecuteCommandAsync( + "--subscription", "sub", + "--vault", "v", + "--resource-group", "rg", + "--datasource-id", "/subscriptions/.../vm1", + "--policy", "DefaultPolicy", + "--datasource-type", datasourceType); + + // Assert: validation error (400), service never called + Assert.Equal(HttpStatusCode.BadRequest, response.Status); + Assert.Contains("Unknown datasource type", response.Message); + + await Service.DidNotReceive().ProtectItemAsync( + Arg.Any(), Arg.Any(), Arg.Any(), Arg.Any(), Arg.Any(), + Arg.Any(), Arg.Any(), Arg.Any(), Arg.Any(), Arg.Any(), Arg.Any(), Arg.Any(), Arg.Any(), Arg.Any(), Arg.Any(), Arg.Any()); + } + + [Theory] + [InlineData("vm")] + [InlineData("VM")] + [InlineData("sql")] + [InlineData("AzureFileShare")] + [InlineData("AzureDisk")] + [InlineData("aks")] + [InlineData("blob")] + [InlineData("Microsoft.Compute/disks")] + [InlineData("Microsoft.Storage/storageAccounts")] + public async Task ExecuteAsync_AcceptsValidDatasourceType(string datasourceType) + { + // Arrange + Service.ProtectItemAsync( + Arg.Any(), Arg.Any(), Arg.Any(), Arg.Any(), Arg.Any(), + Arg.Any(), Arg.Any(), Arg.Any(), Arg.Any(), Arg.Any(), Arg.Any(), Arg.Any(), Arg.Any(), Arg.Any(), Arg.Any(), Arg.Any()) + .Returns(new ProtectResult("Succeeded", "item1", "job1", null)); + + // Act + var response = await ExecuteCommandAsync( + "--subscription", "sub", + "--vault", "v", + "--resource-group", "rg", + "--datasource-id", "/subscriptions/.../vm1", + "--policy", "DefaultPolicy", + "--datasource-type", datasourceType); + + // Assert: accepted, service was called + Assert.Equal(HttpStatusCode.OK, response.Status); + } } diff --git a/tools/Azure.Mcp.Tools.AzureBackup/tests/Azure.Mcp.Tools.AzureBackup.Tests/Services/Policy/PolicyCreateValidatorTests.cs b/tools/Azure.Mcp.Tools.AzureBackup/tests/Azure.Mcp.Tools.AzureBackup.Tests/Services/Policy/PolicyCreateValidatorTests.cs index 5ae210a35d..d3bf4949bd 100644 --- a/tools/Azure.Mcp.Tools.AzureBackup/tests/Azure.Mcp.Tools.AzureBackup.Tests/Services/Policy/PolicyCreateValidatorTests.cs +++ b/tools/Azure.Mcp.Tools.AzureBackup/tests/Azure.Mcp.Tools.AzureBackup.Tests/Services/Policy/PolicyCreateValidatorTests.cs @@ -59,6 +59,27 @@ public void Validate_NoScheduleOrRetention_Fails() Assert.Contains(result.Issues, i => i.Message.StartsWith("Provide at least one schedule")); } + // ----- Unknown workload-type rejection (command-boundary validation) ----- + + [Theory] + [InlineData("garbage")] + [InlineData("s3bucket")] + [InlineData("'); DROP TABLE--")] + public void Validate_UnknownWorkloadType_RejectsWithActionableMessage(string workload) + { + var options = BaseOptions(workload); + options.DailyRetentionDays = "30"; + + var result = PolicyCreateValidator.Validate(options); + + Assert.False(result.IsValid); + Assert.Single(result.Issues); + Assert.Contains("Unknown workload type", result.Issues[0].Message); + Assert.Contains(workload, result.Issues[0].Message); + Assert.Contains("VM", result.Issues[0].Message); + Assert.Contains("AzureDisk", result.Issues[0].Message); + } + [Theory] [InlineData("VM")] [InlineData("SQL")]