From 02ffea40045246e56a69b808aeae8140afb345f9 Mon Sep 17 00:00:00 2001 From: Alex Goncharov Date: Fri, 10 Jul 2020 11:12:03 +0200 Subject: [PATCH 1/5] WIP: add allow_public_access property for storage account --- .../storage/data_source_storage_account.go | 6 +++++ .../storage/resource_arm_storage_account.go | 22 +++++++++++++++++++ 2 files changed, 28 insertions(+) diff --git a/azurerm/internal/services/storage/data_source_storage_account.go b/azurerm/internal/services/storage/data_source_storage_account.go index 6be24f9baa03..6fef5d4f6963 100644 --- a/azurerm/internal/services/storage/data_source_storage_account.go +++ b/azurerm/internal/services/storage/data_source_storage_account.go @@ -67,6 +67,11 @@ func dataSourceArmStorageAccount() *schema.Resource { }, }, + "allow_public_access": { + Type: schema.TypeBool, + Computed: true, + }, + "enable_https_traffic_only": { Type: schema.TypeBool, Computed: true, @@ -305,6 +310,7 @@ func dataSourceArmStorageAccountRead(d *schema.ResourceData, meta interface{}) e if props := resp.AccountProperties; props != nil { d.Set("access_tier", props.AccessTier) + d.Set("allow_public_access", props.AllowBlobPublicAccess) d.Set("enable_https_traffic_only", props.EnableHTTPSTrafficOnly) d.Set("is_hns_enabled", props.IsHnsEnabled) diff --git a/azurerm/internal/services/storage/resource_arm_storage_account.go b/azurerm/internal/services/storage/resource_arm_storage_account.go index 9725b253aa92..f545919bb1cc 100644 --- a/azurerm/internal/services/storage/resource_arm_storage_account.go +++ b/azurerm/internal/services/storage/resource_arm_storage_account.go @@ -132,6 +132,12 @@ func resourceArmStorageAccount() *schema.Resource { }, }, + "allow_public_access": { + Type: schema.TypeBool, + Optional: true, + Default: true, + }, + "enable_https_traffic_only": { Type: schema.TypeBool, Optional: true, @@ -610,6 +616,7 @@ func resourceArmStorageAccountCreate(d *schema.ResourceData, meta interface{}) e location := azure.NormalizeLocation(d.Get("location").(string)) t := d.Get("tags").(map[string]interface{}) enableHTTPSTrafficOnly := d.Get("enable_https_traffic_only").(bool) + allowBlobPublicAccess := d.Get("allow_public_access").(bool) isHnsEnabled := d.Get("is_hns_enabled").(bool) accountTier := d.Get("account_tier").(string) @@ -860,6 +867,20 @@ func resourceArmStorageAccountUpdate(d *schema.ResourceData, meta interface{}) e } } + if d.HasChange("allow_public_access") { + allowBlobPublicAccess := d.Get("allow_public_access").(bool) + + opts := storage.AccountUpdateParameters{ + AccountPropertiesUpdateParameters: &storage.AccountPropertiesUpdateParameters{ + AllowBlobPublicAccess: &allowBlobPublicAccess, + }, + } + + if _, err := client.Update(ctx, resourceGroupName, storageAccountName, opts); err != nil { + return fmt.Errorf("Error updating Azure Storage Account allow_public_access %q: %+v", storageAccountName, err) + } + } + if d.HasChange("identity") { opts := storage.AccountUpdateParameters{ Identity: expandAzureRmStorageAccountIdentity(d), @@ -1013,6 +1034,7 @@ func resourceArmStorageAccountRead(d *schema.ResourceData, meta interface{}) err if props := resp.AccountProperties; props != nil { d.Set("access_tier", props.AccessTier) d.Set("enable_https_traffic_only", props.EnableHTTPSTrafficOnly) + d.Set("allow_public_access", props.AllowBlobPublicAccess) d.Set("is_hns_enabled", props.IsHnsEnabled) if customDomain := props.CustomDomain; customDomain != nil { From 3f30409c55c4f792e04c618c5e2b5d701f70c5f7 Mon Sep 17 00:00:00 2001 From: Alex Goncharov Date: Sat, 11 Jul 2020 11:52:33 +0200 Subject: [PATCH 2/5] add tests, allow_public_access -> allow_blob_public_access --- .../storage/data_source_storage_account.go | 4 +- .../storage/resource_arm_storage_account.go | 12 +-- .../resource_arm_storage_account_test.go | 81 +++++++++++++++++++ 3 files changed, 89 insertions(+), 8 deletions(-) diff --git a/azurerm/internal/services/storage/data_source_storage_account.go b/azurerm/internal/services/storage/data_source_storage_account.go index 6fef5d4f6963..6ffa1c3798cc 100644 --- a/azurerm/internal/services/storage/data_source_storage_account.go +++ b/azurerm/internal/services/storage/data_source_storage_account.go @@ -67,7 +67,7 @@ func dataSourceArmStorageAccount() *schema.Resource { }, }, - "allow_public_access": { + "allow_blob_public_access": { Type: schema.TypeBool, Computed: true, }, @@ -310,7 +310,7 @@ func dataSourceArmStorageAccountRead(d *schema.ResourceData, meta interface{}) e if props := resp.AccountProperties; props != nil { d.Set("access_tier", props.AccessTier) - d.Set("allow_public_access", props.AllowBlobPublicAccess) + d.Set("allow_blob_public_access", props.AllowBlobPublicAccess) d.Set("enable_https_traffic_only", props.EnableHTTPSTrafficOnly) d.Set("is_hns_enabled", props.IsHnsEnabled) diff --git a/azurerm/internal/services/storage/resource_arm_storage_account.go b/azurerm/internal/services/storage/resource_arm_storage_account.go index f545919bb1cc..53d8da326b4e 100644 --- a/azurerm/internal/services/storage/resource_arm_storage_account.go +++ b/azurerm/internal/services/storage/resource_arm_storage_account.go @@ -132,7 +132,7 @@ func resourceArmStorageAccount() *schema.Resource { }, }, - "allow_public_access": { + "allow_blob_public_access": { Type: schema.TypeBool, Optional: true, Default: true, @@ -616,7 +616,7 @@ func resourceArmStorageAccountCreate(d *schema.ResourceData, meta interface{}) e location := azure.NormalizeLocation(d.Get("location").(string)) t := d.Get("tags").(map[string]interface{}) enableHTTPSTrafficOnly := d.Get("enable_https_traffic_only").(bool) - allowBlobPublicAccess := d.Get("allow_public_access").(bool) + allowBlobPublicAccess := d.Get("allow_blob_public_access").(bool) isHnsEnabled := d.Get("is_hns_enabled").(bool) accountTier := d.Get("account_tier").(string) @@ -867,8 +867,8 @@ func resourceArmStorageAccountUpdate(d *schema.ResourceData, meta interface{}) e } } - if d.HasChange("allow_public_access") { - allowBlobPublicAccess := d.Get("allow_public_access").(bool) + if d.HasChange("allow_blob_public_access") { + allowBlobPublicAccess := d.Get("allow_blob_public_access").(bool) opts := storage.AccountUpdateParameters{ AccountPropertiesUpdateParameters: &storage.AccountPropertiesUpdateParameters{ @@ -877,7 +877,7 @@ func resourceArmStorageAccountUpdate(d *schema.ResourceData, meta interface{}) e } if _, err := client.Update(ctx, resourceGroupName, storageAccountName, opts); err != nil { - return fmt.Errorf("Error updating Azure Storage Account allow_public_access %q: %+v", storageAccountName, err) + return fmt.Errorf("Error updating Azure Storage Account allow_blob_public_access %q: %+v", storageAccountName, err) } } @@ -1034,7 +1034,7 @@ func resourceArmStorageAccountRead(d *schema.ResourceData, meta interface{}) err if props := resp.AccountProperties; props != nil { d.Set("access_tier", props.AccessTier) d.Set("enable_https_traffic_only", props.EnableHTTPSTrafficOnly) - d.Set("allow_public_access", props.AllowBlobPublicAccess) + d.Set("allow_blob_public_access", props.AllowBlobPublicAccess) d.Set("is_hns_enabled", props.IsHnsEnabled) if customDomain := props.CustomDomain; customDomain != nil { diff --git a/azurerm/internal/services/storage/tests/resource_arm_storage_account_test.go b/azurerm/internal/services/storage/tests/resource_arm_storage_account_test.go index 82c4feffb0f1..a6228398a83f 100644 --- a/azurerm/internal/services/storage/tests/resource_arm_storage_account_test.go +++ b/azurerm/internal/services/storage/tests/resource_arm_storage_account_test.go @@ -207,6 +207,33 @@ func TestAccAzureRMStorageAccount_blobConnectionString(t *testing.T) { }) } +func TestAccAzureRMStorageAccount_allowBlobPublicAccess(t *testing.T) { + data := acceptance.BuildTestData(t, "azurerm_storage_account", "test") + + resource.ParallelTest(t, resource.TestCase{ + PreCheck: func() { acceptance.PreCheck(t) }, + Providers: acceptance.SupportedProviders, + CheckDestroy: testCheckAzureRMStorageAccountDestroy, + Steps: []resource.TestStep{ + { + Config: testAccAzureRMStorageAccount_allowBlobPublicAccess(data), + Check: resource.ComposeTestCheckFunc( + testCheckAzureRMStorageAccountExists(data.ResourceName), + resource.TestCheckResourceAttr(data.ResourceName, "allow_blob_public_access", "true"), + ), + }, + data.ImportStep(), + { + Config: testAccAzureRMStorageAccount_allowBlobPublicAccessDisabled(data), + Check: resource.ComposeTestCheckFunc( + testCheckAzureRMStorageAccountExists(data.ResourceName), + resource.TestCheckResourceAttr(data.ResourceName, "allow_blob_public_access", "false"), + ), + }, + }, + }) +} + func TestAccAzureRMStorageAccount_enableHttpsTrafficOnly(t *testing.T) { data := acceptance.BuildTestData(t, "azurerm_storage_account", "test") @@ -888,6 +915,60 @@ resource "azurerm_storage_account" "test" { `, data.RandomInteger, data.Locations.Primary, data.RandomString) } +func testAccAzureRMStorageAccount_allowBlobPublicAccess(data acceptance.TestData) string { + return fmt.Sprintf(` +provider "azurerm" { + features {} +} + +resource "azurerm_resource_group" "test" { + name = "acctestRG-storage-%d" + location = "%s" +} + +resource "azurerm_storage_account" "test" { + name = "unlikely23exst2acct%s" + resource_group_name = azurerm_resource_group.test.name + + location = azurerm_resource_group.test.location + account_tier = "Standard" + account_replication_type = "LRS" + allow_blob_public_access = true + + tags = { + environment = "production" + } +} +`, data.RandomInteger, data.Locations.Primary, data.RandomString) +} + +func testAccAzureRMStorageAccount_allowBlobPublicAccessDisabled(data acceptance.TestData) string { + return fmt.Sprintf(` +provider "azurerm" { + features {} +} + +resource "azurerm_resource_group" "test" { + name = "acctestRG-storage-%d" + location = "%s" +} + +resource "azurerm_storage_account" "test" { + name = "unlikely23exst2acct%s" + resource_group_name = azurerm_resource_group.test.name + + location = azurerm_resource_group.test.location + account_tier = "Standard" + account_replication_type = "LRS" + allow_blob_public_access = false + + tags = { + environment = "production" + } +} +`, data.RandomInteger, data.Locations.Primary, data.RandomString) +} + func testAccAzureRMStorageAccount_enableHttpsTrafficOnly(data acceptance.TestData) string { return fmt.Sprintf(` provider "azurerm" { From 4185b49d9a180011a57ef2b14b818dbc6b3aaaf8 Mon Sep 17 00:00:00 2001 From: Alex Goncharov Date: Sat, 11 Jul 2020 12:04:25 +0200 Subject: [PATCH 3/5] fmt --- .../internal/services/storage/resource_arm_storage_account.go | 1 + 1 file changed, 1 insertion(+) diff --git a/azurerm/internal/services/storage/resource_arm_storage_account.go b/azurerm/internal/services/storage/resource_arm_storage_account.go index 53d8da326b4e..d581d0228491 100644 --- a/azurerm/internal/services/storage/resource_arm_storage_account.go +++ b/azurerm/internal/services/storage/resource_arm_storage_account.go @@ -632,6 +632,7 @@ func resourceArmStorageAccountCreate(d *schema.ResourceData, meta interface{}) e Kind: storage.Kind(accountKind), AccountPropertiesCreateParameters: &storage.AccountPropertiesCreateParameters{ EnableHTTPSTrafficOnly: &enableHTTPSTrafficOnly, + AllowBlobPublicAccess: &allowBlobPublicAccess, NetworkRuleSet: expandStorageAccountNetworkRules(d), IsHnsEnabled: &isHnsEnabled, }, From 6df61d94f3b8c12549fc0bbf91c4ca3e4eec6240 Mon Sep 17 00:00:00 2001 From: Alex Goncharov Date: Sat, 11 Jul 2020 12:19:30 +0200 Subject: [PATCH 4/5] update docs --- website/docs/d/storage_account.html.markdown | 2 ++ website/docs/r/storage_account.html.markdown | 4 +++- 2 files changed, 5 insertions(+), 1 deletion(-) diff --git a/website/docs/d/storage_account.html.markdown b/website/docs/d/storage_account.html.markdown index f20cdc38a725..764588ac8df3 100644 --- a/website/docs/d/storage_account.html.markdown +++ b/website/docs/d/storage_account.html.markdown @@ -43,6 +43,8 @@ output "storage_account_tier" { * `access_tier` - The access tier for `BlobStorage` accounts. +* `allow_blob_public_access` - Can blobs in this storage account be configured for public access. + * `enable_https_traffic_only` - Is traffic only allowed via HTTPS? See [here](https://docs.microsoft.com/en-us/azure/storage/storage-require-secure-transfer/) for more information. diff --git a/website/docs/r/storage_account.html.markdown b/website/docs/r/storage_account.html.markdown index 1ed439955a85..f681a801b1de 100644 --- a/website/docs/r/storage_account.html.markdown +++ b/website/docs/r/storage_account.html.markdown @@ -94,10 +94,12 @@ The following arguments are supported: * `access_tier` - (Optional) Defines the access tier for `BlobStorage`, `FileStorage` and `StorageV2` accounts. Valid options are `Hot` and `Cool`, defaults to `Hot`. +* `allow_blob_public_access` = (Optional) Boolean flag wich disallows blobs to be confiured for public access. Enabling it does not allow public access to the blobs directly, each blob still need to be configured to allow public access. Setting this to false will remove all public access from the blobs does not matter per-blob configuration. Defaults to `true` + * `enable_https_traffic_only` - (Optional) Boolean flag which forces HTTPS if enabled, see [here](https://docs.microsoft.com/en-us/azure/storage/storage-require-secure-transfer/) for more information. Defaults to `true`. -* `is_hns_enabled` - (Optional) Is Hierarchical Namespace enabled? This can be used with Azure Data Lake Storage Gen 2 ([see here for more information](https://docs.microsoft.com/en-us/azure/storage/blobs/data-lake-storage-quickstart-create-account/)). Changing this forces a new resource to be created. +* `is_hns_enabled` - (Optional) Is Hierarchical Namespace enabled? This can be used with Azure Data Lake Storage Gen 2 ([see here for more information](https://docs.microsoft.com/en-us/azure/storage/blobs/data-lake-storage-quickstart-create-account/)). Changing this forces a new resource to be created. -> **NOTE:** When this is set to `true` the `account_tier` argument must be set to `Standard` From e0512ef3596e4efa3654734367b513491a69b039 Mon Sep 17 00:00:00 2001 From: Alex Goncharov Date: Mon, 13 Jul 2020 09:58:51 +0200 Subject: [PATCH 5/5] fix formatting and doc typos --- .../tests/resource_arm_storage_account_test.go | 16 ++++++++-------- website/docs/r/storage_account.html.markdown | 2 +- 2 files changed, 9 insertions(+), 9 deletions(-) diff --git a/azurerm/internal/services/storage/tests/resource_arm_storage_account_test.go b/azurerm/internal/services/storage/tests/resource_arm_storage_account_test.go index a6228398a83f..7281ca6271dd 100644 --- a/azurerm/internal/services/storage/tests/resource_arm_storage_account_test.go +++ b/azurerm/internal/services/storage/tests/resource_arm_storage_account_test.go @@ -930,10 +930,10 @@ resource "azurerm_storage_account" "test" { name = "unlikely23exst2acct%s" resource_group_name = azurerm_resource_group.test.name - location = azurerm_resource_group.test.location - account_tier = "Standard" - account_replication_type = "LRS" - allow_blob_public_access = true + location = azurerm_resource_group.test.location + account_tier = "Standard" + account_replication_type = "LRS" + allow_blob_public_access = true tags = { environment = "production" @@ -957,10 +957,10 @@ resource "azurerm_storage_account" "test" { name = "unlikely23exst2acct%s" resource_group_name = azurerm_resource_group.test.name - location = azurerm_resource_group.test.location - account_tier = "Standard" - account_replication_type = "LRS" - allow_blob_public_access = false + location = azurerm_resource_group.test.location + account_tier = "Standard" + account_replication_type = "LRS" + allow_blob_public_access = false tags = { environment = "production" diff --git a/website/docs/r/storage_account.html.markdown b/website/docs/r/storage_account.html.markdown index f681a801b1de..32d7729f5cd0 100644 --- a/website/docs/r/storage_account.html.markdown +++ b/website/docs/r/storage_account.html.markdown @@ -94,7 +94,7 @@ The following arguments are supported: * `access_tier` - (Optional) Defines the access tier for `BlobStorage`, `FileStorage` and `StorageV2` accounts. Valid options are `Hot` and `Cool`, defaults to `Hot`. -* `allow_blob_public_access` = (Optional) Boolean flag wich disallows blobs to be confiured for public access. Enabling it does not allow public access to the blobs directly, each blob still need to be configured to allow public access. Setting this to false will remove all public access from the blobs does not matter per-blob configuration. Defaults to `true` +* `allow_blob_public_access` = (Optional) Boolean flag which disallows blobs to be confiured for public access. Enabling it does not allow public access to the blobs directly, each blob still need to be configured to allow public access. Setting this to false will remove all public access from the blobs does not matter per-blob configuration. Defaults to `true` * `enable_https_traffic_only` - (Optional) Boolean flag which forces HTTPS if enabled, see [here](https://docs.microsoft.com/en-us/azure/storage/storage-require-secure-transfer/) for more information. Defaults to `true`.