From edf87237c04bf3c0a0035197613f0de62d60bdad Mon Sep 17 00:00:00 2001 From: arcturusZhang Date: Fri, 24 Jul 2020 17:49:25 +0800 Subject: [PATCH 1/3] Enable key rotation for des --- .../compute/disk_encryption_set_resource.go | 74 ++++++-- .../disk_encryption_set_resource_test.go | 164 +++++++++++++++--- .../docs/r/disk_encryption_set.html.markdown | 6 +- 3 files changed, 200 insertions(+), 44 deletions(-) diff --git a/azurerm/internal/services/compute/disk_encryption_set_resource.go b/azurerm/internal/services/compute/disk_encryption_set_resource.go index 9fd9521cc548..81ba20ed5ff0 100644 --- a/azurerm/internal/services/compute/disk_encryption_set_resource.go +++ b/azurerm/internal/services/compute/disk_encryption_set_resource.go @@ -22,9 +22,9 @@ import ( func resourceArmDiskEncryptionSet() *schema.Resource { return &schema.Resource{ - Create: resourceArmDiskEncryptionSetCreateUpdate, + Create: resourceArmDiskEncryptionSetCreate, Read: resourceArmDiskEncryptionSetRead, - Update: resourceArmDiskEncryptionSetCreateUpdate, + Update: resourceArmDiskEncryptionSetUpdate, Delete: resourceArmDiskEncryptionSetDelete, Timeouts: &schema.ResourceTimeout{ @@ -54,7 +54,6 @@ func resourceArmDiskEncryptionSet() *schema.Resource { "key_vault_key_id": { Type: schema.TypeString, Required: true, - ForceNew: true, ValidateFunc: azure.ValidateKeyVaultChildId, }, @@ -91,26 +90,24 @@ func resourceArmDiskEncryptionSet() *schema.Resource { } } -func resourceArmDiskEncryptionSetCreateUpdate(d *schema.ResourceData, meta interface{}) error { +func resourceArmDiskEncryptionSetCreate(d *schema.ResourceData, meta interface{}) error { client := meta.(*clients.Client).Compute.DiskEncryptionSetsClient vaultClient := meta.(*clients.Client).KeyVault.VaultsClient - ctx, cancel := timeouts.ForCreateUpdate(meta.(*clients.Client).StopContext, d) + ctx, cancel := timeouts.ForCreate(meta.(*clients.Client).StopContext, d) defer cancel() name := d.Get("name").(string) resourceGroup := d.Get("resource_group_name").(string) - if d.IsNewResource() { - existing, err := client.Get(ctx, resourceGroup, name) - if err != nil { - if !utils.ResponseWasNotFound(existing.Response) { - return fmt.Errorf("Error checking for present of existing Disk Encryption Set %q (Resource Group %q): %+v", name, resourceGroup, err) - } - } - if existing.ID != nil && *existing.ID != "" { - return tf.ImportAsExistsError("azurerm_disk_encryption_set", *existing.ID) + existing, err := client.Get(ctx, resourceGroup, name) + if err != nil { + if !utils.ResponseWasNotFound(existing.Response) { + return fmt.Errorf("Error checking for present of existing Disk Encryption Set %q (Resource Group %q): %+v", name, resourceGroup, err) } } + if existing.ID != nil && *existing.ID != "" { + return tf.ImportAsExistsError("azurerm_disk_encryption_set", *existing.ID) + } keyVaultKeyId := d.Get("key_vault_key_id").(string) keyVaultDetails, err := diskEncryptionSetRetrieveKeyVault(ctx, vaultClient, keyVaultKeyId) @@ -203,6 +200,55 @@ func resourceArmDiskEncryptionSetRead(d *schema.ResourceData, meta interface{}) return tags.FlattenAndSet(d, resp.Tags) } +func resourceArmDiskEncryptionSetUpdate(d *schema.ResourceData, meta interface{}) error { + client := meta.(*clients.Client).Compute.DiskEncryptionSetsClient + vaultClient := meta.(*clients.Client).KeyVault.VaultsClient + ctx, cancel := timeouts.ForCreate(meta.(*clients.Client).StopContext, d) + defer cancel() + + id, err := parse.DiskEncryptionSetID(d.Id()) + if err != nil { + return err + } + + update := compute.DiskEncryptionSetUpdate{} + if d.HasChange("tags") { + update.Tags = tags.Expand(d.Get("tags").(map[string]interface{})) + } + + if d.HasChange("key_vault_key_id") { + keyVaultKeyId := d.Get("key_vault_key_id").(string) + keyVaultDetails, err := diskEncryptionSetRetrieveKeyVault(ctx, vaultClient, keyVaultKeyId) + if err != nil { + return fmt.Errorf("Error validating Key Vault Key %q for Disk Encryption Set: %+v", keyVaultKeyId, err) + } + if !keyVaultDetails.softDeleteEnabled { + return fmt.Errorf("Error validating Key Vault %q (Resource Group %q) for Disk Encryption Set: Soft Delete must be enabled but it isn't!", keyVaultDetails.keyVaultName, keyVaultDetails.resourceGroupName) + } + if !keyVaultDetails.purgeProtectionEnabled { + return fmt.Errorf("Error validating Key Vault %q (Resource Group %q) for Disk Encryption Set: Purge Protection must be enabled but it isn't!", keyVaultDetails.keyVaultName, keyVaultDetails.resourceGroupName) + } + update.DiskEncryptionSetUpdateProperties = &compute.DiskEncryptionSetUpdateProperties{ + ActiveKey: &compute.KeyVaultAndKeyReference{ + KeyURL: utils.String(keyVaultKeyId), + SourceVault: &compute.SourceVault{ + ID: utils.String(keyVaultDetails.keyVaultId), + }, + }, + } + } + + future, err := client.Update(ctx, id.ResourceGroup, id.Name, update) + if err != nil { + return fmt.Errorf("Error updating Disk Encryption Set %q (Resource Group %q): %+v", id.Name, id.ResourceGroup, err) + } + if err = future.WaitForCompletionRef(ctx, client.Client); err != nil { + return fmt.Errorf("Error waiting for update of Disk Encryption Set %q (Resource Group %q): %+v", id.Name, id.ResourceGroup, err) + } + + return resourceArmDiskEncryptionSetRead(d, meta) +} + func resourceArmDiskEncryptionSetDelete(d *schema.ResourceData, meta interface{}) error { client := meta.(*clients.Client).Compute.DiskEncryptionSetsClient ctx, cancel := timeouts.ForDelete(meta.(*clients.Client).StopContext, d) diff --git a/azurerm/internal/services/compute/tests/disk_encryption_set_resource_test.go b/azurerm/internal/services/compute/tests/disk_encryption_set_resource_test.go index ad9edd1823d8..393c267b0f1b 100644 --- a/azurerm/internal/services/compute/tests/disk_encryption_set_resource_test.go +++ b/azurerm/internal/services/compute/tests/disk_encryption_set_resource_test.go @@ -95,6 +95,41 @@ func TestAccAzureRMDiskEncryptionSet_update(t *testing.T) { }) } +func TestAccAzureRMDiskEncryptionSet_keyRotate(t *testing.T) { + data := acceptance.BuildTestData(t, "azurerm_disk_encryption_set", "test") + + resource.ParallelTest(t, resource.TestCase{ + PreCheck: func() { acceptance.PreCheck(t) }, + Providers: acceptance.SupportedProviders, + CheckDestroy: testCheckAzureRMDiskEncryptionSetDestroy, + Steps: []resource.TestStep{ + { + Config: testAccAzureRMDiskEncryptionSet_basic(data), + Check: resource.ComposeTestCheckFunc( + testCheckAzureRMDiskEncryptionSetExists(data.ResourceName), + ), + }, + data.ImportStep(), + // we have to first grant the permission for DiskEncryptionSet to access the KeyVault + { + Config: testAccAzureRMDiskEncryptionSet_grantAccessToKeyVault(data), + Check: resource.ComposeTestCheckFunc( + testCheckAzureRMDiskEncryptionSetExists(data.ResourceName), + ), + }, + data.ImportStep(), + // after the access is granted, we can rotate the key in DiskEncryptionSet + { + Config: testAccAzureRMDiskEncryptionSet_keyRotate(data), + Check: resource.ComposeTestCheckFunc( + testCheckAzureRMDiskEncryptionSetExists(data.ResourceName), + ), + }, + data.ImportStep(), + }, + }) +} + func testCheckAzureRMDiskEncryptionSetExists(resourceName string) resource.TestCheckFunc { return func(s *terraform.State) error { rs, ok := s.RootModule().Resources[resourceName] @@ -148,10 +183,6 @@ func testCheckAzureRMDiskEncryptionSetDestroy(s *terraform.State) error { } func testAccAzureRMDiskEncryptionSet_dependencies(data acceptance.TestData) string { - // whilst this is in Preview it's only supported in: West Central US, Canada Central, North Europe - // TODO: switch back to default location - location := "northeurope" - return fmt.Sprintf(` provider "azurerm" { features {} @@ -170,27 +201,28 @@ resource "azurerm_key_vault" "test" { resource_group_name = azurerm_resource_group.test.name tenant_id = data.azurerm_client_config.current.tenant_id sku_name = "premium" + soft_delete_enabled = true + purge_protection_enabled = true + enabled_for_disk_encryption = true +} - purge_protection_enabled = true - soft_delete_enabled = true - - access_policy { - tenant_id = data.azurerm_client_config.current.tenant_id - object_id = data.azurerm_client_config.current.object_id - - key_permissions = [ - "create", - "delete", - "get", - "update", - ] - - secret_permissions = [ - "get", - "delete", - "set", - ] - } +resource "azurerm_key_vault_access_policy" "service-principal" { + key_vault_id = azurerm_key_vault.test.id + tenant_id = data.azurerm_client_config.current.tenant_id + object_id = data.azurerm_client_config.current.object_id + + key_permissions = [ + "create", + "delete", + "get", + "update", + ] + + secret_permissions = [ + "get", + "delete", + "set", + ] } resource "azurerm_key_vault_key" "test" { @@ -207,8 +239,10 @@ resource "azurerm_key_vault_key" "test" { "verify", "wrapKey", ] + + depends_on = ["azurerm_key_vault_access_policy.service-principal"] } -`, data.RandomInteger, location, data.RandomString) +`, data.RandomInteger, data.Locations.Primary, data.RandomString) } func testAccAzureRMDiskEncryptionSet_basic(data acceptance.TestData) string { @@ -268,3 +302,83 @@ resource "azurerm_disk_encryption_set" "test" { } `, template, data.RandomInteger) } + +func testAccAzureRMDiskEncryptionSet_grantAccessToKeyVault(data acceptance.TestData) string { + template := testAccAzureRMDiskEncryptionSet_dependencies(data) + return fmt.Sprintf(` +%s + +resource "azurerm_key_vault_access_policy" "disk-encryption" { + key_vault_id = azurerm_key_vault.test.id + + key_permissions = [ + "get", + "wrapkey", + "unwrapkey", + ] + + tenant_id = azurerm_disk_encryption_set.test.identity.0.tenant_id + object_id = azurerm_disk_encryption_set.test.identity.0.principal_id +} + +resource "azurerm_disk_encryption_set" "test" { + name = "acctestDES-%d" + resource_group_name = azurerm_resource_group.test.name + location = azurerm_resource_group.test.location + key_vault_key_id = azurerm_key_vault_key.test.id + + identity { + type = "SystemAssigned" + } +} +`, template, data.RandomInteger) +} + +func testAccAzureRMDiskEncryptionSet_keyRotate(data acceptance.TestData) string { + template := testAccAzureRMDiskEncryptionSet_dependencies(data) + return fmt.Sprintf(` +%s + +resource "azurerm_key_vault_key" "new" { + name = "newKey" + key_vault_id = azurerm_key_vault.test.id + key_type = "RSA" + key_size = 2048 + + key_opts = [ + "decrypt", + "encrypt", + "sign", + "unwrapKey", + "verify", + "wrapKey", + ] + + depends_on = ["azurerm_key_vault_access_policy.service-principal"] +} + +resource "azurerm_key_vault_access_policy" "disk-encryption" { + key_vault_id = azurerm_key_vault.test.id + + key_permissions = [ + "get", + "wrapkey", + "unwrapkey", + ] + + tenant_id = azurerm_disk_encryption_set.test.identity.0.tenant_id + object_id = azurerm_disk_encryption_set.test.identity.0.principal_id +} + +resource "azurerm_disk_encryption_set" "test" { + name = "acctestDES-%d" + resource_group_name = azurerm_resource_group.test.name + location = azurerm_resource_group.test.location + key_vault_key_id = azurerm_key_vault_key.new.id + + identity { + type = "SystemAssigned" + } +} +`, template, data.RandomInteger) +} diff --git a/website/docs/r/disk_encryption_set.html.markdown b/website/docs/r/disk_encryption_set.html.markdown index 92909430e329..b46944ea1caa 100644 --- a/website/docs/r/disk_encryption_set.html.markdown +++ b/website/docs/r/disk_encryption_set.html.markdown @@ -14,10 +14,6 @@ Manages a Disk Encryption Set. ## Example Usage ```hcl -provider "azurerm" { - features {} -} - data "azurerm_client_config" "current" {} resource "azurerm_resource_group" "example" { @@ -30,10 +26,10 @@ resource "azurerm_key_vault" "example" { location = azurerm_resource_group.example.location resource_group_name = azurerm_resource_group.example.name tenant_id = data.azurerm_client_config.current.tenant_id + sku_name = "premium" enabled_for_disk_encryption = true soft_delete_enabled = true purge_protection_enabled = true - sku_name = "premium" } resource "azurerm_key_vault_key" "example" { From bb83d3ab747f8c81a6a40db966bfda2f1d96fbad Mon Sep 17 00:00:00 2001 From: arcturusZhang Date: Fri, 24 Jul 2020 17:53:05 +0800 Subject: [PATCH 2/3] Fix typo --- .../internal/services/compute/disk_encryption_set_resource.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/azurerm/internal/services/compute/disk_encryption_set_resource.go b/azurerm/internal/services/compute/disk_encryption_set_resource.go index 81ba20ed5ff0..284f913ec9ee 100644 --- a/azurerm/internal/services/compute/disk_encryption_set_resource.go +++ b/azurerm/internal/services/compute/disk_encryption_set_resource.go @@ -203,7 +203,7 @@ func resourceArmDiskEncryptionSetRead(d *schema.ResourceData, meta interface{}) func resourceArmDiskEncryptionSetUpdate(d *schema.ResourceData, meta interface{}) error { client := meta.(*clients.Client).Compute.DiskEncryptionSetsClient vaultClient := meta.(*clients.Client).KeyVault.VaultsClient - ctx, cancel := timeouts.ForCreate(meta.(*clients.Client).StopContext, d) + ctx, cancel := timeouts.ForUpdate(meta.(*clients.Client).StopContext, d) defer cancel() id, err := parse.DiskEncryptionSetID(d.Id()) From dda13d7a9f62b79ea241b6f311627331045fcbc5 Mon Sep 17 00:00:00 2001 From: Dapeng Zhang Date: Tue, 28 Jul 2020 11:12:20 +0800 Subject: [PATCH 3/3] Fix CI --- .../compute/tests/disk_encryption_set_resource_test.go | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/azurerm/internal/services/compute/tests/disk_encryption_set_resource_test.go b/azurerm/internal/services/compute/tests/disk_encryption_set_resource_test.go index 393c267b0f1b..df089d847d19 100644 --- a/azurerm/internal/services/compute/tests/disk_encryption_set_resource_test.go +++ b/azurerm/internal/services/compute/tests/disk_encryption_set_resource_test.go @@ -196,11 +196,11 @@ resource "azurerm_resource_group" "test" { } resource "azurerm_key_vault" "test" { - name = "acctestkv-%s" - location = azurerm_resource_group.test.location - resource_group_name = azurerm_resource_group.test.name - tenant_id = data.azurerm_client_config.current.tenant_id - sku_name = "premium" + name = "acctestkv-%s" + location = azurerm_resource_group.test.location + resource_group_name = azurerm_resource_group.test.name + tenant_id = data.azurerm_client_config.current.tenant_id + sku_name = "premium" soft_delete_enabled = true purge_protection_enabled = true enabled_for_disk_encryption = true