Skip to content

Commit

Permalink
address review comments
Browse files Browse the repository at this point in the history
  • Loading branch information
minzhang28 committed Jan 14, 2020
1 parent 0a494b1 commit 580e06d
Show file tree
Hide file tree
Showing 6 changed files with 25 additions and 66 deletions.
5 changes: 0 additions & 5 deletions azurerm/internal/services/storage/client/client.go
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,6 @@ type Client struct {
FileSystemsClient *filesystems.Client
ManagementPoliciesClient storage.ManagementPoliciesClient
BlobServicesClient storage.BlobServicesClient
ContainerClient storage.BlobContainersClient
environment az.Environment
}

Expand All @@ -41,17 +40,13 @@ 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{
AccountsClient: &accountsClient,
FileSystemsClient: &fileSystemsClient,
ManagementPoliciesClient: managementPoliciesClient,
BlobServicesClient: blobServicesClient,
ContainerClient: containerClient,
environment: options.Environment,
}
}
Expand Down
59 changes: 17 additions & 42 deletions azurerm/internal/services/storage/data_source_storage_container.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand All @@ -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": {
Expand All @@ -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,
},
},
},
}
}
Expand All @@ -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)

Expand All @@ -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)

Expand Down
2 changes: 1 addition & 1 deletion azurerm/internal/services/storage/registration.go
Original file line number Diff line number Diff line change
Expand Up @@ -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(),
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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"),
Expand Down Expand Up @@ -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)
Expand Down
2 changes: 1 addition & 1 deletion website/azurerm.erb
Original file line number Diff line number Diff line change
Expand Up @@ -463,7 +463,7 @@
</li>

<li>
<a href="/docs/providers/azurerm/d/storage_account_container.html">storage_account_container</a>
<a href="/docs/providers/azurerm/d/storage_container.html">storage_container</a>
</li>

<li>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,28 +13,18 @@ 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"
}
```

## Argument Reference

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

Expand Down

0 comments on commit 580e06d

Please sign in to comment.