From c59c4f8be1ec01ebd5709ac1ce95d346ce92d4d4 Mon Sep 17 00:00:00 2001 From: cmendible Date: Mon, 14 Dec 2020 17:17:46 +0000 Subject: [PATCH 1/7] Added support for network_access_policy --- .../services/compute/managed_disk_resource.go | 46 +++ .../compute/managed_disk_resource_test.go | 337 ++++++++++++++++++ website/docs/d/managed_disk.html.markdown | 4 + website/docs/r/managed_disk.html.markdown | 4 + 4 files changed, 391 insertions(+) diff --git a/azurerm/internal/services/compute/managed_disk_resource.go b/azurerm/internal/services/compute/managed_disk_resource.go index 30fffae4f244..efc073f81527 100644 --- a/azurerm/internal/services/compute/managed_disk_resource.go +++ b/azurerm/internal/services/compute/managed_disk_resource.go @@ -143,6 +143,22 @@ func resourceManagedDisk() *schema.Resource { "encryption_settings": encryptionSettingsSchema(), + "network_access_policy": { + Type: schema.TypeString, + Optional: true, + ValidateFunc: validation.StringInSlice([]string{ + string(compute.AllowAll), + string(compute.AllowPrivate), + string(compute.DenyAll), + }, true), + }, + + "disk_access_id": { + Type: schema.TypeString, + Optional: true, + ValidateFunc: azure.ValidateResourceID, + }, + "tags": tags.Schema(), }, } @@ -256,6 +272,20 @@ func resourceManagedDiskCreateUpdate(d *schema.ResourceData, meta interface{}) e } } + if networkAccessPolicy, ok := d.GetOk("network_access_policy"); ok { + props.NetworkAccessPolicy = compute.NetworkAccessPolicy(networkAccessPolicy.(string)) + } + + if props.NetworkAccessPolicy == compute.AllowPrivate { + if d.HasChange("disk_access_id") { + v := d.Get("disk_access_id") + diskAccessID := v.(string) + props.DiskAccessID = &diskAccessID + } + } else if d.HasChange("disk_access_id") { + return fmt.Errorf("[ERROR] disk_access_id are only available for when network_access_policy is set to AllowPrivate") + } + createDisk := compute.Disk{ Name: &name, Location: &location, @@ -373,6 +403,20 @@ func resourceManagedDiskUpdate(d *schema.ResourceData, meta interface{}) error { } } + if d.HasChange("network_access_policy") { + diskUpdate.NetworkAccessPolicy = compute.NetworkAccessPolicy(d.Get("network_access_policy").(string)) + } + + if diskUpdate.NetworkAccessPolicy == compute.AllowPrivate { + if d.HasChange("disk_access_id") { + v := d.Get("disk_access_id") + diskAccessID := v.(string) + diskUpdate.DiskAccessID = &diskAccessID + } + } else if d.HasChange("disk_access_id") { + return fmt.Errorf("[ERROR] disk_access_id are only available for when network_access_policy is set to AllowPrivate") + } + // whilst we need to shut this down, if we're not attached to anything there's no point if shouldShutDown && disk.ManagedBy == nil { shouldShutDown = false @@ -544,6 +588,8 @@ func resourceManagedDiskRead(d *schema.ResourceData, meta interface{}) error { d.Set("disk_iops_read_write", props.DiskIOPSReadWrite) d.Set("disk_mbps_read_write", props.DiskMBpsReadWrite) d.Set("os_type", props.OsType) + d.Set("network_access_policy", props.NetworkAccessPolicy) + d.Set("disk_access_id", props.DiskAccessID) diskEncryptionSetId := "" if props.Encryption != nil && props.Encryption.DiskEncryptionSetID != nil { diff --git a/azurerm/internal/services/compute/managed_disk_resource_test.go b/azurerm/internal/services/compute/managed_disk_resource_test.go index 202bbc284d3b..5616c18ece93 100644 --- a/azurerm/internal/services/compute/managed_disk_resource_test.go +++ b/azurerm/internal/services/compute/managed_disk_resource_test.go @@ -3,6 +3,7 @@ package compute_test import ( "context" "fmt" + "net/http" "testing" "github.com/Azure/azure-sdk-for-go/services/compute/mgmt/2020-12-01/compute" @@ -322,6 +323,147 @@ func TestAccManagedDisk_attachedStorageTypeUpdate(t *testing.T) { }) } +func TestAccAzureRMManagedDisk_create_withNetworkPolicy(t *testing.T) { + data := acceptance.BuildTestData(t, "azurerm_managed_disk", "test") + r := ManagedDiskResource{} + + data.ResourceTest(t, r, []resource.TestStep{ + { + Config: testAccAzureRMManagedDisk_create_withNetworkPolicy(data), + Check: resource.ComposeTestCheckFunc( + testCheckManagedDiskExists(data.ResourceName, true), + ), + }, + }) +} + +func TestAccAzureRMManagedDisk_update_withNetworkPolicy(t *testing.T) { + data := acceptance.BuildTestData(t, "azurerm_managed_disk", "test") + r := ManagedDiskResource{} + + data.ResourceTest(t, r, []resource.TestStep{ + { + Config: testAccAzureRMManagedDisk_create_withNetworkPolicy(data), + Check: resource.ComposeTestCheckFunc( + testCheckManagedDiskExists(data.ResourceName, true), + resource.TestCheckResourceAttr(data.ResourceName, "network_access_policy", "DenyAll"), + ), + }, + { + Config: testAccAzureRMManagedDisk_update_withNetworkPolicy(data), + Check: resource.ComposeTestCheckFunc( + testCheckManagedDiskExists(data.ResourceName, true), + resource.TestCheckResourceAttr(data.ResourceName, "network_access_policy", "DenyAll"), + ), + }, + }) +} + +func TestAccAzureRMManagedDisk_import_withNetworkPolicy(t *testing.T) { + data := acceptance.BuildTestData(t, "azurerm_managed_disk", "test") + r := ManagedDiskResource{} + + data.ResourceTest(t, r, []resource.TestStep{ + { + Config: testAccAzureRMManagedDisk_create_withNetworkPolicy(data), + Check: resource.ComposeTestCheckFunc( + testCheckManagedDiskExists(data.ResourceName, true), + ), + }, + { + Config: testAccAzureRMManagedDisk_import_withNetworkPolicy(data), + ExpectError: acceptance.RequiresImportError("azurerm_managed_disk"), + }, + }) +} + +func TestAccAzureRMManagedDisk_create_withAllowPrivateNetworkPolicy(t *testing.T) { + data := acceptance.BuildTestData(t, "azurerm_managed_disk", "test") + r := ManagedDiskResource{} + + data.ResourceTest(t, r, []resource.TestStep{ + { + Config: testAccAzureRMManagedDisk_create_withAllowPrivateNetworkPolicy(data), + Check: resource.ComposeTestCheckFunc( + testCheckManagedDiskExists(data.ResourceName, true), + ), + }, + }) +} + +func TestAccAzureRMManagedDisk_update_withAllowPrivateNetworkPolicy(t *testing.T) { + data := acceptance.BuildTestData(t, "azurerm_managed_disk", "test") + r := ManagedDiskResource{} + + data.ResourceTest(t, r, []resource.TestStep{ + { + Config: testAccAzureRMManagedDisk_create_withAllowPrivateNetworkPolicy(data), + Check: resource.ComposeTestCheckFunc( + testCheckManagedDiskExists(data.ResourceName, true), + resource.TestCheckResourceAttr(data.ResourceName, "network_access_policy", "AllowPrivate"), + ), + }, + { + Config: testAccAzureRMManagedDisk_update_withAllowPrivateNetworkPolicy(data), + Check: resource.ComposeTestCheckFunc( + testCheckManagedDiskExists(data.ResourceName, true), + resource.TestCheckResourceAttr(data.ResourceName, "network_access_policy", "AllowPrivate"), + ), + }, + }) +} + +func TestAccAzureRMManagedDisk_import_withAllowPrivateNetworkPolicy(t *testing.T) { + data := acceptance.BuildTestData(t, "azurerm_managed_disk", "test") + r := ManagedDiskResource{} + + data.ResourceTest(t, r, []resource.TestStep{ + { + Config: testAccAzureRMManagedDisk_create_withAllowPrivateNetworkPolicy(data), + Check: resource.ComposeTestCheckFunc( + testCheckManagedDiskExists(data.ResourceName, true), + ), + }, + { + Config: testAccAzureRMManagedDisk_import_withAllowPrivateNetworkPolicy(data), + ExpectError: acceptance.RequiresImportError("azurerm_managed_disk"), + }, + }) +} + +// nolint unparam +func testCheckManagedDiskExists(resourceName string, shouldExist bool) resource.TestCheckFunc { + return func(s *terraform.State) error { + client := acceptance.AzureProvider.Meta().(*clients.Client).Compute.DisksClient + ctx := acceptance.AzureProvider.Meta().(*clients.Client).StopContext + + rs, ok := s.RootModule().Resources[resourceName] + if !ok { + return fmt.Errorf("Not found: %s", resourceName) + } + + dName := rs.Primary.Attributes["name"] + resourceGroup, hasResourceGroup := rs.Primary.Attributes["resource_group_name"] + if !hasResourceGroup { + return fmt.Errorf("Bad: no resource group found in state for disk: %s", dName) + } + + resp, err := client.Get(ctx, resourceGroup, dName) + if err != nil { + return fmt.Errorf("Bad: Get on diskClient: %+v", err) + } + + if resp.StatusCode == http.StatusNotFound && shouldExist { + return fmt.Errorf("Bad: ManagedDisk %q (resource group %q) does not exist", dName, resourceGroup) + } + if resp.StatusCode != http.StatusNotFound && !shouldExist { + return fmt.Errorf("Bad: ManagedDisk %q (resource group %q) still exists", dName, resourceGroup) + } + + return nil + } +} + func (ManagedDiskResource) Exists(ctx context.Context, clients *clients.Client, state *terraform.InstanceState) (*bool, error) { id, err := parse.ManagedDiskID(state.ID) if err != nil { @@ -1031,3 +1173,198 @@ resource "azurerm_linux_virtual_machine" "test" { } `, data.RandomInteger, data.Locations.Primary, data.RandomInteger, data.RandomInteger, data.RandomInteger) } + +func testAccAzureRMManagedDisk_create_withNetworkPolicy(data acceptance.TestData) string { + return fmt.Sprintf(` +provider "azurerm" { + features {} +} + +resource "azurerm_resource_group" "test" { + name = "acctestRG-%d" + location = "%s" +} + +resource "azurerm_managed_disk" "test" { + name = "acctestd-%d" + location = azurerm_resource_group.test.location + resource_group_name = azurerm_resource_group.test.name + storage_account_type = "Standard_LRS" + create_option = "Empty" + disk_size_gb = "4" + disk_iops_read_write = "101" + disk_mbps_read_write = "10" + zones = ["1"] + network_access_policy = "DenyAll" + + tags = { + environment = "acctest" + cost-center = "ops" + } +} +`, data.RandomInteger, data.Locations.Primary, data.RandomInteger) +} + +func testAccAzureRMManagedDisk_update_withNetworkPolicy(data acceptance.TestData) string { + return fmt.Sprintf(` +provider "azurerm" { + features {} +} + +resource "azurerm_resource_group" "test" { + name = "acctestRG-%d" + location = "%s" +} + +resource "azurerm_managed_disk" "test" { + name = "acctestd-%d" + location = azurerm_resource_group.test.location + resource_group_name = azurerm_resource_group.test.name + storage_account_type = "Standard_LRS" + create_option = "Empty" + disk_size_gb = "4" + disk_iops_read_write = "102" + disk_mbps_read_write = "11" + zones = ["1"] + network_access_policy = "DenyAll" + + tags = { + environment = "acctest" + cost-center = "ops" + } +} +`, data.RandomInteger, data.Locations.Primary, data.RandomInteger) +} + +func testAccAzureRMManagedDisk_import_withNetworkPolicy(data acceptance.TestData) string { + template := testAccAzureRMManagedDisk_create_withNetworkPolicy(data) + return fmt.Sprintf(` +%s + +resource "azurerm_managed_disk" "import" { + name = azurerm_managed_disk.test.name + location = azurerm_managed_disk.test.location + resource_group_name = azurerm_managed_disk.test.resource_group_name + storage_account_type = "Standard_LRS" + create_option = "Empty" + disk_size_gb = "4" + disk_iops_read_write = "101" + disk_mbps_read_write = "10" + network_access_policy = "DenyAll" + + tags = { + environment = "acctest" + cost-center = "ops" + } +} +`, template) +} + +func testAccAzureRMManagedDisk_create_withAllowPrivateNetworkPolicy(data acceptance.TestData) string { + return fmt.Sprintf(` +provider "azurerm" { + features {} +} + +resource "azurerm_resource_group" "test" { + name = "acctestRG-%d" + location = "%s" +} + +resource "azurerm_disk_access" "test" { + name = "accda%d" + resource_group_name = azurerm_resource_group.test.name + location = azurerm_resource_group.test.location + + tags = { + environment = "staging" + } +} + +resource "azurerm_managed_disk" "test" { + name = "acctestd-%d" + location = azurerm_resource_group.test.location + resource_group_name = azurerm_resource_group.test.name + storage_account_type = "Standard_LRS" + create_option = "Empty" + disk_size_gb = "4" + disk_iops_read_write = "101" + disk_mbps_read_write = "10" + zones = ["1"] + network_access_policy = "AllowPrivate" + disk_access_id = azurerm_disk_access.test.id + + tags = { + environment = "acctest" + cost-center = "ops" + } +} +`, data.RandomInteger, data.Locations.Primary, data.RandomInteger, data.RandomInteger) +} + +func testAccAzureRMManagedDisk_update_withAllowPrivateNetworkPolicy(data acceptance.TestData) string { + return fmt.Sprintf(` +provider "azurerm" { + features {} +} + +resource "azurerm_resource_group" "test" { + name = "acctestRG-%d" + location = "%s" +} + +resource "azurerm_disk_access" "test" { + name = "accda%d" + resource_group_name = azurerm_resource_group.test.name + location = azurerm_resource_group.test.location + + tags = { + environment = "staging" + } +} + +resource "azurerm_managed_disk" "test" { + name = "acctestd-%d" + location = azurerm_resource_group.test.location + resource_group_name = azurerm_resource_group.test.name + storage_account_type = "Standard_LRS" + create_option = "Empty" + disk_size_gb = "4" + disk_iops_read_write = "102" + disk_mbps_read_write = "11" + zones = ["1"] + network_access_policy = "AllowPrivate" + disk_access_id = azurerm_disk_access.test.id + + tags = { + environment = "acctest" + cost-center = "ops" + } +} +`, data.RandomInteger, data.Locations.Primary, data.RandomInteger, data.RandomInteger) +} + +func testAccAzureRMManagedDisk_import_withAllowPrivateNetworkPolicy(data acceptance.TestData) string { + template := testAccAzureRMManagedDisk_create_withAllowPrivateNetworkPolicy(data) + return fmt.Sprintf(` +%s + +resource "azurerm_managed_disk" "import" { + name = azurerm_managed_disk.test.name + location = azurerm_managed_disk.test.location + resource_group_name = azurerm_managed_disk.test.resource_group_name + storage_account_type = "Standard_LRS" + create_option = "Empty" + disk_size_gb = "4" + disk_iops_read_write = "101" + disk_mbps_read_write = "10" + network_access_policy = "AllowPrivate" + disk_access_id = azurerm_disk_access.test.id + + tags = { + environment = "acctest" + cost-center = "ops" + } +} +`, template) +} diff --git a/website/docs/d/managed_disk.html.markdown b/website/docs/d/managed_disk.html.markdown index d925d36153ac..50032e3c75d0 100644 --- a/website/docs/d/managed_disk.html.markdown +++ b/website/docs/d/managed_disk.html.markdown @@ -55,6 +55,10 @@ output "id" { * `zones` - A list of Availability Zones where the Managed Disk exists. +* `network_access_policy` - Policy for accessing the disk via network. Accepted values: AllowAll, AllowPrivate, DenyAll + +* `disk_access_id` - The ID of the disk access resource for using private endpoints on disks. + ## Timeouts The `timeouts` block allows you to specify [timeouts](https://www.terraform.io/docs/configuration/resources.html#timeouts) for certain actions: diff --git a/website/docs/r/managed_disk.html.markdown b/website/docs/r/managed_disk.html.markdown index 0d151f81df98..5056b527762f 100644 --- a/website/docs/r/managed_disk.html.markdown +++ b/website/docs/r/managed_disk.html.markdown @@ -121,6 +121,10 @@ The following arguments are supported: * `zones` - (Optional) A collection containing the availability zone to allocate the Managed Disk in. +* `network_access_policy` - Policy for accessing the disk via network. Accepted values: AllowAll, AllowPrivate, DenyAll + +* `disk_access_id` - The ID of the disk access resource for using private endpoints on disks. + -> **Note**: Availability Zones are [only supported in select regions at this time](https://docs.microsoft.com/en-us/azure/availability-zones/az-overview). For more information on managed disks, such as sizing options and pricing, please check out the [Azure Documentation](https://docs.microsoft.com/en-us/azure/storage/storage-managed-disks-overview). From dfa80598a6542b6203d9c75a36505a0c9b6edf16 Mon Sep 17 00:00:00 2001 From: cmendible Date: Thu, 4 Feb 2021 08:51:46 +0000 Subject: [PATCH 2/7] changes requested --- .../services/compute/managed_disk_resource.go | 2 +- .../compute/managed_disk_resource_test.go | 53 ++++--------------- website/docs/d/managed_disk.html.markdown | 2 +- website/docs/r/managed_disk.html.markdown | 8 +-- 4 files changed, 18 insertions(+), 47 deletions(-) diff --git a/azurerm/internal/services/compute/managed_disk_resource.go b/azurerm/internal/services/compute/managed_disk_resource.go index efc073f81527..d7adf2218cdc 100644 --- a/azurerm/internal/services/compute/managed_disk_resource.go +++ b/azurerm/internal/services/compute/managed_disk_resource.go @@ -150,7 +150,7 @@ func resourceManagedDisk() *schema.Resource { string(compute.AllowAll), string(compute.AllowPrivate), string(compute.DenyAll), - }, true), + }, false), }, "disk_access_id": { diff --git a/azurerm/internal/services/compute/managed_disk_resource_test.go b/azurerm/internal/services/compute/managed_disk_resource_test.go index 5616c18ece93..6a4a6605beb5 100644 --- a/azurerm/internal/services/compute/managed_disk_resource_test.go +++ b/azurerm/internal/services/compute/managed_disk_resource_test.go @@ -3,7 +3,6 @@ package compute_test import ( "context" "fmt" - "net/http" "testing" "github.com/Azure/azure-sdk-for-go/services/compute/mgmt/2020-12-01/compute" @@ -331,7 +330,7 @@ func TestAccAzureRMManagedDisk_create_withNetworkPolicy(t *testing.T) { { Config: testAccAzureRMManagedDisk_create_withNetworkPolicy(data), Check: resource.ComposeTestCheckFunc( - testCheckManagedDiskExists(data.ResourceName, true), + check.That(data.ResourceName).ExistsInAzure(r), ), }, }) @@ -345,14 +344,14 @@ func TestAccAzureRMManagedDisk_update_withNetworkPolicy(t *testing.T) { { Config: testAccAzureRMManagedDisk_create_withNetworkPolicy(data), Check: resource.ComposeTestCheckFunc( - testCheckManagedDiskExists(data.ResourceName, true), + check.That(data.ResourceName).ExistsInAzure(r), resource.TestCheckResourceAttr(data.ResourceName, "network_access_policy", "DenyAll"), ), }, { Config: testAccAzureRMManagedDisk_update_withNetworkPolicy(data), Check: resource.ComposeTestCheckFunc( - testCheckManagedDiskExists(data.ResourceName, true), + check.That(data.ResourceName).ExistsInAzure(r), resource.TestCheckResourceAttr(data.ResourceName, "network_access_policy", "DenyAll"), ), }, @@ -367,7 +366,7 @@ func TestAccAzureRMManagedDisk_import_withNetworkPolicy(t *testing.T) { { Config: testAccAzureRMManagedDisk_create_withNetworkPolicy(data), Check: resource.ComposeTestCheckFunc( - testCheckManagedDiskExists(data.ResourceName, true), + check.That(data.ResourceName).ExistsInAzure(r), ), }, { @@ -385,9 +384,10 @@ func TestAccAzureRMManagedDisk_create_withAllowPrivateNetworkPolicy(t *testing.T { Config: testAccAzureRMManagedDisk_create_withAllowPrivateNetworkPolicy(data), Check: resource.ComposeTestCheckFunc( - testCheckManagedDiskExists(data.ResourceName, true), + check.That(data.ResourceName).ExistsInAzure(r), ), }, + data.ImportStep(), }) } @@ -399,17 +399,19 @@ func TestAccAzureRMManagedDisk_update_withAllowPrivateNetworkPolicy(t *testing.T { Config: testAccAzureRMManagedDisk_create_withAllowPrivateNetworkPolicy(data), Check: resource.ComposeTestCheckFunc( - testCheckManagedDiskExists(data.ResourceName, true), + check.That(data.ResourceName).ExistsInAzure(r), resource.TestCheckResourceAttr(data.ResourceName, "network_access_policy", "AllowPrivate"), ), }, + data.ImportStep(), { Config: testAccAzureRMManagedDisk_update_withAllowPrivateNetworkPolicy(data), Check: resource.ComposeTestCheckFunc( - testCheckManagedDiskExists(data.ResourceName, true), + check.That(data.ResourceName).ExistsInAzure(r), resource.TestCheckResourceAttr(data.ResourceName, "network_access_policy", "AllowPrivate"), ), }, + data.ImportStep(), }) } @@ -421,7 +423,7 @@ func TestAccAzureRMManagedDisk_import_withAllowPrivateNetworkPolicy(t *testing.T { Config: testAccAzureRMManagedDisk_create_withAllowPrivateNetworkPolicy(data), Check: resource.ComposeTestCheckFunc( - testCheckManagedDiskExists(data.ResourceName, true), + check.That(data.ResourceName).ExistsInAzure(r), ), }, { @@ -431,39 +433,6 @@ func TestAccAzureRMManagedDisk_import_withAllowPrivateNetworkPolicy(t *testing.T }) } -// nolint unparam -func testCheckManagedDiskExists(resourceName string, shouldExist bool) resource.TestCheckFunc { - return func(s *terraform.State) error { - client := acceptance.AzureProvider.Meta().(*clients.Client).Compute.DisksClient - ctx := acceptance.AzureProvider.Meta().(*clients.Client).StopContext - - rs, ok := s.RootModule().Resources[resourceName] - if !ok { - return fmt.Errorf("Not found: %s", resourceName) - } - - dName := rs.Primary.Attributes["name"] - resourceGroup, hasResourceGroup := rs.Primary.Attributes["resource_group_name"] - if !hasResourceGroup { - return fmt.Errorf("Bad: no resource group found in state for disk: %s", dName) - } - - resp, err := client.Get(ctx, resourceGroup, dName) - if err != nil { - return fmt.Errorf("Bad: Get on diskClient: %+v", err) - } - - if resp.StatusCode == http.StatusNotFound && shouldExist { - return fmt.Errorf("Bad: ManagedDisk %q (resource group %q) does not exist", dName, resourceGroup) - } - if resp.StatusCode != http.StatusNotFound && !shouldExist { - return fmt.Errorf("Bad: ManagedDisk %q (resource group %q) still exists", dName, resourceGroup) - } - - return nil - } -} - func (ManagedDiskResource) Exists(ctx context.Context, clients *clients.Client, state *terraform.InstanceState) (*bool, error) { id, err := parse.ManagedDiskID(state.ID) if err != nil { diff --git a/website/docs/d/managed_disk.html.markdown b/website/docs/d/managed_disk.html.markdown index 50032e3c75d0..11ced26622b0 100644 --- a/website/docs/d/managed_disk.html.markdown +++ b/website/docs/d/managed_disk.html.markdown @@ -55,7 +55,7 @@ output "id" { * `zones` - A list of Availability Zones where the Managed Disk exists. -* `network_access_policy` - Policy for accessing the disk via network. Accepted values: AllowAll, AllowPrivate, DenyAll +* `network_access_policy` - Policy for accessing the disk via network. * `disk_access_id` - The ID of the disk access resource for using private endpoints on disks. diff --git a/website/docs/r/managed_disk.html.markdown b/website/docs/r/managed_disk.html.markdown index 5056b527762f..2b8c0ec8e677 100644 --- a/website/docs/r/managed_disk.html.markdown +++ b/website/docs/r/managed_disk.html.markdown @@ -93,7 +93,7 @@ The following arguments are supported: * `disk_encryption_set_id` - (Optional) The ID of a Disk Encryption Set which should be used to encrypt this Managed Disk. --> **NOTE:** The Disk Encryption Set must have the `Reader` Role Assignment scoped on the Key Vault - in addition to an Access Policy to the Key Vault +~> **NOTE:** The Disk Encryption Set must have the `Reader` Role Assignment scoped on the Key Vault - in addition to an Access Policy to the Key Vault ~> **NOTE:** Disk Encryption Sets are in Public Preview in a limited set of regions @@ -121,11 +121,13 @@ The following arguments are supported: * `zones` - (Optional) A collection containing the availability zone to allocate the Managed Disk in. -* `network_access_policy` - Policy for accessing the disk via network. Accepted values: AllowAll, AllowPrivate, DenyAll +~> **Note**: Availability Zones are [only supported in select regions at this time](https://docs.microsoft.com/en-us/azure/availability-zones/az-overview). + +* `network_access_policy` - Policy for accessing the disk via network. Allowed values are `AllowAll`, `AllowPrivate`, and `DenyAll`. * `disk_access_id` - The ID of the disk access resource for using private endpoints on disks. --> **Note**: Availability Zones are [only supported in select regions at this time](https://docs.microsoft.com/en-us/azure/availability-zones/az-overview). +~> **Note**: `disk_access_id` is only supported when `network_access_policy` is set to `AllowPrivate`. For more information on managed disks, such as sizing options and pricing, please check out the [Azure Documentation](https://docs.microsoft.com/en-us/azure/storage/storage-managed-disks-overview). From e9f1f794db5c84aeca44976e6eb4ea8711ddb675 Mon Sep 17 00:00:00 2001 From: cmendible Date: Thu, 15 Apr 2021 10:21:19 +0000 Subject: [PATCH 3/7] Solved AllowAll --> null --- .../services/compute/managed_disk_resource.go | 51 +++++++++++-------- 1 file changed, 30 insertions(+), 21 deletions(-) diff --git a/azurerm/internal/services/compute/managed_disk_resource.go b/azurerm/internal/services/compute/managed_disk_resource.go index d7adf2218cdc..447e544c8442 100644 --- a/azurerm/internal/services/compute/managed_disk_resource.go +++ b/azurerm/internal/services/compute/managed_disk_resource.go @@ -165,6 +165,7 @@ func resourceManagedDisk() *schema.Resource { } func resourceManagedDiskCreateUpdate(d *schema.ResourceData, meta interface{}) error { + subscriptionId := meta.(*clients.Client).Account.SubscriptionId client := meta.(*clients.Client).Compute.DisksClient ctx, cancel := timeouts.ForCreateUpdate(meta.(*clients.Client).StopContext, d) defer cancel() @@ -174,8 +175,9 @@ func resourceManagedDiskCreateUpdate(d *schema.ResourceData, meta interface{}) e name := d.Get("name").(string) resourceGroup := d.Get("resource_group_name").(string) + id := parse.NewManagedDiskID(subscriptionId, d.Get("resource_group_name").(string), d.Get("name").(string)) if d.IsNewResource() { - existing, err := client.Get(ctx, resourceGroup, name) + existing, err := client.Get(ctx, id.ResourceGroup, id.DiskName) if err != nil { if !utils.ResponseWasNotFound(existing.Response) { return fmt.Errorf("Error checking for presence of existing Managed Disk %q (Resource Group %q): %s", name, resourceGroup, err) @@ -272,18 +274,20 @@ func resourceManagedDiskCreateUpdate(d *schema.ResourceData, meta interface{}) e } } - if networkAccessPolicy, ok := d.GetOk("network_access_policy"); ok { - props.NetworkAccessPolicy = compute.NetworkAccessPolicy(networkAccessPolicy.(string)) + if networkAccessPolicy := d.Get("network_access_policy").(string); networkAccessPolicy != "" { + props.NetworkAccessPolicy = compute.NetworkAccessPolicy(networkAccessPolicy) + } else { + props.NetworkAccessPolicy = compute.AllowAll } - if props.NetworkAccessPolicy == compute.AllowPrivate { - if d.HasChange("disk_access_id") { - v := d.Get("disk_access_id") - diskAccessID := v.(string) - props.DiskAccessID = &diskAccessID + if diskAccessID := d.Get("disk_access_id").(string); d.HasChange("disk_access_id") { + if props.NetworkAccessPolicy == compute.AllowPrivate { + props.DiskAccessID = utils.String(diskAccessID) + } else if diskAccessID != "" { + return fmt.Errorf("[ERROR] disk_access_id is only available when network_access_policy is set to AllowPrivate") + } else { + props.DiskAccessID = nil } - } else if d.HasChange("disk_access_id") { - return fmt.Errorf("[ERROR] disk_access_id are only available for when network_access_policy is set to AllowPrivate") } createDisk := compute.Disk{ @@ -314,7 +318,7 @@ func resourceManagedDiskCreateUpdate(d *schema.ResourceData, meta interface{}) e return fmt.Errorf("Error reading Managed Disk %s (Resource Group %q): ID was nil", name, resourceGroup) } - d.SetId(*read.ID) + d.SetId(id.ID()) return resourceManagedDiskRead(d, meta) } @@ -403,18 +407,20 @@ func resourceManagedDiskUpdate(d *schema.ResourceData, meta interface{}) error { } } - if d.HasChange("network_access_policy") { - diskUpdate.NetworkAccessPolicy = compute.NetworkAccessPolicy(d.Get("network_access_policy").(string)) + if networkAccessPolicy := d.Get("network_access_policy").(string); networkAccessPolicy != "" { + diskUpdate.NetworkAccessPolicy = compute.NetworkAccessPolicy(networkAccessPolicy) + } else { + diskUpdate.NetworkAccessPolicy = compute.AllowAll } - if diskUpdate.NetworkAccessPolicy == compute.AllowPrivate { - if d.HasChange("disk_access_id") { - v := d.Get("disk_access_id") - diskAccessID := v.(string) - diskUpdate.DiskAccessID = &diskAccessID + if diskAccessID := d.Get("disk_access_id").(string); d.HasChange("disk_access_id") { + if diskUpdate.NetworkAccessPolicy == compute.AllowPrivate { + diskUpdate.DiskAccessID = utils.String(diskAccessID) + } else if diskAccessID != "" { + return fmt.Errorf("[ERROR] disk_access_id is only available when network_access_policy is set to AllowPrivate") + } else { + diskUpdate.DiskAccessID = nil } - } else if d.HasChange("disk_access_id") { - return fmt.Errorf("[ERROR] disk_access_id are only available for when network_access_policy is set to AllowPrivate") } // whilst we need to shut this down, if we're not attached to anything there's no point @@ -588,7 +594,10 @@ func resourceManagedDiskRead(d *schema.ResourceData, meta interface{}) error { d.Set("disk_iops_read_write", props.DiskIOPSReadWrite) d.Set("disk_mbps_read_write", props.DiskMBpsReadWrite) d.Set("os_type", props.OsType) - d.Set("network_access_policy", props.NetworkAccessPolicy) + + if networkAccessPolicy := props.NetworkAccessPolicy; networkAccessPolicy != compute.AllowAll { + d.Set("network_access_policy", props.NetworkAccessPolicy) + } d.Set("disk_access_id", props.DiskAccessID) diskEncryptionSetId := "" From 652b1f282d48455605d02631b577e6e2c8dac65e Mon Sep 17 00:00:00 2001 From: cmendible Date: Thu, 15 Apr 2021 13:21:35 +0000 Subject: [PATCH 4/7] using switch --- .../services/compute/managed_disk_resource.go | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-) diff --git a/azurerm/internal/services/compute/managed_disk_resource.go b/azurerm/internal/services/compute/managed_disk_resource.go index 447e544c8442..c54150dfca67 100644 --- a/azurerm/internal/services/compute/managed_disk_resource.go +++ b/azurerm/internal/services/compute/managed_disk_resource.go @@ -281,11 +281,12 @@ func resourceManagedDiskCreateUpdate(d *schema.ResourceData, meta interface{}) e } if diskAccessID := d.Get("disk_access_id").(string); d.HasChange("disk_access_id") { - if props.NetworkAccessPolicy == compute.AllowPrivate { + switch { + case props.NetworkAccessPolicy == compute.AllowPrivate: props.DiskAccessID = utils.String(diskAccessID) - } else if diskAccessID != "" { + case diskAccessID != "" && props.NetworkAccessPolicy != compute.AllowPrivate: return fmt.Errorf("[ERROR] disk_access_id is only available when network_access_policy is set to AllowPrivate") - } else { + default: props.DiskAccessID = nil } } @@ -414,11 +415,12 @@ func resourceManagedDiskUpdate(d *schema.ResourceData, meta interface{}) error { } if diskAccessID := d.Get("disk_access_id").(string); d.HasChange("disk_access_id") { - if diskUpdate.NetworkAccessPolicy == compute.AllowPrivate { + switch { + case diskUpdate.NetworkAccessPolicy == compute.AllowPrivate: diskUpdate.DiskAccessID = utils.String(diskAccessID) - } else if diskAccessID != "" { + case diskAccessID != "" && diskUpdate.NetworkAccessPolicy != compute.AllowPrivate: return fmt.Errorf("[ERROR] disk_access_id is only available when network_access_policy is set to AllowPrivate") - } else { + default: diskUpdate.DiskAccessID = nil } } From 176dec9378adb9a67222b690c335a26de9b60ff2 Mon Sep 17 00:00:00 2001 From: cmendible Date: Mon, 26 Apr 2021 07:35:48 +0000 Subject: [PATCH 5/7] renamed some tests and removed duplicates --- .../compute/managed_disk_resource_test.go | 116 +++--------------- 1 file changed, 17 insertions(+), 99 deletions(-) diff --git a/azurerm/internal/services/compute/managed_disk_resource_test.go b/azurerm/internal/services/compute/managed_disk_resource_test.go index 6a4a6605beb5..ef11518431f4 100644 --- a/azurerm/internal/services/compute/managed_disk_resource_test.go +++ b/azurerm/internal/services/compute/managed_disk_resource_test.go @@ -322,67 +322,52 @@ func TestAccManagedDisk_attachedStorageTypeUpdate(t *testing.T) { }) } -func TestAccAzureRMManagedDisk_create_withNetworkPolicy(t *testing.T) { +func TestAccAzureRMManagedDisk_networkPolicy(t *testing.T) { data := acceptance.BuildTestData(t, "azurerm_managed_disk", "test") r := ManagedDiskResource{} data.ResourceTest(t, r, []resource.TestStep{ { - Config: testAccAzureRMManagedDisk_create_withNetworkPolicy(data), + Config: testAccAzureRMManagedDisk_networkPolicy_create(data), Check: resource.ComposeTestCheckFunc( check.That(data.ResourceName).ExistsInAzure(r), ), }, + data.ImportStep(), }) } -func TestAccAzureRMManagedDisk_update_withNetworkPolicy(t *testing.T) { +func TestAccAzureRMManagedDisk_networkPolicy_update(t *testing.T) { data := acceptance.BuildTestData(t, "azurerm_managed_disk", "test") r := ManagedDiskResource{} data.ResourceTest(t, r, []resource.TestStep{ { - Config: testAccAzureRMManagedDisk_create_withNetworkPolicy(data), + Config: testAccAzureRMManagedDisk_networkPolicy_create(data), Check: resource.ComposeTestCheckFunc( check.That(data.ResourceName).ExistsInAzure(r), resource.TestCheckResourceAttr(data.ResourceName, "network_access_policy", "DenyAll"), ), }, + data.ImportStep(), { - Config: testAccAzureRMManagedDisk_update_withNetworkPolicy(data), + Config: testAccAzureRMManagedDisk_networkPolicy_update(data), Check: resource.ComposeTestCheckFunc( check.That(data.ResourceName).ExistsInAzure(r), resource.TestCheckResourceAttr(data.ResourceName, "network_access_policy", "DenyAll"), ), }, + data.ImportStep(), }) } -func TestAccAzureRMManagedDisk_import_withNetworkPolicy(t *testing.T) { - data := acceptance.BuildTestData(t, "azurerm_managed_disk", "test") - r := ManagedDiskResource{} - - data.ResourceTest(t, r, []resource.TestStep{ - { - Config: testAccAzureRMManagedDisk_create_withNetworkPolicy(data), - Check: resource.ComposeTestCheckFunc( - check.That(data.ResourceName).ExistsInAzure(r), - ), - }, - { - Config: testAccAzureRMManagedDisk_import_withNetworkPolicy(data), - ExpectError: acceptance.RequiresImportError("azurerm_managed_disk"), - }, - }) -} - -func TestAccAzureRMManagedDisk_create_withAllowPrivateNetworkPolicy(t *testing.T) { +func TestAccAzureRMManagedDisk_networkPolicy_create_withAllowPrivate(t *testing.T) { data := acceptance.BuildTestData(t, "azurerm_managed_disk", "test") r := ManagedDiskResource{} data.ResourceTest(t, r, []resource.TestStep{ { - Config: testAccAzureRMManagedDisk_create_withAllowPrivateNetworkPolicy(data), + Config: testAccAzureRMManagedDisk_networkPolicy_create_withAllowPrivate(data), Check: resource.ComposeTestCheckFunc( check.That(data.ResourceName).ExistsInAzure(r), ), @@ -391,13 +376,13 @@ func TestAccAzureRMManagedDisk_create_withAllowPrivateNetworkPolicy(t *testing.T }) } -func TestAccAzureRMManagedDisk_update_withAllowPrivateNetworkPolicy(t *testing.T) { +func TestAccAzureRMManagedDisk_networkPolicy_update_withAllowPrivate(t *testing.T) { data := acceptance.BuildTestData(t, "azurerm_managed_disk", "test") r := ManagedDiskResource{} data.ResourceTest(t, r, []resource.TestStep{ { - Config: testAccAzureRMManagedDisk_create_withAllowPrivateNetworkPolicy(data), + Config: testAccAzureRMManagedDisk_networkPolicy_create_withAllowPrivate(data), Check: resource.ComposeTestCheckFunc( check.That(data.ResourceName).ExistsInAzure(r), resource.TestCheckResourceAttr(data.ResourceName, "network_access_policy", "AllowPrivate"), @@ -405,7 +390,7 @@ func TestAccAzureRMManagedDisk_update_withAllowPrivateNetworkPolicy(t *testing.T }, data.ImportStep(), { - Config: testAccAzureRMManagedDisk_update_withAllowPrivateNetworkPolicy(data), + Config: testAccAzureRMManagedDisk_networkPolicy_update_withAllowPrivate(data), Check: resource.ComposeTestCheckFunc( check.That(data.ResourceName).ExistsInAzure(r), resource.TestCheckResourceAttr(data.ResourceName, "network_access_policy", "AllowPrivate"), @@ -415,24 +400,6 @@ func TestAccAzureRMManagedDisk_update_withAllowPrivateNetworkPolicy(t *testing.T }) } -func TestAccAzureRMManagedDisk_import_withAllowPrivateNetworkPolicy(t *testing.T) { - data := acceptance.BuildTestData(t, "azurerm_managed_disk", "test") - r := ManagedDiskResource{} - - data.ResourceTest(t, r, []resource.TestStep{ - { - Config: testAccAzureRMManagedDisk_create_withAllowPrivateNetworkPolicy(data), - Check: resource.ComposeTestCheckFunc( - check.That(data.ResourceName).ExistsInAzure(r), - ), - }, - { - Config: testAccAzureRMManagedDisk_import_withAllowPrivateNetworkPolicy(data), - ExpectError: acceptance.RequiresImportError("azurerm_managed_disk"), - }, - }) -} - func (ManagedDiskResource) Exists(ctx context.Context, clients *clients.Client, state *terraform.InstanceState) (*bool, error) { id, err := parse.ManagedDiskID(state.ID) if err != nil { @@ -1143,7 +1110,7 @@ resource "azurerm_linux_virtual_machine" "test" { `, data.RandomInteger, data.Locations.Primary, data.RandomInteger, data.RandomInteger, data.RandomInteger) } -func testAccAzureRMManagedDisk_create_withNetworkPolicy(data acceptance.TestData) string { +func testAccAzureRMManagedDisk_networkPolicy_create(data acceptance.TestData) string { return fmt.Sprintf(` provider "azurerm" { features {} @@ -1174,7 +1141,7 @@ resource "azurerm_managed_disk" "test" { `, data.RandomInteger, data.Locations.Primary, data.RandomInteger) } -func testAccAzureRMManagedDisk_update_withNetworkPolicy(data acceptance.TestData) string { +func testAccAzureRMManagedDisk_networkPolicy_update(data acceptance.TestData) string { return fmt.Sprintf(` provider "azurerm" { features {} @@ -1205,31 +1172,7 @@ resource "azurerm_managed_disk" "test" { `, data.RandomInteger, data.Locations.Primary, data.RandomInteger) } -func testAccAzureRMManagedDisk_import_withNetworkPolicy(data acceptance.TestData) string { - template := testAccAzureRMManagedDisk_create_withNetworkPolicy(data) - return fmt.Sprintf(` -%s - -resource "azurerm_managed_disk" "import" { - name = azurerm_managed_disk.test.name - location = azurerm_managed_disk.test.location - resource_group_name = azurerm_managed_disk.test.resource_group_name - storage_account_type = "Standard_LRS" - create_option = "Empty" - disk_size_gb = "4" - disk_iops_read_write = "101" - disk_mbps_read_write = "10" - network_access_policy = "DenyAll" - - tags = { - environment = "acctest" - cost-center = "ops" - } -} -`, template) -} - -func testAccAzureRMManagedDisk_create_withAllowPrivateNetworkPolicy(data acceptance.TestData) string { +func testAccAzureRMManagedDisk_networkPolicy_create_withAllowPrivate(data acceptance.TestData) string { return fmt.Sprintf(` provider "azurerm" { features {} @@ -1271,7 +1214,7 @@ resource "azurerm_managed_disk" "test" { `, data.RandomInteger, data.Locations.Primary, data.RandomInteger, data.RandomInteger) } -func testAccAzureRMManagedDisk_update_withAllowPrivateNetworkPolicy(data acceptance.TestData) string { +func testAccAzureRMManagedDisk_networkPolicy_update_withAllowPrivate(data acceptance.TestData) string { return fmt.Sprintf(` provider "azurerm" { features {} @@ -1312,28 +1255,3 @@ resource "azurerm_managed_disk" "test" { } `, data.RandomInteger, data.Locations.Primary, data.RandomInteger, data.RandomInteger) } - -func testAccAzureRMManagedDisk_import_withAllowPrivateNetworkPolicy(data acceptance.TestData) string { - template := testAccAzureRMManagedDisk_create_withAllowPrivateNetworkPolicy(data) - return fmt.Sprintf(` -%s - -resource "azurerm_managed_disk" "import" { - name = azurerm_managed_disk.test.name - location = azurerm_managed_disk.test.location - resource_group_name = azurerm_managed_disk.test.resource_group_name - storage_account_type = "Standard_LRS" - create_option = "Empty" - disk_size_gb = "4" - disk_iops_read_write = "101" - disk_mbps_read_write = "10" - network_access_policy = "AllowPrivate" - disk_access_id = azurerm_disk_access.test.id - - tags = { - environment = "acctest" - cost-center = "ops" - } -} -`, template) -} From d64af57cda7c696cfea2368d967c98b61c71d19a Mon Sep 17 00:00:00 2001 From: cmendible Date: Wed, 28 Apr 2021 19:59:25 +0000 Subject: [PATCH 6/7] fiexed tests --- .../services/compute/managed_disk_resource_test.go | 8 -------- 1 file changed, 8 deletions(-) diff --git a/azurerm/internal/services/compute/managed_disk_resource_test.go b/azurerm/internal/services/compute/managed_disk_resource_test.go index ef11518431f4..1aa4878b3dfd 100644 --- a/azurerm/internal/services/compute/managed_disk_resource_test.go +++ b/azurerm/internal/services/compute/managed_disk_resource_test.go @@ -1128,8 +1128,6 @@ resource "azurerm_managed_disk" "test" { storage_account_type = "Standard_LRS" create_option = "Empty" disk_size_gb = "4" - disk_iops_read_write = "101" - disk_mbps_read_write = "10" zones = ["1"] network_access_policy = "DenyAll" @@ -1159,8 +1157,6 @@ resource "azurerm_managed_disk" "test" { storage_account_type = "Standard_LRS" create_option = "Empty" disk_size_gb = "4" - disk_iops_read_write = "102" - disk_mbps_read_write = "11" zones = ["1"] network_access_policy = "DenyAll" @@ -1200,8 +1196,6 @@ resource "azurerm_managed_disk" "test" { storage_account_type = "Standard_LRS" create_option = "Empty" disk_size_gb = "4" - disk_iops_read_write = "101" - disk_mbps_read_write = "10" zones = ["1"] network_access_policy = "AllowPrivate" disk_access_id = azurerm_disk_access.test.id @@ -1242,8 +1236,6 @@ resource "azurerm_managed_disk" "test" { storage_account_type = "Standard_LRS" create_option = "Empty" disk_size_gb = "4" - disk_iops_read_write = "102" - disk_mbps_read_write = "11" zones = ["1"] network_access_policy = "AllowPrivate" disk_access_id = azurerm_disk_access.test.id From f0cc2de215a712b6d3db6814c0308330d14cf3b2 Mon Sep 17 00:00:00 2001 From: cmendible Date: Thu, 29 Apr 2021 15:32:11 +0000 Subject: [PATCH 7/7] suppress.CaseDifference --- .../internal/services/compute/managed_disk_resource.go | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/azurerm/internal/services/compute/managed_disk_resource.go b/azurerm/internal/services/compute/managed_disk_resource.go index c54150dfca67..4b9cc5e49128 100644 --- a/azurerm/internal/services/compute/managed_disk_resource.go +++ b/azurerm/internal/services/compute/managed_disk_resource.go @@ -152,11 +152,13 @@ func resourceManagedDisk() *schema.Resource { string(compute.DenyAll), }, false), }, - "disk_access_id": { - Type: schema.TypeString, - Optional: true, - ValidateFunc: azure.ValidateResourceID, + Type: schema.TypeString, + Optional: true, + // TODO: make this case-sensitive once this bug in the Azure API has been fixed: + // https://github.com/Azure/azure-rest-api-specs/issues/14192 + DiffSuppressFunc: suppress.CaseDifference, + ValidateFunc: azure.ValidateResourceID, }, "tags": tags.Schema(),