[linux|windows]_virtual_machine - Allow update of OS disk encryption settings

[kazimierzbudzyk]

Fixes: #6183

Inspired by how os_disk resizes have to be handled: https://github.com/kazimierzbudzyk/terraform-provider-azurerm/blob/master/azurerm/internal/services/compute/linux_virtual_machine_resource.go#L835

[kazimierzbudzyk]

Updated tests to match changes in 3765f32

[tombuildsstuff]

LGTM - thanks for this @kazimierzbudzyk

[tombuildsstuff]

@kazimierzbudzyk I've tested this in several Azure Regions but this doesn't appear to be possible at this point in time:

compute.DisksClient#Update: Failure sending request: StatusCode=0 -- Original Error: autorest/azure: Service returned an error. Status= Code="PropertyChangeNotAllowed" Message="Changing property 'encryption.diskEncryptionSetId' is not allowed." Target="encryption.diskEncryptionSetId"

I know this functionality is coming at some point (a few months post GA was the last update I found), but do you happen to know which Azure Region this functionality is supported in?

Thanks!

[kazimierzbudzyk]

@tombuildsstuff I don't have a full list, but tested it successfully in centralus. Was the VM deallocated (although, terraform did it for me correctly as well)?

[kazimierzbudzyk]

Tested in useast successfully as well, but while doing that I discovered a bug in the implementation. While the API interaction works correctly (verified in the UI), state is not getting updated so the update continues showing up as pending change. I am looking into what could be causing it (field not getting set correctly in resourceLinuxVirtualMachineRead?), but if you have any ideas what could be happening here it would be very helpful!

[kazimierzbudzyk]

Looks like it's actually an API issue - I managed to reproduce in az vm. API returns diskEncryptionSet empty for encrypted os_disk:

      "osDisk": {
        "caching": "ReadWrite",
        "createOption": "FromImage",
        "diffDiskSettings": null,
        "diskSizeGb": null,
        "encryptionSettings": null,
        "image": null,
        "managedDisk": {
          "diskEncryptionSet": null,
          "id": "/subscriptions/${subscriptionId}/resourceGroups/ZAZ-TEST-USEAST/providers/Microsoft.Compute/disks/cmk-os-disk",
          "resourceGroup": "ZAZ-TEST-USEAST",
          "storageAccountType": null
        },
        "name": "cmk-os-disk",
        "osType": "Linux",
        "vhd": null,
        "writeAcceleratorEnabled": false
      }

But if I use the disk API (az disk) it does show the encryption properties (consistent with UI):

{
    "creationData": {
      "createOption": "FromImage",
      "imageReference": {
        "id": "/Subscriptions/${subscriptionId}/Providers/Microsoft.Compute/Locations/eastus/Publishers/Canonical/ArtifactTypes/VMImage/Offers/UbuntuServer/Skus/16.04-LTS/Versions/16.04.202003170",
        "lun": null
      },
      "sourceResourceId": null,
      "sourceUniqueId": null,
      "sourceUri": null,
      "storageAccountId": null,
      "uploadSizeBytes": null
    },
    "diskIopsReadWrite": 120,
    "diskMbpsReadWrite": 25,
    "diskSizeBytes": 32213303296,
    "diskSizeGb": 30,
    "diskState": "Reserved",
    "encryption": {
      "diskEncryptionSetId": "/subscriptions/${subscriptionId}/resourceGroups/ZAZ-TEST-USEAST/providers/Microsoft.Compute/diskEncryptionSets/cmk-disk-encryption-set-ue",
      "type": "EncryptionAtRestWithCustomerKey"
    },
    "encryptionSettingsCollection": null,
    "hyperVgeneration": "V1",
    "id": "/subscriptions/${subscriptionId}/resourceGroups/zaz-test-useast/providers/Microsoft.Compute/disks/cmk-os-disk",
    "location": "eastus",
    "managedBy": "/subscriptions/${subscriptionId}/resourceGroups/zaz-test-useast/providers/Microsoft.Compute/virtualMachines/cmk-test-vm",
    "name": "cmk-os-disk",
    "osType": "Linux",
    "provisioningState": "Succeeded",
    "resourceGroup": "zaz-test-useast",
    "sku": {
      "name": "Premium_LRS",
      "tier": "Premium"
    },
    "tags": {},
    "timeCreated": "2020-03-25T15:24:31.687148+00:00",
    "type": "Microsoft.Compute/disks",
    "uniqueId": "59d45a60-fff3-4985-a8a9-ded1dc2e1b92",
    "zones": [
      "1"
    ]
  }

[kazimierzbudzyk]

Looks like I was missing changes to read logic, fixed in 09a21c4.

[kazimierzbudzyk]

@tombuildsstuff What do you think about handling the issue by wrapping PropertyChangeNotAllowed to return message suggesting selected region doesn't support the feature?

[ramankumarlive]

@kazimierzbudzyk @tombuildsstuff I am the Azure PM for the feature. The feature is already available in all the public and Azure Government regions in GA. It will be available in rest of the Sovereign regions soon. Please see the documentation for the supported regions.

https://docs.microsoft.com/en-us/azure/virtual-machines/windows/disk-encryption#supported-regions

[katbyte]

Hi @kazimierzbudzyk,

Thanks for the fixes, however it looks like the tests aren't passing:

Test Failed

------- Stdout: -------
=== RUN   TestAccLinuxVirtualMachine_diskOSDiskEncryptionSetUpdate
=== PAUSE TestAccLinuxVirtualMachine_diskOSDiskEncryptionSetUpdate
=== CONT  TestAccLinuxVirtualMachine_diskOSDiskEncryptionSetUpdate
--- FAIL: TestAccLinuxVirtualMachine_diskOSDiskEncryptionSetUpdate (283.09s)
    testing.go:640: Step 0 error: errors during apply:
        
        Error: Error validating Key Vault "acctestkv8xqyg" (Resource Group "acctestrg-200407214337778564") for Disk Encryption Set: Soft Delete must be enabled but it isn't!
        
          on /opt/teamcity-agent/temp/buildTmp/tf-test525312821/main.tf line 90:
          (source code not available)

I hope you don't mind but i'm going to update the test configs so they pass 🙂

[ashrayjain]

Could we get this in 2.5.0 if possible. This is blocking us from picking up CMKs.
Thanks!

[ghost]

This has been released in version 2.5.0 of the provider. Please see the Terraform documentation on provider versioning or reach out if you need any assistance upgrading. As an example:

provider "azurerm" {
    version = "~> 2.5.0"
}
# ... other configuration ...

[ghost]

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 feel this issue should be reopened, we encourage creating a new issue linking back to this one for added context. If you feel I made an error 🤖 🙉 , please reach out to my human friends 👉 [email protected]. Thanks!