From dfa80598a6542b6203d9c75a36505a0c9b6edf16 Mon Sep 17 00:00:00 2001 From: cmendible Date: Thu, 4 Feb 2021 08:51:46 +0000 Subject: [PATCH] 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).