From ef24004fcf838ea2e0958e69a5881f9c58f752b0 Mon Sep 17 00:00:00 2001 From: Kyriakos Oikonomakos Date: Thu, 22 Jul 2021 10:51:20 +0100 Subject: [PATCH 1/6] Handle nil values for AllowBlobPublicAccess --- .../storage/storage_account_resource.go | 28 +++++++++++++++++-- 1 file changed, 26 insertions(+), 2 deletions(-) diff --git a/internal/services/storage/storage_account_resource.go b/internal/services/storage/storage_account_resource.go index 2f327cb0b322..8e06f3554d95 100644 --- a/internal/services/storage/storage_account_resource.go +++ b/internal/services/storage/storage_account_resource.go @@ -3,6 +3,7 @@ package storage import ( "context" "fmt" + "github.com/terraform-providers/terraform-provider-azurerm/azurerm/internal/features" "log" "net/http" "strings" @@ -239,7 +240,7 @@ func resourceStorageAccount() *pluginsdk.Resource { "allow_blob_public_access": { Type: pluginsdk.TypeBool, Optional: true, - Default: false, + Default: getDefaultAllowBlobPublicAccess(), }, "shared_access_key_enabled": { @@ -1579,7 +1580,21 @@ func resourceStorageAccountRead(d *pluginsdk.ResourceData, meta interface{}) err d.Set("enable_https_traffic_only", props.EnableHTTPSTrafficOnly) d.Set("is_hns_enabled", props.IsHnsEnabled) d.Set("nfsv3_enabled", props.EnableNfsV3) - d.Set("allow_blob_public_access", props.AllowBlobPublicAccess) + + if features.ThreePointOh() { + // There is a certain edge case that could result in the Azure API returning a null value for AllowBLobPublicAccess. + // Since the field is a pointer, this gets marshalled to a nil value instead of a boolean. Here we set this + // to the default value, but since this is a breaking change we'll enforce this in 3.0 + + allowBlobPublicAccess := getDefaultAllowBlobPublicAccess() + if props.AllowBlobPublicAccess != nil { + allowBlobPublicAccess = *props.AllowBlobPublicAccess + } + d.Set("allow_blob_public_access", allowBlobPublicAccess) + } else { + d.Set("allow_blob_public_access", *props.AllowBlobPublicAccess) + } + // For all Clouds except Public, China, and USGovernmentCloud, "min_tls_version" is not returned from Azure so always persist the default values for "min_tls_version". // https://github.com/hashicorp/terraform-provider-azurerm/issues/7812 // https://github.com/hashicorp/terraform-provider-azurerm/issues/8083 @@ -2920,3 +2935,12 @@ func setEndpointAndHost(d *pluginsdk.ResourceData, ordinalString string, endpoin d.Set(fmt.Sprintf("%s_%s_host", ordinalString, typeString), host) return nil } + +func getDefaultAllowBlobPublicAccess() bool { + // The default value for the field that controls if the blobs that belong to a storage account + // can allow anonymous access or not will change from false to true in 3.0. + if features.ThreePointOh() { + return true + } + return false +} From 8346cfdd3580dc830bc173a9d0adfa9e743fc1bc Mon Sep 17 00:00:00 2001 From: Kyriakos Oikonomakos Date: Thu, 22 Jul 2021 15:42:16 +0100 Subject: [PATCH 2/6] address linter issues --- internal/services/storage/storage_account_resource.go | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/internal/services/storage/storage_account_resource.go b/internal/services/storage/storage_account_resource.go index 8e06f3554d95..8748010c5c7d 100644 --- a/internal/services/storage/storage_account_resource.go +++ b/internal/services/storage/storage_account_resource.go @@ -3,7 +3,6 @@ package storage import ( "context" "fmt" - "github.com/terraform-providers/terraform-provider-azurerm/azurerm/internal/features" "log" "net/http" "strings" @@ -17,6 +16,7 @@ import ( "github.com/hashicorp/terraform-provider-azurerm/helpers/azure" "github.com/hashicorp/terraform-provider-azurerm/helpers/tf" "github.com/hashicorp/terraform-provider-azurerm/internal/clients" + "github.com/hashicorp/terraform-provider-azurerm/internal/features" "github.com/hashicorp/terraform-provider-azurerm/internal/locks" msiparse "github.com/hashicorp/terraform-provider-azurerm/internal/services/msi/parse" msiValidate "github.com/hashicorp/terraform-provider-azurerm/internal/services/msi/validate" @@ -2939,8 +2939,5 @@ func setEndpointAndHost(d *pluginsdk.ResourceData, ordinalString string, endpoin func getDefaultAllowBlobPublicAccess() bool { // The default value for the field that controls if the blobs that belong to a storage account // can allow anonymous access or not will change from false to true in 3.0. - if features.ThreePointOh() { - return true - } - return false + return features.ThreePointOh() } From 05f3ea084eee3895284cbcd8fd7f42b08d046a90 Mon Sep 17 00:00:00 2001 From: Kyriakos Oikonomakos Date: Tue, 24 Aug 2021 10:48:59 +0100 Subject: [PATCH 3/6] change attribute name --- .../storage/storage_account_resource.go | 24 ++++++++++++------- 1 file changed, 16 insertions(+), 8 deletions(-) diff --git a/internal/services/storage/storage_account_resource.go b/internal/services/storage/storage_account_resource.go index 8748010c5c7d..130970346232 100644 --- a/internal/services/storage/storage_account_resource.go +++ b/internal/services/storage/storage_account_resource.go @@ -35,6 +35,7 @@ import ( ) var storageAccountResourceName = "azurerm_storage_account" +var allowPublicNestedItemsName = getDefaultAllowBlobPublicAccessName() func resourceStorageAccount() *pluginsdk.Resource { return &pluginsdk.Resource{ @@ -237,7 +238,7 @@ func resourceStorageAccount() *pluginsdk.Resource { ForceNew: true, }, - "allow_blob_public_access": { + allowPublicNestedItemsName: { Type: pluginsdk.TypeBool, Optional: true, Default: getDefaultAllowBlobPublicAccess(), @@ -922,7 +923,7 @@ func resourceStorageAccountCreate(d *pluginsdk.ResourceData, meta interface{}) e minimumTLSVersion := d.Get("min_tls_version").(string) isHnsEnabled := d.Get("is_hns_enabled").(bool) nfsV3Enabled := d.Get("nfsv3_enabled").(bool) - allowBlobPublicAccess := d.Get("allow_blob_public_access").(bool) + allowBlobPublicAccess := d.Get(allowPublicNestedItemsName).(bool) allowSharedKeyAccess := d.Get("shared_access_key_enabled").(bool) accountTier := d.Get("account_tier").(string) @@ -952,7 +953,7 @@ func resourceStorageAccountCreate(d *pluginsdk.ResourceData, meta interface{}) e // https://github.com/hashicorp/terraform-provider-azurerm/issues/9128 if envName != autorestAzure.PublicCloud.Name && envName != autorestAzure.USGovernmentCloud.Name && envName != autorestAzure.ChinaCloud.Name { if allowBlobPublicAccess || minimumTLSVersion != string(storage.TLS10) { - return fmt.Errorf(`"allow_blob_public_access" and "min_tls_version" are not supported for a Storage Account located in %q`, envName) + return fmt.Errorf(`%q and "min_tls_version" are not supported for a Storage Account located in %q`, allowPublicNestedItemsName, envName) } } else { parameters.AccountPropertiesCreateParameters.AllowBlobPublicAccess = &allowBlobPublicAccess @@ -1300,8 +1301,8 @@ func resourceStorageAccountUpdate(d *pluginsdk.ResourceData, meta interface{}) e } } - if d.HasChange("allow_blob_public_access") { - allowBlobPublicAccess := d.Get("allow_blob_public_access").(bool) + if d.HasChange(allowPublicNestedItemsName) { + allowBlobPublicAccess := d.Get(allowPublicNestedItemsName).(bool) // For all Clouds except Public, China, and USGovernmentCloud, don't specify "allow_blob_public_access" in request body. // https://github.com/hashicorp/terraform-provider-azurerm/issues/7812 @@ -1309,7 +1310,7 @@ func resourceStorageAccountUpdate(d *pluginsdk.ResourceData, meta interface{}) e // https://github.com/hashicorp/terraform-provider-azurerm/issues/9128 if envName != autorestAzure.PublicCloud.Name && envName != autorestAzure.USGovernmentCloud.Name && envName != autorestAzure.ChinaCloud.Name { if allowBlobPublicAccess { - return fmt.Errorf(`"allow_blob_public_access" is not supported for a Storage Account located in %q`, envName) + return fmt.Errorf(`%q is not supported for a Storage Account located in %q`, allowPublicNestedItemsName, envName) } } else { opts := storage.AccountUpdateParameters{ @@ -1590,9 +1591,9 @@ func resourceStorageAccountRead(d *pluginsdk.ResourceData, meta interface{}) err if props.AllowBlobPublicAccess != nil { allowBlobPublicAccess = *props.AllowBlobPublicAccess } - d.Set("allow_blob_public_access", allowBlobPublicAccess) + d.Set(allowPublicNestedItemsName, allowBlobPublicAccess) } else { - d.Set("allow_blob_public_access", *props.AllowBlobPublicAccess) + d.Set(allowPublicNestedItemsName, *props.AllowBlobPublicAccess) } // For all Clouds except Public, China, and USGovernmentCloud, "min_tls_version" is not returned from Azure so always persist the default values for "min_tls_version". @@ -2941,3 +2942,10 @@ func getDefaultAllowBlobPublicAccess() bool { // can allow anonymous access or not will change from false to true in 3.0. return features.ThreePointOh() } + +func getDefaultAllowBlobPublicAccessName() string { + if features.ThreePointOh() { + return "allow_nested_items_to_be_public" + } + return "allow_blob_public_access" +} From bb4b64a23837c34aa78e77aae14a9887e6b54caf Mon Sep 17 00:00:00 2001 From: Kyriakos Oikonomakos Date: Wed, 25 Aug 2021 11:33:32 +0100 Subject: [PATCH 4/6] Apply suggestions from code review Co-authored-by: Tom Harvey --- internal/services/storage/storage_account_resource.go | 1 + 1 file changed, 1 insertion(+) diff --git a/internal/services/storage/storage_account_resource.go b/internal/services/storage/storage_account_resource.go index 130970346232..9a05eb702bc5 100644 --- a/internal/services/storage/storage_account_resource.go +++ b/internal/services/storage/storage_account_resource.go @@ -238,6 +238,7 @@ func resourceStorageAccount() *pluginsdk.Resource { ForceNew: true, }, + // TODO: document this new field in 3.0 allowPublicNestedItemsName: { Type: pluginsdk.TypeBool, Optional: true, From a7f32bb62b74b87238673c7e244e686faf23fd60 Mon Sep 17 00:00:00 2001 From: Kyriakos Oikonomakos Date: Wed, 25 Aug 2021 11:48:57 +0100 Subject: [PATCH 5/6] add lintignore --- internal/services/storage/storage_account_resource.go | 2 ++ 1 file changed, 2 insertions(+) diff --git a/internal/services/storage/storage_account_resource.go b/internal/services/storage/storage_account_resource.go index 9a05eb702bc5..e1687b334058 100644 --- a/internal/services/storage/storage_account_resource.go +++ b/internal/services/storage/storage_account_resource.go @@ -1592,8 +1592,10 @@ func resourceStorageAccountRead(d *pluginsdk.ResourceData, meta interface{}) err if props.AllowBlobPublicAccess != nil { allowBlobPublicAccess = *props.AllowBlobPublicAccess } + //lintignore:R001 d.Set(allowPublicNestedItemsName, allowBlobPublicAccess) } else { + //lintignore:R001,R002 d.Set(allowPublicNestedItemsName, *props.AllowBlobPublicAccess) } From 226c812eb3f3bcb58ac3000fa8fbaba9fb4f7d6f Mon Sep 17 00:00:00 2001 From: Kyriakos Oikonomakos Date: Tue, 31 Aug 2021 16:54:00 +0100 Subject: [PATCH 6/6] don't de-reference pointer --- internal/services/storage/storage_account_resource.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/internal/services/storage/storage_account_resource.go b/internal/services/storage/storage_account_resource.go index e1687b334058..2d4ae97ed717 100644 --- a/internal/services/storage/storage_account_resource.go +++ b/internal/services/storage/storage_account_resource.go @@ -1595,8 +1595,8 @@ func resourceStorageAccountRead(d *pluginsdk.ResourceData, meta interface{}) err //lintignore:R001 d.Set(allowPublicNestedItemsName, allowBlobPublicAccess) } else { - //lintignore:R001,R002 - d.Set(allowPublicNestedItemsName, *props.AllowBlobPublicAccess) + //lintignore:R001 + d.Set(allowPublicNestedItemsName, props.AllowBlobPublicAccess) } // For all Clouds except Public, China, and USGovernmentCloud, "min_tls_version" is not returned from Azure so always persist the default values for "min_tls_version".