Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Compute] Add Managed Identity Support in Azure Disk Encryption for VMSS #30657

Merged
merged 10 commits into from
Jan 26, 2025
Merged
Show file tree
Hide file tree
Changes from 5 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions src/azure-cli/azure/cli/command_modules/vm/_help.py
Original file line number Diff line number Diff line change
Expand Up @@ -2643,6 +2643,9 @@
- name: Create a Debian11 VM scaleset with a user assigned identity.
text: >
az vmss create -n MyVmss -g rg1 --image Debian11 --assign-identity /subscriptions/99999999-1bf0-4dda-aec3-cb9272f09590/resourcegroups/myRG/providers/Microsoft.ManagedIdentity/userAssignedIdentities/myID
- name: Create a vmss with user assigned identity and add encryption identity for Azure disk encryption
text: >
az vmss create -n MyVm -g rg1 --image Debian11 --assign-identity myID --encryption-identity /subscriptions/00000000-0000-0000-0000-000000000000/resourcegroups/myRG/providers/Microsoft.ManagedIdentity/userAssignedIdentities/myID --orchestration-mode Uniform --lb-sku Standard
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

May I ask what is the difference between the --assign-identity and --encryption-identity?

Copy link
Contributor Author

@anshuljain26 anshuljain26 Jan 21, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

--assign-identity params is used to assign system or user assigned identities associated with the Virtual Machine Scale set. There can be multiple user assigned identities associated with the virtual machine scale set.

--encryption-identity params is used to set which Identity used by ADE to get access token for keyvault operations.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Got it, thanks~

- name: Create a Debian11 VM scaleset with both system and user assigned identity.
text: >
az vmss create -n MyVmss -g rg1 --image Debian11 --assign-identity [system] /subscriptions/99999999-1bf0-4dda-aec3-cb9272f09590/resourcegroups/myRG/providers/Microsoft.ManagedIdentity/userAssignedIdentities/myID
Expand Down Expand Up @@ -2760,6 +2763,9 @@
- name: encrypt a VM scale set using a key vault in the same resource group
text: >
az vmss encryption enable -g MyResourceGroup -n MyVmss --disk-encryption-keyvault MyVault
- name: Add support for using managed identity to authenticate to customer's keyvault for ADE operation
text: >
az vmss encryption enable --disk-encryption-keyvault MyVault --name MyVm --resource-group MyResourceGroup --encryption-identity EncryptionIdentity
- name: Encrypt a VMSS with managed disks. (autogenerated)
text: |
az vmss encryption enable --disk-encryption-keyvault MyVault --name MyVmss --resource-group MyResourceGroup --volume-type DATA
Expand Down
10 changes: 7 additions & 3 deletions src/azure-cli/azure/cli/command_modules/vm/_params.py
Original file line number Diff line number Diff line change
Expand Up @@ -473,7 +473,6 @@ def load_arguments(self, _):
c.argument('enable_vtpm', enable_vtpm_type)
c.argument('user_data', help='UserData for the VM. It can be passed in as file or string.', completer=FilesCompleter(), type=file_type, min_api='2021-03-01')
c.argument('enable_hibernation', arg_type=get_three_state_flag(), min_api='2021-03-01', help='The flag that enable or disable hibernation capability on the VM.')
c.argument('encryption_identity', help='Resource Id of the user managed identity which can be used for Azure disk encryption')

with self.argument_context('vm create', arg_group='Storage') as c:
c.argument('attach_os_disk', help='Attach an existing OS disk to the VM. Can use the name or ID of a managed disk or the URI to an unmanaged disk VHD.')
Expand Down Expand Up @@ -1191,8 +1190,13 @@ def load_arguments(self, _):
c.argument('key_encryption_key', help='Key vault key name or URL used to encrypt the disk encryption key.')
c.argument('key_encryption_keyvault', help='Name or ID of the key vault containing the key encryption key used to encrypt the disk encryption key. If missing, CLI will use `--disk-encryption-keyvault`.')

with self.argument_context('vm encryption enable') as c:
c.argument('encryption_identity', help='Resource Id of the user managed identity which can be used for Azure disk encryption')
for scope in ['vm create', 'vmss create']:
with self.argument_context(scope) as c:
c.argument('encryption_identity', help='Resource Id of the user managed identity which can be used for Azure disk encryption')
zhoxing-ms marked this conversation as resolved.
Show resolved Hide resolved

for scope in ['vm encryption enable', 'vmss encryption enable']:
with self.argument_context(scope) as c:
c.argument('encryption_identity', help='Resource Id of the user managed identity which can be used for Azure disk encryption')
yanzhudd marked this conversation as resolved.
Show resolved Hide resolved

for scope in ['vm extension', 'vmss extension']:
with self.argument_context(scope) as c:
Expand Down
31 changes: 30 additions & 1 deletion src/azure-cli/azure/cli/command_modules/vm/custom.py
Original file line number Diff line number Diff line change
Expand Up @@ -3171,7 +3171,7 @@ def create_vmss(cmd, vmss_name, resource_group_name, image=None,
public_ip_address_type=None, storage_profile=None,
single_placement_group=None, custom_data=None, secrets=None, platform_fault_domain_count=None,
plan_name=None, plan_product=None, plan_publisher=None, plan_promotion_code=None, license_type=None,
assign_identity=None, identity_scope=None, identity_role=None,
assign_identity=None, identity_scope=None, identity_role=None, encryption_identity=None,
identity_role_id=None, zones=None, priority=None, eviction_policy=None,
application_security_groups=None, ultra_ssd_enabled=None,
ephemeral_os_disk=None, ephemeral_os_disk_placement=None,
Expand Down Expand Up @@ -3532,6 +3532,35 @@ def _get_public_ip_address_allocation(value, sku):
role_assignment_guid = str(_gen_guid())
master_template.add_resource(build_msi_role_assignment(vmss_name, vmss_id, identity_role_id,
role_assignment_guid, identity_scope, False))
if encryption_identity:
if not cmd.supported_api_version(min_api='2023-09-01', resource_type=ResourceType.MGMT_COMPUTE):
raise CLIError("Usage error: Encryption Identity required API version 2023-09-01 or higher."
"You can set the cloud's profile to use the required API Version with:"
"az cloud set --profile latest --name <cloud name>")
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

please set the min_api='2023-09-01' in the parameter definition instead of adding checking here

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Addressed Suggestion


if 'identity' in vmss_resource and 'userAssignedIdentities' in vmss_resource['identity'] \
and encryption_identity.lower() in \
(k.lower() for k in vmss_resource['identity']['userAssignedIdentities'].keys()):

if 'virtualMachineProfile' not in vmss_resource['properties']:
vmss_resource['properties']['virtualMachineProfile'] = {}
if 'securityProfile' not in vmss_resource['properties']['virtualMachineProfile']:
vmss_resource['properties']['virtualMachineProfile']['securityProfile'] = {}
if 'encryptionIdentity' not in vmss_resource['properties']['virtualMachineProfile']['securityProfile']:
vmss_resource['properties']['virtualMachineProfile']['securityProfile']['encryptionIdentity'] = {}

vmss_securityProfile_EncryptionIdentity \
= vmss_resource['properties']['virtualMachineProfile']['securityProfile']['encryptionIdentity']

if 'userAssignedIdentityResourceId' not in vmss_securityProfile_EncryptionIdentity or \
vmss_securityProfile_EncryptionIdentity['userAssignedIdentityResourceId'] \
!= encryption_identity:
vmss_securityProfile_EncryptionIdentity['userAssignedIdentityResourceId'] = encryption_identity
vmss_resource['properties']['virtualMachineProfile']['securityProfile']['encryptionIdentity'] \
= vmss_securityProfile_EncryptionIdentity
else:
raise CLIError("Encryption Identity should be an ARM Resource ID of one of the "
"user assigned identities associated to the resource")
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please use the specific error type ArgumentUsageError in stead of CLIError

Suggested change
raise CLIError("Encryption Identity should be an ARM Resource ID of one of the "
"user assigned identities associated to the resource")
raise ArgumentUsageError("Encryption Identity should be an ARM Resource ID of one of the "
"user assigned identities associated to the resource")

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

During VMSS creation if there is any exception, it will throw cliError exception by default, that's why used that cliError exception here

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In fact, we usually recommend using more specific error type if they can be clearly classified, as this will help us in future Telemetry data analysis

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Addressed Suggestion

else:
raise CLIError('usage error: --orchestration-mode (Uniform | Flexible)')

Expand Down
44 changes: 44 additions & 0 deletions src/azure-cli/azure/cli/command_modules/vm/disk_encryption.py
Original file line number Diff line number Diff line change
Expand Up @@ -92,6 +92,41 @@ def updateVmEncryptionSetting(cmd, vm, resource_group_name, vm_name, encryption_
return True


def updateVmssEncryptionSetting(cmd, vmss, resource_group_name, vmss_name, encryption_identity):
from azure.cli.core.azclierror import ArgumentUsageError
if vmss.identity is None or vmss.identity.user_assigned_identities is None or encryption_identity.lower() not in \
(k.lower() for k in vmss.identity.user_assigned_identities.keys()):
raise ArgumentUsageError("Encryption Identity should be an ARM Resource ID of one of the "
"user assigned identities associated to the resource")

VirtualMachineProfile, SecurityProfile, EncryptionIdentity \
= cmd.get_models('VirtualMachineProfile', 'SecurityProfile', 'EncryptionIdentity')
updateVmss = False

if vmss.virtual_machine_profile is None:
vmss.virtual_machine_profile = VirtualMachineProfile()
if vmss.virtual_machine_profile.security_profile is None:
vmss.virtual_machine_profile.security_profile = SecurityProfile()
if vmss.virtual_machine_profile.security_profile.encryption_identity is None:
vmss.virtual_machine_profile.security_profile.encryption_identity = EncryptionIdentity()
if vmss.virtual_machine_profile.security_profile.encryption_identity.user_assigned_identity_resource_id\
is None or vmss.virtual_machine_profile.security_profile.encryption_identity\
.user_assigned_identity_resource_id.lower() != encryption_identity:
vmss.virtual_machine_profile.security_profile.encryption_identity.user_assigned_identity_resource_id \
= encryption_identity
updateVmss = True

if updateVmss:
compute_client = _compute_client_factory(cmd.cli_ctx)
updateEncryptionIdentity \
= compute_client.virtual_machine_scale_sets.begin_create_or_update(resource_group_name, vmss_name, vmss)
LongRunningOperation(cmd.cli_ctx)(updateEncryptionIdentity)
result = updateEncryptionIdentity.result()
return result is not None and result.provisioning_state == 'Succeeded'
logger.info("No changes in identity")
return True


def isVersionSuppprtedForEncryptionIdentity(cmd):
from azure.cli.core.profiles import ResourceType
from knack.util import CLIError
Expand Down Expand Up @@ -436,8 +471,10 @@ def encrypt_vmss(cmd, resource_group_name, vmss_name, # pylint: disable=too-man
key_encryption_key=None,
key_encryption_algorithm='RSA-OAEP',
volume_type=None,
encryption_identity=None,
force=False):
from azure.mgmt.core.tools import parse_resource_id
from knack.util import CLIError

# pylint: disable=no-member
UpgradeMode, VirtualMachineScaleSetExtension, VirtualMachineScaleSetExtensionProfile = cmd.get_models(
Expand All @@ -459,6 +496,13 @@ def encrypt_vmss(cmd, resource_group_name, vmss_name, # pylint: disable=too-man
if key_encryption_key:
key_encryption_keyvault = key_encryption_keyvault or disk_encryption_keyvault

if encryption_identity and isVersionSuppprtedForEncryptionIdentity(cmd):
yanzhudd marked this conversation as resolved.
Show resolved Hide resolved
result = updateVmssEncryptionSetting(cmd, vmss, resource_group_name, vmss_name, encryption_identity)
if result:
logger.info("Encryption Identity successfully set in virtual machine scale set")
else:
raise CLIError("Failed to update encryption Identity to the VMSS")

# to avoid bad server errors, ensure the vault has the right configurations
_verify_keyvault_good_for_encryption(cmd.cli_ctx, disk_encryption_keyvault, key_encryption_keyvault, vmss, force)

Expand Down
Loading