feat(azurebackup): Add security configure-encryption command for CMK#2609
Open
shrja-ms wants to merge 1 commit intomicrosoft:mainfrom
Open
feat(azurebackup): Add security configure-encryption command for CMK#2609shrja-ms wants to merge 1 commit intomicrosoft:mainfrom
shrja-ms wants to merge 1 commit intomicrosoft:mainfrom
Conversation
shrja-ms
requested review from
JonathanCrd,
jongio,
joshfree,
saikoumudi and
xiangyan99
Contributor
There was a problem hiding this comment.
Pull request overview
Adds a new Azure Backup security command to configure Customer-Managed Key (CMK) encryption for both Recovery Services vaults (RSV) and Backup vaults (DPP), wiring it through the AzureBackup service layer, registering it in DI/command groups, and updating docs/tests for coverage.
Changes:
- Introduces
azurebackup security configure-encryptioncommand + options binding/validation and AOT JSON source-gen registration. - Adds CMK encryption implementation paths for RSV and DPP vaults, plus service-layer routing with vault-type auto-detection.
- Expands unit tests, live tests, command docs, e2e prompts, and server changelog entry.
Reviewed changes
Copilot reviewed 16 out of 16 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| tools/Azure.Mcp.Tools.AzureBackup/src/Commands/Security/SecurityConfigureEncryptionCommand.cs | New command implementation, option registration/validation, error/status mapping, telemetry tagging. |
| tools/Azure.Mcp.Tools.AzureBackup/src/Options/Security/SecurityConfigureEncryptionOptions.cs | New options model for CMK parameters (KV URI/name/version, identity settings). |
| tools/Azure.Mcp.Tools.AzureBackup/src/Options/AzureBackupOptionDefinitions.cs | Adds new CMK-related CLI options (key vault URI/name/version, user-assigned identity id). |
| tools/Azure.Mcp.Tools.AzureBackup/src/Services/IAzureBackupService.cs | Adds ConfigureEncryptionAsync API surface. |
| tools/Azure.Mcp.Tools.AzureBackup/src/Services/AzureBackupService.cs | Routes configure-encryption to RSV vs DPP based on resolved vault type. |
| tools/Azure.Mcp.Tools.AzureBackup/src/Services/IRsvBackupOperations.cs | Adds RSV operations contract for CMK configuration. |
| tools/Azure.Mcp.Tools.AzureBackup/src/Services/RsvBackupOperations.cs | Implements RSV CMK patch operation using vault encryption properties. |
| tools/Azure.Mcp.Tools.AzureBackup/src/Services/IDppBackupOperations.cs | Adds DPP operations contract for CMK configuration. |
| tools/Azure.Mcp.Tools.AzureBackup/src/Services/DppBackupOperations.cs | Implements DPP CMK patch operation using backup vault security/encryption settings. |
| tools/Azure.Mcp.Tools.AzureBackup/src/Commands/AzureBackupJsonContext.cs | Registers new command result for System.Text.Json source generation (AOT). |
| tools/Azure.Mcp.Tools.AzureBackup/src/AzureBackupSetup.cs | Registers the new command in DI and adds it under the security command group. |
| tools/Azure.Mcp.Tools.AzureBackup/tests/Azure.Mcp.Tools.AzureBackup.UnitTests/Security/SecurityConfigureEncryptionCommandTests.cs | Adds unit tests for option validation and response/error behaviors. |
| tools/Azure.Mcp.Tools.AzureBackup/tests/Azure.Mcp.Tools.AzureBackup.LiveTests/AzureBackupCommandTests.cs | Adds LiveTestOnly end-to-end tests for RSV/DPP system-assigned identity paths. |
| servers/Azure.Mcp.Server/docs/azmcp-commands.md | Documents the new command and its arguments. |
| servers/Azure.Mcp.Server/docs/e2eTestPrompts.md | Adds prompts for tool-description evaluation/e2e prompt coverage. |
| servers/Azure.Mcp.Server/changelog-entries/1778209045130.yaml | Adds changelog entry under “Features Added”. |
Comment on lines
+1104
to
+1126
| var normalizedIdentity = identityType.ToUpperInvariant(); | ||
|
|
||
| // Build the full key URI: {keyVaultUri}/keys/{keyName}[/{keyVersion}] | ||
| var kvUri = keyVaultUri.TrimEnd('/'); | ||
| var keyUriString = string.IsNullOrEmpty(keyVersion) | ||
| ? $"{kvUri}/keys/{keyName}" | ||
| : $"{kvUri}/keys/{keyName}/{keyVersion}"; | ||
|
|
||
| var armClient = await CreateArmClientAsync(tenant, retryPolicy, cancellationToken: cancellationToken); | ||
| var vaultId = RecoveryServicesVaultResource.CreateResourceIdentifier(subscription, resourceGroup, vaultName); | ||
| var vaultResource = armClient.GetRecoveryServicesVaultResource(vaultId); | ||
| var vault = await vaultResource.GetAsync(cancellationToken); | ||
|
|
||
| var kekIdentity = new CmkKekIdentity(); | ||
| if (normalizedIdentity == "SYSTEMASSIGNED") | ||
| { | ||
| kekIdentity.UseSystemAssignedIdentity = true; | ||
| } | ||
| else | ||
| { | ||
| kekIdentity.UseSystemAssignedIdentity = false; | ||
| kekIdentity.UserAssignedIdentity = new ResourceIdentifier(userAssignedIdentityId!); | ||
| } |
Comment on lines
+906
to
+924
| var normalizedIdentity = identityType.ToUpperInvariant(); | ||
|
|
||
| // Build the full key URI: {keyVaultUri}/keys/{keyName}[/{keyVersion}] | ||
| var kvUri = keyVaultUri.TrimEnd('/'); | ||
| var keyUriString = string.IsNullOrEmpty(keyVersion) | ||
| ? $"{kvUri}/keys/{keyName}" | ||
| : $"{kvUri}/keys/{keyName}/{keyVersion}"; | ||
|
|
||
| var armClient = await CreateArmClientAsync(tenant, retryPolicy, cancellationToken: cancellationToken); | ||
| var vaultId = DataProtectionBackupVaultResource.CreateResourceIdentifier(subscription, resourceGroup, vaultName); | ||
| var vaultResource = armClient.GetDataProtectionBackupVaultResource(vaultId); | ||
|
|
||
| var kekIdentity = new BackupVaultCmkKekIdentity | ||
| { | ||
| IdentityType = normalizedIdentity == "SYSTEMASSIGNED" | ||
| ? BackupVaultCmkKekIdentityType.SystemAssigned | ||
| : BackupVaultCmkKekIdentityType.UserAssigned, | ||
| IdentityId = normalizedIdentity == "USERASSIGNED" ? userAssignedIdentityId : null | ||
| }; |
Comment on lines
+73
to
+79
| !Uri.TryCreate(keyVaultUri, UriKind.Absolute, out var uri)) | ||
| { | ||
| commandResult.AddError("--key-vault-uri must be a valid URI (e.g., 'https://kv-name.vault.azure.net/')."); | ||
| } | ||
| }); | ||
| } | ||
|
|
Adds azurebackup security configure-encryption command that enables Customer-Managed Key (CMK) encryption on both Recovery Services vaults (RSV) and Backup vaults (DPP) using a key from Azure Key Vault. - Supports SystemAssigned and UserAssigned managed identity types - Validates Key Vault URI, key name, and identity parameters - ARM ID validation on user-assigned identity - Supports both RSV and DPP vault types with auto-detection - Uses [CommandMetadata] attribute pattern - Does not hardcode InfrastructureEncryption (left unchanged) - Includes 23 unit tests and 1 live test
shrja-ms
force-pushed
the
user/azurebackup-cmk-encryption
branch
from
3849e33 to
570e56d
Compare
jongio
reviewed
May 8, 2026
Contributor
jongio
left a comment
jongio
left a comment
There was a problem hiding this comment.
RSV's ConfigureEncryptionAsync doesn't validate identityType at the service level, unlike DPP's version which validates at lines 907-917. The command-layer validator catches invalid inputs today, but these service methods are interface entry points - adding matching validation keeps them defensively consistent. One inline comment.
CI failures (4 build jobs) are infrastructure - 1ES Pipeline Templates credential provider 401s to the NuGet feed, not caused by this PR.
| (nameof(identityType), identityType)); | ||
| ValidateSubscriptionFormat(subscription); | ||
|
|
||
| var normalizedIdentity = identityType.ToUpperInvariant(); |
Contributor
There was a problem hiding this comment.
DppBackupOperations.ConfigureEncryptionAsync validates identity type and userAssignedIdentityId at the service level (lines 907-917 in that file). This RSV version skips that validation and falls into the else branch for any non-"SYSTEMASSIGNED" value, hitting userAssignedIdentityId! without a null check.
The command validator handles this today, but since both methods sit behind service interfaces, they should validate their own inputs. Consider adding the same guard after normalizedIdentity:
if (normalizedIdentity != "SYSTEMASSIGNED" && normalizedIdentity != "USERASSIGNED")
{
throw new ArgumentException(
$"Invalid identity type '{identityType}' for CMK encryption. Supported values: 'SystemAssigned', 'UserAssigned'.");
}
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.
Summary
Adds
azurebackup security configure-encryptioncommand that configures Customer-Managed Key (CMK) encryption on both Recovery Services vaults (RSV) and Backup vaults (DPP) using a key from Azure Key Vault.Changes
New Command:
azurebackup security configure-encryptionUsage:
Features:
--key-versionto pin a specific key version (omit for latest/autorotation)--user-assigned-identity-idrequired when--identity-typeisUserAssigned--user-assigned-identity-id--key-vault-uri[CommandMetadata]attribute patternAzureBackupTelemetryTags.AddVaultTagsValidateSubscriptionFormatin RSV path (consistent with other RSV operations)RSV Implementation: Uses
VaultPropertiesEncryptionwithCmkKekIdentityonRecoveryServicesVaultPatch. Note: For RSV, CMK can only be enabled on new vaults with no registered items (enforced by Azure API, documented in 400 error message).DPP Implementation: Uses
BackupVaultEncryptionSettingsviaBackupVaultSecuritySettingsonDataProtectionBackupVaultPatch. DPP supports CMK before or after protecting items.Files Changed (16 files, ~851 lines)
SecurityConfigureEncryptionCommand.cs(new),SecurityConfigureEncryptionOptions.cs(new)AzureBackupOptionDefinitions.cs(+4 const names, +4 Option fields: key-vault-uri, key-name, key-version, user-assigned-identity-id)IAzureBackupService.cs,AzureBackupService.cs(routing),IRsvBackupOperations.cs,RsvBackupOperations.cs,IDppBackupOperations.cs,DppBackupOperations.csAzureBackupSetup.cs(DI + command group),AzureBackupJsonContext.cs(AOT)SecurityConfigureEncryptionCommandTests.cs- 23 testsAzureBackupCommandTests.cs- 2[LiveTestOnly]tests (RSV + DPP SystemAssigned)azmcp-commands.md,e2eTestPrompts.md(3 prompts), changelog YAMLValidation
dotnet builddotnet format --verify-no-changesUpdate-AzCommandsMetadata.ps1servers/Azure.Mcp.Server/changelog-entries/1778209045130.yamlDesign Decisions
RegisterOptionsvalidators, not duplicated in service layer.BackupVaultEncryptionSettingson the vault patchSecuritySettings(per Azure docs).Invoking Livetests
Copilot submitted PRs are not trustworthy by default. Users with
writeaccess to the repo need to validate the contents of this PR before leaving a comment with the text/azp run mcp - pullrequest - live. This will trigger the necessary livetest workflows to complete required validation.