-
Notifications
You must be signed in to change notification settings - Fork 3.1k
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 #30457
Changes from 8 commits
4be87fe
8ed447b
ee0d69f
57630f2
6f3ed9a
a95da2a
54b1e4d
1c86dc2
7b2db68
c2d7321
dd8f178
375c3f0
058c5c8
2e3c647
dc62428
3b20f86
553563a
dcbc33c
7cc1dec
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change | ||||
---|---|---|---|---|---|---|
|
@@ -1602,6 +1602,9 @@ | |||||
- name: Create a Debian11 VM with both system and user assigned identity. | ||||||
text: > | ||||||
az vm create -n MyVm -g rg1 --image Debian11 --assign-identity [system] /subscriptions/99999999-1bf0-4dda-aec3-cb9272f09590/resourcegroups/myRG/providers/Microsoft.ManagedIdentity/userAssignedIdentities/myID | ||||||
- name: Create a vm with user assigned identity and add encryption identity for Azure disk encryption | ||||||
text: > | ||||||
az vm create -n MyVm -g rg1 --image Debian11 --assign-identity myID --encryption-identity /subscriptions/99999999-1bf0-4dda-aec3-cb9272f09590/resourcegroups/myRG/providers/Microsoft.ManagedIdentity/userAssignedIdentities/myID | ||||||
- name: Create a VM in an availability zone in the current resource group's region. | ||||||
supported-profiles: latest | ||||||
text: > | ||||||
|
@@ -1810,6 +1813,9 @@ | |||||
- name: Enable disk encryption on the OS disk and/or data disks. Encrypt mounted disks. (autogenerated) | ||||||
text: | | ||||||
az vm encryption enable --disk-encryption-keyvault MyVault --name MyVm --resource-group MyResourceGroup --volume-type DATA | ||||||
- name: Adding support for using managed identity to authenticate to customer's keyvault for ADE operation | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Addressed suggestion |
||||||
text: > | ||||||
az vm encryption enable --disk-encryption-keyvault MyVault --name MyVm --resource-group MyResourceGroup --encryption-identity EncryptionIdentity | ||||||
crafted: true | ||||||
""" | ||||||
|
||||||
|
Original file line number | Diff line number | Diff line change | ||||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
@@ -886,8 +886,8 @@ def create_vm(cmd, vm_name, resource_group_name, image=None, size='Standard_DS1_ | |||||||||||||||||
storage_account_type=None, vnet_type=None, nsg_type=None, public_ip_address_type=None, nic_type=None, | ||||||||||||||||||
validate=False, custom_data=None, secrets=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, identity_role_id=None, application_security_groups=None, zone=None, | ||||||||||||||||||
boot_diagnostics_storage=None, ultra_ssd_enabled=None, | ||||||||||||||||||
identity_role=None, identity_role_id=None, encryption_identity=None, | ||||||||||||||||||
application_security_groups=None, zone=None, boot_diagnostics_storage=None, ultra_ssd_enabled=None, | ||||||||||||||||||
ephemeral_os_disk=None, ephemeral_os_disk_placement=None, | ||||||||||||||||||
proximity_placement_group=None, dedicated_host=None, dedicated_host_group=None, aux_subscriptions=None, | ||||||||||||||||||
priority=None, max_price=None, eviction_policy=None, enable_agent=None, workspace=None, vmss=None, | ||||||||||||||||||
|
@@ -1145,6 +1145,26 @@ def create_vm(cmd, vm_name, resource_group_name, image=None, size='Standard_DS1_ | |||||||||||||||||
master_template.add_resource(build_msi_role_assignment(vm_name, vm_id, identity_role_id, | ||||||||||||||||||
role_assignment_guid, identity_scope)) | ||||||||||||||||||
|
||||||||||||||||||
if encryption_identity: | ||||||||||||||||||
if not cmd.supported_api_version(min_api='2023-07-01', resource_type=ResourceType.MGMT_COMPUTE): | ||||||||||||||||||
raise CLIInternalError("Usage error: Encryption identity is not available under current profile." | ||||||||||||||||||
"You can set the cloud's profile to latest with:" | ||||||||||||||||||
"az cloud set --profile latest --name <cloud name>") | ||||||||||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Addressed suggestion |
||||||||||||||||||
|
||||||||||||||||||
if 'identity' in vm_resource and 'userAssignedIdentities' in vm_resource['identity'] \ | ||||||||||||||||||
and encryption_identity.lower() in \ | ||||||||||||||||||
(k.lower() for k in vm_resource['identity']['userAssignedIdentities'].keys()): | ||||||||||||||||||
if 'securityProfile' not in vm_resource['properties']: | ||||||||||||||||||
vm_resource['properties']['securityProfile'] = {} | ||||||||||||||||||
if 'encryptionIdentity' not in vm_resource['properties']['securityProfile']: | ||||||||||||||||||
vm_resource['properties']['securityProfile']['encryptionIdentity'] = {} | ||||||||||||||||||
if 'userAssignedIdentityResourceId' not \ | ||||||||||||||||||
in vm_resource['properties']['securityProfile']['encryptionIdentity'] or \ | ||||||||||||||||||
vm_resource['properties']['securityProfile']['encryptionIdentity']['userAssignedIdentityResourceId'] \ | ||||||||||||||||||
!= encryption_identity: | ||||||||||||||||||
vm_resource['properties']['securityProfile']['encryptionIdentity']['userAssignedIdentityResourceId'] \ | ||||||||||||||||||
= encryption_identity | ||||||||||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This judgment condition is too long. Could we assign There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Addressed suggestion |
||||||||||||||||||
|
||||||||||||||||||
if workspace is not None: | ||||||||||||||||||
workspace_id = _prepare_workspace(cmd, resource_group_name, workspace) | ||||||||||||||||||
master_template.add_secure_parameter('workspaceId', workspace_id) | ||||||||||||||||||
|
Original file line number | Diff line number | Diff line change | ||||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
@@ -61,6 +61,40 @@ def _detect_ade_status(vm): | |||||||||||||||||
return True, False # we believe impossible to have both old & new ADE | ||||||||||||||||||
|
||||||||||||||||||
|
||||||||||||||||||
def updateVmEncryptionSetting(cmd, vm, resource_group_name, vm_name, encryption_identity): | ||||||||||||||||||
SecurityProfile, EncryptionIdentity = cmd.get_models('SecurityProfile', 'EncryptionIdentity') | ||||||||||||||||||
updateVm = False | ||||||||||||||||||
if vm.security_profile is None: | ||||||||||||||||||
vm.security_profile = SecurityProfile() | ||||||||||||||||||
if vm.security_profile.encryption_identity is None: | ||||||||||||||||||
vm.security_profile.encryption_identity = EncryptionIdentity() | ||||||||||||||||||
if vm.security_profile.encryption_identity.user_assigned_identity_resource_id is None \ | ||||||||||||||||||
or vm.security_profile.encryption_identity.user_assigned_identity_resource_id.lower() \ | ||||||||||||||||||
!= encryption_identity: | ||||||||||||||||||
vm.security_profile.encryption_identity.user_assigned_identity_resource_id = encryption_identity | ||||||||||||||||||
updateVm = True | ||||||||||||||||||
|
||||||||||||||||||
if updateVm: | ||||||||||||||||||
compute_client = _compute_client_factory(cmd.cli_ctx) | ||||||||||||||||||
updateEncryptionIdentity \ | ||||||||||||||||||
= compute_client.virtual_machines.begin_create_or_update(resource_group_name, vm_name, vm) | ||||||||||||||||||
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 | ||||||||||||||||||
if not cmd.supported_api_version(min_api='2023-07-01', resource_type=ResourceType.MGMT_COMPUTE): | ||||||||||||||||||
raise CLIError("Usage error: Encryption identity is not available under current profile." | ||||||||||||||||||
"You can set the cloud's profile to latest with:" | ||||||||||||||||||
"az cloud set --profile latest --name <cloud name>") | ||||||||||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Addressed suggestion |
||||||||||||||||||
return True | ||||||||||||||||||
|
||||||||||||||||||
|
||||||||||||||||||
def encrypt_vm(cmd, resource_group_name, vm_name, # pylint: disable=too-many-locals, too-many-statements | ||||||||||||||||||
disk_encryption_keyvault, | ||||||||||||||||||
aad_client_id=None, | ||||||||||||||||||
|
@@ -70,7 +104,7 @@ def encrypt_vm(cmd, resource_group_name, vm_name, # pylint: disable=too-many-lo | |||||||||||||||||
key_encryption_algorithm='RSA-OAEP', | ||||||||||||||||||
volume_type=None, | ||||||||||||||||||
encrypt_format_all=False, | ||||||||||||||||||
force=False): | ||||||||||||||||||
force=False, encryption_identity=None): | ||||||||||||||||||
from azure.mgmt.core.tools import parse_resource_id | ||||||||||||||||||
from knack.util import CLIError | ||||||||||||||||||
|
||||||||||||||||||
|
@@ -109,6 +143,12 @@ def encrypt_vm(cmd, resource_group_name, vm_name, # pylint: disable=too-many-lo | |||||||||||||||||
# disk encryption key itself can be further protected, so let us verify | ||||||||||||||||||
if key_encryption_key: | ||||||||||||||||||
key_encryption_keyvault = key_encryption_keyvault or disk_encryption_keyvault | ||||||||||||||||||
if encryption_identity and isVersionSuppprtedForEncryptionIdentity(cmd): | ||||||||||||||||||
result = updateVmEncryptionSetting(cmd, vm, resource_group_name, vm_name, encryption_identity) | ||||||||||||||||||
if result: | ||||||||||||||||||
logger.info("Encryption Identity successfully set in virtual machine") | ||||||||||||||||||
else: | ||||||||||||||||||
raise CLIError("Failed to update encryption Identity to the VM") | ||||||||||||||||||
|
||||||||||||||||||
# 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, vm, force) | ||||||||||||||||||
|
@@ -553,11 +593,18 @@ def _report_client_side_validation_error(msg): | |||||||||||||||||
disk_vault_resource_info = parse_resource_id(disk_vault_id) | ||||||||||||||||||
key_vault = client.get(disk_vault_resource_info['resource_group'], disk_vault_resource_info['name']) | ||||||||||||||||||
|
||||||||||||||||||
# ensure vault has 'EnabledForDiskEncryption' permission | ||||||||||||||||||
if not key_vault.properties or not key_vault.properties.enabled_for_disk_encryption: | ||||||||||||||||||
_report_client_side_validation_error("Keyvault '{}' is not enabled for disk encryption.".format( | ||||||||||||||||||
disk_vault_resource_info['resource_name'])) | ||||||||||||||||||
|
||||||||||||||||||
# ensure vault has 'EnabledForDiskEncryption' permission or VM has encryption identity set for ADE operation | ||||||||||||||||||
if resource_type == 'VM': | ||||||||||||||||||
if vm_or_vmss.security_profile and vm_or_vmss.security_profile.encryption_identity and \ | ||||||||||||||||||
vm_or_vmss.security_profile.encryption_identity.user_assigned_identity_resource_id: | ||||||||||||||||||
pass | ||||||||||||||||||
elif not key_vault.properties or not key_vault.properties.enabled_for_disk_encryption: | ||||||||||||||||||
_report_client_side_validation_error( | ||||||||||||||||||
"Keyvault '{}' is not enabled for disk encryption.".format(disk_vault_resource_info['resource_name'])) | ||||||||||||||||||
else: | ||||||||||||||||||
if not key_vault.properties or not key_vault.properties.enabled_for_disk_encryption: | ||||||||||||||||||
_report_client_side_validation_error( | ||||||||||||||||||
"Keyvault '{}' is not enabled for disk encryption.".format(disk_vault_resource_info['resource_name'])) | ||||||||||||||||||
if kek_vault_id: | ||||||||||||||||||
kek_vault_info = parse_resource_id(kek_vault_id) | ||||||||||||||||||
if disk_vault_resource_info['name'].lower() != kek_vault_info['name'].lower(): | ||||||||||||||||||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Addressed suggestion