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

Add disk_encryption_set for data disks in azure_rm_virtualmachine #1309

Conversation

ephracis
Copy link
Contributor

SUMMARY

Add the parameter data_disks.disk_encryption_set to the azure_rm_virtualmachine module, making it possible to specify which DES to use when encrypting the data disk.

ISSUE TYPE
  • Feature Pull Request
COMPONENT NAME

azure_rm_virtualmachine

ADDITIONAL INFORMATION

This is required when creating a VM (with data disks) from an encrypted image which resides in another landing zone.

@Fred-sun Fred-sun added enhancement New feature or request medium_priority Medium priority work in In trying to solve, or in working with contributors labels Nov 1, 2023
@Fred-sun
Copy link
Collaborator

Fred-sun commented Nov 2, 2023

@ephracis Please fix the conflict in the document! Thank you very much!

Add the parameter `data_disks.disk_encryption_set` to the
`azure_rm_virtualmachine` module, making it possible to specify which
DES to use when encrypting the data disk.

This is required when creating a VM (with data disks) from an encrypted
image which resides in another landing zone.
@ephracis ephracis force-pushed the add-virtualmachine-data-encryption-set branch from 845db86 to 8888585 Compare November 2, 2023 08:19
@ephracis
Copy link
Contributor Author

ephracis commented Nov 2, 2023

@ephracis Please fix the conflict in the document! Thank you very much!

Done!

@ephracis ephracis mentioned this pull request Nov 3, 2023
@@ -1865,6 +1869,10 @@ def exec_module(self, **kwargs):
else:
data_disk_vhd = None
data_disk_managed_disk = self.compute_models.ManagedDiskParameters(storage_account_type=data_disk['managed_disk_type'])
if data_disk.get('disk_encryption_set'):
Copy link
Collaborator

@Fred-sun Fred-sun Nov 7, 2023

Choose a reason for hiding this comment

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

Does it mean that 'disk_encryption_set' can only be specified if 'managed_disk_type' is not configured for data_disks(self.data_disks.get('managed_disk_type')=None)?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think so. I'm not an expert at Azure but from what I can see, it is only the class ManagedDiskParameters which has the parameter disk_encryption_set as per the Azure Python SDK: https://learn.microsoft.com/en-us/python/api/azure-mgmt-compute/azure.mgmt.compute.v2023_07_01.models.manageddiskparameters?view=azure-python

I'm not sure if other types of disk can be encrypted using a DES. That might be the case. But for example my current customer has a use case where they use managed disk and need encryption with CMK. This would solve that use case at least.

@Fred-sun
Copy link
Collaborator

Fred-sun commented Nov 7, 2023

@ephracis Please add 'disk_encryption_set' to argument. Thank you very much!

nonexistent-parameter-documented: Argument 'disk_encryption_set' found in data_disks is listed in DOCUMENTATION.options, but not accepted by the module argument_spec

@Fred-sun Fred-sun added ready_for_review The PR has been modified and can be reviewed and merged and removed work in In trying to solve, or in working with contributors labels Nov 9, 2023
@xuzhang3
Copy link
Collaborator

@ephracis LGTM 🚢

@xuzhang3 xuzhang3 merged commit 2158040 into ansible-collections:dev Nov 14, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request medium_priority Medium priority ready_for_review The PR has been modified and can be reviewed and merged
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants