From 580e06de1fda1ad9956fb8ee0c7764ec6baeb350 Mon Sep 17 00:00:00 2001 From: minzhang Date: Mon, 13 Jan 2020 21:35:24 -0800 Subject: [PATCH] address review comments --- .../services/storage/client/client.go | 5 -- .../storage/data_source_storage_container.go | 59 ++++++------------- .../internal/services/storage/registration.go | 2 +- .../data_source_storage_container_test.go | 5 +- website/azurerm.erb | 2 +- ...rkdown => storage_container.html.markdown} | 18 ++---- 6 files changed, 25 insertions(+), 66 deletions(-) rename website/docs/d/{storage_account_container.html.markdown => storage_container.html.markdown} (65%) diff --git a/azurerm/internal/services/storage/client/client.go b/azurerm/internal/services/storage/client/client.go index 8b7ca0ed5c70c..9e0af9c918bfb 100644 --- a/azurerm/internal/services/storage/client/client.go +++ b/azurerm/internal/services/storage/client/client.go @@ -24,7 +24,6 @@ type Client struct { FileSystemsClient *filesystems.Client ManagementPoliciesClient storage.ManagementPoliciesClient BlobServicesClient storage.BlobServicesClient - ContainerClient storage.BlobContainersClient environment az.Environment } @@ -41,9 +40,6 @@ func NewClient(options *common.ClientOptions) *Client { blobServicesClient := storage.NewBlobServicesClientWithBaseURI(options.ResourceManagerEndpoint, options.SubscriptionId) options.ConfigureClient(&blobServicesClient.Client, options.ResourceManagerAuthorizer) - containerClient := storage.NewBlobContainersClientWithBaseURI(options.ResourceManagerEndpoint, options.SubscriptionId) - options.ConfigureClient(&containerClient.Client, options.ResourceManagerAuthorizer) - // TODO: switch Storage Containers to using the storage.BlobContainersClient // (which should fix #2977) when the storage clients have been moved in here return &Client{ @@ -51,7 +47,6 @@ func NewClient(options *common.ClientOptions) *Client { FileSystemsClient: &fileSystemsClient, ManagementPoliciesClient: managementPoliciesClient, BlobServicesClient: blobServicesClient, - ContainerClient: containerClient, environment: options.Environment, } } diff --git a/azurerm/internal/services/storage/data_source_storage_container.go b/azurerm/internal/services/storage/data_source_storage_container.go index be3e1ea27a575..071388f684b62 100644 --- a/azurerm/internal/services/storage/data_source_storage_container.go +++ b/azurerm/internal/services/storage/data_source_storage_container.go @@ -6,11 +6,9 @@ import ( "time" "github.com/hashicorp/terraform-plugin-sdk/helper/schema" - "github.com/terraform-providers/terraform-provider-azurerm/azurerm/helpers/azure" "github.com/terraform-providers/terraform-provider-azurerm/azurerm/internal/clients" "github.com/terraform-providers/terraform-provider-azurerm/azurerm/internal/timeouts" "github.com/terraform-providers/terraform-provider-azurerm/azurerm/utils" - "github.com/tombuildsstuff/giovanni/storage/2018-11-09/blob/containers" ) func dataSourceArmStorageContainer() *schema.Resource { @@ -22,19 +20,14 @@ func dataSourceArmStorageContainer() *schema.Resource { }, Schema: map[string]*schema.Schema{ - "storage_container_id": { - Type: schema.TypeString, - Required: true, - }, - "name": { Type: schema.TypeString, - Computed: true, + Required: true, }, "storage_account_name": { Type: schema.TypeString, - Computed: true, + Required: true, }, "container_access_type": { @@ -54,17 +47,6 @@ func dataSourceArmStorageContainer() *schema.Resource { Type: schema.TypeBool, Computed: true, }, - - "resource_group_name": azure.SchemaResourceGroupNameDeprecated(), - - "properties": { - Type: schema.TypeMap, - Computed: true, - Deprecated: "This field will be removed in version 2.0 of the Azure Provider", - Elem: &schema.Schema{ - Type: schema.TypeString, - }, - }, }, } } @@ -74,43 +56,40 @@ func dataSourceArmStorageContainerRead(d *schema.ResourceData, meta interface{}) ctx, cancel := timeouts.ForRead(meta.(*clients.Client).StopContext, d) defer cancel() - id, err := containers.ParseResourceID(d.Get("storage_container_id").(string)) - - if err != nil { - return err - } + containerName := d.Get("name").(string) + accountName := d.Get("storage_account_name").(string) - account, err := storageClient.FindAccount(ctx, id.AccountName) + account, err := storageClient.FindAccount(ctx, accountName) if err != nil { - return fmt.Errorf("Error retrieving Account %q for Container %q: %s", id.AccountName, id.ContainerName, err) + return fmt.Errorf("Error retrieving Account %q for Container %q: %s", accountName, containerName, err) } if account == nil { - log.Printf("[DEBUG] Unable to locate Account %q for Storage Container %q - assuming removed & removing from state", id.AccountName, id.ContainerName) + log.Printf("[DEBUG] Unable to locate Account %q for Storage Container %q - assuming removed & removing from state", accountName, containerName) d.SetId("") - return nil + return fmt.Errorf("Unable to locate Account %q for Storage Container %q", accountName, containerName) } client, err := storageClient.ContainersClient(ctx, *account) - d.SetId(client.GetResourceID(id.AccountName, id.ContainerName)) - if err != nil { - return fmt.Errorf("Error building Containers Client for Storage Account %q (Resource Group %q): %s", id.AccountName, account.ResourceGroup, err) + return fmt.Errorf("Error building Containers Client for Storage Account %q (Resource Group %q): %s", accountName, account.ResourceGroup, err) } - props, err := client.GetProperties(ctx, id.AccountName, id.ContainerName) + d.SetId(client.GetResourceID(accountName, containerName)) + + props, err := client.GetProperties(ctx, accountName, containerName) if err != nil { if utils.ResponseWasNotFound(props.Response) { - log.Printf("[DEBUG] Container %q was not found in Account %q / Resource Group %q - assuming removed & removing from state", id.ContainerName, id.AccountName, account.ResourceGroup) + log.Printf("[DEBUG] Container %q was not found in Account %q / Resource Group %q - assuming removed & removing from state", containerName, accountName, account.ResourceGroup) d.SetId("") - return nil + return fmt.Errorf("Container %q was not found in Account %q / Resource Group %q", containerName, accountName, account.ResourceGroup) } - return fmt.Errorf("Error retrieving Container %q (Account %q / Resource Group %q): %s", id.ContainerName, id.AccountName, account.ResourceGroup, err) + return fmt.Errorf("Error retrieving Container %q (Account %q / Resource Group %q): %s", containerName, accountName, account.ResourceGroup, err) } - d.Set("name", id.ContainerName) + d.Set("name", containerName) - d.Set("storage_account_name", id.AccountName) + d.Set("storage_account_name", accountName) d.Set("resource_group_name", account.ResourceGroup) @@ -120,10 +99,6 @@ func dataSourceArmStorageContainerRead(d *schema.ResourceData, meta interface{}) return fmt.Errorf("Error setting `metadata`: %+v", err) } - if err := d.Set("properties", flattenStorageContainerProperties(props)); err != nil { - return fmt.Errorf("Error setting `properties`: %+v", err) - } - d.Set("has_immutability_policy", props.HasImmutabilityPolicy) d.Set("has_legal_hold", props.HasLegalHold) diff --git a/azurerm/internal/services/storage/registration.go b/azurerm/internal/services/storage/registration.go index 607172ec66e32..bdf608f1635cf 100644 --- a/azurerm/internal/services/storage/registration.go +++ b/azurerm/internal/services/storage/registration.go @@ -17,8 +17,8 @@ func (r Registration) SupportedDataSources() map[string]*schema.Resource { "azurerm_storage_account_blob_container_sas": dataSourceArmStorageAccountBlobContainerSharedAccessSignature(), "azurerm_storage_account_sas": dataSourceArmStorageAccountSharedAccessSignature(), "azurerm_storage_account": dataSourceArmStorageAccount(), - "azurerm_storage_management_policy": dataSourceArmStorageManagementPolicy(), "azurerm_storage_container": dataSourceArmStorageContainer(), + "azurerm_storage_management_policy": dataSourceArmStorageManagementPolicy(), } } diff --git a/azurerm/internal/services/storage/tests/data_source_storage_container_test.go b/azurerm/internal/services/storage/tests/data_source_storage_container_test.go index 08a913ad90c9c..373fb510d228d 100644 --- a/azurerm/internal/services/storage/tests/data_source_storage_container_test.go +++ b/azurerm/internal/services/storage/tests/data_source_storage_container_test.go @@ -21,9 +21,7 @@ func TestAccDataSourceArmStorageContainer_basic(t *testing.T) { resource.TestCheckResourceAttr(data.ResourceName, "name", "containerdstest-"+data.RandomString), resource.TestCheckResourceAttr(data.ResourceName, "container_access_type", "private"), resource.TestCheckResourceAttr(data.ResourceName, "has_immutability_policy", "false"), - resource.TestCheckResourceAttr(data.ResourceName, "resource_group_name", "containerdstest-"+data.RandomString), resource.TestCheckResourceAttr(data.ResourceName, "storage_account_name", "acctestsadsc"+data.RandomString), - resource.TestCheckResourceAttr(data.ResourceName, "properties.%", "4"), resource.TestCheckResourceAttr(data.ResourceName, "metadata.%", "2"), resource.TestCheckResourceAttr(data.ResourceName, "metadata.k1", "v1"), resource.TestCheckResourceAttr(data.ResourceName, "metadata.k2", "v2"), @@ -61,7 +59,8 @@ resource "azurerm_storage_container" "test" { } data "azurerm_storage_container" "test" { - storage_container_id = "${azurerm_storage_container.test.id}" + name = azurerm_storage_container.test.name + storage_account_name = azurerm_storage_container.test.storage_account_name } `, data.RandomString, data.Locations.Primary, data.RandomString, data.RandomString) diff --git a/website/azurerm.erb b/website/azurerm.erb index 2ef5553f4c33c..927f16492c339 100644 --- a/website/azurerm.erb +++ b/website/azurerm.erb @@ -463,7 +463,7 @@
  • - storage_account_container + storage_container
  • diff --git a/website/docs/d/storage_account_container.html.markdown b/website/docs/d/storage_container.html.markdown similarity index 65% rename from website/docs/d/storage_account_container.html.markdown rename to website/docs/d/storage_container.html.markdown index 70280afa0c3a9..ff219aed1a48f 100644 --- a/website/docs/d/storage_account_container.html.markdown +++ b/website/docs/d/storage_container.html.markdown @@ -13,20 +13,9 @@ Use this data source to access information about an existing Storage Container. ## Example Usage ```hcl - -resource "azurerm_storage_container" "test" { - name = "containerdstest-%s" - resource_group_name = "${azurerm_resource_group.test.name}" - storage_account_name = "${azurerm_storage_account.test.name}" - container_access_type = "private" - metadata = { - key1 = "value1" - key2 = "value2" - } -} - data "azurerm_storage_container" "example" { - storage_container_id = "${azurerm_storage_container.test.id}" + name = "example-container-name" + storage_account_name = "example-storage-account-name" } ``` @@ -34,7 +23,8 @@ data "azurerm_storage_container" "example" { The following arguments are supported: -* `storage_container_id` - (Required) Specifies the id of the storage container. +* `name` - (Required) The name of the Container. +* `storage_account_name` - (Required) The name of the Storage Account where the Container was created. ## Attributes Reference