Add support for disks using encrypted storage policies

[belleatkins]

Community Guidelines

• [X] I have read and agree to the HashiCorp Community Guidelines .
• [X] Vote on this issue by adding a 👍 reaction to the original issue initial description to help the maintainers prioritize.
• [X] Do not leave "+1" or other comments that do not add relevant information or questions.
• [X] If you are interested in working on this issue or have submitted a pull request, please leave a comment.

Description

Currently, if you specify an encrypted storage policy for a disk subresource (where the disk already exists) when creating a VM in vsphere, you hit an error.

This is because vsphere does not support encrypting a disk on attachment to a new VM with an encrypted storage policy.

Vsphere does support a two step process - attaching disks to a VM which has an encrypted storage policy, and then changing the disks to have an encrypted storage policy.

Terraform-provider-vsphere could handle creating a VM, with disk subresources which have encrypted storage policies by using this two step process - creating the VM with its storage policy and attaching the disks with the default storage policy, and as a post-creation step, setting the disk storage policy to the encrypted storage policy specified.

It should also handle attaching already-encrypted disks to a new VM which has an encrypted storage policy. I don't quite know how to do this yet.

A trickier scenario for terraform-provider-vsphere to handle would be creating a VM with an unencrypted storage policy and attaching encrypted disks to this as unencrypted subresource disks. This would require something like: creating an encrypted VM with the disks attached as encrypted subresource disks, and then changing all storage policies to unencrypted in a separate step.

I'd like to add support for:

• creating a VM with an encrypted storage policy and attaching subresource disks with encrypted storage policies.
• attaching already encrypted disks to a new VM with an encrypted storage policy

Would I be able to get support in doing this?

Use Case(s)

We want to deploy VMs with encrypted disks.

Potential Terraform Provider Configuration

No response

References

#584 https://docs.vmware.com/en/VMware-vSphere/7.0/com.vmware.vsphere.security.doc/GUID-06E45092-22DD-4064-AF55-FB5D0FD4E588.html

[github-actions[bot]]

Hello, belleatkins! 🖐

Thank you for submitting an issue for this provider. The issue will now enter into the issue lifecycle.

If you want to contribute to this project, please review the contributing guidelines and information on submitting pull requests.

[tenthirtyam]

Hi @belleatkins - contributions are welcome - please review the contributing guidelines and information on submitting pull requests.

Happy to assist you.

Ryan Johnson Senior Staff Solutions Architect | Product Engineering @ VMware, Inc.

[belleatkins]

EDIT: RESOLVED, NO NEED TO REPLY TO THIS COMMENT

Thanks @tenthirtyam , I've reviewed those guidelines.

I've made some progress but I'm hoping you can answer some questions to get me past some sticky bits.

Some context:

1. I've changed the "Create" function under the disk subresource so that it doesn't generate a spec slice to set the storage policy id of a disk subresource when it is first created
2. I've added some code to resourceVsphereVirtualMachinePostDeployChanges (the function called after the VM is initially deployed, which attaches the already existing disks to the VM) to check for a spec change for the disks (which it will identify since vprops.Config shall not have the storage policy id but d shall have the storage policy id for the disks). I then call code to update the disks - applyVirtualDevices, which in theory should update their storage policy ids.

Details of 2: I inserted my code here I called expandVirtualMachineConfigSpecChanged I get the virtual devices calling object.VirtualDeviceList I call applyVirtualDevices I call resourceVSphereVirtualMachineUpdateReconfigureWithSDRS

The thing going wrong - is that "d" (the resource data - which includes the "new" and "old" data) isn't up to date - so the "old" data is empty. So when it goes to applyVirtualDevices, it doesn't know that the disks have been attached to the VM, which causes problems. I want to call something just before the added code which "updates d" so that it knows about the old disks.

I can't quite piece together how to do this - I know we store the terraform state somewhere, I just can't find how to update it!

Any ideas?

[belleatkins]

I've implemented this feature but I want to check a few things:

1. In this line https://github.com/hashicorp/terraform-provider-vsphere/blob/49989dea485e629788875d2c44112edc18b9109a/vsphere/internal/virtualdevice/virtual_machine_disk_subresource.go#L1801 why is this logic inside the check? ie why is there no path interpolation for attached disks? I've needed to remove this check, and wanted to make sure I understand the consequences of that.

2. Same question as above for this check: https://github.com/hashicorp/terraform-provider-vsphere/blob/49989dea485e629788875d2c44112edc18b9109a/vsphere/internal/virtualdevice/virtual_machine_disk_subresource.go#L1286 Why does it not apply to attached disks? I've had to remove this to implement the feature and want to understand the consequences

[belleatkins]

PR available here: https://github.com/hashicorp/terraform-provider-vsphere/pull/1839

[github-actions[bot]]

Marking this issue as stale due to inactivity in the past 180 days. This helps us focus on the active issues. If this issue is reproducible with the latest version of the provider, please comment. If this issue receives no comments in the next 30 days it will automatically be closed. If this issue was automatically closed and you feel this issue should be reopened, we encourage creating a new issue linking back to this one for added context. Thank you!

[github-actions[bot]]

Marking this issue as stale due to inactivity in the past 180 days. This helps us focus on the active issues. If this issue is reproducible with the latest version of the provider, please comment. If this issue receives no comments in the next 30 days it will automatically be closed. If this issue was automatically closed and you feel this issue should be reopened, we encourage creating a new issue linking back to this one for added context. Thank you!

[github-actions[bot]]

I'm going to lock this issue because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active issues. If you have found a problem that seems similar to this, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further.