Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

provider/azurerm: add account_kind and access_tier to storage_account #9408

Merged
merged 1 commit into from
Oct 24, 2016

Conversation

pmcatominey
Copy link
Contributor

TF_ACC=1 go test ./builtin/providers/azurerm -v -run TestAccAzureRMStorageAccount -timeout 120m
=== RUN   TestAccAzureRMStorageAccount_importBasic
--- PASS: TestAccAzureRMStorageAccount_importBasic (140.06s)
=== RUN   TestAccAzureRMStorageAccount_basic
--- PASS: TestAccAzureRMStorageAccount_basic (155.43s)
=== RUN   TestAccAzureRMStorageAccount_disappears
--- PASS: TestAccAzureRMStorageAccount_disappears (134.99s)
=== RUN   TestAccAzureRMStorageAccount_blobEncryption
--- PASS: TestAccAzureRMStorageAccount_blobEncryption (161.59s)
=== RUN   TestAccAzureRMStorageAccount_blobStorageWithUpdate
--- PASS: TestAccAzureRMStorageAccount_blobStorageWithUpdate (131.49s)
PASS
ok      github.com/hashicorp/terraform/builtin/providers/azurerm    723.694s

… resource

TF_ACC=1 go test ./builtin/providers/azurerm -v -run TestAccAzureRMStorageAccount -timeout 120m
=== RUN   TestAccAzureRMStorageAccount_importBasic
--- PASS: TestAccAzureRMStorageAccount_importBasic (140.06s)
=== RUN   TestAccAzureRMStorageAccount_basic
--- PASS: TestAccAzureRMStorageAccount_basic (155.43s)
=== RUN   TestAccAzureRMStorageAccount_disappears
--- PASS: TestAccAzureRMStorageAccount_disappears (134.99s)
=== RUN   TestAccAzureRMStorageAccount_blobEncryption
--- PASS: TestAccAzureRMStorageAccount_blobEncryption (161.59s)
=== RUN   TestAccAzureRMStorageAccount_blobStorageWithUpdate
--- PASS: TestAccAzureRMStorageAccount_blobStorageWithUpdate (131.49s)
PASS
ok  	github.com/hashicorp/terraform/builtin/providers/azurerm	723.694s
Copy link
Contributor

@stack72 stack72 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi @pmcatominey

Just left a few questions within :)

P

@@ -153,6 +181,17 @@ func resourceArmStorageAccountCreate(d *schema.ResourceData, meta interface{}) e
},
}

// AccessTier is only valid for BlobStorage accounts
if accountKind == string(storage.BlobStorage) {
accessTier, ok := d.GetOk("access_tier")
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we should add the default as part of the scheme declaration rather than defaulting in code

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The issue I faced is that accessTier can only be specified on requests for BlobStorage accounts, using the Default parameter meant it would be set for all types

d.Set("account_type", resp.Sku.Name)
d.Set("primary_location", resp.Properties.PrimaryLocation)
d.Set("secondary_location", resp.Properties.SecondaryLocation)

if resp.Properties.AccessTier != "" {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If access tier is computed: true, is there ever any way this can be nil?

@stack72
Copy link
Contributor

stack72 commented Oct 24, 2016

Thanks for the responses - LGTM :)

@ghost
Copy link

ghost commented Apr 21, 2020

I'm going to lock this issue because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active issues.

If you have found a problem that seems similar to this, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further.

@ghost ghost locked and limited conversation to collaborators Apr 21, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants